[net-next,v2,4/4] net: dsa: hellcreek: Add devlink FDB region

Commit Message

Kurt Kanzenbach March 13, 2021, 9:39 a.m. UTC

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

Signed-off-by: Kurt Kanzenbach <[e-mail]>
Reviewed-by: Andrew Lunn <[e-mail]>
---
 drivers/net/dsa/hirschmann/hellcreek.c | 62 ++++++++++++++++++++++++++
 drivers/net/dsa/hirschmann/hellcreek.h |  1 +
 2 files changed, 63 insertions(+)

Comments

Vladimir Oltean March 13, 2021, 11:46 a.m. UTC | #1

On Sat, Mar 13, 2021 at 10:39:39AM +0100, Kurt Kanzenbach wrote:
> Allow to dump the FDB table via devlink. This is a useful debugging feature.
> 
> Signed-off-by: Kurt Kanzenbach <[e-mail]>
> Reviewed-by: Andrew Lunn <[e-mail]>
> ---

Reviewed-by: Vladimir Oltean <[e-mail]>

By the way, what user space program do you use to dump these? Did you
derive something from Andrew's mv88e6xxx_dump too? Maybe we should work
on something common?

Florian Fainelli March 14, 2021, 1:41 a.m. UTC | #2

On 3/13/2021 1:39 AM, Kurt Kanzenbach wrote:
> Allow to dump the FDB table via devlink. This is a useful debugging feature.
> 
> Signed-off-by: Kurt Kanzenbach <[e-mail]>
> Reviewed-by: Andrew Lunn <[e-mail]>

Reviewed-by: Florian Fainelli <[e-mail]>

Kurt Kanzenbach March 14, 2021, 1 p.m. UTC | #3

On Sat Mar 13 2021, Vladimir Oltean wrote:
> On Sat, Mar 13, 2021 at 10:39:39AM +0100, Kurt Kanzenbach wrote:
>> Allow to dump the FDB table via devlink. This is a useful debugging feature.
>> 
>> Signed-off-by: Kurt Kanzenbach <[e-mail]>
>> Reviewed-by: Andrew Lunn <[e-mail]>
>> ---
>
> Reviewed-by: Vladimir Oltean <[e-mail]>
>
> By the way, what user space program do you use to dump these? Did you
> derive something from Andrew's mv88e6xxx_dump too? Maybe we should work
> on something common?

Actually there is no user space tooling, yet. My original approach to
debugging was different using debugfs and tracing. I played a bit with
mv88e6xxx_dump today. Having a common tool would be quite nice.

Thanks,
Kurt

Kurt Kanzenbach March 20, 2021, 11:31 a.m. UTC | #4

On Sun Mar 14 2021, Kurt Kanzenbach wrote:
> On Sat Mar 13 2021, Vladimir Oltean wrote:
>> On Sat, Mar 13, 2021 at 10:39:39AM +0100, Kurt Kanzenbach wrote:
>>> Allow to dump the FDB table via devlink. This is a useful debugging feature.
>>> 
>>> Signed-off-by: Kurt Kanzenbach <[e-mail]>
>>> Reviewed-by: Andrew Lunn <[e-mail]>
>>> ---
>>
>> Reviewed-by: Vladimir Oltean <[e-mail]>
>>
>> By the way, what user space program do you use to dump these? Did you
>> derive something from Andrew's mv88e6xxx_dump too? Maybe we should work
>> on something common?
>
> Actually there is no user space tooling, yet. My original approach to
> debugging was different using debugfs and tracing. I played a bit with
> mv88e6xxx_dump today. Having a common tool would be quite nice.

Thanks for the pointer to mv88e6xxx_dump. Just needed to implement
.devlink_info_get() callback, so that the tool can distinguish between
the Marvell and Hellcreek devices (and implement some hellcreek
specifics).

Thanks,
Kurt

Patch

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 38ff0f12e8a4..02d8bcb37f31 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -1111,18 +1111,62 @@  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);
@@ -1134,13 +1178,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 42339f9359d9..9e303b8ab13c 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 */