Netdev Archive on lore.kernel.org
 help / color / Atom feed
From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: David Laight <David.Laight@ACULAB.COM>
Cc: 'Florian Fainelli' <f.fainelli@gmail.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"andrew@lunn.ch" <andrew@lunn.ch>,
	"vivien.didelot@gmail.com" <vivien.didelot@gmail.com>,
	"kuba@kernel.org" <kuba@kernel.org>,
	Christian Eggers <ceggers@arri.de>,
	Kurt Kanzenbach <kurt@linutronix.de>
Subject: Re: [RFC PATCH 02/13] net: dsa: implement a central TX reallocation procedure
Date: Mon, 19 Oct 2020 11:41:56 +0000
Message-ID: <20201019114155.lcweo2suwo73eopo@skbuf> (raw)
In-Reply-To: <45ac0c697c164991a99a35a2d981b5db@AcuMS.aculab.com>

On Mon, Oct 19, 2020 at 11:14:07AM +0000, David Laight wrote:
> If the skb are 'cloned' then I suspect you can't even put the MAC addresses
> into the skb because they might be being transmitted on another interface.

No, that's not right. The bridge floods frames by cloning them, and L2
forwarding doesn't need to modify the L2 header in any way. So when you
have a bridge upper, you will get cloned skbs with a valid MAC header in
them.

> OTOH TCP will have cloned the skb for retransmit so may ensure than there
> isn't (still) a second reference (from an old transmit) before doing the
> transmit - in which case you can write into the head/tail space.

In the case of TCP, I suppose the DSA layer will never see cloned skbs
for the reason you mention, true.

> Hmmm... I was thinking you were doing something for a specific driver.
> But it looks more like it depends on what the interface is connected to.

I'm not sure what you mean here.

> If the MAC addresses (and ethertype) can be written into the skb head
> space then it must be valid to rewrite the header containing the tag.
> (With the proviso that none of the MAC drivers try to decode it again.)

This phrase is almost correct.
Hardware TX timestamping also clones the skb, because the clone needs to
be ultimately reinjected into the socket's error queue, for the user
space program to retrieve the timestamp via a cmsg.

> One thing I have noticed is that the size of the 'headroom' for UDP
> and RAWIP (where I was looking) packets depends on the final 'dev'.
> This means that you can't copy the frame into an skb until you know
> the 'dev' - but that might depend on the frame data.
> This is a 'catch-22' problem.
> I actually wonder how much the headroom varies.
> It might be worth having a system-wide 'headroom' value.
> A few extra bytes aren't really going to make any difference.
>
> That might make it far less likely that there isn't the available
> headroom for the tag - in which case you can just log once and discard.

Again, the case of the bridge, and of TX timestamping, and of tail
taggers that need to perform padding, is enough that we need to
reallocate skbs, and can't just drop them when they're not writable or
there isn't enough head/tailroom.

  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 ` [RFC PATCH 02/13] net: dsa: implement a central TX reallocation procedure Vladimir Oltean
2020-10-17 22:01   ` 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 [this message]
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=20201019114155.lcweo2suwo73eopo@skbuf \
    --to=vladimir.oltean@nxp.com \
    --cc=David.Laight@ACULAB.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