netdev - Re: DSA using cpsw and lan9303

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220215205418.a25ro255qbv5hpjk@skbuf>
Date:   Tue, 15 Feb 2022 22:54:18 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Måns Rullgård <mans@...sr.com>,
        netdev@...r.kernel.org,
        Egil Hjelmeland <privat@...l-hjelmeland.no>,
        Andrew Lunn <andrew@...n.ch>,
        Juergen Borleis <jbe@...gutronix.de>,
        Grygorii Strashko <grygorii.strashko@...com>,
        lorenzo@...nel.org
Subject: Re: DSA using cpsw and lan9303

Hi Måns,

On Mon, Feb 14, 2022 at 09:16:10AM -0800, Florian Fainelli wrote:
> +others,
> 
> netdev is a high volume list, you should probably copy directly the
> people involved with the code you are working with.
> 
> On 2/14/22 8:44 AM, Måns Rullgård wrote:
> > The hardware I'm working on has a LAN9303 switch connected to the
> > Ethernet port of an AM335x (ZCE package).  In trying to make DSA work
> > with this combination, I have encountered two problems.
> > 
> > Firstly, the cpsw driver configures the hardware to filter out frames
> > with unknown VLAN tags.  To make it accept the tagged frames coming from
> > the LAN9303, I had to modify the latter driver like this:
> > 
> > diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> > index 2de67708bbd2..460c998c0c33 100644
> > --- a/drivers/net/dsa/lan9303-core.c
> > +++ b/drivers/net/dsa/lan9303-core.c
> > @@ -1078,20 +1079,28 @@ static int lan9303_port_enable(struct dsa_switch *ds, int port,
> >                                struct phy_device *phy)
> >  {
> >         struct lan9303 *chip = ds->priv;
> > +       struct net_device *master;
> >  
> >         if (!dsa_is_user_port(ds, port))
> >                 return 0;
> >  
> > +       master = dsa_to_port(chip->ds, 0)->master;
> > +       vlan_vid_add(master, htons(ETH_P_8021Q), port);
> 
> That looks about right given that net/dsa/tag_lan9303.c appears to be a
> quasi DSA_TAG_PROTO_8021Q implementation AFAICT.

In case it was not clear, I agree with Florian that this looks "about right",
I just thought that mentioning it wouldn't bring much to the table.
But I noticed you submitted a patch for the other issue and not for this.

Some complaints about accessing the CPU port as dsa_to_port(chip->ds, 0),
but it's not the first place in this driver where that is done.

dsa_tag_8021q_port_setup() also has:

	/* Add @rx_vid to the master's RX filter. */
	vlan_vid_add(master, ctx->proto, rx_vid);

which is an indication that other switches with VLAN-based tagging
protocols should handle this somehow, somewhere.

Note, though, that vlan_vid_add() allocates memory, so it would be good
to have a vlan_vid_del() too at some point.