Orphan and clone packets transmitted on dp_dev (proper fix for bug #478).
authorBen Pfaff <blp@nicira.com>
Tue, 11 Nov 2008 22:08:41 +0000 (14:08 -0800)
committerBen Pfaff <blp@nicira.com>
Thu, 13 Nov 2008 20:38:44 +0000 (12:38 -0800)
We need to orphan packets transmitted on dp_dev so that any socket that
the packets came from is no longer charged for them.  If we don't do
this, then the sk_buff will be destructed eventually, but it is
harder to predict when.

We need to clone packets because we are sticking them on
&dp_dev->xmit_queue, which is done by modifying the sk_buff, which we
can only do if it is not shared.

datapath/dp_dev.c

index 9bf4bf53b811520aa67f37ba3a7a5db896d54e2f..7a726c39a3c7d4384e096578996f6ba5af7556bc 100644 (file)
@@ -57,23 +57,24 @@ static int dp_dev_mac_addr(struct net_device *dev, void *p)
        return 0;
 }
 
-static int dp_dev_xmit(struct sk_buff *oskb, struct net_device *netdev)
+static int dp_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
 {
        struct dp_dev *dp_dev = dp_dev_priv(netdev);
        struct datapath *dp = dp_dev->dp;
-       struct sk_buff *skb;
 
-       /* FIXME: doing a full copy here is far too expensive and most the time
-        * it is unnecessary.  However, it is a stopgap fix for bug #478. */
-       skb = skb_copy(oskb, GFP_ATOMIC);
-       skb->dev = oskb->dev;
-       kfree_skb(oskb);
-       if (!skb) {
-               if (net_ratelimit()) {
-                       printk("failed to copy skb destined for dp_dev\n");
-               }
+       /* By orphaning 'skb' we will screw up socket accounting slightly, but
+        * the effect is limited to the device queue length.  If we don't
+        * do this, then the sk_buff will be destructed eventually, but it is
+        * harder to predict when. */
+       skb_orphan(skb);
+
+       /* We are going to modify 'skb', by sticking it on &dp_dev->xmit_queue,
+        * so we need to have our own clone.  (At any rate, fwd_port_input()
+        * will need its own clone, so there's no benefit to queuing any other
+        * way.) */
+       skb = skb_share_check(skb, GFP_ATOMIC);
+       if (!skb)
                return 0;
-       }
 
        dp_dev->stats.tx_packets++;
        dp_dev->stats.tx_bytes += skb->len;