datapath: Better handle vlan packets sent to userspace.

We no longer clone packets that are sent via the userspace action
because placing them in Netlink attributes makes a copy so we
generally don't touch the original.  The one exception to this is
accelerated vlan tags, which are currently inserted into the
original packet as long as it isn't cloned.  Although the clone
check prevents us from causing problems for past packets it has
issues for future processing:

 * It turns accelerated tags into non-accelerated tags.  This isn't
   inherently a problem but some cards may not properly support
   offloads with in-band tags.
 * It doesn't update CHECKSUM_COMPLETE if there is one.
 * If the operation fails, it will free the packet resulting in a
   later use-after-free.

This patch fixes the above issues with a conservative approach.
It's possible to do it more efficiently but it probably doesn't
matter in most cases.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 54248e2e6fab35a907ef9ca901147f5d45f3f351..1ea5d1e9a7c2d850cf989b34c6a5f3e7b53d25ac 100644 (file)
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -437,17 +437,28 @@ static int queue_userspace_packet(int dp_ifindex, struct sk_buff *skb,
                                  const struct dp_upcall_info *upcall_info)
 {
        struct ovs_header *upcall;
+       struct sk_buff *nskb = NULL;
        struct sk_buff *user_skb; /* to be queued to userspace */
        struct nlattr *nla;
        unsigned int len;
        int err;
 
-       err = vlan_deaccel_tag(skb);
-       if (unlikely(err))
-               return err;
+       if (vlan_tx_tag_present(skb)) {
+               nskb = skb_clone(skb, GFP_ATOMIC);
+               if (!nskb)
+                       return -ENOMEM;
+               
+               err = vlan_deaccel_tag(nskb);
+               if (err)
+                       return err;
+
+               skb = nskb;
+       }
 
-       if (nla_attr_size(skb->len) > USHRT_MAX)
-               return -EFBIG;
+       if (nla_attr_size(skb->len) > USHRT_MAX) {
+               err = -EFBIG;
+               goto out;
+       }
 
        len = sizeof(struct ovs_header);
        len += nla_total_size(skb->len);
@@ -456,8 +467,10 @@ static int queue_userspace_packet(int dp_ifindex, struct sk_buff *skb,
                len += nla_total_size(8);
 
        user_skb = genlmsg_new(len, GFP_ATOMIC);
-       if (!user_skb)
-               return -ENOMEM;
+       if (!user_skb) {
+               err = -ENOMEM;
+               goto out;
+       }
 
        upcall = genlmsg_put(user_skb, 0, 0, &dp_packet_genl_family,
                             0, upcall_info->cmd);
@@ -475,7 +488,11 @@ static int queue_userspace_packet(int dp_ifindex, struct sk_buff *skb,
 
        skb_copy_and_csum_dev(skb, nla_data(nla));
 
-       return genlmsg_unicast(&init_net, user_skb, upcall_info->pid);
+       err = genlmsg_unicast(&init_net, user_skb, upcall_info->pid);
+
+out:
+       kfree_skb(nskb);
+       return err;
 }
 
 /* Called with genl_mutex. */