From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:36:01
From: Vladimir Oltean <vladimir.oltean@nxp.com>
The objective of this series is to make LAG uppers on top of switchdev
ports work regardless of which order we link interfaces to their masters
(first make the port join the LAG, then the LAG join the bridge, or the
other way around).
There was a design decision to be made in patches 2-4 on whether we
should adopt the "push" model (which attempts to solve the problem
centrally, in the bridge layer) where the driver just calls:
switchdev_bridge_port_offloaded(brport_dev,
&atomic_notifier_block,
&blocking_notifier_block,
extack);
and the bridge just replays the entire collection of switchdev port
attributes and objects that it has, in some predefined order and with
some predefined error handling logic;
or the "pull" model (which attempts to solve the problem by giving the
driver the rope to hang itself), where the driver, apart from calling:
switchdev_bridge_port_offloaded(brport_dev, extack);
has the task of "dumpster diving" (as Tobias puts it) through the bridge
attributes and objects by itself, by calling:
- br_vlan_replay
- br_fdb_replay
- br_mdb_replay
- br_vlan_enabled
- br_port_flag_is_set
- br_port_get_stp_state
- br_multicast_router
- br_get_ageing_time
(not necessarily all of them, and not necessarily in this order, and
with driver-defined error handling).
Even though I'm not in love myself with the "pull" model, I chose it
because there is a fundamental trick with replaying switchdev events
like this:
ip link add br0 type bridge
ip link add bond0 type bond
ip link set bond0 master br0
ip link set swp0 master bond0 <- this will replay the objects once for
the bond0 bridge port, and the swp0
switchdev port will process them
ip link set swp1 master bond0 <- this will replay the objects again for
the bond0 bridge port, and the swp1
switchdev port will see them, but swp0
will see them for the second time now
Basically I believe that it is implementation defined whether the driver
wants to error out on switchdev objects seen twice on a port, and the
bridge should not enforce a certain model for that. For example, for FDB
entries added to a bonding interface, the underling switchdev driver
might have an abstraction for just that: an FDB entry pointing towards a
logical (as opposed to physical) port. So when the second port joins the
bridge, it doesn't realy need to replay FDB entries, since there is
already at least one hardware port which has been receiving those
events, and the FDB entries don't need to be added a second time to the
same logical port.
In the other corner, we have the drivers that handle switchdev port
attributes on a LAG as individual switchdev port attributes on physical
ports (example: VLAN filtering). In fact, the switchdev_handle_port_attr_set
helper facilitates this: it is a fan-out from a single orig_dev towards
multiple lowers that pass the check_cb().
But that's the point: switchdev_handle_port_attr_set is just a helper
which the driver _opts_ to use. The bridge can't enforce the "push"
model, because that would assume that all drivers handle port attributes
in the same way, which is probably false.
For this reason, I preferred to go with the "pull" mode for this patch
set. Just to see how bad it is for other switchdev drivers to copy-paste
this logic, I added the pull support to ocelot too, and I think it's
pretty manageable.
Vladimir Oltean (12):
net: dsa: call dsa_port_bridge_join when joining a LAG that is already
in a bridge
net: dsa: pass extack to dsa_port_{bridge,lag}_join
net: dsa: inherit the actual bridge port flags at join time
net: dsa: sync up with bridge port's STP state when joining
net: dsa: sync up VLAN filtering state when joining the bridge
net: dsa: sync multicast router state when joining the bridge
net: dsa: sync ageing time when joining the bridge
net: dsa: replay port and host-joined mdb entries when joining the
bridge
net: dsa: replay port and local fdb entries when joining the bridge
net: dsa: replay VLANs installed on port when joining the bridge
net: ocelot: call ocelot_netdevice_bridge_join when joining a bridged
LAG
net: ocelot: replay switchdev events when joining bridge
drivers/net/dsa/ocelot/felix.c | 4 +-
drivers/net/ethernet/mscc/ocelot.c | 18 +--
drivers/net/ethernet/mscc/ocelot_net.c | 208 +++++++++++++++++++++----
include/linux/if_bridge.h | 40 +++++
include/net/switchdev.h | 1 +
include/soc/mscc/ocelot.h | 6 +-
net/bridge/br_fdb.c | 52 +++++++
net/bridge/br_mdb.c | 84 ++++++++++
net/bridge/br_stp.c | 27 ++++
net/bridge/br_vlan.c | 71 +++++++++
net/dsa/dsa_priv.h | 9 +-
net/dsa/port.c | 203 ++++++++++++++++++------
net/dsa/slave.c | 11 +-
13 files changed, 631 insertions(+), 103 deletions(-)
--
2.25.1
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:36:01
From: Vladimir Oltean <vladimir.oltean@nxp.com>
DSA can properly detect and offload this sequence of operations:
ip link add br0 type bridge
ip link add bond0 type bond
ip link set swp0 master bond0
ip link set bond0 master br0
But not this one:
ip link add br0 type bridge
ip link add bond0 type bond
ip link set bond0 master br0
ip link set swp0 master bond0
Actually the second one is more complicated, due to the elapsed time
between the enslavement of bond0 and the offloading of it via swp0, a
lot of things could have happened to the bond0 bridge port in terms of
switchdev objects (host MDBs, VLANs, altered STP state etc). So this is
a bit of a can of worms, and making sure that the DSA port's state is in
sync with this already existing bridge port is handled in the next
patches.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v3:
None.
net/dsa/port.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:36:02
From: Vladimir Oltean <vladimir.oltean@nxp.com>
This is a pretty noisy change that was broken out of the larger change
for replaying switchdev attributes and objects at bridge join time,
which is when these extack objects are actually used.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v3:
None.
net/dsa/dsa_priv.h | 6 ++++--
net/dsa/port.c | 8 +++++---
net/dsa/slave.c | 7 +++++--
3 files changed, 14 insertions(+), 7 deletions(-)
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:36:02
From: Vladimir Oltean <vladimir.oltean@nxp.com>
DSA currently assumes that the bridge port starts off with this
constellation of bridge port flags:
- learning on
- unicast flooding on
- multicast flooding on
- broadcast flooding on
just by virtue of code copy-pasta from the bridge layer (new_nbp).
This was a simple enough strategy thus far, because the 'bridge join'
moment always coincided with the 'bridge port creation' moment.
But with sandwiched interfaces, such as:
br0
|
bond0
|
swp0
it may happen that the user has had time to change the bridge port flags
of bond0 before enslaving swp0 to it. In that case, swp0 will falsely
assume that the bridge port flags are those determined by new_nbp, when
in fact this can happen:
ip link add br0 type bridge
ip link add bond0 type bond
ip link set bond0 master br0
ip link set bond0 type bridge_slave learning off
ip link set swp0 master br0
Now swp0 has learning enabled, bond0 has learning disabled. Not nice.
Fix this by "dumpster diving" through the actual bridge port flags with
br_port_flag_is_set, at bridge join time.
We use this opportunity to split dsa_port_change_brport_flags into two
distinct functions called dsa_port_inherit_brport_flags and
dsa_port_clear_brport_flags, now that the implementation for the two
cases is no longer similar.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
Rewrote dsa_port_clear_brport_flags to at least catch errors, and to use
the same "for" loop structure as dsa_port_inherit_brport_flags.
net/dsa/port.c | 125 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 83 insertions(+), 42 deletions(-)
@@ -122,26 +122,82 @@ void dsa_port_disable(struct dsa_port *dp)rtnl_unlock();}-staticvoiddsa_port_change_brport_flags(structdsa_port*dp,-boolbridge_offload)+staticintdsa_port_inherit_brport_flags(structdsa_port*dp,+structnetlink_ext_ack*extack){-structswitchdev_brport_flagsflags;-intflag;+constunsignedlongmask=BR_LEARNING|BR_FLOOD|BR_MCAST_FLOOD|+BR_BCAST_FLOOD;+structnet_device*brport_dev=dsa_port_to_bridge_port(dp);+intflag,err;-flags.mask=BR_LEARNING|BR_FLOOD|BR_MCAST_FLOOD|BR_BCAST_FLOOD;-if(bridge_offload)-flags.val=flags.mask;-else-flags.val=flags.mask&~BR_LEARNING;+for_each_set_bit(flag,&mask,32){+structswitchdev_brport_flagsflags={0};-for_each_set_bit(flag,&flags.mask,32){-structswitchdev_brport_flagstmp;+flags.mask=BIT(flag);-tmp.val=flags.val&BIT(flag);-tmp.mask=BIT(flag);+if(br_port_flag_is_set(brport_dev,BIT(flag)))+flags.val=BIT(flag);-dsa_port_bridge_flags(dp,tmp,NULL);+err=dsa_port_bridge_flags(dp,flags,extack);+if(err&&err!=-EOPNOTSUPP)+returnerr;}++return0;+}++staticvoiddsa_port_clear_brport_flags(structdsa_port*dp,+structnetlink_ext_ack*extack)+{+constunsignedlongval=BR_FLOOD|BR_MCAST_FLOOD|BR_BCAST_FLOOD;+constunsignedlongmask=BR_LEARNING|BR_FLOOD|BR_MCAST_FLOOD|+BR_BCAST_FLOOD;+intflag,err;++for_each_set_bit(flag,&mask,32){+structswitchdev_brport_flagsflags={0};++flags.mask=BIT(flag);+flags.val=val&BIT(flag);++err=dsa_port_bridge_flags(dp,flags,extack);+if(err&&err!=-EOPNOTSUPP)+dev_err(dp->ds->dev,+"failed to clear bridge port flag %d: %d (%pe)\n",+flags.val,err,ERR_PTR(err));+}+}++staticintdsa_port_switchdev_sync(structdsa_port*dp,+structnetlink_ext_ack*extack)+{+interr;++err=dsa_port_inherit_brport_flags(dp,extack);+if(err)+returnerr;++return0;+}++/* Configure the port for standalone mode (no address learning, flood+*everything,BR_STATE_FORWARDING,etc).+*ThebridgeonlyemitsSWITCHDEV_ATTR_ID_PORT_*eventswhentheuser+*requestsitthroughnetlinkorsysfs,butnotautomaticallyatport+*joinorleave,soweneedtohandleresettingthebrportflagsourselves.+*Butweevenpreferitthatway,becauseotherwise,somesetupsmightnever+*getthenotificationtheyneed,forexample,whenaportleavesaLAGthat+*offloadsthebridge,itbecomesstandalone,butasfarasthebridgeis+*concerned,noporteverleft.+*/+staticvoiddsa_port_switchdev_unsync(structdsa_port*dp)+{+dsa_port_clear_brport_flags(dp,NULL);++/* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer,+*soallowittobeinBR_STATE_FORWARDINGtobekeptfunctional+*/+dsa_port_set_state_now(dp,BR_STATE_FORWARDING);}intdsa_port_bridge_join(structdsa_port*dp,structnet_device*br,
@@ -155,24 +211,25 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,};interr;-/* Notify the port driver to set its configurable flags in a way that-*matchestheinitialsettingsofabridgeport.-*/-dsa_port_change_brport_flags(dp,true);-/* Here the interface is already bridged. Reflect the current*configurationsothatdriverscanprogramtheirchipsaccordingly.*/dp->bridge_dev=br;err=dsa_broadcast(DSA_NOTIFIER_BRIDGE_JOIN,&info);+if(err)+gotoout_rollback;-/* The bridging is rolled back on error */-if(err){-dsa_port_change_brport_flags(dp,false);-dp->bridge_dev=NULL;-}+err=dsa_port_switchdev_sync(dp,extack);+if(err)+gotoout_rollback_unbridge;+return0;++out_rollback_unbridge:+dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE,&info);+out_rollback:+dp->bridge_dev=NULL;returnerr;}
@@ -186,6 +243,8 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)};interr;+dsa_port_switchdev_unsync(dp);+/* Here the port is already unbridged. Reflect the current configuration*sothatdriverscanprogramtheirchipsaccordingly.*/
@@ -194,24 +253,6 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)err=dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE,&info);if(err)pr_err("DSA: failed to notify DSA_NOTIFIER_BRIDGE_LEAVE\n");--/* Configure the port for standalone mode (no address learning,-*floodeverything).-*ThebridgeonlyemitsSWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGSevents-*whentheuserrequestsitthroughnetlinkorsysfs,butnot-*automaticallyatportjoinorleave,soweneedtohandleresetting-*thebrportflagsourselves.Butweevenpreferitthatway,because-*otherwise,somesetupsmightnevergetthenotificationtheyneed,-*forexample,whenaportleavesaLAGthatoffloadsthebridge,-*itbecomesstandalone,butasfarasthebridgeisconcerned,no-*porteverleft.-*/-dsa_port_change_brport_flags(dp,false);--/* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer,-*soallowittobeinBR_STATE_FORWARDINGtobekeptfunctional-*/-dsa_port_set_state_now(dp,BR_STATE_FORWARDING);}intdsa_port_lag_change(structdsa_port*dp,
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:37:52
From: Vladimir Oltean <vladimir.oltean@nxp.com>
It may happen that we have the following topology:
ip link add br0 type bridge stp_state 1
ip link add bond0 type bond
ip link set bond0 master br0
ip link set swp0 master bond0
ip link set swp1 master bond0
STP decides that it should put bond0 into the BLOCKING state, and
that's that. The ports that are actively listening for the switchdev
port attributes emitted for the bond0 bridge port (because they are
offloading it) and have the honor of seeing that switchdev port
attribute can react to it, so we can program swp0 and swp1 into the
BLOCKING state.
But if then we do:
ip link set swp2 master bond0
then as far as the bridge is concerned, nothing has changed: it still
has one bridge port. But this new bridge port will not see any STP state
change notification and will remain FORWARDING, which is how the
standalone code leaves it in.
Add a function to the bridge which retrieves the current STP state, such
that drivers can synchronize to it when they may have missed switchdev
events.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v3:
None.
include/linux/if_bridge.h | 6 ++++++
net/bridge/br_stp.c | 14 ++++++++++++++
net/dsa/port.c | 7 +++++++
3 files changed, 27 insertions(+)
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:37:52
From: Vladimir Oltean <vladimir.oltean@nxp.com>
This is the same situation as for other switchdev port attributes: if we
join an already-created bridge port, such as a bond master interface,
then we can miss the initial switchdev notification emitted by the
bridge for this port.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v3:
None.
net/dsa/port.c | 7 +++++++
1 file changed, 7 insertions(+)
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:37:53
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Make sure that the multicast router setting of the bridge is picked up
correctly by DSA when joining, regardless of whether there are
sandwiched interfaces or not. The SWITCHDEV_ATTR_ID_BRIDGE_MROUTER port
attribute is only emitted from br_mc_router_state_change.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v3:
None.
net/dsa/port.c | 10 ++++++++++
1 file changed, 10 insertions(+)
@@ -189,6 +189,10 @@ static int dsa_port_switchdev_sync(struct dsa_port *dp,if(err&&err!=-EOPNOTSUPP)returnerr;+err=dsa_port_mrouter(dp->cpu_dp,br_multicast_router(br),extack);+if(err&&err!=-EOPNOTSUPP)+returnerr;+return0;}
@@ -212,6 +216,12 @@ static void dsa_port_switchdev_unsync(struct dsa_port *dp)dsa_port_set_state_now(dp,BR_STATE_FORWARDING);/* VLAN filtering is handled by dsa_switch_bridge_leave */++/* Some drivers treat the notification for having a local multicast+*routerbyallowingmulticasttobefloodedtotheCPU,soweshould+*allowthisinstandalonemodetoo.+*/+dsa_port_mrouter(dp->cpu_dp,true,NULL);}intdsa_port_bridge_join(structdsa_port*dp,structnet_device*br,
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:37:53
From: Vladimir Oltean <vladimir.oltean@nxp.com>
I have udhcpcd in my system and this is configured to bring interfaces
up as soon as they are created.
I create a bridge as follows:
ip link add br0 type bridge
As soon as I create the bridge and udhcpcd brings it up, I also have
avahi which automatically starts sending IPv6 packets to advertise some
local services, and because of that, the br0 bridge joins the following
IPv6 groups due to the code path detailed below:
33:33:ff:6d:c1:9c vid 0
33:33:00:00:00:6a vid 0
33:33:00:00:00:fb vid 0
br_dev_xmit
-> br_multicast_rcv
-> br_ip6_multicast_add_group
-> __br_multicast_add_group
-> br_multicast_host_join
-> br_mdb_notify
This is all fine, but inside br_mdb_notify we have br_mdb_switchdev_host
hooked up, and switchdev will attempt to offload the host joined groups
to an empty list of ports. Of course nobody offloads them.
Then when we add a port to br0:
ip link set swp0 master br0
the bridge doesn't replay the host-joined MDB entries from br_add_if,
and eventually the host joined addresses expire, and a switchdev
notification for deleting it is emitted, but surprise, the original
addition was already completely missed.
The strategy to address this problem is to replay the MDB entries (both
the port ones and the host joined ones) when the new port joins the
bridge, similar to what vxlan_fdb_replay does (in that case, its FDB can
be populated and only then attached to a bridge that you offload).
However there are 2 possibilities: the addresses can be 'pushed' by the
bridge into the port, or the port can 'pull' them from the bridge.
Considering that in the general case, the new port can be really late to
the party, and there may have been many other switchdev ports that
already received the initial notification, we would like to avoid
delivering duplicate events to them, since they might misbehave. And
currently, the bridge calls the entire switchdev notifier chain, whereas
for replaying it should just call the notifier block of the new guy.
But the bridge doesn't know what is the new guy's notifier block, it
just knows where the switchdev notifier chain is. So for simplification,
we make this a driver-initiated pull for now, and the notifier block is
passed as an argument.
To emulate the calling context for mdb objects (deferred and put on the
blocking notifier chain), we must iterate under RCU protection through
the bridge's mdb entries, queue them, and only call them once we're out
of the RCU read-side critical section.
Suggested-by: Ido Schimmel <redacted>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
- Removed the implication that avahi is crap from the commit message.
- Made the br_mdb_replay shim return -EOPNOTSUPP.
include/linux/if_bridge.h | 9 +++++
net/bridge/br_mdb.c | 84 +++++++++++++++++++++++++++++++++++++++
net/dsa/dsa_priv.h | 2 +
net/dsa/port.c | 6 +++
net/dsa/slave.c | 2 +-
5 files changed, 102 insertions(+), 1 deletion(-)
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:37:53
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Similar to the DSA situation, ocelot supports LAG offload but treats
this scenario improperly:
ip link add br0 type bridge
ip link add bond0 type bond
ip link set bond0 master br0
ip link set swp0 master bond0
We do the same thing as we do there, which is to simulate a 'bridge join'
on 'lag join', if we detect that the bonding upper has a bridge upper.
Again, same as DSA, ocelot supports software fallback for LAG, and in
that case, we should avoid calling ocelot_netdevice_changeupper.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
None.
drivers/net/ethernet/mscc/ocelot_net.c | 111 +++++++++++++++++++------
1 file changed, 86 insertions(+), 25 deletions(-)
@@ -1117,10 +1117,15 @@ static int ocelot_port_obj_del(struct net_device *dev,returnret;}-staticintocelot_netdevice_bridge_join(structocelot*ocelot,intport,-structnet_device*bridge)+staticintocelot_netdevice_bridge_join(structnet_device*dev,+structnet_device*bridge,+structnetlink_ext_ack*extack){+structocelot_port_private*priv=netdev_priv(dev);+structocelot_port*ocelot_port=&priv->port;+structocelot*ocelot=ocelot_port->ocelot;structswitchdev_brport_flagsflags;+intport=priv->chip_port;interr;flags.mask=BR_LEARNING|BR_FLOOD|BR_MCAST_FLOOD|BR_BCAST_FLOOD;
@@ -1135,10 +1140,14 @@ static int ocelot_netdevice_bridge_join(struct ocelot *ocelot, int port,return0;}-staticintocelot_netdevice_bridge_leave(structocelot*ocelot,intport,+staticintocelot_netdevice_bridge_leave(structnet_device*dev,structnet_device*bridge){+structocelot_port_private*priv=netdev_priv(dev);+structocelot_port*ocelot_port=&priv->port;+structocelot*ocelot=ocelot_port->ocelot;structswitchdev_brport_flagsflags;+intport=priv->chip_port;interr;flags.mask=BR_LEARNING|BR_FLOOD|BR_MCAST_FLOOD|BR_BCAST_FLOOD;
@@ -1151,43 +1160,89 @@ static int ocelot_netdevice_bridge_leave(struct ocelot *ocelot, int port,returnerr;}-staticintocelot_netdevice_changeupper(structnet_device*dev,-structnetdev_notifier_changeupper_info*info)+staticintocelot_netdevice_lag_join(structnet_device*dev,+structnet_device*bond,+structnetdev_lag_upper_info*info,+structnetlink_ext_ack*extack){structocelot_port_private*priv=netdev_priv(dev);structocelot_port*ocelot_port=&priv->port;structocelot*ocelot=ocelot_port->ocelot;+structnet_device*bridge_dev;intport=priv->chip_port;+interr;++err=ocelot_port_lag_join(ocelot,port,bond,info);+if(err==-EOPNOTSUPP){+NL_SET_ERR_MSG_MOD(extack,"Offloading not supported");+return0;+}++bridge_dev=netdev_master_upper_dev_get(bond);+if(!bridge_dev||!netif_is_bridge_master(bridge_dev))+return0;++err=ocelot_netdevice_bridge_join(dev,bridge_dev,extack);+if(err)+gotoerr_bridge_join;++return0;++err_bridge_join:+ocelot_port_lag_leave(ocelot,port,bond);+returnerr;+}++staticintocelot_netdevice_lag_leave(structnet_device*dev,+structnet_device*bond)+{+structocelot_port_private*priv=netdev_priv(dev);+structocelot_port*ocelot_port=&priv->port;+structocelot*ocelot=ocelot_port->ocelot;+structnet_device*bridge_dev;+intport=priv->chip_port;++ocelot_port_lag_leave(ocelot,port,bond);++bridge_dev=netdev_master_upper_dev_get(bond);+if(!bridge_dev||!netif_is_bridge_master(bridge_dev))+return0;++returnocelot_netdevice_bridge_leave(dev,bridge_dev);+}++staticintocelot_netdevice_changeupper(structnet_device*dev,+structnetdev_notifier_changeupper_info*info)+{+structnetlink_ext_ack*extack;interr=0;+extack=netdev_notifier_info_to_extack(&info->info);+if(netif_is_bridge_master(info->upper_dev)){-if(info->linking){-err=ocelot_netdevice_bridge_join(ocelot,port,-info->upper_dev);-}else{-err=ocelot_netdevice_bridge_leave(ocelot,port,-info->upper_dev);-}+if(info->linking)+err=ocelot_netdevice_bridge_join(dev,info->upper_dev,+extack);+else+err=ocelot_netdevice_bridge_leave(dev,info->upper_dev);}if(netif_is_lag_master(info->upper_dev)){-if(info->linking){-err=ocelot_port_lag_join(ocelot,port,-info->upper_dev,-info->upper_info);-if(err==-EOPNOTSUPP){-NL_SET_ERR_MSG_MOD(info->info.extack,-"Offloading not supported");-err=0;-}-}else{-ocelot_port_lag_leave(ocelot,port,-info->upper_dev);-}+if(info->linking)+err=ocelot_netdevice_lag_join(dev,info->upper_dev,+info->upper_info,extack);+else+ocelot_netdevice_lag_leave(dev,info->upper_dev);}returnnotifier_from_errno(err);}+/* Treat CHANGEUPPER events on an offloaded LAG as individual CHANGEUPPER+*eventsforthelowerphysicalportsoftheLAG.+*IftheLAGupperisn'toffloaded,ignoreitsCHANGEUPPERevents.+*IncasetheLAGjoinedabridge,notifythatweareoffloadingitandcando+*forwardinginhardwaretowardsit.+*/staticintocelot_netdevice_lag_changeupper(structnet_device*dev,structnetdev_notifier_changeupper_info*info)
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:37:53
From: Vladimir Oltean <vladimir.oltean@nxp.com>
The SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME attribute is only emitted from:
sysfs/ioctl/netlink
-> br_set_ageing_time
-> __set_ageing_time
therefore not at bridge port creation time, so:
(a) drivers had to hardcode the initial value for the address ageing time,
because they didn't get any notification
(b) that hardcoded value can be out of sync, if the user changes the
ageing time before enslaving the port to the bridge
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
None.
include/linux/if_bridge.h | 6 ++++++
net/bridge/br_stp.c | 13 +++++++++++++
net/dsa/port.c | 10 ++++++++++
3 files changed, 29 insertions(+)
@@ -639,6 +639,19 @@ int br_set_ageing_time(struct net_bridge *br, clock_t ageing_time)return0;}+clock_tbr_get_ageing_time(structnet_device*br_dev)+{+structnet_bridge*br;++if(!netif_is_bridge_master(br_dev))+return0;++br=netdev_priv(br_dev);++returnjiffies_to_clock_t(br->ageing_time);+}+EXPORT_SYMBOL_GPL(br_get_ageing_time);+/* called under bridge lock */void__br_set_topology_change(structnet_bridge*br,unsignedcharval){
@@ -173,6 +173,7 @@ static int dsa_port_switchdev_sync(struct dsa_port *dp,{structnet_device*brport_dev=dsa_port_to_bridge_port(dp);structnet_device*br=dp->bridge_dev;+clock_tageing_time;u8stp_state;interr;
@@ -193,6 +194,11 @@ static int dsa_port_switchdev_sync(struct dsa_port *dp,if(err&&err!=-EOPNOTSUPP)returnerr;+ageing_time=br_get_ageing_time(br);+err=dsa_port_ageing_time(dp,ageing_time);+if(err&&err!=-EOPNOTSUPP)+returnerr;+return0;}
@@ -222,6 +228,10 @@ static void dsa_port_switchdev_unsync(struct dsa_port *dp)*allowthisinstandalonemodetoo.*/dsa_port_mrouter(dp->cpu_dp,true,NULL);++/* Ageing time may be global to the switch chip, so don't change it+*herebecausewehavenogoodreason(orvalue)tochangeitto.+*/}intdsa_port_bridge_join(structdsa_port*dp,structnet_device*br,
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:37:53
From: Vladimir Oltean <vladimir.oltean@nxp.com>
When a DSA port joins a LAG that already had an FDB entry pointing to it:
ip link set bond0 master br0
bridge fdb add dev bond0 00:01:02:03:04:05 master static
ip link set swp0 master bond0
the DSA port will have no idea that this FDB entry is there, because it
missed the switchdev event emitted at its creation.
Ido Schimmel pointed this out during a discussion about challenges with
switchdev offloading of stacked interfaces between the physical port and
the bridge, and recommended to just catch that condition and deny the
CHANGEUPPER event:
https://lore.kernel.org/netdev/20210210105949.GB287766@shredder.lan/
But in fact, we might need to deal with the hard thing anyway, which is
to replay all FDB addresses relevant to this port, because it isn't just
static FDB entries, but also local addresses (ones that are not
forwarded but terminated by the bridge). There, we can't just say 'oh
yeah, there was an upper already so I'm not joining that'.
So, similar to the logic for replaying MDB entries, add a function that
must be called by individual switchdev drivers and replays local FDB
entries as well as ones pointing towards a bridge port. This time, we
use the atomic switchdev notifier block, since that's what FDB entries
expect for some reason.
Reported-by: Ido Schimmel <redacted>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
Made the br_fdb_replay shim return -EOPNOTSUPP.
include/linux/if_bridge.h | 9 +++++++
include/net/switchdev.h | 1 +
net/bridge/br_fdb.c | 52 +++++++++++++++++++++++++++++++++++++++
net/dsa/dsa_priv.h | 1 +
net/dsa/port.c | 4 +++
net/dsa/slave.c | 2 +-
6 files changed, 68 insertions(+), 1 deletion(-)
@@ -205,6 +205,7 @@ struct switchdev_notifier_info {structswitchdev_notifier_fdb_info{structswitchdev_notifier_infoinfo;/* must be first */+structlist_headlist;constunsignedchar*addr;u16vid;u8added_by_user:1,
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:37:53
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Currently this simple setup:
ip link add br0 type bridge vlan_filtering 1
ip link add bond0 type bond
ip link set bond0 master br0
ip link set swp0 master bond0
will not work because the bridge has created the PVID in br_add_if ->
nbp_vlan_init, and it has notified switchdev of the existence of VLAN 1,
but that was too early, since swp0 was not yet a lower of bond0, so it
had no reason to act upon that notification.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
Made the br_vlan_replay shim return -EOPNOTSUPP.
include/linux/if_bridge.h | 10 ++++++
net/bridge/br_vlan.c | 71 +++++++++++++++++++++++++++++++++++++++
net/dsa/port.c | 6 ++++
3 files changed, 87 insertions(+)
@@ -1751,6 +1751,77 @@ void br_vlan_notify(const struct net_bridge *br,kfree_skb(skb);}+staticintbr_vlan_replay_one(structnotifier_block*nb,+structnet_device*dev,+structswitchdev_obj_port_vlan*vlan,+structnetlink_ext_ack*extack)+{+structswitchdev_notifier_port_obj_infoobj_info={+.info={+.dev=dev,+.extack=extack,+},+.obj=&vlan->obj,+};+interr;++err=nb->notifier_call(nb,SWITCHDEV_PORT_OBJ_ADD,&obj_info);+returnnotifier_to_errno(err);+}++intbr_vlan_replay(structnet_device*br_dev,structnet_device*dev,+structnotifier_block*nb,structnetlink_ext_ack*extack)+{+structnet_bridge_vlan_group*vg;+structnet_bridge_vlan*v;+structnet_bridge_port*p;+structnet_bridge*br;+interr=0;+u16pvid;++ASSERT_RTNL();++if(!netif_is_bridge_master(br_dev))+return-EINVAL;++if(!netif_is_bridge_master(dev)&&!netif_is_bridge_port(dev))+return-EINVAL;++if(netif_is_bridge_master(dev)){+br=netdev_priv(dev);+vg=br_vlan_group(br);+p=NULL;+}else{+p=br_port_get_rtnl(dev);+if(WARN_ON(!p))+return-EINVAL;+vg=nbp_vlan_group(p);+br=p->br;+}++if(!vg)+return0;++pvid=br_get_pvid(vg);++list_for_each_entry(v,&vg->vlan_list,vlist){+structswitchdev_obj_port_vlanvlan={+.obj.orig_dev=dev,+.obj.id=SWITCHDEV_OBJ_ID_PORT_VLAN,+.flags=br_vlan_flags(v,pvid),+.vid=v->vid,+};++if(!br_vlan_should_use(v))+continue;++br_vlan_replay_one(nb,dev,&vlan,extack);+if(err)+returnerr;+}++returnerr;+}/* check if v_curr can enter a range ending in range_end */boolbr_vlan_can_enter_range(conststructnet_bridge_vlan*v_curr,conststructnet_bridge_vlan*range_end)
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-20 22:37:53
From: Vladimir Oltean <vladimir.oltean@nxp.com>
The premise of this change is that the switchdev port attributes and
objects offloaded by ocelot might have been missed when we are joining
an already existing bridge port, such as a bonding interface.
The patch pulls these switchdev attributes and objects from the bridge,
on behalf of the 'bridge port' net device which might be either the
ocelot switch interface, or the bonding upper interface.
The ocelot_net.c belongs strictly to the switchdev ocelot driver, while
ocelot.c is part of a library shared with the DSA felix driver.
The ocelot_port_bridge_leave function (part of the common library) used
to call ocelot_port_vlan_filtering(false), something which is not
necessary for DSA, since the framework deals with that already there.
So we move this function to ocelot_switchdev_unsync, which is specific
to the switchdev driver.
The code movement described above makes ocelot_port_bridge_leave no
longer return an error code, so we change its type from int to void.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
Added -EOPNOTSUPP to br_mdb_replay and br_vlan_replay, which can be
compiled out.
drivers/net/dsa/ocelot/felix.c | 4 +-
drivers/net/ethernet/mscc/ocelot.c | 18 ++--
drivers/net/ethernet/mscc/ocelot_net.c | 117 +++++++++++++++++++++----
include/soc/mscc/ocelot.h | 6 +-
4 files changed, 111 insertions(+), 34 deletions(-)
@@ -1514,34 +1514,28 @@ int ocelot_port_mdb_del(struct ocelot *ocelot, int port,}EXPORT_SYMBOL(ocelot_port_mdb_del);-intocelot_port_bridge_join(structocelot*ocelot,intport,-structnet_device*bridge)+voidocelot_port_bridge_join(structocelot*ocelot,intport,+structnet_device*bridge){structocelot_port*ocelot_port=ocelot->ports[port];ocelot_port->bridge=bridge;-return0;+ocelot_apply_bridge_fwd_mask(ocelot);}EXPORT_SYMBOL(ocelot_port_bridge_join);-intocelot_port_bridge_leave(structocelot*ocelot,intport,-structnet_device*bridge)+voidocelot_port_bridge_leave(structocelot*ocelot,intport,+structnet_device*bridge){structocelot_port*ocelot_port=ocelot->ports[port];structocelot_vlanpvid={0},native_vlan={0};-intret;ocelot_port->bridge=NULL;-ret=ocelot_port_vlan_filtering(ocelot,port,false);-if(ret)-returnret;-ocelot_port_set_pvid(ocelot,port,pvid);ocelot_port_set_native_vlan(ocelot,port,native_vlan);--return0;+ocelot_apply_bridge_fwd_mask(ocelot);}EXPORT_SYMBOL(ocelot_port_bridge_leave);
@@ -803,10 +803,10 @@ int ocelot_port_pre_bridge_flags(struct ocelot *ocelot, int port,structswitchdev_brport_flagsval);voidocelot_port_bridge_flags(structocelot*ocelot,intport,structswitchdev_brport_flagsval);-intocelot_port_bridge_join(structocelot*ocelot,intport,-structnet_device*bridge);-intocelot_port_bridge_leave(structocelot*ocelot,intport,+voidocelot_port_bridge_join(structocelot*ocelot,intport,structnet_device*bridge);+voidocelot_port_bridge_leave(structocelot*ocelot,intport,+structnet_device*bridge);intocelot_fdb_dump(structocelot*ocelot,intport,dsa_fdb_dump_cb_t*cb,void*data);intocelot_fdb_add(structocelot*ocelot,intport,
From: Vladimir Oltean <vladimir.oltean@nxp.com>
The SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME attribute is only emitted from:
sysfs/ioctl/netlink
-> br_set_ageing_time
-> __set_ageing_time
therefore not at bridge port creation time, so:
(a) drivers had to hardcode the initial value for the address ageing time,
because they didn't get any notification
(b) that hardcoded value can be out of sync, if the user changes the
ageing time before enslaving the port to the bridge
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
From: Nikolay Aleksandrov <hidden> Date: 2021-03-22 16:28:25
On 21/03/2021 00:34, Vladimir Oltean wrote:
From: Vladimir Oltean <vladimir.oltean@nxp.com>
The objective of this series is to make LAG uppers on top of switchdev
ports work regardless of which order we link interfaces to their masters
(first make the port join the LAG, then the LAG join the bridge, or the
other way around).
There was a design decision to be made in patches 2-4 on whether we
should adopt the "push" model (which attempts to solve the problem
centrally, in the bridge layer) where the driver just calls:
switchdev_bridge_port_offloaded(brport_dev,
&atomic_notifier_block,
&blocking_notifier_block,
extack);
and the bridge just replays the entire collection of switchdev port
attributes and objects that it has, in some predefined order and with
some predefined error handling logic;
or the "pull" model (which attempts to solve the problem by giving the
driver the rope to hang itself), where the driver, apart from calling:
switchdev_bridge_port_offloaded(brport_dev, extack);
has the task of "dumpster diving" (as Tobias puts it) through the bridge
attributes and objects by itself, by calling:
- br_vlan_replay
- br_fdb_replay
- br_mdb_replay
- br_vlan_enabled
- br_port_flag_is_set
- br_port_get_stp_state
- br_multicast_router
- br_get_ageing_time
(not necessarily all of them, and not necessarily in this order, and
with driver-defined error handling).
Even though I'm not in love myself with the "pull" model, I chose it
because there is a fundamental trick with replaying switchdev events
like this:
ip link add br0 type bridge
ip link add bond0 type bond
ip link set bond0 master br0
ip link set swp0 master bond0 <- this will replay the objects once for
the bond0 bridge port, and the swp0
switchdev port will process them
ip link set swp1 master bond0 <- this will replay the objects again for
the bond0 bridge port, and the swp1
switchdev port will see them, but swp0
will see them for the second time now
Basically I believe that it is implementation defined whether the driver
wants to error out on switchdev objects seen twice on a port, and the
bridge should not enforce a certain model for that. For example, for FDB
entries added to a bonding interface, the underling switchdev driver
might have an abstraction for just that: an FDB entry pointing towards a
logical (as opposed to physical) port. So when the second port joins the
bridge, it doesn't realy need to replay FDB entries, since there is
already at least one hardware port which has been receiving those
events, and the FDB entries don't need to be added a second time to the
same logical port.
In the other corner, we have the drivers that handle switchdev port
attributes on a LAG as individual switchdev port attributes on physical
ports (example: VLAN filtering). In fact, the switchdev_handle_port_attr_set
helper facilitates this: it is a fan-out from a single orig_dev towards
multiple lowers that pass the check_cb().
But that's the point: switchdev_handle_port_attr_set is just a helper
which the driver _opts_ to use. The bridge can't enforce the "push"
model, because that would assume that all drivers handle port attributes
in the same way, which is probably false.
For this reason, I preferred to go with the "pull" mode for this patch
set. Just to see how bad it is for other switchdev drivers to copy-paste
this logic, I added the pull support to ocelot too, and I think it's
pretty manageable.
Vladimir Oltean (12):
net: dsa: call dsa_port_bridge_join when joining a LAG that is already
in a bridge
net: dsa: pass extack to dsa_port_{bridge,lag}_join
net: dsa: inherit the actual bridge port flags at join time
net: dsa: sync up with bridge port's STP state when joining
net: dsa: sync up VLAN filtering state when joining the bridge
net: dsa: sync multicast router state when joining the bridge
net: dsa: sync ageing time when joining the bridge
net: dsa: replay port and host-joined mdb entries when joining the
bridge
net: dsa: replay port and local fdb entries when joining the bridge
net: dsa: replay VLANs installed on port when joining the bridge
net: ocelot: call ocelot_netdevice_bridge_join when joining a bridged
LAG
net: ocelot: replay switchdev events when joining bridge
drivers/net/dsa/ocelot/felix.c | 4 +-
drivers/net/ethernet/mscc/ocelot.c | 18 +--
drivers/net/ethernet/mscc/ocelot_net.c | 208 +++++++++++++++++++++----
include/linux/if_bridge.h | 40 +++++
include/net/switchdev.h | 1 +
include/soc/mscc/ocelot.h | 6 +-
net/bridge/br_fdb.c | 52 +++++++
net/bridge/br_mdb.c | 84 ++++++++++
net/bridge/br_stp.c | 27 ++++
net/bridge/br_vlan.c | 71 +++++++++
net/dsa/dsa_priv.h | 9 +-
net/dsa/port.c | 203 ++++++++++++++++++------
net/dsa/slave.c | 11 +-
13 files changed, 631 insertions(+), 103 deletions(-)
Hi Vladimir,
Please pull all of the new bridge code into separate patches with the proper
bridge subsystems tagged in the subject.
I'll review the bridge changes in a minute.
Thanks,
Nik
From: Nikolay Aleksandrov <hidden> Date: 2021-03-22 16:36:28
On 21/03/2021 00:34, Vladimir Oltean wrote:
quoted hunk
From: Vladimir Oltean <vladimir.oltean@nxp.com>
I have udhcpcd in my system and this is configured to bring interfaces
up as soon as they are created.
I create a bridge as follows:
ip link add br0 type bridge
As soon as I create the bridge and udhcpcd brings it up, I also have
avahi which automatically starts sending IPv6 packets to advertise some
local services, and because of that, the br0 bridge joins the following
IPv6 groups due to the code path detailed below:
33:33:ff:6d:c1:9c vid 0
33:33:00:00:00:6a vid 0
33:33:00:00:00:fb vid 0
br_dev_xmit
-> br_multicast_rcv
-> br_ip6_multicast_add_group
-> __br_multicast_add_group
-> br_multicast_host_join
-> br_mdb_notify
This is all fine, but inside br_mdb_notify we have br_mdb_switchdev_host
hooked up, and switchdev will attempt to offload the host joined groups
to an empty list of ports. Of course nobody offloads them.
Then when we add a port to br0:
ip link set swp0 master br0
the bridge doesn't replay the host-joined MDB entries from br_add_if,
and eventually the host joined addresses expire, and a switchdev
notification for deleting it is emitted, but surprise, the original
addition was already completely missed.
The strategy to address this problem is to replay the MDB entries (both
the port ones and the host joined ones) when the new port joins the
bridge, similar to what vxlan_fdb_replay does (in that case, its FDB can
be populated and only then attached to a bridge that you offload).
However there are 2 possibilities: the addresses can be 'pushed' by the
bridge into the port, or the port can 'pull' them from the bridge.
Considering that in the general case, the new port can be really late to
the party, and there may have been many other switchdev ports that
already received the initial notification, we would like to avoid
delivering duplicate events to them, since they might misbehave. And
currently, the bridge calls the entire switchdev notifier chain, whereas
for replaying it should just call the notifier block of the new guy.
But the bridge doesn't know what is the new guy's notifier block, it
just knows where the switchdev notifier chain is. So for simplification,
we make this a driver-initiated pull for now, and the notifier block is
passed as an argument.
To emulate the calling context for mdb objects (deferred and put on the
blocking notifier chain), we must iterate under RCU protection through
the bridge's mdb entries, queue them, and only call them once we're out
of the RCU read-side critical section.
Suggested-by: Ido Schimmel <redacted>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
- Removed the implication that avahi is crap from the commit message.
- Made the br_mdb_replay shim return -EOPNOTSUPP.
include/linux/if_bridge.h | 9 +++++
net/bridge/br_mdb.c | 84 +++++++++++++++++++++++++++++++++++++++
net/dsa/dsa_priv.h | 2 +
net/dsa/port.c | 6 +++
net/dsa/slave.c | 2 +-
5 files changed, 102 insertions(+), 1 deletion(-)
You cannot walk over these lists without the multicast lock or RCU. RTNL is not
enough because of various timers and leave messages that can alter both the mdb_list
and the port group lists. I'd prefer RCU to avoid blocking the bridge mcast.
From: Nikolay Aleksandrov <hidden> Date: 2021-03-22 16:40:40
On 21/03/2021 00:34, Vladimir Oltean wrote:
quoted hunk
From: Vladimir Oltean <vladimir.oltean@nxp.com>
When a DSA port joins a LAG that already had an FDB entry pointing to it:
ip link set bond0 master br0
bridge fdb add dev bond0 00:01:02:03:04:05 master static
ip link set swp0 master bond0
the DSA port will have no idea that this FDB entry is there, because it
missed the switchdev event emitted at its creation.
Ido Schimmel pointed this out during a discussion about challenges with
switchdev offloading of stacked interfaces between the physical port and
the bridge, and recommended to just catch that condition and deny the
CHANGEUPPER event:
https://lore.kernel.org/netdev/20210210105949.GB287766@shredder.lan/
But in fact, we might need to deal with the hard thing anyway, which is
to replay all FDB addresses relevant to this port, because it isn't just
static FDB entries, but also local addresses (ones that are not
forwarded but terminated by the bridge). There, we can't just say 'oh
yeah, there was an upper already so I'm not joining that'.
So, similar to the logic for replaying MDB entries, add a function that
must be called by individual switchdev drivers and replays local FDB
entries as well as ones pointing towards a bridge port. This time, we
use the atomic switchdev notifier block, since that's what FDB entries
expect for some reason.
Reported-by: Ido Schimmel <redacted>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
Made the br_fdb_replay shim return -EOPNOTSUPP.
include/linux/if_bridge.h | 9 +++++++
include/net/switchdev.h | 1 +
net/bridge/br_fdb.c | 52 +++++++++++++++++++++++++++++++++++++++
net/dsa/dsa_priv.h | 1 +
net/dsa/port.c | 4 +++
net/dsa/slave.c | 2 +-
6 files changed, 68 insertions(+), 1 deletion(-)
@@ -205,6 +205,7 @@ struct switchdev_notifier_info {structswitchdev_notifier_fdb_info{structswitchdev_notifier_infoinfo;/* must be first */+structlist_headlist;constunsignedchar*addr;u16vid;u8added_by_user:1,
From: Nikolay Aleksandrov <hidden> Date: 2021-03-22 16:49:31
On 21/03/2021 00:34, Vladimir Oltean wrote:
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Currently this simple setup:
ip link add br0 type bridge vlan_filtering 1
ip link add bond0 type bond
ip link set bond0 master br0
ip link set swp0 master bond0
will not work because the bridge has created the PVID in br_add_if ->
nbp_vlan_init, and it has notified switchdev of the existence of VLAN 1,
but that was too early, since swp0 was not yet a lower of bond0, so it
had no reason to act upon that notification.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
Made the br_vlan_replay shim return -EOPNOTSUPP.
include/linux/if_bridge.h | 10 ++++++
net/bridge/br_vlan.c | 71 +++++++++++++++++++++++++++++++++++++++
net/dsa/port.c | 6 ++++
3 files changed, 87 insertions(+)
/* 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,
const struct net_bridge_vlan *range_end)
You cannot walk over these lists without the multicast lock or RCU. RTNL is not
enough because of various timers and leave messages that can alter both the mdb_list
and the port group lists. I'd prefer RCU to avoid blocking the bridge mcast.
The trouble is that I need to emulate the calling context that is
provided to SWITCHDEV_OBJ_ID_HOST_MDB and SWITCHDEV_OBJ_ID_PORT_MDB, and
that means blocking context.
So if I hold rcu_read_lock(), I need to queue up the mdb entries, and
notify the driver only after I leave the RCU critical section. The
memory footprint may temporarily blow up.
In fact this is what I did in v1:
https://patchwork.kernel.org/project/netdevbpf/patch/20210224114350.2791260-15-olteanv@gmail.com/
I just figured I could get away with rtnl_mutex protection, but it looks
like I can't. So I guess you prefer my v1?
You cannot walk over these lists without the multicast lock or RCU. RTNL is not
enough because of various timers and leave messages that can alter both the mdb_list
and the port group lists. I'd prefer RCU to avoid blocking the bridge mcast.
The trouble is that I need to emulate the calling context that is
provided to SWITCHDEV_OBJ_ID_HOST_MDB and SWITCHDEV_OBJ_ID_PORT_MDB, and
that means blocking context.
So if I hold rcu_read_lock(), I need to queue up the mdb entries, and
notify the driver only after I leave the RCU critical section. The
memory footprint may temporarily blow up.
In fact this is what I did in v1:
https://patchwork.kernel.org/project/netdevbpf/patch/20210224114350.2791260-15-olteanv@gmail.com/
I just figured I could get away with rtnl_mutex protection, but it looks
like I can't. So I guess you prefer my v1?
Indeed, if you need a blocking context then you'd have to go with v1.