netdev - [PATCH net-next 1/2] net: dsa: Remove deferred_xmit from dsa_skb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20191227014208.7189-2-olteanv@gmail.com>
Date:   Fri, 27 Dec 2019 03:42:07 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     f.fainelli@...il.com, vivien.didelot@...il.com, andrew@...n.ch,
        davem@...emloft.net
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Vladimir Oltean <olteanv@...il.com>
Subject: [PATCH net-next 1/2] net: dsa: Remove deferred_xmit from dsa_skb_cb

The introduction of the deferred xmit mechanism in DSA has made the
hotpath slightly more inefficient for everybody, since
DSA_SKB_CB(skb)->deferred_xmit needed to be initialized to false for
every transmitted frame, in order to figure out whether the driver
requested deferral or not. That was necessary to avoid kfree_skb from
freeing the skb.

But actually we can just remove that variable in the skb->cb and
counteract the effect of kfree_skb with skb_get, much to the same
effect. The advantage, of course, being that anybody who doesn't use
deferred xmit doesn't need to do any extra operation in the hotpath.

Signed-off-by: Vladimir Oltean <olteanv@...il.com>
---
 include/net/dsa.h |  1 -
 net/dsa/slave.c   | 12 ++++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 6767dc3f66c0..5d510a4da5d0 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -88,7 +88,6 @@ struct dsa_device_ops {
 
 struct dsa_skb_cb {
 	struct sk_buff *clone;
-	bool deferred_xmit;
 };
 
 struct __dsa_skb_cb {
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 78ffc87dc25e..9f7e47dcdc20 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -518,7 +518,6 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
 	s->tx_bytes += skb->len;
 	u64_stats_update_end(&s->syncp);
 
-	DSA_SKB_CB(skb)->deferred_xmit = false;
 	DSA_SKB_CB(skb)->clone = NULL;
 
 	/* Identify PTP protocol packets, clone them, and pass them to the
@@ -531,8 +530,7 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
 	 */
 	nskb = p->xmit(skb, dev);
 	if (!nskb) {
-		if (!DSA_SKB_CB(skb)->deferred_xmit)
-			kfree_skb(skb);
+		kfree_skb(skb);
 		return NETDEV_TX_OK;
 	}
 
@@ -543,10 +541,12 @@ void *dsa_defer_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 
-	DSA_SKB_CB(skb)->deferred_xmit = true;
-
-	skb_queue_tail(&dp->xmit_queue, skb);
+	/* Increase refcount so the kfree_skb in dsa_slave_xmit
+	 * won't really free the packet.
+	 */
+	skb_queue_tail(&dp->xmit_queue, skb_get(skb));
 	schedule_work(&dp->xmit_work);
+
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(dsa_defer_xmit);
-- 
2.17.1