From: Ben Pfaff <blp@nicira.com>
Date: Tue, 11 Nov 2008 22:08:41 +0000 (-0800)
Subject: Orphan and clone packets transmitted on dp_dev (proper fix for bug #478).
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c8833cd64280d5562becaa73e0acaee85d4c1be2;p=openvswitch

Orphan and clone packets transmitted on dp_dev (proper fix for bug #478).

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.
---

diff --git a/datapath/dp_dev.c b/datapath/dp_dev.c
index 9bf4bf53..7a726c39 100644
--- a/datapath/dp_dev.c
+++ b/datapath/dp_dev.c
@@ -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;