mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net-next v3 0/2] net: dsa: motorcomm: Add devlink info and resources
@ 2026-11-09 16:30 David Yang
  2026-11-09 16:30 ` [PATCH net-next v3 1/2] net: dsa: motorcomm: Report error when FDB fails to add David Yang
  2026-11-09 16:30 ` [PATCH net-next v3 2/2] net: dsa: motorcomm: Add devlink info and resources David Yang
  0 siblings, 2 replies; 5+ messages in thread
From: David Yang @ 2026-11-09 16:30 UTC (permalink / raw)
  To: netdev
  Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel

Add basic devlink support: expose the ASIC id through .devlink_info_get,
and register devlink resources for the hardware blocks, reporting ACL,
FDB and meter usage.

v3:
 - remove the buggy sub-resources
v2: https://lore.kernel.org/r/20261108094512.1285796-1-mmyangfl@gmail.com
 - fix issues reported by sashiko
v1: https://lore.kernel.org/r/20261106184811.350700-1-mmyangfl@gmail.com

David Yang (2):
  net: dsa: motorcomm: Report error when FDB fails to add
  net: dsa: motorcomm: Add devlink info and resources

 drivers/net/dsa/motorcomm/Makefile  |   1 +
 drivers/net/dsa/motorcomm/chip.c    |  46 +++++++--
 drivers/net/dsa/motorcomm/chip.h    |   3 +
 drivers/net/dsa/motorcomm/devlink.c | 148 ++++++++++++++++++++++++++++
 drivers/net/dsa/motorcomm/devlink.h |  18 ++++
 5 files changed, 209 insertions(+), 7 deletions(-)
 create mode 100644 drivers/net/dsa/motorcomm/devlink.c
 create mode 100644 drivers/net/dsa/motorcomm/devlink.h

-- 
2.53.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net-next v3 1/2] net: dsa: motorcomm: Report error when FDB fails to add
  2026-11-09 16:30 [PATCH net-next v3 0/2] net: dsa: motorcomm: Add devlink info and resources David Yang
@ 2026-11-09 16:30 ` David Yang
  2026-11-11 22:41   ` netdev-bot+sashiko
  2026-11-09 16:30 ` [PATCH net-next v3 2/2] net: dsa: motorcomm: Add devlink info and resources David Yang
  1 sibling, 1 reply; 5+ messages in thread
From: David Yang @ 2026-11-09 16:30 UTC (permalink / raw)
  To: netdev
  Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel

fdb_add() ignored any errors returned by the hardware. Return error for
that case.

Not carrying a fix tag: filling up 4096-entry table with static entries
is just asking for trouble.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/dsa/motorcomm/chip.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
index 33e5fc127244..a7a9a7439304 100644
--- a/drivers/net/dsa/motorcomm/chip.c
+++ b/drivers/net/dsa/motorcomm/chip.c
@@ -725,7 +725,13 @@ yt921x_fdb_add(struct yt921x_priv *priv, const unsigned char *addr, u16 vid,
 	if (res)
 		return res;
 
-	return yt921x_fdb_wait(priv, &val);
+	res = yt921x_fdb_wait(priv, &val);
+	if (res)
+		return res;
+	if (val & YT921X_FDB_RESULT_NOTFOUND)
+		return -ENOSPC;
+
+	return 0;
 }
 
 static int
-- 
2.53.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net-next v3 2/2] net: dsa: motorcomm: Add devlink info and resources
  2026-11-09 16:30 [PATCH net-next v3 0/2] net: dsa: motorcomm: Add devlink info and resources David Yang
  2026-11-09 16:30 ` [PATCH net-next v3 1/2] net: dsa: motorcomm: Report error when FDB fails to add David Yang
@ 2026-11-09 16:30 ` David Yang
  2026-11-11 22:41   ` netdev-bot+sashiko
  1 sibling, 1 reply; 5+ messages in thread
From: David Yang @ 2026-11-09 16:30 UTC (permalink / raw)
  To: netdev
  Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel

Expose the ASIC model via devlink info (ASIC_ID), and register devlink
resources for ACLs, FDB and meters.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/dsa/motorcomm/Makefile  |   1 +
 drivers/net/dsa/motorcomm/chip.c    |  38 +++++--
 drivers/net/dsa/motorcomm/chip.h    |   3 +
 drivers/net/dsa/motorcomm/devlink.c | 148 ++++++++++++++++++++++++++++
 drivers/net/dsa/motorcomm/devlink.h |  18 ++++
 5 files changed, 202 insertions(+), 6 deletions(-)
 create mode 100644 drivers/net/dsa/motorcomm/devlink.c
 create mode 100644 drivers/net/dsa/motorcomm/devlink.h

diff --git a/drivers/net/dsa/motorcomm/Makefile b/drivers/net/dsa/motorcomm/Makefile
index 9eaa59f699e8..0b31d815ce43 100644
--- a/drivers/net/dsa/motorcomm/Makefile
+++ b/drivers/net/dsa/motorcomm/Makefile
@@ -2,6 +2,7 @@
 obj-$(CONFIG_NET_DSA_YT921X) += yt921x.o
 yt921x-objs := chip.o
 yt921x-$(CONFIG_NET_DSA_YT921X_LEDS) += leds.o
+yt921x-objs += devlink.o
 yt921x-objs += mdio_bus.o
 yt921x-objs += mib.o
 yt921x-objs += pcs-921x.o
diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
index a7a9a7439304..b441d82f6d99 100644
--- a/drivers/net/dsa/motorcomm/chip.c
+++ b/drivers/net/dsa/motorcomm/chip.c
@@ -26,6 +26,7 @@
 #include <net/ieee8021q.h>
 
 #include "chip.h"
+#include "devlink.h"
 #include "leds.h"
 #include "mdio_bus.h"
 #include "mib.h"
@@ -731,6 +732,7 @@ yt921x_fdb_add(struct yt921x_priv *priv, const unsigned char *addr, u16 vid,
 	if (val & YT921X_FDB_RESULT_NOTFOUND)
 		return -ENOSPC;
 
+	priv->fdb_static_cnt++;
 	return 0;
 }
 
@@ -761,18 +763,27 @@ yt921x_fdb_leave(struct yt921x_priv *priv, const unsigned char *addr,
 	ctrl2 = val2 & ~YT921X_FDB_IO2_EGR_PORTS(ports_mask);
 	if (ctrl2 == val2)
 		return 0;
+
+	res = yt921x_reg_read(priv, YT921X_FDB_OUT1, &ctrl1);
+	if (res)
+		return res;
+
 	if (!(ctrl2 & YT921X_FDB_IO2_EGR_PORTS_M)) {
 		ctrl = YT921X_FDB_OP_OP_DEL | YT921X_FDB_OP_START;
 		res = yt921x_reg_write(priv, YT921X_FDB_OP, ctrl);
 		if (res)
 			return res;
 
-		return yt921x_fdb_wait(priv, &val);
-	}
+		res = yt921x_fdb_wait(priv, &val);
+		if (res)
+			return res;
 
-	res = yt921x_reg_read(priv, YT921X_FDB_OUT1, &ctrl1);
-	if (res)
-		return res;
+		if ((ctrl1 & YT921X_FDB_IO1_STATUS_M) ==
+		    YT921X_FDB_IO1_STATUS_STATIC &&
+		    !WARN_ON(!priv->fdb_static_cnt))
+			priv->fdb_static_cnt--;
+		return 0;
+	}
 
 	return yt921x_fdb_add_index_in12(priv, index, ctrl1, ctrl2);
 }
@@ -809,7 +820,13 @@ yt921x_fdb_join(struct yt921x_priv *priv, const unsigned char *addr, u16 vid,
 	if (ctrl1 == val1 && ctrl2 == val2)
 		return 0;
 
-	return yt921x_fdb_add_index_in12(priv, index, ctrl1, ctrl2);
+	res = yt921x_fdb_add_index_in12(priv, index, ctrl1, ctrl2);
+	if (res)
+		return res;
+
+	if (ctrl1 != val1)
+		priv->fdb_static_cnt++;
+	return 0;
 }
 
 static int
@@ -2154,6 +2171,8 @@ static void yt921x_dsa_teardown(struct dsa_switch *ds)
 
 	yt921x_leds_remove(priv);
 #endif
+
+	dsa_devlink_resources_unregister(ds);
 }
 
 static int yt921x_chip_detect(struct yt921x_priv *priv)
@@ -2481,6 +2500,8 @@ static int yt921x_dsa_setup(struct dsa_switch *ds)
 			  pp->pcs.supported_interfaces);
 	}
 
+	priv->fdb_static_cnt = 0;
+
 	mutex_lock(&priv->reg_lock);
 	res = yt921x_chip_setup(priv);
 	mutex_unlock(&priv->reg_lock);
@@ -2488,6 +2509,10 @@ static int yt921x_dsa_setup(struct dsa_switch *ds)
 	if (res)
 		return res;
 
+	res = yt921x_devlink_setup(priv);
+	if (res)
+		return res;
+
 #if IS_ENABLED(CONFIG_NET_DSA_YT921X_LEDS)
 	res = yt921x_leds_setup(priv);
 	if (res)
@@ -2576,6 +2601,7 @@ static const struct dsa_switch_ops yt921x_dsa_switch_ops = {
 	.port_add_dscp_prio	= yt921x_dsa_port_add_dscp_prio,
 #endif
 	/* chip */
+	.devlink_info_get	= yt921x_dsa_devlink_info_get,
 	.teardown		= yt921x_dsa_teardown,
 	.setup			= yt921x_dsa_setup,
 };
diff --git a/drivers/net/dsa/motorcomm/chip.h b/drivers/net/dsa/motorcomm/chip.h
index fc315f1d8f87..5408ec268ba0 100644
--- a/drivers/net/dsa/motorcomm/chip.h
+++ b/drivers/net/dsa/motorcomm/chip.h
@@ -248,6 +248,7 @@ enum yt921x_app_selector {
 #define   YT921X_PORT_LEARN_MODE_AUTO			YT921X_PORT_LEARN_MODE(0)
 #define   YT921X_PORT_LEARN_MODE_AUTO_AND_COPY		YT921X_PORT_LEARN_MODE(1)
 #define   YT921X_PORT_LEARN_MODE_CPU_CONTROL		YT921X_PORT_LEARN_MODE(2)
+#define YT921X_LEARN_UCAST_CNT		0x18043c
 #define YT921X_AGEING			0x180440
 #define  YT921X_AGEING_INTERVAL_M		GENMASK(15, 0)
 #define YT921X_FDB_IN0			0x180454
@@ -312,6 +313,7 @@ enum yt921x_app_selector {
 #define   YT921X_FDB_IO2_PRIO(x)			FIELD_PREP(YT921X_FDB_IO2_PRIO_M, (x))
 #define  YT921X_FDB_IO2_NEW_VID_M		GENMASK(11, 0)
 #define   YT921X_FDB_IO2_NEW_VID(x)			FIELD_PREP(YT921X_FDB_IO2_NEW_VID_M, (x))
+#define YT921X_LEARN_MCAST_CNT		0x180504
 #define YT921X_FILTER_UNK_UCAST		0x180508
 #define YT921X_FILTER_UNK_MCAST		0x18050c
 #define YT921X_FILTER_MCAST		0x180510
@@ -525,6 +527,7 @@ struct yt921x_priv {
 	struct yt921x_port ports[YT921X_PORT_NUM];
 
 	u16 eee_ports_mask;
+	unsigned short fdb_static_cnt;
 
 	DECLARE_BITMAP(meters_map, YT921X_METER_NUM);
 
diff --git a/drivers/net/dsa/motorcomm/devlink.c b/drivers/net/dsa/motorcomm/devlink.c
new file mode 100644
index 000000000000..529d2decc403
--- /dev/null
+++ b/drivers/net/dsa/motorcomm/devlink.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2026 David Yang
+ */
+
+#include "chip.h"
+#include "devlink.h"
+#include "smi.h"
+#include "tc.h"
+
+int
+yt921x_dsa_devlink_info_get(struct dsa_switch *ds, struct devlink_info_req *req,
+			    struct netlink_ext_ack *extack)
+{
+	struct yt921x_priv *priv = dsa_to_yt921x_priv(ds);
+	int res;
+
+	if (!priv->info)
+		return -ENODEV;
+
+	res = devlink_info_version_fixed_put(req,
+					     DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
+					     priv->info->name);
+	if (res)
+		return res;
+
+	return 0;
+}
+
+static u64 yt921x_devlink_occ_get_acl(void *ctx)
+{
+	struct yt921x_priv *priv = ctx;
+	unsigned int count = 0;
+
+	mutex_lock(&priv->reg_lock);
+	for (unsigned int i = 0; i < YT921X_ACL_BLK_NUM; i++)
+		count += hweight8(priv->acl_masks[i]);
+	mutex_unlock(&priv->reg_lock);
+
+	return count;
+}
+
+static u64 yt921x_devlink_occ_get_fdb(void *ctx)
+{
+	struct yt921x_priv *priv = ctx;
+	unsigned int count = 0;
+	u32 val;
+	int res;
+
+	mutex_lock(&priv->reg_lock);
+
+	res = yt921x_reg_read(priv, YT921X_LEARN_UCAST_CNT, &val);
+	if (res)
+		goto end;
+	count += val;
+
+	res = yt921x_reg_read(priv, YT921X_LEARN_MCAST_CNT, &val);
+	if (res)
+		goto end;
+	count += val;
+
+	count += priv->fdb_static_cnt;
+
+end:
+	mutex_unlock(&priv->reg_lock);
+
+	if (res)
+		return 0;
+	return count;
+}
+
+static u64 yt921x_devlink_occ_get_meter(void *ctx)
+{
+	struct yt921x_priv *priv = ctx;
+	u64 count;
+
+	mutex_lock(&priv->reg_lock);
+	count = bitmap_weight(priv->meters_map, YT921X_METER_NUM);
+	mutex_unlock(&priv->reg_lock);
+
+	return count;
+}
+
+enum yt921x_devlink_resource_id {
+	YT921X_DEVLINK_RESOURCE_ID_NONE,  /* DEVLINK_RESOURCE_ID_PARENT_TOP */
+	YT921X_DEVLINK_RESOURCE_ID_ACL,
+	YT921X_DEVLINK_RESOURCE_ID_FDB,
+	YT921X_DEVLINK_RESOURCE_ID_METER,
+};
+
+struct yt921x_devlink_resource {
+	const char *name;
+	devlink_resource_occ_get_t *occ_get;
+	unsigned short size;
+	unsigned char id;
+	unsigned char parent_id;
+};
+
+static const struct yt921x_devlink_resource yt921x_devlink_resources[] = {
+	{
+		"ACL", yt921x_devlink_occ_get_acl,
+		YT921X_ACL_NUM,
+		YT921X_DEVLINK_RESOURCE_ID_ACL,
+		DEVLINK_RESOURCE_ID_PARENT_TOP,
+	},
+	{
+		"FDB", yt921x_devlink_occ_get_fdb,
+		YT921X_FDB_NUM,
+		YT921X_DEVLINK_RESOURCE_ID_FDB,
+		DEVLINK_RESOURCE_ID_PARENT_TOP,
+	},
+	{
+		"meters", yt921x_devlink_occ_get_meter,
+		YT921X_METER_NUM,
+		YT921X_DEVLINK_RESOURCE_ID_METER,
+		DEVLINK_RESOURCE_ID_PARENT_TOP,
+	},
+};
+
+int yt921x_devlink_setup(struct yt921x_priv *priv)
+{
+	struct dsa_switch *ds = &priv->ds;
+	int res;
+
+	for (unsigned int i = 0; i < ARRAY_SIZE(yt921x_devlink_resources);
+	     i++) {
+		struct devlink_resource_size_params size_params;
+		const struct yt921x_devlink_resource *r;
+
+		r = &yt921x_devlink_resources[i];
+
+		devlink_resource_size_params_init(&size_params, r->size,
+						  r->size, 1,
+						  DEVLINK_RESOURCE_UNIT_ENTRY);
+
+		res = dsa_devlink_resource_register(ds, r->name, r->size, r->id,
+						    r->parent_id, &size_params);
+		if (res) {
+			dsa_devlink_resources_unregister(ds);
+			return res;
+		}
+
+		dsa_devlink_resource_occ_get_register(ds, r->id, r->occ_get,
+						      priv);
+	}
+
+	return 0;
+}
diff --git a/drivers/net/dsa/motorcomm/devlink.h b/drivers/net/dsa/motorcomm/devlink.h
new file mode 100644
index 000000000000..184b9f05742e
--- /dev/null
+++ b/drivers/net/dsa/motorcomm/devlink.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2026 David Yang
+ */
+
+#ifndef _YT_DEVLINK_H
+#define _YT_DEVLINK_H
+
+#include <net/dsa.h>
+
+struct yt921x_priv;
+
+int
+yt921x_dsa_devlink_info_get(struct dsa_switch *ds, struct devlink_info_req *req,
+			    struct netlink_ext_ack *extack);
+int yt921x_devlink_setup(struct yt921x_priv *priv);
+
+#endif
-- 
2.53.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v3 1/2] net: dsa: motorcomm: Report error when FDB fails to add
  2026-11-09 16:30 ` [PATCH net-next v3 1/2] net: dsa: motorcomm: Report error when FDB fails to add David Yang
@ 2026-11-11 22:41   ` netdev-bot+sashiko
  0 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-11-11 22:41 UTC (permalink / raw)
  To: mmyangfl
  Cc: netdev, andrew, olteanv, davem, edumazet, kuba, pabeni, linux-kernel

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The code change is right, but the commit message gives a wrong
  reason for leaving out the Fixes: tag.
- [Medium] yt921x_fdb_add() in drivers/net/dsa/motorcomm/chip.c now
  treats only YT921X_FDB_RESULT_NOTFOUND (bit 14) as a failure of the…

--

net: dsa: motorcomm: Report error when FDB fails to add

This patch changes yt921x_fdb_add() to check the FDB result register
after the ADD operation finishes. If the hardware reports
YT921X_FDB_RESULT_NOTFOUND, the function now returns -ENOSPC instead of
success.

> Not carrying a fix tag: filling up 4096-entry table with static entries
> is just asking for trouble.

[Severity: Medium]
Is this reasoning accurate? Before this patch, a hardware ADD failure was
reported as success. DSA and the bridge then treated the entry as
offloaded even though the switch never installed it. That looks like it
dates back to 186623f4aa72 ("net: dsa: yt921x: Add support for Motorcomm
YT921x").

Admin-added static FDB entries are not the only way to fill the table.
yt921x_dsa_port_mdb_add() goes through the same path, and every entry it
writes is YT921X_FDB_IO1_STATUS_STATIC:

yt921x_dsa_port_mdb_add()
    yt921x_fdb_join()
        yt921x_fdb_add()

Bridge IGMP/MLD snooping is enabled by default. It allows up to
BR_MULTICAST_DEFAULT_HASH_MAX MDB entries, and that limit is also 4096:

net/bridge/br_private.h:
#define BR_MULTICAST_DEFAULT_HASH_MAX 4096

Those MDB entries are offloaded via switchdev into port_mdb_add. So it
seems hosts on the LAN could fill the shared hardware table just by
sending group joins. The same table also holds unicast static FDB
entries and host entries.

Also, the ADD is issued without YT921X_FDB_OP_MODE_INDEX, so the
hardware places the entry by MAC+FID. If the table uses
limited-associativity hash buckets, could ADD fail before all 4096
entries are in use? This depends on the datasheet.

Should this carry:

Fixes: 186623f4aa72 ("net: dsa: yt921x: Add support for Motorcomm YT921x")

so that stable kernels pick it up? If not, could the commit message
describe the failure conditions more precisely, and mention that MDB
entries are affected as well as unicast FDB?

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index 33e5fc127244d..a7a9a74393045 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c
> @@ -725,7 +725,13 @@ yt921x_fdb_add(struct yt921x_priv *priv, const unsigned char *addr, u16 vid,
>  	if (res)
>  		return res;
>  
> -	return yt921x_fdb_wait(priv, &val);
> +	res = yt921x_fdb_wait(priv, &val);
> +	if (res)
> +		return res;
> +	if (val & YT921X_FDB_RESULT_NOTFOUND)
> +		return -ENOSPC;
> +
> +	return 0;

[Severity: Medium]
What should happen when YT921X_FDB_RESULT_OVERWRITED is set here? The
result register defines it in chip.h next to NOTFOUND:

#define  YT921X_FDB_RESULT_NOTFOUND		BIT(14)
#define  YT921X_FDB_RESULT_OVERWRITED		BIT(13)

Nothing in the driver reads it.

yt921x_fdb_join() only calls yt921x_fdb_add() after yt921x_fdb_has() has
reported the {MAC, FID} key absent, with reg_lock held:

	res = yt921x_fdb_has(priv, addr, vid, &index);
	if (res)
		return res;
	if (index >= YT921X_FDB_NUM)
		return yt921x_fdb_add(priv, addr, vid, ports_mask);

So an overwrite here probably means some other entry in the same bucket
was replaced. The exception would be a dynamic entry for this key being
learned between the two operations.

If the hardware can evict an earlier static FDB or MDB entry this way,
this would return 0 while that entry silently disappears from the
switch. A later yt921x_fdb_leave() for it would then see NOTFOUND and
also return 0.

Could OVERWRITED mean a static entry was evicted? If it only ever means
a dynamic entry was replaced, a short comment saying why it is safe to
ignore might help.

>  }
>  
>  static int

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109163117.1792478-1-mmyangfl%40gmail.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v3 2/2] net: dsa: motorcomm: Add devlink info and resources
  2026-11-09 16:30 ` [PATCH net-next v3 2/2] net: dsa: motorcomm: Add devlink info and resources David Yang
@ 2026-11-11 22:41   ` netdev-bot+sashiko
  0 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-11-11 22:41 UTC (permalink / raw)
  To: mmyangfl
  Cc: netdev, andrew, olteanv, davem, edumazet, kuba, pabeni, linux-kernel

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The FDB occupancy that yt921x_devlink_occ_get_fdb()
  (drivers/net/dsa/motorcomm/devlink.c) reports for the "FDB" devlink…
- [Low] The patch adds devlink info (DEVLINK_INFO_VERSION_GENERIC_ASIC_ID
  from yt921x_dsa_devlink_info_get()) and three driver-specific devlink…
- [Low] The commit message says only "Expose the ASIC model via devlink
  info (ASIC_ID), and register devlink resources for ACLs, FDB and…

--

net: dsa: motorcomm: Add devlink info and resources

This commit adds devlink info to the yt921x DSA driver, reporting the ASIC
model as ASIC_ID. It also registers devlink resources for the ACL, FDB and
meter tables. To compute FDB occupancy it adds a software counter of
static FDB entries.

> Expose the ASIC model via devlink info (ASIC_ID), and register devlink
> resources for ACLs, FDB and meters.

[Severity: Low]
Could the commit message also describe the changes to the FDB programming
paths?

The patch adds priv->fdb_static_cnt and updates it in yt921x_fdb_add(),
yt921x_fdb_join() and yt921x_fdb_leave(). These run for every bridge
fdb/mdb add and delete.

yt921x_fdb_leave() is also restructured. YT921X_FDB_OUT1 is now read
before the DEL operation, not only on the rewrite path. A
WARN_ON(!priv->fdb_static_cnt) is also added on the delete path.

It would help to explain why FDB occupancy is computed as
LEARN_UCAST_CNT + LEARN_MCAST_CNT + a software static count. That formula
assumes the hardware counters only include learned entries.

> diff --git a/drivers/net/dsa/motorcomm/devlink.c b/drivers/net/dsa/motorcomm/devlink.c
> new file mode 100644
> index 0000000000000..529d2decc4036
> --- /dev/null
> +++ b/drivers/net/dsa/motorcomm/devlink.c

[ ... ]

> +static u64 yt921x_devlink_occ_get_fdb(void *ctx)
> +{
> +	struct yt921x_priv *priv = ctx;
> +	unsigned int count = 0;
> +	u32 val;
> +	int res;
> +
> +	mutex_lock(&priv->reg_lock);
> +
> +	res = yt921x_reg_read(priv, YT921X_LEARN_UCAST_CNT, &val);
> +	if (res)
> +		goto end;
> +	count += val;
> +
> +	res = yt921x_reg_read(priv, YT921X_LEARN_MCAST_CNT, &val);
> +	if (res)
> +		goto end;
> +	count += val;
> +
> +	count += priv->fdb_static_cnt;

[Severity: Medium]
Can this report more FDB occupancy than actually exists, possibly even
more than the advertised size of YT921X_FDB_NUM?

The new YT921X_LEARN_UCAST_CNT and YT921X_LEARN_MCAST_CNT definitions in
chip.h have no field mask. Neighbouring registers do, for example
YT921X_AGEING_INTERVAL_M. The full 32-bit value is added here, so any
reserved, limit or flag bits next to the count would end up in the
reported occupancy. Is the whole register the counter?

The sum also assumes the hardware learn counters and fdb_static_cnt never
overlap. In yt921x_fdb_join(), an existing hardware-learned entry is
rewritten as static with an index-mode ADD:

	ctrl1 = val1 & ~YT921X_FDB_IO1_STATUS_M;
	ctrl1 |= YT921X_FDB_IO1_STATUS_STATIC;
	...
	if (ctrl1 != val1)
		priv->fdb_static_cnt++;

Is that entry counted twice if the hardware learn counter does not drop
when the CPU rewrites the status?

The same could happen in yt921x_fdb_add():

1. yt921x_fdb_has() returns not found.
2. The learning engine then learns the same MAC/FID. reg_lock only
   serializes CPU access, not hardware learning.
3. The hash-mode ADD overwrites that entry, but fdb_static_cnt++ still
   runs.

MDB entries are also added as static through yt921x_fdb_join() from
port_mdb_add, so they are counted in fdb_static_cnt. Source MAC learning
never creates multicast entries. If LEARN_MCAST_CNT counts all multicast
table entries, would every MDB entry be counted twice?

> +
> +end:
> +	mutex_unlock(&priv->reg_lock);

[ ... ]

> +static const struct yt921x_devlink_resource yt921x_devlink_resources[] = {
> +	{
> +		"ACL", yt921x_devlink_occ_get_acl,
> +		YT921X_ACL_NUM,
> +		YT921X_DEVLINK_RESOURCE_ID_ACL,
> +		DEVLINK_RESOURCE_ID_PARENT_TOP,
> +	},
> +	{
> +		"FDB", yt921x_devlink_occ_get_fdb,
> +		YT921X_FDB_NUM,
> +		YT921X_DEVLINK_RESOURCE_ID_FDB,
> +		DEVLINK_RESOURCE_ID_PARENT_TOP,
> +	},
> +	{
> +		"meters", yt921x_devlink_occ_get_meter,
> +		YT921X_METER_NUM,
> +		YT921X_DEVLINK_RESOURCE_ID_METER,
> +		DEVLINK_RESOURCE_ID_PARENT_TOP,
> +	},
> +};

[Severity: Low]
Should this come with a driver page under Documentation/networking/devlink/?

Documentation/networking/devlink/index.rst says each driver that
implements devlink is expected to document what parameters, info
versions, and other features it supports.

There is no yt921x entry there yet. For comparison, mv88e6xxx.rst
documents the devlink features of another DSA driver.

The "ACL", "FDB" and "meters" resource names and the ASIC_ID info version
are new user-visible interfaces. For example, "FDB" also includes MDB
entries because the hardware table is shared.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109163117.1792478-1-mmyangfl%40gmail.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-11 22:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-09 16:30 [PATCH net-next v3 0/2] net: dsa: motorcomm: Add devlink info and resources David Yang
2026-11-09 16:30 ` [PATCH net-next v3 1/2] net: dsa: motorcomm: Report error when FDB fails to add David Yang
2026-11-11 22:41   ` netdev-bot+sashiko
2026-11-09 16:30 ` [PATCH net-next v3 2/2] net: dsa: motorcomm: Add devlink info and resources David Yang
2026-11-11 22:41   ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®