net: bridge: Notify about bridge VLANs
Subsystems (7)
v2 cover net: bridge: Notify about bridge VLANs
In commit 946a11e7408e ("mlxsw: spectrum_span: Allow bridge for gretap mirror"), mlxsw got support for offloading mirror-to-gretap such that the underlay packet path involves a bridge. In that case, the offload is also influenced by PVID setting of said bridge. However, changes to VLAN configuration of the bridge itself do not generate switchdev notifications, so there's no mechanism to prod mlxsw to update the offload when these settings change.
In this patchset, the problem is resolved by distributing the switchdev notification SWITCHDEV_OBJ_ID_PORT_VLAN also for configuration changes on bridge VLANs. Since stacked devices distribute the notification to lower devices, such event eventually reaches the driver, which can determine whether it's a bridge or port VLAN by inspecting orig_dev.
To keep things consistent, the newly-distributed notifications observe the same protocol as the existing ones: dual prepare/commit, with -EOPNOTSUPP indicating lack of support, even though there's currently nothing to prepare for and nothing to support. Correspondingly, all switchdev drivers have been updated to return -EOPNOTSUPP for bridge VLAN notifications.
In patch #1, the code to send notifications for adding and deleting is factored out into two named functions.
In patches #2-#5, respectively for mlxsw, rocker, DSA and DPAA2 ethsw, the new notifications (which are not enabled yet) are ignored to maintain the current behavior.
In patch #6, the notification is actually enabled.
In patch #7, mlxsw is changed to update offloads of mirror-to-gre also for bridge-related notifications.
Changes from v1 to v2:
- Rename br_switchdev_port_obj_add() and br_switchdev_port_obj_del() to br_switchdev_port_vlan_add() and br_switchdev_port_vlan_del(), and move from br_vlan.c to br_switchdev.c.
Petr Machata (7):
net: bridge: Extract boilerplate around switchdev_port_obj_*()
mlxsw: spectrum_switchdev: Ignore bridge VLAN events
rocker: rocker_main: Ignore bridge VLAN events
dsa: port: Ignore bridge VLAN events
staging: fsl-dpaa2: ethsw: Ignore bridge VLAN events
net: bridge: Notify about bridge VLANs
mlxsw: spectrum_switchdev: Schedule respin during trans prepare
.../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 8 +++- drivers/net/ethernet/rocker/rocker_main.c | 6 +++ drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 6 +++ net/bridge/br_private.h | 3 ++ net/bridge/br_switchdev.c | 25 ++++++++++++ net/bridge/br_vlan.c | 45 +++++++++------------- net/dsa/port.c | 6 +++ 7 files changed, 71 insertions(+), 28 deletions(-)
Signature · 1 line
2.4.11
PATCH v2 1/7 net: bridge: Extract boilerplate around switchdev_port_obj_*()
A call to switchdev_port_obj_add() or switchdev_port_obj_del() involves initializing a struct switchdev_obj_port_vlan, a piece of code that repeats on each call site almost verbatim. While in the current codebase there is just one duplicated add call, the follow-up patches add more of both add and del calls.
Thus to remove the duplication, extract the repetition into named functions and reuse.
--- net/bridge/br_private.h | 3 +++ net/bridge/br_switchdev.c | 25 +++++++++++++++++++++++++ net/bridge/br_vlan.c | 31 ++++--------------------------- 3 files changed, 32 insertions(+), 27 deletions(-)
net/bridge/br_private.h +3 -0
@@ -1178,6 +1178,9 @@ static inline void br_switchdev_frame_unmark(struct sk_buff *skb) }#endif /* CONFIG_NET_SWITCHDEV */ +int br_switchdev_port_vlan_add (struct net_device *dev,u16 vid,u16 flags); +int br_switchdev_port_vlan_del (struct net_device *dev,u16 vid); +/* br_arp_nd_proxy.c */ void br_recalculate_neigh_suppress_enabled (struct net_bridge *br); void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
net/bridge/br_switchdev.c +25 -0
@@ -136,3 +136,28 @@ br_switchdev_fdb_notify(const struct net_bridge_fdb_entry *fdb, int type)break ; } } + +int br_switchdev_port_vlan_add (struct net_device *dev,u16 vid,u16 flags) +{ +struct switchdev_obj_port_vlan v = { + .obj.orig_dev = dev, + .obj.id =SWITCHDEV_OBJ_ID_PORT_VLAN , + .flags = flags, + .vid_begin = vid, + .vid_end = vid, + }; + +return switchdev_port_obj_add (dev, &v.obj); +} + +int br_switchdev_port_vlan_del (struct net_device *dev,u16 vid) +{ +struct switchdev_obj_port_vlan v = { + .obj.orig_dev = dev, + .obj.id =SWITCHDEV_OBJ_ID_PORT_VLAN , + .vid_begin = vid, + .vid_end = vid, + }; + +return switchdev_port_obj_del (dev, &v.obj); +}
net/bridge/br_vlan.c +4 -27
@@ -82,19 +82,10 @@ static bool __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)static int __vlan_vid_add (struct net_device *dev,struct net_bridge *br,u16 vid,u16 flags) { -struct switchdev_obj_port_vlan v = { - .obj.orig_dev = dev, - .obj.id =SWITCHDEV_OBJ_ID_PORT_VLAN , - .flags = flags, - .vid_begin = vid, - .vid_end = vid, - }; -int err; -/* Try switchdev op first. In case it is not supported, fallback to * 8021q add. */ - err =switchdev_port_obj_add (dev, &v.obj); +int err =br_switchdev_port_vlan_add (dev, vid, flags);if (err == -EOPNOTSUPP )return vlan_vid_add (dev, br->vlan_proto, vid);return err;
@@ -130,18 +121,11 @@ static void __vlan_del_list(struct net_bridge_vlan *v)static int __vlan_vid_del (struct net_device *dev,struct net_bridge *br,u16 vid) { -struct switchdev_obj_port_vlan v = { - .obj.orig_dev = dev, - .obj.id =SWITCHDEV_OBJ_ID_PORT_VLAN , - .vid_begin = vid, - .vid_end = vid, - }; -int err; -/* Try switchdev op first. In case it is not supported, fallback to * 8021q del. */ - err =switchdev_port_obj_del (dev, &v.obj); +int err =br_switchdev_port_vlan_del (dev, vid); +if (err == -EOPNOTSUPP ) {vlan_vid_del (dev, br->vlan_proto, vid);return 0 ;
@@ -1053,13 +1037,6 @@ int nbp_vlan_init(struct net_bridge_port *p)int nbp_vlan_add (struct net_bridge_port *port,u16 vid,u16 flags,bool *changed) { -struct switchdev_obj_port_vlan v = { - .obj.orig_dev = port->dev, - .obj.id =SWITCHDEV_OBJ_ID_PORT_VLAN , - .flags = flags, - .vid_begin = vid, - .vid_end = vid, - };struct net_bridge_vlan *vlan;int ret;
@@ -1069,7 +1046,7 @@ int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags, vlan =br_vlan_find (nbp_vlan_group (port), vid);if (vlan) {/* Pass the flags to the hardware bridge */ - ret =switchdev_port_obj_add (port->dev, &v.obj); + ret =br_switchdev_port_vlan_add (port->dev, vid, flags);if (ret && ret != -EOPNOTSUPP )return ret; *changed =__vlan_add_flags (vlan, flags);
Signature · 1 line
2.4.11
Re: [PATCH net-next v2 1/7] net: bridge: Extract boilerplate around switchdev_port_obj_*()
net/bridge/br_private.h static inline void br_switchdev_frame_unmark(struct sk_buff *skb) · line 1183
Quoted 11 lines · Petr Machata
net/bridge/br_private.h +3
-0@@ -1178,6 +1178,9 @@ static inline void br_switchdev_frame_unmark(struct sk_buff *skb) }#endif /* CONFIG_NET_SWITCHDEV */ +int br_switchdev_port_vlan_add (struct net_device *dev,u16 vid,u16 flags); +int br_switchdev_port_vlan_del (struct net_device *dev,u16 vid); +
Nope, this actually does need to be in the ifdef block. The whole br_switchdev.c is built conditionally.
I'll send v3.
Petr
PATCH v2 2/7 mlxsw: spectrum_switchdev: Ignore bridge VLAN events
In reply to Petr MachataIgnore VLAN events where the orig_dev is the bridge device itself.
--- drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +6 -0
@@ -1144,6 +1144,9 @@ static int mlxsw_sp_port_vlans_add(struct mlxsw_sp_port *mlxsw_sp_port,struct mlxsw_sp_bridge_port *bridge_port;u16 vid; +if (netif_is_bridge_master (orig_dev)) +return -EOPNOTSUPP ; +if (switchdev_trans_ph_prepare (trans))return 0 ;
@@ -1741,6 +1744,9 @@ static int mlxsw_sp_port_vlans_del(struct mlxsw_sp_port *mlxsw_sp_port,struct mlxsw_sp_bridge_port *bridge_port;u16 vid; +if (netif_is_bridge_master (orig_dev)) +return -EOPNOTSUPP ; + bridge_port =mlxsw_sp_bridge_port_find (mlxsw_sp->bridge, orig_dev);if (WARN_ON (!bridge_port))return -EINVAL ;
Signature · 1 line
2.4.11
PATCH v2 3/7 rocker: rocker_main: Ignore bridge VLAN events
In reply to Petr MachataIgnore VLAN events where the orig_dev is the bridge device itself.
--- drivers/net/ethernet/rocker/rocker_main.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/net/ethernet/rocker/rocker_main.c +6 -0
@@ -1632,6 +1632,9 @@ rocker_world_port_obj_vlan_add(struct rocker_port *rocker_port, {struct rocker_world_ops *wops = rocker_port->rocker->wops; +if (netif_is_bridge_master (vlan->obj.orig_dev)) +return -EOPNOTSUPP ; +if (!wops->port_obj_vlan_add)return -EOPNOTSUPP ;
@@ -1647,6 +1650,9 @@ rocker_world_port_obj_vlan_del(struct rocker_port *rocker_port, {struct rocker_world_ops *wops = rocker_port->rocker->wops; +if (netif_is_bridge_master (vlan->obj.orig_dev)) +return -EOPNOTSUPP ; +if (!wops->port_obj_vlan_del)return -EOPNOTSUPP ;return wops->port_obj_vlan_del (rocker_port, vlan);
Signature · 1 line
2.4.11
Re: [PATCH net-next v2 3/7] rocker: rocker_main: Ignore bridge VLAN events
Hi Petr,
Quoted 12 lines · Petr Machata
Ignore VLAN events where the
orig_devis the bridge device itself.Signed-off-by Petr Machata <[email protected]>--- drivers/net/ethernet/rocker/rocker_main.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/net/ethernet/rocker/rocker_main.c +0
-0
drivers/net/ethernet/rocker/rocker_main.c rocker_world_port_obj_vlan_add(struct rocker_port *rocker_port, · line 1637
Quoted 6 lines
drivers/net/ethernet/rocker/rocker_main.c +3
-0@@ -1632,6 +1632,9 @@ rocker_world_port_obj_vlan_add(struct rocker_port *rocker_port, {struct rocker_world_ops *wops = rocker_port->rocker->wops; +if (netif_is_bridge_master (vlan->obj.orig_dev)) +return -EOPNOTSUPP ; +
What will happen to the "bridge vlan add dev br0 vid X pvid untagged self" when the lower level (the driver) returns -EOPNOTSUPP? Will it avoid adding a vlan on the bridge ?
Quoted 16 lines
drivers/net/ethernet/rocker/rocker_main.c +3
-0if (!wops->port_obj_vlan_add)return -EOPNOTSUPP ;@@ -1647,6 +1650,9 @@ rocker_world_port_obj_vlan_del(struct rocker_port *rocker_port, {struct rocker_world_ops *wops = rocker_port->rocker->wops; +if (netif_is_bridge_master (vlan->obj.orig_dev)) +return -EOPNOTSUPP ; +if (!wops->port_obj_vlan_del)return -EOPNOTSUPP ;return wops->port_obj_vlan_del (rocker_port, vlan);Signature · 1 line
2.4.11
Re: [PATCH net-next v2 3/7] rocker: rocker_main: Ignore bridge VLAN events
Quoted 8 lines · Ilias Apalodimas
Quoted 8 lines
drivers/net/ethernet/rocker/rocker_main.c +0
-0@@ -1632,6 +1632,9 @@ rocker_world_port_obj_vlan_add(struct rocker_port *rocker_port, {struct rocker_world_ops *wops = rocker_port->rocker->wops;
Quoted 6 lines
drivers/net/ethernet/rocker/rocker_main.c rocker_world_port_obj_vlan_add(struct rocker_port *rocker_port, · line 1637
Quoted 3 lines
drivers/net/ethernet/rocker/rocker_main.c +3
-0+if (netif_is_bridge_master (vlan->obj.orig_dev)) +return -EOPNOTSUPP ; +What will happen to the "bridge vlan add dev br0 vid X pvid untagged self" when the lower level (the driver) returns
-EOPNOTSUPP? Will it avoid adding a vlan on the bridge ?
No, it will still do it. The reasons are:
- that's what currently happens anyway: none of the drivers has any support, yet the bridge logic is done
- -EOPNOTSUPP is what switchdev_port_obj_*() return if switchdev is not compiled in
In order to suppress the setting, return e.g. -EINVAL:
# bridge vlan add dev br vid 111 self RTNETLINK answers: Invalid argument # bridge vlan show dev br port vlan ids br 1 PVID Egress Untagged
Thanks,
Petr
Re: [PATCH net-next v2 3/7] rocker: rocker_main: Ignore bridge VLAN events
Quoted 27 lines · Petr Machata
Quoted 8 lines · Ilias Apalodimas
Quoted 8 lines
drivers/net/ethernet/rocker/rocker_main.c +0
-0@@ -1632,6 +1632,9 @@ rocker_world_port_obj_vlan_add(struct rocker_port *rocker_port, {struct rocker_world_ops *wops = rocker_port->rocker->wops;Quoted 6 lines
drivers/net/ethernet/rocker/rocker_main.c rocker_world_port_obj_vlan_add(struct rocker_port *rocker_port, · line 1637
Quoted 3 lines
drivers/net/ethernet/rocker/rocker_main.c +3
-0+if (netif_is_bridge_master (vlan->obj.orig_dev)) +return -EOPNOTSUPP ; +What will happen to the "bridge vlan add dev br0 vid X pvid untagged self" when the lower level (the driver) returns
-EOPNOTSUPP? Will it avoid adding a vlan on the bridge ?No, it will still do it. The reasons are:
- that's what currently happens anyway: none of the drivers has any support, yet the bridge logic is done
-
-EOPNOTSUPPis what switchdev_port_obj_*() return if switchdev is not compiled inIn order to suppress the setting, return e.g.
-EINVAL:
Quoted 8 lines
# bridge vlan add dev br vid 111 self RTNETLINK answers: Invalid argument # bridge vlan show dev br port vlan ids br 1 PVID Egress UntaggedThanks,
Petr
Ok that's perfect then. As i mentioned it's really useful for a use case i am doing on a switch that needs it's cpu port configured individually.
Thanks,
Ilias
PATCH v2 4/7 dsa: port: Ignore bridge VLAN events
In reply to Petr MachataIgnore VLAN events where the orig_dev is the bridge device itself.
--- net/dsa/port.c | 6 ++++++ 1 file changed, 6 insertions(+)
net/dsa/port.c +6 -0
@@ -252,6 +252,9 @@ int dsa_port_vlan_add(struct dsa_port *dp, .vlan = vlan, }; +if (netif_is_bridge_master (vlan->obj.orig_dev)) +return -EOPNOTSUPP ; +if (br_vlan_enabled (dp->bridge_dev))return dsa_port_notify (dp,DSA_NOTIFIER_VLAN_ADD , &info);
@@ -267,6 +270,9 @@ int dsa_port_vlan_del(struct dsa_port *dp, .vlan = vlan, }; +if (netif_is_bridge_master (vlan->obj.orig_dev)) +return -EOPNOTSUPP ; +if (br_vlan_enabled (dp->bridge_dev))return dsa_port_notify (dp,DSA_NOTIFIER_VLAN_DEL , &info);
Signature · 1 line
2.4.11
PATCH v2 5/7 staging: fsl-dpaa2: ethsw: Ignore bridge VLAN events
In reply to Petr MachataIgnore VLAN events where the orig_dev is the bridge device itself.
--- drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/staging/fsl-dpaa2/ethsw/ethsw.c +6 -0
@@ -719,6 +719,9 @@ static int port_vlans_add(struct net_device *netdev,struct ethsw_port_priv *port_priv =netdev_priv (netdev);int vid, err; +if (netif_is_bridge_master (vlan->obj.orig_dev)) +return -EOPNOTSUPP ; +if (switchdev_trans_ph_prepare (trans))return 0 ;
@@ -873,6 +876,9 @@ static int port_vlans_del(struct net_device *netdev,struct ethsw_port_priv *port_priv =netdev_priv (netdev);int vid, err; +if (netif_is_bridge_master (vlan->obj.orig_dev)) +return -EOPNOTSUPP ; +for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) { err =ethsw_port_del_vlan (port_priv, vid);if (err)
Signature · 1 line
2.4.11
PATCH v2 6/7 net: bridge: Notify about bridge VLANs
In reply to Petr MachataA driver might need to react to changes in settings of brentry VLANs. Therefore send switchdev port notifications for these as well. Reuse SWITCHDEV_OBJ_ID_PORT_VLAN for this purpose. Listeners should use netif_is_bridge_master() on orig_dev to determine whether the notification is about a bridge port or a bridge.
--- net/bridge/br_vlan.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
net/bridge/br_vlan.c +14 -0
@@ -243,6 +243,10 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags) goto out_filt; v->brvlan = masterv; v->stats = masterv->stats; + }else { + err =br_switchdev_port_vlan_add (dev, v->vid, flags); +if (err && err != -EOPNOTSUPP ) + goto out; }/* Add the dev mac and count the vlan only if it's usable */
@@ -278,6 +282,8 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)br_vlan_put_master (masterv); v->brvlan =NULL ; } + }else { +br_switchdev_port_vlan_del (dev, v->vid); } goto out;
@@ -303,6 +309,11 @@ static int __vlan_del(struct net_bridge_vlan *v) err =__vlan_vid_del (p->dev, p->br, v->vid);if (err) goto out; + }else { + err =br_switchdev_port_vlan_del (v->br->dev, v->vid); +if (err && err != -EOPNOTSUPP ) + goto out; + err =0 ; }if (br_vlan_should_use (v)) {
@@ -580,6 +591,9 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags, bool *changed) vg->num_vlans++; *changed = true; } + ret =br_switchdev_port_vlan_add (br->dev, vid, flags); +if (ret && ret != -EOPNOTSUPP ) +return ret;if (__vlan_add_flags (vlan, flags)) *changed = true;
Signature · 1 line
2.4.11
PATCH v2 7/7 mlxsw: spectrum_switchdev: Schedule respin during trans prepare
In reply to Petr MachataSince there's no special support for the bridge events, the driver returns -EOPNOTSUPP, and thus the commit never happens. Therefore schedule respin during the prepare stage: there's no real difference one way or another.
This fixes the problem that mirror-to-gretap offload wouldn't adapt to changes in bridge vlan configuration right away and another notification would have to arrive for mlxsw to catch up.
--- drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +1 -1
@@ -1697,7 +1697,7 @@ static int mlxsw_sp_port_obj_add(struct net_device *dev, vlan =SWITCHDEV_OBJ_PORT_VLAN (obj); err =mlxsw_sp_port_vlans_add (mlxsw_sp_port, vlan, trans); -if (switchdev_trans_ph_commit (trans)) { +if (switchdev_trans_ph_prepare (trans)) {/* The event is emitted before the changes are actually * applied to the bridge. Therefore schedule the respin * call for later, so that the respin logic sees the
Signature · 1 line
2.4.11