1. 21 Sep, 2020 7 commits
    • Vladimir Oltean's avatar
      net: dsa: install VLANs into the master's RX filter too · 2209158c
      Vladimir Oltean authored
      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, tag_8021q etc),
      and if the DSA master has "rx-vlan-filter: on" in ethtool -k, then we
      have a problem.
      
      Therefore, we could populate the VLAN table of the master, just in case
      (for some switches it will not make a difference), so that network I/O
      can work even with a VLAN filtering master.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      2209158c
    • Vladimir Oltean's avatar
      net: dsa: allow 8021q uppers while the bridge has vlan_filtering=0 · adb256eb
      Vladimir Oltean authored
      When the bridge has VLAN awareness disabled there isn't any duplication
      of functionality, since the bridge does not process VLAN. Don't deny
      adding 8021q uppers to DSA switch ports in that case. The switch is
      supposed to simply pass traffic leaving the VLAN tag as-is, and the
      stack will happily strip the VLAN tag for all 8021q uppers that exist.
      
      We need to ensure that there are no 8021q uppers when the user attempts
      to enable bridge vlan_filtering.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      adb256eb
    • Vladimir Oltean's avatar
      net: dsa: refuse configuration in prepare phase of dsa_port_vlan_filtering() · 707ec383
      Vladimir Oltean authored
      The current logic beats me a little bit. The comment that "bridge skips
      -EOPNOTSUPP, so skip the prepare phase" was introduced in commit
      fb2dabad ("net: dsa: support VLAN filtering switchdev attr").
      
      I'm not sure:
      (a) ok, the bridge skips -EOPNOTSUPP, but, so what, where are we
          returning -EOPNOTSUPP?
      (b) even if we are, and I'm just not seeing it, what is the causality
          relationship between the bridge skipping -EOPNOTSUPP and DSA
          skipping the prepare phase, and just returning zero?
      
      One thing is certain beyond doubt though, and that is that DSA currently
      refuses VLAN filtering from the "commit" phase instead of "prepare", and
      that this is not a good thing:
      
      ip link add br0 type bridge
      ip link add br1 type bridge vlan_filtering 1
      ip link set swp2 master br0
      ip link set swp3 master br1
      [ 3790.379389] 001: sja1105 spi0.1: VLAN filtering is a global setting
      [ 3790.379399] 001: ------------[ cut here ]------------
      [ 3790.379403] 001: WARNING: CPU: 1 PID: 515 at net/switchdev/switchdev.c:157 switchdev_port_attr_set_now+0x9c/0xa4
      [ 3790.379420] 001: swp3: Commit of attribute (id=6) failed.
      [ 3790.379533] 001: [<c11ff588>] (switchdev_port_attr_set_now) from [<c11b62e4>] (nbp_vlan_init+0x84/0x148)
      [ 3790.379544] 001: [<c11b62e4>] (nbp_vlan_init) from [<c11a2ff0>] (br_add_if+0x514/0x670)
      [ 3790.379554] 001: [<c11a2ff0>] (br_add_if) from [<c1031b5c>] (do_setlink+0x38c/0xab0)
      [ 3790.379565] 001: [<c1031b5c>] (do_setlink) from [<c1036fe8>] (__rtnl_newlink+0x44c/0x748)
      [ 3790.379573] 001: [<c1036fe8>] (__rtnl_newlink) from [<c1037328>] (rtnl_newlink+0x44/0x60)
      [ 3790.379580] 001: [<c1037328>] (rtnl_newlink) from [<c10315fc>] (rtnetlink_rcv_msg+0x124/0x2f8)
      [ 3790.379590] 001: [<c10315fc>] (rtnetlink_rcv_msg) from [<c10926b8>] (netlink_rcv_skb+0xb8/0x110)
      [ 3790.379806] 001: ---[ end trace 0000000000000002 ]---
      [ 3790.379819] 001: sja1105 spi0.1 swp3: failed to initialize vlan filtering on this port
      
      So move the current logic that may fail (except ds->ops->port_vlan_filtering,
      that is way harder) into the prepare stage of the switchdev transaction.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      707ec383
    • Vladimir Oltean's avatar
      net: dsa: convert denying bridge VLAN with existing 8021q upper to PRECHANGEUPPER · 1ce39f0e
      Vladimir Oltean authored
      This is checking for the following order of operations, and makes sure
      to deny that configuration:
      
      ip link add link swp2 name swp2.100 type vlan id 100
      ip link add br0 type bridge vlan_filtering 1
      ip link set swp2 master br0
      bridge vlan add dev swp2 vid 100
      
      Instead of using vlan_for_each(), which looks at the VLAN filters
      installed with vlan_vid_add(), just track the 8021q uppers. This has the
      advantage of freeing up the vlan_vid_add() call for actual VLAN
      filtering.
      
      There is another change in this patch. The check is moved in slave.c,
      from switch.c. I don't think it makes sense to have this 8021q upper
      check for each switch port that gets notified of that VLAN addition
      (these include DSA links and CPU ports, we know those can't have 8021q
      uppers because they don't have a net_device registered for them), so
      just do it in slave.c, for that one slave interface.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      1ce39f0e
    • Vladimir Oltean's avatar
      net: dsa: convert check for 802.1Q upper when bridged into PRECHANGEUPPER · 2b138406
      Vladimir Oltean authored
      DSA tries to prevent having a VLAN added by a bridge and by an 802.1Q
      upper at the same time. It does that by checking the VID in
      .ndo_vlan_rx_add_vid(), since that's something that the 8021q module
      calls, via vlan_vid_add(). When a VLAN matches in both subsystems, this
      check returns -EBUSY.
      
      However the vlan_vid_add() function isn't specific to the 8021q module
      in any way at all. It is simply the kernel's way to tell an interface to
      add a VLAN to its RX filter and not drop that VLAN. So there's no reason
      to return -EBUSY when somebody tries to call vlan_vid_add() for a VLAN
      that was installed by the bridge. The proper behavior is to accept that
      configuration.
      
      So what's wrong is how DSA checks that it has an 8021q upper. It should
      look at the actual uppers for that, not just assume that the 8021q
      module was somewhere in the call stack of .ndo_vlan_rx_add_vid().
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      2b138406
    • Vladimir Oltean's avatar
      net: dsa: rename dsa_slave_upper_vlan_check to something more suggestive · eb46e8da
      Vladimir Oltean authored
      We'll be adding a new check in the PRECHANGEUPPER notifier, where we'll
      need to check some VLAN uppers. It is hard to do that when there is
      already a function named dsa_slave_upper_vlan_check. So rename this one.
      
      Not to mention that this function probably shouldn't have started with
      "dsa_slave_" in the first place, since the struct net_device argument
      isn't a DSA slave, but an 8021q upper of one.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      eb46e8da
    • Vladimir Oltean's avatar
      net: dsa: deny enslaving 802.1Q upper to VLAN-aware bridge from PRECHANGEUPPER · 83501299
      Vladimir Oltean authored
      There doesn't seem to be any strong technical reason for doing it this
      way, but we'll be adding more checks for invalid upper device
      configurations, and it will be easier to have them all grouped under
      PRECHANGEUPPER.
      
      Tested that it still works:
      ip link set br0 type bridge vlan_filtering 1
      ip link add link swp2 name swp2.100 type vlan id 100
      ip link set swp2.100 master br0
      [   20.321312] br0: port 5(swp2.100) entered blocking state
      [   20.326711] br0: port 5(swp2.100) entered disabled state
      Error: dsa_core: Cannot enslave VLAN device into VLAN aware bridge.
      [   20.346549] br0: port 5(swp2.100) entered blocking state
      [   20.351957] br0: port 5(swp2.100) entered disabled state
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      83501299
  2. 20 Sep, 2020 4 commits
  3. 19 Sep, 2020 29 commits