On Mon, Jun 1, 2015 at 5:18 PM, Vivien Didelot <vivien.dide...@savoirfairelinux.com> wrote: > > On May 29, 2015, at 1:02 AM, Scott Feldman sfel...@gmail.com wrote: >> On Thu, May 28, 2015 at 2:37 PM, Vivien Didelot >> <vivien.dide...@savoirfairelinux.com> wrote: >>> This RFC is based on v4.1-rc3. >>> >>> It is meant to get a glance to the commits responsible to implement the >>> necessary NDOs between DSA and the Marvell 88E6352 switch driver. >>> >>> With this support, I am able to create VLANs with (un)tagged ports, setting >>> their default VID, from a bridge. >>> >>> To create a bridge containing all switch ports, with a VLAN ID 400, swp2 and >>> swp3 untagged (pvid), and swp4 tagged, the userspace commands look like >>> this: >>> >>> ip link add name br0 type bridge >>> [...] >>> ip link set dev swp2 up master br0 >>> [...] >>> bridge vlan add vid 400 pvid untagged dev swp2 >>> bridge vlan add vid 400 pvid untagged dev swp3 >>> bridge vlan add vid 400 dev swp4 >>> [...] >>> ip link add link br0 name br0.400 type vlan id 400 >>> [...] >>> bridge vlan add dev br0 vid 400 self >>> >>> The code is currently being rebased to the latest net-next/master. >>> >>> Seems like the way to go now is through switchdev attr getter/setter... >> >> Indeed, for dsa_slave you should be able to port this to switchdev and >> set your ndo_bridge_setlink/dellink handlers to >> switchdev_port_bridge_setlink/dellink. (And also implement the >> switchdev ops for vlans). >> >> If you use switchdev_port_bridge_setlink/dellink, you shouldn't need >> to implement ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid at all. > > Scott, > > In fact I have to define these ndo, otherwise I get the "Buggy VLAN > acceleration in driver!" warning from net/core/dev.c and the switch > ports won't register. > > I'm actually defining a noop function for them in dsa_slave_netdev_ops. > > Is it correct to set NETIF_F_HW_VLAN_CTAG_FILTER in slave_dev->features?
If your nooping ndo VLAN ops then just remove setting NETIF_F_HW_VLAN_CTAG_FILTER and then you can remove the noop funcs. >> The setlink/dellink callbacks will give the same info (and more, e.g. >> pvid, untagged flags) and you'll automatically get support for stacked >> drivers, for example if you bonded swp2/3 and then included that bond >> in your vlan bridge. Your commands will be slightly modified: when >> adding the vid to the port, specify master and self: >> >> bridge vlan add vid 400 dev swp4 master self > > Thanks it works! Now the switch VLAN database is consistent with the > bridge commands, I'm sending a complete RFC very soon. > > Scott, David, > > I use this mail to expose a potential problem between iproute2 and the > kernel, found with my previous code. When issuing "ip link set dev swp0 > master br0", ndo_vlan_rx_add_vid is called, but not ndo_bridge_setlink, Remove NETIF_F_HW_VLAN_CTAG_FILTER and ndo_vlan_rx_add_vid will not be called. Issuing "ip link set dev swp0 master br0" should only be setting the bridge member, not setting up any VLAN. I suspect when you did this swp0 was admin UP and you're getting untagged VLAN 0 installed, which is the call to ndo_vlan_rx_add_vid. > which results in an inconsistency between my switch VLAN database (and > port settings) and "bridge vlan", which shows "swp0 1 PVID Egress > Untagged". So that is a result of /sys/class/net/br0/bridge/default_pvid set to 1. If you don't want that, turn default_pvid off: echo 0 >/sys/class/net/br0/bridge/default_pvid Now you'll see "None" in the "bridge vlan" output. > Seems like there is a call to ndo_bridge_setlink to add somewhere, but I > have no clue where. > > In the meantime, I call "bridge vlan add vid 1 dev swp0 pvid untagged > [master self]" at boot, to be consistent with the bridge output. Or turn off default_pvid. -- 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
