| Message ID | 1432601703-26774-1-git-send-email-simon.horman@netronome.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, May 25, 2015 at 5:55 PM, Simon Horman <simon.horman@netronome.com> wrote: > This will occur anyway if the 8021q module is loaded as it will > call rocker_port_vlan_rx_add_vid for vlan 0. This code is here > to handle the case where the 8021q module is not loaded. > > This patch also handles the case where the 8021q is unloaded > removing all VLANs from all ports. > > This change should not affect bridging, although the rules are > harmlessly installed anyway. This is in keeping with the behaviour > for VLANs when the 8021q modules is loaded. > > To aid implementation of the above provide a helper > and use it to replace some existing code. > > Signed-off-by: Simon Horman <simon.horman@netronome.com> > --- > drivers/net/ethernet/rocker/rocker.c | 51 +++++++++++++++++++++++++++--------- > 1 file changed, 39 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c > index 36f7edfc3c7a..bc00e0abd8b6 100644 > --- a/drivers/net/ethernet/rocker/rocker.c > +++ b/drivers/net/ethernet/rocker/rocker.c > @@ -3720,6 +3720,19 @@ static int rocker_port_router_mac(struct rocker_port *rocker_port, > return err; > } > > +static int rocker_port_vlan_rx_vid(struct rocker_port *rocker_port, > + enum switchdev_trans trans, int flag, > + u16 vid) > +{ > + int err; > + > + err = rocker_port_vlan(rocker_port, trans, flag, vid); > + if (err) > + return err; > + > + return rocker_port_router_mac(rocker_port, trans, flag, htons(vid)); > +} > + > static int rocker_port_fwding(struct rocker_port *rocker_port, > enum switchdev_trans trans) > { > @@ -4009,6 +4022,16 @@ static int rocker_port_open(struct net_device *dev) > goto err_request_rx_irq; > } > > + /* By default accept untagged vlan packets. > + * > + * This will occur anyway if the 8021q module is loaded as it will > + * call rocker_port_vlan_rx_add_vid for vlan 0. This code is here > + * to handle the case where the 8021q module is not loaded. > + */ > + err = rocker_port_vlan_rx_vid(rocker_port, SWITCHDEV_TRANS_NONE, 0, 0); > + if (err) > + goto err_fwd_enable; > + > err = rocker_port_fwd_enable(rocker_port, SWITCHDEV_TRANS_NONE); > if (err) > goto err_fwd_enable; > @@ -4187,29 +4210,33 @@ static int rocker_port_vlan_rx_add_vid(struct net_device *dev, > __be16 proto, u16 vid) > { > struct rocker_port *rocker_port = netdev_priv(dev); > - int err; > - > - err = rocker_port_vlan(rocker_port, SWITCHDEV_TRANS_NONE, 0, vid); > - if (err) > - return err; > > - return rocker_port_router_mac(rocker_port, SWITCHDEV_TRANS_NONE, > - 0, htons(vid)); > + return rocker_port_vlan_rx_vid(rocker_port, SWITCHDEV_TRANS_NONE, > + 0, vid); > } > > static int rocker_port_vlan_rx_kill_vid(struct net_device *dev, > __be16 proto, u16 vid) > { > struct rocker_port *rocker_port = netdev_priv(dev); > - int err; > + int err, i; > > - err = rocker_port_router_mac(rocker_port, SWITCHDEV_TRANS_NONE, > - ROCKER_OP_FLAG_REMOVE, htons(vid)); > + err = rocker_port_vlan_rx_vid(rocker_port, SWITCHDEV_TRANS_NONE, > + ROCKER_OP_FLAG_REMOVE, vid); > if (err) > return err; > > - return rocker_port_vlan(rocker_port, SWITCHDEV_TRANS_NONE, > - ROCKER_OP_FLAG_REMOVE, vid); > + /* If no vlans are set then the last one has been removed; > + * restore the default behaviour of accepting untagged packets. > + * > + * This may occur if the 8021q module is unloaded. > + */ > + for (i = 0; i < ROCKER_VLAN_BITMAP_LEN; i++) > + if (rocker_port->vlan_bitmap[i]) > + return 0; > + return rocker_port_vlan_rx_vid(rocker_port, SWITCHDEV_TRANS_NONE, > + 0, 0); > + > } > > static int rocker_port_get_phys_port_name(struct net_device *dev, > -- > 2.1.4 > Hi Simon, Thanks for looking into this one. I looked at your patch and the code and I think we can streamline it a little bit more. For the no-8021q-module case we use rocker_port_vlan_add() and rocker_port_vlan_del() to add/del bridge vlans. We should be able to move those functions up in the file so they can be called from rocker_port_vlan_rx_add_vid() and rocker_port_vlan_rx_kill_vid(), passing trans=SWITCHDEV_TRANS_NONE, but only if vid != 0. Next, like in your patch, we need to call rocker_port_vlan_add() in rocker_port_open(), passing in vid=0 for untagged. And in rocker_port_stop(), call rocker_port_vlan_del(), again passing in vid=0. To summarize: Call rocker_port_vlan_add() from: 1) rocker_port_open with vid=0 2) rocker_port_vlans_add() // bridge vlan 3) rocker_port_vlan_rx_add_vid() if vid != 0 // 8021q vlan Call rocker_port_vlan_del() from: 1) rocker_port_stop with vid=0 2) rocker_port_vlans_del() // bridge vlan 3) rocker_port_vlan_rx_kill_vid() if vid != 0 // 8021q vlan Does this look right? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 26, 2015 at 12:28 AM, Scott Feldman <sfeldma@gmail.com> wrote: > On Mon, May 25, 2015 at 5:55 PM, Simon Horman > <simon.horman@netronome.com> wrote: >> This will occur anyway if the 8021q module is loaded as it will >> call rocker_port_vlan_rx_add_vid for vlan 0. This code is here >> to handle the case where the 8021q module is not loaded. >> >> This patch also handles the case where the 8021q is unloaded >> removing all VLANs from all ports. >> >> This change should not affect bridging, although the rules are >> harmlessly installed anyway. This is in keeping with the behaviour >> for VLANs when the 8021q modules is loaded. >> >> To aid implementation of the above provide a helper >> and use it to replace some existing code. >> >> Signed-off-by: Simon Horman <simon.horman@netronome.com> [cut] > > Hi Simon, > > Thanks for looking into this one. I looked at your patch and the code > and I think we can streamline it a little bit more. For the > no-8021q-module case we use rocker_port_vlan_add() and > rocker_port_vlan_del() to add/del bridge vlans. We should be able to > move those functions up in the file so they can be called from > rocker_port_vlan_rx_add_vid() and rocker_port_vlan_rx_kill_vid(), > passing trans=SWITCHDEV_TRANS_NONE, but only if vid != 0. Next, like > in your patch, we need to call rocker_port_vlan_add() in > rocker_port_open(), passing in vid=0 for untagged. And in > rocker_port_stop(), call rocker_port_vlan_del(), again passing in > vid=0. > > To summarize: > > Call rocker_port_vlan_add() from: > > 1) rocker_port_open with vid=0 > 2) rocker_port_vlans_add() // bridge vlan > 3) rocker_port_vlan_rx_add_vid() if vid != 0 // 8021q vlan > > Call rocker_port_vlan_del() from: > > 1) rocker_port_stop with vid=0 > 2) rocker_port_vlans_del() // bridge vlan > 3) rocker_port_vlan_rx_kill_vid() if vid != 0 // 8021q vlan > > Does this look right? Hmmmm...things get simpler if we removed support for 8021q module in rocker driver by removing NETIF_F_HW_VLAN_CTAG_FILTER. That gets rid of rocker_port_vlan_rx_add_vid() and rocker_port_vlan_rx_kill_vid(). Leaving us with the bridge VLAN interface to add/del/show vlans on the port. I'm wondering if we can also avoid setting up untagged traffic on the port during port open by requiring a explicit command on the port from the user: bridge vlan add vid 0 dev DEV master self // enable untagged traffic on port Do you have a requirement for 8021q module? I'm leaning towards a clean break from 8021q and using just the built-in bridge VLAN support. I'm curious if others have an opinion about 8021q module used with switchdev devices. -scott -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Scott, On Tue, May 26, 2015 at 02:04:00AM -0700, Scott Feldman wrote: > On Tue, May 26, 2015 at 12:28 AM, Scott Feldman <sfeldma@gmail.com> wrote: > > On Mon, May 25, 2015 at 5:55 PM, Simon Horman > > <simon.horman@netronome.com> wrote: > >> This will occur anyway if the 8021q module is loaded as it will > >> call rocker_port_vlan_rx_add_vid for vlan 0. This code is here > >> to handle the case where the 8021q module is not loaded. > >> > >> This patch also handles the case where the 8021q is unloaded > >> removing all VLANs from all ports. > >> > >> This change should not affect bridging, although the rules are > >> harmlessly installed anyway. This is in keeping with the behaviour > >> for VLANs when the 8021q modules is loaded. > >> > >> To aid implementation of the above provide a helper > >> and use it to replace some existing code. > >> > >> Signed-off-by: Simon Horman <simon.horman@netronome.com> > > [cut] > > > > > Hi Simon, > > > > Thanks for looking into this one. I looked at your patch and the code > > and I think we can streamline it a little bit more. For the > > no-8021q-module case we use rocker_port_vlan_add() and > > rocker_port_vlan_del() to add/del bridge vlans. We should be able to > > move those functions up in the file so they can be called from > > rocker_port_vlan_rx_add_vid() and rocker_port_vlan_rx_kill_vid(), > > passing trans=SWITCHDEV_TRANS_NONE, but only if vid != 0. Next, like > > in your patch, we need to call rocker_port_vlan_add() in > > rocker_port_open(), passing in vid=0 for untagged. And in > > rocker_port_stop(), call rocker_port_vlan_del(), again passing in > > vid=0. > > > > To summarize: > > > > Call rocker_port_vlan_add() from: > > > > 1) rocker_port_open with vid=0 > > 2) rocker_port_vlans_add() // bridge vlan > > 3) rocker_port_vlan_rx_add_vid() if vid != 0 // 8021q vlan > > > > Call rocker_port_vlan_del() from: > > > > 1) rocker_port_stop with vid=0 > > 2) rocker_port_vlans_del() // bridge vlan > > 3) rocker_port_vlan_rx_kill_vid() if vid != 0 // 8021q vlan > > > > Does this look right? It seems like it ought to work, I can try implementing the above idea if you think it is worthwhile. Can I clarify that its ok to ignore vid != 0 in rocker_port_vlan_rx_add_vid() and rocker_port_vlan_rx_kill_vid()? If so I think that leads to an easy simplification of my proposed change to rocker_port_vlan_rx_kill_vid(): the logic to restore vlan 0 if no vlans are present can be dropped. Of course your suggestion goes further than that. > Hmmmm...things get simpler if we removed support for 8021q module in > rocker driver by removing NETIF_F_HW_VLAN_CTAG_FILTER. That gets rid > of rocker_port_vlan_rx_add_vid() and rocker_port_vlan_rx_kill_vid(). > Leaving us with the bridge VLAN interface to add/del/show vlans on the > port. I'm wondering if we can also avoid setting up untagged traffic > on the port during port open by requiring a explicit command on the > port from the user: > > bridge vlan add vid 0 dev DEV master self // enable untagged > traffic on port I have some questions about that approach: * Does that behaviour differ from other devices (that don't set NETIF_F_HW_VLAN_CTAG_FILTER)? It seems like it may be an extra unnecessary step to me. * Does that behaviour change the current behaviour supported by rocker? If so it seems unwise to change it. * Does the scheme described above work when rocker ports are not bridged? This is the scenario I am interested in at this time. > > Do you have a requirement for 8021q module? I'm leaning towards a > clean break from 8021q and using just the built-in bridge VLAN > support. I'm curious if others have an opinion about 8021q module > used with switchdev devices. I do not have a requirement on the 8021q module at this time. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 26, 2015 at 6:07 PM, Simon Horman <simon.horman@netronome.com> wrote: > Hi Scott, > > On Tue, May 26, 2015 at 02:04:00AM -0700, Scott Feldman wrote: >> On Tue, May 26, 2015 at 12:28 AM, Scott Feldman <sfeldma@gmail.com> wrote: >> > On Mon, May 25, 2015 at 5:55 PM, Simon Horman >> > <simon.horman@netronome.com> wrote: >> >> This will occur anyway if the 8021q module is loaded as it will >> >> call rocker_port_vlan_rx_add_vid for vlan 0. This code is here >> >> to handle the case where the 8021q module is not loaded. >> >> >> >> This patch also handles the case where the 8021q is unloaded >> >> removing all VLANs from all ports. >> >> >> >> This change should not affect bridging, although the rules are >> >> harmlessly installed anyway. This is in keeping with the behaviour >> >> for VLANs when the 8021q modules is loaded. >> >> >> >> To aid implementation of the above provide a helper >> >> and use it to replace some existing code. >> >> >> >> Signed-off-by: Simon Horman <simon.horman@netronome.com> >> >> [cut] >> >> > >> > Hi Simon, >> > >> > Thanks for looking into this one. I looked at your patch and the code >> > and I think we can streamline it a little bit more. For the >> > no-8021q-module case we use rocker_port_vlan_add() and >> > rocker_port_vlan_del() to add/del bridge vlans. We should be able to >> > move those functions up in the file so they can be called from >> > rocker_port_vlan_rx_add_vid() and rocker_port_vlan_rx_kill_vid(), >> > passing trans=SWITCHDEV_TRANS_NONE, but only if vid != 0. Next, like >> > in your patch, we need to call rocker_port_vlan_add() in >> > rocker_port_open(), passing in vid=0 for untagged. And in >> > rocker_port_stop(), call rocker_port_vlan_del(), again passing in >> > vid=0. >> > >> > To summarize: >> > >> > Call rocker_port_vlan_add() from: >> > >> > 1) rocker_port_open with vid=0 >> > 2) rocker_port_vlans_add() // bridge vlan >> > 3) rocker_port_vlan_rx_add_vid() if vid != 0 // 8021q vlan >> > >> > Call rocker_port_vlan_del() from: >> > >> > 1) rocker_port_stop with vid=0 >> > 2) rocker_port_vlans_del() // bridge vlan >> > 3) rocker_port_vlan_rx_kill_vid() if vid != 0 // 8021q vlan >> > >> > Does this look right? > > It seems like it ought to work, I can try implementing the above > idea if you think it is worthwhile. > > Can I clarify that its ok to ignore vid != 0 in > rocker_port_vlan_rx_add_vid() and rocker_port_vlan_rx_kill_vid()? > > If so I think that leads to an easy simplification of > my proposed change to rocker_port_vlan_rx_kill_vid(): > the logic to restore vlan 0 if no vlans are present can be dropped. > > Of course your suggestion goes further than that. > >> Hmmmm...things get simpler if we removed support for 8021q module in >> rocker driver by removing NETIF_F_HW_VLAN_CTAG_FILTER. That gets rid >> of rocker_port_vlan_rx_add_vid() and rocker_port_vlan_rx_kill_vid(). >> Leaving us with the bridge VLAN interface to add/del/show vlans on the >> port. I'm wondering if we can also avoid setting up untagged traffic >> on the port during port open by requiring a explicit command on the >> port from the user: >> >> bridge vlan add vid 0 dev DEV master self // enable untagged >> traffic on port > > I have some questions about that approach: > > * Does that behaviour differ from other devices > (that don't set NETIF_F_HW_VLAN_CTAG_FILTER)? > It seems like it may be an extra unnecessary step to me. The answer is which drivers use bridge_setlink/bridge_dellink for VLANs, which is none outside of switchdev drivers. There are some drivers that use bridge_setlink (benet, i40e, ixgbe) for setting hw_mode, but ignore the IFLA_BRIDGE_VLAN_INFO attr. > * Does that behaviour change the current behaviour supported by rocker? > If so it seems unwise to change it. I did some experiments, and with rocker there currently are two ways to enable a port for rx untagged traffic: 1) Load the 8021q driver, which in turn will add vid=0 on interface supporting NETIF_F_HW_VLAN_CTAG_FILTER, when interface opens. 2) Without 8021q driver loaded, by manually adding vid=0 to interface using: bridge vlan add vid=0 dev DEV self > * Does the scheme described above work when rocker ports are not bridged? > This is the scenario I am interested in at this time. Yes, confusingly, since the user command is "bridge vlan". But the command works for bridged or un-bridged ports. In either case, by targeting "self", the port driver's bridge_setlink/bridge_getlink are called. For switchdev port drivers, that means a call into switchdev_port_bridge_setlink/switchdev_port_bridge_dellink. In rocker, as an experiment, I removed NETIF_F_HW_VLAN_CTAG_FILTER (and associated ndo ops), and was able to run/pass all my tests. For the tests with unbridged ports, I had to run "bridge vlan add vid=0 dev DEV self" before passing traffic. The advantages of using bridge_setlink/bridge_dellink for VLANs over 8021q module are: 1) single API to implement in port driver 2) can handle VLAN ranges efficiently (thanks Roopa!) 3) switchdev already handles stacked driver case (and transaction model) 4) includes support for PVID on bridge 5) includes support for egress untagged property on port 6) user space command included with iproute2 pkg I see no disadvantages over using 8021q module. One thing that's missing or incomplete is support for bridge_getlink for VLANs. Currently switchdev defaults to ndo_dflt_bridge_getlink() which doesn't return any IFLA_BRIDGE_VLAN_INFO attrs. And the "bridge vlan show" command might need some help in displaying that info, it it was available. Cc:ing Ronen as he's been looking into bridge_getlink for switchdev. So I think since yesterday I'm becoming even more biased against 8021q for switchdev. For rocker, I think we need to figure out if the driver does an automatic vid=0 install, say on ndo_open, and then a removal on ndo_stop. That would simplify the logic in rocker_port_bridge_join()/leave(). Doing the automatic add makes the device more like a NIC where passing untagged traffic is the baseline default. -scott -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Scott, On Wed, May 27, 2015 at 12:34:57AM -0700, Scott Feldman wrote: > On Tue, May 26, 2015 at 6:07 PM, Simon Horman > <simon.horman@netronome.com> wrote: [snip] > > I have some questions about that approach: > > > > * Does that behaviour differ from other devices > > (that don't set NETIF_F_HW_VLAN_CTAG_FILTER)? > > It seems like it may be an extra unnecessary step to me. > > The answer is which drivers use bridge_setlink/bridge_dellink for > VLANs, which is none outside of switchdev drivers. > > There are some drivers that use bridge_setlink (benet, i40e, ixgbe) > for setting hw_mode, but ignore the IFLA_BRIDGE_VLAN_INFO attr. > > > * Does that behaviour change the current behaviour supported by rocker? > > If so it seems unwise to change it. > > I did some experiments, and with rocker there currently are two ways > to enable a port for rx untagged traffic: > > 1) Load the 8021q driver, which in turn will add vid=0 on interface > supporting NETIF_F_HW_VLAN_CTAG_FILTER, when interface opens. > > 2) Without 8021q driver loaded, by manually adding vid=0 to interface using: > > bridge vlan add vid=0 dev DEV self > > > * Does the scheme described above work when rocker ports are not bridged? > > This is the scenario I am interested in at this time. > > Yes, confusingly, since the user command is "bridge vlan". But the > command works for bridged or un-bridged ports. In either case, by > targeting "self", the port driver's bridge_setlink/bridge_getlink are > called. For switchdev port drivers, that means a call into > switchdev_port_bridge_setlink/switchdev_port_bridge_dellink. That is indeed confusing. > In rocker, as an experiment, I removed NETIF_F_HW_VLAN_CTAG_FILTER > (and associated ndo ops), and was able to run/pass all my tests. For > the tests with unbridged ports, I had to run "bridge vlan add vid=0 > dev DEV self" before passing traffic. The advantages of using > bridge_setlink/bridge_dellink for VLANs over 8021q module are: > > 1) single API to implement in port driver > 2) can handle VLAN ranges efficiently (thanks Roopa!) > 3) switchdev already handles stacked driver case (and transaction model) > 4) includes support for PVID on bridge > 5) includes support for egress untagged property on port > 6) user space command included with iproute2 pkg > > I see no disadvantages over using 8021q module. > > One thing that's missing or incomplete is support for bridge_getlink > for VLANs. Currently switchdev defaults to ndo_dflt_bridge_getlink() > which doesn't return any IFLA_BRIDGE_VLAN_INFO attrs. And the "bridge > vlan show" command might need some help in displaying that info, it it > was available. Cc:ing Ronen as he's been looking into bridge_getlink > for switchdev. > > So I think since yesterday I'm becoming even more biased against 8021q > for switchdev. For rocker, I think we need to figure out if the > driver does an automatic vid=0 install, say on ndo_open, and then a > removal on ndo_stop. That would simplify the logic in > rocker_port_bridge_join()/leave(). Doing the automatic add makes the > device more like a NIC where passing untagged traffic is the baseline > default. My feeling at this time is that it would be nice to to the automatic add to make rocker feel more like a NIC. That is the behaviour that I for one would expect without other knowledge of the situation. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c index 36f7edfc3c7a..bc00e0abd8b6 100644 --- a/drivers/net/ethernet/rocker/rocker.c +++ b/drivers/net/ethernet/rocker/rocker.c @@ -3720,6 +3720,19 @@ static int rocker_port_router_mac(struct rocker_port *rocker_port, return err; } +static int rocker_port_vlan_rx_vid(struct rocker_port *rocker_port, + enum switchdev_trans trans, int flag, + u16 vid) +{ + int err; + + err = rocker_port_vlan(rocker_port, trans, flag, vid); + if (err) + return err; + + return rocker_port_router_mac(rocker_port, trans, flag, htons(vid)); +} + static int rocker_port_fwding(struct rocker_port *rocker_port, enum switchdev_trans trans) { @@ -4009,6 +4022,16 @@ static int rocker_port_open(struct net_device *dev) goto err_request_rx_irq; } + /* By default accept untagged vlan packets. + * + * This will occur anyway if the 8021q module is loaded as it will + * call rocker_port_vlan_rx_add_vid for vlan 0. This code is here + * to handle the case where the 8021q module is not loaded. + */ + err = rocker_port_vlan_rx_vid(rocker_port, SWITCHDEV_TRANS_NONE, 0, 0); + if (err) + goto err_fwd_enable; + err = rocker_port_fwd_enable(rocker_port, SWITCHDEV_TRANS_NONE); if (err) goto err_fwd_enable; @@ -4187,29 +4210,33 @@ static int rocker_port_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid) { struct rocker_port *rocker_port = netdev_priv(dev); - int err; - - err = rocker_port_vlan(rocker_port, SWITCHDEV_TRANS_NONE, 0, vid); - if (err) - return err; - return rocker_port_router_mac(rocker_port, SWITCHDEV_TRANS_NONE, - 0, htons(vid)); + return rocker_port_vlan_rx_vid(rocker_port, SWITCHDEV_TRANS_NONE, + 0, vid); } static int rocker_port_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid) { struct rocker_port *rocker_port = netdev_priv(dev); - int err; + int err, i; - err = rocker_port_router_mac(rocker_port, SWITCHDEV_TRANS_NONE, - ROCKER_OP_FLAG_REMOVE, htons(vid)); + err = rocker_port_vlan_rx_vid(rocker_port, SWITCHDEV_TRANS_NONE, + ROCKER_OP_FLAG_REMOVE, vid); if (err) return err; - return rocker_port_vlan(rocker_port, SWITCHDEV_TRANS_NONE, - ROCKER_OP_FLAG_REMOVE, vid); + /* If no vlans are set then the last one has been removed; + * restore the default behaviour of accepting untagged packets. + * + * This may occur if the 8021q module is unloaded. + */ + for (i = 0; i < ROCKER_VLAN_BITMAP_LEN; i++) + if (rocker_port->vlan_bitmap[i]) + return 0; + return rocker_port_vlan_rx_vid(rocker_port, SWITCHDEV_TRANS_NONE, + 0, 0); + } static int rocker_port_get_phys_port_name(struct net_device *dev,
This will occur anyway if the 8021q module is loaded as it will call rocker_port_vlan_rx_add_vid for vlan 0. This code is here to handle the case where the 8021q module is not loaded. This patch also handles the case where the 8021q is unloaded removing all VLANs from all ports. This change should not affect bridging, although the rules are harmlessly installed anyway. This is in keeping with the behaviour for VLANs when the 8021q modules is loaded. To aid implementation of the above provide a helper and use it to replace some existing code. Signed-off-by: Simon Horman <simon.horman@netronome.com> --- drivers/net/ethernet/rocker/rocker.c | 51 +++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 12 deletions(-)