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 <jesse@nicira.com>
Reviewed-by: Ben Pfaff <blp@nicira.com>

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 390acc8a4e5f8a029fbf207b58c6eb45593aab3a..5996d6ed7e104b54fd9ec8370828372c06bb5ef3 100644 (file)
--- 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 dacc3a42c301c1c11d90d395f729c5006254f8fb..f28513bb7efe0ef3421045cd95ee59a864587689 100644 (file)
--- 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 dfbf76938e0c8f7d1979384b8b0adcb1048318f9..1aa6e291bf903f7fdb690cee1dadf19521d9df67 100644 (file)
--- 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 3f434677bdac956c6450650f8d5b866b4cdc132c..25b7204497bcea6580e60ce79bebdef191a39033 100644 (file)
--- 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);