netdev - Re: [PATCH v3 net] net: ftmac100: support frames with DSA tag

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] [day] [month] [year] [list]

Message-ID: <20221019163144.k723gzjnbq6ith4j@skbuf>
Date:   Wed, 19 Oct 2022 19:31:44 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Sergei Antonov <saproj@...il.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH v3 net] net: ftmac100: support frames with DSA tag

On Wed, Oct 19, 2022 at 07:19:11PM +0300, Sergei Antonov wrote:
> On Mon, 17 Oct 2022 at 18:55, Vladimir Oltean <olteanv@...il.com> wrote:
> > 6. Actually implement the ndo_change_mtu() for this driver, and even though you
> >    don't make any hardware change based on the precise value, you could do one
> >    important thing. Depending on whether the dev->mtu is <= 1500 or not, you could
> >    set or unset the FTL bit, and this could allow for less CPU cycles spent
> >    dropping S/G frames when the MTU of the interface is standard 1500.
> 
> Can I do the mtu>1500 check and set FTL in ndo_open() (which is called
> after ndo_change_mtu())?

Through which code path is ndo_open called after ndo_change_mtu()?
To my knowledge they are independent. The MTU can be changed even on an
interface which is up, and the driver needs to handle this.

> I am submitting a v4 of the patch, and your feedback to it would be
> helpful.

Well, I see you posted v4 within 2 minutes of requesting further
feedback....

> I got rid of all mentions of DSA tagging there, so patch description
> and a code comment are new. Thanks for clearing things up regarding
> EtherType 0800.

I didn't ask to get rid of all mentions of DSA tagging. On the contrary,
it is relevant to mention that DSA is the reason you are making these
changes. The changes would have looked quite different if you needed to
increase the MTU for other reasons (also see below).

> > With these changes, you would leave the ftmac100 driver in a more civilized
> > state, you wouldn't need to implement S/G RX unless you wanted to, and
> > as a side effect, it would also operate well as a DSA master.
> 
> What does S/G stand for?

S/G means scatter/gather, or in other words, the ability to create an
skb from multiple RX buffer descriptors. Since each BD is limited to 2K,
this limits the size of the skb also to 2K, whereas a scatter/gather skb
is essentially unlimited in size regardless of the length of individual
buffers.

What I was saying is that for DSA on mv88e6060 (which doesn't support
jumbo frames AFAIK) you're essentially still fine within the confines of
single buffer skb, you just need to allow MTU values just a little bit
higher than 1500, but not as high as, say, 9k, which would require more
effort/rework.