linux-kernel - Re: [PATCH net-next v3 6/6] net: lan966x: Add switchdev support

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:   Mon, 13 Dec 2021 14:37:16 +0100
From:   Horatiu Vultur <horatiu.vultur@...rochip.com>
To:     Vladimir Oltean <vladimir.oltean@....com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "vivien.didelot@...il.com" <vivien.didelot@...il.com>,
        "andrew@...n.ch" <andrew@...n.ch>
Subject: Re: [PATCH net-next v3 6/6] net: lan966x: Add switchdev support

The 12/13/2021 11:46, Vladimir Oltean wrote:
> 
> On Thu, Dec 09, 2021 at 05:43:11PM +0100, Horatiu Vultur wrote:
> > > My documentation of CPU_SRC_COPY_ENA says:
> > >
> > > If set, all frames received on this port are
> > > copied to the CPU extraction queue given by
> > > CPUQ_CFG.CPUQ_SRC_COPY.
> > >
> > > I think it was established a while ago that this isn't what promiscuous
> > > mode is about? Instead it is about accepting packets on a port
> > > regardless of whether the MAC DA is in their RX filter or not.
> >
> > Yes, I am aware that this change interprets the things differently and I
> > am totally OK to drop this promisc if it is needed.
> 
> I think we just need to agree on the observable behavior. Promiscuous
> means for an interface to receive packets with unknown destination, and
> while in standalone mode you do support that, in bridge mode you're a
> bit on the edge: the port accepts them but will deliver them anywhere
> except to the CPU. I suppose you could try to make an argument that you
> know better than the bridge, and as long as the use cases for that are
> restricted enough, maybe it could work for most scenarios. I don't know.

I think this requires some proper explanations of the intended
behaviour for both the standalone and bridge mode. I will drop this
promisc for now, as other drivers are doing it and at a later point
send some patch series with all the explanations.

> 
> > > Hence the oddity of your change. I understand what it intends to do:
> > > if this is a standalone port you support IFF_UNICAST_FLT, so you drop
> > > frames with unknown MAC DA. But if IFF_PROMISC is set, then why do you
> > > copy all frames to the CPU? Why don't you just put the CPU in the
> > > unknown flooding mask?
> >
> > Because I don't want the CPU to be in the unknown flooding mask. I want
> > to send frames to the CPU only if it is required.
> 
> What is the strategy through which this driver accepts things like
> pinging the bridge device over IPv6, with the Neighbor Discovery
> protocol having the ICMP6 neighbor solicitation messages delivered to
> (according to my knowledge) an unregistered IPv6 multicast address?
> Whose responsibility is it to notify the driver of that address, and
> does the driver copy those packets to the CPU in the right VLAN?

I think in that case the CPU should be part of the multicast flooding
mask. I will need to look more on this because I don't know much about
the IPv6.

> 
> > > How do you handle migration of an FDB entry pointing towards the CPU,
> > > towards one pointing towards a port?
> >
> > Shouldn't I get 2 calls that the entry is removed from CPU and then
> > added to a port?
> 
> Ok.

-- 
/Horatiu