netdev - Re: [PATCH net-next 0/2] implement microchip,no-tag-protocol flag

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: <CAHvy4ArS2vgsu0XLE3heUeVrk_mzRjfPszdCg22_xJnuKuKr-A@mail.gmail.com>
Date: Thu, 1 Aug 2024 17:37:43 +0200
From: Pieter <vtpieter@...il.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: devicetree@...r.kernel.org, woojung.huh@...rochip.com, 
	UNGLinuxDriver@...rochip.com, netdev@...r.kernel.org, o.rempel@...gutronix.de, 
	Pieter Van Trappen <pieter.van.trappen@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, 
	Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH net-next 0/2] implement microchip,no-tag-protocol flag

On Thu, Aug 01, 2024 at 15:44, Vladimir Oltean <olteanv@...il.com> wrote:
>
> Hi Pieter,
>
> On Thu, Aug 01, 2024 at 02:31:41PM +0200, vtpieter@...il.com wrote:
> > From: Pieter Van Trappen <pieter.van.trappen@...n.ch>
> >
> > Add and implement microchip,no-tag-protocol flag to allow disabling
> > the switch' tagging protocol. For cases where the CPU MAC does not
> > support MTU size > 1500 such as the Zynq GEM.
> >
> > This code was tested with a KSZ8794 chip.
> >
> > Pieter Van Trappen (2):
> >   dt-bindings: net: dsa: microchip: add microchip,no-tag-protocol flag
> >   net: dsa: microchip: implement microchip,no-tag-protocol flag
> >
> >  .../devicetree/bindings/net/dsa/microchip,ksz.yaml    |  5 +++++
> >  drivers/net/dsa/microchip/ksz8795.c                   |  2 +-
> >  drivers/net/dsa/microchip/ksz9477.c                   |  2 +-
> >  drivers/net/dsa/microchip/ksz_common.c                | 11 ++++++++---
> >  drivers/net/dsa/microchip/ksz_common.h                |  1 +
> >  drivers/net/dsa/microchip/lan937x_main.c              |  2 +-
> >  6 files changed, 17 insertions(+), 6 deletions(-)
> >
> >
> > base-commit: 0a658d088cc63745528cf0ec8a2c2df0f37742d9
> > --
> > 2.43.0
>
> Please use ./scripts/get_maintainer.pl when generating the To: and Cc: fields.
>
> Not to say that they don't exist, but I have never seen a NIC where MTU=1500
> is the absolute hard upper limit. How seriously did you study this before
> determining that it is impossible to raise that? We're talking about one
> byte for the tail tag, FWIW.
>
> There are also alternative paths to explore, like reducing the DSA user ports
> MTU to 1499. This is currently not done when dev_set_mtu() fails on the conduit,
> because Andrew said in the past it's likelier that the conduit is coded
> to accept up to 1500 but will still work for small oversized packets.
>
> Disabling DSA tagging is a very heavy hammer, because it cuts off a whole lot
> of functionality (the driver should no longer accept PTP hwtimestamping ioctls,
> etc), so the patch set gets this tag from me currently, due to very shallow
> justification:
>
> Nacked-by: Vladimir Oltean <olteanv@...il.com>
>
> Please carry it forward if you choose to resubmit.

Hi Vladimir,

I do understand your reservation for this path, it defeats most of the
advantages
of DSA but I still do need the driver to handle the PHYs over SPI and for the
WoL functionality; using the switch in a bridge configuration. My reason for
mainlining is that I though there might be more people like me in a
similar situation.

This is actually an older issue and solution of mine so inspired by your comment
I revisited the documentation and indeed hardware-wise I can't find back the
MTU limitation. I see now that it's actually a limitation of the macb driver [1]
so I will try to rework this one instead. Or implement `dsa-tag-protocol` as you
propose. Or event better, both!

Patch can be thus be cancelled, sorry for the spam.

Cheers, Pieter

[1]: https://elixir.bootlin.com/linux/v6.11-rc1/source/drivers/net/ethernet/cadence/macb_main.c#L5127