1. 23 Jul, 2021 7 commits
    • David S. Miller's avatar
      Merge branch 'net-remove-compat-alloc-user-space' · 090597b4
      David S. Miller authored
      Arnd Bergmann says:
      
      ====================
      remove compat_alloc_user_space()
      
      This is the fifth version of my series, now spanning four patches
      instead of two, with a new approach for handling struct ifreq
      compatibility after I realized that my earlier approach introduces
      additional problems.
      
      The idea here is to always push down the compat conversion
      deeper into the call stack: rather than pretending to be
      native mode with a modified copy of the original data on
      the user space stack, have the code that actually works on
      the data understand the difference between native and compat
      versions.
      
      I have spent a long time looking at all drivers that implement
      an ndo_do_ioctl callback to verify that my assumptions are
      correct. This has led to a series of ~30 additional patches
      that I am not including here but will post separately, fixing
      a number of bugs in SIOCDEVPRIVATE ioctls, removing dead
      code, and splitting ndo_do_ioctl into multiple new ndo callbacks
      for private and ethernet specific commands.
      
            Arnd
      
      Link: https://lore.kernel.org/netdev/20201124151828.169152-1-arnd@kernel.org/
      
      Changes in v6:
       - Split out and expand linux/compat.h rework
       - Split ifconf change into two patches
       - Rebase on latest net-next/master
      
      Changes in v5:
       - Rebase to v5.14-rc2
       - Fix a few build issues
      
      Changes in v4:
       - build fix without CONFIG_INET
       - build fix without CONFIG_COMPAT
       - style fixes pointed out by hch
      
      Changes in v3:
       - complete rewrite of the series
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      090597b4
    • Arnd Bergmann's avatar
      net: socket: rework compat_ifreq_ioctl() · 29c49648
      Arnd Bergmann authored
      compat_ifreq_ioctl() is one of the last users of copy_in_user() and
      compat_alloc_user_space(), as it attempts to convert the 'struct ifreq'
      arguments from 32-bit to 64-bit format as used by dev_ioctl() and a
      couple of socket family specific interpretations.
      
      The current implementation works correctly when calling dev_ioctl(),
      inet_ioctl(), ieee802154_sock_ioctl(), atalk_ioctl(), qrtr_ioctl()
      and packet_ioctl(). The ioctl handlers for x25, netrom, rose and x25 do
      not interpret the arguments and only block the corresponding commands,
      so they do not care.
      
      For af_inet6 and af_decnet however, the compat conversion is slightly
      incorrect, as it will copy more data than the native handler accesses,
      both of them use a structure that is shorter than ifreq.
      
      Replace the copy_in_user() conversion with a pair of accessor functions
      to read and write the ifreq data in place with the correct length where
      needed, while leaving the other ones to copy the (already compatible)
      structures directly.
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      29c49648
    • Arnd Bergmann's avatar
      net: socket: simplify dev_ifconf handling · 876f0bf9
      Arnd Bergmann authored
      The dev_ifconf() calling conventions make compat handling
      more complicated than necessary, simplify this by moving
      the in_compat_syscall() check into the function.
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      876f0bf9
    • Arnd Bergmann's avatar
      net: socket: remove register_gifconf · b0e99d03
      Arnd Bergmann authored
      Since dynamic registration of the gifconf() helper is only used for
      IPv4, and this can not be in a loadable module, this can be simplified
      noticeably by turning it into a direct function call as a preparation
      for cleaning up the compat handling.
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      b0e99d03
    • Arnd Bergmann's avatar
      net: socket: rework SIOC?IFMAP ioctls · 709566d7
      Arnd Bergmann authored
      SIOCGIFMAP and SIOCSIFMAP currently require compat_alloc_user_space()
      and copy_in_user() for compat mode.
      
      Move the compat handling into the location where the structures are
      actually used, to avoid using those interfaces and get a clearer
      implementation.
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      709566d7
    • Arnd Bergmann's avatar
      ethtool: improve compat ioctl handling · dd98d289
      Arnd Bergmann authored
      The ethtool compat ioctl handling is hidden away in net/socket.c,
      which introduces a couple of minor oddities:
      
      - The implementation may end up diverging, as seen in the RXNFC
        extension in commit 84a1d9c4 ("net: ethtool: extend RXNFC
        API to support RSS spreading of filter matches") that does not work
        in compat mode.
      
      - Most architectures do not need the compat handling at all
        because u64 and compat_u64 have the same alignment.
      
      - On x86, the conversion is done for both x32 and i386 user space,
        but it's actually wrong to do it for x32 and cannot work there.
      
      - On 32-bit Arm, it never worked for compat oabi user space, since
        that needs to do the same conversion but does not.
      
      - It would be nice to get rid of both compat_alloc_user_space()
        and copy_in_user() throughout the kernel.
      
      None of these actually seems to be a serious problem that real
      users are likely to encounter, but fixing all of them actually
      leads to code that is both shorter and more readable.
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      dd98d289
    • Arnd Bergmann's avatar
      compat: make linux/compat.h available everywhere · 1a33b18b
      Arnd Bergmann authored
      Parts of linux/compat.h are under an #ifdef, but we end up
      using more of those over time, moving things around bit by
      bit.
      
      To get it over with once and for all, make all of this file
      uncondititonal now so it can be accessed everywhere. There
      are only a few types left that are in asm/compat.h but not
      yet in the asm-generic version, so add those in the process.
      
      This requires providing a few more types in asm-generic/compat.h
      that were not already there. The only tricky one is
      compat_sigset_t, which needs a little help on 32-bit architectures
      and for x86.
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      1a33b18b
  2. 22 Jul, 2021 24 commits
    • Niklas Söderlund's avatar
      nfp: fix return statement in nfp_net_parse_meta() · 4431531c
      Niklas Söderlund authored
      The return type of the function is bool and while NULL do evaluate to
      false it's not very nice, fix this by explicitly returning false. There
      is no functional change.
      Signed-off-by: default avatarNiklas Söderlund <niklas.soderlund@corigine.com>
      Signed-off-by: default avatarLouis Peens <louis.peens@corigine.com>
      Signed-off-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      4431531c
    • Matthieu Baerts's avatar
      ipv6: fix "'ioam6_if_id_max' defined but not used" warn · 176f716c
      Matthieu Baerts authored
      When compiling without CONFIG_SYSCTL, this warning appears:
      
        net/ipv6/addrconf.c:99:12: error: 'ioam6_if_id_max' defined but not used [-Werror=unused-variable]
           99 | static u32 ioam6_if_id_max = U16_MAX;
              |            ^~~~~~~~~~~~~~~
        cc1: all warnings being treated as errors
      
      Simply moving the declaration of this variable under ...
      
        #ifdef CONFIG_SYSCTL
      
      ... with other similar variables fixes the issue.
      
      Fixes: 9ee11f0f ("ipv6: ioam: Data plane support for Pre-allocated Trace")
      Signed-off-by: default avatarMatthieu Baerts <matthieu.baerts@tessares.net>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      176f716c
    • David S. Miller's avatar
      Merge branch 'nfp-flower-ct-offload' · 552a2a3f
      David S. Miller authored
      Simon Horman says:
      
      ====================
      nfp: flower: conntrack offload
      
      Louis Peens says:
      
      This series takes the preparation from previous two series
      and finally creates the structures and control messages
      to offload the conntrack flows to the card. First we
      do a bit of refactoring in the existing functions
      to make them re-usable for the conntrack implementation,
      after which the control messages are compiled and
      transmitted to the card. Lastly we add stats handling
      for the conntrack flows.
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      552a2a3f
    • Louis Peens's avatar
      nfp: flower-tc: add flow stats updates for ct · 40c10bd9
      Louis Peens authored
      Add in the logic to update flow stats. The flow stats from the nfp
      is saved in the flow_pay struct, which is associated with the final
      merged flow. This saves deltas however, so once read it needs to
      be cleared. However the flow stats requests from the kernel is
      from the other side of the chain, and a single tc flow from
      the kernel can be merged into multiple other tc flows to form
      multiple offloaded flows. This means that all linked flows
      needs to be updated for each stats request.
      Signed-off-by: default avatarLouis Peens <louis.peens@corigine.com>
      Signed-off-by: default avatarYinjun Zhang <yinjun.zhang@corigine.com>
      Signed-off-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      40c10bd9
    • Louis Peens's avatar
      nfp: flower-ct: add offload calls to the nfp · 400a5e5f
      Louis Peens authored
      Add the offload parts (ADD_FLOW/DEL_FLOW) calls to add and delete
      the flows from the nfp.
      Signed-off-by: default avatarLouis Peens <louis.peens@corigine.com>
      Signed-off-by: default avatarYinjun Zhang <yinjun.zhang@corigine.com>
      Signed-off-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      400a5e5f
    • Louis Peens's avatar
      nfp: flower-ct: add flow_pay to the offload table · 453cdc30
      Louis Peens authored
      Compile the offload flow metadata and add flow_pay to the offload
      table. Also add in the delete paths. This does not include actual
      offloading to the card yet, this will follow soon.
      Signed-off-by: default avatarLouis Peens <louis.peens@corigine.com>
      Signed-off-by: default avatarYinjun Zhang <yinjun.zhang@corigine.com>
      Signed-off-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      453cdc30
    • Louis Peens's avatar
      nfp: flower-ct: add actions into flow_pay for offload · d94a63b4
      Louis Peens authored
      Combine the actions from the three different rules into one and
      convert into the payload format expected by the nfp.
      Signed-off-by: default avatarLouis Peens <louis.peens@corigine.com>
      Signed-off-by: default avatarYinjun Zhang <yinjun.zhang@corigine.com>
      Signed-off-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      d94a63b4
    • Louis Peens's avatar
      nfp: flower-ct: compile match sections of flow_payload · 5a2b9304
      Louis Peens authored
      Add in the code to compile match part of the payload that will be
      sent to the firmware. This works similar to match.c does it, but
      since three flows needs to be merged it iterates through all three
      rules in a loop and combine the match fields to get the most strict
      match as result.
      Signed-off-by: default avatarLouis Peens <louis.peens@corigine.com>
      Signed-off-by: default avatarYinjun Zhang <yinjun.zhang@corigine.com>
      Signed-off-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      5a2b9304
    • Louis Peens's avatar
      nfp: flower-ct: calculate required key_layers · 71e88cfb
      Louis Peens authored
      This calculates the correct combined keylayers and key_layer_size
      for the to-be-offloaded flow.
      Signed-off-by: default avatarLouis Peens <louis.peens@corigine.com>
      Signed-off-by: default avatarYinjun Zhang <yinjun.zhang@corigine.com>
      Signed-off-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      71e88cfb
    • Louis Peens's avatar
      nfp: flower: refactor action offload code slightly · e75dc265
      Louis Peens authored
      Change the action related offload functions to take in flow_rule *
      as input instead of flow_cls_offload * as input. The flow_rule
      parts of flow_cls_offload is the only part that is used in any
      case, and this is required for more conntrack offload patches
      which will follow later.
      Signed-off-by: default avatarLouis Peens <louis.peens@corigine.com>
      Signed-off-by: default avatarYinjun Zhang <yinjun.zhang@corigine.com>
      Signed-off-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e75dc265
    • Louis Peens's avatar
      nfp: flower: refactor match functions to take flow_rule as input · 4b15fb18
      Louis Peens authored
      This is a small cleanup to pass in flow->rule to some of the compile
      functions instead of extracting it every time. This is will also be
      useful for conntrack patches later.
      Signed-off-by: default avatarLouis Peens <louis.peens@corigine.com>
      Signed-off-by: default avatarYinjun Zhang <yinjun.zhang@corigine.com>
      Signed-off-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      4b15fb18
    • Yinjun Zhang's avatar
      nfp: flower: make the match compilation functions reusable · 16416d37
      Yinjun Zhang authored
      Expose and refactor the match compilation functions so that they
      can be invoked externally. Also update the functions so they can
      be called multiple times with the results OR'd together. This is
      applicable for the flows-merging scenario, in which there could be
      overlapped and non-conflicting match fields. This will be used
      in upcoming conntrack patches. This is safe to do in the in the
      single call case as well since both unmasked_data and mask_data
      gets initialised to 0.
      Signed-off-by: default avatarYinjun Zhang <yinjun.zhang@corigine.com>
      Signed-off-by: default avatarLouis Peens <louis.peens@corigine.com>
      Signed-off-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      16416d37
    • Oleksij Rempel's avatar
      net: selftests: add MTU test · 802a76af
      Oleksij Rempel authored
      Test if we actually can send/receive packets with MTU size. This kind of
      issue was detected on ASIX HW with bogus EEPROM.
      Signed-off-by: default avatarOleksij Rempel <o.rempel@pengutronix.de>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      802a76af
    • Oleksij Rempel's avatar
      net: usb: asix: ax88772: add missing stop · 9c267095
      Oleksij Rempel authored
      Add missing stop and let phylib framework suspend attached PHY.
      
      Fixes: e532a096 ("net: usb: asix: ax88772: add phylib support")
      Signed-off-by: default avatarOleksij Rempel <o.rempel@pengutronix.de>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      9c267095
    • Oleksij Rempel's avatar
      net: usb: asix: ax88772: do not poll for PHY before registering it · fdc362bf
      Oleksij Rempel authored
      asix_get_phyid() is used for two reasons here. To print debug message
      with the PHY ID and to wait until the PHY is powered up.
      
      After migrating to the phylib, we can read PHYID from sysfs. If polling
      for the PHY is really needed, then we will need to handle it in the
      phylib as well.
      
      This change was tested with:
      - ax88772a + internal PHY
      - ax88772b + external PHY
      Signed-off-by: default avatarOleksij Rempel <o.rempel@pengutronix.de>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      fdc362bf
    • Vladimir Oltean's avatar
      net: switchdev: fix FDB entries towards foreign ports not getting propagated to us · 2b0a5688
      Vladimir Oltean authored
      The newly introduced switchdev_handle_fdb_{add,del}_to_device helpers
      solved a problem but introduced another one. They have a severe design
      bug: they do not propagate FDB events on foreign interfaces to us, i.e.
      this use case:
      
               br0
              /   \
             /     \
            /       \
           /         \
         swp0       eno0
      (switchdev)  (foreign)
      
      when an address is learned on eno0, what is supposed to happen is that
      this event should also be propagated towards swp0. Somehow I managed to
      convince myself that this did work correctly, but obviously it does not.
      
      The trouble with foreign interfaces is that we must reach a switchdev
      net_device pointer through a foreign net_device that has no direct
      upper/lower relationship with it. So we need to do exploratory searching
      through the lower interfaces of the foreign net_device's bridge upper
      (to reach swp0 from eno0, we must check its upper, br0, for lower
      interfaces that pass the check_cb and foreign_dev_check_cb). This is
      something that the previous code did not do, it just assumed that "dev"
      will become a switchdev interface at some point, somehow, probably by
      magic.
      
      With this patch, assisted address learning on the CPU port works again
      in DSA:
      
      ip link add br0 type bridge
      ip link set swp0 master br0
      ip link set eno0 master br0
      ip link set br0 up
      
      [   46.708929] mscc_felix 0000:00:00.5 swp0: Adding FDB entry towards eno0, addr 00:04:9f:05:f4:ab vid 0 as host address
      
      Fixes: 8ca07176 ("net: switchdev: introduce a fanout helper for SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE")
      Reported-by: default avatarEric Woudstra <ericwouds@gmail.com>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      2b0a5688
    • David S. Miller's avatar
      Merge branch 'bridge-port-offload' · f796fcd6
      David S. Miller authored
      Vladimir Oltean says:
      
      ====================
      Let switchdev drivers offload and unoffload bridge ports at their own convenience
      
      This series introduces an explicit API through which switchdev drivers
      mark a bridge port as offloaded or not:
      - switchdev_bridge_port_offload()
      - switchdev_bridge_port_unoffload()
      
      Currently, the bridge assumes that a port is offloaded if
      dev_get_port_parent_id(dev, &ppid, recurse=true) returns something, but
      that is just an assumption that breaks some use cases (like a
      non-offloaded LAG interface on top of a switchdev port, bridged with
      other switchdev ports).
      
      Along with some consolidation of the bridge logic to assign a "switchdev
      offloading mark" to a port (now better called a "hardware domain"), this
      series allows the bridge driver side to no longer impose restrictions on
      that configuration.
      
      Right now, all switchdev drivers must be modified to use the explicit
      API, but more and more logic can then be placed centrally in the bridge
      and therefore ease the job of a switchdev driver writer in the future.
      
      For example, the first thing we can hook into the explicit switchdev
      offloading API calls are the switchdev object and FDB replay helpers.
      So far, these have only been used by DSA in "pull" mode (where the
      driver must ask for them). Adding the replay helpers to other drivers
      involves a lot of repetition. But by moving the helpers inside the
      bridge port offload/unoffload hook points, we can move the entire replay
      process to "push" mode (where the bridge provides them automatically).
      
      The explicit switchdev offloading API will see further extensions in the
      future.
      
      The patches were split from a larger series for easier review:
      https://patchwork.kernel.org/project/netdevbpf/cover/20210718214434.3938850-1-vladimir.oltean@nxp.com/
      
      Changes in v6:
      - Make the switchdev replay helpers opt-in
      - Opt out of the replay helpers for mlxsw, rocker, prestera, sparx5,
        cpsw, am65-cpsw
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f796fcd6
    • Vladimir Oltean's avatar
      net: bridge: move the switchdev object replay helpers to "push" mode · 4e51bf44
      Vladimir Oltean authored
      Starting with commit 4f2673b3 ("net: bridge: add helper to replay
      port and host-joined mdb entries"), DSA has introduced some bridge
      helpers that replay switchdev events (FDB/MDB/VLAN additions and
      deletions) that can be lost by the switchdev drivers in a variety of
      circumstances:
      
      - an IP multicast group was host-joined on the bridge itself before any
        switchdev port joined the bridge, leading to the host MDB entries
        missing in the hardware database.
      - during the bridge creation process, the MAC address of the bridge was
        added to the FDB as an entry pointing towards the bridge device
        itself, but with no switchdev ports being part of the bridge yet, this
        local FDB entry would remain unknown to the switchdev hardware
        database.
      - a VLAN/FDB/MDB was added to a bridge port that is a LAG interface,
        before any switchdev port joined that LAG, leading to the hardware
        database missing those entries.
      - a switchdev port left a LAG that is a bridge port, while the LAG
        remained part of the bridge, and all FDB/MDB/VLAN entries remained
        installed in the hardware database of the switchdev port.
      
      Also, since commit 0d2cfbd4 ("net: bridge: ignore switchdev events
      for LAG ports which didn't request replay"), DSA introduced a method,
      based on a const void *ctx, to ensure that two switchdev ports under the
      same LAG that is a bridge port do not see the same MDB/VLAN entry being
      replayed twice by the bridge, once for every bridge port that joins the
      LAG.
      
      With so many ordering corner cases being possible, it seems unreasonable
      to expect a switchdev driver writer to get it right from the first try.
      Therefore, now that DSA has experimented with the bridge replay helpers
      for a little bit, we can move the code to the bridge driver where it is
      more readily available to all switchdev drivers.
      
      To convert the switchdev object replay helpers from "pull mode" (where
      the driver asks for them) to a "push mode" (where the bridge offers them
      automatically), the biggest problem is that the bridge needs to be aware
      when a switchdev port joins and leaves, even when the switchdev is only
      indirectly a bridge port (for example when the bridge port is a LAG
      upper of the switchdev).
      
      Luckily, we already have a hook for that, in the form of the newly
      introduced switchdev_bridge_port_offload() and
      switchdev_bridge_port_unoffload() calls. These offer a natural place for
      hooking the object addition and deletion replays.
      
      Extend the above 2 functions with:
      - pointers to the switchdev atomic notifier (for FDB replays) and the
        blocking notifier (for MDB and VLAN replays).
      - the "const void *ctx" argument required for drivers to be able to
        disambiguate between which port is targeted, when multiple ports are
        lowers of the same LAG that is a bridge port. Most of the drivers pass
        NULL to this argument, except the ones that support LAG offload and have
        the proper context check already in place in the switchdev blocking
        notifier handler.
      
      Also unexport the replay helpers, since nobody except the bridge calls
      them directly now.
      
      Note that:
      (a) we abuse the terminology slightly, because FDB entries are not
          "switchdev objects", but we count them as objects nonetheless.
          With no direct way to prove it, I think they are not modeled as
          switchdev objects because those can only be installed by the bridge
          to the hardware (as opposed to FDB entries which can be propagated
          in the other direction too). This is merely an abuse of terms, FDB
          entries are replayed too, despite not being objects.
      (b) the bridge does not attempt to sync port attributes to newly joined
          ports, just the countable stuff (the objects). The reason for this
          is simple: no universal and symmetric way to sync and unsync them is
          known. For example, VLAN filtering: what to do on unsync, disable or
          leave it enabled? Similarly, STP state, ageing timer, etc etc. What
          a switchdev port does when it becomes standalone again is not really
          up to the bridge's competence, and the driver should deal with it.
          On the other hand, replaying deletions of switchdev objects can be
          seen a matter of cleanup and therefore be treated by the bridge,
          hence this patch.
      
      We make the replay helpers opt-in for drivers, because they might not
      bring immediate benefits for them:
      
      - nbp_vlan_init() is called _after_ netdev_master_upper_dev_link(),
        so br_vlan_replay() should not do anything for the new drivers on
        which we call it. The existing drivers where there was even a slight
        possibility for there to exist a VLAN on a bridge port before they
        join it are already guarded against this: mlxsw and prestera deny
        joining LAG interfaces that are members of a bridge.
      
      - br_fdb_replay() should now notify of local FDB entries, but I patched
        all drivers except DSA to ignore these new entries in commit
        2c4eca3e ("net: bridge: switchdev: include local flag in FDB
        notifications"). Driver authors can lift this restriction as they
        wish, and when they do, they can also opt into the FDB replay
        functionality.
      
      - br_mdb_replay() should fix a real issue which is described in commit
        4f2673b3 ("net: bridge: add helper to replay port and host-joined
        mdb entries"). However most drivers do not offload the
        SWITCHDEV_OBJ_ID_HOST_MDB to see this issue: only cpsw and am65_cpsw
        offload this switchdev object, and I don't completely understand the
        way in which they offload this switchdev object anyway. So I'll leave
        it up to these drivers' respective maintainers to opt into
        br_mdb_replay().
      
      So most of the drivers pass NULL notifier blocks for the replay helpers,
      except:
      - dpaa2-switch which was already acked/regression-tested with the
        helpers enabled (and there isn't much of a downside in having them)
      - ocelot which already had replay logic in "pull" mode
      - DSA which already had replay logic in "pull" mode
      
      An important observation is that the drivers which don't currently
      request bridge event replays don't even have the
      switchdev_bridge_port_{offload,unoffload} calls placed in proper places
      right now. This was done to avoid unnecessary rework for drivers which
      might never even add support for this. For driver writers who wish to
      add replay support, this can be used as a tentative placement guide:
      https://patchwork.kernel.org/project/netdevbpf/patch/20210720134655.892334-11-vladimir.oltean@nxp.com/
      
      Cc: Vadym Kochan <vkochan@marvell.com>
      Cc: Taras Chornyi <tchornyi@marvell.com>
      Cc: Ioana Ciornei <ioana.ciornei@nxp.com>
      Cc: Lars Povlsen <lars.povlsen@microchip.com>
      Cc: Steen Hegelund <Steen.Hegelund@microchip.com>
      Cc: UNGLinuxDriver@microchip.com
      Cc: Claudiu Manoil <claudiu.manoil@nxp.com>
      Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
      Cc: Grygorii Strashko <grygorii.strashko@ti.com>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Acked-by: Ioana Ciornei <ioana.ciornei@nxp.com> # dpaa2-switch
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      4e51bf44
    • Vladimir Oltean's avatar
      net: bridge: guard the switchdev replay helpers against a NULL notifier block · 7105b50b
      Vladimir Oltean authored
      There is a desire to make the object and FDB replay helpers optional
      when moving them inside the bridge driver. For example a certain driver
      might not offload host MDBs and there is no case where the replay
      helpers would be of immediate use to it.
      
      So it would be nice if we could allow drivers to pass NULL pointers for
      the atomic and blocking notifier blocks, and the replay helpers to do
      nothing in that case.
      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>
      7105b50b
    • Vladimir Oltean's avatar
      net: bridge: switchdev: let drivers inform which bridge ports are offloaded · 2f5dc00f
      Vladimir Oltean authored
      On reception of an skb, the bridge checks if it was marked as 'already
      forwarded in hardware' (checks if skb->offload_fwd_mark == 1), and if it
      is, it assigns the source hardware domain of that skb based on the
      hardware domain of the ingress port. Then during forwarding, it enforces
      that the egress port must have a different hardware domain than the
      ingress one (this is done in nbp_switchdev_allowed_egress).
      
      Non-switchdev drivers don't report any physical switch id (neither
      through devlink nor .ndo_get_port_parent_id), therefore the bridge
      assigns them a hardware domain of 0, and packets coming from them will
      always have skb->offload_fwd_mark = 0. So there aren't any restrictions.
      
      Problems appear due to the fact that DSA would like to perform software
      fallback for bonding and team interfaces that the physical switch cannot
      offload.
      
             +-- br0 ---+
            / /   |      \
           / /    |       \
          /  |    |      bond0
         /   |    |     /    \
       swp0 swp1 swp2 swp3 swp4
      
      There, it is desirable that the presence of swp3 and swp4 under a
      non-offloaded LAG does not preclude us from doing hardware bridging
      beteen swp0, swp1 and swp2. The bandwidth of the CPU is often times high
      enough that software bridging between {swp0,swp1,swp2} and bond0 is not
      impractical.
      
      But this creates an impossible paradox given the current way in which
      port hardware domains are assigned. When the driver receives a packet
      from swp0 (say, due to flooding), it must set skb->offload_fwd_mark to
      something.
      
      - If we set it to 0, then the bridge will forward it towards swp1, swp2
        and bond0. But the switch has already forwarded it towards swp1 and
        swp2 (not to bond0, remember, that isn't offloaded, so as far as the
        switch is concerned, ports swp3 and swp4 are not looking up the FDB,
        and the entire bond0 is a destination that is strictly behind the
        CPU). But we don't want duplicated traffic towards swp1 and swp2, so
        it's not ok to set skb->offload_fwd_mark = 0.
      
      - If we set it to 1, then the bridge will not forward the skb towards
        the ports with the same switchdev mark, i.e. not to swp1, swp2 and
        bond0. Towards swp1 and swp2 that's ok, but towards bond0? It should
        have forwarded the skb there.
      
      So the real issue is that bond0 will be assigned the same hardware
      domain as {swp0,swp1,swp2}, because the function that assigns hardware
      domains to bridge ports, nbp_switchdev_add(), recurses through bond0's
      lower interfaces until it finds something that implements devlink (calls
      dev_get_port_parent_id with bool recurse = true). This is a problem
      because the fact that bond0 can be offloaded by swp3 and swp4 in our
      example is merely an assumption.
      
      A solution is to give the bridge explicit hints as to what hardware
      domain it should use for each port.
      
      Currently, the bridging offload is very 'silent': a driver registers a
      netdevice notifier, which is put on the netns's notifier chain, and
      which sniffs around for NETDEV_CHANGEUPPER events where the upper is a
      bridge, and the lower is an interface it knows about (one registered by
      this driver, normally). Then, from within that notifier, it does a bunch
      of stuff behind the bridge's back, without the bridge necessarily
      knowing that there's somebody offloading that port. It looks like this:
      
           ip link set swp0 master br0
                        |
                        v
       br_add_if() calls netdev_master_upper_dev_link()
                        |
                        v
              call_netdevice_notifiers
                        |
                        v
             dsa_slave_netdevice_event
                        |
                        v
              oh, hey! it's for me!
                        |
                        v
                 .port_bridge_join
      
      What we do to solve the conundrum is to be less silent, and change the
      switchdev drivers to present themselves to the bridge. Something like this:
      
           ip link set swp0 master br0
                        |
                        v
       br_add_if() calls netdev_master_upper_dev_link()
                        |
                        v                    bridge: Aye! I'll use this
              call_netdevice_notifiers           ^  ppid as the
                        |                        |  hardware domain for
                        v                        |  this port, and zero
             dsa_slave_netdevice_event           |  if I got nothing.
                        |                        |
                        v                        |
              oh, hey! it's for me!              |
                        |                        |
                        v                        |
                 .port_bridge_join               |
                        |                        |
                        +------------------------+
                   switchdev_bridge_port_offload(swp0, swp0)
      
      Then stacked interfaces (like bond0 on top of swp3/swp4) would be
      treated differently in DSA, depending on whether we can or cannot
      offload them.
      
      The offload case:
      
          ip link set bond0 master br0
                        |
                        v
       br_add_if() calls netdev_master_upper_dev_link()
                        |
                        v                    bridge: Aye! I'll use this
              call_netdevice_notifiers           ^  ppid as the
                        |                        |  switchdev mark for
                        v                        |        bond0.
             dsa_slave_netdevice_event           | Coincidentally (or not),
                        |                        | bond0 and swp0, swp1, swp2
                        v                        | all have the same switchdev
              hmm, it's not quite for me,        | mark now, since the ASIC
               but my driver has already         | is able to forward towards
                 called .port_lag_join           | all these ports in hw.
                for it, because I have           |
            a port with dp->lag_dev == bond0.    |
                        |                        |
                        v                        |
                 .port_bridge_join               |
                 for swp3 and swp4               |
                        |                        |
                        +------------------------+
                  switchdev_bridge_port_offload(bond0, swp3)
                  switchdev_bridge_port_offload(bond0, swp4)
      
      And the non-offload case:
      
          ip link set bond0 master br0
                        |
                        v
       br_add_if() calls netdev_master_upper_dev_link()
                        |
                        v                    bridge waiting:
              call_netdevice_notifiers           ^  huh, switchdev_bridge_port_offload
                        |                        |  wasn't called, okay, I'll use a
                        v                        |  hwdom of zero for this one.
             dsa_slave_netdevice_event           :  Then packets received on swp0 will
                        |                        :  not be software-forwarded towards
                        v                        :  swp1, but they will towards bond0.
               it's not for me, but
             bond0 is an upper of swp3
            and swp4, but their dp->lag_dev
             is NULL because they couldn't
                  offload it.
      
      Basically we can draw the conclusion that the lowers of a bridge port
      can come and go, so depending on the configuration of lowers for a
      bridge port, it can dynamically toggle between offloaded and unoffloaded.
      Therefore, we need an equivalent switchdev_bridge_port_unoffload too.
      
      This patch changes the way any switchdev driver interacts with the
      bridge. From now on, everybody needs to call switchdev_bridge_port_offload
      and switchdev_bridge_port_unoffload, otherwise the bridge will treat the
      port as non-offloaded and allow software flooding to other ports from
      the same ASIC.
      
      Note that these functions lay the ground for a more complex handshake
      between switchdev drivers and the bridge in the future.
      
      For drivers that will request a replay of the switchdev objects when
      they offload and unoffload a bridge port (DSA, dpaa2-switch, ocelot), we
      place the call to switchdev_bridge_port_unoffload() strategically inside
      the NETDEV_PRECHANGEUPPER notifier's code path, and not inside
      NETDEV_CHANGEUPPER. This is because the switchdev object replay helpers
      need the netdev adjacency lists to be valid, and that is only true in
      NETDEV_PRECHANGEUPPER.
      
      Cc: Vadym Kochan <vkochan@marvell.com>
      Cc: Taras Chornyi <tchornyi@marvell.com>
      Cc: Ioana Ciornei <ioana.ciornei@nxp.com>
      Cc: Lars Povlsen <lars.povlsen@microchip.com>
      Cc: Steen Hegelund <Steen.Hegelund@microchip.com>
      Cc: UNGLinuxDriver@microchip.com
      Cc: Claudiu Manoil <claudiu.manoil@nxp.com>
      Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
      Cc: Grygorii Strashko <grygorii.strashko@ti.com>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Tested-by: Ioana Ciornei <ioana.ciornei@nxp.com> # dpaa2-switch: regression
      Acked-by: Ioana Ciornei <ioana.ciornei@nxp.com> # dpaa2-switch
      Tested-by: Horatiu Vultur <horatiu.vultur@microchip.com> # ocelot-switch
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      2f5dc00f
    • Tobias Waldekranz's avatar
      net: bridge: switchdev: recycle unused hwdoms · 85826610
      Tobias Waldekranz authored
      Since hwdoms have only been used thus far for equality comparisons, the
      bridge has used the simplest possible assignment policy; using a
      counter to keep track of the last value handed out.
      
      With the upcoming transmit offloading, we need to perform set
      operations efficiently based on hwdoms, e.g. we want to answer
      questions like "has this skb been forwarded to any port within this
      hwdom?"
      
      Move to a bitmap-based allocation scheme that recycles hwdoms once all
      members leaves the bridge. This means that we can use a single
      unsigned long to keep track of the hwdoms that have received an skb.
      
      v1->v2: convert the typedef DECLARE_BITMAP(br_hwdom_map_t, BR_HWDOM_MAX)
              into a plain unsigned long.
      v2->v6: none
      Signed-off-by: default avatarTobias Waldekranz <tobias@waldekranz.com>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      85826610
    • Tobias Waldekranz's avatar
      net: bridge: disambiguate offload_fwd_mark · f7cf972f
      Tobias Waldekranz authored
      Before this change, four related - but distinct - concepts where named
      offload_fwd_mark:
      
      - skb->offload_fwd_mark: Set by the switchdev driver if the underlying
        hardware has already forwarded this frame to the other ports in the
        same hardware domain.
      
      - nbp->offload_fwd_mark: An idetifier used to group ports that share
        the same hardware forwarding domain.
      
      - br->offload_fwd_mark: Counter used to make sure that unique IDs are
        used in cases where a bridge contains ports from multiple hardware
        domains.
      
      - skb->cb->offload_fwd_mark: The hardware domain on which the frame
        ingressed and was forwarded.
      
      Introduce the term "hardware forwarding domain" ("hwdom") in the
      bridge to denote a set of ports with the following property:
      
          If an skb with skb->offload_fwd_mark set, is received on a port
          belonging to hwdom N, that frame has already been forwarded to all
          other ports in hwdom N.
      
      By decoupling the name from "offload_fwd_mark", we can extend the
      term's definition in the future - e.g. to add constraints that
      describe expected egress behavior - without overloading the meaning of
      "offload_fwd_mark".
      
      - nbp->offload_fwd_mark thus becomes nbp->hwdom.
      
      - br->offload_fwd_mark becomes br->last_hwdom.
      
      - skb->cb->offload_fwd_mark becomes skb->cb->src_hwdom. The slight
        change in naming here mandates a slight change in behavior of the
        nbp_switchdev_frame_mark() function. Previously, it only set this
        value in skb->cb for packets with skb->offload_fwd_mark true (ones
        which were forwarded in hardware). Whereas now we always track the
        incoming hwdom for all packets coming from a switchdev (even for the
        packets which weren't forwarded in hardware, such as STP BPDUs, IGMP
        reports etc). As all uses of skb->cb->offload_fwd_mark were already
        gated behind checks of skb->offload_fwd_mark, this will not introduce
        any functional change, but it paves the way for future changes where
        the ingressing hwdom must be known for frames coming from a switchdev
        regardless of whether they were forwarded in hardware or not
        (basically, if the skb comes from a switchdev, skb->cb->src_hwdom now
        always tracks which one).
      
        A typical example where this is relevant: the switchdev has a fixed
        configuration to trap STP BPDUs, but STP is not running on the bridge
        and the group_fwd_mask allows them to be forwarded. Say we have this
        setup:
      
              br0
             / | \
            /  |  \
        swp0 swp1 swp2
      
        A BPDU comes in on swp0 and is trapped to the CPU; the driver does not
        set skb->offload_fwd_mark. The bridge determines that the frame should
        be forwarded to swp{1,2}. It is imperative that forward offloading is
        _not_ allowed in this case, as the source hwdom is already "poisoned".
      
        Recording the source hwdom allows this case to be handled properly.
      
      v2->v3: added code comments
      v3->v6: none
      Signed-off-by: default avatarTobias Waldekranz <tobias@waldekranz.com>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarGrygorii Strashko <grygorii.strashko@ti.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f7cf972f
    • Vladimir Oltean's avatar
      net: dpaa2-switch: refactor prechangeupper sanity checks · 45035feb
      Vladimir Oltean authored
      Make more room for some extra code in the NETDEV_PRECHANGEUPPER handler
      by moving what already exists into a dedicated function.
      
      Cc: Ioana Ciornei <ioana.ciornei@nxp.com>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Acked-by: default avatarIoana Ciornei <ioana.ciornei@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      45035feb
    • Vladimir Oltean's avatar
      net: dpaa2-switch: use extack in dpaa2_switch_port_bridge_join · 123338d7
      Vladimir Oltean authored
      We need to propagate the extack argument for
      dpaa2_switch_port_bridge_join to use it in a future patch, and it looks
      like there is already an error message there which is currently printed
      to the console. Move it over netlink so it is properly transmitted to
      user space.
      
      Cc: Ioana Ciornei <ioana.ciornei@nxp.com>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Tested-by: default avatarIoana Ciornei <ioana.ciornei@nxp.com>
      Acked-by: default avatarIoana Ciornei <ioana.ciornei@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      123338d7
  3. 21 Jul, 2021 9 commits