[PATCH net-next 0/6] net: dsa: hellcreek: Add support for dumping tables

[PATCH net-next 0/6] net: dsa: hellcreek: Add support for dumping tables

[Kurt Kanzenbach]

Hi,

add support for dumping the VLAN and FDB table via devlink. As the driver uses
internal VLANs and static FDB entries, this is a useful debugging feature. Also
report the current memory and descriptor usage.

Thanks,
Kurt

Kurt Kanzenbach (6):
  net: dsa: hellcreek: Report RAM usage
  net: dsa: hellcreek: Report META data usage
  net: dsa: hellcreek: Add devlink VLAN region
  net: dsa: hellcreek: Use boolean value
  net: dsa: hellcreek: Move common code to helper
  net: dsa: hellcreek: Add devlink FDB region

 drivers/net/dsa/hirschmann/hellcreek.c | 293 +++++++++++++++++++++----
 drivers/net/dsa/hirschmann/hellcreek.h |  11 +
 2 files changed, 258 insertions(+), 46 deletions(-)

-- 
2.30.2

[PATCH net-next 1/6] net: dsa: hellcreek: Report RAM usage

[Kurt Kanzenbach]

Report the RAM usage via devlink. This is a useful debug feature. The actual
size depends on the used Hellcreek version:

|root@tsn:~# devlink resource show platform/ff240000.switch
|platform/ff240000.switch:
|  name VLAN size 4096 occ 3 unit entry dpipe_tables none
|  name FDB size 256 occ 6 unit entry dpipe_tables none
|  name RAM size 320 occ 14 unit entry dpipe_tables none

Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
---
 drivers/net/dsa/hirschmann/hellcreek.c | 38 ++++++++++++++++++++++++--
 drivers/net/dsa/hirschmann/hellcreek.h |  2 ++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 463137c39db2..c7a439336336 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -221,11 +221,13 @@ static void hellcreek_feature_detect(struct hellcreek *hellcreek)
 
 	features = hellcreek_read(hellcreek, HR_FEABITS0);
 
-	/* Only detect the size of the FDB table. The size and current
-	 * utilization can be queried via devlink.
+	/* Detect the FDB table size and the maximum RAM page count. The size
+	 * and current utilization can be queried via devlink.
 	 */
 	hellcreek->fdb_entries = ((features & HR_FEABITS0_FDBBINS_MASK) >>
-			       HR_FEABITS0_FDBBINS_SHIFT) * 32;
+				  HR_FEABITS0_FDBBINS_SHIFT) * 32;
+	hellcreek->page_count  = ((features & HR_FEABITS0_PCNT_MASK) >>
+				  HR_FEABITS0_PCNT_SHIFT) * 32;
 }
 
 static enum dsa_tag_protocol hellcreek_get_tag_protocol(struct dsa_switch *ds,
@@ -1034,10 +1036,23 @@ static u64 hellcreek_devlink_fdb_table_get(void *priv)
 	return count;
 }
 
+static u64 hellcreek_devlink_ram_usage_get(void *priv)
+{
+	struct hellcreek *hellcreek = priv;
+	u64 usage = 0;
+
+	/* Indicates how many free ram pages are available. */
+	usage = hellcreek_read(hellcreek, HR_PFREE);
+	usage = hellcreek->page_count - usage;
+
+	return usage;
+}
+
 static int hellcreek_setup_devlink_resources(struct dsa_switch *ds)
 {
 	struct devlink_resource_size_params size_vlan_params;
 	struct devlink_resource_size_params size_fdb_params;
+	struct devlink_resource_size_params size_ram_params;
 	struct hellcreek *hellcreek = ds->priv;
 	int err;
 
@@ -1050,6 +1065,11 @@ static int hellcreek_setup_devlink_resources(struct dsa_switch *ds)
 					  hellcreek->fdb_entries,
 					  1, DEVLINK_RESOURCE_UNIT_ENTRY);
 
+	devlink_resource_size_params_init(&size_ram_params,
+					  hellcreek->page_count,
+					  hellcreek->page_count,
+					  1, DEVLINK_RESOURCE_UNIT_ENTRY);
+
 	err = dsa_devlink_resource_register(ds, "VLAN", VLAN_N_VID,
 					    HELLCREEK_DEVLINK_PARAM_ID_VLAN_TABLE,
 					    DEVLINK_RESOURCE_ID_PARENT_TOP,
@@ -1064,6 +1084,13 @@ static int hellcreek_setup_devlink_resources(struct dsa_switch *ds)
 	if (err)
 		goto out;
 
+	err = dsa_devlink_resource_register(ds, "RAM", hellcreek->page_count,
+					    HELLCREEK_DEVLINK_PARAM_ID_RAM_USAGE,
+					    DEVLINK_RESOURCE_ID_PARENT_TOP,
+					    &size_ram_params);
+	if (err)
+		goto out;
+
 	dsa_devlink_resource_occ_get_register(ds,
 					      HELLCREEK_DEVLINK_PARAM_ID_VLAN_TABLE,
 					      hellcreek_devlink_vlan_table_get,
@@ -1074,6 +1101,11 @@ static int hellcreek_setup_devlink_resources(struct dsa_switch *ds)
 					      hellcreek_devlink_fdb_table_get,
 					      hellcreek);
 
+	dsa_devlink_resource_occ_get_register(ds,
+					      HELLCREEK_DEVLINK_PARAM_ID_RAM_USAGE,
+					      hellcreek_devlink_ram_usage_get,
+					      hellcreek);
+
 	return 0;
 
 out:
diff --git a/drivers/net/dsa/hirschmann/hellcreek.h b/drivers/net/dsa/hirschmann/hellcreek.h
index 305e76dab34d..9c08aeabbc24 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.h
+++ b/drivers/net/dsa/hirschmann/hellcreek.h
@@ -286,6 +286,7 @@ struct hellcreek {
 	u64 last_ts;		/* Used for overflow detection */
 	u16 status_out;		/* ptp.status_out shadow */
 	size_t fdb_entries;
+	size_t page_count;
 };
 
 /* A Qbv schedule can only started up to 8 seconds in the future. If the delta
@@ -302,6 +303,7 @@ struct hellcreek {
 enum hellcreek_devlink_resource_id {
 	HELLCREEK_DEVLINK_PARAM_ID_VLAN_TABLE,
 	HELLCREEK_DEVLINK_PARAM_ID_FDB_TABLE,
+	HELLCREEK_DEVLINK_PARAM_ID_RAM_USAGE,
 };
 
 #endif /* _HELLCREEK_H_ */
-- 
2.30.2

Re: [PATCH net-next 1/6] net: dsa: hellcreek: Report RAM usage

[Jakub Kicinski]

On Thu, 11 Mar 2021 18:53:39 +0100 Kurt Kanzenbach wrote:
> Report the RAM usage via devlink. This is a useful debug feature. The actual
> size depends on the used Hellcreek version:
> 
> |root@tsn:~# devlink resource show platform/ff240000.switch
> |platform/ff240000.switch:
> |  name VLAN size 4096 occ 3 unit entry dpipe_tables none
> |  name FDB size 256 occ 6 unit entry dpipe_tables none
> |  name RAM size 320 occ 14 unit entry dpipe_tables none

What is stored in this RAM? 

[PATCH net-next 2/6] net: dsa: hellcreek: Report META data usage

[Kurt Kanzenbach]

Report the META data descriptor usage via devlink. This is a useful debug
feature. The actual size depends on the used Hellcreek version:

|root@tsn:~# devlink resource show platform/ff240000.switch
|platform/ff240000.switch:
|  name VLAN size 4096 occ 3 unit entry dpipe_tables none
|  name FDB size 256 occ 6 unit entry dpipe_tables none
|  name RAM size 320 occ 14 unit entry dpipe_tables none
|  name META size 320 occ 5 unit entry dpipe_tables none

Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
---
 drivers/net/dsa/hirschmann/hellcreek.c | 36 ++++++++++++++++++++++++--
 drivers/net/dsa/hirschmann/hellcreek.h |  2 ++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index c7a439336336..d3760e2c9d8a 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -221,13 +221,15 @@ static void hellcreek_feature_detect(struct hellcreek *hellcreek)
 
 	features = hellcreek_read(hellcreek, HR_FEABITS0);
 
-	/* Detect the FDB table size and the maximum RAM page count. The size
-	 * and current utilization can be queried via devlink.
+	/* Detect the FDB table size and the maximum RAM page and meta data
+	 * count. The size and current utilization can be queried via devlink.
 	 */
 	hellcreek->fdb_entries = ((features & HR_FEABITS0_FDBBINS_MASK) >>
 				  HR_FEABITS0_FDBBINS_SHIFT) * 32;
 	hellcreek->page_count  = ((features & HR_FEABITS0_PCNT_MASK) >>
 				  HR_FEABITS0_PCNT_SHIFT) * 32;
+	hellcreek->meta_count  = ((features & HR_FEABITS0_MCNT_MASK) >>
+				  HR_FEABITS0_MCNT_SHIFT) * 32;
 }
 
 static enum dsa_tag_protocol hellcreek_get_tag_protocol(struct dsa_switch *ds,
@@ -1048,9 +1050,22 @@ static u64 hellcreek_devlink_ram_usage_get(void *priv)
 	return usage;
 }
 
+static u64 hellcreek_devlink_meta_usage_get(void *priv)
+{
+	struct hellcreek *hellcreek = priv;
+	u64 usage = 0;
+
+	/* Indicates how many free meta data descriptors are available. */
+	usage = hellcreek_read(hellcreek, HR_MFREE);
+	usage = hellcreek->meta_count - usage;
+
+	return usage;
+}
+
 static int hellcreek_setup_devlink_resources(struct dsa_switch *ds)
 {
 	struct devlink_resource_size_params size_vlan_params;
+	struct devlink_resource_size_params size_meta_params;
 	struct devlink_resource_size_params size_fdb_params;
 	struct devlink_resource_size_params size_ram_params;
 	struct hellcreek *hellcreek = ds->priv;
@@ -1070,6 +1085,11 @@ static int hellcreek_setup_devlink_resources(struct dsa_switch *ds)
 					  hellcreek->page_count,
 					  1, DEVLINK_RESOURCE_UNIT_ENTRY);
 
+	devlink_resource_size_params_init(&size_meta_params,
+					  hellcreek->meta_count,
+					  hellcreek->meta_count,
+					  1, DEVLINK_RESOURCE_UNIT_ENTRY);
+
 	err = dsa_devlink_resource_register(ds, "VLAN", VLAN_N_VID,
 					    HELLCREEK_DEVLINK_PARAM_ID_VLAN_TABLE,
 					    DEVLINK_RESOURCE_ID_PARENT_TOP,
@@ -1091,6 +1111,13 @@ static int hellcreek_setup_devlink_resources(struct dsa_switch *ds)
 	if (err)
 		goto out;
 
+	err = dsa_devlink_resource_register(ds, "META", hellcreek->meta_count,
+					    HELLCREEK_DEVLINK_PARAM_ID_METADATA_USAGE,
+					    DEVLINK_RESOURCE_ID_PARENT_TOP,
+					    &size_meta_params);
+	if (err)
+		goto out;
+
 	dsa_devlink_resource_occ_get_register(ds,
 					      HELLCREEK_DEVLINK_PARAM_ID_VLAN_TABLE,
 					      hellcreek_devlink_vlan_table_get,
@@ -1106,6 +1133,11 @@ static int hellcreek_setup_devlink_resources(struct dsa_switch *ds)
 					      hellcreek_devlink_ram_usage_get,
 					      hellcreek);
 
+	dsa_devlink_resource_occ_get_register(ds,
+					      HELLCREEK_DEVLINK_PARAM_ID_METADATA_USAGE,
+					      hellcreek_devlink_meta_usage_get,
+					      hellcreek);
+
 	return 0;
 
 out:
diff --git a/drivers/net/dsa/hirschmann/hellcreek.h b/drivers/net/dsa/hirschmann/hellcreek.h
index 9c08aeabbc24..06737caac37e 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.h
+++ b/drivers/net/dsa/hirschmann/hellcreek.h
@@ -287,6 +287,7 @@ struct hellcreek {
 	u16 status_out;		/* ptp.status_out shadow */
 	size_t fdb_entries;
 	size_t page_count;
+	size_t meta_count;
 };
 
 /* A Qbv schedule can only started up to 8 seconds in the future. If the delta
@@ -304,6 +305,7 @@ enum hellcreek_devlink_resource_id {
 	HELLCREEK_DEVLINK_PARAM_ID_VLAN_TABLE,
 	HELLCREEK_DEVLINK_PARAM_ID_FDB_TABLE,
 	HELLCREEK_DEVLINK_PARAM_ID_RAM_USAGE,
+	HELLCREEK_DEVLINK_PARAM_ID_METADATA_USAGE,
 };
 
 #endif /* _HELLCREEK_H_ */
-- 
2.30.2

Re: [PATCH net-next 2/6] net: dsa: hellcreek: Report META data usage

[Andrew Lunn]

On Thu, Mar 11, 2021 at 06:53:40PM +0100, Kurt Kanzenbach wrote:
> Report the META data descriptor usage via devlink.

Jakubs question is also relevant here. Please could you give a bit
more background about what the meta data is?

Thanks
	Andrew

Re: [PATCH net-next 2/6] net: dsa: hellcreek: Report META data usage

[Vladimir Oltean]

On Thu, Mar 11, 2021 at 11:52:41PM +0100, Andrew Lunn wrote:
> On Thu, Mar 11, 2021 at 06:53:40PM +0100, Kurt Kanzenbach wrote:
> > Report the META data descriptor usage via devlink.
> 
> Jakubs question is also relevant here. Please could you give a bit
> more background about what the meta data is?

Not having seen any documentation for this device, my guess is that
metadata descriptors are frame references, and the RAM page count is for
packet memory buffers. Nonetheless, I would still like to hear it from
Kurt. There is still a lot unknown even if I am correct. For example, if
the frame references or buffers can be partitioned, or if watermarks for
things like congestion/flow control can be set, then maybe devlink-sb is
a better choice (as that has an occupancy facility as well)?
Fully understand that it is not as trivial as exposing a devlink
resource, but on Ocelot/Felix I quite appreciate having the feature
(drivers/net/ethernet/mscc/ocelot_devlink.c). And knowing that this is a
TSN switch, I expect that sooner or later, the need to have control over
resource partitioning per traffic class will arise anyway.

Re: [PATCH net-next 2/6] net: dsa: hellcreek: Report META data usage

[Kurt Kanzenbach]

[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]

On Fri Mar 12 2021, Vladimir Oltean wrote:
> On Thu, Mar 11, 2021 at 11:52:41PM +0100, Andrew Lunn wrote:
>> On Thu, Mar 11, 2021 at 06:53:40PM +0100, Kurt Kanzenbach wrote:
>> > Report the META data descriptor usage via devlink.
>> 
>> Jakubs question is also relevant here. Please could you give a bit
>> more background about what the meta data is?
>
> Not having seen any documentation for this device, my guess is that
> metadata descriptors are frame references, and the RAM page count is
> for packet memory buffers. Nonetheless, I would still like to hear it
> from Kurt.

Yes, exactly.

> There is still a lot unknown even if I am correct. For example, if
> the frame references or buffers can be partitioned, or if watermarks for
> things like congestion/flow control can be set, then maybe devlink-sb is
> a better choice (as that has an occupancy facility as well)?
> Fully understand that it is not as trivial as exposing a devlink
> resource, but on Ocelot/Felix I quite appreciate having the feature
> (drivers/net/ethernet/mscc/ocelot_devlink.c). And knowing that this is a
> TSN switch, I expect that sooner or later, the need to have control over
> resource partitioning per traffic class will arise anyway.
>

True. The switch can actually distinguish between critical and
background traffic. Multiple limits can be configured: Maximum memory,
reserved memory for critical traffic, background traffic rates and queue
depths. I'll take a look at devlink-sb for that.

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 865 bytes --]

[PATCH net-next 3/6] net: dsa: hellcreek: Add devlink VLAN region

[Kurt Kanzenbach]

Allow to dump the VLAN table via devlink. This especially useful, because the
driver internally leverages VLANs for the port separation. These are not visible
via the bridge utility.

Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
---
 drivers/net/dsa/hirschmann/hellcreek.c | 73 ++++++++++++++++++++++++++
 drivers/net/dsa/hirschmann/hellcreek.h |  6 +++
 2 files changed, 79 insertions(+)

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index d3760e2c9d8a..910c992a84b0 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -1146,6 +1146,66 @@ static int hellcreek_setup_devlink_resources(struct dsa_switch *ds)
 	return err;
 }
 
+static int hellcreek_devlink_region_vlan_snapshot(struct devlink *dl,
+						  const struct devlink_region_ops *ops,
+						  struct netlink_ext_ack *extack,
+						  u8 **data)
+{
+	struct hellcreek_devlink_vlan_entry *table, *entry;
+	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
+	struct hellcreek *hellcreek = ds->priv;
+	int i;
+
+	table = kcalloc(VLAN_N_VID, sizeof(*entry), GFP_KERNEL);
+	if (!table)
+		return -ENOMEM;
+
+	entry = table;
+
+	mutex_lock(&hellcreek->reg_lock);
+	for (i = 0; i < VLAN_N_VID; ++i, ++entry) {
+		entry->member = hellcreek->vidmbrcfg[i];
+		entry->vid    = i;
+	}
+	mutex_unlock(&hellcreek->reg_lock);
+
+	*data = (u8 *)table;
+
+	return 0;
+}
+
+static struct devlink_region_ops hellcreek_region_vlan_ops = {
+	.name	    = "vlan",
+	.snapshot   = hellcreek_devlink_region_vlan_snapshot,
+	.destructor = kfree,
+};
+
+static int hellcreek_setup_devlink_regions(struct dsa_switch *ds)
+{
+	struct hellcreek *hellcreek = ds->priv;
+	struct devlink_region_ops *ops;
+	struct devlink_region *region;
+	u64 size;
+
+	size = VLAN_N_VID * sizeof(struct hellcreek_devlink_vlan_entry);
+	ops  = &hellcreek_region_vlan_ops;
+
+	region = dsa_devlink_region_create(ds, ops, 1, size);
+	if (IS_ERR(region))
+		return PTR_ERR(region);
+
+	hellcreek->vlan_region = region;
+
+	return 0;
+}
+
+static void hellcreek_teardown_devlink_regions(struct dsa_switch *ds)
+{
+	struct hellcreek *hellcreek = ds->priv;
+
+	dsa_devlink_region_destroy(hellcreek->vlan_region);
+}
+
 static int hellcreek_setup(struct dsa_switch *ds)
 {
 	struct hellcreek *hellcreek = ds->priv;
@@ -1207,11 +1267,24 @@ static int hellcreek_setup(struct dsa_switch *ds)
 		return ret;
 	}
 
+	ret = hellcreek_setup_devlink_regions(ds);
+	if (ret) {
+		dev_err(hellcreek->dev,
+			"Failed to setup devlink regions!\n");
+		goto err_regions;
+	}
+
 	return 0;
+
+err_regions:
+	dsa_devlink_resources_unregister(ds);
+
+	return ret;
 }
 
 static void hellcreek_teardown(struct dsa_switch *ds)
 {
+	hellcreek_teardown_devlink_regions(ds);
 	dsa_devlink_resources_unregister(ds);
 }
 
diff --git a/drivers/net/dsa/hirschmann/hellcreek.h b/drivers/net/dsa/hirschmann/hellcreek.h
index 06737caac37e..c0f17a2f13a0 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.h
+++ b/drivers/net/dsa/hirschmann/hellcreek.h
@@ -278,6 +278,7 @@ struct hellcreek {
 	struct mutex reg_lock;	/* Switch IP register lock */
 	struct mutex vlan_lock;	/* VLAN bitmaps lock */
 	struct mutex ptp_lock;	/* PTP IP register lock */
+	struct devlink_region *vlan_region;
 	void __iomem *base;
 	void __iomem *ptp_base;
 	u16 swcfg;		/* swcfg shadow */
@@ -308,4 +309,9 @@ enum hellcreek_devlink_resource_id {
 	HELLCREEK_DEVLINK_PARAM_ID_METADATA_USAGE,
 };
 
+struct hellcreek_devlink_vlan_entry {
+	u16 vid;
+	u16 member;
+};
+
 #endif /* _HELLCREEK_H_ */
-- 
2.30.2

[PATCH net-next 4/6] net: dsa: hellcreek: Use boolean value

[Kurt Kanzenbach]

hellcreek_select_vlan() takes a boolean instead of an integer.
So, use false accordingly.

Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
---
 drivers/net/dsa/hirschmann/hellcreek.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 910c992a84b0..d21f614f1c23 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -437,7 +437,7 @@ static void hellcreek_unapply_vlan(struct hellcreek *hellcreek, int port,
 
 	mutex_lock(&hellcreek->reg_lock);
 
-	hellcreek_select_vlan(hellcreek, vid, 0);
+	hellcreek_select_vlan(hellcreek, vid, false);
 
 	/* Setup port vlan membership */
 	hellcreek_select_vlan_params(hellcreek, port, &shift, &mask);
-- 
2.30.2

Re: [PATCH net-next 4/6] net: dsa: hellcreek: Use boolean value

[Andrew Lunn]

On Thu, Mar 11, 2021 at 06:53:42PM +0100, Kurt Kanzenbach wrote:
> hellcreek_select_vlan() takes a boolean instead of an integer.
> So, use false accordingly.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 5/6] net: dsa: hellcreek: Move common code to helper

[Kurt Kanzenbach]

There are two functions which need to populate fdb entries. Move that to a
helper function.

Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
---
 drivers/net/dsa/hirschmann/hellcreek.c | 85 +++++++++++++-------------
 1 file changed, 43 insertions(+), 42 deletions(-)

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index d21f614f1c23..6edb6b4c9041 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -674,6 +674,40 @@ static int __hellcreek_fdb_del(struct hellcreek *hellcreek,
 	return hellcreek_wait_fdb_ready(hellcreek);
 }
 
+static void hellcreek_populate_fdb_entry(struct hellcreek *hellcreek,
+					 struct hellcreek_fdb_entry *entry,
+					 size_t idx)
+{
+	unsigned char addr[ETH_ALEN];
+	u16 meta, mac;
+
+	/* Read values */
+	meta	= hellcreek_read(hellcreek, HR_FDBMDRD);
+	mac	= hellcreek_read(hellcreek, HR_FDBRDL);
+	addr[5] = mac & 0xff;
+	addr[4] = (mac & 0xff00) >> 8;
+	mac	= hellcreek_read(hellcreek, HR_FDBRDM);
+	addr[3] = mac & 0xff;
+	addr[2] = (mac & 0xff00) >> 8;
+	mac	= hellcreek_read(hellcreek, HR_FDBRDH);
+	addr[1] = mac & 0xff;
+	addr[0] = (mac & 0xff00) >> 8;
+
+	/* Populate @entry */
+	memcpy(entry->mac, addr, sizeof(addr));
+	entry->idx	    = idx;
+	entry->portmask	    = (meta & HR_FDBMDRD_PORTMASK_MASK) >>
+		HR_FDBMDRD_PORTMASK_SHIFT;
+	entry->age	    = (meta & HR_FDBMDRD_AGE_MASK) >>
+		HR_FDBMDRD_AGE_SHIFT;
+	entry->is_obt	    = !!(meta & HR_FDBMDRD_OBT);
+	entry->pass_blocked = !!(meta & HR_FDBMDRD_PASS_BLOCKED);
+	entry->is_static    = !!(meta & HR_FDBMDRD_STATIC);
+	entry->reprio_tc    = (meta & HR_FDBMDRD_REPRIO_TC_MASK) >>
+		HR_FDBMDRD_REPRIO_TC_SHIFT;
+	entry->reprio_en    = !!(meta & HR_FDBMDRD_REPRIO_EN);
+}
+
 /* Retrieve the index of a FDB entry by mac address. Currently we search through
  * the complete table in hardware. If that's too slow, we might have to cache
  * the complete FDB table in software.
@@ -695,39 +729,19 @@ static int hellcreek_fdb_get(struct hellcreek *hellcreek,
 	 * enter new entries anywhere.
 	 */
 	for (i = 0; i < hellcreek->fdb_entries; ++i) {
-		unsigned char addr[ETH_ALEN];
-		u16 meta, mac;
-
-		meta	= hellcreek_read(hellcreek, HR_FDBMDRD);
-		mac	= hellcreek_read(hellcreek, HR_FDBRDL);
-		addr[5] = mac & 0xff;
-		addr[4] = (mac & 0xff00) >> 8;
-		mac	= hellcreek_read(hellcreek, HR_FDBRDM);
-		addr[3] = mac & 0xff;
-		addr[2] = (mac & 0xff00) >> 8;
-		mac	= hellcreek_read(hellcreek, HR_FDBRDH);
-		addr[1] = mac & 0xff;
-		addr[0] = (mac & 0xff00) >> 8;
+		struct hellcreek_fdb_entry tmp = { 0 };
+
+		/* Read entry */
+		hellcreek_populate_fdb_entry(hellcreek, &tmp, i);
 
 		/* Force next entry */
 		hellcreek_write(hellcreek, 0x00, HR_FDBRDH);
 
-		if (memcmp(addr, dest, ETH_ALEN))
+		if (memcmp(tmp.mac, dest, ETH_ALEN))
 			continue;
 
 		/* Match found */
-		entry->idx	    = i;
-		entry->portmask	    = (meta & HR_FDBMDRD_PORTMASK_MASK) >>
-			HR_FDBMDRD_PORTMASK_SHIFT;
-		entry->age	    = (meta & HR_FDBMDRD_AGE_MASK) >>
-			HR_FDBMDRD_AGE_SHIFT;
-		entry->is_obt	    = !!(meta & HR_FDBMDRD_OBT);
-		entry->pass_blocked = !!(meta & HR_FDBMDRD_PASS_BLOCKED);
-		entry->is_static    = !!(meta & HR_FDBMDRD_STATIC);
-		entry->reprio_tc    = (meta & HR_FDBMDRD_REPRIO_TC_MASK) >>
-			HR_FDBMDRD_REPRIO_TC_SHIFT;
-		entry->reprio_en    = !!(meta & HR_FDBMDRD_REPRIO_EN);
-		memcpy(entry->mac, addr, sizeof(addr));
+		memcpy(entry, &tmp, sizeof(*entry));
 
 		return 0;
 	}
@@ -842,18 +856,9 @@ static int hellcreek_fdb_dump(struct dsa_switch *ds, int port,
 	for (i = 0; i < hellcreek->fdb_entries; ++i) {
 		unsigned char null_addr[ETH_ALEN] = { 0 };
 		struct hellcreek_fdb_entry entry = { 0 };
-		u16 meta, mac;
-
-		meta	= hellcreek_read(hellcreek, HR_FDBMDRD);
-		mac	= hellcreek_read(hellcreek, HR_FDBRDL);
-		entry.mac[5] = mac & 0xff;
-		entry.mac[4] = (mac & 0xff00) >> 8;
-		mac	= hellcreek_read(hellcreek, HR_FDBRDM);
-		entry.mac[3] = mac & 0xff;
-		entry.mac[2] = (mac & 0xff00) >> 8;
-		mac	= hellcreek_read(hellcreek, HR_FDBRDH);
-		entry.mac[1] = mac & 0xff;
-		entry.mac[0] = (mac & 0xff00) >> 8;
+
+		/* Read entry */
+		hellcreek_populate_fdb_entry(hellcreek, &entry, i);
 
 		/* Force next entry */
 		hellcreek_write(hellcreek, 0x00, HR_FDBRDH);
@@ -862,10 +867,6 @@ static int hellcreek_fdb_dump(struct dsa_switch *ds, int port,
 		if (!memcmp(entry.mac, null_addr, ETH_ALEN))
 			continue;
 
-		entry.portmask	= (meta & HR_FDBMDRD_PORTMASK_MASK) >>
-			HR_FDBMDRD_PORTMASK_SHIFT;
-		entry.is_static	= !!(meta & HR_FDBMDRD_STATIC);
-
 		/* Check port mask */
 		if (!(entry.portmask & BIT(port)))
 			continue;
-- 
2.30.2

Re: [PATCH net-next 5/6] net: dsa: hellcreek: Move common code to helper

[Andrew Lunn]

On Thu, Mar 11, 2021 at 06:53:43PM +0100, Kurt Kanzenbach wrote:
> There are two functions which need to populate fdb entries. Move that to a
> helper function.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 6/6] net: dsa: hellcreek: Add devlink FDB region

[Kurt Kanzenbach]

Allow to dump the FDB table via devlink. This is a useful debugging feature.

Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
---
 drivers/net/dsa/hirschmann/hellcreek.c | 63 ++++++++++++++++++++++++++
 drivers/net/dsa/hirschmann/hellcreek.h |  1 +
 2 files changed, 64 insertions(+)

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 6edb6b4c9041..c1f873a4fbc4 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -1175,19 +1175,64 @@ static int hellcreek_devlink_region_vlan_snapshot(struct devlink *dl,
 	return 0;
 }
 
+static int hellcreek_devlink_region_fdb_snapshot(struct devlink *dl,
+						 const struct devlink_region_ops *ops,
+						 struct netlink_ext_ack *extack,
+						 u8 **data)
+{
+	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
+	struct hellcreek_fdb_entry *table, *entry;
+	struct hellcreek *hellcreek = ds->priv;
+	size_t i;
+
+	table = kcalloc(hellcreek->fdb_entries, sizeof(*entry), GFP_KERNEL);
+	if (!table)
+		return -ENOMEM;
+
+	entry = table;
+
+	mutex_lock(&hellcreek->reg_lock);
+
+	/* Start table read */
+	hellcreek_read(hellcreek, HR_FDBMAX);
+	hellcreek_write(hellcreek, 0x00, HR_FDBMAX);
+
+	for (i = 0; i < hellcreek->fdb_entries; ++i, ++entry) {
+		/* Read current entry */
+		hellcreek_populate_fdb_entry(hellcreek, entry, i);
+
+		/* Advance read pointer */
+		hellcreek_write(hellcreek, 0x00, HR_FDBRDH);
+	}
+
+	mutex_unlock(&hellcreek->reg_lock);
+
+	*data = (u8 *)table;
+
+	return 0;
+}
+
 static struct devlink_region_ops hellcreek_region_vlan_ops = {
 	.name	    = "vlan",
 	.snapshot   = hellcreek_devlink_region_vlan_snapshot,
 	.destructor = kfree,
 };
 
+static struct devlink_region_ops hellcreek_region_fdb_ops = {
+	.name	    = "fdb",
+	.snapshot   = hellcreek_devlink_region_fdb_snapshot,
+	.destructor = kfree,
+};
+
 static int hellcreek_setup_devlink_regions(struct dsa_switch *ds)
 {
 	struct hellcreek *hellcreek = ds->priv;
 	struct devlink_region_ops *ops;
 	struct devlink_region *region;
 	u64 size;
+	int ret;
 
+	/* VLAN table */
 	size = VLAN_N_VID * sizeof(struct hellcreek_devlink_vlan_entry);
 	ops  = &hellcreek_region_vlan_ops;
 
@@ -1197,13 +1242,31 @@ static int hellcreek_setup_devlink_regions(struct dsa_switch *ds)
 
 	hellcreek->vlan_region = region;
 
+	/* FDB table */
+	size = hellcreek->fdb_entries * sizeof(struct hellcreek_fdb_entry);
+	ops  = &hellcreek_region_fdb_ops;
+
+	region = dsa_devlink_region_create(ds, ops, 1, size);
+	if (IS_ERR(region)) {
+		ret = PTR_ERR(region);
+		goto err_fdb;
+	}
+
+	hellcreek->fdb_region = region;
+
 	return 0;
+
+err_fdb:
+	dsa_devlink_region_destroy(hellcreek->vlan_region);
+
+	return ret;
 }
 
 static void hellcreek_teardown_devlink_regions(struct dsa_switch *ds)
 {
 	struct hellcreek *hellcreek = ds->priv;
 
+	dsa_devlink_region_destroy(hellcreek->fdb_region);
 	dsa_devlink_region_destroy(hellcreek->vlan_region);
 }
 
diff --git a/drivers/net/dsa/hirschmann/hellcreek.h b/drivers/net/dsa/hirschmann/hellcreek.h
index c0f17a2f13a0..17445e429f1f 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.h
+++ b/drivers/net/dsa/hirschmann/hellcreek.h
@@ -279,6 +279,7 @@ struct hellcreek {
 	struct mutex vlan_lock;	/* VLAN bitmaps lock */
 	struct mutex ptp_lock;	/* PTP IP register lock */
 	struct devlink_region *vlan_region;
+	struct devlink_region *fdb_region;
 	void __iomem *base;
 	void __iomem *ptp_base;
 	u16 swcfg;		/* swcfg shadow */
-- 
2.30.2

Re: [PATCH net-next 6/6] net: dsa: hellcreek: Add devlink FDB region

[Andrew Lunn]

>  static int hellcreek_setup_devlink_regions(struct dsa_switch *ds)
>  {
>  	struct hellcreek *hellcreek = ds->priv;
>  	struct devlink_region_ops *ops;
>  	struct devlink_region *region;
>  	u64 size;
> +	int ret;
>  
> +	/* VLAN table */
>  	size = VLAN_N_VID * sizeof(struct hellcreek_devlink_vlan_entry);
>  	ops  = &hellcreek_region_vlan_ops;

I think this comment belongs in the previous patch adding the VLAN
region.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew