| 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
| ||
|
Date: Mon, 3 Jan 2022 16:57:46 +0000
From: Vladimir Oltean <vladimir.oltean@....com>
To: Horatiu Vultur <horatiu.vultur@...rochip.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"kuba@...nel.org" <kuba@...nel.org>,
"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 3/3] net: lan966x: Extend switchdev with mdb
support
On Mon, Jan 03, 2022 at 05:22:44PM +0100, Horatiu Vultur wrote:
> > > +static int lan966x_mdb_ip_del(struct lan966x_port *port,
> > > + const struct switchdev_obj_port_mdb *mdb,
> > > + enum macaccess_entry_type type)
> > > +{
> > > + bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
> > > + struct lan966x *lan966x = port->lan966x;
> > > + struct lan966x_mdb_entry *mdb_entry;
> > > + unsigned char mac[ETH_ALEN];
> > > +
> > > + mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
> > > + if (!mdb_entry) {
> > > + /* If the CPU originted this and the entry was not found, it is
> >
> > s/originted/originated/
> >
> > > + * because already another port has removed the entry
> > > + */
> > > + if (cpu_port)
> > > + return 0;
> >
> > Could you explain again why this is normal? If another port has removed
> > the entry, how is the address copied to the CPU any longer?
>
> The cpu_port is set only when the entry is deleted by the bridge. So then
> all the ports under the bridge will be notified about this. So when the first
> port is notified it would delete the entry and then when the second port is
> notified, it would try to delete the entry which is already deleted by
> the first port. That is the reason why it returns 0 and not an error.
> > The HOST_MDB switchdev events are replicated per the number of ports in
> > the bridge.
>
> Correct.
>
> > So I would expect that you keep refcounts on them, otherwise
> > the first deletion of such an element would trigger the removal of the
> > entry from hardware even though it's still in use.
>
> Sorry, I am not sure I am following, by whom is it still in use?
>
> I was trying the following commands:
>
> ip link set dev eth0 master br0
> ip link set dev eth1 master br0
> bridge mdb add dev br0 port br0 grp 225.1.2.3
>
> Then both ports eth0 and eth1 will get a notification about this. And
> they will add an entry in the MAC table for this.
> Then when the following command is run:
>
> bridge mdb del dev br0 port br0 grp 225.1.2.3
>
> Then again both ports will get a notification about this and eth0 will
> delete the entry from the MAC table. So why is this not correct? Then
> will be eth1 who will get the notification and try to delete the entry
> which is already deleted.
root@...ian:~# ip link set swp0 up
[ 42.938394] fsl_enetc 0000:00:00.2 eno2: configuring for fixed/internal link mode
[ 42.949675] fsl_enetc 0000:00:00.2 eno2: Link is Up - 2.5Gbps/Full - flow control rx/tx
[ 42.959044] mscc_felix 0000:00:00.5 swp0: configuring for inband/qsgmii link mode
root@...ian:~# ip link set swp1 up
[ 44.440675] mscc_felix 0000:00:00.5 swp1: configuring for inband/qsgmii link mode
root@...ian:~# ip link set swp2 up
[ 45.788811] mscc_felix 0000:00:00.5 swp2: configuring for inband/qsgmii link mode
root@...ian:~# ip link set swp3 up
[ 47.784439] mscc_felix 0000:00:00.5 swp3: configuring for inband/qsgmii link mode
[ 51.887374] mscc_felix 0000:00:00.5 swp3: Link is Up - 1Gbps/Full - flow control rx/tx
[ 51.895491] IPv6: ADDRCONF(NETDEV_CHANGE): swp3: link becomes ready
[ 100.303070] mscc_felix 0000:00:00.5 swp0: Link is Up - 1Gbps/Full - flow control off
[ 100.311016] IPv6: ADDRCONF(NETDEV_CHANGE): swp0: link becomes ready
root@...ian:~# ip link add br0 type bridge
root@...ian:~# ip link set swp0 master br0
[ 115.436652] br0: port 1(swp0) entered blocking state
[ 115.441736] br0: port 1(swp0) entered disabled state
[ 115.447618] device swp0 entered promiscuous mode
root@...ian:~# ip link set swp1 master br0
[ 117.760700] br0: port 2(swp1) entered blocking state
[ 117.765797] br0: port 2(swp1) entered disabled state
[ 117.771335] device swp1 entered promiscuous mode
root@...ian:~# ip link set swp2 master br0
[ 119.612665] br0: port 3(swp2) entered blocking state
[ 119.617750] br0: port 3(swp2) entered disabled state
[ 119.623304] device swp2 entered promiscuous mode
root@...ian:~# ip link set swp3 master br0
[ 121.388388] br0: port 4(swp3) entered blocking state
[ 121.393416] br0: port 4(swp3) entered disabled state
[ 121.399292] device swp3 entered promiscuous mode
root@...ian:~# [ 148.905819] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 0 addr 33:33:ff:9e:9e:96 vid 0
[ 148.914819] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 1 addr 33:33:ff:9e:9e:96 vid 0
[ 148.923723] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 2 addr 33:33:ff:9e:9e:96 vid 0
[ 148.932606] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 3 addr 33:33:ff:9e:9e:96 vid 0
[ 148.941426] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 0 addr 33:33:00:00:00:6a vid 0
[ 148.950351] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 1 addr 33:33:00:00:00:6a vid 0
[ 148.959164] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 2 addr 33:33:00:00:00:6a vid 0
[ 148.967967] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 3 addr 33:33:00:00:00:6a vid 0
[ 150.649744] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 0 addr 33:33:00:00:00:fb vid 0
[ 150.658674] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 1 addr 33:33:00:00:00:fb vid 0
[ 150.667538] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 2 addr 33:33:00:00:00:fb vid 0
[ 150.676389] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 3 addr 33:33:00:00:00:fb vid 0
root@...ian:~# ip link set br0 up
[ 148.868268] br0: port 4(swp3) entered blocking state
[ 148.873283] br0: port 4(swp3) entered forwarding state
[ 148.878530] br0: port 1(swp0) entered blocking state
[ 148.883546] br0: port 1(swp0) entered forwarding state
[ 148.889443] IPv6: ADDRCONF(NETDEV_CHANGE): br0: link becomes ready
[ 148.905819] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 0 addr 33:33:ff:9e:9e:96 vid 0
[ 148.914819] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 1 addr 33:33:ff:9e:9e:96 vid 0
[ 148.923723] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 2 addr 33:33:ff:9e:9e:96 vid 0
[ 148.932606] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 3 addr 33:33:ff:9e:9e:96 vid 0
[ 148.941426] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 0 addr 33:33:00:00:00:6a vid 0
[ 148.950351] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 1 addr 33:33:00:00:00:6a vid 0
[ 148.959164] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 2 addr 33:33:00:00:00:6a vid 0
[ 148.967967] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 3 addr 33:33:00:00:00:6a vid 0
[ 150.649744] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 0 addr 33:33:00:00:00:fb vid 0
[ 150.658674] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 1 addr 33:33:00:00:00:fb vid 0
[ 150.667538] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 2 addr 33:33:00:00:00:fb vid 0
[ 150.676389] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 3 addr 33:33:00:00:00:fb vid 0
root@...ian:~# ip link set swp3 nomaster
[ 160.416054] device swp3 left promiscuous mode
[ 160.420745] br0: port 4(swp3) entered disabled state
[ 160.445427] mscc_felix 0000:00:00.5: dsa_port_host_mdb_del: port 3 addr 33:33:00:00:00:fb vid 0
[ 160.454201] mscc_felix 0000:00:00.5: dsa_port_host_mdb_del: port 3 addr 33:33:00:00:00:6a vid 0
[ 160.464843] mscc_felix 0000:00:00.5: dsa_port_host_mdb_del: port 3 addr 33:33:ff:9e:9e:96 vid 0
Hope it's a bit clearer now. swp0, swp1 and swp2 are still under br0.
Powered by blists - more mailing lists