Message ID | 20200910150738.mwhh2i6j2qgacqev@skbuf |
---|---|
State | New |
Headers | show |
Series | VLAN filtering with DSA | expand |
On 9/10/2020 8:07 AM, Vladimir Oltean wrote: > Hi, > > Problem background: > > Most DSA switch tags shift the EtherType to the right, causing the > master to not parse the VLAN as VLAN. > However, not all switches do that (example: tail tags), and if the DSA > master has "rx-vlan-filter: on" in ethtool -k, then we have a problem. > Therefore, I was thinking we could populate the VLAN table of the > master, just in case, so that it can work with a VLAN filtering master. > It would look something like this: Yes, doing what you suggest would make perfect sense for a DSA master that is capable of VLAN filtering, I did encounter that problem with e1000 and the dsa-loop.c mockup driver while working on a mock-up 802.1Q data path. > > diff --git a/net/dsa/slave.c b/net/dsa/slave.c > index 19b98a7231ec..b8aca2301c59 100644 > --- a/net/dsa/slave.c > +++ b/net/dsa/slave.c > @@ -307,9 +307,10 @@ static int dsa_slave_vlan_add(struct net_device *dev, > const struct switchdev_obj *obj, > struct switchdev_trans *trans) > { > + struct net_device *master = dsa_slave_to_master(dev); > struct dsa_port *dp = dsa_slave_to_port(dev); > struct switchdev_obj_port_vlan vlan; > - int err; > + int vid, err; > > if (obj->orig_dev != dev) > return -EOPNOTSUPP; > @@ -336,6 +337,12 @@ static int dsa_slave_vlan_add(struct net_device *dev, > if (err) > return err; > > + for (vid = vlan.vid_begin; vid <= vlan.vid_end; vid++) { > + err = vlan_vid_add(master, htons(ETH_P_8021Q), vid); > + if (err) > + return err; > + } > + > return 0; > } > > @@ -379,8 +386,10 @@ static int dsa_slave_port_obj_add(struct net_device *dev, > static int dsa_slave_vlan_del(struct net_device *dev, > const struct switchdev_obj *obj) > { > + struct net_device *master = dsa_slave_to_master(dev); > struct dsa_port *dp = dsa_slave_to_port(dev); > struct switchdev_obj_port_vlan *vlan; > + int vid, err; > > if (obj->orig_dev != dev) > return -EOPNOTSUPP; > @@ -396,7 +405,14 @@ static int dsa_slave_vlan_del(struct net_device *dev, > /* 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 dsa_port_vlan_del(dp, vlan); > + err = dsa_port_vlan_del(dp, vlan); > + if (err) > + return err; > + > + for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) > + vlan_vid_del(master, htons(ETH_P_8021Q), vid); > + > + return 0; > } > > static int dsa_slave_port_obj_del(struct net_device *dev, > @@ -1241,6 +1257,7 @@ static int dsa_slave_get_ts_info(struct net_device *dev, > static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto, > u16 vid) > { > + struct net_device *master = dsa_slave_to_master(dev); > struct dsa_port *dp = dsa_slave_to_port(dev); > struct switchdev_obj_port_vlan vlan = { > .obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN, > @@ -1294,12 +1311,13 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto, > if (ret) > return ret; > > - return 0; > + return vlan_vid_add(master, proto, vid); > } > > static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, > u16 vid) > { > + struct net_device *master = dsa_slave_to_master(dev); > struct dsa_port *dp = dsa_slave_to_port(dev); > struct switchdev_obj_port_vlan vlan = { > .vid_begin = vid, > @@ -1332,7 +1350,13 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, > /* 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 dsa_port_vlan_del(dp, &vlan); > + ret = dsa_port_vlan_del(dp, &vlan); > + if (ret) > + return ret; > + > + vlan_vid_del(master, proto, vid); > + > + return 0; > } > > struct dsa_hw_port { >
On Thu, Sep 10, 2020 at 11:42:02AM -0700, Florian Fainelli wrote: > On 9/10/2020 8:07 AM, Vladimir Oltean wrote: > Yes, doing what you suggest would make perfect sense for a DSA master that > is capable of VLAN filtering, I did encounter that problem with e1000 and > the dsa-loop.c mockup driver while working on a mock-up 802.1Q data path. Yes, I have another patch where I add those VLANs from tag_8021q.c which I did not show here. But if the DSA switch that uses tag_8021q is cascaded to another one, that's of little use if the upper switch does not propagate that configuration to its own upstream. Thanks, -Vladimir
On 9/10/2020 12:01 PM, Vladimir Oltean wrote: > On Thu, Sep 10, 2020 at 11:42:02AM -0700, Florian Fainelli wrote: >> On 9/10/2020 8:07 AM, Vladimir Oltean wrote: >> Yes, doing what you suggest would make perfect sense for a DSA master that >> is capable of VLAN filtering, I did encounter that problem with e1000 and >> the dsa-loop.c mockup driver while working on a mock-up 802.1Q data path. > > Yes, I have another patch where I add those VLANs from tag_8021q.c which > I did not show here. > > But if the DSA switch that uses tag_8021q is cascaded to another one, > that's of little use if the upper switch does not propagate that > configuration to its own upstream. Yes, that would not work. As soon as you have a bridge spanning any of those switches, does not the problem go away by virtue of the switch port forcing the DSA master/upstream to be in promiscuous mode?
On Fri, Sep 11, 2020 at 07:30:42PM +0300, Vladimir Oltean wrote: > Currently there are other places in the network stack that don't really > work with a network interface that has problems with an interface that > has "rx-vlan-filter: on" in ethtool -k. Wow, I should learn how to write. I meant: Currently there are other places in the network stack that don't really work with a network interface that has "rx-vlan-filter: on" in ethtool -k.
diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 19b98a7231ec..b8aca2301c59 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -307,9 +307,10 @@ static int dsa_slave_vlan_add(struct net_device *dev, const struct switchdev_obj *obj, struct switchdev_trans *trans) { + struct net_device *master = dsa_slave_to_master(dev); struct dsa_port *dp = dsa_slave_to_port(dev); struct switchdev_obj_port_vlan vlan; - int err; + int vid, err; if (obj->orig_dev != dev) return -EOPNOTSUPP; @@ -336,6 +337,12 @@ static int dsa_slave_vlan_add(struct net_device *dev, if (err) return err; + for (vid = vlan.vid_begin; vid <= vlan.vid_end; vid++) { + err = vlan_vid_add(master, htons(ETH_P_8021Q), vid); + if (err) + return err; + } + return 0; } @@ -379,8 +386,10 @@ static int dsa_slave_port_obj_add(struct net_device *dev, static int dsa_slave_vlan_del(struct net_device *dev, const struct switchdev_obj *obj) { + struct net_device *master = dsa_slave_to_master(dev); struct dsa_port *dp = dsa_slave_to_port(dev); struct switchdev_obj_port_vlan *vlan; + int vid, err; if (obj->orig_dev != dev) return -EOPNOTSUPP; @@ -396,7 +405,14 @@ static int dsa_slave_vlan_del(struct net_device *dev, /* 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 dsa_port_vlan_del(dp, vlan); + err = dsa_port_vlan_del(dp, vlan); + if (err) + return err; + + for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) + vlan_vid_del(master, htons(ETH_P_8021Q), vid); + + return 0; } static int dsa_slave_port_obj_del(struct net_device *dev, @@ -1241,6 +1257,7 @@ static int dsa_slave_get_ts_info(struct net_device *dev, static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid) { + struct net_device *master = dsa_slave_to_master(dev); struct dsa_port *dp = dsa_slave_to_port(dev); struct switchdev_obj_port_vlan vlan = { .obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN, @@ -1294,12 +1311,13 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto, if (ret) return ret; - return 0; + return vlan_vid_add(master, proto, vid); } static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid) { + struct net_device *master = dsa_slave_to_master(dev); struct dsa_port *dp = dsa_slave_to_port(dev); struct switchdev_obj_port_vlan vlan = { .vid_begin = vid, @@ -1332,7 +1350,13 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, /* 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 dsa_port_vlan_del(dp, &vlan); + ret = dsa_port_vlan_del(dp, &vlan); + if (ret) + return ret; + + vlan_vid_del(master, proto, vid); + + return 0; } struct dsa_hw_port {