On Thu, 22 Aug 2019 at 23:13, Vivien Didelot <vivien.dide...@gmail.com> wrote: > > DSA currently programs a VLAN on the CPU port implicitly after the > related notifier is received by a switch. > > While we still need to do this transparent programmation of the DSA > links in the fabric, programming the CPU port this way may cause > problems in some corners such as the tag_8021q driver. > > Because the dedicated CPU port is specific to a slave, make their > programmation explicit a few layers up, in the slave code. > > Note that technically, DSA links have a dedicated CPU port as well, > but since they are only used as conduit between interconnected switches > of a fabric, programming them transparently this way is fine. > > Signed-off-by: Vivien Didelot <vivien.dide...@gmail.com> > --- > net/dsa/slave.c | 14 ++++++++++++++ > net/dsa/switch.c | 5 ++++- > 2 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/net/dsa/slave.c b/net/dsa/slave.c > index 82e48d247b81..8267c156a51a 100644 > --- a/net/dsa/slave.c > +++ b/net/dsa/slave.c > @@ -332,6 +332,10 @@ static int dsa_slave_vlan_add(struct net_device *dev, > if (err) > return err; > > + err = dsa_port_vlan_add(dp->cpu_dp, &vlan, trans); > + if (err) > + return err; > + > return 0; > } > > @@ -383,6 +387,9 @@ static int dsa_slave_vlan_del(struct net_device *dev, > if (dp->bridge_dev && !br_vlan_enabled(dp->bridge_dev)) > return 0; > > + /* Do not deprogram the CPU port as it may be shared with other user > + * ports which can be members of this VLAN as well. > + */
+1 for the comments, the deletion of dp->cpu_dp is less likely to get patched into the code in the future now. > return dsa_port_vlan_del(dp, SWITCHDEV_OBJ_PORT_VLAN(obj)); > } > > @@ -1121,6 +1128,10 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device > *dev, __be16 proto, > if (ret && ret != -EOPNOTSUPP) > return ret; > > + ret = dsa_port_vid_add(dp->cpu_dp, vid, 0); > + if (ret && ret != -EOPNOTSUPP) > + return ret; > + I think it's worth understanding what the EOPNOTSUPP -> 0 is avoiding. > return 0; > } > > @@ -1151,6 +1162,9 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device > *dev, __be16 proto, > if (ret == -EOPNOTSUPP) > ret = 0; > > + /* Do not deprogram the CPU port as it may be shared with other user > + * ports which can be members of this VLAN as well. > + */ > return ret; > } > > diff --git a/net/dsa/switch.c b/net/dsa/switch.c > index 489eb7b430a4..6a9607518823 100644 > --- a/net/dsa/switch.c > +++ b/net/dsa/switch.c > @@ -232,7 +232,7 @@ static bool dsa_switch_vlan_match(struct dsa_switch *ds, > int port, > if (ds->index == info->sw_index && port == info->port) > return true; > > - if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) > + if (dsa_is_dsa_port(ds, port)) Much better, thank you. > return true; > > return false; > @@ -288,6 +288,9 @@ static int dsa_switch_vlan_del(struct dsa_switch *ds, > if (ds->index == info->sw_index) > return ds->ops->port_vlan_del(ds, info->port, info->vlan); > > + /* Do not deprogram the DSA links as they may be used as conduit > + * for other VLAN members in the fabric. > + */ > return 0; > } > > -- > 2.23.0 >
