From b7a31ec13d0617868378d39a72beb4c4ffcb7e5c Mon Sep 17 00:00:00 2001 From: Jesse Gross Date: Sun, 29 Aug 2010 14:28:58 -0700 Subject: [PATCH] datapath: Move is_frag out of struct ovs_skb_cb. is_frag is only used for communication between two functions, which means that it doesn't really need to be in the SKB CB. This wouldn't necessarily be a problem except that there are also a number of other paths that lead to this being uninitialized. This isn't a problem now but uninitialized memory seems dangerous and there isn't much upside. Signed-off-by: Jesse Gross Reviewed-by: Ben Pfaff --- datapath/datapath.c | 8 +++++--- datapath/datapath.h | 2 -- datapath/flow.c | 14 +++++++------- datapath/flow.h | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/datapath/datapath.c b/datapath/datapath.c index 390acc8a4..5996d6ed7 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -552,15 +552,16 @@ void dp_process_received_packet(struct dp_port *p, struct sk_buff *skb) if (!OVS_CB(skb)->flow) { struct odp_flow_key key; struct tbl_node *flow_node; + bool is_frag; /* Extract flow from 'skb' into 'key'. */ - error = flow_extract(skb, p ? p->port_no : ODPP_NONE, &key); + error = flow_extract(skb, p ? p->port_no : ODPP_NONE, &key, &is_frag); if (unlikely(error)) { kfree_skb(skb); return; } - if (OVS_CB(skb)->is_frag && dp->drop_frags) { + if (is_frag && dp->drop_frags) { kfree_skb(skb); stats_counter_off = offsetof(struct dp_stats_percpu, n_frags); goto out; @@ -1325,6 +1326,7 @@ static int do_execute(struct datapath *dp, const struct odp_execute *execute) struct sk_buff *skb; struct sw_flow_actions *actions; struct ethhdr *eth; + bool is_frag; int err; err = -EINVAL; @@ -1372,7 +1374,7 @@ static int do_execute(struct datapath *dp, const struct odp_execute *execute) else skb->protocol = htons(ETH_P_802_2); - err = flow_extract(skb, execute->in_port, &key); + err = flow_extract(skb, execute->in_port, &key, &is_frag); if (err) goto error_free_skb; diff --git a/datapath/datapath.h b/datapath/datapath.h index dacc3a42c..f28513bb7 100644 --- a/datapath/datapath.h +++ b/datapath/datapath.h @@ -147,7 +147,6 @@ enum csum_type { * struct ovs_skb_cb - OVS data in skb CB * @dp_port: The datapath port on which the skb entered the switch. * @flow: The flow associated with this packet. May be %NULL if no flow. - * @is_frag: %true if this packet is an IPv4 fragment, %false otherwise. * @ip_summed: Consistently stores L4 checksumming status across different * kernel versions. * @tun_id: ID (in network byte order) of the tunnel that encapsulated this @@ -156,7 +155,6 @@ enum csum_type { struct ovs_skb_cb { struct dp_port *dp_port; struct sw_flow *flow; - bool is_frag; enum csum_type ip_summed; __be32 tun_id; }; diff --git a/datapath/flow.c b/datapath/flow.c index dfbf76938..1aa6e291b 100644 --- a/datapath/flow.c +++ b/datapath/flow.c @@ -267,7 +267,8 @@ static __be16 parse_ethertype(struct sk_buff *skb) * Sets OVS_CB(skb)->is_frag to %true if @skb is an IPv4 fragment, otherwise to * %false. */ -int flow_extract(struct sk_buff *skb, u16 in_port, struct odp_flow_key *key) +int flow_extract(struct sk_buff *skb, u16 in_port, struct odp_flow_key *key, + bool *is_frag) { struct ethhdr *eth; @@ -275,7 +276,7 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct odp_flow_key *key) key->tun_id = OVS_CB(skb)->tun_id; key->in_port = in_port; key->dl_vlan = htons(ODP_VLAN_NONE); - OVS_CB(skb)->is_frag = false; + *is_frag = false; /* * We would really like to pull as many bytes as we could possibly @@ -356,9 +357,9 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct odp_flow_key *key) key->tp_dst = htons(icmp->code); } } - } else { - OVS_CB(skb)->is_frag = true; - } + } else + *is_frag = true; + } else if (key->dl_type == htons(ETH_P_ARP) && arphdr_ok(skb)) { struct arp_eth_header *arp; @@ -370,9 +371,8 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct odp_flow_key *key) && arp->ar_pln == 4) { /* We only match on the lower 8 bits of the opcode. */ - if (ntohs(arp->ar_op) <= 0xff) { + if (ntohs(arp->ar_op) <= 0xff) key->nw_proto = ntohs(arp->ar_op); - } if (key->nw_proto == ARPOP_REQUEST || key->nw_proto == ARPOP_REPLY) { diff --git a/datapath/flow.h b/datapath/flow.h index 3f434677b..25b720449 100644 --- a/datapath/flow.h +++ b/datapath/flow.h @@ -74,7 +74,7 @@ void flow_deferred_free_acts(struct sw_flow_actions *); void flow_hold(struct sw_flow *); void flow_put(struct sw_flow *); -int flow_extract(struct sk_buff *, u16 in_port, struct odp_flow_key *); +int flow_extract(struct sk_buff *, u16 in_port, struct odp_flow_key *, bool *is_frag); void flow_used(struct sw_flow *, struct sk_buff *); u32 flow_hash(const struct odp_flow_key *key); -- 2.39.2