RE: [RFC PATCH 02/13] net: dsa: implement a central TX reallocation procedure

From: David Laight <David.Laight@ACULAB.COM>
To: 'Vladimir Oltean' <vladimir.oltean@nxp.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:14:07 +0000	[thread overview]
Message-ID: <45ac0c697c164991a99a35a2d981b5db@AcuMS.aculab.com> (raw)
In-Reply-To: <20201019103047.oq5ki3jlhnwzz2xv@skbuf>

From: Vladimir Oltean
> Sent: 19 October 2020 11:31
> 
> On Mon, Oct 19, 2020 at 08:33:27AM +0000, David Laight wrote:
> > Is it possible to send the extra bytes from a separate buffer fragment?
> > The entire area could be allocated (coherent) when the rings are
> > allocated.
> > That would save having to modify the skb at all.
> >
> > Even if some bytes of the frame header need 'adjusting' transmitting
> > from a copy may be faster - especially on systems with an iommu.
> >
> > Many (many) moons ago we found the cutoff point for copying frames
> > on a system with an iommu to be around 1k bytes.
> 
> Please help me understand better how to implement what you're suggesting.
> DSA switches have 3 places where they might insert a tag:
> 1. Between the source MAC address and the EtherType (this is the most
>    common)
> 2. Before the destination MAC address
> 3. Before the FCS
> 
> I imagine that the most common scenario (1) is also the most difficult
> to implement using fragments, since I would need to split the Ethernet
> header from the rest of the skb data area, which might defeat the
> purpose.
> 
> Also, simply integrating these 3 code paths into something generic will
> bring challenges of its own.
> 
> Lastly, I fully expect the buffers to have proper headroom and tailroom
> now (if they don't, then it's worth investigating what's the code path
> that doesn't observe our dev->needed_headroom and dev->needed_tailroom).
> The reallocation code is there just for clones (and as far as I
> understand, fragments won't save us from the need of reallocating the
> data areas of clones), and for short frames with DSA switches in case
> (3).

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.

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.

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.

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

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.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)