Netdev Archive on lore.kernel.org
 help / color / Atom feed
From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: netdev@vger.kernel.org
Cc: andrew@lunn.ch, f.fainelli@gmail.com, vivien.didelot@gmail.com,
	kuba@kernel.org, Christian Eggers <ceggers@arri.de>,
	Kurt Kanzenbach <kurt@linutronix.de>
Subject: [RFC PATCH 02/13] net: dsa: implement a central TX reallocation procedure
Date: Sun, 18 Oct 2020 00:36:00 +0300
Message-ID: <20201017213611.2557565-3-vladimir.oltean@nxp.com> (raw)
In-Reply-To: <20201017213611.2557565-1-vladimir.oltean@nxp.com>

At the moment, taggers are left with the task of ensuring that the skb
headers are writable (which they aren't, if the frames were cloned for
TX timestamping, for flooding by the bridge, etc), and that there is
enough space in the skb data area for the DSA tag to be pushed.

Moreover, the life of tail taggers is even harder, because they need to
ensure that short frames have enough padding, a problem that normal
taggers don't have.

The principle of the DSA framework is that everything except for the
most intimate hardware specifics (like in this case, the actual packing
of the DSA tag bits) should be done inside the core, to avoid having
code paths that are very rarely tested.

So provide a TX reallocation procedure that should cover the known needs
of DSA today.

Note that this patch also gives the network stack a good hint about the
headroom/tailroom it's going to need. Up till now it wasn't doing that.
So the reallocation procedure should really be there only for the
exceptional cases, and for cloned packets which need to be unshared.
The tx_reallocs counter should prove that.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/slave.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d4326940233c..790f5c8deb13 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -548,6 +548,36 @@ netdev_tx_t dsa_enqueue_skb(struct sk_buff *skb, struct net_device *dev)
 }
 EXPORT_SYMBOL_GPL(dsa_enqueue_skb);
 
+static int dsa_realloc_skb(struct sk_buff *skb, struct net_device *dev)
+{
+	struct net_device *master = dsa_slave_to_master(dev);
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_slave_stats *e;
+	int headroom, tailroom;
+
+	headroom = dev->needed_headroom;
+	tailroom = dev->needed_tailroom;
+	/* For tail taggers, we need to pad short frames ourselves, to ensure
+	 * that the tail tag does not fail at its role of being at the end of
+	 * the packet, once the master interface pads the frame.
+	 */
+	if (unlikely(tailroom && skb->len < ETH_ZLEN))
+		tailroom += ETH_ZLEN - skb->len;
+
+	if (likely(skb_headroom(skb) >= headroom &&
+		   skb_tailroom(skb) >= tailroom) &&
+		   !skb_cloned(skb))
+		/* No reallocation needed, yay! */
+		return 0;
+
+	e = this_cpu_ptr(p->extra_stats);
+	u64_stats_update_begin(&e->syncp);
+	e->tx_reallocs++;
+	u64_stats_update_end(&e->syncp);
+
+	return pskb_expand_head(skb, headroom, tailroom, GFP_ATOMIC);
+}
+
 static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
@@ -567,6 +597,11 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
 	 */
 	dsa_skb_tx_timestamp(p, skb);
 
+	if (dsa_realloc_skb(skb, dev)) {
+		kfree_skb(skb);
+		return NETDEV_TX_OK;
+	}
+
 	/* Transmit function may have to reallocate the original SKB,
 	 * in which case it must have freed it. Only free it here on error.
 	 */
@@ -1802,6 +1837,14 @@ int dsa_slave_create(struct dsa_port *port)
 	slave_dev->netdev_ops = &dsa_slave_netdev_ops;
 	if (ds->ops->port_max_mtu)
 		slave_dev->max_mtu = ds->ops->port_max_mtu(ds, port->index);
+	/* Try to save one extra realloc later in the TX path (in the master)
+	 * by also inheriting the master's needed headroom and tailroom.
+	 * The 8021q driver also does this.
+	 */
+	if (cpu_dp->tag_ops->tail_tag)
+		slave_dev->needed_tailroom = cpu_dp->tag_ops->overhead;
+	else
+		slave_dev->needed_headroom = cpu_dp->tag_ops->overhead;
 	SET_NETDEV_DEVTYPE(slave_dev, &dsa_type);
 
 	netdev_for_each_tx_queue(slave_dev, dsa_slave_set_lockdep_class_one,
-- 
2.25.1


  parent reply index

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-17 21:35 [RFC PATCH 00/13] Generic TX reallocation for DSA Vladimir Oltean
2020-10-17 21:35 ` [RFC PATCH 01/13] net: dsa: add plumbing for custom netdev statistics Vladimir Oltean
2020-10-17 22:13   ` Florian Fainelli
2020-10-17 23:15   ` Andrew Lunn
2020-10-18  0:23     ` Vladimir Oltean
2020-10-18 12:02   ` Heiner Kallweit
2020-10-18 12:16     ` Vladimir Oltean
2020-10-18 13:09       ` Heiner Kallweit
2020-10-18 13:48         ` Vladimir Oltean
2020-10-18 14:13           ` Heiner Kallweit
2020-10-18 22:58             ` Vladimir Oltean
2020-10-18 23:11               ` Florian Fainelli
2020-10-19  0:21                 ` Vladimir Oltean
2020-10-19  3:49                   ` Florian Fainelli
2020-10-19 12:05                     ` Andrew Lunn
2020-10-19 16:27                       ` Florian Fainelli
2020-10-18 16:49   ` Jakub Kicinski
2020-10-18 17:36     ` Andrew Lunn
2020-10-18 18:26       ` Jakub Kicinski
2020-10-18 18:33         ` Jakub Kicinski
2020-10-17 21:36 ` Vladimir Oltean [this message]
2020-10-17 22:01   ` [RFC PATCH 02/13] net: dsa: implement a central TX reallocation procedure Vladimir Oltean
2020-10-17 22:11     ` Florian Fainelli
2020-10-17 22:17       ` Vladimir Oltean
2020-10-18  0:37         ` Florian Fainelli
2020-10-19  8:33       ` David Laight
2020-10-19 10:30         ` Vladimir Oltean
2020-10-19 11:14           ` David Laight
2020-10-19 11:41             ` Vladimir Oltean
2020-10-19 12:19           ` Andrew Lunn
2020-10-17 22:31     ` Vladimir Oltean
2020-10-18  0:13       ` Vladimir Oltean
2020-10-18 10:36         ` Christian Eggers
2020-10-18 11:42           ` Vladimir Oltean
2020-10-18 11:59             ` Christian Eggers
2020-10-18 12:15               ` Vladimir Oltean
2020-10-17 21:36 ` [RFC PATCH 03/13] net: dsa: tag_ksz: don't allocate additional memory for padding/tagging Vladimir Oltean
2020-10-17 21:36 ` [RFC PATCH 04/13] net: dsa: trailer: " Vladimir Oltean
2020-10-17 21:36 ` [RFC PATCH 05/13] net: dsa: tag_qca: let DSA core deal with TX reallocation Vladimir Oltean
2020-10-17 21:36 ` [RFC PATCH 06/13] net: dsa: tag_ocelot: " Vladimir Oltean
2020-10-17 21:36 ` [RFC PATCH 07/13] net: dsa: tag_mtk: " Vladimir Oltean
2020-10-17 21:36 ` [RFC PATCH 08/13] net: dsa: tag_lan9303: " Vladimir Oltean
2020-10-17 21:36 ` [RFC PATCH 09/13] net: dsa: tag_edsa: " Vladimir Oltean
2020-10-17 21:36 ` [RFC PATCH 10/13] net: dsa: tag_brcm: " Vladimir Oltean
2020-10-17 21:36 ` [RFC PATCH 11/13] net: dsa: tag_dsa: " Vladimir Oltean
2020-10-17 21:36 ` [RFC PATCH 12/13] net: dsa: tag_gswip: " Vladimir Oltean
2020-10-17 22:19   ` Vladimir Oltean
2020-10-17 21:36 ` [RFC PATCH 13/13] net: dsa: tag_ar9331: " Vladimir Oltean
2020-10-17 23:07 ` [RFC PATCH 00/13] Generic TX reallocation for DSA Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201017213611.2557565-3-vladimir.oltean@nxp.com \
    --to=vladimir.oltean@nxp.com \
    --cc=andrew@lunn.ch \
    --cc=ceggers@arri.de \
    --cc=f.fainelli@gmail.com \
    --cc=kuba@kernel.org \
    --cc=kurt@linutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=vivien.didelot@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git