Vladimir Oltean
2021-Jul-20 13:46 UTC
[Bridge] [PATCH v5 net-next 00/10] 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 at nxp.com/ Tobias Waldekranz (2): net: bridge: disambiguate offload_fwd_mark net: bridge: switchdev: recycle unused hwdoms Vladimir Oltean (8): net: dpaa2-switch: use extack in dpaa2_switch_port_bridge_join net: dpaa2-switch: refactor prechangeupper sanity checks mlxsw: spectrum: refactor prechangeupper sanity checks mlxsw: spectrum: refactor leaving an 8021q upper that is a bridge port net: marvell: prestera: refactor prechangeupper sanity checks net: switchdev: guard drivers against multiple obj replays on same bridge port net: bridge: switchdev: let drivers inform which bridge ports are offloaded net: bridge: switchdev object replay helpers for everybody .../ethernet/freescale/dpaa2/dpaa2-switch.c | 69 +++- .../ethernet/marvell/prestera/prestera_main.c | 99 +++-- .../marvell/prestera/prestera_switchdev.c | 42 ++- .../marvell/prestera/prestera_switchdev.h | 7 +- .../net/ethernet/mellanox/mlxsw/spectrum.c | 347 ++++++++++++------ .../net/ethernet/mellanox/mlxsw/spectrum.h | 4 + .../mellanox/mlxsw/spectrum_switchdev.c | 28 +- .../microchip/sparx5/sparx5_switchdev.c | 48 ++- drivers/net/ethernet/mscc/ocelot_net.c | 115 ++++-- drivers/net/ethernet/rocker/rocker.h | 9 +- drivers/net/ethernet/rocker/rocker_main.c | 34 +- drivers/net/ethernet/rocker/rocker_ofdpa.c | 42 ++- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 34 +- drivers/net/ethernet/ti/am65-cpsw-switchdev.c | 14 +- drivers/net/ethernet/ti/am65-cpsw-switchdev.h | 3 + drivers/net/ethernet/ti/cpsw_new.c | 32 +- drivers/net/ethernet/ti/cpsw_switchdev.c | 4 +- drivers/net/ethernet/ti/cpsw_switchdev.h | 3 + include/linux/if_bridge.h | 60 +-- net/bridge/br_fdb.c | 1 - net/bridge/br_if.c | 11 +- net/bridge/br_mdb.c | 1 - net/bridge/br_private.h | 61 ++- net/bridge/br_switchdev.c | 254 +++++++++++-- net/bridge/br_vlan.c | 1 - net/dsa/port.c | 83 ++--- 26 files changed, 1059 insertions(+), 347 deletions(-) -- 2.25.1
Vladimir Oltean
2021-Jul-20 13:46 UTC
[Bridge] [PATCH v5 net-next 01/10] net: dpaa2-switch: use extack in dpaa2_switch_port_bridge_join
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 at nxp.com> Signed-off-by: Vladimir Oltean <vladimir.oltean at nxp.com> Reviewed-by: Florian Fainelli <f.fainelli at gmail.com> Tested-by: Ioana Ciornei <ioana.ciornei at nxp.com> Acked-by: Ioana Ciornei <ioana.ciornei at nxp.com> --- v2->v3: patch is new v3->v5: none drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c index f3d12d0714fb..62d322ebf1f2 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c @@ -1890,7 +1890,8 @@ static int dpaa2_switch_port_attr_set_event(struct net_device *netdev, } static int dpaa2_switch_port_bridge_join(struct net_device *netdev, - struct net_device *upper_dev) + struct net_device *upper_dev, + struct netlink_ext_ack *extack) { struct ethsw_port_priv *port_priv = netdev_priv(netdev); struct ethsw_core *ethsw = port_priv->ethsw_data; @@ -1906,8 +1907,8 @@ static int dpaa2_switch_port_bridge_join(struct net_device *netdev, other_port_priv = netdev_priv(other_dev); if (other_port_priv->ethsw_data != port_priv->ethsw_data) { - netdev_err(netdev, - "Interface from a different DPSW is in the bridge already!\n"); + NL_SET_ERR_MSG_MOD(extack, + "Interface from a different DPSW is in the bridge already"); return -EINVAL; } } @@ -2067,7 +2068,9 @@ static int dpaa2_switch_port_netdevice_event(struct notifier_block *nb, upper_dev = info->upper_dev; if (netif_is_bridge_master(upper_dev)) { if (info->linking) - err = dpaa2_switch_port_bridge_join(netdev, upper_dev); + err = dpaa2_switch_port_bridge_join(netdev, + upper_dev, + extack); else err = dpaa2_switch_port_bridge_leave(netdev); } -- 2.25.1
Vladimir Oltean
2021-Jul-20 13:46 UTC
[Bridge] [PATCH v5 net-next 02/10] net: dpaa2-switch: refactor prechangeupper sanity checks
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 at nxp.com> Signed-off-by: Vladimir Oltean <vladimir.oltean at nxp.com> Reviewed-by: Florian Fainelli <f.fainelli at gmail.com> Acked-by: Ioana Ciornei <ioana.ciornei at nxp.com> --- v2->v3: patch is new v3->v4: fix build error (s/dev/netdev/) v4->v5: none .../ethernet/freescale/dpaa2/dpaa2-switch.c | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c index 62d322ebf1f2..23798feb40b2 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c @@ -2030,6 +2030,28 @@ static int dpaa2_switch_prevent_bridging_with_8021q_upper(struct net_device *net return 0; } +static int +dpaa2_switch_prechangeupper_sanity_checks(struct net_device *netdev, + struct net_device *upper_dev, + struct netlink_ext_ack *extack) +{ + int err; + + if (!br_vlan_enabled(upper_dev)) { + NL_SET_ERR_MSG_MOD(extack, "Cannot join a VLAN-unaware bridge"); + return -EOPNOTSUPP; + } + + err = dpaa2_switch_prevent_bridging_with_8021q_upper(netdev); + if (err) { + NL_SET_ERR_MSG_MOD(extack, + "Cannot join a bridge while VLAN uppers are present"); + return 0; + } + + return 0; +} + static int dpaa2_switch_port_netdevice_event(struct notifier_block *nb, unsigned long event, void *ptr) { @@ -2050,18 +2072,11 @@ static int dpaa2_switch_port_netdevice_event(struct notifier_block *nb, if (!netif_is_bridge_master(upper_dev)) break; - if (!br_vlan_enabled(upper_dev)) { - NL_SET_ERR_MSG_MOD(extack, "Cannot join a VLAN-unaware bridge"); - err = -EOPNOTSUPP; - goto out; - } - - err = dpaa2_switch_prevent_bridging_with_8021q_upper(netdev); - if (err) { - NL_SET_ERR_MSG_MOD(extack, - "Cannot join a bridge while VLAN uppers are present"); + err = dpaa2_switch_prechangeupper_sanity_checks(netdev, + upper_dev, + extack); + if (err) goto out; - } break; case NETDEV_CHANGEUPPER: -- 2.25.1
Vladimir Oltean
2021-Jul-20 13:46 UTC
[Bridge] [PATCH v5 net-next 03/10] mlxsw: spectrum: refactor prechangeupper sanity checks
Make more room for extra code in the NETDEV_PRECHANGEUPPER handlers from mlxsw by moving the existing sanity checks to 2 new dedicated functions. Signed-off-by: Vladimir Oltean <vladimir.oltean at nxp.com> Reviewed-by: Florian Fainelli <f.fainelli at gmail.com> --- v2->v3: patch is new v3->v5: none .../net/ethernet/mellanox/mlxsw/spectrum.c | 272 +++++++++++------- 1 file changed, 169 insertions(+), 103 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c index 88699e678544..c1b78878e5cf 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c @@ -4055,6 +4055,118 @@ static bool mlxsw_sp_bridge_vxlan_is_valid(struct net_device *br_dev, return true; } +static int +mlxsw_sp_prechangeupper_sanity_checks(struct mlxsw_sp *mlxsw_sp, + struct net_device *dev, + struct net_device *lower_dev, + struct net_device *upper_dev, + struct netdev_notifier_changeupper_info *info, + struct netlink_ext_ack *extack) +{ + u16 proto; + + if (!is_vlan_dev(upper_dev) && + !netif_is_lag_master(upper_dev) && + !netif_is_bridge_master(upper_dev) && + !netif_is_ovs_master(upper_dev) && + !netif_is_macvlan(upper_dev)) { + NL_SET_ERR_MSG_MOD(extack, "Unknown upper device type"); + return -EINVAL; + } + + if (!info->linking) + return 0; + + if (netif_is_bridge_master(upper_dev) && + !mlxsw_sp_bridge_device_is_offloaded(mlxsw_sp, upper_dev) && + mlxsw_sp_bridge_has_vxlan(upper_dev) && + !mlxsw_sp_bridge_vxlan_is_valid(upper_dev, extack)) + return -EOPNOTSUPP; + + if (netdev_has_any_upper_dev(upper_dev) && + (!netif_is_bridge_master(upper_dev) || + !mlxsw_sp_bridge_device_is_offloaded(mlxsw_sp, + upper_dev))) { + NL_SET_ERR_MSG_MOD(extack, + "Enslaving a port to a device that already has an upper device is not supported"); + return -EINVAL; + } + + if (netif_is_lag_master(upper_dev) && + !mlxsw_sp_master_lag_check(mlxsw_sp, upper_dev, + info->upper_info, extack)) + return -EINVAL; + + if (netif_is_lag_master(upper_dev) && vlan_uses_dev(dev)) { + NL_SET_ERR_MSG_MOD(extack, + "Master device is a LAG master and this device has a VLAN"); + return -EINVAL; + } + + if (netif_is_lag_port(dev) && is_vlan_dev(upper_dev) && + !netif_is_lag_master(vlan_dev_real_dev(upper_dev))) { + NL_SET_ERR_MSG_MOD(extack, "Can not put a VLAN on a LAG port"); + return -EINVAL; + } + + if (netif_is_macvlan(upper_dev) && + !mlxsw_sp_rif_exists(mlxsw_sp, lower_dev)) { + NL_SET_ERR_MSG_MOD(extack, + "macvlan is only supported on top of router interfaces"); + return -EOPNOTSUPP; + } + + if (netif_is_ovs_master(upper_dev) && vlan_uses_dev(dev)) { + NL_SET_ERR_MSG_MOD(extack, + "Master device is an OVS master and this device has a VLAN"); + return -EINVAL; + } + + if (netif_is_ovs_port(dev) && is_vlan_dev(upper_dev)) { + NL_SET_ERR_MSG_MOD(extack, "Can not put a VLAN on an OVS port"); + return -EINVAL; + } + + if (netif_is_bridge_master(upper_dev)) { + br_vlan_get_proto(upper_dev, &proto); + if (br_vlan_enabled(upper_dev) && + proto != ETH_P_8021Q && proto != ETH_P_8021AD) { + NL_SET_ERR_MSG_MOD(extack, + "Enslaving a port to a bridge with unknown VLAN protocol is not supported"); + return -EOPNOTSUPP; + } + if (vlan_uses_dev(lower_dev) && + br_vlan_enabled(upper_dev) && + proto == ETH_P_8021AD) { + NL_SET_ERR_MSG_MOD(extack, + "Enslaving a port that already has a VLAN upper to an 802.1ad bridge is not supported"); + return -EOPNOTSUPP; + } + } + + if (netif_is_bridge_port(lower_dev) && is_vlan_dev(upper_dev)) { + struct net_device *br_dev = netdev_master_upper_dev_get(lower_dev); + + if (br_vlan_enabled(br_dev)) { + br_vlan_get_proto(br_dev, &proto); + if (proto == ETH_P_8021AD) { + NL_SET_ERR_MSG_MOD(extack, + "VLAN uppers are not supported on a port enslaved to an 802.1ad bridge"); + return -EOPNOTSUPP; + } + } + } + + if (is_vlan_dev(upper_dev) && + ntohs(vlan_dev_vlan_proto(upper_dev)) != ETH_P_8021Q) { + NL_SET_ERR_MSG_MOD(extack, + "VLAN uppers are only supported with 802.1q VLAN protocol"); + return -EOPNOTSUPP; + } + + return 0; +} + static int mlxsw_sp_netdevice_port_upper_event(struct net_device *lower_dev, struct net_device *dev, unsigned long event, void *ptr) @@ -4065,7 +4177,6 @@ static int mlxsw_sp_netdevice_port_upper_event(struct net_device *lower_dev, struct net_device *upper_dev; struct mlxsw_sp *mlxsw_sp; int err = 0; - u16 proto; mlxsw_sp_port = netdev_priv(dev); mlxsw_sp = mlxsw_sp_port->mlxsw_sp; @@ -4075,84 +4186,15 @@ static int mlxsw_sp_netdevice_port_upper_event(struct net_device *lower_dev, switch (event) { case NETDEV_PRECHANGEUPPER: upper_dev = info->upper_dev; - if (!is_vlan_dev(upper_dev) && - !netif_is_lag_master(upper_dev) && - !netif_is_bridge_master(upper_dev) && - !netif_is_ovs_master(upper_dev) && - !netif_is_macvlan(upper_dev)) { - NL_SET_ERR_MSG_MOD(extack, "Unknown upper device type"); - return -EINVAL; - } - if (!info->linking) - break; - if (netif_is_bridge_master(upper_dev) && - !mlxsw_sp_bridge_device_is_offloaded(mlxsw_sp, upper_dev) && - mlxsw_sp_bridge_has_vxlan(upper_dev) && - !mlxsw_sp_bridge_vxlan_is_valid(upper_dev, extack)) - return -EOPNOTSUPP; - if (netdev_has_any_upper_dev(upper_dev) && - (!netif_is_bridge_master(upper_dev) || - !mlxsw_sp_bridge_device_is_offloaded(mlxsw_sp, - upper_dev))) { - NL_SET_ERR_MSG_MOD(extack, "Enslaving a port to a device that already has an upper device is not supported"); - return -EINVAL; - } - if (netif_is_lag_master(upper_dev) && - !mlxsw_sp_master_lag_check(mlxsw_sp, upper_dev, - info->upper_info, extack)) - return -EINVAL; - if (netif_is_lag_master(upper_dev) && vlan_uses_dev(dev)) { - NL_SET_ERR_MSG_MOD(extack, "Master device is a LAG master and this device has a VLAN"); - return -EINVAL; - } - if (netif_is_lag_port(dev) && is_vlan_dev(upper_dev) && - !netif_is_lag_master(vlan_dev_real_dev(upper_dev))) { - NL_SET_ERR_MSG_MOD(extack, "Can not put a VLAN on a LAG port"); - return -EINVAL; - } - if (netif_is_macvlan(upper_dev) && - !mlxsw_sp_rif_exists(mlxsw_sp, lower_dev)) { - NL_SET_ERR_MSG_MOD(extack, "macvlan is only supported on top of router interfaces"); - return -EOPNOTSUPP; - } - if (netif_is_ovs_master(upper_dev) && vlan_uses_dev(dev)) { - NL_SET_ERR_MSG_MOD(extack, "Master device is an OVS master and this device has a VLAN"); - return -EINVAL; - } - if (netif_is_ovs_port(dev) && is_vlan_dev(upper_dev)) { - NL_SET_ERR_MSG_MOD(extack, "Can not put a VLAN on an OVS port"); - return -EINVAL; - } - if (netif_is_bridge_master(upper_dev)) { - br_vlan_get_proto(upper_dev, &proto); - if (br_vlan_enabled(upper_dev) && - proto != ETH_P_8021Q && proto != ETH_P_8021AD) { - NL_SET_ERR_MSG_MOD(extack, "Enslaving a port to a bridge with unknown VLAN protocol is not supported"); - return -EOPNOTSUPP; - } - if (vlan_uses_dev(lower_dev) && - br_vlan_enabled(upper_dev) && - proto == ETH_P_8021AD) { - NL_SET_ERR_MSG_MOD(extack, "Enslaving a port that already has a VLAN upper to an 802.1ad bridge is not supported"); - return -EOPNOTSUPP; - } - } - if (netif_is_bridge_port(lower_dev) && is_vlan_dev(upper_dev)) { - struct net_device *br_dev = netdev_master_upper_dev_get(lower_dev); - - if (br_vlan_enabled(br_dev)) { - br_vlan_get_proto(br_dev, &proto); - if (proto == ETH_P_8021AD) { - NL_SET_ERR_MSG_MOD(extack, "VLAN uppers are not supported on a port enslaved to an 802.1ad bridge"); - return -EOPNOTSUPP; - } - } - } - if (is_vlan_dev(upper_dev) && - ntohs(vlan_dev_vlan_proto(upper_dev)) != ETH_P_8021Q) { - NL_SET_ERR_MSG_MOD(extack, "VLAN uppers are only supported with 802.1q VLAN protocol"); - return -EOPNOTSUPP; - } + + err = mlxsw_sp_prechangeupper_sanity_checks(mlxsw_sp, + dev, lower_dev, + upper_dev, + info, + extack); + if (err) + return err; + break; case NETDEV_CHANGEUPPER: upper_dev = info->upper_dev; @@ -4260,6 +4302,46 @@ static int mlxsw_sp_netdevice_lag_event(struct net_device *lag_dev, return 0; } +static int +mlxsw_sp_vlan_prechangeupper_sanity_checks(struct mlxsw_sp *mlxsw_sp, + struct net_device *vlan_dev, + struct net_device *upper_dev, + struct netdev_notifier_changeupper_info *info, + struct netlink_ext_ack *extack) +{ + if (!netif_is_bridge_master(upper_dev) && + !netif_is_macvlan(upper_dev)) { + NL_SET_ERR_MSG_MOD(extack, "Unknown upper device type"); + return -EINVAL; + } + + if (!info->linking) + return 0; + + if (netif_is_bridge_master(upper_dev) && + !mlxsw_sp_bridge_device_is_offloaded(mlxsw_sp, upper_dev) && + mlxsw_sp_bridge_has_vxlan(upper_dev) && + !mlxsw_sp_bridge_vxlan_is_valid(upper_dev, extack)) + return -EOPNOTSUPP; + + if (netdev_has_any_upper_dev(upper_dev) && + (!netif_is_bridge_master(upper_dev) || + !mlxsw_sp_bridge_device_is_offloaded(mlxsw_sp, + upper_dev))) { + NL_SET_ERR_MSG_MOD(extack, + "Enslaving a port to a device that already has an upper device is not supported"); + return -EINVAL; + } + + if (netif_is_macvlan(upper_dev) && + !mlxsw_sp_rif_exists(mlxsw_sp, vlan_dev)) { + NL_SET_ERR_MSG_MOD(extack, "macvlan is only supported on top of router interfaces"); + return -EOPNOTSUPP; + } + + return 0; +} + static int mlxsw_sp_netdevice_port_vlan_event(struct net_device *vlan_dev, struct net_device *dev, unsigned long event, void *ptr, @@ -4277,30 +4359,14 @@ static int mlxsw_sp_netdevice_port_vlan_event(struct net_device *vlan_dev, switch (event) { case NETDEV_PRECHANGEUPPER: upper_dev = info->upper_dev; - if (!netif_is_bridge_master(upper_dev) && - !netif_is_macvlan(upper_dev)) { - NL_SET_ERR_MSG_MOD(extack, "Unknown upper device type"); - return -EINVAL; - } - if (!info->linking) - break; - if (netif_is_bridge_master(upper_dev) && - !mlxsw_sp_bridge_device_is_offloaded(mlxsw_sp, upper_dev) && - mlxsw_sp_bridge_has_vxlan(upper_dev) && - !mlxsw_sp_bridge_vxlan_is_valid(upper_dev, extack)) - return -EOPNOTSUPP; - if (netdev_has_any_upper_dev(upper_dev) && - (!netif_is_bridge_master(upper_dev) || - !mlxsw_sp_bridge_device_is_offloaded(mlxsw_sp, - upper_dev))) { - NL_SET_ERR_MSG_MOD(extack, "Enslaving a port to a device that already has an upper device is not supported"); - return -EINVAL; - } - if (netif_is_macvlan(upper_dev) && - !mlxsw_sp_rif_exists(mlxsw_sp, vlan_dev)) { - NL_SET_ERR_MSG_MOD(extack, "macvlan is only supported on top of router interfaces"); - return -EOPNOTSUPP; - } + + err = mlxsw_sp_vlan_prechangeupper_sanity_checks(mlxsw_sp, + vlan_dev, + upper_dev, + info, extack); + if (err) + return err; + break; case NETDEV_CHANGEUPPER: upper_dev = info->upper_dev; -- 2.25.1
Vladimir Oltean
2021-Jul-20 13:46 UTC
[Bridge] [PATCH v5 net-next 04/10] mlxsw: spectrum: refactor leaving an 8021q upper that is a bridge port
For symmetry with mlxsw_sp_port_lag_leave(), introduce a small function called mlxsw_sp_port_vlan_leave() which checks whether the 8021q upper we're leaving is a bridge port, and if it is, stop offloading that bridge too. Signed-off-by: Vladimir Oltean <vladimir.oltean at nxp.com> Reviewed-by: Florian Fainelli <f.fainelli at gmail.com> --- v3->v4: patch is new v4->v5: none .../net/ethernet/mellanox/mlxsw/spectrum.c | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c index c1b78878e5cf..b3d1fdc2d094 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c @@ -3994,6 +3994,19 @@ static void mlxsw_sp_port_ovs_leave(struct mlxsw_sp_port *mlxsw_sp_port) mlxsw_sp_port_vp_mode_set(mlxsw_sp_port, false); } +static void mlxsw_sp_port_vlan_leave(struct mlxsw_sp_port *mlxsw_sp_port, + struct net_device *vlan_dev) +{ + struct net_device *br_dev; + + if (!netif_is_bridge_port(vlan_dev)) + return; + + br_dev = netdev_master_upper_dev_get(vlan_dev); + + mlxsw_sp_port_bridge_leave(mlxsw_sp_port, vlan_dev, br_dev); +} + static bool mlxsw_sp_bridge_has_multiple_vxlans(struct net_device *br_dev) { unsigned int num_vxlans = 0; @@ -4225,16 +4238,8 @@ static int mlxsw_sp_netdevice_port_upper_event(struct net_device *lower_dev, } else if (netif_is_macvlan(upper_dev)) { if (!info->linking) mlxsw_sp_rif_macvlan_del(mlxsw_sp, upper_dev); - } else if (is_vlan_dev(upper_dev)) { - struct net_device *br_dev; - - if (!netif_is_bridge_port(upper_dev)) - break; - if (info->linking) - break; - br_dev = netdev_master_upper_dev_get(upper_dev); - mlxsw_sp_port_bridge_leave(mlxsw_sp_port, upper_dev, - br_dev); + } else if (is_vlan_dev(upper_dev) && !info->linking) { + mlxsw_sp_port_vlan_leave(mlxsw_sp_port, upper_dev); } break; } -- 2.25.1
Vladimir Oltean
2021-Jul-20 13:46 UTC
[Bridge] [PATCH v5 net-next 05/10] net: marvell: prestera: refactor prechangeupper sanity checks
In preparation of adding more code to the NETDEV_PRECHANGEUPPER handler, move the existing sanity checks into a dedicated function. Cc: Vadym Kochan <vkochan at marvell.com> Cc: Taras Chornyi <tchornyi at marvell.com> Signed-off-by: Vladimir Oltean <vladimir.oltean at nxp.com> Reviewed-by: Florian Fainelli <f.fainelli at gmail.com> --- v2->v3: patch is new v3->v4: none v4->v5: none .../ethernet/marvell/prestera/prestera_main.c | 71 ++++++++++++------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c index 226f4ff29f6e..1f3c8cd6ced2 100644 --- a/drivers/net/ethernet/marvell/prestera/prestera_main.c +++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c @@ -699,6 +699,45 @@ static bool prestera_lag_master_check(struct net_device *lag_dev, return true; } +static int prestera_prechangeupper_sanity_checks(struct net_device *dev, + struct net_device *upper, + struct netdev_notifier_changeupper_info *info, + struct netlink_ext_ack *extack) +{ + if (!netif_is_bridge_master(upper) && + !netif_is_lag_master(upper)) { + NL_SET_ERR_MSG_MOD(extack, "Unknown upper device type"); + return -EINVAL; + } + + if (!info->linking) + return 0; + + if (netdev_has_any_upper_dev(upper)) { + NL_SET_ERR_MSG_MOD(extack, "Upper device is already enslaved"); + return -EINVAL; + } + + if (netif_is_lag_master(upper) && + !prestera_lag_master_check(upper, info->upper_info, extack)) + return -EOPNOTSUPP; + + if (netif_is_lag_master(upper) && vlan_uses_dev(dev)) { + NL_SET_ERR_MSG_MOD(extack, + "Master device is a LAG master and port has a VLAN"); + return -EINVAL; + } + + if (netif_is_lag_port(dev) && is_vlan_dev(upper) && + !netif_is_lag_master(vlan_dev_real_dev(upper))) { + NL_SET_ERR_MSG_MOD(extack, + "Can not put a VLAN on a LAG port"); + return -EINVAL; + } + + return 0; +} + static int prestera_netdev_port_event(struct net_device *lower, struct net_device *dev, unsigned long event, void *ptr) @@ -707,40 +746,18 @@ static int prestera_netdev_port_event(struct net_device *lower, struct prestera_port *port = netdev_priv(dev); struct netlink_ext_ack *extack; struct net_device *upper; + int err; extack = netdev_notifier_info_to_extack(&info->info); upper = info->upper_dev; switch (event) { case NETDEV_PRECHANGEUPPER: - if (!netif_is_bridge_master(upper) && - !netif_is_lag_master(upper)) { - NL_SET_ERR_MSG_MOD(extack, "Unknown upper device type"); - return -EINVAL; - } - - if (!info->linking) - break; - - if (netdev_has_any_upper_dev(upper)) { - NL_SET_ERR_MSG_MOD(extack, "Upper device is already enslaved"); - return -EINVAL; - } + err = prestera_prechangeupper_sanity_checks(dev, upper, info, + extack); + if (err) + return err; - if (netif_is_lag_master(upper) && - !prestera_lag_master_check(upper, info->upper_info, extack)) - return -EOPNOTSUPP; - if (netif_is_lag_master(upper) && vlan_uses_dev(dev)) { - NL_SET_ERR_MSG_MOD(extack, - "Master device is a LAG master and port has a VLAN"); - return -EINVAL; - } - if (netif_is_lag_port(dev) && is_vlan_dev(upper) && - !netif_is_lag_master(vlan_dev_real_dev(upper))) { - NL_SET_ERR_MSG_MOD(extack, - "Can not put a VLAN on a LAG port"); - return -EINVAL; - } break; case NETDEV_CHANGEUPPER: -- 2.25.1
Vladimir Oltean
2021-Jul-20 13:46 UTC
[Bridge] [PATCH v5 net-next 06/10] net: switchdev: guard drivers against multiple obj replays on same bridge port
Prepare the drivers which support LAG offload but don't have support for switchdev object replay yet, i.e. the mlxsw and prestera drivers, to deal with bridge switchdev objects being replayed on the LAG bridge port multiple times, once for each time a physical port beneath the LAG calls switchdev_bridge_port_offload(). Cc: Vadym Kochan <vkochan at marvell.com> Cc: Taras Chornyi <tchornyi at marvell.com> Signed-off-by: Vladimir Oltean <vladimir.oltean at nxp.com> Reviewed-by: Florian Fainelli <f.fainelli at gmail.com> --- v3->v4: squash mlxsw and prestera into a single patch v4->v5: none drivers/net/ethernet/marvell/prestera/prestera_switchdev.c | 6 ++++++ drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c index 0b3e8f2db294..a1fc4ab53ccf 100644 --- a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c +++ b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c @@ -1047,6 +1047,9 @@ static int prestera_port_obj_add(struct net_device *dev, const void *ctx, struct prestera_port *port = netdev_priv(dev); const struct switchdev_obj_port_vlan *vlan; + if (ctx && ctx != port) + return 0; + switch (obj->id) { case SWITCHDEV_OBJ_ID_PORT_VLAN: vlan = SWITCHDEV_OBJ_PORT_VLAN(obj); @@ -1083,6 +1086,9 @@ static int prestera_port_obj_del(struct net_device *dev, const void *ctx, { struct prestera_port *port = netdev_priv(dev); + if (ctx && ctx != port) + return 0; + switch (obj->id) { case SWITCHDEV_OBJ_ID_PORT_VLAN: return prestera_port_vlans_del(port, SWITCHDEV_OBJ_PORT_VLAN(obj)); diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c index 61911fed6aeb..5a0c7c94874e 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c @@ -1772,6 +1772,9 @@ static int mlxsw_sp_port_obj_add(struct net_device *dev, const void *ctx, const struct switchdev_obj_port_vlan *vlan; int err = 0; + if (ctx && ctx != mlxsw_sp_port) + return 0; + switch (obj->id) { case SWITCHDEV_OBJ_ID_PORT_VLAN: vlan = SWITCHDEV_OBJ_PORT_VLAN(obj); @@ -1920,6 +1923,9 @@ static int mlxsw_sp_port_obj_del(struct net_device *dev, const void *ctx, struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev); int err = 0; + if (ctx && ctx != mlxsw_sp_port) + return 0; + switch (obj->id) { case SWITCHDEV_OBJ_ID_PORT_VLAN: err = mlxsw_sp_port_vlans_del(mlxsw_sp_port, -- 2.25.1
Vladimir Oltean
2021-Jul-20 13:46 UTC
[Bridge] [PATCH v5 net-next 07/10] net: bridge: disambiguate offload_fwd_mark
From: Tobias Waldekranz <tobias at waldekranz.com> 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->v5: none Signed-off-by: Tobias Waldekranz <tobias at waldekranz.com> Signed-off-by: Vladimir Oltean <vladimir.oltean at nxp.com> Reviewed-by: Grygorii Strashko <grygorii.strashko at ti.com> Reviewed-by: Florian Fainelli <f.fainelli at gmail.com> --- net/bridge/br_if.c | 2 +- net/bridge/br_private.h | 21 ++++++++++++++++----- net/bridge/br_switchdev.c | 16 ++++++++-------- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index 6e4a32354a13..838a277e3cf7 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -643,7 +643,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev, if (err) goto err5; - err = nbp_switchdev_mark_set(p); + err = nbp_switchdev_hwdom_set(p); if (err) goto err6; diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 2b48b204205e..54e29a8576a1 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -329,7 +329,10 @@ struct net_bridge_port { struct netpoll *np; #endif #ifdef CONFIG_NET_SWITCHDEV - int offload_fwd_mark; + /* Identifier used to group ports that share the same switchdev + * hardware domain. + */ + int hwdom; #endif u16 group_fwd_mask; u16 backup_redirected_cnt; @@ -476,7 +479,10 @@ struct net_bridge { u32 auto_cnt; #ifdef CONFIG_NET_SWITCHDEV - int offload_fwd_mark; + /* Counter used to make sure that hardware domains get unique + * identifiers in case a bridge spans multiple switchdev instances. + */ + int last_hwdom; #endif struct hlist_head fdb_list; @@ -506,7 +512,12 @@ struct br_input_skb_cb { #endif #ifdef CONFIG_NET_SWITCHDEV - int offload_fwd_mark; + /* The switchdev hardware domain from which this packet was received. + * If skb->offload_fwd_mark was set, then this packet was already + * forwarded by hardware to the other ports in the source hardware + * domain, otherwise it wasn't. + */ + int src_hwdom; #endif }; @@ -1645,7 +1656,7 @@ static inline void br_sysfs_delbr(struct net_device *dev) { return; } /* br_switchdev.c */ #ifdef CONFIG_NET_SWITCHDEV -int nbp_switchdev_mark_set(struct net_bridge_port *p); +int nbp_switchdev_hwdom_set(struct net_bridge_port *p); void nbp_switchdev_frame_mark(const struct net_bridge_port *p, struct sk_buff *skb); bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, @@ -1665,7 +1676,7 @@ static inline void br_switchdev_frame_unmark(struct sk_buff *skb) skb->offload_fwd_mark = 0; } #else -static inline int nbp_switchdev_mark_set(struct net_bridge_port *p) +static inline int nbp_switchdev_hwdom_set(struct net_bridge_port *p) { return 0; } diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c index d3adee0f91f9..833fd30482c2 100644 --- a/net/bridge/br_switchdev.c +++ b/net/bridge/br_switchdev.c @@ -8,20 +8,20 @@ #include "br_private.h" -static int br_switchdev_mark_get(struct net_bridge *br, struct net_device *dev) +static int br_switchdev_hwdom_get(struct net_bridge *br, struct net_device *dev) { struct net_bridge_port *p; /* dev is yet to be added to the port list. */ list_for_each_entry(p, &br->port_list, list) { if (netdev_port_same_parent_id(dev, p->dev)) - return p->offload_fwd_mark; + return p->hwdom; } - return ++br->offload_fwd_mark; + return ++br->last_hwdom; } -int nbp_switchdev_mark_set(struct net_bridge_port *p) +int nbp_switchdev_hwdom_set(struct net_bridge_port *p) { struct netdev_phys_item_id ppid = { }; int err; @@ -35,7 +35,7 @@ int nbp_switchdev_mark_set(struct net_bridge_port *p) return err; } - p->offload_fwd_mark = br_switchdev_mark_get(p->br, p->dev); + p->hwdom = br_switchdev_hwdom_get(p->br, p->dev); return 0; } @@ -43,15 +43,15 @@ int nbp_switchdev_mark_set(struct net_bridge_port *p) void nbp_switchdev_frame_mark(const struct net_bridge_port *p, struct sk_buff *skb) { - if (skb->offload_fwd_mark && !WARN_ON_ONCE(!p->offload_fwd_mark)) - BR_INPUT_SKB_CB(skb)->offload_fwd_mark = p->offload_fwd_mark; + if (p->hwdom) + BR_INPUT_SKB_CB(skb)->src_hwdom = p->hwdom; } bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, const struct sk_buff *skb) { return !skb->offload_fwd_mark || - BR_INPUT_SKB_CB(skb)->offload_fwd_mark != p->offload_fwd_mark; + BR_INPUT_SKB_CB(skb)->src_hwdom != p->hwdom; } /* Flags that can be offloaded to hardware */ -- 2.25.1
Vladimir Oltean
2021-Jul-20 13:46 UTC
[Bridge] [PATCH v5 net-next 08/10] net: bridge: switchdev: recycle unused hwdoms
From: Tobias Waldekranz <tobias at waldekranz.com> 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->v5: none Signed-off-by: Tobias Waldekranz <tobias at waldekranz.com> Signed-off-by: Vladimir Oltean <vladimir.oltean at nxp.com> --- net/bridge/br_if.c | 4 +- net/bridge/br_private.h | 27 ++++++++--- net/bridge/br_switchdev.c | 94 ++++++++++++++++++++++++++------------- 3 files changed, 86 insertions(+), 39 deletions(-) diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index 838a277e3cf7..c0df50e4abbb 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -349,6 +349,7 @@ static void del_nbp(struct net_bridge_port *p) nbp_backup_clear(p); nbp_update_port_count(br); + nbp_switchdev_del(p); netdev_upper_dev_unlink(dev, br->dev); @@ -643,7 +644,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev, if (err) goto err5; - err = nbp_switchdev_hwdom_set(p); + err = nbp_switchdev_add(p); if (err) goto err6; @@ -719,6 +720,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev, list_del_rcu(&p->list); br_fdb_delete_by_port(br, p, 0, 1); nbp_update_port_count(br); + nbp_switchdev_del(p); err6: netdev_upper_dev_unlink(dev, br->dev); err5: diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 54e29a8576a1..a23c565b8970 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -29,6 +29,8 @@ #define BR_MULTICAST_DEFAULT_HASH_MAX 4096 +#define BR_HWDOM_MAX BITS_PER_LONG + #define BR_VERSION "2.3" /* Control of forwarding link local multicast */ @@ -483,6 +485,8 @@ struct net_bridge { * identifiers in case a bridge spans multiple switchdev instances. */ int last_hwdom; + /* Bit mask of hardware domain numbers in use */ + unsigned long busy_hwdoms; #endif struct hlist_head fdb_list; @@ -1656,7 +1660,6 @@ static inline void br_sysfs_delbr(struct net_device *dev) { return; } /* br_switchdev.c */ #ifdef CONFIG_NET_SWITCHDEV -int nbp_switchdev_hwdom_set(struct net_bridge_port *p); void nbp_switchdev_frame_mark(const struct net_bridge_port *p, struct sk_buff *skb); bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, @@ -1670,17 +1673,15 @@ void br_switchdev_fdb_notify(struct net_bridge *br, int br_switchdev_port_vlan_add(struct net_device *dev, u16 vid, u16 flags, struct netlink_ext_ack *extack); int br_switchdev_port_vlan_del(struct net_device *dev, u16 vid); +int nbp_switchdev_add(struct net_bridge_port *p); +void nbp_switchdev_del(struct net_bridge_port *p); +void br_switchdev_init(struct net_bridge *br); static inline void br_switchdev_frame_unmark(struct sk_buff *skb) { skb->offload_fwd_mark = 0; } #else -static inline int nbp_switchdev_hwdom_set(struct net_bridge_port *p) -{ - return 0; -} - static inline void nbp_switchdev_frame_mark(const struct net_bridge_port *p, struct sk_buff *skb) { @@ -1721,6 +1722,20 @@ br_switchdev_fdb_notify(struct net_bridge *br, static inline void br_switchdev_frame_unmark(struct sk_buff *skb) { } + +static inline int nbp_switchdev_add(struct net_bridge_port *p) +{ + return 0; +} + +static inline void nbp_switchdev_del(struct net_bridge_port *p) +{ +} + +static inline void br_switchdev_init(struct net_bridge *br) +{ +} + #endif /* CONFIG_NET_SWITCHDEV */ /* br_arp_nd_proxy.c */ diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c index 833fd30482c2..f3120f13c293 100644 --- a/net/bridge/br_switchdev.c +++ b/net/bridge/br_switchdev.c @@ -8,38 +8,6 @@ #include "br_private.h" -static int br_switchdev_hwdom_get(struct net_bridge *br, struct net_device *dev) -{ - struct net_bridge_port *p; - - /* dev is yet to be added to the port list. */ - list_for_each_entry(p, &br->port_list, list) { - if (netdev_port_same_parent_id(dev, p->dev)) - return p->hwdom; - } - - return ++br->last_hwdom; -} - -int nbp_switchdev_hwdom_set(struct net_bridge_port *p) -{ - struct netdev_phys_item_id ppid = { }; - int err; - - ASSERT_RTNL(); - - err = dev_get_port_parent_id(p->dev, &ppid, true); - if (err) { - if (err == -EOPNOTSUPP) - return 0; - return err; - } - - p->hwdom = br_switchdev_hwdom_get(p->br, p->dev); - - return 0; -} - void nbp_switchdev_frame_mark(const struct net_bridge_port *p, struct sk_buff *skb) { @@ -156,3 +124,65 @@ int br_switchdev_port_vlan_del(struct net_device *dev, u16 vid) return switchdev_port_obj_del(dev, &v.obj); } + +static int nbp_switchdev_hwdom_set(struct net_bridge_port *joining) +{ + struct net_bridge *br = joining->br; + struct net_bridge_port *p; + int hwdom; + + /* joining is yet to be added to the port list. */ + list_for_each_entry(p, &br->port_list, list) { + if (netdev_port_same_parent_id(joining->dev, p->dev)) { + joining->hwdom = p->hwdom; + return 0; + } + } + + hwdom = find_next_zero_bit(&br->busy_hwdoms, BR_HWDOM_MAX, 1); + if (hwdom >= BR_HWDOM_MAX) + return -EBUSY; + + set_bit(hwdom, &br->busy_hwdoms); + joining->hwdom = hwdom; + return 0; +} + +static void nbp_switchdev_hwdom_put(struct net_bridge_port *leaving) +{ + struct net_bridge *br = leaving->br; + struct net_bridge_port *p; + + /* leaving is no longer in the port list. */ + list_for_each_entry(p, &br->port_list, list) { + if (p->hwdom == leaving->hwdom) + return; + } + + clear_bit(leaving->hwdom, &br->busy_hwdoms); +} + +int nbp_switchdev_add(struct net_bridge_port *p) +{ + struct netdev_phys_item_id ppid = { }; + int err; + + ASSERT_RTNL(); + + err = dev_get_port_parent_id(p->dev, &ppid, true); + if (err) { + if (err == -EOPNOTSUPP) + return 0; + return err; + } + + return nbp_switchdev_hwdom_set(p); +} + +void nbp_switchdev_del(struct net_bridge_port *p) +{ + ASSERT_RTNL(); + + if (p->hwdom) + nbp_switchdev_hwdom_put(p); +} -- 2.25.1
Vladimir Oltean
2021-Jul-20 13:46 UTC
[Bridge] [PATCH v5 net-next 09/10] net: bridge: switchdev: let drivers inform which bridge ports are offloaded
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. During the info->linking == false path, switchdev_bridge_port_unoffload() is strategically put in the NETDEV_PRECHANGEUPPER notifier as opposed to NETDEV_CHANGEUPPER. The reason for this has to do with a future migration of the switchdev object replay helpers (br_*_replay) from a pull mode (completely initiated by the driver) to a semi-push mode (the bridge initiates the replay when the switchdev driver declares that it offloads a port). On deletion, the switchdev object replay helpers need the netdev adjacency lists to be valid, and that is only true in NETDEV_PRECHANGEUPPER. So we need to add trivial glue code to all drivers to handle a "pre bridge leave" event, and that is where we hook the switchdev_bridge_port_unoffload() call. Cc: Vadym Kochan <vkochan at marvell.com> Cc: Taras Chornyi <tchornyi at marvell.com> Cc: Ioana Ciornei <ioana.ciornei at nxp.com> Cc: Lars Povlsen <lars.povlsen at microchip.com> Cc: Steen Hegelund <Steen.Hegelund at microchip.com> Cc: UNGLinuxDriver at microchip.com Cc: Claudiu Manoil <claudiu.manoil at nxp.com> Cc: Alexandre Belloni <alexandre.belloni at bootlin.com> Cc: Grygorii Strashko <grygorii.strashko at ti.com> Signed-off-by: Vladimir Oltean <vladimir.oltean at nxp.com> Tested-by: Ioana Ciornei <ioana.ciornei at nxp.com> # dpaa2-switch: regression Acked-by: Ioana Ciornei <ioana.ciornei at nxp.com> # dpaa2-switch Tested-by: Horatiu Vultur <horatiu.vultur at microchip.com> # ocelot-switch --- v2->v3: patch is new v3->v4: added mlxsw_sp_port_pre_lag_leave() and mlxsw_sp_port_pre_vlan_leave() v4->v5: use for the comparison in nbp_switchdev_hwdom_set() the nbp->ppid provided by switchdev_bridge_port_offload() instead of recursing through the lower interfaces of the nbp->dev .../ethernet/freescale/dpaa2/dpaa2-switch.c | 14 ++- .../ethernet/marvell/prestera/prestera_main.c | 30 +++++- .../marvell/prestera/prestera_switchdev.c | 29 +++++- .../marvell/prestera/prestera_switchdev.h | 7 +- .../net/ethernet/mellanox/mlxsw/spectrum.c | 50 ++++++++++ .../net/ethernet/mellanox/mlxsw/spectrum.h | 4 + .../mellanox/mlxsw/spectrum_switchdev.c | 13 ++- .../microchip/sparx5/sparx5_switchdev.c | 41 +++++++- drivers/net/ethernet/mscc/ocelot_net.c | 81 ++++++++++++++++ drivers/net/ethernet/rocker/rocker.h | 6 +- drivers/net/ethernet/rocker/rocker_main.c | 30 +++++- drivers/net/ethernet/rocker/rocker_ofdpa.c | 36 ++++++- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 28 +++++- drivers/net/ethernet/ti/cpsw_new.c | 26 ++++- include/linux/if_bridge.h | 26 +++++ net/bridge/br_if.c | 13 +-- net/bridge/br_private.h | 13 +-- net/bridge/br_switchdev.c | 96 ++++++++++++++++--- net/dsa/port.c | 20 +++- 19 files changed, 507 insertions(+), 56 deletions(-) diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c index 23798feb40b2..2cd8a38e4f30 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c @@ -1930,7 +1930,7 @@ static int dpaa2_switch_port_bridge_join(struct net_device *netdev, if (err) goto err_egress_flood; - return 0; + return switchdev_bridge_port_offload(netdev, netdev, extack); err_egress_flood: dpaa2_switch_port_set_fdb(port_priv, NULL); @@ -1957,6 +1957,13 @@ static int dpaa2_switch_port_restore_rxvlan(struct net_device *vdev, int vid, vo return dpaa2_switch_port_vlan_add(arg, vlan_proto, vid); } +static int dpaa2_switch_port_pre_bridge_leave(struct net_device *netdev, + struct net_device *upper_dev, + struct netlink_ext_ack *extack) +{ + return switchdev_bridge_port_unoffload(netdev, netdev, extack); +} + static int dpaa2_switch_port_bridge_leave(struct net_device *netdev) { struct ethsw_port_priv *port_priv = netdev_priv(netdev); @@ -2078,6 +2085,11 @@ static int dpaa2_switch_port_netdevice_event(struct notifier_block *nb, if (err) goto out; + if (!info->linking) + err = dpaa2_switch_port_pre_bridge_leave(netdev, + upper_dev, + extack); + break; case NETDEV_CHANGEUPPER: upper_dev = info->upper_dev; diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c index 1f3c8cd6ced2..62a0ccfdbe6b 100644 --- a/drivers/net/ethernet/marvell/prestera/prestera_main.c +++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c @@ -540,7 +540,8 @@ static void prestera_lag_destroy(struct prestera_switch *sw, } static int prestera_lag_port_add(struct prestera_port *port, - struct net_device *lag_dev) + struct net_device *lag_dev, + struct netlink_ext_ack *extack) { struct prestera_switch *sw = port->sw; struct prestera_lag *lag; @@ -570,6 +571,21 @@ static int prestera_lag_port_add(struct prestera_port *port, return 0; } +static int prestera_pre_lag_leave(struct prestera_port *port, + struct net_device *lag_dev, + struct netlink_ext_ack *extack) +{ + if (netif_is_bridge_port(lag_dev)) { + struct net_device *br_dev; + + br_dev = netdev_master_upper_dev_get(lag_dev); + + return prestera_pre_bridge_port_leave(br_dev, port, extack); + } + + return 0; +} + static int prestera_lag_port_del(struct prestera_port *port) { struct prestera_switch *sw = port->sw; @@ -758,17 +774,25 @@ static int prestera_netdev_port_event(struct net_device *lower, if (err) return err; + if (netif_is_bridge_master(upper) && !info->linking) + return prestera_pre_bridge_port_leave(upper, port, + extack); + else if (netif_is_lag_master(upper) && !info->linking) + return prestera_pre_lag_leave(port, upper, extack); + break; case NETDEV_CHANGEUPPER: if (netif_is_bridge_master(upper)) { if (info->linking) - return prestera_bridge_port_join(upper, port); + return prestera_bridge_port_join(upper, port, + extack); else prestera_bridge_port_leave(upper, port); } else if (netif_is_lag_master(upper)) { if (info->linking) - return prestera_lag_port_add(port, upper); + return prestera_lag_port_add(port, upper, + extack); else prestera_lag_port_del(port); } diff --git a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c index a1fc4ab53ccf..4be82c043991 100644 --- a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c +++ b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c @@ -480,7 +480,8 @@ prestera_bridge_1d_port_join(struct prestera_bridge_port *br_port) } int prestera_bridge_port_join(struct net_device *br_dev, - struct prestera_port *port) + struct prestera_port *port, + struct netlink_ext_ack *extack) { struct prestera_switchdev *swdev = port->sw->swdev; struct prestera_bridge_port *br_port; @@ -500,6 +501,10 @@ int prestera_bridge_port_join(struct net_device *br_dev, goto err_brport_create; } + err = switchdev_bridge_port_offload(br_port->dev, port->dev, extack); + if (err) + goto err_brport_offload; + if (bridge->vlan_enabled) return 0; @@ -510,12 +515,34 @@ int prestera_bridge_port_join(struct net_device *br_dev, return 0; err_port_join: + switchdev_bridge_port_unoffload(br_port->dev, port->dev, extack); +err_brport_offload: prestera_bridge_port_put(br_port); err_brport_create: prestera_bridge_put(bridge); return err; } +int prestera_pre_bridge_port_leave(struct net_device *br_dev, + struct prestera_port *port, + struct netlink_ext_ack *extack) +{ + struct prestera_switchdev *swdev = port->sw->swdev; + struct prestera_bridge_port *br_port; + struct prestera_bridge *bridge; + + bridge = prestera_bridge_by_dev(swdev, br_dev); + if (!bridge) + return -ENODEV; + + br_port = __prestera_bridge_port_by_dev(bridge, port->dev); + if (!br_port) + return -ENODEV; + + return switchdev_bridge_port_unoffload(br_port->dev, port->dev, + extack); +} + static void prestera_bridge_1q_port_leave(struct prestera_bridge_port *br_port) { struct prestera_port *port = netdev_priv(br_port->dev); diff --git a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.h b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.h index a91bc35d235f..05ec54e2af26 100644 --- a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.h +++ b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.h @@ -8,7 +8,12 @@ int prestera_switchdev_init(struct prestera_switch *sw); void prestera_switchdev_fini(struct prestera_switch *sw); int prestera_bridge_port_join(struct net_device *br_dev, - struct prestera_port *port); + struct prestera_port *port, + struct netlink_ext_ack *extack); + +int prestera_pre_bridge_port_leave(struct net_device *br_dev, + struct prestera_port *port, + struct netlink_ext_ack *extack); void prestera_bridge_port_leave(struct net_device *br_dev, struct prestera_port *port); diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c index b3d1fdc2d094..05ff4936cf5d 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c @@ -3811,6 +3811,21 @@ static int mlxsw_sp_port_lag_join(struct mlxsw_sp_port *mlxsw_sp_port, return err; } +static int mlxsw_sp_port_pre_lag_leave(struct mlxsw_sp_port *mlxsw_sp_port, + struct net_device *lag_dev, + struct netlink_ext_ack *extack) +{ + struct net_device *br_dev; + + if (!netif_is_bridge_port(lag_dev)) + return 0; + + br_dev = netdev_master_upper_dev_get(lag_dev); + + return mlxsw_sp_port_pre_bridge_leave(mlxsw_sp_port, lag_dev, + br_dev, extack); +} + static void mlxsw_sp_port_lag_leave(struct mlxsw_sp_port *mlxsw_sp_port, struct net_device *lag_dev) { @@ -3994,6 +4009,21 @@ static void mlxsw_sp_port_ovs_leave(struct mlxsw_sp_port *mlxsw_sp_port) mlxsw_sp_port_vp_mode_set(mlxsw_sp_port, false); } +static int mlxsw_sp_port_pre_vlan_leave(struct mlxsw_sp_port *mlxsw_sp_port, + struct net_device *vlan_dev, + struct netlink_ext_ack *extack) +{ + struct net_device *br_dev; + + if (!netif_is_bridge_port(vlan_dev)) + return 0; + + br_dev = netdev_master_upper_dev_get(vlan_dev); + + return mlxsw_sp_port_pre_bridge_leave(mlxsw_sp_port, vlan_dev, + br_dev, extack); +} + static void mlxsw_sp_port_vlan_leave(struct mlxsw_sp_port *mlxsw_sp_port, struct net_device *vlan_dev) { @@ -4208,6 +4238,20 @@ static int mlxsw_sp_netdevice_port_upper_event(struct net_device *lower_dev, if (err) return err; + if (netif_is_bridge_master(upper_dev) && !info->linking) + err = mlxsw_sp_port_pre_bridge_leave(mlxsw_sp_port, + lower_dev, + upper_dev, + extack); + if (netif_is_lag_master(upper_dev) && !info->linking) + err = mlxsw_sp_port_pre_lag_leave(mlxsw_sp_port, + upper_dev, + extack); + if (is_vlan_dev(upper_dev) && !info->linking) + err = mlxsw_sp_port_pre_vlan_leave(mlxsw_sp_port, + upper_dev, + extack); + break; case NETDEV_CHANGEUPPER: upper_dev = info->upper_dev; @@ -4372,6 +4416,12 @@ static int mlxsw_sp_netdevice_port_vlan_event(struct net_device *vlan_dev, if (err) return err; + if (netif_is_bridge_master(upper_dev) && !info->linking) + err = mlxsw_sp_port_pre_bridge_leave(mlxsw_sp_port, + vlan_dev, + upper_dev, + extack); + break; case NETDEV_CHANGEUPPER: upper_dev = info->upper_dev; diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h index f99db88ee884..54d22ecee4e2 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h @@ -596,6 +596,10 @@ int mlxsw_sp_port_bridge_join(struct mlxsw_sp_port *mlxsw_sp_port, struct net_device *brport_dev, struct net_device *br_dev, struct netlink_ext_ack *extack); +int mlxsw_sp_port_pre_bridge_leave(struct mlxsw_sp_port *mlxsw_sp_port, + struct net_device *brport_dev, + struct net_device *br_dev, + struct netlink_ext_ack *extack); void mlxsw_sp_port_bridge_leave(struct mlxsw_sp_port *mlxsw_sp_port, struct net_device *brport_dev, struct net_device *br_dev); diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c index 5a0c7c94874e..731234a2ace3 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c @@ -2367,6 +2367,7 @@ int mlxsw_sp_port_bridge_join(struct mlxsw_sp_port *mlxsw_sp_port, { struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp; struct mlxsw_sp_bridge_device *bridge_device; + struct net_device *dev = mlxsw_sp_port->dev; struct mlxsw_sp_bridge_port *bridge_port; int err; @@ -2381,13 +2382,23 @@ int mlxsw_sp_port_bridge_join(struct mlxsw_sp_port *mlxsw_sp_port, if (err) goto err_port_join; - return 0; + return switchdev_bridge_port_offload(brport_dev, dev, extack); err_port_join: mlxsw_sp_bridge_port_put(mlxsw_sp->bridge, bridge_port); return err; } +int mlxsw_sp_port_pre_bridge_leave(struct mlxsw_sp_port *mlxsw_sp_port, + struct net_device *brport_dev, + struct net_device *br_dev, + struct netlink_ext_ack *extack) +{ + struct net_device *dev = mlxsw_sp_port->dev; + + return switchdev_bridge_port_unoffload(brport_dev, dev, extack); +} + void mlxsw_sp_port_bridge_leave(struct mlxsw_sp_port *mlxsw_sp_port, struct net_device *brport_dev, struct net_device *br_dev) diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c b/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c index a72e3b3b596e..270b9fabce91 100644 --- a/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c @@ -93,9 +93,11 @@ static int sparx5_port_attr_set(struct net_device *dev, const void *ctx, } static int sparx5_port_bridge_join(struct sparx5_port *port, - struct net_device *bridge) + struct net_device *bridge, + struct netlink_ext_ack *extack) { struct sparx5 *sparx5 = port->sparx5; + struct net_device *ndev = port->ndev; if (bitmap_empty(sparx5->bridge_mask, SPX5_PORTS)) /* First bridged port */ @@ -112,9 +114,17 @@ static int sparx5_port_bridge_join(struct sparx5_port *port, /* Port enters in bridge mode therefor don't need to copy to CPU * frames for multicast in case the bridge is not requesting them */ - __dev_mc_unsync(port->ndev, sparx5_mc_unsync); + __dev_mc_unsync(ndev, sparx5_mc_unsync); - return 0; + return switchdev_bridge_port_offload(ndev, ndev, extack); +} + +static int sparx5_port_pre_bridge_leave(struct sparx5_port *port, + struct netlink_ext_ack *extack) +{ + struct net_device *ndev = port->ndev; + + return switchdev_bridge_port_unoffload(ndev, ndev, extack); } static void sparx5_port_bridge_leave(struct sparx5_port *port, @@ -135,15 +145,35 @@ static void sparx5_port_bridge_leave(struct sparx5_port *port, __dev_mc_sync(port->ndev, sparx5_mc_sync, sparx5_mc_unsync); } +static int +sparx5_port_prechangeupper(struct net_device *dev, + struct netdev_notifier_changeupper_info *info) +{ + struct sparx5_port *port = netdev_priv(dev); + struct netlink_ext_ack *extack; + int err = 0; + + extack = netdev_notifier_info_to_extack(&info->info); + + if (netif_is_bridge_master(info->upper_dev) && !info->linking) + err = sparx5_port_pre_bridge_leave(port, extack); + + return err; +} + static int sparx5_port_changeupper(struct net_device *dev, struct netdev_notifier_changeupper_info *info) { struct sparx5_port *port = netdev_priv(dev); + struct netlink_ext_ack *extack; int err = 0; + extack = netdev_notifier_info_to_extack(&info->info); + if (netif_is_bridge_master(info->upper_dev)) { if (info->linking) - err = sparx5_port_bridge_join(port, info->upper_dev); + err = sparx5_port_bridge_join(port, info->upper_dev, + extack); else sparx5_port_bridge_leave(port, info->upper_dev); @@ -177,6 +207,9 @@ static int sparx5_netdevice_port_event(struct net_device *dev, return 0; switch (event) { + case NETDEV_PRECHANGEUPPER: + err = sparx5_port_prechangeupper(dev, ptr); + break; case NETDEV_CHANGEUPPER: err = sparx5_port_changeupper(dev, ptr); break; diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c index e9d260d84bf3..dcb393a35c0e 100644 --- a/drivers/net/ethernet/mscc/ocelot_net.c +++ b/drivers/net/ethernet/mscc/ocelot_net.c @@ -1216,6 +1216,10 @@ static int ocelot_netdevice_bridge_join(struct net_device *dev, ocelot_port_bridge_join(ocelot, port, bridge); + err = switchdev_bridge_port_offload(brport_dev, dev, extack); + if (err) + goto err_switchdev_offload; + err = ocelot_switchdev_sync(ocelot, port, brport_dev, bridge, extack); if (err) goto err_switchdev_sync; @@ -1223,10 +1227,19 @@ static int ocelot_netdevice_bridge_join(struct net_device *dev, return 0; err_switchdev_sync: + switchdev_bridge_port_unoffload(brport_dev, dev, extack); +err_switchdev_offload: ocelot_port_bridge_leave(ocelot, port, bridge); return err; } +static int ocelot_netdevice_pre_bridge_leave(struct net_device *dev, + struct net_device *brport_dev, + struct netlink_ext_ack *extack) +{ + return switchdev_bridge_port_unoffload(brport_dev, dev, extack); +} + static int ocelot_netdevice_bridge_leave(struct net_device *dev, struct net_device *brport_dev, struct net_device *bridge) @@ -1279,6 +1292,19 @@ static int ocelot_netdevice_lag_join(struct net_device *dev, return err; } +static int ocelot_netdevice_pre_lag_leave(struct net_device *dev, + struct net_device *bond, + struct netlink_ext_ack *extack) +{ + struct net_device *bridge_dev; + + bridge_dev = netdev_master_upper_dev_get(bond); + if (!bridge_dev || !netif_is_bridge_master(bridge_dev)) + return 0; + + return ocelot_netdevice_pre_bridge_leave(dev, bond, extack); +} + static int ocelot_netdevice_lag_leave(struct net_device *dev, struct net_device *bond) { @@ -1355,6 +1381,50 @@ ocelot_netdevice_lag_changeupper(struct net_device *dev, return NOTIFY_DONE; } +static int +ocelot_netdevice_prechangeupper(struct net_device *dev, + struct net_device *brport_dev, + struct netdev_notifier_changeupper_info *info) +{ + struct netlink_ext_ack *extack; + int err = 0; + + extack = netdev_notifier_info_to_extack(&info->info); + + if (netif_is_bridge_master(info->upper_dev) && !info->linking) + err = ocelot_netdevice_pre_bridge_leave(dev, brport_dev, + extack); + + if (netif_is_lag_master(info->upper_dev) && !info->linking) + err = ocelot_netdevice_pre_lag_leave(dev, info->upper_dev, + extack); + + return notifier_from_errno(err); +} + +static int +ocelot_netdevice_lag_prechangeupper(struct net_device *dev, + struct netdev_notifier_changeupper_info *info) +{ + struct net_device *lower; + struct list_head *iter; + int err = NOTIFY_DONE; + + netdev_for_each_lower_dev(dev, lower, iter) { + struct ocelot_port_private *priv = netdev_priv(lower); + struct ocelot_port *ocelot_port = &priv->port; + + if (ocelot_port->bond != dev) + return NOTIFY_OK; + + err = ocelot_netdevice_prechangeupper(dev, lower, info); + if (err) + return notifier_from_errno(err); + } + + return NOTIFY_DONE; +} + static int ocelot_netdevice_changelowerstate(struct net_device *dev, struct netdev_lag_lower_state_info *info) @@ -1382,6 +1452,17 @@ static int ocelot_netdevice_event(struct notifier_block *unused, struct net_device *dev = netdev_notifier_info_to_dev(ptr); switch (event) { + case NETDEV_PRECHANGEUPPER: { + struct netdev_notifier_changeupper_info *info = ptr; + + if (ocelot_netdevice_dev_check(dev)) + return ocelot_netdevice_prechangeupper(dev, dev, info); + + if (netif_is_lag_master(dev)) + return ocelot_netdevice_lag_prechangeupper(dev, info); + + break; + } case NETDEV_CHANGEUPPER: { struct netdev_notifier_changeupper_info *info = ptr; diff --git a/drivers/net/ethernet/rocker/rocker.h b/drivers/net/ethernet/rocker/rocker.h index 315a6e5c0f59..d31cee1cdda9 100644 --- a/drivers/net/ethernet/rocker/rocker.h +++ b/drivers/net/ethernet/rocker/rocker.h @@ -119,7 +119,11 @@ struct rocker_world_ops { int (*port_obj_fdb_del)(struct rocker_port *rocker_port, u16 vid, const unsigned char *addr); int (*port_master_linked)(struct rocker_port *rocker_port, - struct net_device *master); + struct net_device *master, + struct netlink_ext_ack *extack); + int (*port_master_pre_unlink)(struct rocker_port *rocker_port, + struct net_device *master, + struct netlink_ext_ack *extack); int (*port_master_unlinked)(struct rocker_port *rocker_port, struct net_device *master); int (*port_neigh_update)(struct rocker_port *rocker_port, diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c index a46633606cae..2e3e413406ac 100644 --- a/drivers/net/ethernet/rocker/rocker_main.c +++ b/drivers/net/ethernet/rocker/rocker_main.c @@ -1670,13 +1670,25 @@ rocker_world_port_fdb_del(struct rocker_port *rocker_port, } static int rocker_world_port_master_linked(struct rocker_port *rocker_port, - struct net_device *master) + struct net_device *master, + struct netlink_ext_ack *extack) { struct rocker_world_ops *wops = rocker_port->rocker->wops; if (!wops->port_master_linked) return -EOPNOTSUPP; - return wops->port_master_linked(rocker_port, master); + return wops->port_master_linked(rocker_port, master, extack); +} + +static int rocker_world_port_master_pre_unlink(struct rocker_port *rocker_port, + struct net_device *master, + struct netlink_ext_ack *extack) +{ + struct rocker_world_ops *wops = rocker_port->rocker->wops; + + if (!wops->port_master_pre_unlink) + return -EOPNOTSUPP; + return wops->port_master_pre_unlink(rocker_port, master, extack); } static int rocker_world_port_master_unlinked(struct rocker_port *rocker_port, @@ -3107,6 +3119,7 @@ struct rocker_port *rocker_port_dev_lower_find(struct net_device *dev, static int rocker_netdevice_event(struct notifier_block *unused, unsigned long event, void *ptr) { + struct netlink_ext_ack *extack = netdev_notifier_info_to_extack(ptr); struct net_device *dev = netdev_notifier_info_to_dev(ptr); struct netdev_notifier_changeupper_info *info; struct rocker_port *rocker_port; @@ -3116,6 +3129,16 @@ static int rocker_netdevice_event(struct notifier_block *unused, return NOTIFY_DONE; switch (event) { + case NETDEV_PRECHANGEUPPER: + info = ptr; + if (!info->master) + goto out; + rocker_port = netdev_priv(dev); + if (!info->linking) + err = rocker_world_port_master_pre_unlink(rocker_port, + info->upper_dev, + extack); + break; case NETDEV_CHANGEUPPER: info = ptr; if (!info->master) @@ -3123,7 +3146,8 @@ static int rocker_netdevice_event(struct notifier_block *unused, rocker_port = netdev_priv(dev); if (info->linking) { err = rocker_world_port_master_linked(rocker_port, - info->upper_dev); + info->upper_dev, + extack); if (err) netdev_warn(dev, "failed to reflect master linked (err %d)\n", err); diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c index 967a634ee9ac..c32d076bcbf6 100644 --- a/drivers/net/ethernet/rocker/rocker_ofdpa.c +++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c @@ -2571,8 +2571,10 @@ static int ofdpa_port_obj_fdb_del(struct rocker_port *rocker_port, } static int ofdpa_port_bridge_join(struct ofdpa_port *ofdpa_port, - struct net_device *bridge) + struct net_device *bridge, + struct netlink_ext_ack *extack) { + struct net_device *dev = ofdpa_port->dev; int err; /* Port is joining bridge, so the internal VLAN for the @@ -2592,7 +2594,19 @@ static int ofdpa_port_bridge_join(struct ofdpa_port *ofdpa_port, ofdpa_port->bridge_dev = bridge; - return ofdpa_port_vlan_add(ofdpa_port, OFDPA_UNTAGGED_VID, 0); + err = ofdpa_port_vlan_add(ofdpa_port, OFDPA_UNTAGGED_VID, 0); + if (err) + return err; + + return switchdev_bridge_port_offload(dev, dev, extack); +} + +static int ofdpa_port_pre_bridge_leave(struct ofdpa_port *ofdpa_port, + struct netlink_ext_ack *extack) +{ + struct net_device *dev = ofdpa_port->dev; + + return switchdev_bridge_port_unoffload(dev, dev, extack); } static int ofdpa_port_bridge_leave(struct ofdpa_port *ofdpa_port) @@ -2637,18 +2651,31 @@ static int ofdpa_port_ovs_changed(struct ofdpa_port *ofdpa_port, } static int ofdpa_port_master_linked(struct rocker_port *rocker_port, - struct net_device *master) + struct net_device *master, + struct netlink_ext_ack *extack) { struct ofdpa_port *ofdpa_port = rocker_port->wpriv; int err = 0; if (netif_is_bridge_master(master)) - err = ofdpa_port_bridge_join(ofdpa_port, master); + err = ofdpa_port_bridge_join(ofdpa_port, master, extack); else if (netif_is_ovs_master(master)) err = ofdpa_port_ovs_changed(ofdpa_port, master); return err; } +static int ofdpa_port_master_pre_unlink(struct rocker_port *rocker_port, + struct net_device *master, + struct netlink_ext_ack *extack) +{ + struct ofdpa_port *ofdpa_port = rocker_port->wpriv; + int err = 0; + + if (netif_is_bridge_master(master)) + err = ofdpa_port_pre_bridge_leave(ofdpa_port, extack); + return err; +} + static int ofdpa_port_master_unlinked(struct rocker_port *rocker_port, struct net_device *master) { @@ -2800,6 +2827,7 @@ struct rocker_world_ops rocker_ofdpa_ops = { .port_obj_fdb_add = ofdpa_port_obj_fdb_add, .port_obj_fdb_del = ofdpa_port_obj_fdb_del, .port_master_linked = ofdpa_port_master_linked, + .port_master_pre_unlink = ofdpa_port_master_pre_unlink, .port_master_unlinked = ofdpa_port_master_unlinked, .port_neigh_update = ofdpa_port_neigh_update, .port_neigh_destroy = ofdpa_port_neigh_destroy, diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 718539cdd2f2..30e8b21dc6db 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -7,6 +7,7 @@ #include <linux/clk.h> #include <linux/etherdevice.h> +#include <linux/if_bridge.h> #include <linux/if_vlan.h> #include <linux/interrupt.h> #include <linux/kernel.h> @@ -2077,10 +2078,13 @@ bool am65_cpsw_port_dev_check(const struct net_device *ndev) return false; } -static int am65_cpsw_netdevice_port_link(struct net_device *ndev, struct net_device *br_ndev) +static int am65_cpsw_netdevice_port_link(struct net_device *ndev, + struct net_device *br_ndev, + struct netlink_ext_ack *extack) { struct am65_cpsw_common *common = am65_ndev_to_common(ndev); struct am65_cpsw_ndev_priv *priv = am65_ndev_to_priv(ndev); + int err; if (!common->br_members) { common->hw_bridge_dev = br_ndev; @@ -2092,6 +2096,10 @@ static int am65_cpsw_netdevice_port_link(struct net_device *ndev, struct net_dev return -EOPNOTSUPP; } + err = switchdev_bridge_port_offload(ndev, ndev, extack); + if (err) + return err; + common->br_members |= BIT(priv->port->port_id); am65_cpsw_port_offload_fwd_mark_update(common); @@ -2099,6 +2107,12 @@ static int am65_cpsw_netdevice_port_link(struct net_device *ndev, struct net_dev return NOTIFY_DONE; } +static int am65_cpsw_netdevice_port_pre_unlink(struct net_device *ndev, + struct netlink_ext_ack *extack) +{ + return switchdev_bridge_port_unoffload(ndev, ndev, extack); +} + static void am65_cpsw_netdevice_port_unlink(struct net_device *ndev) { struct am65_cpsw_common *common = am65_ndev_to_common(ndev); @@ -2116,6 +2130,7 @@ static void am65_cpsw_netdevice_port_unlink(struct net_device *ndev) static int am65_cpsw_netdevice_event(struct notifier_block *unused, unsigned long event, void *ptr) { + struct netlink_ext_ack *extack = netdev_notifier_info_to_extack(ptr); struct net_device *ndev = netdev_notifier_info_to_dev(ptr); struct netdev_notifier_changeupper_info *info; int ret = NOTIFY_DONE; @@ -2124,12 +2139,21 @@ static int am65_cpsw_netdevice_event(struct notifier_block *unused, return NOTIFY_DONE; switch (event) { + case NETDEV_PRECHANGEUPPER: + info = ptr; + + if (netif_is_bridge_master(info->upper_dev) && !info->linking) + ret = am65_cpsw_netdevice_port_pre_unlink(ndev, extack); + + break; case NETDEV_CHANGEUPPER: info = ptr; if (netif_is_bridge_master(info->upper_dev)) { if (info->linking) - ret = am65_cpsw_netdevice_port_link(ndev, info->upper_dev); + ret = am65_cpsw_netdevice_port_link(ndev, + info->upper_dev, + extack); else am65_cpsw_netdevice_port_unlink(ndev); } diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c index 57d279fdcc9f..8c586d1ff7d7 100644 --- a/drivers/net/ethernet/ti/cpsw_new.c +++ b/drivers/net/ethernet/ti/cpsw_new.c @@ -11,6 +11,7 @@ #include <linux/module.h> #include <linux/irqreturn.h> #include <linux/interrupt.h> +#include <linux/if_bridge.h> #include <linux/if_ether.h> #include <linux/etherdevice.h> #include <linux/net_tstamp.h> @@ -1499,10 +1500,12 @@ static void cpsw_port_offload_fwd_mark_update(struct cpsw_common *cpsw) } static int cpsw_netdevice_port_link(struct net_device *ndev, - struct net_device *br_ndev) + struct net_device *br_ndev, + struct netlink_ext_ack *extack) { struct cpsw_priv *priv = netdev_priv(ndev); struct cpsw_common *cpsw = priv->cpsw; + int err; if (!cpsw->br_members) { cpsw->hw_bridge_dev = br_ndev; @@ -1514,6 +1517,10 @@ static int cpsw_netdevice_port_link(struct net_device *ndev, return -EOPNOTSUPP; } + err = switchdev_bridge_port_offload(ndev, ndev, extack); + if (err) + return err; + cpsw->br_members |= BIT(priv->emac_port); cpsw_port_offload_fwd_mark_update(cpsw); @@ -1521,6 +1528,12 @@ static int cpsw_netdevice_port_link(struct net_device *ndev, return NOTIFY_DONE; } +static int cpsw_netdevice_port_pre_unlink(struct net_device *ndev, + struct netlink_ext_ack *extack) +{ + return switchdev_bridge_port_unoffload(ndev, ndev, extack); +} + static void cpsw_netdevice_port_unlink(struct net_device *ndev) { struct cpsw_priv *priv = netdev_priv(ndev); @@ -1538,6 +1551,7 @@ static void cpsw_netdevice_port_unlink(struct net_device *ndev) static int cpsw_netdevice_event(struct notifier_block *unused, unsigned long event, void *ptr) { + struct netlink_ext_ack *extack = netdev_notifier_info_to_extack(ptr); struct net_device *ndev = netdev_notifier_info_to_dev(ptr); struct netdev_notifier_changeupper_info *info; int ret = NOTIFY_DONE; @@ -1546,13 +1560,21 @@ static int cpsw_netdevice_event(struct notifier_block *unused, return NOTIFY_DONE; switch (event) { + case NETDEV_PRECHANGEUPPER: + info = ptr; + + if (netif_is_bridge_master(info->upper_dev) && !info->linking) + ret = cpsw_netdevice_port_pre_unlink(ndev, extack); + + break; case NETDEV_CHANGEUPPER: info = ptr; if (netif_is_bridge_master(info->upper_dev)) { if (info->linking) ret = cpsw_netdevice_port_link(ndev, - info->upper_dev); + info->upper_dev, + extack); else cpsw_netdevice_port_unlink(ndev); } diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h index b651c5e32a28..d0bec83488b9 100644 --- a/include/linux/if_bridge.h +++ b/include/linux/if_bridge.h @@ -206,4 +206,30 @@ static inline int br_fdb_replay(const struct net_device *br_dev, } #endif +#if IS_ENABLED(CONFIG_BRIDGE) && IS_ENABLED(CONFIG_NET_SWITCHDEV) + +int switchdev_bridge_port_offload(struct net_device *brport_dev, + struct net_device *dev, + struct netlink_ext_ack *extack); +int switchdev_bridge_port_unoffload(struct net_device *brport_dev, + struct net_device *dev, + struct netlink_ext_ack *extack); + +#else + +static inline int switchdev_bridge_port_offload(struct net_device *brport_dev, + struct net_device *dev, + struct netlink_ext_ack *extack) +{ + return -EINVAL; +} + +static inline int switchdev_bridge_port_unoffload(struct net_device *brport_dev, + struct net_device *dev, + struct netlink_ext_ack *extack) +{ + return -EINVAL; +} +#endif + #endif diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index c0df50e4abbb..86f6d7e93ea8 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -349,7 +349,6 @@ static void del_nbp(struct net_bridge_port *p) nbp_backup_clear(p); nbp_update_port_count(br); - nbp_switchdev_del(p); netdev_upper_dev_unlink(dev, br->dev); @@ -644,10 +643,6 @@ int br_add_if(struct net_bridge *br, struct net_device *dev, if (err) goto err5; - err = nbp_switchdev_add(p); - if (err) - goto err6; - dev_disable_lro(dev); list_add_rcu(&p->list, &br->port_list); @@ -685,13 +680,13 @@ int br_add_if(struct net_bridge *br, struct net_device *dev, */ err = dev_pre_changeaddr_notify(br->dev, dev->dev_addr, extack); if (err) - goto err7; + goto err6; } err = nbp_vlan_init(p, extack); if (err) { netdev_err(dev, "failed to initialize vlan filtering on this port\n"); - goto err7; + goto err6; } spin_lock_bh(&br->lock); @@ -714,14 +709,12 @@ int br_add_if(struct net_bridge *br, struct net_device *dev, return 0; -err7: +err6: if (fdb_synced) br_fdb_unsync_static(br, p); list_del_rcu(&p->list); br_fdb_delete_by_port(br, p, 0, 1); nbp_update_port_count(br); - nbp_switchdev_del(p); -err6: netdev_upper_dev_unlink(dev, br->dev); err5: dev->priv_flags &= ~IFF_BRIDGE_PORT; diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index a23c565b8970..46236302eed5 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -335,6 +335,8 @@ struct net_bridge_port { * hardware domain. */ int hwdom; + int offload_count; + struct netdev_phys_item_id ppid; #endif u16 group_fwd_mask; u16 backup_redirected_cnt; @@ -1673,8 +1675,6 @@ void br_switchdev_fdb_notify(struct net_bridge *br, int br_switchdev_port_vlan_add(struct net_device *dev, u16 vid, u16 flags, struct netlink_ext_ack *extack); int br_switchdev_port_vlan_del(struct net_device *dev, u16 vid); -int nbp_switchdev_add(struct net_bridge_port *p); -void nbp_switchdev_del(struct net_bridge_port *p); void br_switchdev_init(struct net_bridge *br); static inline void br_switchdev_frame_unmark(struct sk_buff *skb) @@ -1723,15 +1723,6 @@ static inline void br_switchdev_frame_unmark(struct sk_buff *skb) { } -static inline int nbp_switchdev_add(struct net_bridge_port *p) -{ - return 0; -} - -static inline void nbp_switchdev_del(struct net_bridge_port *p) -{ -} - static inline void br_switchdev_init(struct net_bridge *br) { } diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c index f3120f13c293..0eb4bc272bff 100644 --- a/net/bridge/br_switchdev.c +++ b/net/bridge/br_switchdev.c @@ -133,7 +133,7 @@ static int nbp_switchdev_hwdom_set(struct net_bridge_port *joining) /* joining is yet to be added to the port list. */ list_for_each_entry(p, &br->port_list, list) { - if (netdev_port_same_parent_id(joining->dev, p->dev)) { + if (netdev_phys_item_id_same(&joining->ppid, &p->ppid)) { joining->hwdom = p->hwdom; return 0; } @@ -162,27 +162,101 @@ static void nbp_switchdev_hwdom_put(struct net_bridge_port *leaving) clear_bit(leaving->hwdom, &br->busy_hwdoms); } -int nbp_switchdev_add(struct net_bridge_port *p) +static int nbp_switchdev_add(struct net_bridge_port *p, + struct netdev_phys_item_id ppid, + struct netlink_ext_ack *extack) { - struct netdev_phys_item_id ppid = { }; - int err; + if (p->offload_count) { + /* Prevent unsupported configurations such as a bridge port + * which is a bonding interface, and the member ports are from + * different hardware switches. + */ + if (!netdev_phys_item_id_same(&p->ppid, &ppid)) { + NL_SET_ERR_MSG_MOD(extack, + "Same bridge port cannot be offloaded by two physical switches"); + return -EBUSY; + } - ASSERT_RTNL(); + /* Tolerate drivers that call switchdev_bridge_port_offload() + * more than once for the same bridge port, such as when the + * bridge port is an offloaded bonding/team interface. + */ + p->offload_count++; - err = dev_get_port_parent_id(p->dev, &ppid, true); - if (err) { - if (err == -EOPNOTSUPP) - return 0; - return err; + return 0; } + p->ppid = ppid; + p->offload_count = 1; + return nbp_switchdev_hwdom_set(p); } -void nbp_switchdev_del(struct net_bridge_port *p) +static void nbp_switchdev_del(struct net_bridge_port *p, + struct netdev_phys_item_id ppid) { ASSERT_RTNL(); + if (WARN_ON(!netdev_phys_item_id_same(&p->ppid, &ppid))) + return; + + if (WARN_ON(!p->offload_count)) + return; + + p->offload_count--; + + if (p->offload_count) + return; + if (p->hwdom) nbp_switchdev_hwdom_put(p); } + +/* Let the bridge know that this port is offloaded, so that it can assign a + * switchdev hardware domain to it. + */ +int switchdev_bridge_port_offload(struct net_device *brport_dev, + struct net_device *dev, + struct netlink_ext_ack *extack) +{ + struct netdev_phys_item_id ppid; + struct net_bridge_port *p; + int err; + + ASSERT_RTNL(); + + p = br_port_get_rtnl(brport_dev); + if (!p) + return -ENODEV; + + err = dev_get_port_parent_id(dev, &ppid, false); + if (err) + return err; + + return nbp_switchdev_add(p, ppid, extack); +} +EXPORT_SYMBOL_GPL(switchdev_bridge_port_offload); + +int switchdev_bridge_port_unoffload(struct net_device *brport_dev, + struct net_device *dev, + struct netlink_ext_ack *extack) +{ + struct netdev_phys_item_id ppid; + struct net_bridge_port *p; + int err; + + ASSERT_RTNL(); + + p = br_port_get_rtnl(dev); + if (!p) + return -ENODEV; + + err = dev_get_port_parent_id(dev, &ppid, false); + if (err) + return err; + + nbp_switchdev_del(p, ppid); + + return 0; +} +EXPORT_SYMBOL_GPL(switchdev_bridge_port_unoffload); diff --git a/net/dsa/port.c b/net/dsa/port.c index 28b45b7e66df..b824b6f8aa84 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -292,6 +292,8 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br, .port = dp->index, .br = br, }; + struct net_device *dev = dp->slave; + struct net_device *brport_dev; int err; /* Here the interface is already bridged. Reflect the current @@ -299,16 +301,24 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br, */ dp->bridge_dev = br; + brport_dev = dsa_port_to_bridge_port(dp); + err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_JOIN, &info); if (err) goto out_rollback; - err = dsa_port_switchdev_sync(dp, extack); + err = switchdev_bridge_port_offload(brport_dev, dev, extack); if (err) goto out_rollback_unbridge; + err = dsa_port_switchdev_sync(dp, extack); + if (err) + goto out_rollback_unoffload; + return 0; +out_rollback_unoffload: + switchdev_bridge_port_unoffload(brport_dev, dev, extack); out_rollback_unbridge: dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info); out_rollback: @@ -319,6 +329,14 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br, int dsa_port_pre_bridge_leave(struct dsa_port *dp, struct net_device *br, struct netlink_ext_ack *extack) { + struct net_device *brport_dev = dsa_port_to_bridge_port(dp); + struct net_device *dev = dp->slave; + int err; + + err = switchdev_bridge_port_unoffload(brport_dev, dev, extack); + if (err) + return err; + return dsa_port_switchdev_unsync_objs(dp, br, extack); } -- 2.25.1
Vladimir Oltean
2021-Jul-20 13:46 UTC
[Bridge] [PATCH v5 net-next 10/10] net: bridge: switchdev object replay helpers for everybody
Starting with commit 4f2673b3a2b6 ("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 0d2cfbd41c4a ("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 we are past the beta testing period for the bridge replay helpers used in DSA only, it is time to roll them out 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. am65_cpsw and cpsw had the same name for the switchdev notifiers, so I renamed the am65_cpsw ones with an am65_ prefix. 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. (c) I do not expect a lot of functional change introduced for drivers in this patch, because: - 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 2c4eca3ef716 ("net: bridge: switchdev: include local flag in FDB notifications"). Driver authors can lift this restriction as they wish. - br_mdb_replay() should now fix the issue described in commit 4f2673b3a2b6 ("net: bridge: add helper to replay port and host-joined mdb entries") for all drivers, I don't see any downside. Cc: Vadym Kochan <vkochan at marvell.com> Cc: Taras Chornyi <tchornyi at marvell.com> Cc: Ioana Ciornei <ioana.ciornei at nxp.com> Cc: Lars Povlsen <lars.povlsen at microchip.com> Cc: Steen Hegelund <Steen.Hegelund at microchip.com> Cc: UNGLinuxDriver at microchip.com Cc: Claudiu Manoil <claudiu.manoil at nxp.com> Cc: Alexandre Belloni <alexandre.belloni at bootlin.com> Cc: Grygorii Strashko <grygorii.strashko at ti.com> Signed-off-by: Vladimir Oltean <vladimir.oltean at nxp.com> Acked-by: Ioana Ciornei <ioana.ciornei at nxp.com> # dpaa2-switch --- v2->v3: patch is new v3->v4: passing the switchdev notifier blocks as argument to switchdev_bridge_port_offload() v4->v5: fixed the reference to the proper br_mdb_replay() patch in the commit message .../ethernet/freescale/dpaa2/dpaa2-switch.c | 13 ++- .../marvell/prestera/prestera_switchdev.c | 13 ++- .../mellanox/mlxsw/spectrum_switchdev.c | 13 ++- .../microchip/sparx5/sparx5_switchdev.c | 11 ++- drivers/net/ethernet/mscc/ocelot_net.c | 40 ++++----- drivers/net/ethernet/rocker/rocker.h | 3 + drivers/net/ethernet/rocker/rocker_main.c | 4 +- drivers/net/ethernet/rocker/rocker_ofdpa.c | 10 ++- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 10 ++- drivers/net/ethernet/ti/am65-cpsw-switchdev.c | 14 +-- drivers/net/ethernet/ti/am65-cpsw-switchdev.h | 3 + drivers/net/ethernet/ti/cpsw_new.c | 10 ++- drivers/net/ethernet/ti/cpsw_switchdev.c | 4 +- drivers/net/ethernet/ti/cpsw_switchdev.h | 3 + include/linux/if_bridge.h | 56 ++++-------- net/bridge/br_fdb.c | 1 - net/bridge/br_mdb.c | 1 - net/bridge/br_private.h | 26 ++++++ net/bridge/br_switchdev.c | 86 ++++++++++++++++++- net/bridge/br_vlan.c | 1 - net/dsa/port.c | 79 ++++------------- 21 files changed, 243 insertions(+), 158 deletions(-) diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c index 2cd8a38e4f30..a7db964a9ee6 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c @@ -1889,6 +1889,9 @@ static int dpaa2_switch_port_attr_set_event(struct net_device *netdev, return notifier_from_errno(err); } +static struct notifier_block dpaa2_switch_port_switchdev_nb; +static struct notifier_block dpaa2_switch_port_switchdev_blocking_nb; + static int dpaa2_switch_port_bridge_join(struct net_device *netdev, struct net_device *upper_dev, struct netlink_ext_ack *extack) @@ -1930,7 +1933,10 @@ static int dpaa2_switch_port_bridge_join(struct net_device *netdev, if (err) goto err_egress_flood; - return switchdev_bridge_port_offload(netdev, netdev, extack); + return switchdev_bridge_port_offload(netdev, netdev, NULL, + &dpaa2_switch_port_switchdev_nb, + &dpaa2_switch_port_switchdev_blocking_nb, + extack); err_egress_flood: dpaa2_switch_port_set_fdb(port_priv, NULL); @@ -1961,7 +1967,10 @@ static int dpaa2_switch_port_pre_bridge_leave(struct net_device *netdev, struct net_device *upper_dev, struct netlink_ext_ack *extack) { - return switchdev_bridge_port_unoffload(netdev, netdev, extack); + return switchdev_bridge_port_unoffload(netdev, netdev, NULL, + &dpaa2_switch_port_switchdev_nb, + &dpaa2_switch_port_switchdev_blocking_nb, + extack); } static int dpaa2_switch_port_bridge_leave(struct net_device *netdev) diff --git a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c index 4be82c043991..0072b6251522 100644 --- a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c +++ b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c @@ -501,7 +501,10 @@ int prestera_bridge_port_join(struct net_device *br_dev, goto err_brport_create; } - err = switchdev_bridge_port_offload(br_port->dev, port->dev, extack); + err = switchdev_bridge_port_offload(br_port->dev, port->dev, port, + &swdev->swdev_nb, + &swdev->swdev_nb_blk, + extack); if (err) goto err_brport_offload; @@ -515,7 +518,9 @@ int prestera_bridge_port_join(struct net_device *br_dev, return 0; err_port_join: - switchdev_bridge_port_unoffload(br_port->dev, port->dev, extack); + switchdev_bridge_port_unoffload(br_port->dev, port->dev, port, + &swdev->swdev_nb, &swdev->swdev_nb_blk, + extack); err_brport_offload: prestera_bridge_port_put(br_port); err_brport_create: @@ -539,7 +544,9 @@ int prestera_pre_bridge_port_leave(struct net_device *br_dev, if (!br_port) return -ENODEV; - return switchdev_bridge_port_unoffload(br_port->dev, port->dev, + return switchdev_bridge_port_unoffload(br_port->dev, port->dev, port, + &swdev->swdev_nb, + &swdev->swdev_nb_blk, extack); } diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c index 731234a2ace3..517cf5b498af 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c @@ -2360,6 +2360,9 @@ static const struct mlxsw_sp_bridge_ops mlxsw_sp2_bridge_8021ad_ops = { .fid_vid = mlxsw_sp_bridge_8021q_fid_vid, }; +static struct notifier_block mlxsw_sp_switchdev_blocking_notifier; +struct notifier_block mlxsw_sp_switchdev_notifier; + int mlxsw_sp_port_bridge_join(struct mlxsw_sp_port *mlxsw_sp_port, struct net_device *brport_dev, struct net_device *br_dev, @@ -2382,7 +2385,10 @@ int mlxsw_sp_port_bridge_join(struct mlxsw_sp_port *mlxsw_sp_port, if (err) goto err_port_join; - return switchdev_bridge_port_offload(brport_dev, dev, extack); + return switchdev_bridge_port_offload(brport_dev, dev, mlxsw_sp_port, + &mlxsw_sp_switchdev_notifier, + &mlxsw_sp_switchdev_blocking_notifier, + extack); err_port_join: mlxsw_sp_bridge_port_put(mlxsw_sp->bridge, bridge_port); @@ -2396,7 +2402,10 @@ int mlxsw_sp_port_pre_bridge_leave(struct mlxsw_sp_port *mlxsw_sp_port, { struct net_device *dev = mlxsw_sp_port->dev; - return switchdev_bridge_port_unoffload(brport_dev, dev, extack); + return switchdev_bridge_port_unoffload(brport_dev, dev, mlxsw_sp_port, + &mlxsw_sp_switchdev_notifier, + &mlxsw_sp_switchdev_blocking_notifier, + extack); } void mlxsw_sp_port_bridge_leave(struct mlxsw_sp_port *mlxsw_sp_port, diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c b/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c index 270b9fabce91..f8b1deffbfb4 100644 --- a/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c @@ -116,15 +116,22 @@ static int sparx5_port_bridge_join(struct sparx5_port *port, */ __dev_mc_unsync(ndev, sparx5_mc_unsync); - return switchdev_bridge_port_offload(ndev, ndev, extack); + return switchdev_bridge_port_offload(ndev, ndev, NULL, + &sparx5->switchdev_nb, + &sparx5->switchdev_blocking_nb, + extack); } static int sparx5_port_pre_bridge_leave(struct sparx5_port *port, struct netlink_ext_ack *extack) { + struct sparx5 *sparx5 = port->sparx5; struct net_device *ndev = port->ndev; - return switchdev_bridge_port_unoffload(ndev, ndev, extack); + return switchdev_bridge_port_unoffload(ndev, ndev, NULL, + &sparx5->switchdev_nb, + &sparx5->switchdev_blocking_nb, + extack); } static void sparx5_port_bridge_leave(struct sparx5_port *port, diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c index dcb393a35c0e..f24f6a790814 100644 --- a/drivers/net/ethernet/mscc/ocelot_net.c +++ b/drivers/net/ethernet/mscc/ocelot_net.c @@ -1154,38 +1154,19 @@ static int ocelot_switchdev_sync(struct ocelot *ocelot, int port, struct net_device *bridge_dev, struct netlink_ext_ack *extack) { - struct ocelot_port *ocelot_port = ocelot->ports[port]; - struct ocelot_port_private *priv; clock_t ageing_time; u8 stp_state; - int err; - - priv = container_of(ocelot_port, struct ocelot_port_private, port); ocelot_inherit_brport_flags(ocelot, port, brport_dev); stp_state = br_port_get_stp_state(brport_dev); ocelot_bridge_stp_state_set(ocelot, port, stp_state); - err = ocelot_port_vlan_filtering(ocelot, port, - br_vlan_enabled(bridge_dev)); - if (err) - return err; - ageing_time = br_get_ageing_time(bridge_dev); ocelot_port_attr_ageing_set(ocelot, port, ageing_time); - err = br_mdb_replay(bridge_dev, brport_dev, priv, true, - &ocelot_switchdev_blocking_nb, extack); - if (err && err != -EOPNOTSUPP) - return err; - - err = br_vlan_replay(bridge_dev, brport_dev, priv, true, - &ocelot_switchdev_blocking_nb, extack); - if (err && err != -EOPNOTSUPP) - return err; - - return 0; + return ocelot_port_vlan_filtering(ocelot, port, + br_vlan_enabled(bridge_dev)); } static int ocelot_switchdev_unsync(struct ocelot *ocelot, int port) @@ -1216,7 +1197,10 @@ static int ocelot_netdevice_bridge_join(struct net_device *dev, ocelot_port_bridge_join(ocelot, port, bridge); - err = switchdev_bridge_port_offload(brport_dev, dev, extack); + err = switchdev_bridge_port_offload(brport_dev, dev, priv, + &ocelot_netdevice_nb, + &ocelot_switchdev_blocking_nb, + extack); if (err) goto err_switchdev_offload; @@ -1227,7 +1211,10 @@ static int ocelot_netdevice_bridge_join(struct net_device *dev, return 0; err_switchdev_sync: - switchdev_bridge_port_unoffload(brport_dev, dev, extack); + switchdev_bridge_port_unoffload(brport_dev, dev, priv, + &ocelot_netdevice_nb, + &ocelot_switchdev_blocking_nb, + extack); err_switchdev_offload: ocelot_port_bridge_leave(ocelot, port, bridge); return err; @@ -1237,7 +1224,12 @@ static int ocelot_netdevice_pre_bridge_leave(struct net_device *dev, struct net_device *brport_dev, struct netlink_ext_ack *extack) { - return switchdev_bridge_port_unoffload(brport_dev, dev, extack); + struct ocelot_port_private *priv = netdev_priv(dev); + + return switchdev_bridge_port_unoffload(brport_dev, dev, priv, + &ocelot_netdevice_nb, + &ocelot_switchdev_blocking_nb, + extack); } static int ocelot_netdevice_bridge_leave(struct net_device *dev, diff --git a/drivers/net/ethernet/rocker/rocker.h b/drivers/net/ethernet/rocker/rocker.h index d31cee1cdda9..89854ba6f314 100644 --- a/drivers/net/ethernet/rocker/rocker.h +++ b/drivers/net/ethernet/rocker/rocker.h @@ -142,4 +142,7 @@ struct rocker_world_ops { extern struct rocker_world_ops rocker_ofdpa_ops; +extern struct notifier_block rocker_switchdev_notifier; +extern struct notifier_block rocker_switchdev_blocking_notifier; + #endif diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c index 2e3e413406ac..c9f73e944a4f 100644 --- a/drivers/net/ethernet/rocker/rocker_main.c +++ b/drivers/net/ethernet/rocker/rocker_main.c @@ -2861,11 +2861,11 @@ static int rocker_switchdev_blocking_event(struct notifier_block *unused, return NOTIFY_DONE; } -static struct notifier_block rocker_switchdev_notifier = { +struct notifier_block rocker_switchdev_notifier = { .notifier_call = rocker_switchdev_event, }; -static struct notifier_block rocker_switchdev_blocking_notifier = { +struct notifier_block rocker_switchdev_blocking_notifier = { .notifier_call = rocker_switchdev_blocking_event, }; diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c index c32d076bcbf6..e1ffac8f78b8 100644 --- a/drivers/net/ethernet/rocker/rocker_ofdpa.c +++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c @@ -2598,7 +2598,10 @@ static int ofdpa_port_bridge_join(struct ofdpa_port *ofdpa_port, if (err) return err; - return switchdev_bridge_port_offload(dev, dev, extack); + return switchdev_bridge_port_offload(dev, dev, NULL, + &rocker_switchdev_notifier, + &rocker_switchdev_blocking_notifier, + extack); } static int ofdpa_port_pre_bridge_leave(struct ofdpa_port *ofdpa_port, @@ -2606,7 +2609,10 @@ static int ofdpa_port_pre_bridge_leave(struct ofdpa_port *ofdpa_port, { struct net_device *dev = ofdpa_port->dev; - return switchdev_bridge_port_unoffload(dev, dev, extack); + return switchdev_bridge_port_unoffload(dev, dev, NULL, + &rocker_switchdev_notifier, + &rocker_switchdev_blocking_notifier, + extack); } static int ofdpa_port_bridge_leave(struct ofdpa_port *ofdpa_port) diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 30e8b21dc6db..b6f9bc885c52 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -2096,7 +2096,10 @@ static int am65_cpsw_netdevice_port_link(struct net_device *ndev, return -EOPNOTSUPP; } - err = switchdev_bridge_port_offload(ndev, ndev, extack); + err = switchdev_bridge_port_offload(ndev, ndev, NULL, + &am65_cpsw_switchdev_notifier, + &am65_cpsw_switchdev_bl_notifier, + extack); if (err) return err; @@ -2110,7 +2113,10 @@ static int am65_cpsw_netdevice_port_link(struct net_device *ndev, static int am65_cpsw_netdevice_port_pre_unlink(struct net_device *ndev, struct netlink_ext_ack *extack) { - return switchdev_bridge_port_unoffload(ndev, ndev, extack); + return switchdev_bridge_port_unoffload(ndev, ndev, NULL, + &am65_cpsw_switchdev_notifier, + &am65_cpsw_switchdev_bl_notifier, + extack); } static void am65_cpsw_netdevice_port_unlink(struct net_device *ndev) diff --git a/drivers/net/ethernet/ti/am65-cpsw-switchdev.c b/drivers/net/ethernet/ti/am65-cpsw-switchdev.c index 9c29b363e9ae..744d76121d2c 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-switchdev.c +++ b/drivers/net/ethernet/ti/am65-cpsw-switchdev.c @@ -473,7 +473,7 @@ static int am65_cpsw_switchdev_event(struct notifier_block *unused, return NOTIFY_BAD; } -static struct notifier_block cpsw_switchdev_notifier = { +struct notifier_block am65_cpsw_switchdev_notifier = { .notifier_call = am65_cpsw_switchdev_event, }; @@ -506,7 +506,7 @@ static int am65_cpsw_switchdev_blocking_event(struct notifier_block *unused, return NOTIFY_DONE; } -static struct notifier_block cpsw_switchdev_bl_notifier = { +struct notifier_block am65_cpsw_switchdev_bl_notifier = { .notifier_call = am65_cpsw_switchdev_blocking_event, }; @@ -514,18 +514,18 @@ int am65_cpsw_switchdev_register_notifiers(struct am65_cpsw_common *cpsw) { int ret = 0; - ret = register_switchdev_notifier(&cpsw_switchdev_notifier); + ret = register_switchdev_notifier(&am65_cpsw_switchdev_notifier); if (ret) { dev_err(cpsw->dev, "register switchdev notifier fail ret:%d\n", ret); return ret; } - ret = register_switchdev_blocking_notifier(&cpsw_switchdev_bl_notifier); + ret = register_switchdev_blocking_notifier(&am65_cpsw_switchdev_bl_notifier); if (ret) { dev_err(cpsw->dev, "register switchdev blocking notifier ret:%d\n", ret); - unregister_switchdev_notifier(&cpsw_switchdev_notifier); + unregister_switchdev_notifier(&am65_cpsw_switchdev_notifier); } return ret; @@ -533,6 +533,6 @@ int am65_cpsw_switchdev_register_notifiers(struct am65_cpsw_common *cpsw) void am65_cpsw_switchdev_unregister_notifiers(struct am65_cpsw_common *cpsw) { - unregister_switchdev_blocking_notifier(&cpsw_switchdev_bl_notifier); - unregister_switchdev_notifier(&cpsw_switchdev_notifier); + unregister_switchdev_blocking_notifier(&am65_cpsw_switchdev_bl_notifier); + unregister_switchdev_notifier(&am65_cpsw_switchdev_notifier); } diff --git a/drivers/net/ethernet/ti/am65-cpsw-switchdev.h b/drivers/net/ethernet/ti/am65-cpsw-switchdev.h index a67a7606bc80..5f06383f3598 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-switchdev.h +++ b/drivers/net/ethernet/ti/am65-cpsw-switchdev.h @@ -7,6 +7,9 @@ #include <linux/skbuff.h> +extern struct notifier_block am65_cpsw_switchdev_notifier; +extern struct notifier_block am65_cpsw_switchdev_bl_notifier; + #if IS_ENABLED(CONFIG_TI_K3_AM65_CPSW_SWITCHDEV) static inline void am65_cpsw_nuss_set_offload_fwd_mark(struct sk_buff *skb, bool val) { diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c index 8c586d1ff7d7..c8f54d6d7e17 100644 --- a/drivers/net/ethernet/ti/cpsw_new.c +++ b/drivers/net/ethernet/ti/cpsw_new.c @@ -1517,7 +1517,10 @@ static int cpsw_netdevice_port_link(struct net_device *ndev, return -EOPNOTSUPP; } - err = switchdev_bridge_port_offload(ndev, ndev, extack); + err = switchdev_bridge_port_offload(ndev, ndev, NULL, + &cpsw_switchdev_notifier, + &cpsw_switchdev_bl_notifier, + extack); if (err) return err; @@ -1531,7 +1534,10 @@ static int cpsw_netdevice_port_link(struct net_device *ndev, static int cpsw_netdevice_port_pre_unlink(struct net_device *ndev, struct netlink_ext_ack *extack) { - return switchdev_bridge_port_unoffload(ndev, ndev, extack); + return switchdev_bridge_port_unoffload(ndev, ndev, NULL, + &cpsw_switchdev_notifier, + &cpsw_switchdev_bl_notifier, + extack); } static void cpsw_netdevice_port_unlink(struct net_device *ndev) diff --git a/drivers/net/ethernet/ti/cpsw_switchdev.c b/drivers/net/ethernet/ti/cpsw_switchdev.c index f7fb6e17dadd..a6afe4cb71a0 100644 --- a/drivers/net/ethernet/ti/cpsw_switchdev.c +++ b/drivers/net/ethernet/ti/cpsw_switchdev.c @@ -483,7 +483,7 @@ static int cpsw_switchdev_event(struct notifier_block *unused, return NOTIFY_BAD; } -static struct notifier_block cpsw_switchdev_notifier = { +struct notifier_block cpsw_switchdev_notifier = { .notifier_call = cpsw_switchdev_event, }; @@ -516,7 +516,7 @@ static int cpsw_switchdev_blocking_event(struct notifier_block *unused, return NOTIFY_DONE; } -static struct notifier_block cpsw_switchdev_bl_notifier = { +struct notifier_block cpsw_switchdev_bl_notifier = { .notifier_call = cpsw_switchdev_blocking_event, }; diff --git a/drivers/net/ethernet/ti/cpsw_switchdev.h b/drivers/net/ethernet/ti/cpsw_switchdev.h index 04a045dba7d4..7c67d37e17fa 100644 --- a/drivers/net/ethernet/ti/cpsw_switchdev.h +++ b/drivers/net/ethernet/ti/cpsw_switchdev.h @@ -12,4 +12,7 @@ bool cpsw_port_dev_check(const struct net_device *dev); int cpsw_switchdev_register_notifiers(struct cpsw_common *cpsw); void cpsw_switchdev_unregister_notifiers(struct cpsw_common *cpsw); +extern struct notifier_block cpsw_switchdev_notifier; +extern struct notifier_block cpsw_switchdev_bl_notifier; + #endif /* DRIVERS_NET_ETHERNET_TI_CPSW_SWITCHDEV_H_ */ diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h index d0bec83488b9..425a4196f9c6 100644 --- a/include/linux/if_bridge.h +++ b/include/linux/if_bridge.h @@ -70,9 +70,6 @@ bool br_multicast_has_querier_adjacent(struct net_device *dev, int proto); bool br_multicast_has_router_adjacent(struct net_device *dev, int proto); bool br_multicast_enabled(const struct net_device *dev); bool br_multicast_router(const struct net_device *dev); -int br_mdb_replay(struct net_device *br_dev, struct net_device *dev, - const void *ctx, bool adding, struct notifier_block *nb, - struct netlink_ext_ack *extack); #else static inline int br_multicast_list_adjacent(struct net_device *dev, struct list_head *br_ip_list) @@ -104,13 +101,6 @@ static inline bool br_multicast_router(const struct net_device *dev) { return false; } -static inline int br_mdb_replay(const struct net_device *br_dev, - const struct net_device *dev, const void *ctx, - bool adding, struct notifier_block *nb, - struct netlink_ext_ack *extack) -{ - return -EOPNOTSUPP; -} #endif #if IS_ENABLED(CONFIG_BRIDGE) && IS_ENABLED(CONFIG_BRIDGE_VLAN_FILTERING) @@ -120,9 +110,6 @@ int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid); int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto); int br_vlan_get_info(const struct net_device *dev, u16 vid, struct bridge_vlan_info *p_vinfo); -int br_vlan_replay(struct net_device *br_dev, struct net_device *dev, - const void *ctx, bool adding, struct notifier_block *nb, - struct netlink_ext_ack *extack); #else static inline bool br_vlan_enabled(const struct net_device *dev) { @@ -149,14 +136,6 @@ static inline int br_vlan_get_info(const struct net_device *dev, u16 vid, { return -EINVAL; } - -static inline int br_vlan_replay(struct net_device *br_dev, - struct net_device *dev, const void *ctx, - bool adding, struct notifier_block *nb, - struct netlink_ext_ack *extack) -{ - return -EOPNOTSUPP; -} #endif #if IS_ENABLED(CONFIG_BRIDGE) @@ -167,8 +146,6 @@ void br_fdb_clear_offload(const struct net_device *dev, u16 vid); bool br_port_flag_is_set(const struct net_device *dev, unsigned long flag); u8 br_port_get_stp_state(const struct net_device *dev); clock_t br_get_ageing_time(const struct net_device *br_dev); -int br_fdb_replay(const struct net_device *br_dev, const struct net_device *dev, - const void *ctx, bool adding, struct notifier_block *nb); #else static inline struct net_device * br_fdb_find_port(const struct net_device *br_dev, @@ -197,36 +174,39 @@ static inline clock_t br_get_ageing_time(const struct net_device *br_dev) { return 0; } - -static inline int br_fdb_replay(const struct net_device *br_dev, - const struct net_device *dev, const void *ctx, - bool adding, struct notifier_block *nb) -{ - return -EOPNOTSUPP; -} #endif #if IS_ENABLED(CONFIG_BRIDGE) && IS_ENABLED(CONFIG_NET_SWITCHDEV) int switchdev_bridge_port_offload(struct net_device *brport_dev, - struct net_device *dev, + struct net_device *dev, const void *ctx, + struct notifier_block *atomic_nb, + struct notifier_block *blocking_nb, struct netlink_ext_ack *extack); int switchdev_bridge_port_unoffload(struct net_device *brport_dev, - struct net_device *dev, + struct net_device *dev, const void *ctx, + struct notifier_block *atomic_nb, + struct notifier_block *blocking_nb, struct netlink_ext_ack *extack); #else -static inline int switchdev_bridge_port_offload(struct net_device *brport_dev, - struct net_device *dev, - struct netlink_ext_ack *extack) +static inline int +switchdev_bridge_port_offload(struct net_device *brport_dev, + struct net_device *dev, const void *ctx, + struct notifier_block *atomic_nb, + struct notifier_block *blocking_nb, + struct netlink_ext_ack *extack) { return -EINVAL; } -static inline int switchdev_bridge_port_unoffload(struct net_device *brport_dev, - struct net_device *dev, - struct netlink_ext_ack *extack) +static inline int +switchdev_bridge_port_unoffload(struct net_device *brport_dev, + struct net_device *dev, const void *ctx, + struct notifier_block *atomic_nb, + struct notifier_block *blocking_nb, + struct netlink_ext_ack *extack) { return -EINVAL; } diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c index 2b862cffc03a..33c071462799 100644 --- a/net/bridge/br_fdb.c +++ b/net/bridge/br_fdb.c @@ -792,7 +792,6 @@ int br_fdb_replay(const struct net_device *br_dev, const struct net_device *dev, return err; } -EXPORT_SYMBOL_GPL(br_fdb_replay); static void fdb_notify(struct net_bridge *br, const struct net_bridge_fdb_entry *fdb, int type, diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c index 17a720b4473f..364cb1a851fc 100644 --- a/net/bridge/br_mdb.c +++ b/net/bridge/br_mdb.c @@ -686,7 +686,6 @@ int br_mdb_replay(struct net_device *br_dev, struct net_device *dev, return err; } -EXPORT_SYMBOL_GPL(br_mdb_replay); static void br_mdb_switchdev_host_port(struct net_device *dev, struct net_device *lower_dev, diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 46236302eed5..1d3e5957d4d5 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -735,6 +735,8 @@ int br_fdb_external_learn_del(struct net_bridge *br, struct net_bridge_port *p, bool swdev_notify); void br_fdb_offloaded_set(struct net_bridge *br, struct net_bridge_port *p, const unsigned char *addr, u16 vid, bool offloaded); +int br_fdb_replay(const struct net_device *br_dev, const struct net_device *dev, + const void *ctx, bool adding, struct notifier_block *nb); /* br_forward.c */ enum br_pkt_type { @@ -877,6 +879,10 @@ br_multicast_find_group_src(struct net_bridge_port_group *pg, struct br_ip *ip); void br_multicast_del_group_src(struct net_bridge_group_src *src, bool fastleave); +int br_mdb_replay(struct net_device *br_dev, struct net_device *dev, + const void *ctx, bool adding, struct notifier_block *nb, + struct netlink_ext_ack *extack); + static inline bool br_group_is_l2(const struct br_ip *group) { return group->proto == 0; @@ -1135,6 +1141,15 @@ static inline int br_multicast_igmp_type(const struct sk_buff *skb) { return 0; } + +static inline int br_mdb_replay(struct net_device *br_dev, + struct net_device *dev, const void *ctx, + bool adding, struct notifier_block *nb, + struct netlink_ext_ack *extack) +{ + return -EOPNOTSUPP; +} + #endif /* br_vlan.c */ @@ -1185,6 +1200,9 @@ void br_vlan_notify(const struct net_bridge *br, const struct net_bridge_port *p, u16 vid, u16 vid_range, int cmd); +int br_vlan_replay(struct net_device *br_dev, struct net_device *dev, + const void *ctx, bool adding, struct notifier_block *nb, + struct netlink_ext_ack *extack); bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr, const struct net_bridge_vlan *range_end); @@ -1427,6 +1445,14 @@ static inline bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr, { return true; } + +static inline int br_vlan_replay(struct net_device *br_dev, + struct net_device *dev, const void *ctx, + bool adding, struct notifier_block *nb, + struct netlink_ext_ack *extack) +{ + return -EOPNOTSUPP; +} #endif /* br_vlan_options.c */ diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c index 0eb4bc272bff..3252c8e78bb9 100644 --- a/net/bridge/br_switchdev.c +++ b/net/bridge/br_switchdev.c @@ -212,11 +212,74 @@ static void nbp_switchdev_del(struct net_bridge_port *p, nbp_switchdev_hwdom_put(p); } +static int nbp_switchdev_sync_objs(struct net_bridge_port *p, const void *ctx, + struct notifier_block *atomic_nb, + struct notifier_block *blocking_nb, + struct netlink_ext_ack *extack) +{ + struct net_device *br_dev = p->br->dev; + struct net_device *dev = p->dev; + int err; + + err = br_vlan_replay(br_dev, dev, ctx, true, blocking_nb, extack); + if (err && err != -EOPNOTSUPP) + return err; + + err = br_mdb_replay(br_dev, dev, ctx, true, blocking_nb, extack); + if (err && err != -EOPNOTSUPP) + return err; + + /* Forwarding and termination FDB entries on the port */ + err = br_fdb_replay(br_dev, dev, ctx, true, atomic_nb); + if (err && err != -EOPNOTSUPP) + return err; + + /* Termination FDB entries on the bridge itself */ + err = br_fdb_replay(br_dev, br_dev, ctx, true, atomic_nb); + if (err && err != -EOPNOTSUPP) + return err; + + return 0; +} + +static int nbp_switchdev_unsync_objs(struct net_bridge_port *p, + const void *ctx, + struct notifier_block *atomic_nb, + struct notifier_block *blocking_nb, + struct netlink_ext_ack *extack) +{ + struct net_device *br_dev = p->br->dev; + struct net_device *dev = p->dev; + int err; + + err = br_vlan_replay(br_dev, dev, ctx, false, blocking_nb, extack); + if (err && err != -EOPNOTSUPP) + return err; + + err = br_mdb_replay(br_dev, dev, ctx, false, blocking_nb, extack); + if (err && err != -EOPNOTSUPP) + return err; + + /* Forwarding and termination FDB entries on the port */ + err = br_fdb_replay(br_dev, dev, ctx, false, atomic_nb); + if (err && err != -EOPNOTSUPP) + return err; + + /* Termination FDB entries on the bridge itself */ + err = br_fdb_replay(br_dev, br_dev, ctx, false, atomic_nb); + if (err && err != -EOPNOTSUPP) + return err; + + return 0; +} + /* Let the bridge know that this port is offloaded, so that it can assign a * switchdev hardware domain to it. */ int switchdev_bridge_port_offload(struct net_device *brport_dev, - struct net_device *dev, + struct net_device *dev, const void *ctx, + struct notifier_block *atomic_nb, + struct notifier_block *blocking_nb, struct netlink_ext_ack *extack) { struct netdev_phys_item_id ppid; @@ -233,12 +296,27 @@ int switchdev_bridge_port_offload(struct net_device *brport_dev, if (err) return err; - return nbp_switchdev_add(p, ppid, extack); + err = nbp_switchdev_add(p, ppid, extack); + if (err) + return err; + + err = nbp_switchdev_sync_objs(p, ctx, atomic_nb, blocking_nb, extack); + if (err) + goto out_switchdev_del; + + return 0; + +out_switchdev_del: + nbp_switchdev_del(p, ppid); + + return err; } EXPORT_SYMBOL_GPL(switchdev_bridge_port_offload); int switchdev_bridge_port_unoffload(struct net_device *brport_dev, - struct net_device *dev, + struct net_device *dev, const void *ctx, + struct notifier_block *atomic_nb, + struct notifier_block *blocking_nb, struct netlink_ext_ack *extack) { struct netdev_phys_item_id ppid; @@ -255,6 +333,8 @@ int switchdev_bridge_port_unoffload(struct net_device *brport_dev, if (err) return err; + nbp_switchdev_unsync_objs(p, ctx, atomic_nb, blocking_nb, extack); + nbp_switchdev_del(p, ppid); return 0; diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c index a08e9f193009..18f5d0380ee1 100644 --- a/net/bridge/br_vlan.c +++ b/net/bridge/br_vlan.c @@ -1884,7 +1884,6 @@ int br_vlan_replay(struct net_device *br_dev, struct net_device *dev, return err; } -EXPORT_SYMBOL_GPL(br_vlan_replay); /* check if v_curr can enter a range ending in range_end */ bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr, diff --git a/net/dsa/port.c b/net/dsa/port.c index b824b6f8aa84..632c33c63064 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -167,8 +167,8 @@ static void dsa_port_clear_brport_flags(struct dsa_port *dp) } } -static int dsa_port_switchdev_sync(struct dsa_port *dp, - struct netlink_ext_ack *extack) +static int dsa_port_switchdev_sync_attrs(struct dsa_port *dp, + struct netlink_ext_ack *extack) { struct net_device *brport_dev = dsa_port_to_bridge_port(dp); struct net_device *br = dp->bridge_dev; @@ -194,59 +194,6 @@ static int dsa_port_switchdev_sync(struct dsa_port *dp, if (err && err != -EOPNOTSUPP) return err; - err = br_mdb_replay(br, brport_dev, dp, true, - &dsa_slave_switchdev_blocking_notifier, extack); - if (err && err != -EOPNOTSUPP) - return err; - - /* Forwarding and termination FDB entries on the port */ - err = br_fdb_replay(br, brport_dev, dp, true, - &dsa_slave_switchdev_notifier); - if (err && err != -EOPNOTSUPP) - return err; - - /* Termination FDB entries on the bridge itself */ - err = br_fdb_replay(br, br, dp, true, &dsa_slave_switchdev_notifier); - if (err && err != -EOPNOTSUPP) - return err; - - err = br_vlan_replay(br, brport_dev, dp, true, - &dsa_slave_switchdev_blocking_notifier, extack); - if (err && err != -EOPNOTSUPP) - return err; - - return 0; -} - -static int dsa_port_switchdev_unsync_objs(struct dsa_port *dp, - struct net_device *br, - struct netlink_ext_ack *extack) -{ - struct net_device *brport_dev = dsa_port_to_bridge_port(dp); - int err; - - /* Delete the switchdev objects left on this port */ - err = br_mdb_replay(br, brport_dev, dp, false, - &dsa_slave_switchdev_blocking_notifier, extack); - if (err && err != -EOPNOTSUPP) - return err; - - /* Forwarding and termination FDB entries on the port */ - err = br_fdb_replay(br, brport_dev, dp, false, - &dsa_slave_switchdev_notifier); - if (err && err != -EOPNOTSUPP) - return err; - - /* Termination FDB entries on the bridge itself */ - err = br_fdb_replay(br, br, dp, false, &dsa_slave_switchdev_notifier); - if (err && err != -EOPNOTSUPP) - return err; - - err = br_vlan_replay(br, brport_dev, dp, false, - &dsa_slave_switchdev_blocking_notifier, extack); - if (err && err != -EOPNOTSUPP) - return err; - return 0; } @@ -307,18 +254,24 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br, if (err) goto out_rollback; - err = switchdev_bridge_port_offload(brport_dev, dev, extack); + err = switchdev_bridge_port_offload(brport_dev, dev, dp, + &dsa_slave_switchdev_notifier, + &dsa_slave_switchdev_blocking_notifier, + extack); if (err) goto out_rollback_unbridge; - err = dsa_port_switchdev_sync(dp, extack); + err = dsa_port_switchdev_sync_attrs(dp, extack); if (err) goto out_rollback_unoffload; return 0; out_rollback_unoffload: - switchdev_bridge_port_unoffload(brport_dev, dev, extack); + switchdev_bridge_port_unoffload(brport_dev, dev, dp, + &dsa_slave_switchdev_notifier, + &dsa_slave_switchdev_blocking_notifier, + extack); out_rollback_unbridge: dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info); out_rollback: @@ -331,13 +284,11 @@ int dsa_port_pre_bridge_leave(struct dsa_port *dp, struct net_device *br, { struct net_device *brport_dev = dsa_port_to_bridge_port(dp); struct net_device *dev = dp->slave; - int err; - - err = switchdev_bridge_port_unoffload(brport_dev, dev, extack); - if (err) - return err; - return dsa_port_switchdev_unsync_objs(dp, br, extack); + return switchdev_bridge_port_unoffload(brport_dev, dev, dp, + &dsa_slave_switchdev_notifier, + &dsa_slave_switchdev_blocking_notifier, + extack); } void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br) -- 2.25.1
Ido Schimmel
2021-Jul-20 14:01 UTC
[Bridge] [PATCH v5 net-next 00/10] Let switchdev drivers offload and unoffload bridge ports at their own convenience
On Tue, Jul 20, 2021 at 04:46:45PM +0300, Vladimir Oltean wrote:> 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 at nxp.com/This is not what I meant. I specifically suggested to get the TX forwarding offload first and then extending the API with an argument to opt-in for the replay / cleanup: https://lore.kernel.org/netdev/YPbU20%2Fcjkz04s8b at shredder/> > Tobias Waldekranz (2): > net: bridge: disambiguate offload_fwd_mark > net: bridge: switchdev: recycle unused hwdoms > > Vladimir Oltean (8): > net: dpaa2-switch: use extack in dpaa2_switch_port_bridge_join > net: dpaa2-switch: refactor prechangeupper sanity checks > mlxsw: spectrum: refactor prechangeupper sanity checks > mlxsw: spectrum: refactor leaving an 8021q upper that is a bridge port > net: marvell: prestera: refactor prechangeupper sanity checks > net: switchdev: guard drivers against multiple obj replays on same > bridge port > net: bridge: switchdev: let drivers inform which bridge ports are > offloaded > net: bridge: switchdev object replay helpers for everybody > > .../ethernet/freescale/dpaa2/dpaa2-switch.c | 69 +++- > .../ethernet/marvell/prestera/prestera_main.c | 99 +++-- > .../marvell/prestera/prestera_switchdev.c | 42 ++- > .../marvell/prestera/prestera_switchdev.h | 7 +- > .../net/ethernet/mellanox/mlxsw/spectrum.c | 347 ++++++++++++------ > .../net/ethernet/mellanox/mlxsw/spectrum.h | 4 + > .../mellanox/mlxsw/spectrum_switchdev.c | 28 +- > .../microchip/sparx5/sparx5_switchdev.c | 48 ++- > drivers/net/ethernet/mscc/ocelot_net.c | 115 ++++-- > drivers/net/ethernet/rocker/rocker.h | 9 +- > drivers/net/ethernet/rocker/rocker_main.c | 34 +- > drivers/net/ethernet/rocker/rocker_ofdpa.c | 42 ++- > drivers/net/ethernet/ti/am65-cpsw-nuss.c | 34 +- > drivers/net/ethernet/ti/am65-cpsw-switchdev.c | 14 +- > drivers/net/ethernet/ti/am65-cpsw-switchdev.h | 3 + > drivers/net/ethernet/ti/cpsw_new.c | 32 +- > drivers/net/ethernet/ti/cpsw_switchdev.c | 4 +- > drivers/net/ethernet/ti/cpsw_switchdev.h | 3 + > include/linux/if_bridge.h | 60 +-- > net/bridge/br_fdb.c | 1 - > net/bridge/br_if.c | 11 +- > net/bridge/br_mdb.c | 1 - > net/bridge/br_private.h | 61 ++- > net/bridge/br_switchdev.c | 254 +++++++++++-- > net/bridge/br_vlan.c | 1 - > net/dsa/port.c | 83 ++--- > 26 files changed, 1059 insertions(+), 347 deletions(-) > > -- > 2.25.1 >