diff mbox series

[RFC,net-next,v0,2/2] net: dsa: add option for bridge port HW offload

Message ID 20241111013218.1491641-3-aryan.srivastava@alliedtelesis.co.nz (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series Add bridge offloading devlink config | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
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: 3 this patch: 3
netdev/build_tools success Errors and warnings before: 0 (+0) this patch: 0 (+0)
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 3 this patch: 3
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: 19 this patch: 19
netdev/checkpatch warning WARNING: line length of 88 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 1 this patch: 1
netdev/source_inline success Was 0 now: 0

Commit Message

Aryan Srivastava Nov. 11, 2024, 1:32 a.m. UTC 
Currently the DSA framework will HW offload any bridge port if there is
a driver available to support HW offloading. This may not always be the
preferred case.

In cases where the ports on the switch chip are being used as purely L3
interfaces, it is preferred that every packet hits the CPU. In the case
where these ports are added to a bridge, there is a likelihood of
packets completely bypassing the CPU and being switched. This scenario
will occur when a L3 port is added to bridge with an L2/L3 port, and both
ports happen to be attached to the CPU via a switch-chip. This will
result in the switch chip bridging traffic in hardware, where in some
cases it would be preferred that packets entering the L3 port always hit
the CPU, for various userland operations like packet inspection etc.

To prevent this, make the DSA framework aware of the devlink port
function attr, bridge_offload, and add a matching field to the port
struct. Add get/set functions to configure the field, and read the field
when adding a port to a bridge in HW.

Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
---
 include/net/dsa.h |  1 +
 net/dsa/devlink.c | 27 ++++++++++++++++++++++++++-
 net/dsa/dsa.c     |  1 +
 net/dsa/port.c    |  3 ++-
 4 files changed, 30 insertions(+), 2 deletions(-)

Comments

Andrew Lunn Nov. 11, 2024, 3:40 p.m. UTC | #1 
On Mon, Nov 11, 2024 at 02:32:17PM +1300, Aryan Srivastava wrote:
> Currently the DSA framework will HW offload any bridge port if there is
> a driver available to support HW offloading. This may not always be the
> preferred case.
> 
> In cases where the ports on the switch chip are being used as purely L3
> interfaces, it is preferred that every packet hits the CPU. In the case
> where these ports are added to a bridge, there is a likelihood of
> packets completely bypassing the CPU and being switched.

This does not make much sense to me. If it is purely L3, you don't
need a bridge, since that is only needed for L2.

I think we need more details to understand what you really mean here.

	Andrew
diff mbox series

Patch

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 72ae65e7246a..32ed8b36f06b 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -291,6 +291,7 @@  struct dsa_port {
 
 	struct device_node	*dn;
 	unsigned int		ageing_time;
+	bool bridge_offloading;
 
 	struct dsa_bridge	*bridge;
 	struct devlink_port	devlink_port;
diff --git a/net/dsa/devlink.c b/net/dsa/devlink.c
index f41f9fc2194e..dc98e5311921 100644
--- a/net/dsa/devlink.c
+++ b/net/dsa/devlink.c
@@ -298,6 +298,31 @@  void dsa_devlink_region_destroy(struct devlink_region *region)
 }
 EXPORT_SYMBOL_GPL(dsa_devlink_region_destroy);
 
+static void
+dsa_devlink_port_bridge_offload_get(struct devlink_port *dlp,
+				    bool *is_enable)
+{
+	struct dsa_port *dp = dsa_to_port(dsa_devlink_port_to_ds(dlp),
+					  dsa_devlink_port_to_port(dlp));
+
+	*is_enable = dp->bridge_offloading;
+}
+
+static void
+dsa_devlink_port_bridge_offload_set(struct devlink_port *dlp,
+				    bool enable)
+{
+	struct dsa_port *dp = dsa_to_port(dsa_devlink_port_to_ds(dlp),
+					  dsa_devlink_port_to_port(dlp));
+
+	dp->bridge_offloading = enable;
+}
+
+static const struct devlink_port_ops dsa_devlink_port_ops = {
+	.port_fn_bridge_offload_get = dsa_devlink_port_bridge_offload_get,
+	.port_fn_bridge_offload_set = dsa_devlink_port_bridge_offload_set,
+};
+
 int dsa_port_devlink_setup(struct dsa_port *dp)
 {
 	struct devlink_port *dlp = &dp->devlink_port;
@@ -341,7 +366,7 @@  int dsa_port_devlink_setup(struct dsa_port *dp)
 	}
 
 	devlink_port_attrs_set(dlp, &attrs);
-	err = devlink_port_register(dl, dlp, dp->index);
+	err = devlink_port_register_with_ops(dl, dlp, dp->index, &dsa_devlink_port_ops);
 	if (err) {
 		if (ds->ops->port_teardown)
 			ds->ops->port_teardown(ds, dp->index);
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 5a7c0e565a89..20cf55bd42db 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -529,6 +529,7 @@  static int dsa_port_setup(struct dsa_port *dp)
 		return err;
 	}
 
+	dp->bridge_offloading = true;
 	dp->setup = true;
 
 	return 0;
diff --git a/net/dsa/port.c b/net/dsa/port.c
index ee0aaec4c8e0..de5908495287 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -493,7 +493,8 @@  int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
 	struct net_device *brport_dev;
 	int err;
 
-	if (br_mst_enabled(br) && !dsa_port_supports_mst(dp))
+	if ((br_mst_enabled(br) && !dsa_port_supports_mst(dp)) ||
+	    !dp->bridge_offloading)
 		return -EOPNOTSUPP;
 
 	/* Here the interface is already bridged. Reflect the current