netdev - Re: [PATCH net-next v2 1/3] dsa: Implement RMU layer in DSA

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]

Date:   Fri, 26 Aug 2022 21:27:17 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Mattias Forsblad <mattias.forsblad@...il.com>
Cc:     netdev@...r.kernel.org, Vivien Didelot <vivien.didelot@...il.com>,
        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>
Subject: Re: [PATCH net-next v2 1/3] dsa: Implement RMU layer in DSA

On Fri, Aug 26, 2022 at 08:38:14AM +0200, Mattias Forsblad wrote:
> Support handling of layer 2 part for RMU frames which is
> handled in-band with other DSA traffic.
> 
> Signed-off-by: Mattias Forsblad <mattias.forsblad@...il.com>
> ---
>  include/net/dsa.h             |   7 +++
>  include/uapi/linux/if_ether.h |   1 +
>  net/dsa/tag_dsa.c             | 109 +++++++++++++++++++++++++++++++++-
>  3 files changed, 114 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index f2ce12860546..54f7f3494f84 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -92,6 +92,7 @@ struct dsa_switch;
>  struct dsa_device_ops {
>  	struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev);
>  	struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev);
> +	int (*inband_xmit)(struct sk_buff *skb, struct net_device *dev, int seq_no);
>  	void (*flow_dissect)(const struct sk_buff *skb, __be16 *proto,
>  			     int *offset);
>  	int (*connect)(struct dsa_switch *ds);
> @@ -1193,6 +1194,12 @@ struct dsa_switch_ops {
>  	void	(*master_state_change)(struct dsa_switch *ds,
>  				       const struct net_device *master,
>  				       bool operational);
> +
> +	/*
> +	 * RMU operations
> +	 */
> +	int (*inband_receive)(struct dsa_switch *ds, struct sk_buff *skb,
> +			int seq_no);
>  };

Hi Mattias

Vladimer pointed you towards the qca driver, in a comment for your
RFC. qca already has support for switch commands via Ethernet frames.

The point he was trying to make is that you should look at that
code. The concept of executing a command via an Ethernet frame, and
expecting a reply via an Ethernet frame is generic. The format of
those frames is specific to the switch. We want the generic parts to
look the same for all switches. If possible, we want to implement it
once in the dsa core, so all switch drivers share it. Less code,
better tested code, less bugs, easier maintenance.

Take a look at qca_tagger_data. Please use the same mechanism with
mv88e6xxx. But also look deeper. What else can be shared? You need a
buffer to put the request in, you need to send it, you need to wait
for the reply, you need to pass the results to the driver, you need to
free that buffer afterwards. That should all be common. Look at these
parts in the qca driver. Can you make them generic, move them into the
DSA core? Are there other parts which could be shared?

    Andrew