merged. Bruce
In message: [linux-yocto][linux-yocto v5.10/standard/nxp-sdk-5.10/nxp-soc][PATCH] net: dsa: reference count the host mdb addresses on 20/05/2022 Xulin Sun wrote: > From: Vladimir Oltean <[email protected]> > > Currently any DSA switch that implements the multicast ops (properly, > that is) gets these errors after just sitting for a while, with at least > 2 ports bridged: > > [ 286.013814] mscc_felix 0000:00:00.5 swp3: failed (err=-2) to del object > (id=3) > > The reason has to do with this piece of code: > > netdev_for_each_lower_dev(dev, lower_dev, iter) > br_mdb_switchdev_host_port(dev, lower_dev, mp, type); > > called from: > > br_multicast_group_expired > -> br_multicast_host_leave > -> br_mdb_notify > -> br_mdb_switchdev_host > > Basically, that code is correct. It tells each switchdev port that the > host can leave that multicast group. But in the case of DSA, all user > ports are connected to the host through the same pipe. So, because DSA > translates a host MDB to a normal MDB on the CPU port, this means that > when all user ports leave a multicast group, DSA tries to remove it N > times from the CPU port. > > We should be reference-counting these addresses. > > Fixes: 5f4dbc50ce4d ("net: dsa: slave: Handle switchdev host mdb add/del") > Signed-off-by: Vladimir Oltean <[email protected]> > [Xulin: Pick patch from Link: > https://www.mail-archive.com/[email protected]/msg362526.html] > Signed-off-by: Xulin Sun <[email protected]> > --- > include/net/dsa.h | 9 +++++ > net/dsa/dsa2.c | 2 ++ > net/dsa/slave.c | 92 ++++++++++++++++++++++++++++++++++++++++++----- > 3 files changed, 94 insertions(+), 9 deletions(-) > > diff --git a/include/net/dsa.h b/include/net/dsa.h > index 35429a140dfa..38e9d449ce76 100644 > --- a/include/net/dsa.h > +++ b/include/net/dsa.h > @@ -251,6 +251,13 @@ struct dsa_link { > struct list_head list; > }; > > +struct dsa_host_addr { > + unsigned char addr[ETH_ALEN]; > + u16 vid; > + refcount_t refcount; > + struct list_head list; > +}; > + > struct dsa_switch { > bool setup; > > @@ -333,6 +340,8 @@ struct dsa_switch { > */ > bool mtu_enforcement_ingress; > > + struct list_head host_mdb; > + > size_t num_ports; > }; > > diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c > index f543fca6dfcb..045a39323bae 100644 > --- a/net/dsa/dsa2.c > +++ b/net/dsa/dsa2.c > @@ -417,6 +417,8 @@ static int dsa_switch_setup(struct dsa_switch *ds) > if (ds->setup) > return 0; > > + INIT_LIST_HEAD(&ds->host_mdb); > + > /* Initialize ds->phys_mii_mask before registering the slave MDIO bus > * driver and before ops->setup() has run, since the switch drivers and > * the slave MDIO bus driver rely on these values for probing PHY > diff --git a/net/dsa/slave.c b/net/dsa/slave.c > index 65b125bb3b86..dae7071dd4c4 100644 > --- a/net/dsa/slave.c > +++ b/net/dsa/slave.c > @@ -376,6 +376,87 @@ static int dsa_slave_vlan_add(struct net_device *dev, > return 0; > } > > +static struct dsa_host_addr * > +dsa_host_mdb_find(struct dsa_switch *ds, > + const struct switchdev_obj_port_mdb *mdb) > +{ > + struct dsa_host_addr *a; > + > + list_for_each_entry(a, &ds->host_mdb, list) > + if (ether_addr_equal(a->addr, mdb->addr) && a->vid == > mdb->vid) > + return a; > + > + return NULL; > +} > + > +/* DSA can directly translate this to a normal MDB add, but on the CPU port. > + * But because multiple user ports can join the same multicast group and the > + * bridge will emit a notification for each port, we need to add/delete the > + * entry towards the host only once, so we reference count it. > + */ > +static int dsa_host_mdb_add(struct dsa_port *dp, > + const struct switchdev_obj_port_mdb *mdb, > + struct switchdev_trans *trans) > +{ > + struct dsa_port *cpu_dp = dp->cpu_dp; > + struct dsa_switch *ds = dp->ds; > + struct dsa_host_addr *a; > + int err; > + > + /* Only the commit phase is refcounted, which means that for the > + * second, third, etc port which is member of this host address, > + * we'll call the prepare phase but never commit. > + */ > + if (switchdev_trans_ph_prepare(trans)) > + return dsa_port_mdb_add(cpu_dp, mdb, trans); > + > + a = dsa_host_mdb_find(ds, mdb); > + if (a) { > + refcount_inc(&a->refcount); > + return 0; > + } > + > + a = kzalloc(sizeof(*a), GFP_KERNEL); > + if (!a) > + return -ENOMEM; > + > + err = dsa_port_mdb_add(cpu_dp, mdb, trans); > + if (err) > + return err; > + > + ether_addr_copy(a->addr, mdb->addr); > + a->vid = mdb->vid; > + refcount_set(&a->refcount, 1); > + list_add_tail(&a->list, &ds->host_mdb); > + > + return 0; > +} > + > +static int dsa_host_mdb_del(struct dsa_port *dp, > + const struct switchdev_obj_port_mdb *mdb) > +{ > + struct dsa_port *cpu_dp = dp->cpu_dp; > + struct dsa_switch *ds = dp->ds; > + struct dsa_host_addr *a; > + int err; > + > + a = dsa_host_mdb_find(ds, mdb); > + if (!a) > + return -ENOENT; > + > + if (!refcount_dec_and_test(&a->refcount)) > + return 0; > + > + err = dsa_port_mdb_del(cpu_dp, mdb); > + if (err) > + return err; > + > + list_del(&a->list); > + kfree(a); > + > + return 0; > +} > + > static int dsa_slave_port_obj_add(struct net_device *dev, > const struct switchdev_obj *obj, > struct switchdev_trans *trans, > @@ -396,11 +477,7 @@ static int dsa_slave_port_obj_add(struct net_device *dev, > err = dsa_port_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj), trans); > break; > case SWITCHDEV_OBJ_ID_HOST_MDB: > - /* DSA can directly translate this to a normal MDB add, > - * but on the CPU port. > - */ > - err = dsa_port_mdb_add(dp->cpu_dp, SWITCHDEV_OBJ_PORT_MDB(obj), > - trans); > + err = dsa_host_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj), trans); > break; > case SWITCHDEV_OBJ_ID_PORT_VLAN: > err = dsa_slave_vlan_add(dev, obj, trans); > @@ -455,10 +532,7 @@ static int dsa_slave_port_obj_del(struct net_device *dev, > err = dsa_port_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj)); > break; > case SWITCHDEV_OBJ_ID_HOST_MDB: > - /* DSA can directly translate this to a normal MDB add, > - * but on the CPU port. > - */ > - err = dsa_port_mdb_del(dp->cpu_dp, SWITCHDEV_OBJ_PORT_MDB(obj)); > + err = dsa_host_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj)); > break; > case SWITCHDEV_OBJ_ID_PORT_VLAN: > err = dsa_slave_vlan_del(dev, obj); > -- > 2.36.0 >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#11318): https://lists.yoctoproject.org/g/linux-yocto/message/11318 Mute This Topic: https://lists.yoctoproject.org/mt/91224046/21656 Group Owner: [email protected] Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
