Re: [PATCH v3 net-next 0/7] Make SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE blocking

[Alexandra Winter <wintera@linux.ibm.com>]

On 20.08.21 13:57, Vladimir Oltean wrote:
> Problem statement:
> 
> Any time a driver needs to create a private association between a bridge
> upper interface and use that association within its
> SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE handler, we have an issue with FDB
> entries deleted by the bridge when the port leaves. The issue is that
> all switchdev drivers schedule a work item to have sleepable context,
> and that work item can be actually scheduled after the port has left the
> bridge, which means the association might have already been broken by
> the time the scheduled FDB work item attempts to use it.
> 
> The solution is to modify switchdev to use its embedded SWITCHDEV_F_DEFER
> mechanism to make the FDB notifiers emitted from the fastpath be
> scheduled in sleepable context. All drivers are converted to handle
> SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE from their blocking notifier block
> handler (or register a blocking switchdev notifier handler if they
> didn't have one). This solves the aforementioned problem because the
> bridge waits for the switchdev deferred work items to finish before a
> port leaves (del_nbp calls switchdev_deferred_process), whereas a work
> item privately scheduled by the driver will obviously not be waited upon
> by the bridge, leading to the possibility of having the race.
> 
> This is a dependency for the "DSA FDB isolation" posted here. It was
> split out of that series hence the numbering starts directly at v2.
> 
> https://patchwork.kernel.org/project/netdevbpf/cover/20210818120150.892647-1-vladimir.oltean@nxp.com/
> 
> Changes in v3:
> - make "addr" part of switchdev_fdb_notifier_info to avoid dangling
>   pointers not watched by RCU
> - mlx5 correction
> - build fixes in the S/390 qeth driver
> 
> Vladimir Oltean (7):
>   net: bridge: move br_fdb_replay inside br_switchdev.c
>   net: switchdev: keep the MAC address by value in struct
>     switchdev_notifier_fdb_info
>   net: switchdev: move SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE to the blocking
>     notifier chain
>   net: bridge: switchdev: make br_fdb_replay offer sleepable context to
>     consumers
>   net: switchdev: drop the atomic notifier block from
>     switchdev_bridge_port_{,un}offload
>   net: switchdev: don't assume RCU context in
>     switchdev_handle_fdb_{add,del}_to_device
>   net: dsa: handle SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE synchronously
> 
>  .../ethernet/freescale/dpaa2/dpaa2-switch.c   |  75 ++++------
>  .../marvell/prestera/prestera_switchdev.c     | 104 ++++++-------
>  .../mellanox/mlx5/core/en/rep/bridge.c        |  65 +++++++--
>  .../ethernet/mellanox/mlx5/core/esw/bridge.c  |   2 +-
>  .../ethernet/mellanox/mlxsw/spectrum_router.c |   4 +-
>  .../mellanox/mlxsw/spectrum_switchdev.c       |  62 ++++++--
>  .../microchip/sparx5/sparx5_mactable.c        |   2 +-
>  .../microchip/sparx5/sparx5_switchdev.c       |  72 ++++-----
>  drivers/net/ethernet/mscc/ocelot_net.c        |   3 -
>  drivers/net/ethernet/rocker/rocker_main.c     |  67 ++++-----
>  drivers/net/ethernet/rocker/rocker_ofdpa.c    |   6 +-
>  drivers/net/ethernet/ti/am65-cpsw-nuss.c      |   4 +-
>  drivers/net/ethernet/ti/am65-cpsw-switchdev.c |  54 +++----
>  drivers/net/ethernet/ti/cpsw_new.c            |   4 +-
>  drivers/net/ethernet/ti/cpsw_switchdev.c      |  57 ++++----
>  drivers/s390/net/qeth_l2_main.c               |  26 ++--
>  include/net/switchdev.h                       |  33 ++++-
>  net/bridge/br.c                               |   5 +-
>  net/bridge/br_fdb.c                           |  54 -------
>  net/bridge/br_private.h                       |   6 -
>  net/bridge/br_switchdev.c                     | 128 +++++++++++++---
>  net/dsa/dsa.c                                 |  15 --
>  net/dsa/dsa_priv.h                            |  15 --
>  net/dsa/port.c                                |   3 -
>  net/dsa/slave.c                               | 138 ++++++------------
>  net/switchdev/switchdev.c                     |  61 +++++++-
>  26 files changed, 550 insertions(+), 515 deletions(-)
> 

For drivers/s390/net/qeth_l2_main.c :

Reviewed-and-tested-by: Alexandra Winter <wintera@linux.ibm.com>

Thank you for this proposal, it makes qeth switchdev code more robust, cleaner and gives the
opportunity for future enhancements, like you proposed.