Drivers that offload bridges need to iterate over the ports that are
members of a given bridge, for example to rebuild per-port forwarding
bitmaps when membership changes. Currently each driver must open-code
this by combining dsa_switch_for_each_user_port() with a
dsa_port_offloads_bridge() check, or cache bridge membership within
the driver.
Add dsa_bridge_for_each_member() to express this pattern directly, and
a dsa_bridge_ports() helper that returns a bitmask of member ports,
matching the existing dsa_user_ports() and dsa_cpu_ports() helpers.
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v6: new patch
include/net/dsa.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 1c6f6c3b88e86..4f43d518f3de3 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -838,6 +838,22 @@ static inline bool dsa_port_tree_same(const struct dsa_port *a,
return a->ds->dst == b->ds->dst;
}
+#define dsa_bridge_for_each_member(_dp, _ds, _bridge) \
+ dsa_switch_for_each_user_port((_dp), (_ds)) \
+ if (dsa_port_offloads_bridge((_dp), (_bridge)))
+
+static inline u32 dsa_bridge_ports(struct dsa_switch *ds,
+ const struct dsa_bridge *bridge)
+{
+ struct dsa_port *dp;
+ u32 mask = 0;
+
+ dsa_bridge_for_each_member(dp, ds, bridge)
+ mask |= BIT(dp->index);
+
+ return mask;
+}
+
typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid,
bool is_static, void *data);
struct dsa_switch_ops {
--
2.53.0Hi 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/net-dsa-add-driver-private-pointer-to-struct-dsa_bridge/20260323-041502
base: net-next/main
patch link: https://lore.kernel.org/r/ff936ce4ad3044102c367c55e7a455ab0020e4b7.1774136876.git.daniel%40makrotopia.org
patch subject: [PATCH net-next v6 2/4] net: dsa: add bridge member iteration macro and port mask helper
config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20260325/202603250450.uUuzfUx3-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260325/202603250450.uUuzfUx3-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/202603250450.uUuzfUx3-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/net/dsa/yt921x.c:2158:1: error: conflicting types for 'dsa_bridge_ports'
dsa_bridge_ports(struct dsa_switch *ds, const struct net_device *bdev)
^
include/net/dsa.h:845:19: note: previous definition is here
static inline u32 dsa_bridge_ports(struct dsa_switch *ds,
^
>> drivers/net/dsa/yt921x.c:2213:38: error: incompatible pointer types passing 'struct net_device *' to parameter of type 'const struct dsa_bridge *' [-Werror,-Wincompatible-pointer-types]
ports_mask = dsa_bridge_ports(ds, bdev);
^~~~
include/net/dsa.h:846:33: note: passing argument to parameter 'bridge' here
const struct dsa_bridge *bridge)
^
drivers/net/dsa/yt921x.c:2285:36: error: incompatible pointer types passing 'struct net_device *' to parameter of type 'const struct dsa_bridge *' [-Werror,-Wincompatible-pointer-types]
ports_mask = dsa_bridge_ports(ds, bridge.dev);
^~~~~~~~~~
include/net/dsa.h:846:33: note: passing argument to parameter 'bridge' here
const struct dsa_bridge *bridge)
^
3 errors generated.
vim +2213 drivers/net/dsa/yt921x.c
186623f4aa724c David Yang 2025-10-17 2169
186623f4aa724c David Yang 2025-10-17 2170 static int
186623f4aa724c David Yang 2025-10-17 2171 yt921x_bridge_flags(struct yt921x_priv *priv, int port,
186623f4aa724c David Yang 2025-10-17 2172 struct switchdev_brport_flags flags)
186623f4aa724c David Yang 2025-10-17 2173 {
186623f4aa724c David Yang 2025-10-17 2174 struct yt921x_port *pp = &priv->ports[port];
186623f4aa724c David Yang 2025-10-17 2175 bool do_flush;
186623f4aa724c David Yang 2025-10-17 2176 u32 mask;
186623f4aa724c David Yang 2025-10-17 2177 int res;
186623f4aa724c David Yang 2025-10-17 2178
186623f4aa724c David Yang 2025-10-17 2179 if (flags.mask & BR_LEARNING) {
186623f4aa724c David Yang 2025-10-17 2180 bool learning = flags.val & BR_LEARNING;
186623f4aa724c David Yang 2025-10-17 2181
186623f4aa724c David Yang 2025-10-17 2182 mask = YT921X_PORT_LEARN_DIS;
186623f4aa724c David Yang 2025-10-17 2183 res = yt921x_reg_toggle_bits(priv, YT921X_PORTn_LEARN(port),
186623f4aa724c David Yang 2025-10-17 2184 mask, !learning);
186623f4aa724c David Yang 2025-10-17 2185 if (res)
186623f4aa724c David Yang 2025-10-17 2186 return res;
186623f4aa724c David Yang 2025-10-17 2187 }
186623f4aa724c David Yang 2025-10-17 2188
186623f4aa724c David Yang 2025-10-17 2189 /* BR_FLOOD, BR_MCAST_FLOOD: see the comment where ACT_UNK_ACTn_TRAP
186623f4aa724c David Yang 2025-10-17 2190 * is set
186623f4aa724c David Yang 2025-10-17 2191 */
186623f4aa724c David Yang 2025-10-17 2192
186623f4aa724c David Yang 2025-10-17 2193 /* BR_BCAST_FLOOD: we can filter bcast, but cannot trap them */
186623f4aa724c David Yang 2025-10-17 2194
186623f4aa724c David Yang 2025-10-17 2195 do_flush = false;
186623f4aa724c David Yang 2025-10-17 2196 if (flags.mask & BR_HAIRPIN_MODE) {
186623f4aa724c David Yang 2025-10-17 2197 pp->hairpin = flags.val & BR_HAIRPIN_MODE;
186623f4aa724c David Yang 2025-10-17 2198 do_flush = true;
186623f4aa724c David Yang 2025-10-17 2199 }
186623f4aa724c David Yang 2025-10-17 2200 if (flags.mask & BR_ISOLATED) {
186623f4aa724c David Yang 2025-10-17 2201 pp->isolated = flags.val & BR_ISOLATED;
186623f4aa724c David Yang 2025-10-17 2202 do_flush = true;
186623f4aa724c David Yang 2025-10-17 2203 }
186623f4aa724c David Yang 2025-10-17 2204 if (do_flush) {
186623f4aa724c David Yang 2025-10-17 2205 struct dsa_switch *ds = &priv->ds;
186623f4aa724c David Yang 2025-10-17 2206 struct dsa_port *dp = dsa_to_port(ds, port);
186623f4aa724c David Yang 2025-10-17 2207 struct net_device *bdev;
186623f4aa724c David Yang 2025-10-17 2208
186623f4aa724c David Yang 2025-10-17 2209 bdev = dsa_port_bridge_dev_get(dp);
186623f4aa724c David Yang 2025-10-17 2210 if (bdev) {
186623f4aa724c David Yang 2025-10-17 2211 u32 ports_mask;
186623f4aa724c David Yang 2025-10-17 2212
186623f4aa724c David Yang 2025-10-17 @2213 ports_mask = dsa_bridge_ports(ds, bdev);
186623f4aa724c David Yang 2025-10-17 2214 ports_mask |= priv->cpu_ports_mask;
186623f4aa724c David Yang 2025-10-17 2215 res = yt921x_bridge(priv, ports_mask);
186623f4aa724c David Yang 2025-10-17 2216 if (res)
186623f4aa724c David Yang 2025-10-17 2217 return res;
186623f4aa724c David Yang 2025-10-17 2218 }
186623f4aa724c David Yang 2025-10-17 2219 }
186623f4aa724c David Yang 2025-10-17 2220
186623f4aa724c David Yang 2025-10-17 2221 return 0;
186623f4aa724c David Yang 2025-10-17 2222 }
186623f4aa724c David Yang 2025-10-17 2223
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
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/net-dsa-add-driver-private-pointer-to-struct-dsa_bridge/20260323-041502
base: net-next/main
patch link: https://lore.kernel.org/r/ff936ce4ad3044102c367c55e7a455ab0020e4b7.1774136876.git.daniel%40makrotopia.org
patch subject: [PATCH net-next v6 2/4] net: dsa: add bridge member iteration macro and port mask helper
config: arm-randconfig-002-20260323 (https://download.01.org/0day-ci/archive/20260323/202603231807.3CRfrjTg-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project c911b8492374942bf4cfe35411e90a35d3837f6a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260323/202603231807.3CRfrjTg-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/202603231807.3CRfrjTg-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/net/dsa/yt921x.c:2158:1: error: conflicting types for 'dsa_bridge_ports'
2158 | dsa_bridge_ports(struct dsa_switch *ds, const struct net_device *bdev)
| ^
include/net/dsa.h:845:19: note: previous definition is here
845 | static inline u32 dsa_bridge_ports(struct dsa_switch *ds,
| ^
>> drivers/net/dsa/yt921x.c:2213:38: error: incompatible pointer types passing 'struct net_device *' to parameter of type 'const struct dsa_bridge *' [-Wincompatible-pointer-types]
2213 | ports_mask = dsa_bridge_ports(ds, bdev);
| ^~~~
include/net/dsa.h:846:33: note: passing argument to parameter 'bridge' here
846 | const struct dsa_bridge *bridge)
| ^
drivers/net/dsa/yt921x.c:2285:36: error: incompatible pointer types passing 'struct net_device *' to parameter of type 'const struct dsa_bridge *' [-Wincompatible-pointer-types]
2285 | ports_mask = dsa_bridge_ports(ds, bridge.dev);
| ^~~~~~~~~~
include/net/dsa.h:846:33: note: passing argument to parameter 'bridge' here
846 | const struct dsa_bridge *bridge)
| ^
3 errors generated.
vim +/dsa_bridge_ports +2158 drivers/net/dsa/yt921x.c
186623f4aa724c David Yang 2025-10-17 2156
186623f4aa724c David Yang 2025-10-17 2157 static u32
186623f4aa724c David Yang 2025-10-17 @2158 dsa_bridge_ports(struct dsa_switch *ds, const struct net_device *bdev)
186623f4aa724c David Yang 2025-10-17 2159 {
186623f4aa724c David Yang 2025-10-17 2160 struct dsa_port *dp;
186623f4aa724c David Yang 2025-10-17 2161 u32 mask = 0;
186623f4aa724c David Yang 2025-10-17 2162
186623f4aa724c David Yang 2025-10-17 2163 dsa_switch_for_each_user_port(dp, ds)
186623f4aa724c David Yang 2025-10-17 2164 if (dsa_port_offloads_bridge_dev(dp, bdev))
186623f4aa724c David Yang 2025-10-17 2165 mask |= BIT(dp->index);
186623f4aa724c David Yang 2025-10-17 2166
186623f4aa724c David Yang 2025-10-17 2167 return mask;
186623f4aa724c David Yang 2025-10-17 2168 }
186623f4aa724c David Yang 2025-10-17 2169
186623f4aa724c David Yang 2025-10-17 2170 static int
186623f4aa724c David Yang 2025-10-17 2171 yt921x_bridge_flags(struct yt921x_priv *priv, int port,
186623f4aa724c David Yang 2025-10-17 2172 struct switchdev_brport_flags flags)
186623f4aa724c David Yang 2025-10-17 2173 {
186623f4aa724c David Yang 2025-10-17 2174 struct yt921x_port *pp = &priv->ports[port];
186623f4aa724c David Yang 2025-10-17 2175 bool do_flush;
186623f4aa724c David Yang 2025-10-17 2176 u32 mask;
186623f4aa724c David Yang 2025-10-17 2177 int res;
186623f4aa724c David Yang 2025-10-17 2178
186623f4aa724c David Yang 2025-10-17 2179 if (flags.mask & BR_LEARNING) {
186623f4aa724c David Yang 2025-10-17 2180 bool learning = flags.val & BR_LEARNING;
186623f4aa724c David Yang 2025-10-17 2181
186623f4aa724c David Yang 2025-10-17 2182 mask = YT921X_PORT_LEARN_DIS;
186623f4aa724c David Yang 2025-10-17 2183 res = yt921x_reg_toggle_bits(priv, YT921X_PORTn_LEARN(port),
186623f4aa724c David Yang 2025-10-17 2184 mask, !learning);
186623f4aa724c David Yang 2025-10-17 2185 if (res)
186623f4aa724c David Yang 2025-10-17 2186 return res;
186623f4aa724c David Yang 2025-10-17 2187 }
186623f4aa724c David Yang 2025-10-17 2188
186623f4aa724c David Yang 2025-10-17 2189 /* BR_FLOOD, BR_MCAST_FLOOD: see the comment where ACT_UNK_ACTn_TRAP
186623f4aa724c David Yang 2025-10-17 2190 * is set
186623f4aa724c David Yang 2025-10-17 2191 */
186623f4aa724c David Yang 2025-10-17 2192
186623f4aa724c David Yang 2025-10-17 2193 /* BR_BCAST_FLOOD: we can filter bcast, but cannot trap them */
186623f4aa724c David Yang 2025-10-17 2194
186623f4aa724c David Yang 2025-10-17 2195 do_flush = false;
186623f4aa724c David Yang 2025-10-17 2196 if (flags.mask & BR_HAIRPIN_MODE) {
186623f4aa724c David Yang 2025-10-17 2197 pp->hairpin = flags.val & BR_HAIRPIN_MODE;
186623f4aa724c David Yang 2025-10-17 2198 do_flush = true;
186623f4aa724c David Yang 2025-10-17 2199 }
186623f4aa724c David Yang 2025-10-17 2200 if (flags.mask & BR_ISOLATED) {
186623f4aa724c David Yang 2025-10-17 2201 pp->isolated = flags.val & BR_ISOLATED;
186623f4aa724c David Yang 2025-10-17 2202 do_flush = true;
186623f4aa724c David Yang 2025-10-17 2203 }
186623f4aa724c David Yang 2025-10-17 2204 if (do_flush) {
186623f4aa724c David Yang 2025-10-17 2205 struct dsa_switch *ds = &priv->ds;
186623f4aa724c David Yang 2025-10-17 2206 struct dsa_port *dp = dsa_to_port(ds, port);
186623f4aa724c David Yang 2025-10-17 2207 struct net_device *bdev;
186623f4aa724c David Yang 2025-10-17 2208
186623f4aa724c David Yang 2025-10-17 2209 bdev = dsa_port_bridge_dev_get(dp);
186623f4aa724c David Yang 2025-10-17 2210 if (bdev) {
186623f4aa724c David Yang 2025-10-17 2211 u32 ports_mask;
186623f4aa724c David Yang 2025-10-17 2212
186623f4aa724c David Yang 2025-10-17 @2213 ports_mask = dsa_bridge_ports(ds, bdev);
186623f4aa724c David Yang 2025-10-17 2214 ports_mask |= priv->cpu_ports_mask;
186623f4aa724c David Yang 2025-10-17 2215 res = yt921x_bridge(priv, ports_mask);
186623f4aa724c David Yang 2025-10-17 2216 if (res)
186623f4aa724c David Yang 2025-10-17 2217 return res;
186623f4aa724c David Yang 2025-10-17 2218 }
186623f4aa724c David Yang 2025-10-17 2219 }
186623f4aa724c David Yang 2025-10-17 2220
186623f4aa724c David Yang 2025-10-17 2221 return 0;
186623f4aa724c David Yang 2025-10-17 2222 }
186623f4aa724c David Yang 2025-10-17 2223
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Sun, Mar 22, 2026 at 12:19:06AM +0000, Daniel Golle wrote:
> Drivers that offload bridges need to iterate over the ports that are
> members of a given bridge, for example to rebuild per-port forwarding
> bitmaps when membership changes. Currently each driver must open-code
> this by combining dsa_switch_for_each_user_port() with a
> dsa_port_offloads_bridge() check, or cache bridge membership within
> the driver.
>
> Add dsa_bridge_for_each_member() to express this pattern directly, and
> a dsa_bridge_ports() helper that returns a bitmask of member ports,
> matching the existing dsa_user_ports() and dsa_cpu_ports() helpers.
> [...]
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 1c6f6c3b88e86..4f43d518f3de3 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -838,6 +838,22 @@ static inline bool dsa_port_tree_same(const struct dsa_port *a,
> return a->ds->dst == b->ds->dst;
> }
>
> +#define dsa_bridge_for_each_member(_dp, _ds, _bridge) \
> + dsa_switch_for_each_user_port((_dp), (_ds)) \
> + if (dsa_port_offloads_bridge((_dp), (_bridge)))
> +
> +static inline u32 dsa_bridge_ports(struct dsa_switch *ds,
> + const struct dsa_bridge *bridge)
> +{
I wasn't aware of the similar and equally named function in
yt921x.c[1]
Patchwork CI has loudly uncovered it and I've learned my lesson to
always test an all-y build when changing even the most innocent
looking things which technically may affect other drivers.
I'll move that function as-is, in a dedicated commit, to
include/net/dsa.h as an inline function instead of the helper
suggested here.
A second commit will introduce the new helper macro
dsa_switch_for_each_bridge_member (modified to use (struct net_device
*) as parameter instead of (struct dsa_bridge *), and make use of it
in dsa_bridge_ports().
I'm also planning to introduce dsa_bridge_for_each_member macro in
addition to that which works on (struct dsa_bridge *) and uses
dsa_switch_for_each_bridge_member.
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/yt921x.c?id=fb78a629b4f0eb399b413f6c093a3da177b3a4eb#n2158
In code, it's going to look like this:
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b43fa7a708c72..c10f05f3e96ed 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -832,15 +832,21 @@ dsa_tree_offloads_bridge_dev(struct dsa_switch_tree *dst,
return false;
}
+#define dsa_switch_for_each_bridge_member(_dp, _ds, _bdev) \
+ dsa_switch_for_each_user_port(_dp, _ds) \
+ if (dsa_port_offloads_bridge_dev(_dp, _bdev))
+
+#define dsa_bridge_for_each_member(_dp, _ds, _db) \
+ dsa_switch_for_each_bridge_member(_dp, _ds, (_db)->dev)
+
static inline u32
dsa_bridge_ports(struct dsa_switch *ds, const struct net_device *bdev)
{
struct dsa_port *dp;
u32 mask = 0;
- dsa_switch_for_each_user_port(dp, ds)
- if (dsa_port_offloads_bridge_dev(dp, bdev))
- mask |= BIT(dp->index);
+ dsa_switch_for_each_bridge_member(dp, ds, bdev)
+ mask |= BIT(dp->index);
return mask;
}
Please let me know if any objections you may have.
On Mon, Mar 23, 2026 at 02:29:41AM +0000, Daniel Golle wrote: > I wasn't aware of the similar and equally named function in > yt921x.c[1] Yeah, I did request from the beginning for it to be moved to include/net/dsa.h, but I didn't follow through with the request (I guess I forgot): https://lore.kernel.org/netdev/20250825212357.3acgen2qezuy533y@skbuf/ > Patchwork CI has loudly uncovered it and I've learned my lesson to > always test an all-y build when changing even the most innocent > looking things which technically may affect other drivers. Since all DSA drivers are all in the same folder, technically you may be excused if you just enable all those when working on infra, instead of allyesconfig. > I'll move that function as-is, in a dedicated commit, to > include/net/dsa.h as an inline function instead of the helper > suggested here. > > A second commit will introduce the new helper macro > dsa_switch_for_each_bridge_member (modified to use (struct net_device > *) as parameter instead of (struct dsa_bridge *), and make use of it > in dsa_bridge_ports(). > > I'm also planning to introduce dsa_bridge_for_each_member macro in > addition to that which works on (struct dsa_bridge *) and uses > dsa_switch_for_each_bridge_member. I don't like the dsa_bridge_for_each_member() name. You are likely not considering cross-chip bridging, where a bridge spans multiple dsa_switch structures. That is also a serious reason why your bridge->priv design is not viable (two switches can't lay their eggs in the same basket without overwriting each other).
On Mon, Mar 23, 2026 at 06:31:04PM +0200, Vladimir Oltean wrote: > On Mon, Mar 23, 2026 at 02:29:41AM +0000, Daniel Golle wrote: > > I'm also planning to introduce dsa_bridge_for_each_member macro in > > addition to that which works on (struct dsa_bridge *) and uses > > dsa_switch_for_each_bridge_member. > > I don't like the dsa_bridge_for_each_member() name. You are likely > not considering cross-chip bridging, where a bridge spans multiple > dsa_switch structures. That is also a serious reason why your > bridge->priv design is not viable (two switches can't lay their eggs > in the same basket without overwriting each other). dsa_bridge_for_each_local_member() ? dsa_switch_for_each_dsa_bridge_member() (but that's too close to dsa_switch_for_each_bridge_member imho) I was planning to have both, dsa_switch and dsa_bridge as parameters, of course, limiting the scope to a single dsa_switch.
On Tue, Mar 24, 2026 at 11:31:30PM +0000, Daniel Golle wrote: > On Mon, Mar 23, 2026 at 06:31:04PM +0200, Vladimir Oltean wrote: > > On Mon, Mar 23, 2026 at 02:29:41AM +0000, Daniel Golle wrote: > > > I'm also planning to introduce dsa_bridge_for_each_member macro in > > > addition to that which works on (struct dsa_bridge *) and uses > > > dsa_switch_for_each_bridge_member. > > > > I don't like the dsa_bridge_for_each_member() name. You are likely > > not considering cross-chip bridging, where a bridge spans multiple > > dsa_switch structures. That is also a serious reason why your > > bridge->priv design is not viable (two switches can't lay their eggs > > in the same basket without overwriting each other). > > dsa_bridge_for_each_local_member() ? > > dsa_switch_for_each_dsa_bridge_member() > (but that's too close to dsa_switch_for_each_bridge_member imho) > > I was planning to have both, dsa_switch and dsa_bridge as parameters, > of course, limiting the scope to a single dsa_switch. I'm not sure that we do need two iterators. Just dsa_switch_for_each_bridge_member() should be fine.
On Wed, Mar 25, 2026 at 10:34:27AM +0200, Vladimir Oltean wrote: > On Tue, Mar 24, 2026 at 11:31:30PM +0000, Daniel Golle wrote: > > On Mon, Mar 23, 2026 at 06:31:04PM +0200, Vladimir Oltean wrote: > > > On Mon, Mar 23, 2026 at 02:29:41AM +0000, Daniel Golle wrote: > > > > I'm also planning to introduce dsa_bridge_for_each_member macro in > > > > addition to that which works on (struct dsa_bridge *) and uses > > > > dsa_switch_for_each_bridge_member. > > > > > > I don't like the dsa_bridge_for_each_member() name. You are likely > > > not considering cross-chip bridging, where a bridge spans multiple > > > dsa_switch structures. That is also a serious reason why your > > > bridge->priv design is not viable (two switches can't lay their eggs > > > in the same basket without overwriting each other). > > > > dsa_bridge_for_each_local_member() ? > > > > dsa_switch_for_each_dsa_bridge_member() > > (but that's too close to dsa_switch_for_each_bridge_member imho) > > > > I was planning to have both, dsa_switch and dsa_bridge as parameters, > > of course, limiting the scope to a single dsa_switch. > > I'm not sure that we do need two iterators. Just > dsa_switch_for_each_bridge_member() should be fine. Just for convenience. In my case, all callers always have a reference to 'struct dsa_bridge *', so instead of having the callers dereference 'struct net_device *' from that, have a macro that does it for me.
On Mon, Mar 23, 2026 at 06:31:04PM +0200, Vladimir Oltean wrote: > On Mon, Mar 23, 2026 at 02:29:41AM +0000, Daniel Golle wrote: > > I wasn't aware of the similar and equally named function in > > yt921x.c[1] > > Yeah, I did request from the beginning for it to be moved to > include/net/dsa.h, but I didn't follow through with the request (I guess > I forgot): > https://lore.kernel.org/netdev/20250825212357.3acgen2qezuy533y@skbuf/ > > > Patchwork CI has loudly uncovered it and I've learned my lesson to > > always test an all-y build when changing even the most innocent > > looking things which technically may affect other drivers. > > Since all DSA drivers are all in the same folder, technically you may be > excused if you just enable all those when working on infra, instead of > allyesconfig. > > > I'll move that function as-is, in a dedicated commit, to > > include/net/dsa.h as an inline function instead of the helper > > suggested here. > > > > A second commit will introduce the new helper macro > > dsa_switch_for_each_bridge_member (modified to use (struct net_device > > *) as parameter instead of (struct dsa_bridge *), and make use of it > > in dsa_bridge_ports(). > > > > I'm also planning to introduce dsa_bridge_for_each_member macro in > > addition to that which works on (struct dsa_bridge *) and uses > > dsa_switch_for_each_bridge_member. > > I don't like the dsa_bridge_for_each_member() name. You are likely > not considering cross-chip bridging, where a bridge spans multiple > dsa_switch structures. That is also a serious reason why your > bridge->priv design is not viable (two switches can't lay their eggs > in the same basket without overwriting each other). That means the driver *does* have to track a mapping between DSA bridges and hardware/firmware bridges on that specific switch somehow. Or is there another existing structure in DSA I'm not aware of and which could be used to implement Paolo's suggestion: https://lore.kernel.org/netdev/fe46d64b-1b12-48b6-b663-e7d5122b7b8a@redhat.com/
On Mon, Mar 23, 2026 at 04:45:18PM +0000, Daniel Golle wrote: > That means the driver *does* have to track a mapping between DSA > bridges and hardware/firmware bridges on that specific switch somehow. That's correct. From DSA's perspective, it's not an mxlbridge, it's just a bridge with at least one DSA switch port under it, which is given a kernel-wide identifier. The ports can hold a single reference to the common bridge, but the common bridge cannot hold a single back-reference to individual ports. And holding multiple references would severely complicate the implementation. Even if you don't implement accelerated cross-switch bridging, you can still have two mxl ports belonging to different switches that are both put by the user under the same bridge. And both switch driver instances would want to allocate a potentially different bridge ID, and surely maintain a different portmap, for that same bridge. > Or is there another existing structure in DSA I'm not aware of and > which could be used to implement Paolo's suggestion: > > https://lore.kernel.org/netdev/fe46d64b-1b12-48b6-b663-e7d5122b7b8a@redhat.com/ The API was intended so that you could either use the bridge.num directly, transform it through some linear formula or as an index into a privately maintained array of data structures with switch-specific meaning. I didn't question why you maintain a parallel list of dynamically allocated mxlbridge structures instead of just having an array of MXL862XX_MAX_BRIDGES * sizeof(u16) for the bridge identifiers, plus maintain a separate portmap instead of walking through the DSA data structures, but now that Paolo brought it up: why? I see in v6 you're halfway there with the conversion, you got rid of the portmap and you could just use the array for the rest.
On Mon, Mar 23, 2026 at 11:15:36PM +0200, Vladimir Oltean wrote: > On Mon, Mar 23, 2026 at 04:45:18PM +0000, Daniel Golle wrote: > > That means the driver *does* have to track a mapping between DSA > > bridges and hardware/firmware bridges on that specific switch somehow. > > That's correct. From DSA's perspective, it's not an mxlbridge, it's just > a bridge with at least one DSA switch port under it, which is given a > kernel-wide identifier. The ports can hold a single reference to the > common bridge, but the common bridge cannot hold a single back-reference > to individual ports. And holding multiple references would severely > complicate the implementation. > > Even if you don't implement accelerated cross-switch bridging, you can > still have two mxl ports belonging to different switches that are both > put by the user under the same bridge. And both switch driver instances > would want to allocate a potentially different bridge ID, and surely > maintain a different portmap, for that same bridge. > > > Or is there another existing structure in DSA I'm not aware of and > > which could be used to implement Paolo's suggestion: > > > > https://lore.kernel.org/netdev/fe46d64b-1b12-48b6-b663-e7d5122b7b8a@redhat.com/ > > The API was intended so that you could either use the bridge.num directly, > transform it through some linear formula or as an index into a privately > maintained array of data structures with switch-specific meaning. > > I didn't question why you maintain a parallel list of dynamically > allocated mxlbridge structures instead of just having an array of > MXL862XX_MAX_BRIDGES * sizeof(u16) for the bridge identifiers, plus > maintain a separate portmap instead of walking through the DSA data > structures, but now that Paolo brought it up: why? The firmware requires bridges to be allocated, and the bridge allocation firmware API returns the ID we got to use. Maintaing a simple array ((ds->max_num_bridges + 1) * sizeof(u16)) to map DSA bridge num to firmware bridge ID in the driver works fine, of course. Regarding the request not to maintain a separate portmap for each bridge (which was my initial approach) but instead walk through DSA data: My guess was the intention is to avoid duplciating data, as in: if there is only one copy, we can be sure it's not stale or out-of-sync...? But that's just my guess, only Paolo can tell his reasons for having requested that. > > I see in v6 you're halfway there with the conversion, you got rid of the > portmap and you could just use the array for the rest. Ack. I'll do that and post v7.
© 2016 - 2026 Red Hat, Inc.