[PATCH net-next v2 2/2] net: dsa: mxl862xx: implement bridge offloading

Daniel Golle posted 2 patches 1 month ago
There is a newer version of this series
[PATCH net-next v2 2/2] net: dsa: mxl862xx: implement bridge offloading
Posted by Daniel Golle 1 month ago
Add support for bridge offloading on the mxl862xx DSA driver.

Implement joining and leaving bridges as well as add, delete and dump
operations on isolated FDBs and setting a port's STP state.

The switch supports a maximum of 63 bridges, however, up to 12 may
be used as "single-port bridges" to isolate standalone ports.
Allowing up to 48 bridges to be offloaded seems more than enough on
that hardware, hence that is set as max_num_bridges.

A total of 128 bridge ports are supported in the bridge portmap, and
virtual bridge ports have to be used eg. for link-aggregation, hence
potentially exceeding the number of hardware ports.

As there are now more users of the BRIDGEPORT_CONFIG_SET API and the
state of each port is cached locally, introduce a helper function
mxl862xx_set_bridge_port(struct dsa_switch *ds, int port) which is
then also be used to replace the direct calls to the API in
mxl862xx_setup_cpu_bridge() and mxl862xx_add_single_port_bridge().

Note that there is no convenient way to control flooding on per-port
level, so the driver is using a 0-rate QoS meter setup as a stopper in
lack of any better option. This works, but allows a single 64-byte
packet to pass once after reset. While this limitation doesn't seem to
be a problem in practice, it has the effect that the
bridge_vlan_unaware.sh selftest only passes the FDB test the 2nd time
the test is run after boot (and any subsequent time after that, of
course).

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2:
 * fix kernel-doc comments in API header
 * use bitfield helpers for compound tci field in fdb API
 * add missing endian conversion for mxl862xx_stp_port_cfg.port_state
   as well as mxl862xx_mac_table_read.tci (spotted by AI review)
 * drop manually resetting port learning state on bridge<->standalone
   transitions, DSA framework takes care of that
 * don't abort updating bridge ports on error, return error at the end
 * report error in mxl862xx_port_bridge_leave()
 * create mxl862xx_get_fid() helper and use it in
   mxl862xx_port_fdb_add() and mxl862xx_port_fdb_del()
 * propagete error of callback function in mxl862xx_port_fdb_dump()
 * manually mxl862xx_port_fast_age() in mxl862xx_port_stp_state_set()
   to avoid FDB poisoning due to race condition

 drivers/net/dsa/mxl862xx/mxl862xx-api.h | 178 ++++++++
 drivers/net/dsa/mxl862xx/mxl862xx-cmd.h |  19 +-
 drivers/net/dsa/mxl862xx/mxl862xx.c     | 579 ++++++++++++++++++++++--
 drivers/net/dsa/mxl862xx/mxl862xx.h     |  21 +
 4 files changed, 764 insertions(+), 33 deletions(-)

diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-api.h b/drivers/net/dsa/mxl862xx/mxl862xx-api.h
index a9f599dbca25b..6d700c16eaa16 100644
--- a/drivers/net/dsa/mxl862xx/mxl862xx-api.h
+++ b/drivers/net/dsa/mxl862xx/mxl862xx-api.h
@@ -3,6 +3,7 @@
 #ifndef __MXL862XX_API_H
 #define __MXL862XX_API_H
 
+#include <linux/bits.h>
 #include <linux/if_ether.h>
 
 /**
@@ -34,6 +35,123 @@ struct mxl862xx_register_mod {
 	__le16 mask;
 } __packed;
 
+/**
+ * enum mxl862xx_mac_table_filter - Source/Destination MAC address filtering
+ *
+ * @MXL862XX_MAC_FILTER_NONE: no filter
+ * @MXL862XX_MAC_FILTER_SRC: source address filter
+ * @MXL862XX_MAC_FILTER_DEST: destination address filter
+ * @MXL862XX_MAC_FILTER_BOTH: both source and destination filter
+ */
+enum mxl862xx_mac_table_filter {
+	MXL862XX_MAC_FILTER_NONE = 0,
+	MXL862XX_MAC_FILTER_SRC = BIT(0),
+	MXL862XX_MAC_FILTER_DEST = BIT(1),
+	MXL862XX_MAC_FILTER_BOTH = BIT(0) | BIT(1),
+};
+
+#define MXL862XX_TCI_VLAN_ID		GENMASK(11, 0)
+#define MXL862XX_TCI_VLAN_CFI_DEI	BIT(12)
+#define MXL862XX_TCI_VLAN_PRI		GENMASK(15, 13)
+
+/**
+ * struct mxl862xx_mac_table_add - MAC Table Entry to be added
+ * @fid: Filtering Identifier (FID) (not supported by all switches)
+ * @port_id: Ethernet Port number
+ * @port_map: Bridge Port Map
+ * @sub_if_id: Sub-Interface Identifier Destination
+ * @age_timer: Aging Time in seconds
+ * @vlan_id: STAG VLAN Id
+ * @static_entry: Static Entry (value will be aged out if not set to static)
+ * @traffic_class: Egress queue traffic class
+ * @mac: MAC Address to add to the table
+ * @filter_flag: See &enum mxl862xx_mac_table_filter
+ * @igmp_controlled: Packet is marked as IGMP controlled if destination MAC
+ *                   address matche MAC in this entry
+ * @associated_mac: Associated Mac address
+ * @tci: TCI for B-Step
+ *	Bit [0:11] - VLAN ID
+ *	Bit [12] - VLAN CFI/DEI
+ *	Bit [13:15] - VLAN PRI
+ */
+struct mxl862xx_mac_table_add {
+	__le16 fid;
+	__le32 port_id;
+	__le16 port_map[8];
+	__le16 sub_if_id;
+	int age_timer;
+	__le16 vlan_id;
+	u8 static_entry;
+	u8 traffic_class;
+	u8 mac[ETH_ALEN];
+	u8 filter_flag;
+	u8 igmp_controlled;
+	u8 associated_mac[ETH_ALEN];
+	__le16 tci;
+} __packed;
+
+/**
+ * struct mxl862xx_mac_table_remove - MAC Table Entry to be removed
+ * @fid: Filtering Identifier (FID)
+ * @mac: MAC Address to be removed from the table.
+ * @filter_flag: See &enum mxl862xx_mac_table_filter
+ * @tci: TCI for B-Step
+ *	Bit [0:11] - VLAN ID
+ *	Bit [12] - VLAN CFI/DEI
+ *	Bit [13:15] - VLAN PRI
+ */
+struct mxl862xx_mac_table_remove {
+	__le16 fid;
+	u8 mac[ETH_ALEN];
+	u8 filter_flag;
+	__le16 tci;
+} __packed;
+
+/**
+ * struct mxl862xx_mac_table_read - MAC Table Entry to be read
+ * @initial: Restart the get operation from the beginning of the table
+ * @last: Indicates that the read operation returned last entry
+ * @fid: Get the MAC table entry belonging to the given Filtering Identifier
+ * @port_id: The Bridge Port ID
+ * @port_map: Bridge Port Map
+ * @age_timer: Aging Time
+ * @vlan_id: STAG VLAN Id
+ * @static_entry: Indicates if this is a Static Entry
+ * @sub_if_id: Sub-Interface Identifier Destination
+ * @mac: MAC Address. Filled out by the switch API implementation.
+ * @filter_flag: See &enum mxl862xx_mac_table_filter
+ * @igmp_controlled: Packet is marked as IGMP controlled if destination MAC
+ *                   address matches the MAC in this entry
+ * @entry_changed: Indicate if the Entry has Changed
+ * @associated_mac: Associated MAC address
+ * @hit_status: MAC Table Hit Status Update
+ * @tci: TCI for B-Step
+ *	Bit [0:11] - VLAN ID
+ *	Bit [12] - VLAN CFI/DEI
+ *	Bit [13:15] - VLAN PRI
+ * @first_bridge_port_id: The port this MAC address has first been learned.
+ *                        This is used for loop detection.
+ */
+struct mxl862xx_mac_table_read {
+	u8 initial;
+	u8 last;
+	__le16 fid;
+	__le32 port_id;
+	__le16 port_map[8];
+	int age_timer;
+	__le16 vlan_id;
+	u8 static_entry;
+	__le16 sub_if_id;
+	u8 mac[ETH_ALEN];
+	u8 filter_flag;
+	u8 igmp_controlled;
+	u8 entry_changed;
+	u8 associated_mac[ETH_ALEN];
+	u8 hit_status;
+	__le16 tci;
+	__le16 first_bridge_port_id;
+} __packed;
+
 /**
  * enum mxl862xx_mac_clear_type - MAC table clear type
  * @MXL862XX_MAC_CLEAR_PHY_PORT: clear dynamic entries based on port_id
@@ -138,6 +256,40 @@ enum mxl862xx_bridge_port_egress_meter {
 	MXL862XX_BRIDGE_PORT_EGRESS_METER_MAX,
 };
 
+/**
+ * struct mxl862xx_qos_meter_cfg - Rate meter configuration
+ * @enable: Enable/disable meter
+ * @meter_id: Meter ID (assigned by firmware on alloc)
+ * @meter_name: Meter name string
+ * @meter_type: Meter algorithm type (srTCM = 0, trTCM = 1)
+ * @cbs: Committed Burst Size (in bytes)
+ * @res1: Reserved
+ * @ebs: Excess Burst Size (in bytes)
+ * @res2: Reserved
+ * @rate: Committed Information Rate (in kbit/s)
+ * @pi_rate: Peak Information Rate (in kbit/s)
+ * @colour_blind_mode: Colour-blind mode enable
+ * @pkt_mode: Packet mode enable
+ * @local_overhd: Local overhead accounting enable
+ * @local_overhd_val: Local overhead accounting value
+ */
+struct mxl862xx_qos_meter_cfg {
+	u8 enable;
+	__le16 meter_id;
+	char meter_name[32];
+	__le32 meter_type;
+	__le32 cbs;
+	__le32 res1;
+	__le32 ebs;
+	__le32 res2;
+	__le32 rate;
+	__le32 pi_rate;
+	u8 colour_blind_mode;
+	u8 pkt_mode;
+	u8 local_overhd;
+	__le16 local_overhd_val;
+} __packed;
+
 /**
  * enum mxl862xx_bridge_forward_mode - Bridge forwarding type of packet
  * @MXL862XX_BRIDGE_FORWARD_FLOOD: Packet is flooded to port members of
@@ -658,6 +810,32 @@ struct mxl862xx_ctp_port_assignment {
 	__le16 bridge_port_id;
 } __packed;
 
+/**
+ * enum mxl862xx_stp_port_state - Spanning Tree Protocol port states
+ * @MXL862XX_STP_PORT_STATE_FORWARD: Forwarding state
+ * @MXL862XX_STP_PORT_STATE_DISABLE: Disabled/Discarding state
+ * @MXL862XX_STP_PORT_STATE_LEARNING: Learning state
+ * @MXL862XX_STP_PORT_STATE_BLOCKING: Blocking/Listening
+ */
+enum mxl862xx_stp_port_state {
+	MXL862XX_STP_PORT_STATE_FORWARD = 0,
+	MXL862XX_STP_PORT_STATE_DISABLE,
+	MXL862XX_STP_PORT_STATE_LEARNING,
+	MXL862XX_STP_PORT_STATE_BLOCKING,
+};
+
+/**
+ * struct mxl862xx_stp_port_cfg - Configures the Spanning Tree Protocol state of an Ethernet port
+ * @port_id: Port number
+ * @fid: Filtering Identifier (FID)
+ * @port_state: See &enum mxl862xx_stp_port_state
+ */
+struct mxl862xx_stp_port_cfg {
+	__le16 port_id;
+	__le16 fid;
+	__le32 port_state; /* enum mxl862xx_stp_port_state */
+} __packed;
+
 /**
  * struct mxl862xx_sys_fw_image_version - Firmware version information
  * @iv_major: firmware major version
diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-cmd.h b/drivers/net/dsa/mxl862xx/mxl862xx-cmd.h
index f6852ade64e7c..43d04c019b50c 100644
--- a/drivers/net/dsa/mxl862xx/mxl862xx-cmd.h
+++ b/drivers/net/dsa/mxl862xx/mxl862xx-cmd.h
@@ -15,12 +15,15 @@
 #define MXL862XX_BRDG_MAGIC		0x300
 #define MXL862XX_BRDGPORT_MAGIC		0x400
 #define MXL862XX_CTP_MAGIC		0x500
+#define MXL862XX_QOS_MAGIC		0x600
 #define MXL862XX_SWMAC_MAGIC		0xa00
+#define MXL862XX_STP_MAGIC		0xf00
 #define MXL862XX_SS_MAGIC		0x1600
 #define GPY_GPY2XX_MAGIC		0x1800
 #define SYS_MISC_MAGIC			0x1900
 
 #define MXL862XX_COMMON_CFGGET		(MXL862XX_COMMON_MAGIC + 0x9)
+#define MXL862XX_COMMON_CFGSET		(MXL862XX_COMMON_MAGIC + 0xa)
 #define MXL862XX_COMMON_REGISTERMOD	(MXL862XX_COMMON_MAGIC + 0x11)
 
 #define MXL862XX_BRIDGE_ALLOC		(MXL862XX_BRDG_MAGIC + 0x1)
@@ -35,14 +38,22 @@
 
 #define MXL862XX_CTP_PORTASSIGNMENTSET	(MXL862XX_CTP_MAGIC + 0x3)
 
+#define MXL862XX_QOS_METERCFGSET	(MXL862XX_QOS_MAGIC + 0x2)
+#define MXL862XX_QOS_METERALLOC		(MXL862XX_QOS_MAGIC + 0x2a)
+
+#define MXL862XX_MAC_TABLEENTRYADD	(MXL862XX_SWMAC_MAGIC + 0x2)
+#define MXL862XX_MAC_TABLEENTRYREAD	(MXL862XX_SWMAC_MAGIC + 0x3)
+#define MXL862XX_MAC_TABLEENTRYREMOVE	(MXL862XX_SWMAC_MAGIC + 0x5)
 #define MXL862XX_MAC_TABLECLEARCOND	(MXL862XX_SWMAC_MAGIC + 0x8)
 
-#define MXL862XX_SS_SPTAG_SET		(MXL862XX_SS_MAGIC + 0x02)
+#define MXL862XX_SS_SPTAG_SET		(MXL862XX_SS_MAGIC + 0x2)
+
+#define MXL862XX_STP_PORTCFGSET		(MXL862XX_STP_MAGIC + 0x2)
 
-#define INT_GPHY_READ			(GPY_GPY2XX_MAGIC + 0x01)
-#define INT_GPHY_WRITE			(GPY_GPY2XX_MAGIC + 0x02)
+#define INT_GPHY_READ			(GPY_GPY2XX_MAGIC + 0x1)
+#define INT_GPHY_WRITE			(GPY_GPY2XX_MAGIC + 0x2)
 
-#define SYS_MISC_FW_VERSION		(SYS_MISC_MAGIC + 0x02)
+#define SYS_MISC_FW_VERSION		(SYS_MISC_MAGIC + 0x2)
 
 #define MMD_API_MAXIMUM_ID		0x7fff
 
diff --git a/drivers/net/dsa/mxl862xx/mxl862xx.c b/drivers/net/dsa/mxl862xx/mxl862xx.c
index 8281c749967af..278a9132a9c56 100644
--- a/drivers/net/dsa/mxl862xx/mxl862xx.c
+++ b/drivers/net/dsa/mxl862xx/mxl862xx.c
@@ -7,8 +7,10 @@
  * Copyright (C) 2025 Daniel Golle <daniel@makrotopia.org>
  */
 
-#include <linux/module.h>
+#include <linux/bitfield.h>
 #include <linux/delay.h>
+#include <linux/if_bridge.h>
+#include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/of_mdio.h>
 #include <linux/phy.h>
@@ -36,6 +38,13 @@
 #define MXL862XX_READY_TIMEOUT_MS	10000
 #define MXL862XX_READY_POLL_MS		100
 
+static const int mxl862xx_flood_meters[] = {
+	MXL862XX_BRIDGE_PORT_EGRESS_METER_UNKNOWN_UC,
+	MXL862XX_BRIDGE_PORT_EGRESS_METER_UNKNOWN_MC_IP,
+	MXL862XX_BRIDGE_PORT_EGRESS_METER_UNKNOWN_MC_NON_IP,
+	MXL862XX_BRIDGE_PORT_EGRESS_METER_BROADCAST,
+};
+
 static enum dsa_tag_protocol mxl862xx_get_tag_protocol(struct dsa_switch *ds,
 						       int port,
 						       enum dsa_tag_protocol m)
@@ -169,6 +178,66 @@ static int mxl862xx_setup_mdio(struct dsa_switch *ds)
 	return ret;
 }
 
+static int mxl862xx_bridge_config_fwd(struct dsa_switch *ds, u16 bridge_id,
+				      bool ucast_flood, bool mcast_flood,
+				      bool bcast_flood)
+{
+	struct mxl862xx_bridge_config bridge_config = { };
+	struct mxl862xx_priv *priv = ds->priv;
+	int ret;
+
+	bridge_config.mask = cpu_to_le32(MXL862XX_BRIDGE_CONFIG_MASK_FORWARDING_MODE);
+	bridge_config.bridge_id = cpu_to_le16(bridge_id);
+
+	bridge_config.forward_unknown_unicast = ucast_flood ?
+		MXL862XX_BRIDGE_FORWARD_FLOOD : MXL862XX_BRIDGE_FORWARD_DISCARD;
+
+	bridge_config.forward_unknown_multicast_ip = mcast_flood ?
+		MXL862XX_BRIDGE_FORWARD_FLOOD : MXL862XX_BRIDGE_FORWARD_DISCARD;
+	bridge_config.forward_unknown_multicast_non_ip =
+		bridge_config.forward_unknown_multicast_ip;
+
+	bridge_config.forward_broadcast = bcast_flood ?
+		MXL862XX_BRIDGE_FORWARD_FLOOD : MXL862XX_BRIDGE_FORWARD_DISCARD;
+
+	ret = MXL862XX_API_WRITE(priv, MXL862XX_BRIDGE_CONFIGSET, bridge_config);
+	if (ret)
+		dev_err(ds->dev, "failed to configure bridge %u forwarding: %d\n",
+			bridge_id, ret);
+
+	return ret;
+}
+
+/* Allocate a single zero-rate meter shared by all ports and flood types.
+ * All flood-blocking egress sub-meters point to this one meter so that
+ * the single CBS=64 token bucket fills as quickly as possible, minimising
+ * the one-packet leak inherent with the hardware minimum CBS.
+ */
+static int mxl862xx_setup_drop_meter(struct dsa_switch *ds)
+{
+	struct mxl862xx_qos_meter_cfg meter = {};
+	struct mxl862xx_priv *priv = ds->priv;
+	int ret;
+
+	/* meter_id=0 means auto-alloc */
+	ret = MXL862XX_API_READ(priv, MXL862XX_QOS_METERALLOC, meter);
+	if (ret)
+		return ret;
+
+	meter.enable = true;
+	meter.cbs = cpu_to_le32(64);
+	meter.ebs = cpu_to_le32(64);
+	snprintf(meter.meter_name, sizeof(meter.meter_name), "drop");
+
+	ret = MXL862XX_API_WRITE(priv, MXL862XX_QOS_METERCFGSET, meter);
+	if (ret)
+		return ret;
+
+	priv->drop_meter = le16_to_cpu(meter.meter_id);
+
+	return 0;
+}
+
 static int mxl862xx_setup(struct dsa_switch *ds)
 {
 	struct mxl862xx_priv *priv = ds->priv;
@@ -182,6 +251,15 @@ static int mxl862xx_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
+	ret = mxl862xx_bridge_config_fwd(ds, MXL862XX_DEFAULT_BRIDGE,
+					 false, false, true);
+	if (ret < 0)
+		return ret;
+
+	ret = mxl862xx_setup_drop_meter(ds);
+	if (ret)
+		return ret;
+
 	return mxl862xx_setup_mdio(ds);
 }
 
@@ -259,64 +337,238 @@ static int mxl862xx_configure_sp_tag_proto(struct dsa_switch *ds, int port,
 	return MXL862XX_API_WRITE(ds->priv, MXL862XX_SS_SPTAG_SET, tag);
 }
 
-static int mxl862xx_setup_cpu_bridge(struct dsa_switch *ds, int port)
+static int mxl862xx_set_bridge_port(struct dsa_switch *ds, int port)
 {
 	struct mxl862xx_bridge_port_config br_port_cfg = {};
 	struct mxl862xx_priv *priv = ds->priv;
-	u16 bridge_port_map = 0;
-	struct dsa_port *dp;
+	struct mxl862xx_port *p = &priv->ports[port];
+	u16 bridge_id = p->bridge ? p->bridge->bridge_id : p->fid;
+	bool enable;
+	int i, idx;
 
-	/* CPU port bridge setup */
-	br_port_cfg.mask = cpu_to_le32(MXL862XX_BRIDGE_PORT_CONFIG_MASK_BRIDGE_PORT_MAP |
+	br_port_cfg.bridge_port_id = cpu_to_le16(port);
+	br_port_cfg.bridge_id = cpu_to_le16(bridge_id);
+	br_port_cfg.mask = cpu_to_le32(MXL862XX_BRIDGE_PORT_CONFIG_MASK_BRIDGE_ID |
+				       MXL862XX_BRIDGE_PORT_CONFIG_MASK_BRIDGE_PORT_MAP |
 				       MXL862XX_BRIDGE_PORT_CONFIG_MASK_MC_SRC_MAC_LEARNING |
-				       MXL862XX_BRIDGE_PORT_CONFIG_MASK_VLAN_BASED_MAC_LEARNING);
+				       MXL862XX_BRIDGE_PORT_CONFIG_MASK_EGRESS_SUB_METER);
+	br_port_cfg.src_mac_learning_disable = !p->learning;
 
-	br_port_cfg.bridge_port_id = cpu_to_le16(port);
-	br_port_cfg.src_mac_learning_disable = false;
-	br_port_cfg.vlan_src_mac_vid_enable = true;
-	br_port_cfg.vlan_dst_mac_vid_enable = true;
+	for (i = 0; i < ARRAY_SIZE(br_port_cfg.bridge_port_map); i++)
+		br_port_cfg.bridge_port_map[i] =
+			cpu_to_le16(bitmap_read(p->portmap, i * 16, 16));
+
+	for (i = 0; i < ARRAY_SIZE(mxl862xx_flood_meters); i++) {
+		idx = mxl862xx_flood_meters[i];
+		enable = !!(p->flood_block & BIT(idx));
+
+		br_port_cfg.egress_traffic_sub_meter_id[idx] =
+			enable ? cpu_to_le16(priv->drop_meter) : 0;
+		br_port_cfg.egress_sub_metering_enable[idx] = enable;
+	}
+
+	return MXL862XX_API_WRITE(priv, MXL862XX_BRIDGEPORT_CONFIGSET,
+				  br_port_cfg);
+}
+
+static int mxl862xx_setup_cpu_bridge(struct dsa_switch *ds, int port)
+{
+	struct mxl862xx_priv *priv = ds->priv;
+	struct dsa_port *dp;
+
+	priv->ports[port].fid = MXL862XX_DEFAULT_BRIDGE;
+	priv->ports[port].learning = true;
 
 	/* include all assigned user ports in the CPU portmap */
+	bitmap_zero(priv->ports[port].portmap, MXL862XX_MAX_BRIDGE_PORTS);
 	dsa_switch_for_each_user_port(dp, ds) {
 		/* it's safe to rely on cpu_dp being valid for user ports */
 		if (dp->cpu_dp->index != port)
 			continue;
 
-		bridge_port_map |= BIT(dp->index);
+		__set_bit(dp->index, priv->ports[port].portmap);
 	}
-	br_port_cfg.bridge_port_map[0] |= cpu_to_le16(bridge_port_map);
 
-	return MXL862XX_API_WRITE(priv, MXL862XX_BRIDGEPORT_CONFIGSET, br_port_cfg);
+	return mxl862xx_set_bridge_port(ds, port);
 }
 
 static int mxl862xx_add_single_port_bridge(struct dsa_switch *ds, int port)
 {
-	struct mxl862xx_bridge_port_config br_port_cfg = {};
 	struct dsa_port *dp = dsa_to_port(ds, port);
 	struct mxl862xx_bridge_alloc br_alloc = {};
+	struct mxl862xx_priv *priv = ds->priv;
 	int ret;
 
-	ret = MXL862XX_API_READ(ds->priv, MXL862XX_BRIDGE_ALLOC, br_alloc);
+	ret = MXL862XX_API_READ(priv, MXL862XX_BRIDGE_ALLOC, br_alloc);
 	if (ret) {
 		dev_err(ds->dev, "failed to allocate a bridge for port %d\n", port);
 		return ret;
 	}
 
-	br_port_cfg.bridge_id = br_alloc.bridge_id;
-	br_port_cfg.bridge_port_id = cpu_to_le16(port);
-	br_port_cfg.mask = cpu_to_le32(MXL862XX_BRIDGE_PORT_CONFIG_MASK_BRIDGE_ID |
-				       MXL862XX_BRIDGE_PORT_CONFIG_MASK_BRIDGE_PORT_MAP |
-				       MXL862XX_BRIDGE_PORT_CONFIG_MASK_MC_SRC_MAC_LEARNING |
-				       MXL862XX_BRIDGE_PORT_CONFIG_MASK_VLAN_BASED_MAC_LEARNING);
-	br_port_cfg.src_mac_learning_disable = true;
-	br_port_cfg.vlan_src_mac_vid_enable = false;
-	br_port_cfg.vlan_dst_mac_vid_enable = false;
-	/* As this function is only called for user ports it is safe to rely on
-	 * cpu_dp being valid
+	priv->ports[port].fid = le16_to_cpu(br_alloc.bridge_id);
+	priv->ports[port].learning = false;
+	bitmap_zero(priv->ports[port].portmap, MXL862XX_MAX_BRIDGE_PORTS);
+	__set_bit(dp->cpu_dp->index, priv->ports[port].portmap);
+
+	ret = mxl862xx_set_bridge_port(ds, port);
+	if (ret)
+		return ret;
+
+	/* Standalone ports should not flood unknown unicast or multicast
+	 * towards the CPU by default; only broadcast is needed initially.
 	 */
-	br_port_cfg.bridge_port_map[0] = cpu_to_le16(BIT(dp->cpu_dp->index));
+	return mxl862xx_bridge_config_fwd(ds, priv->ports[port].fid,
+					 false, false, true);
+}
+
+static struct mxl862xx_bridge *mxl862xx_allocate_bridge(struct dsa_switch *ds,
+							int num)
+{
+	struct mxl862xx_bridge_alloc br_alloc = {};
+	struct mxl862xx_priv *priv = ds->priv;
+	struct mxl862xx_bridge *mxlbridge;
+	int ret;
+
+	mxlbridge = kzalloc_obj(*mxlbridge);
+	if (!mxlbridge)
+		return ERR_PTR(-ENOMEM);
+
+	ret = MXL862XX_API_READ(priv, MXL862XX_BRIDGE_ALLOC, br_alloc);
+	if (ret) {
+		kfree(mxlbridge);
+		return ERR_PTR(ret);
+	}
+
+	mxlbridge->bridge_id = le16_to_cpu(br_alloc.bridge_id);
+	mxlbridge->dsa_bridge_num = num;
+
+	ret = mxl862xx_bridge_config_fwd(ds, mxlbridge->bridge_id,
+					 true, true, true);
+	if (ret) {
+		br_alloc.bridge_id = cpu_to_le16(mxlbridge->bridge_id);
+		MXL862XX_API_WRITE(priv, MXL862XX_BRIDGE_FREE, br_alloc);
+		kfree(mxlbridge);
+		return ERR_PTR(ret);
+	}
+
+	list_add(&mxlbridge->list, &priv->bridges);
+
+	return mxlbridge;
+}
+
+static void mxl862xx_free_bridge(struct dsa_switch *ds,
+				 struct mxl862xx_bridge *mxlbridge)
+{
+	struct mxl862xx_bridge_alloc br_alloc = {
+		.bridge_id = cpu_to_le16(mxlbridge->bridge_id),
+	};
+
+	MXL862XX_API_WRITE(ds->priv, MXL862XX_BRIDGE_FREE, br_alloc);
+	list_del(&mxlbridge->list);
+	kfree(mxlbridge);
+}
+
+static struct mxl862xx_bridge *mxl862xx_find_bridge(struct dsa_switch *ds,
+						    struct dsa_bridge bridge)
+{
+	struct mxl862xx_priv *priv = ds->priv;
+	struct mxl862xx_bridge *mxlbridge;
+
+	if (!bridge.num)
+		return NULL;
+
+	list_for_each_entry(mxlbridge, &priv->bridges, list) {
+		if (mxlbridge->dsa_bridge_num == bridge.num)
+			return mxlbridge;
+	}
+
+	return NULL;
+}
+
+static int mxl862xx_update_bridge(struct dsa_switch *ds,
+				  struct mxl862xx_bridge *mxlbridge,
+				  int port, bool join)
+{
+	struct mxl862xx_priv *priv = ds->priv;
+	int member, err, ret = 0;
+	struct dsa_port *dp;
+
+	if (join) {
+		__set_bit(port, mxlbridge->portmap);
+		priv->ports[port].bridge = mxlbridge;
+	} else {
+		__clear_bit(port, mxlbridge->portmap);
+		priv->ports[port].bridge = NULL;
+	}
+
+	/* Update all current bridge members' portmaps */
+	for_each_set_bit(member, mxlbridge->portmap,
+			 MXL862XX_MAX_BRIDGE_PORTS) {
+		dp = dsa_to_port(ds, member);
+
+		/* Build portmap: CPU port + all bridge members except self */
+		bitmap_copy(priv->ports[member].portmap, mxlbridge->portmap,
+			    MXL862XX_MAX_BRIDGE_PORTS);
+		__clear_bit(member, priv->ports[member].portmap);
+		__set_bit(dp->cpu_dp->index, priv->ports[member].portmap);
+
+		err = mxl862xx_set_bridge_port(ds, member);
+		if (err)
+			ret = err;
+	}
+
+	/* Revert leaving port to its single-port bridge */
+	if (!join) {
+		dp = dsa_to_port(ds, port);
+
+		bitmap_zero(priv->ports[port].portmap, MXL862XX_MAX_BRIDGE_PORTS);
+		__set_bit(dp->cpu_dp->index, priv->ports[port].portmap);
+		priv->ports[port].flood_block = 0;
+		priv->ports[port].learning = false;
+		ret = mxl862xx_set_bridge_port(ds, port);
+		if (err)
+			ret = err;
+
+		mxl862xx_port_fast_age(ds, port);
+	}
+
+	return ret;
+}
+
+static int mxl862xx_port_bridge_join(struct dsa_switch *ds, int port,
+				     struct dsa_bridge bridge,
+				     bool *tx_fwd_offload,
+				     struct netlink_ext_ack *extack)
+{
+	struct mxl862xx_bridge *mxlbridge;
+
+	mxlbridge = mxl862xx_find_bridge(ds, bridge);
+	if (!mxlbridge) {
+		mxlbridge = mxl862xx_allocate_bridge(ds, bridge.num);
+		if (IS_ERR(mxlbridge))
+			return PTR_ERR(mxlbridge);
+	}
+
+	return mxl862xx_update_bridge(ds, mxlbridge, port, true);
+}
+
+static void mxl862xx_port_bridge_leave(struct dsa_switch *ds, int port,
+				       struct dsa_bridge bridge)
+{
+	struct mxl862xx_bridge *mxlbridge;
+	int err;
+
+	mxlbridge = mxl862xx_find_bridge(ds, bridge);
+	if (!mxlbridge)
+		return;
+
+	err = mxl862xx_update_bridge(ds, mxlbridge, port, false);
+	if (err)
+		dev_err(ds->dev,
+			"updating bridge failed for leaving port %d\n", port);
 
-	return MXL862XX_API_WRITE(ds->priv, MXL862XX_BRIDGEPORT_CONFIGSET, br_port_cfg);
+	if (bitmap_empty(mxlbridge->portmap, MXL862XX_MAX_BRIDGE_PORTS))
+		mxl862xx_free_bridge(ds, mxlbridge);
 }
 
 static int mxl862xx_port_setup(struct dsa_switch *ds, int port)
@@ -366,6 +618,259 @@ static void mxl862xx_phylink_get_caps(struct dsa_switch *ds, int port,
 		  config->supported_interfaces);
 }
 
+static int mxl862xx_get_fid(struct dsa_switch *ds, struct dsa_db db)
+{
+	struct mxl862xx_priv *priv = ds->priv;
+	struct mxl862xx_bridge *mxlbridge;
+
+	switch (db.type) {
+	case DSA_DB_PORT:
+		return priv->ports[db.dp->index].fid;
+
+	case DSA_DB_BRIDGE:
+		mxlbridge = mxl862xx_find_bridge(ds, db.bridge);
+		if (!mxlbridge)
+			return -ENOENT;
+		return mxlbridge->bridge_id;
+
+	default:
+		return  -EOPNOTSUPP;
+	}
+}
+
+static int mxl862xx_port_fdb_add(struct dsa_switch *ds, int port,
+				 const unsigned char *addr, u16 vid, struct dsa_db db)
+{
+	struct mxl862xx_mac_table_add param = { };
+	int fid = mxl862xx_get_fid(ds, db), ret;
+	struct mxl862xx_priv *priv = ds->priv;
+
+	if (fid < 0)
+		return fid;
+
+	param.port_id = cpu_to_le32(port);
+	param.static_entry = true;
+	param.fid = cpu_to_le16(fid);
+	param.tci = cpu_to_le16(FIELD_PREP(MXL862XX_TCI_VLAN_ID, vid));
+	ether_addr_copy(param.mac, addr);
+
+	ret = MXL862XX_API_WRITE(priv, MXL862XX_MAC_TABLEENTRYADD, param);
+	if (ret)
+		dev_err(ds->dev, "failed to add FDB entry on port %d\n", port);
+
+	return ret;
+}
+
+static int mxl862xx_port_fdb_del(struct dsa_switch *ds, int port,
+				 const unsigned char *addr, u16 vid, struct dsa_db db)
+{
+	struct mxl862xx_mac_table_remove param = { };
+	int fid = mxl862xx_get_fid(ds, db), ret;
+	struct mxl862xx_priv *priv = ds->priv;
+
+	if (fid < 0)
+		return fid;
+
+	param.fid = cpu_to_le16(fid);
+	param.tci = cpu_to_le16(FIELD_PREP(MXL862XX_TCI_VLAN_ID, vid));
+	ether_addr_copy(param.mac, addr);
+
+	ret = MXL862XX_API_WRITE(priv, MXL862XX_MAC_TABLEENTRYREMOVE, param);
+	if (ret)
+		dev_err(ds->dev, "failed to remove FDB entry on port %d\n", port);
+
+	return ret;
+}
+
+static int mxl862xx_port_fdb_dump(struct dsa_switch *ds, int port,
+				  dsa_fdb_dump_cb_t *cb, void *data)
+{
+	struct mxl862xx_mac_table_read param = { };
+	struct mxl862xx_priv *priv = ds->priv;
+	u32 entry_port_id;
+	int ret;
+
+	while (true) {
+		ret = MXL862XX_API_READ(priv, MXL862XX_MAC_TABLEENTRYREAD, param);
+		if (ret)
+			return ret;
+
+		if (param.last)
+			break;
+
+		entry_port_id = le32_to_cpu(param.port_id);
+
+		if (entry_port_id == port) {
+			ret = cb(param.mac, FIELD_GET(MXL862XX_TCI_VLAN_ID,
+						      le16_to_cpu(param.tci)),
+				 param.static_entry, data);
+			if (ret)
+				return ret;
+		}
+
+		memset(&param, 0, sizeof(param));
+	}
+
+	return 0;
+}
+
+static int mxl862xx_port_mdb_add(struct dsa_switch *ds, int port,
+				 const struct switchdev_obj_port_mdb *mdb,
+				 struct dsa_db db)
+{
+	return mxl862xx_port_fdb_add(ds, port, mdb->addr, mdb->vid, db);
+}
+
+static int mxl862xx_port_mdb_del(struct dsa_switch *ds, int port,
+				 const struct switchdev_obj_port_mdb *mdb,
+				 struct dsa_db db)
+{
+	return mxl862xx_port_fdb_del(ds, port, mdb->addr, mdb->vid, db);
+}
+
+static int mxl862xx_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
+{
+	struct mxl862xx_cfg param;
+	int ret;
+
+	ret = MXL862XX_API_READ(ds->priv, MXL862XX_COMMON_CFGGET, param);
+	if (ret) {
+		dev_err(ds->dev, "failed to read switch config\n");
+		return ret;
+	}
+
+	param.mac_table_age_timer = cpu_to_le32(MXL862XX_AGETIMER_CUSTOM);
+	param.age_timer = cpu_to_le32(msecs / 1000);
+	ret = MXL862XX_API_WRITE(ds->priv, MXL862XX_COMMON_CFGSET, param);
+	if (ret)
+		dev_err(ds->dev, "failed to set ageing\n");
+
+	return ret;
+}
+
+static void mxl862xx_port_stp_state_set(struct dsa_switch *ds, int port,
+					u8 state)
+{
+	struct mxl862xx_stp_port_cfg param = {
+		.port_id = cpu_to_le16(port),
+	};
+	struct mxl862xx_priv *priv = ds->priv;
+	int ret;
+
+	switch (state) {
+	case BR_STATE_DISABLED:
+		param.port_state = cpu_to_le32(MXL862XX_STP_PORT_STATE_DISABLE);
+		break;
+	case BR_STATE_BLOCKING:
+	case BR_STATE_LISTENING:
+		param.port_state = cpu_to_le32(MXL862XX_STP_PORT_STATE_BLOCKING);
+		break;
+	case BR_STATE_LEARNING:
+		param.port_state = cpu_to_le32(MXL862XX_STP_PORT_STATE_LEARNING);
+		break;
+	case BR_STATE_FORWARDING:
+		param.port_state = cpu_to_le32(MXL862XX_STP_PORT_STATE_FORWARD);
+		break;
+	default:
+		dev_err(ds->dev, "invalid STP state: %d\n", state);
+		return;
+	}
+
+	ret = MXL862XX_API_WRITE(priv, MXL862XX_STP_PORTCFGSET, param);
+	if (ret) {
+		dev_err(ds->dev, "failed to set STP state on port %d\n", port);
+		return;
+	}
+
+	/* The firmware may re-enable MAC learning as a side-effect of entering
+	 * LEARNING or FORWARDING state (per 802.1D defaults).
+	 * Re-apply the driver's intended learning and metering config so that
+	 * standalone ports keep learning disabled.
+	 * This is likely to get fixed in future firmware releases, however,
+	 * the additional API call even then doesn't hurt much.
+	 */
+	ret = mxl862xx_set_bridge_port(ds, port);
+	if (ret)
+		dev_err(ds->dev, "failed to reapply brport flags on port %d\n", port);
+
+	mxl862xx_port_fast_age(ds, port);
+}
+
+static void mxl862xx_port_set_host_flood(struct dsa_switch *ds, int port,
+					 bool uc, bool mc)
+{
+	struct mxl862xx_priv *priv = ds->priv;
+
+	if (priv->ports[port].bridge)
+		return;
+
+	/* Standalone ports must always keep broadcast flooding enabled
+	 * towards the CPU so that ARP and other broadcast protocols work.
+	 * Only unknown unicast and multicast should follow the host flood
+	 * knobs driven by IFF_PROMISC / IFF_ALLMULTI.
+	 */
+	mxl862xx_bridge_config_fwd(ds, priv->ports[port].fid, uc, mc, true);
+}
+
+static int mxl862xx_port_pre_bridge_flags(struct dsa_switch *ds, int port,
+					  struct switchdev_brport_flags flags,
+					  struct netlink_ext_ack *extack)
+{
+	if (flags.mask & ~(BR_FLOOD | BR_MCAST_FLOOD | BR_BCAST_FLOOD |
+			   BR_LEARNING))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int mxl862xx_port_bridge_flags(struct dsa_switch *ds, int port,
+				      struct switchdev_brport_flags flags,
+				      struct netlink_ext_ack *extack)
+{
+	struct mxl862xx_priv *priv = ds->priv;
+	unsigned long old_block = priv->ports[port].flood_block;
+	unsigned long block = old_block;
+	bool need_update = false;
+	int ret;
+
+	if (flags.mask & BR_FLOOD) {
+		if (flags.val & BR_FLOOD)
+			block &= ~BIT(MXL862XX_BRIDGE_PORT_EGRESS_METER_UNKNOWN_UC);
+		else
+			block |= BIT(MXL862XX_BRIDGE_PORT_EGRESS_METER_UNKNOWN_UC);
+	}
+
+	if (flags.mask & BR_MCAST_FLOOD) {
+		if (flags.val & BR_MCAST_FLOOD) {
+			block &= ~BIT(MXL862XX_BRIDGE_PORT_EGRESS_METER_UNKNOWN_MC_IP);
+			block &= ~BIT(MXL862XX_BRIDGE_PORT_EGRESS_METER_UNKNOWN_MC_NON_IP);
+		} else {
+			block |= BIT(MXL862XX_BRIDGE_PORT_EGRESS_METER_UNKNOWN_MC_IP);
+			block |= BIT(MXL862XX_BRIDGE_PORT_EGRESS_METER_UNKNOWN_MC_NON_IP);
+		}
+	}
+
+	if (flags.mask & BR_BCAST_FLOOD) {
+		if (flags.val & BR_BCAST_FLOOD)
+			block &= ~BIT(MXL862XX_BRIDGE_PORT_EGRESS_METER_BROADCAST);
+		else
+			block |= BIT(MXL862XX_BRIDGE_PORT_EGRESS_METER_BROADCAST);
+	}
+
+	if (flags.mask & BR_LEARNING)
+		priv->ports[port].learning = !!(flags.val & BR_LEARNING);
+
+	need_update = (block != old_block) || (flags.mask & BR_LEARNING);
+	if (need_update) {
+		priv->ports[port].flood_block = block;
+		ret = mxl862xx_set_bridge_port(ds, port);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static const struct dsa_switch_ops mxl862xx_switch_ops = {
 	.get_tag_protocol = mxl862xx_get_tag_protocol,
 	.setup = mxl862xx_setup,
@@ -374,6 +879,18 @@ static const struct dsa_switch_ops mxl862xx_switch_ops = {
 	.port_enable = mxl862xx_port_enable,
 	.port_disable = mxl862xx_port_disable,
 	.port_fast_age = mxl862xx_port_fast_age,
+	.set_ageing_time = mxl862xx_set_ageing_time,
+	.port_bridge_join = mxl862xx_port_bridge_join,
+	.port_bridge_leave = mxl862xx_port_bridge_leave,
+	.port_pre_bridge_flags = mxl862xx_port_pre_bridge_flags,
+	.port_bridge_flags = mxl862xx_port_bridge_flags,
+	.port_stp_state_set = mxl862xx_port_stp_state_set,
+	.port_set_host_flood = mxl862xx_port_set_host_flood,
+	.port_fdb_add = mxl862xx_port_fdb_add,
+	.port_fdb_del = mxl862xx_port_fdb_del,
+	.port_fdb_dump = mxl862xx_port_fdb_dump,
+	.port_mdb_add = mxl862xx_port_mdb_add,
+	.port_mdb_del = mxl862xx_port_mdb_del,
 };
 
 static void mxl862xx_phylink_mac_config(struct phylink_config *config,
@@ -415,6 +932,8 @@ static int mxl862xx_probe(struct mdio_device *mdiodev)
 
 	priv->mdiodev = mdiodev;
 
+	INIT_LIST_HEAD(&priv->bridges);
+
 	ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
 	if (!ds)
 		return -ENOMEM;
@@ -425,6 +944,8 @@ static int mxl862xx_probe(struct mdio_device *mdiodev)
 	ds->ops = &mxl862xx_switch_ops;
 	ds->phylink_mac_ops = &mxl862xx_phylink_mac_ops;
 	ds->num_ports = MXL862XX_MAX_PORTS;
+	ds->fdb_isolation = true;
+	ds->max_num_bridges = MXL862XX_MAX_BRIDGES;
 
 	dev_set_drvdata(dev, ds);
 
diff --git a/drivers/net/dsa/mxl862xx/mxl862xx.h b/drivers/net/dsa/mxl862xx/mxl862xx.h
index bfeb436942d5f..d5e1a4dee9ea7 100644
--- a/drivers/net/dsa/mxl862xx/mxl862xx.h
+++ b/drivers/net/dsa/mxl862xx/mxl862xx.h
@@ -7,10 +7,31 @@
 #include <net/dsa.h>
 
 #define MXL862XX_MAX_PORTS		17
+#define MXL862XX_DEFAULT_BRIDGE		0
+#define MXL862XX_MAX_BRIDGES		48
+#define MXL862XX_MAX_BRIDGE_PORTS	128
+
+struct mxl862xx_bridge {
+	unsigned int dsa_bridge_num;
+	u16 bridge_id;
+	DECLARE_BITMAP(portmap, MXL862XX_MAX_BRIDGE_PORTS);
+	struct list_head list;
+};
+
+struct mxl862xx_port {
+	u16 fid; /* single-port bridge ID (permanent) */
+	struct mxl862xx_bridge *bridge;
+	DECLARE_BITMAP(portmap, MXL862XX_MAX_BRIDGE_PORTS);
+	unsigned long flood_block; /* bitmask of meter indices with metering on */
+	bool learning;
+};
 
 struct mxl862xx_priv {
 	struct dsa_switch *ds;
 	struct mdio_device *mdiodev;
+	u16 drop_meter; /* single zero-rate meter shared by all ports */
+	struct mxl862xx_port ports[MXL862XX_MAX_PORTS];
+	struct list_head bridges;
 };
 
 #endif /* __MXL862XX_H */
-- 
2.53.0
Re: [PATCH net-next v2 2/2] net: dsa: mxl862xx: implement bridge offloading
Posted by Vladimir Oltean 4 weeks, 1 day ago
Hi Daniel,

On Tue, Mar 10, 2026 at 12:40:29AM +0000, Daniel Golle wrote:
>  * drop manually resetting port learning state on bridge<->standalone
>    transitions, DSA framework takes care of that
(...)
> +	/* Revert leaving port to its single-port bridge */
> +	if (!join) {
> +		dp = dsa_to_port(ds, port);
> +
> +		bitmap_zero(priv->ports[port].portmap, MXL862XX_MAX_BRIDGE_PORTS);
> +		__set_bit(dp->cpu_dp->index, priv->ports[port].portmap);
> +		priv->ports[port].flood_block = 0;
> +		priv->ports[port].learning = false;

So is this needed or not? Change log says "drop" but code says "keep".

The core does:
dsa_port_bridge_leave()
-> dsa_port_switchdev_unsync_attrs()
   -> dsa_port_clear_brport_flags()
      -> dsa_port_bridge_flags() // BR_LEARNING in mask and not in val

> +		ret = mxl862xx_set_bridge_port(ds, port);
> +		if (err)
> +			ret = err;
> +
> +		mxl862xx_port_fast_age(ds, port);
> +	}
(...)
>  * manually mxl862xx_port_fast_age() in mxl862xx_port_stp_state_set()
>    to avoid FDB poisoning due to race condition
(...)
> +	/* Revert leaving port to its single-port bridge */
> +	if (!join) {
> +		dp = dsa_to_port(ds, port);
> +
> +		bitmap_zero(priv->ports[port].portmap, MXL862XX_MAX_BRIDGE_PORTS);
> +		__set_bit(dp->cpu_dp->index, priv->ports[port].portmap);
> +		priv->ports[port].flood_block = 0;
> +		priv->ports[port].learning = false;
> +		ret = mxl862xx_set_bridge_port(ds, port);
> +		if (err)
> +			ret = err;
> +
> +		mxl862xx_port_fast_age(ds, port);
> +	}

I only requested this to be done on mxl862xx_port_stp_state_set(), as a
consequence to your workaround, not on mxl862xx_port_bridge_leave() ->
mxl862xx_update_bridge().

The framework actually has logic to fast age the FDB. A standalone port
is in BR_STATE_FORWARDING, and a leaving/joining bridge port goes
through BR_STATE_DISABLED - del_nbp() -> br_stp_disable_port().
So we have a guaranteed STP transition based on which this hook runs:

dsa_port_switchdev_unsync_attrs():
	/* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer,
	 * so allow it to be in BR_STATE_FORWARDING to be kept functional
	 */
	dsa_port_set_state_now(dp, BR_STATE_FORWARDING, true);
->
		/* Fast age FDB entries or flush appropriate forwarding database
		 * for the given port, if we are moving it from Learning or
		 * Forwarding state, to Disabled or Blocking or Listening state.
		 * Ports that were standalone before the STP state change don't
		 * need to fast age the FDB, since address learning is off in
		 * standalone mode.
		 */

		if ((dp->stp_state == BR_STATE_LEARNING ||
		     dp->stp_state == BR_STATE_FORWARDING) &&
		    (state == BR_STATE_DISABLED ||
		     state == BR_STATE_BLOCKING ||
		     state == BR_STATE_LISTENING))
			dsa_port_fast_age(dp);

so I think fast aging is unnecessary here.

Your workaround is different, DSA doesn't know that
dsa_port_set_state(BR_STATE_LEARNING) with dp->learning == false
actually temporarily enables learning. It assumes it doesn't, so it
doesn't call dsa_port_fast_age(). That's why you have to do it.


Did you reply to my comment from v1 to remove the "bool join" false
sharing from mxl862xx_update_bridge()? Because you didn't, and I'm not
sure why.

I meant to see:

static int mxl862xx_sync_bridge_members(struct dsa_switch *ds,
					struct mxl862xx_bridge *mxlbridge)
{
	struct mxl862xx_priv *priv = ds->priv;
	int member, ret = 0;

	/* Update all current bridge members' portmaps */
	for_each_set_bit(member, mxlbridge->portmap,
			 MXL862XX_MAX_BRIDGE_PORTS) {
		struct dsa_port *dp = dsa_to_port(ds, member);
		int err;

		/* Build portmap: CPU port + all bridge members except self */
		bitmap_copy(priv->ports[member].portmap, mxlbridge->portmap,
			    MXL862XX_MAX_BRIDGE_PORTS);
		__clear_bit(member, priv->ports[member].portmap);
		__set_bit(dp->cpu_dp->index, priv->ports[member].portmap);

		err = mxl862xx_set_bridge_port(ds, member);
		if (err)
			ret = err;
	}

	return ret;
}

static int mxl862xx_port_bridge_join(struct dsa_switch *ds, int port,
				     struct dsa_bridge bridge,
				     bool *tx_fwd_offload,
				     struct netlink_ext_ack *extack)
{
	struct mxl862xx_bridge *mxlbridge;

	mxlbridge = mxl862xx_find_bridge(ds, bridge);
	if (!mxlbridge) {
		mxlbridge = mxl862xx_allocate_bridge(ds, bridge.num);
		if (IS_ERR(mxlbridge))
			return PTR_ERR(mxlbridge);
	}

	__set_bit(port, mxlbridge->portmap);
	priv->ports[port].bridge = mxlbridge;

	/* The operation may fail mid way and there is no way to restore
	 * the driver in sync with a known FW state. So we consider FW
	 * I/O failure as catastrophic, no point to complicate the
	 * driver by restoring mxlbridge->portmap or the bridge pointer.
	 */
	return mxl862xx_sync_bridge_members(ds, mxlbridge);
}

static void mxl862xx_port_bridge_leave(struct dsa_switch *ds, int port,
				       struct dsa_bridge bridge)
{
	struct dsa_port *dp = dsa_to_port(ds, port);
	struct mxl862xx_bridge *mxlbridge;
	int err;

	mxlbridge = mxl862xx_find_bridge(ds, bridge);
	if (!mxlbridge)
		return;

	__clear_bit(port, mxlbridge->portmap);
	priv->ports[port].bridge = NULL;

	err = mxl862xx_sync_bridge_members(ds, mxlbridge);
	if (err) {
		dev_err(ds->dev,
			"failed to sync bridge members after port %d left: %pe\n",
			port, ERR_PTR(err));
	}

	/* Revert leaving port, omitted by the sync above, to its
	 * single-port bridge
	 */
	bitmap_zero(priv->ports[port].portmap, MXL862XX_MAX_BRIDGE_PORTS);
	__set_bit(dp->cpu_dp->index, priv->ports[port].portmap);
	priv->ports[port].flood_block = 0;
	err = mxl862xx_set_bridge_port(ds, port);
	if (err) {
		dev_err(ds->dev,
			"failed to update bridge port %d state: %pe\n", port,
			ERR_PTR(err));
	}

	return MXL862XX_API_WRITE(ds->priv, MXL862XX_BRIDGEPORT_CONFIGSET, br_port_cfg);
	if (bitmap_empty(mxlbridge->portmap, MXL862XX_MAX_BRIDGE_PORTS))
		mxl862xx_free_bridge(ds, mxlbridge);
}
Re: [PATCH net-next v2 2/2] net: dsa: mxl862xx: implement bridge offloading
Posted by Daniel Golle 4 weeks, 1 day ago
On Tue, Mar 10, 2026 at 05:53:40PM +0200, Vladimir Oltean wrote:
> Hi Daniel,
> 
> On Tue, Mar 10, 2026 at 12:40:29AM +0000, Daniel Golle wrote:
> >  * drop manually resetting port learning state on bridge<->standalone
> >    transitions, DSA framework takes care of that
> (...)
> > +	/* Revert leaving port to its single-port bridge */
> > +	if (!join) {
> > +		dp = dsa_to_port(ds, port);
> > +
> > +		bitmap_zero(priv->ports[port].portmap, MXL862XX_MAX_BRIDGE_PORTS);
> > +		__set_bit(dp->cpu_dp->index, priv->ports[port].portmap);
> > +		priv->ports[port].flood_block = 0;
> > +		priv->ports[port].learning = false;
> 
> So is this needed or not? Change log says "drop" but code says "keep".
> 
> The core does:
> dsa_port_bridge_leave()
> -> dsa_port_switchdev_unsync_attrs()
>    -> dsa_port_clear_brport_flags()
>       -> dsa_port_bridge_flags() // BR_LEARNING in mask and not in val

Sorry, I just forgot to remove it there. It should not be needed, but
I'll rebuild and test without it to be sure.

> 
> > +		ret = mxl862xx_set_bridge_port(ds, port);
> > +		if (err)
> > +			ret = err;
> > +
> > +		mxl862xx_port_fast_age(ds, port);
> > +	}
> (...)
> >  * manually mxl862xx_port_fast_age() in mxl862xx_port_stp_state_set()
> >    to avoid FDB poisoning due to race condition
> (...)
> > +	/* Revert leaving port to its single-port bridge */
> > +	if (!join) {
> > +		dp = dsa_to_port(ds, port);
> > +
> > +		bitmap_zero(priv->ports[port].portmap, MXL862XX_MAX_BRIDGE_PORTS);
> > +		__set_bit(dp->cpu_dp->index, priv->ports[port].portmap);
> > +		priv->ports[port].flood_block = 0;
> > +		priv->ports[port].learning = false;
> > +		ret = mxl862xx_set_bridge_port(ds, port);
> > +		if (err)
> > +			ret = err;
> > +
> > +		mxl862xx_port_fast_age(ds, port);
> > +	}
> 
> I only requested this to be done on mxl862xx_port_stp_state_set(), as a
> consequence to your workaround, not on mxl862xx_port_bridge_leave() ->
> mxl862xx_update_bridge().

I'll remove that then, it was already present in v1.

> 
> The framework actually has logic to fast age the FDB. A standalone port
> is in BR_STATE_FORWARDING, and a leaving/joining bridge port goes
> through BR_STATE_DISABLED - del_nbp() -> br_stp_disable_port().
> So we have a guaranteed STP transition based on which this hook runs:
> 
> dsa_port_switchdev_unsync_attrs():
> 	/* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer,
> 	 * so allow it to be in BR_STATE_FORWARDING to be kept functional
> 	 */
> 	dsa_port_set_state_now(dp, BR_STATE_FORWARDING, true);
> ->
> 		/* Fast age FDB entries or flush appropriate forwarding database
> 		 * for the given port, if we are moving it from Learning or
> 		 * Forwarding state, to Disabled or Blocking or Listening state.
> 		 * Ports that were standalone before the STP state change don't
> 		 * need to fast age the FDB, since address learning is off in
> 		 * standalone mode.
> 		 */
> 
> 		if ((dp->stp_state == BR_STATE_LEARNING ||
> 		     dp->stp_state == BR_STATE_FORWARDING) &&
> 		    (state == BR_STATE_DISABLED ||
> 		     state == BR_STATE_BLOCKING ||
> 		     state == BR_STATE_LISTENING))
> 			dsa_port_fast_age(dp);
> 
> so I think fast aging is unnecessary here.
> 
> Your workaround is different, DSA doesn't know that
> dsa_port_set_state(BR_STATE_LEARNING) with dp->learning == false
> actually temporarily enables learning. It assumes it doesn't, so it
> doesn't call dsa_port_fast_age(). That's why you have to do it.

Understood. Thank you for explaining the context in detail.

> 
> 
> Did you reply to my comment from v1 to remove the "bool join" false
> sharing from mxl862xx_update_bridge()? Because you didn't, and I'm not
> sure why.

I wanted to reply to that but then forgot...
You sample code below makes it much more clear also what you meant,
and I will follow your suggestion in v3.


> 
> I meant to see:
> 
> static int mxl862xx_sync_bridge_members(struct dsa_switch *ds,
> 					struct mxl862xx_bridge *mxlbridge)
> {
> 	struct mxl862xx_priv *priv = ds->priv;
> 	int member, ret = 0;
> 
> 	/* Update all current bridge members' portmaps */
> 	for_each_set_bit(member, mxlbridge->portmap,
> 			 MXL862XX_MAX_BRIDGE_PORTS) {
> 		struct dsa_port *dp = dsa_to_port(ds, member);
> 		int err;
> 
> 		/* Build portmap: CPU port + all bridge members except self */
> 		bitmap_copy(priv->ports[member].portmap, mxlbridge->portmap,
> 			    MXL862XX_MAX_BRIDGE_PORTS);
> 		__clear_bit(member, priv->ports[member].portmap);
> 		__set_bit(dp->cpu_dp->index, priv->ports[member].portmap);
> 
> 		err = mxl862xx_set_bridge_port(ds, member);
> 		if (err)
> 			ret = err;
> 	}
> 
> 	return ret;
> }
> 
> static int mxl862xx_port_bridge_join(struct dsa_switch *ds, int port,
> 				     struct dsa_bridge bridge,
> 				     bool *tx_fwd_offload,
> 				     struct netlink_ext_ack *extack)
> {
> 	struct mxl862xx_bridge *mxlbridge;
> 
> 	mxlbridge = mxl862xx_find_bridge(ds, bridge);
> 	if (!mxlbridge) {
> 		mxlbridge = mxl862xx_allocate_bridge(ds, bridge.num);
> 		if (IS_ERR(mxlbridge))
> 			return PTR_ERR(mxlbridge);
> 	}
> 
> 	__set_bit(port, mxlbridge->portmap);
> 	priv->ports[port].bridge = mxlbridge;
> 
> 	/* The operation may fail mid way and there is no way to restore
> 	 * the driver in sync with a known FW state. So we consider FW
> 	 * I/O failure as catastrophic, no point to complicate the
> 	 * driver by restoring mxlbridge->portmap or the bridge pointer.
> 	 */
> 	return mxl862xx_sync_bridge_members(ds, mxlbridge);
> }
> 
> static void mxl862xx_port_bridge_leave(struct dsa_switch *ds, int port,
> 				       struct dsa_bridge bridge)
> {
> 	struct dsa_port *dp = dsa_to_port(ds, port);
> 	struct mxl862xx_bridge *mxlbridge;
> 	int err;
> 
> 	mxlbridge = mxl862xx_find_bridge(ds, bridge);
> 	if (!mxlbridge)
> 		return;
> 
> 	__clear_bit(port, mxlbridge->portmap);
> 	priv->ports[port].bridge = NULL;
> 
> 	err = mxl862xx_sync_bridge_members(ds, mxlbridge);
> 	if (err) {
> 		dev_err(ds->dev,
> 			"failed to sync bridge members after port %d left: %pe\n",
> 			port, ERR_PTR(err));
> 	}
> 
> 	/* Revert leaving port, omitted by the sync above, to its
> 	 * single-port bridge
> 	 */
> 	bitmap_zero(priv->ports[port].portmap, MXL862XX_MAX_BRIDGE_PORTS);
> 	__set_bit(dp->cpu_dp->index, priv->ports[port].portmap);
> 	priv->ports[port].flood_block = 0;
> 	err = mxl862xx_set_bridge_port(ds, port);
> 	if (err) {
> 		dev_err(ds->dev,
> 			"failed to update bridge port %d state: %pe\n", port,
> 			ERR_PTR(err));
> 	}
> 
> 	return MXL862XX_API_WRITE(ds->priv, MXL862XX_BRIDGEPORT_CONFIGSET, br_port_cfg);
> 	if (bitmap_empty(mxlbridge->portmap, MXL862XX_MAX_BRIDGE_PORTS))
> 		mxl862xx_free_bridge(ds, mxlbridge);
> }
Re: [PATCH net-next v2 2/2] net: dsa: mxl862xx: implement bridge offloading
Posted by kernel test robot 4 weeks, 1 day ago
Hi Daniel,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Golle/dsa-tag_mxl862xx-set-dsa_default_offload_fwd_mark/20260310-084425
base:   net-next/main
patch link:    https://lore.kernel.org/r/f6fb8c9c6d38585c0efce72b3d1aebe923277d5a.1773102942.git.daniel%40makrotopia.org
patch subject: [PATCH net-next v2 2/2] net: dsa: mxl862xx: implement bridge offloading
config: x86_64-randconfig-004-20260310 (https://download.01.org/0day-ci/archive/20260310/202603102333.SSWo6WNe-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260310/202603102333.SSWo6WNe-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603102333.SSWo6WNe-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/net/dsa/mxl862xx/mxl862xx.c:655:2: error: call to undeclared function 'ether_addr_copy'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     655 |         ether_addr_copy(param.mac, addr);
         |         ^
   drivers/net/dsa/mxl862xx/mxl862xx.c:676:2: error: call to undeclared function 'ether_addr_copy'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     676 |         ether_addr_copy(param.mac, addr);
         |         ^
   2 errors generated.


vim +/ether_addr_copy +655 drivers/net/dsa/mxl862xx/mxl862xx.c

   640	
   641	static int mxl862xx_port_fdb_add(struct dsa_switch *ds, int port,
   642					 const unsigned char *addr, u16 vid, struct dsa_db db)
   643	{
   644		struct mxl862xx_mac_table_add param = { };
   645		int fid = mxl862xx_get_fid(ds, db), ret;
   646		struct mxl862xx_priv *priv = ds->priv;
   647	
   648		if (fid < 0)
   649			return fid;
   650	
   651		param.port_id = cpu_to_le32(port);
   652		param.static_entry = true;
   653		param.fid = cpu_to_le16(fid);
   654		param.tci = cpu_to_le16(FIELD_PREP(MXL862XX_TCI_VLAN_ID, vid));
 > 655		ether_addr_copy(param.mac, addr);
   656	
   657		ret = MXL862XX_API_WRITE(priv, MXL862XX_MAC_TABLEENTRYADD, param);
   658		if (ret)
   659			dev_err(ds->dev, "failed to add FDB entry on port %d\n", port);
   660	
   661		return ret;
   662	}
   663	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH net-next v2 2/2] net: dsa: mxl862xx: implement bridge offloading
Posted by kernel test robot 4 weeks, 1 day ago
Hi Daniel,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Golle/dsa-tag_mxl862xx-set-dsa_default_offload_fwd_mark/20260310-084425
base:   net-next/main
patch link:    https://lore.kernel.org/r/f6fb8c9c6d38585c0efce72b3d1aebe923277d5a.1773102942.git.daniel%40makrotopia.org
patch subject: [PATCH net-next v2 2/2] net: dsa: mxl862xx: implement bridge offloading
config: sh-randconfig-001-20260310 (https://download.01.org/0day-ci/archive/20260310/202603102342.ymeQ8hLm-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260310/202603102342.ymeQ8hLm-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603102342.ymeQ8hLm-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/net/dsa/mxl862xx/mxl862xx.c: In function 'mxl862xx_port_fdb_add':
>> drivers/net/dsa/mxl862xx/mxl862xx.c:655:9: error: implicit declaration of function 'ether_addr_copy'; did you mean 'inetpeer_addr_cmp'? [-Wimplicit-function-declaration]
     655 |         ether_addr_copy(param.mac, addr);
         |         ^~~~~~~~~~~~~~~
         |         inetpeer_addr_cmp


vim +655 drivers/net/dsa/mxl862xx/mxl862xx.c

   640	
   641	static int mxl862xx_port_fdb_add(struct dsa_switch *ds, int port,
   642					 const unsigned char *addr, u16 vid, struct dsa_db db)
   643	{
   644		struct mxl862xx_mac_table_add param = { };
   645		int fid = mxl862xx_get_fid(ds, db), ret;
   646		struct mxl862xx_priv *priv = ds->priv;
   647	
   648		if (fid < 0)
   649			return fid;
   650	
   651		param.port_id = cpu_to_le32(port);
   652		param.static_entry = true;
   653		param.fid = cpu_to_le16(fid);
   654		param.tci = cpu_to_le16(FIELD_PREP(MXL862XX_TCI_VLAN_ID, vid));
 > 655		ether_addr_copy(param.mac, addr);
   656	
   657		ret = MXL862XX_API_WRITE(priv, MXL862XX_MAC_TABLEENTRYADD, param);
   658		if (ret)
   659			dev_err(ds->dev, "failed to add FDB entry on port %d\n", port);
   660	
   661		return ret;
   662	}
   663	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki