On Tue, May 26, 2015 at 12:28 AM, Scott Feldman <sfel...@gmail.com> wrote: > On Mon, May 25, 2015 at 5:55 PM, Simon Horman > <simon.hor...@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.hor...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
