netdev - Re: [RFC] net: dsa: microchip: add KSZ8 change_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] [thread-next>] [day] [month] [year] [list]

Message-ID: <e3dff6ce-7fb2-47fa-9141-9281e5e9de5e@kernel.org>
Date: Fri, 16 Aug 2024 09:12:20 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: vtpieter@...il.com, Andrew Lunn <andrew@...n.ch>,
 Florian Fainelli <f.fainelli@...il.com>, Vladimir Oltean
 <olteanv@...il.com>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Woojung Huh <woojung.huh@...rochip.com>,
 UNGLinuxDriver@...rochip.com, Russell King <linux@...linux.org.uk>
Cc: Pieter Van Trappen <pieter.van.trappen@...n.ch>, netdev@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC] net: dsa: microchip: add KSZ8 change_tag_protocol support

On 16/08/2024 08:59, vtpieter@...il.com wrote:
> From: Pieter Van Trappen <pieter.van.trappen@...n.ch>
> 
> Add support for changing the KSZ8 switches tag protocol. In fact
> these devices can only enable or disable the tail tag, so there's
> really only three supported protocols:
> - DSA_TAG_PROTO_KSZ8795 for KSZ87xx
> - DSA_TAG_PROTO_KSZ9893 for KSZ88x3
> - DSA_TAG_PROTO_NONE
> 
> When disabled, this can be used as a workaround for the 'Common
> pitfalls using DSA setups' [1] to use the conduit network interface as
> a regular one, admittedly forgoing most DSA functionality and using
> the device as an unmanaged switch whilst allowing control
> operations (ethtool, PHY management, WoL). Implementing the new
> software-defined DSA tagging protocol tag_8021q [2] for these devices
> seems overkill for this use case at the time being.
> 
> Link: Documentation/networking/dsa/dsa.rst [1]
> Link: https://lpc.events/event/11/contributions/949/attachments/823/1555/paper.pdf [2]
> Signed-off-by: Pieter Van Trappen <pieter.van.trappen@...n.ch>
> ---
>  .../devicetree/bindings/net/dsa/dsa-port.yaml |  1 +
>  drivers/net/dsa/microchip/ksz8.h              |  2 ++
>  drivers/net/dsa/microchip/ksz8795.c           | 28 +++++++++++++++++++
>  drivers/net/dsa/microchip/ksz_common.c        | 19 +++++++++++--
>  drivers/net/dsa/microchip/ksz_common.h        |  2 ++
>  5 files changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> index 480120469953..ded8019b6ba6 100644
> --- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> @@ -53,6 +53,7 @@ properties:
>      enum:
>        - dsa
>        - edsa
> +      - none
>        - ocelot
>        - ocelot-8021q
>        - rtl8_4

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.

Anyway, what does "none" mean in terms of protocol? Is there a "none"
protocol? Or you mean, disable tagging entirely?

Best regards,
Krzysztof