linux-kernel - Re: [Patch RFC net-next 2/4] net: dsa: microchip: lan937x: remove vlan_filtering_is

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: <1fe46b15172ff82125c46369d9ed235f67ed5afe.camel@microchip.com>
Date:   Tue, 2 Aug 2022 16:09:35 +0000
From:   <Arun.Ramadoss@...rochip.com>
To:     <olteanv@...il.com>
CC:     <andrew@...n.ch>, <linux-kernel@...r.kernel.org>,
        <UNGLinuxDriver@...rochip.com>, <vivien.didelot@...il.com>,
        <linux@...linux.org.uk>, <f.fainelli@...il.com>, <kuba@...nel.org>,
        <edumazet@...gle.com>, <pabeni@...hat.com>,
        <netdev@...r.kernel.org>, <Woojung.Huh@...rochip.com>,
        <davem@...emloft.net>
Subject: Re: [Patch RFC net-next 2/4] net: dsa: microchip: lan937x: remove
 vlan_filtering_is_global flag

Hi Vladimir,
Thanks for the comment.

On Tue, 2022-08-02 at 13:40 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Fri, Jul 29, 2022 at 08:47:31PM +0530, Arun Ramadoss wrote:
> > To have the similar implementation among the ksz switches, removed
> > the
> > vlan_filtering_is_global flag which is only present in the lan937x.
> > 
> > Signed-off-by: Arun Ramadoss <arun.ramadoss@...rochip.com>
> > ---
> >  drivers/net/dsa/microchip/lan937x_main.c | 5 -----
> >  1 file changed, 5 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/microchip/lan937x_main.c
> > b/drivers/net/dsa/microchip/lan937x_main.c
> > index daedd2bf20c1..9c1fe38efd1a 100644
> > --- a/drivers/net/dsa/microchip/lan937x_main.c
> > +++ b/drivers/net/dsa/microchip/lan937x_main.c
> > @@ -401,11 +401,6 @@ int lan937x_setup(struct dsa_switch *ds)
> >               return ret;
> >       }
> > 
> > -     /* The VLAN aware is a global setting. Mixed vlan
> > -      * filterings are not supported.
> > -      */
> > -     ds->vlan_filtering_is_global = true;
> > -
> 
> You understand what this flag does, right? It ensures that if you
> have
> lan0 and lan1 under VLAN-aware br0, then lan2 which is standalone
> will
> declare NETIF_F_HW_VLAN_CTAG_FILTER. In turn, this makes the network
> stack know that lan2 won't accept VLAN-tagged packets unless there is
> an
> 8021q interface with the given VID on top of it. This 8021q interface
> calls vlan_vid_add() to populate the driver's VLAN RX filter with its
> VID, and this gets translated into dsa_slave_vlan_rx_add_vid() which
> ultimately reaches the driver's ->port_vlan_add() function.
> 
> If VLAN filtering *is* a global setting, and looking at this call
> from
> ksz9477_port_vlan_filtering() which is not per port, I'd say it is:
> 
>                 ksz_cfg(dev, REG_SW_LUE_CTRL_0, SW_VLAN_ENABLE,
> true);
> 
> then what would happen is that all VLAN tagged traffic would be
> dropped
> on the standalone lan2.
> 
> I'd say that the ksz9477 is buggy for not declaring
> vlan_filtering_is_global,
> rather than encouraging you to delete it from lan937x. In turn,
> fixing
> ksz9477 would make setting this flag from a common location possible,
> because ksz8 needs it too.

I have done some study on this SW_VLAN_ENABLE bit. By default the pvid
of the port is 1 and vlan port membership (0xNA04) is 0xff. So if the
bit is 0, then unknown dest addr packets are broadcasted which is the
default behaviour of switch.
Now consider when the bit is 1,
- If the invalid vlan packet is received, then based on drop invalid
vid or unknown vid forward bit, packets are discarded or forwarded.
- If the valid vlan packet is received, then broadcast to ports in vlan
port membership list.
The port membership register set using the ksz9477_cfg_port_member
function.
In summary, I infer that we can enable this bit in the port_setup and
based on the port membership packets can be forwarded. Is my
understanding correct?
Can I remove this patch from this series and submit as the separate
one?