diff mbox series

[net-next,02/17] net: switchdev: Add a helper to replay objects on a bridge port

Message ID 3145c726aa2290b82841a51590554e97f3c099b6.1689763088.git.petrm@nvidia.com (mailing list archive)
State Accepted
Commit f2e2857b352277a451e2f91409e461fa7ebf2d15
Delegated to: Netdev Maintainers
Headers show
Series mlxsw: Permit enslavement to netdevices with uppers | expand

Checks

Context Check Description
netdev/series_format fail Series longer than 15 patches (and no cover letter)
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1390 this patch: 1390
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 1372 this patch: 1372
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1428 this patch: 1428
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 100 lines checked
netdev/kdoc success Errors and warnings before: 3 this patch: 3
netdev/source_inline success Was 0 now: 0

Commit Message

Petr Machata July 19, 2023, 11:01 a.m. UTC 
When a front panel joins a bridge via another netdevice (typically a LAG),
the driver needs to learn about the objects configured on the bridge port.
When the bridge port is offloaded by the driver for the first time, this
can be achieved by passing a notifier to switchdev_bridge_port_offload().
The notifier is then invoked for the individual objects (such as VLANs)
configured on the bridge, and can look for the interesting ones.

Calling switchdev_bridge_port_offload() when the second port joins the
bridge lower is unnecessary, but the replay is still needed. To that end,
add a new function, switchdev_bridge_port_replay(), which does only the
replay part of the _offload() function in exactly the same way as that
function.

Cc: Jiri Pirko <jiri@resnulli.us>
Cc: Ivan Vecera <ivecera@redhat.com>
Cc: Roopa Prabhu <roopa@nvidia.com>
Cc: Nikolay Aleksandrov <razor@blackwall.org>
Cc: bridge@lists.linux-foundation.org
Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Danielle Ratson <danieller@nvidia.com>
---
 include/net/switchdev.h   |  6 ++++++
 net/bridge/br.c           |  8 ++++++++
 net/bridge/br_private.h   | 16 ++++++++++++++++
 net/bridge/br_switchdev.c |  9 +++++++++
 net/switchdev/switchdev.c | 25 +++++++++++++++++++++++++
 5 files changed, 64 insertions(+)

Comments

Vladimir Oltean Jan. 31, 2024, 3:47 p.m. UTC | #1 
Hi Petr,

On Wed, Jul 19, 2023 at 01:01:17PM +0200, Petr Machata wrote:
> When a front panel joins a bridge via another netdevice (typically a LAG),
> the driver needs to learn about the objects configured on the bridge port.
> When the bridge port is offloaded by the driver for the first time, this
> can be achieved by passing a notifier to switchdev_bridge_port_offload().
> The notifier is then invoked for the individual objects (such as VLANs)
> configured on the bridge, and can look for the interesting ones.
> 
> Calling switchdev_bridge_port_offload() when the second port joins the
> bridge lower is unnecessary, but the replay is still needed. To that end,
> add a new function, switchdev_bridge_port_replay(), which does only the
> replay part of the _offload() function in exactly the same way as that
> function.
> 
> Cc: Jiri Pirko <jiri@resnulli.us>
> Cc: Ivan Vecera <ivecera@redhat.com>
> Cc: Roopa Prabhu <roopa@nvidia.com>
> Cc: Nikolay Aleksandrov <razor@blackwall.org>
> Cc: bridge@lists.linux-foundation.org
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> Reviewed-by: Danielle Ratson <danieller@nvidia.com>
> ---

I just noticed this commit in the kernel, and the commit message did not
really convince me.

As unnecessary as the second switchdev_bridge_port_offload() call may be,
it does not hurt either, because it just bumps p->offload_count in the bridge.
And with just this justification, adding a new function to the switchdev API
is equally unnecessary.

Even worse, switchdev_bridge_port_replay() is a partial misnomer, and
will become a complete misnomer soon. Out of 3 object classes (FDB, MDB
and VLAN), FDB and VLAN are not actually replayed just for the given
bridge port given as argument, but for all ports of the bridge. And Tobias
wants to change things so that the same will happen for MDB too.
https://lore.kernel.org/netdev/87bk927bxl.fsf@waldekranz.com/

Can mlxsw not use the same replay code path as DSA, through the _offload()
and _unoffload() entry points?
diff mbox series

Patch

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index ca0312b78294..4d324e2a2eef 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -231,6 +231,7 @@  enum switchdev_notifier_type {
 
 	SWITCHDEV_BRPORT_OFFLOADED,
 	SWITCHDEV_BRPORT_UNOFFLOADED,
+	SWITCHDEV_BRPORT_REPLAY,
 };
 
 struct switchdev_notifier_info {
@@ -299,6 +300,11 @@  void switchdev_bridge_port_unoffload(struct net_device *brport_dev,
 				     const void *ctx,
 				     struct notifier_block *atomic_nb,
 				     struct notifier_block *blocking_nb);
+int switchdev_bridge_port_replay(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);
 
 void switchdev_deferred_process(void);
 int switchdev_port_attr_set(struct net_device *dev,
diff --git a/net/bridge/br.c b/net/bridge/br.c
index 4f5098d33a46..a6e94ceb7c9a 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -234,6 +234,14 @@  static int br_switchdev_blocking_event(struct notifier_block *nb,
 		br_switchdev_port_unoffload(p, b->ctx, b->atomic_nb,
 					    b->blocking_nb);
 		break;
+	case SWITCHDEV_BRPORT_REPLAY:
+		brport_info = ptr;
+		b = &brport_info->brport;
+
+		err = br_switchdev_port_replay(p, b->dev, b->ctx, b->atomic_nb,
+					       b->blocking_nb, extack);
+		err = notifier_from_errno(err);
+		break;
 	}
 
 out:
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index a63b32c1638e..a69774131535 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -2115,6 +2115,12 @@  void br_switchdev_port_unoffload(struct net_bridge_port *p, const void *ctx,
 				 struct notifier_block *atomic_nb,
 				 struct notifier_block *blocking_nb);
 
+int br_switchdev_port_replay(struct net_bridge_port *p,
+			     struct net_device *dev, const void *ctx,
+			     struct notifier_block *atomic_nb,
+			     struct notifier_block *blocking_nb,
+			     struct netlink_ext_ack *extack);
+
 bool br_switchdev_frame_uses_tx_fwd_offload(struct sk_buff *skb);
 
 void br_switchdev_frame_set_offload_fwd_mark(struct sk_buff *skb);
@@ -2165,6 +2171,16 @@  br_switchdev_port_unoffload(struct net_bridge_port *p, const void *ctx,
 {
 }
 
+static inline int
+br_switchdev_port_replay(struct net_bridge_port *p,
+			 struct net_device *dev, const void *ctx,
+			 struct notifier_block *atomic_nb,
+			 struct notifier_block *blocking_nb,
+			 struct netlink_ext_ack *extack)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline bool br_switchdev_frame_uses_tx_fwd_offload(struct sk_buff *skb)
 {
 	return false;
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index e92e0338afee..ee84e783e1df 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -829,3 +829,12 @@  void br_switchdev_port_unoffload(struct net_bridge_port *p, const void *ctx,
 
 	nbp_switchdev_del(p);
 }
+
+int br_switchdev_port_replay(struct net_bridge_port *p,
+			     struct net_device *dev, const void *ctx,
+			     struct notifier_block *atomic_nb,
+			     struct notifier_block *blocking_nb,
+			     struct netlink_ext_ack *extack)
+{
+	return nbp_switchdev_sync_objs(p, ctx, atomic_nb, blocking_nb, extack);
+}
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 8cc42aea19c7..5b045284849e 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -862,3 +862,28 @@  void switchdev_bridge_port_unoffload(struct net_device *brport_dev,
 					  NULL);
 }
 EXPORT_SYMBOL_GPL(switchdev_bridge_port_unoffload);
+
+int switchdev_bridge_port_replay(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)
+{
+	struct switchdev_notifier_brport_info brport_info = {
+		.brport = {
+			.dev = dev,
+			.ctx = ctx,
+			.atomic_nb = atomic_nb,
+			.blocking_nb = blocking_nb,
+		},
+	};
+	int err;
+
+	ASSERT_RTNL();
+
+	err = call_switchdev_blocking_notifiers(SWITCHDEV_BRPORT_REPLAY,
+						brport_dev, &brport_info.info,
+						extack);
+	return notifier_to_errno(err);
+}
+EXPORT_SYMBOL_GPL(switchdev_bridge_port_replay);