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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 16 Feb 2022 13:17:47 +0000
From:   Måns Rullgård <mans@...sr.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Florian Fainelli <f.fainelli@...il.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

Vladimir Oltean <olteanv@...il.com> writes:

> 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.

What would be the proper way to do it?

> 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.

Yes, I just didn't include that bit in the initial query.

-- 
Måns Rullgård