diff mbox

[net-next,v3] rocker: add debugfs support to dump internal tables

Message ID 1439850977-31079-1-git-send-email-sfeldma@gmail.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Scott Feldman Aug. 17, 2015, 10:36 p.m. UTC 
From: Scott Feldman <sfeldma@gmail.com>

> tree /sys/kernel/debug/rocker
/sys/kernel/debug/rocker
└── 5254001235010000
    ├── fdb_tbl
    ├── internal_vlan_tbl
    ├── neigh_tbl
    ├── of_dpa_flow_tbl
    └── of_dpa_group_tbl

1 directory, 5 files

> cat /sys/kernel/debug/rocker/5254001235010000/internal_vlan_tbl
ifindex 5 ref_count 1 vlan 3843
ifindex 7 ref_count 2 vlan 3840
ifindex 4 ref_count 1 vlan 3842

> cat /sys/kernel/debug/rocker/5254001235010000/fdb_tbl
learned 1 pport 1 addr 00:02:00:00:02:00 vlan 3840
learned 1 pport 2 addr 00:02:00:00:03:00 vlan 3840

> cat /sys/kernel/debug/rocker/5254001235010000/neigh_tbl
11.0.0.9 dev sw1p2 ref_count 3 index 1 dst 00:02:00:00:01:00 ttl_check 1
11.0.0.1 dev sw1p1 ref_count 3 index 0 dst 00:02:00:00:00:00 ttl_check 1

> cat /sys/kernel/debug/rocker/5254001235010000/of_dpa_flow_tbl
cmd 3 cookie 15 priority 3  tbl acl            in_pport 2 01:80:c2:00:00:00/ff:ff:ff:ff:ff:f0 eth_type 0x0000 vlan_id 3841 ip proto 0/0 ip tos 0/0 group_id 0x0f010000
cmd 3 cookie 2  priority 0  tbl term_mac       in_pport 1 eth_type 0x0800 52:54:00:12:35:01 vlan_id 3840 goto_tbl ucast_routing copy_to_cpu 0
cmd 3 cookie 1f priority 3  tbl bridge         00:02:00:00:00:00 vlan_id 3840 tunnel_id 0 goto_tbl acl group_id 0x00000000 copy_to_cpu 0
cmd 3 cookie 4  priority 1  tbl vlan           in_pport 2 vlan_id 0 goto_tbl term_mac untagged 1 new_vlan_id 3841
cmd 3 cookie 20 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.1 goto_tbl acl group_id 0x20000000
cmd 3 cookie 21 priority 3  tbl bridge         00:02:00:00:01:00 vlan_id 3841 tunnel_id 0 goto_tbl acl group_id 0x00000000 copy_to_cpu 0
cmd 3 cookie 16 priority 2  tbl acl            in_pport 2 eth_type 0x0806 vlan_id 3841 ip proto 0/0 ip tos 0/0 group_id 0x0f010000
cmd 3 cookie 12 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.0 goto_tbl acl group_id 0x0f000000
cmd 3 cookie 9  priority 3  tbl acl            in_pport 1 01:80:c2:00:00:00/ff:ff:ff:ff:ff:f0 eth_type 0x0000 vlan_id 3840 ip proto 0/0 ip tos 0/0 group_id 0x0f000000
cmd 3 cookie 6  priority 0  tbl term_mac       in_pport 2 eth_type 0x86dd 52:54:00:12:35:02 vlan_id 3841 goto_tbl ucast_routing copy_to_cpu 0
cmd 4 cookie 0  priority 1  tbl ig_port        in_pport 0/0xffff0000 goto_tbl vlan
cmd 4 cookie e  priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.3 goto_tbl acl group_id 0x0f000000
cmd 3 cookie 1  priority 1  tbl vlan           in_pport 1 vlan_id 0 goto_tbl term_mac untagged 1 new_vlan_id 3840
cmd 3 cookie 24 priority 20 tbl ucast_routing  eth_type 0x0800 11.0.0.4/255.255.255.252 goto_tbl acl group_id 0x20000000
cmd 4 cookie 14 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.10 goto_tbl acl group_id 0x0f010000
cmd 3 cookie 2c priority 20 tbl ucast_routing  eth_type 0x0800 12.0.0.4 goto_tbl acl group_id 0x20000001
cmd 3 cookie 17 priority 1  tbl term_mac       in_pport 2 eth_type 0x0800 01:00:5e:00:00:00/ff:ff:ff:80:00:00 vlan_id 3841 goto_tbl mcast_routing copy_to_cpu 1
cmd 3 cookie 26 priority 20 tbl ucast_routing  eth_type 0x0800 12.0.0.3 goto_tbl acl group_id 0x20000000
cmd 3 cookie 2e priority 30 tbl ucast_routing  eth_type 0x0800 12.0.0.2 goto_tbl acl group_id 0x0f010000
cmd 3 cookie 22 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.9 goto_tbl acl group_id 0x20000001
cmd 3 cookie 1c priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.8/255.255.255.252 goto_tbl acl group_id 0x0f010000
cmd 3 cookie 18 priority 1  tbl term_mac       in_pport 2 eth_type 0x86dd 33:33:00:00:00:00/ff:ff:00:00:00:00 vlan_id 3841 goto_tbl mcast_routing copy_to_cpu 1
cmd 3 cookie 5  priority 0  tbl term_mac       in_pport 2 eth_type 0x0800 52:54:00:12:35:02 vlan_id 3841 goto_tbl ucast_routing copy_to_cpu 0
cmd 3 cookie a  priority 2  tbl acl            in_pport 1 eth_type 0x0806 vlan_id 3840 ip proto 0/0 ip tos 0/0 group_id 0x0f000000
cmd 4 cookie 1a priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.11 goto_tbl acl group_id 0x0f010000
cmd 3 cookie 1e priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.8 goto_tbl acl group_id 0x0f010000
cmd 3 cookie 3  priority 0  tbl term_mac       in_pport 1 eth_type 0x86dd 52:54:00:12:35:01 vlan_id 3840 goto_tbl ucast_routing copy_to_cpu 0
cmd 3 cookie b  priority 1  tbl term_mac       in_pport 1 eth_type 0x0800 01:00:5e:00:00:00/ff:ff:ff:80:00:00 vlan_id 3840 goto_tbl mcast_routing copy_to_cpu 1
cmd 4 cookie 8  priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.2 goto_tbl acl group_id 0x0f000000
cmd 3 cookie 10 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.0/255.255.255.252 goto_tbl acl group_id 0x0f000000
cmd 3 cookie 28 priority 20 tbl ucast_routing  eth_type 0x0800 11.0.0.12/255.255.255.252 goto_tbl acl group_id 0x20000001
cmd 3 cookie c  priority 1  tbl term_mac       in_pport 1 eth_type 0x86dd 33:33:00:00:00:00/ff:ff:00:00:00:00 vlan_id 3840 goto_tbl mcast_routing copy_to_cpu 1

> cat /sys/kernel/debug/rocker/5254001235010000/of_dpa_group_tbl
cmd 7 group_id 0x0f000000 (L2 interface vlan 3840 port 0) pop_vlan 1
cmd 7 group_id 0x0f010000 (L2 interface vlan 3841 port 0) pop_vlan 1
cmd 7 group_id 0x20000000 (L3 unicast index 0) src 52:54:00:12:35:01 dst 00:02:00:00:00:00 vlan 3840 ttl_check 1 group_id 0x0f000001
cmd 7 group_id 0x0f010002 (L2 interface vlan 3841 port 2) pop_vlan 1
cmd 7 group_id 0x0f000001 (L2 interface vlan 3840 port 1) pop_vlan 1
cmd 8 group_id 0x20000001 (L3 unicast index 1) src 52:54:00:12:35:02 dst 00:02:00:00:01:00 vlan 3841 ttl_check 1 group_id 0x0f010002

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
v3: undo v2 change: Jiri is right: debugfs calls are no-ops when
    CONFIG_DEBUG_FS=n so we don't need to do anything extra in the
    CONFIG_DEBUG_FS=n case.  v3 has adds a few wrapper funcs around the
    debugfs calls to keep mainline funcs clean, so let's go with this
    version over v1.

v2: wrap feature with CONFIG_ROCKER_DEBUGFS to allow config-time
    enable/disable

 drivers/net/ethernet/rocker/rocker.c |  379 ++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/rocker/rocker.h |    2 +-
 2 files changed, 380 insertions(+), 1 deletion(-)

Comments

Jiri Pirko Aug. 18, 2015, 5:55 a.m. UTC | #1 
Tue, Aug 18, 2015 at 12:36:17AM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>
>
>> tree /sys/kernel/debug/rocker
>/sys/kernel/debug/rocker
>└── 5254001235010000
>    ├── fdb_tbl
>    ├── internal_vlan_tbl
>    ├── neigh_tbl
>    ├── of_dpa_flow_tbl
>    └── of_dpa_group_tbl
>
>1 directory, 5 files
>
>> cat /sys/kernel/debug/rocker/5254001235010000/internal_vlan_tbl
>ifindex 5 ref_count 1 vlan 3843
>ifindex 7 ref_count 2 vlan 3840
>ifindex 4 ref_count 1 vlan 3842
>
>> cat /sys/kernel/debug/rocker/5254001235010000/fdb_tbl
>learned 1 pport 1 addr 00:02:00:00:02:00 vlan 3840
>learned 1 pport 2 addr 00:02:00:00:03:00 vlan 3840
>
>> cat /sys/kernel/debug/rocker/5254001235010000/neigh_tbl
>11.0.0.9 dev sw1p2 ref_count 3 index 1 dst 00:02:00:00:01:00 ttl_check 1
>11.0.0.1 dev sw1p1 ref_count 3 index 0 dst 00:02:00:00:00:00 ttl_check 1
>
>> cat /sys/kernel/debug/rocker/5254001235010000/of_dpa_flow_tbl
>cmd 3 cookie 15 priority 3  tbl acl            in_pport 2 01:80:c2:00:00:00/ff:ff:ff:ff:ff:f0 eth_type 0x0000 vlan_id 3841 ip proto 0/0 ip tos 0/0 group_id 0x0f010000
>cmd 3 cookie 2  priority 0  tbl term_mac       in_pport 1 eth_type 0x0800 52:54:00:12:35:01 vlan_id 3840 goto_tbl ucast_routing copy_to_cpu 0
>cmd 3 cookie 1f priority 3  tbl bridge         00:02:00:00:00:00 vlan_id 3840 tunnel_id 0 goto_tbl acl group_id 0x00000000 copy_to_cpu 0
>cmd 3 cookie 4  priority 1  tbl vlan           in_pport 2 vlan_id 0 goto_tbl term_mac untagged 1 new_vlan_id 3841
>cmd 3 cookie 20 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.1 goto_tbl acl group_id 0x20000000
>cmd 3 cookie 21 priority 3  tbl bridge         00:02:00:00:01:00 vlan_id 3841 tunnel_id 0 goto_tbl acl group_id 0x00000000 copy_to_cpu 0
>cmd 3 cookie 16 priority 2  tbl acl            in_pport 2 eth_type 0x0806 vlan_id 3841 ip proto 0/0 ip tos 0/0 group_id 0x0f010000
>cmd 3 cookie 12 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.0 goto_tbl acl group_id 0x0f000000
>cmd 3 cookie 9  priority 3  tbl acl            in_pport 1 01:80:c2:00:00:00/ff:ff:ff:ff:ff:f0 eth_type 0x0000 vlan_id 3840 ip proto 0/0 ip tos 0/0 group_id 0x0f000000
>cmd 3 cookie 6  priority 0  tbl term_mac       in_pport 2 eth_type 0x86dd 52:54:00:12:35:02 vlan_id 3841 goto_tbl ucast_routing copy_to_cpu 0
>cmd 4 cookie 0  priority 1  tbl ig_port        in_pport 0/0xffff0000 goto_tbl vlan
>cmd 4 cookie e  priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.3 goto_tbl acl group_id 0x0f000000
>cmd 3 cookie 1  priority 1  tbl vlan           in_pport 1 vlan_id 0 goto_tbl term_mac untagged 1 new_vlan_id 3840
>cmd 3 cookie 24 priority 20 tbl ucast_routing  eth_type 0x0800 11.0.0.4/255.255.255.252 goto_tbl acl group_id 0x20000000
>cmd 4 cookie 14 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.10 goto_tbl acl group_id 0x0f010000
>cmd 3 cookie 2c priority 20 tbl ucast_routing  eth_type 0x0800 12.0.0.4 goto_tbl acl group_id 0x20000001
>cmd 3 cookie 17 priority 1  tbl term_mac       in_pport 2 eth_type 0x0800 01:00:5e:00:00:00/ff:ff:ff:80:00:00 vlan_id 3841 goto_tbl mcast_routing copy_to_cpu 1
>cmd 3 cookie 26 priority 20 tbl ucast_routing  eth_type 0x0800 12.0.0.3 goto_tbl acl group_id 0x20000000
>cmd 3 cookie 2e priority 30 tbl ucast_routing  eth_type 0x0800 12.0.0.2 goto_tbl acl group_id 0x0f010000
>cmd 3 cookie 22 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.9 goto_tbl acl group_id 0x20000001
>cmd 3 cookie 1c priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.8/255.255.255.252 goto_tbl acl group_id 0x0f010000
>cmd 3 cookie 18 priority 1  tbl term_mac       in_pport 2 eth_type 0x86dd 33:33:00:00:00:00/ff:ff:00:00:00:00 vlan_id 3841 goto_tbl mcast_routing copy_to_cpu 1
>cmd 3 cookie 5  priority 0  tbl term_mac       in_pport 2 eth_type 0x0800 52:54:00:12:35:02 vlan_id 3841 goto_tbl ucast_routing copy_to_cpu 0
>cmd 3 cookie a  priority 2  tbl acl            in_pport 1 eth_type 0x0806 vlan_id 3840 ip proto 0/0 ip tos 0/0 group_id 0x0f000000
>cmd 4 cookie 1a priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.11 goto_tbl acl group_id 0x0f010000
>cmd 3 cookie 1e priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.8 goto_tbl acl group_id 0x0f010000
>cmd 3 cookie 3  priority 0  tbl term_mac       in_pport 1 eth_type 0x86dd 52:54:00:12:35:01 vlan_id 3840 goto_tbl ucast_routing copy_to_cpu 0
>cmd 3 cookie b  priority 1  tbl term_mac       in_pport 1 eth_type 0x0800 01:00:5e:00:00:00/ff:ff:ff:80:00:00 vlan_id 3840 goto_tbl mcast_routing copy_to_cpu 1
>cmd 4 cookie 8  priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.2 goto_tbl acl group_id 0x0f000000
>cmd 3 cookie 10 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.0/255.255.255.252 goto_tbl acl group_id 0x0f000000
>cmd 3 cookie 28 priority 20 tbl ucast_routing  eth_type 0x0800 11.0.0.12/255.255.255.252 goto_tbl acl group_id 0x20000001
>cmd 3 cookie c  priority 1  tbl term_mac       in_pport 1 eth_type 0x86dd 33:33:00:00:00:00/ff:ff:00:00:00:00 vlan_id 3840 goto_tbl mcast_routing copy_to_cpu 1
>
>> cat /sys/kernel/debug/rocker/5254001235010000/of_dpa_group_tbl
>cmd 7 group_id 0x0f000000 (L2 interface vlan 3840 port 0) pop_vlan 1
>cmd 7 group_id 0x0f010000 (L2 interface vlan 3841 port 0) pop_vlan 1
>cmd 7 group_id 0x20000000 (L3 unicast index 0) src 52:54:00:12:35:01 dst 00:02:00:00:00:00 vlan 3840 ttl_check 1 group_id 0x0f000001
>cmd 7 group_id 0x0f010002 (L2 interface vlan 3841 port 2) pop_vlan 1
>cmd 7 group_id 0x0f000001 (L2 interface vlan 3840 port 1) pop_vlan 1
>cmd 8 group_id 0x20000001 (L3 unicast index 1) src 52:54:00:12:35:02 dst 00:02:00:00:01:00 vlan 3841 ttl_check 1 group_id 0x0f010002
>
>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>---
>v3: undo v2 change: Jiri is right: debugfs calls are no-ops when
>    CONFIG_DEBUG_FS=n so we don't need to do anything extra in the
>    CONFIG_DEBUG_FS=n case.  v3 has adds a few wrapper funcs around the
>    debugfs calls to keep mainline funcs clean, so let's go with this
>    version over v1.
>
>v2: wrap feature with CONFIG_ROCKER_DEBUGFS to allow config-time
>    enable/disable
>
> drivers/net/ethernet/rocker/rocker.c |  379 ++++++++++++++++++++++++++++++++++
> drivers/net/ethernet/rocker/rocker.h |    2 +-
> 2 files changed, 380 insertions(+), 1 deletion(-)
>

<snip>

>+static int rocker_probe_debugfs_init(struct rocker *rocker)
>+{
>+	char dbg_dir_name[sizeof(rocker->hw.id) * 2 + 1];
>+
>+	sprintf(dbg_dir_name, "%*phN", (int)sizeof(rocker->hw.id),
>+		&rocker->hw.id);

You can use PCI address here. Might be better.


>+	rocker->dbg_dir = debugfs_create_dir(dbg_dir_name, rocker_dbg_root);
>+	if (!rocker->dbg_dir)

You still check the retval here and 


>+		return -ENOMEM;
>+	return 0;
>+}
>+

<snip>

>+static int rocker_debugfs_init(void)
>+{
>+	rocker_dbg_root = debugfs_create_dir(rocker_driver_name, NULL);
>+	if (!rocker_dbg_root)

here. When debugfs is not enabled, init will fail.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Feldman Aug. 18, 2015, 6:14 a.m. UTC | #2 
On Mon, Aug 17, 2015 at 10:55 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Tue, Aug 18, 2015 at 12:36:17AM CEST, sfeldma@gmail.com wrote:
>>From: Scott Feldman <sfeldma@gmail.com>
>>
>>> tree /sys/kernel/debug/rocker
>>/sys/kernel/debug/rocker
>>└── 5254001235010000
>>    ├── fdb_tbl
>>    ├── internal_vlan_tbl
>>    ├── neigh_tbl
>>    ├── of_dpa_flow_tbl
>>    └── of_dpa_group_tbl
>>
>>1 directory, 5 files
>>
>>> cat /sys/kernel/debug/rocker/5254001235010000/internal_vlan_tbl
>>ifindex 5 ref_count 1 vlan 3843
>>ifindex 7 ref_count 2 vlan 3840
>>ifindex 4 ref_count 1 vlan 3842
>>
>>> cat /sys/kernel/debug/rocker/5254001235010000/fdb_tbl
>>learned 1 pport 1 addr 00:02:00:00:02:00 vlan 3840
>>learned 1 pport 2 addr 00:02:00:00:03:00 vlan 3840
>>
>>> cat /sys/kernel/debug/rocker/5254001235010000/neigh_tbl
>>11.0.0.9 dev sw1p2 ref_count 3 index 1 dst 00:02:00:00:01:00 ttl_check 1
>>11.0.0.1 dev sw1p1 ref_count 3 index 0 dst 00:02:00:00:00:00 ttl_check 1
>>
>>> cat /sys/kernel/debug/rocker/5254001235010000/of_dpa_flow_tbl
>>cmd 3 cookie 15 priority 3  tbl acl            in_pport 2 01:80:c2:00:00:00/ff:ff:ff:ff:ff:f0 eth_type 0x0000 vlan_id 3841 ip proto 0/0 ip tos 0/0 group_id 0x0f010000
>>cmd 3 cookie 2  priority 0  tbl term_mac       in_pport 1 eth_type 0x0800 52:54:00:12:35:01 vlan_id 3840 goto_tbl ucast_routing copy_to_cpu 0
>>cmd 3 cookie 1f priority 3  tbl bridge         00:02:00:00:00:00 vlan_id 3840 tunnel_id 0 goto_tbl acl group_id 0x00000000 copy_to_cpu 0
>>cmd 3 cookie 4  priority 1  tbl vlan           in_pport 2 vlan_id 0 goto_tbl term_mac untagged 1 new_vlan_id 3841
>>cmd 3 cookie 20 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.1 goto_tbl acl group_id 0x20000000
>>cmd 3 cookie 21 priority 3  tbl bridge         00:02:00:00:01:00 vlan_id 3841 tunnel_id 0 goto_tbl acl group_id 0x00000000 copy_to_cpu 0
>>cmd 3 cookie 16 priority 2  tbl acl            in_pport 2 eth_type 0x0806 vlan_id 3841 ip proto 0/0 ip tos 0/0 group_id 0x0f010000
>>cmd 3 cookie 12 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.0 goto_tbl acl group_id 0x0f000000
>>cmd 3 cookie 9  priority 3  tbl acl            in_pport 1 01:80:c2:00:00:00/ff:ff:ff:ff:ff:f0 eth_type 0x0000 vlan_id 3840 ip proto 0/0 ip tos 0/0 group_id 0x0f000000
>>cmd 3 cookie 6  priority 0  tbl term_mac       in_pport 2 eth_type 0x86dd 52:54:00:12:35:02 vlan_id 3841 goto_tbl ucast_routing copy_to_cpu 0
>>cmd 4 cookie 0  priority 1  tbl ig_port        in_pport 0/0xffff0000 goto_tbl vlan
>>cmd 4 cookie e  priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.3 goto_tbl acl group_id 0x0f000000
>>cmd 3 cookie 1  priority 1  tbl vlan           in_pport 1 vlan_id 0 goto_tbl term_mac untagged 1 new_vlan_id 3840
>>cmd 3 cookie 24 priority 20 tbl ucast_routing  eth_type 0x0800 11.0.0.4/255.255.255.252 goto_tbl acl group_id 0x20000000
>>cmd 4 cookie 14 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.10 goto_tbl acl group_id 0x0f010000
>>cmd 3 cookie 2c priority 20 tbl ucast_routing  eth_type 0x0800 12.0.0.4 goto_tbl acl group_id 0x20000001
>>cmd 3 cookie 17 priority 1  tbl term_mac       in_pport 2 eth_type 0x0800 01:00:5e:00:00:00/ff:ff:ff:80:00:00 vlan_id 3841 goto_tbl mcast_routing copy_to_cpu 1
>>cmd 3 cookie 26 priority 20 tbl ucast_routing  eth_type 0x0800 12.0.0.3 goto_tbl acl group_id 0x20000000
>>cmd 3 cookie 2e priority 30 tbl ucast_routing  eth_type 0x0800 12.0.0.2 goto_tbl acl group_id 0x0f010000
>>cmd 3 cookie 22 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.9 goto_tbl acl group_id 0x20000001
>>cmd 3 cookie 1c priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.8/255.255.255.252 goto_tbl acl group_id 0x0f010000
>>cmd 3 cookie 18 priority 1  tbl term_mac       in_pport 2 eth_type 0x86dd 33:33:00:00:00:00/ff:ff:00:00:00:00 vlan_id 3841 goto_tbl mcast_routing copy_to_cpu 1
>>cmd 3 cookie 5  priority 0  tbl term_mac       in_pport 2 eth_type 0x0800 52:54:00:12:35:02 vlan_id 3841 goto_tbl ucast_routing copy_to_cpu 0
>>cmd 3 cookie a  priority 2  tbl acl            in_pport 1 eth_type 0x0806 vlan_id 3840 ip proto 0/0 ip tos 0/0 group_id 0x0f000000
>>cmd 4 cookie 1a priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.11 goto_tbl acl group_id 0x0f010000
>>cmd 3 cookie 1e priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.8 goto_tbl acl group_id 0x0f010000
>>cmd 3 cookie 3  priority 0  tbl term_mac       in_pport 1 eth_type 0x86dd 52:54:00:12:35:01 vlan_id 3840 goto_tbl ucast_routing copy_to_cpu 0
>>cmd 3 cookie b  priority 1  tbl term_mac       in_pport 1 eth_type 0x0800 01:00:5e:00:00:00/ff:ff:ff:80:00:00 vlan_id 3840 goto_tbl mcast_routing copy_to_cpu 1
>>cmd 4 cookie 8  priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.2 goto_tbl acl group_id 0x0f000000
>>cmd 3 cookie 10 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.0/255.255.255.252 goto_tbl acl group_id 0x0f000000
>>cmd 3 cookie 28 priority 20 tbl ucast_routing  eth_type 0x0800 11.0.0.12/255.255.255.252 goto_tbl acl group_id 0x20000001
>>cmd 3 cookie c  priority 1  tbl term_mac       in_pport 1 eth_type 0x86dd 33:33:00:00:00:00/ff:ff:00:00:00:00 vlan_id 3840 goto_tbl mcast_routing copy_to_cpu 1
>>
>>> cat /sys/kernel/debug/rocker/5254001235010000/of_dpa_group_tbl
>>cmd 7 group_id 0x0f000000 (L2 interface vlan 3840 port 0) pop_vlan 1
>>cmd 7 group_id 0x0f010000 (L2 interface vlan 3841 port 0) pop_vlan 1
>>cmd 7 group_id 0x20000000 (L3 unicast index 0) src 52:54:00:12:35:01 dst 00:02:00:00:00:00 vlan 3840 ttl_check 1 group_id 0x0f000001
>>cmd 7 group_id 0x0f010002 (L2 interface vlan 3841 port 2) pop_vlan 1
>>cmd 7 group_id 0x0f000001 (L2 interface vlan 3840 port 1) pop_vlan 1
>>cmd 8 group_id 0x20000001 (L3 unicast index 1) src 52:54:00:12:35:02 dst 00:02:00:00:01:00 vlan 3841 ttl_check 1 group_id 0x0f010002
>>
>>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>---
>>v3: undo v2 change: Jiri is right: debugfs calls are no-ops when
>>    CONFIG_DEBUG_FS=n so we don't need to do anything extra in the
>>    CONFIG_DEBUG_FS=n case.  v3 has adds a few wrapper funcs around the
>>    debugfs calls to keep mainline funcs clean, so let's go with this
>>    version over v1.
>>
>>v2: wrap feature with CONFIG_ROCKER_DEBUGFS to allow config-time
>>    enable/disable
>>
>> drivers/net/ethernet/rocker/rocker.c |  379 ++++++++++++++++++++++++++++++++++
>> drivers/net/ethernet/rocker/rocker.h |    2 +-
>> 2 files changed, 380 insertions(+), 1 deletion(-)
>>
>
> <snip>
>
>>+static int rocker_probe_debugfs_init(struct rocker *rocker)
>>+{
>>+      char dbg_dir_name[sizeof(rocker->hw.id) * 2 + 1];
>>+
>>+      sprintf(dbg_dir_name, "%*phN", (int)sizeof(rocker->hw.id),
>>+              &rocker->hw.id);
>
> You can use PCI address here. Might be better.

I wanted it to be same label as sysfs phys_switch_id and driver sign-on msg.

>
>
>>+      rocker->dbg_dir = debugfs_create_dir(dbg_dir_name, rocker_dbg_root);
>>+      if (!rocker->dbg_dir)
>
> You still check the retval here and

It's OK.  See include/linux/debugfs.h when CONFIG_DEBUG_FS=n.
debugfs_create_dir() returns ERR_PTR(-ENODEV), which is !NULL.

Seems like a nice design, debugfs.

>
>>+              return -ENOMEM;
>>+      return 0;
>>+}
>>+
>
> <snip>
>
>>+static int rocker_debugfs_init(void)
>>+{
>>+      rocker_dbg_root = debugfs_create_dir(rocker_driver_name, NULL);
>>+      if (!rocker_dbg_root)
>
> here. When debugfs is not enabled, init will fail.

Doesn't fail, same reason.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Pirko Aug. 18, 2015, 6:48 a.m. UTC | #3 
Tue, Aug 18, 2015 at 08:14:48AM CEST, sfeldma@gmail.com wrote:
>On Mon, Aug 17, 2015 at 10:55 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Tue, Aug 18, 2015 at 12:36:17AM CEST, sfeldma@gmail.com wrote:
>>>From: Scott Feldman <sfeldma@gmail.com>
>>>
>>>> tree /sys/kernel/debug/rocker
>>>/sys/kernel/debug/rocker
>>>└── 5254001235010000
>>>    ├── fdb_tbl
>>>    ├── internal_vlan_tbl
>>>    ├── neigh_tbl
>>>    ├── of_dpa_flow_tbl
>>>    └── of_dpa_group_tbl
>>>
>>>1 directory, 5 files
>>>
>>>> cat /sys/kernel/debug/rocker/5254001235010000/internal_vlan_tbl
>>>ifindex 5 ref_count 1 vlan 3843
>>>ifindex 7 ref_count 2 vlan 3840
>>>ifindex 4 ref_count 1 vlan 3842
>>>
>>>> cat /sys/kernel/debug/rocker/5254001235010000/fdb_tbl
>>>learned 1 pport 1 addr 00:02:00:00:02:00 vlan 3840
>>>learned 1 pport 2 addr 00:02:00:00:03:00 vlan 3840
>>>
>>>> cat /sys/kernel/debug/rocker/5254001235010000/neigh_tbl
>>>11.0.0.9 dev sw1p2 ref_count 3 index 1 dst 00:02:00:00:01:00 ttl_check 1
>>>11.0.0.1 dev sw1p1 ref_count 3 index 0 dst 00:02:00:00:00:00 ttl_check 1
>>>
>>>> cat /sys/kernel/debug/rocker/5254001235010000/of_dpa_flow_tbl
>>>cmd 3 cookie 15 priority 3  tbl acl            in_pport 2 01:80:c2:00:00:00/ff:ff:ff:ff:ff:f0 eth_type 0x0000 vlan_id 3841 ip proto 0/0 ip tos 0/0 group_id 0x0f010000
>>>cmd 3 cookie 2  priority 0  tbl term_mac       in_pport 1 eth_type 0x0800 52:54:00:12:35:01 vlan_id 3840 goto_tbl ucast_routing copy_to_cpu 0
>>>cmd 3 cookie 1f priority 3  tbl bridge         00:02:00:00:00:00 vlan_id 3840 tunnel_id 0 goto_tbl acl group_id 0x00000000 copy_to_cpu 0
>>>cmd 3 cookie 4  priority 1  tbl vlan           in_pport 2 vlan_id 0 goto_tbl term_mac untagged 1 new_vlan_id 3841
>>>cmd 3 cookie 20 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.1 goto_tbl acl group_id 0x20000000
>>>cmd 3 cookie 21 priority 3  tbl bridge         00:02:00:00:01:00 vlan_id 3841 tunnel_id 0 goto_tbl acl group_id 0x00000000 copy_to_cpu 0
>>>cmd 3 cookie 16 priority 2  tbl acl            in_pport 2 eth_type 0x0806 vlan_id 3841 ip proto 0/0 ip tos 0/0 group_id 0x0f010000
>>>cmd 3 cookie 12 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.0 goto_tbl acl group_id 0x0f000000
>>>cmd 3 cookie 9  priority 3  tbl acl            in_pport 1 01:80:c2:00:00:00/ff:ff:ff:ff:ff:f0 eth_type 0x0000 vlan_id 3840 ip proto 0/0 ip tos 0/0 group_id 0x0f000000
>>>cmd 3 cookie 6  priority 0  tbl term_mac       in_pport 2 eth_type 0x86dd 52:54:00:12:35:02 vlan_id 3841 goto_tbl ucast_routing copy_to_cpu 0
>>>cmd 4 cookie 0  priority 1  tbl ig_port        in_pport 0/0xffff0000 goto_tbl vlan
>>>cmd 4 cookie e  priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.3 goto_tbl acl group_id 0x0f000000
>>>cmd 3 cookie 1  priority 1  tbl vlan           in_pport 1 vlan_id 0 goto_tbl term_mac untagged 1 new_vlan_id 3840
>>>cmd 3 cookie 24 priority 20 tbl ucast_routing  eth_type 0x0800 11.0.0.4/255.255.255.252 goto_tbl acl group_id 0x20000000
>>>cmd 4 cookie 14 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.10 goto_tbl acl group_id 0x0f010000
>>>cmd 3 cookie 2c priority 20 tbl ucast_routing  eth_type 0x0800 12.0.0.4 goto_tbl acl group_id 0x20000001
>>>cmd 3 cookie 17 priority 1  tbl term_mac       in_pport 2 eth_type 0x0800 01:00:5e:00:00:00/ff:ff:ff:80:00:00 vlan_id 3841 goto_tbl mcast_routing copy_to_cpu 1
>>>cmd 3 cookie 26 priority 20 tbl ucast_routing  eth_type 0x0800 12.0.0.3 goto_tbl acl group_id 0x20000000
>>>cmd 3 cookie 2e priority 30 tbl ucast_routing  eth_type 0x0800 12.0.0.2 goto_tbl acl group_id 0x0f010000
>>>cmd 3 cookie 22 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.9 goto_tbl acl group_id 0x20000001
>>>cmd 3 cookie 1c priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.8/255.255.255.252 goto_tbl acl group_id 0x0f010000
>>>cmd 3 cookie 18 priority 1  tbl term_mac       in_pport 2 eth_type 0x86dd 33:33:00:00:00:00/ff:ff:00:00:00:00 vlan_id 3841 goto_tbl mcast_routing copy_to_cpu 1
>>>cmd 3 cookie 5  priority 0  tbl term_mac       in_pport 2 eth_type 0x0800 52:54:00:12:35:02 vlan_id 3841 goto_tbl ucast_routing copy_to_cpu 0
>>>cmd 3 cookie a  priority 2  tbl acl            in_pport 1 eth_type 0x0806 vlan_id 3840 ip proto 0/0 ip tos 0/0 group_id 0x0f000000
>>>cmd 4 cookie 1a priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.11 goto_tbl acl group_id 0x0f010000
>>>cmd 3 cookie 1e priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.8 goto_tbl acl group_id 0x0f010000
>>>cmd 3 cookie 3  priority 0  tbl term_mac       in_pport 1 eth_type 0x86dd 52:54:00:12:35:01 vlan_id 3840 goto_tbl ucast_routing copy_to_cpu 0
>>>cmd 3 cookie b  priority 1  tbl term_mac       in_pport 1 eth_type 0x0800 01:00:5e:00:00:00/ff:ff:ff:80:00:00 vlan_id 3840 goto_tbl mcast_routing copy_to_cpu 1
>>>cmd 4 cookie 8  priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.2 goto_tbl acl group_id 0x0f000000
>>>cmd 3 cookie 10 priority 0  tbl ucast_routing  eth_type 0x0800 11.0.0.0/255.255.255.252 goto_tbl acl group_id 0x0f000000
>>>cmd 3 cookie 28 priority 20 tbl ucast_routing  eth_type 0x0800 11.0.0.12/255.255.255.252 goto_tbl acl group_id 0x20000001
>>>cmd 3 cookie c  priority 1  tbl term_mac       in_pport 1 eth_type 0x86dd 33:33:00:00:00:00/ff:ff:00:00:00:00 vlan_id 3840 goto_tbl mcast_routing copy_to_cpu 1
>>>
>>>> cat /sys/kernel/debug/rocker/5254001235010000/of_dpa_group_tbl
>>>cmd 7 group_id 0x0f000000 (L2 interface vlan 3840 port 0) pop_vlan 1
>>>cmd 7 group_id 0x0f010000 (L2 interface vlan 3841 port 0) pop_vlan 1
>>>cmd 7 group_id 0x20000000 (L3 unicast index 0) src 52:54:00:12:35:01 dst 00:02:00:00:00:00 vlan 3840 ttl_check 1 group_id 0x0f000001
>>>cmd 7 group_id 0x0f010002 (L2 interface vlan 3841 port 2) pop_vlan 1
>>>cmd 7 group_id 0x0f000001 (L2 interface vlan 3840 port 1) pop_vlan 1
>>>cmd 8 group_id 0x20000001 (L3 unicast index 1) src 52:54:00:12:35:02 dst 00:02:00:00:01:00 vlan 3841 ttl_check 1 group_id 0x0f010002
>>>
>>>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>>---
>>>v3: undo v2 change: Jiri is right: debugfs calls are no-ops when
>>>    CONFIG_DEBUG_FS=n so we don't need to do anything extra in the
>>>    CONFIG_DEBUG_FS=n case.  v3 has adds a few wrapper funcs around the
>>>    debugfs calls to keep mainline funcs clean, so let's go with this
>>>    version over v1.
>>>
>>>v2: wrap feature with CONFIG_ROCKER_DEBUGFS to allow config-time
>>>    enable/disable
>>>
>>> drivers/net/ethernet/rocker/rocker.c |  379 ++++++++++++++++++++++++++++++++++
>>> drivers/net/ethernet/rocker/rocker.h |    2 +-
>>> 2 files changed, 380 insertions(+), 1 deletion(-)
>>>
>>
>> <snip>
>>
>>>+static int rocker_probe_debugfs_init(struct rocker *rocker)
>>>+{
>>>+      char dbg_dir_name[sizeof(rocker->hw.id) * 2 + 1];
>>>+
>>>+      sprintf(dbg_dir_name, "%*phN", (int)sizeof(rocker->hw.id),
>>>+              &rocker->hw.id);
>>
>> You can use PCI address here. Might be better.
>
>I wanted it to be same label as sysfs phys_switch_id and driver sign-on msg.
>
>>
>>
>>>+      rocker->dbg_dir = debugfs_create_dir(dbg_dir_name, rocker_dbg_root);
>>>+      if (!rocker->dbg_dir)
>>
>> You still check the retval here and
>
>It's OK.  See include/linux/debugfs.h when CONFIG_DEBUG_FS=n.
>debugfs_create_dir() returns ERR_PTR(-ENODEV), which is !NULL.

Does not make sense to check it for NULL then. It is never NULL...

>
>Seems like a nice design, debugfs.
>
>>
>>>+              return -ENOMEM;
>>>+      return 0;
>>>+}
>>>+
>>
>> <snip>
>>
>>>+static int rocker_debugfs_init(void)
>>>+{
>>>+      rocker_dbg_root = debugfs_create_dir(rocker_driver_name, NULL);
>>>+      if (!rocker_dbg_root)
>>
>> here. When debugfs is not enabled, init will fail.
>
>Doesn't fail, same reason.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Feldman Aug. 18, 2015, 7:21 a.m. UTC | #4 
On Mon, Aug 17, 2015 at 11:48 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Tue, Aug 18, 2015 at 08:14:48AM CEST, sfeldma@gmail.com wrote:
>>On Mon, Aug 17, 2015 at 10:55 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>> Tue, Aug 18, 2015 at 12:36:17AM CEST, sfeldma@gmail.com wrote:
>>>>From: Scott Feldman <sfeldma@gmail.com>
>>>
>>> <snip>
>>>
>>>>+      rocker->dbg_dir = debugfs_create_dir(dbg_dir_name, rocker_dbg_root);
>>>>+      if (!rocker->dbg_dir)
>>>
>>> You still check the retval here and
>>
>>It's OK.  See include/linux/debugfs.h when CONFIG_DEBUG_FS=n.
>>debugfs_create_dir() returns ERR_PTR(-ENODEV), which is !NULL.
>
> Does not make sense to check it for NULL then. It is never NULL...

It can be NULL when CONFIG_DEBUG_FS=y.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Pirko Aug. 18, 2015, 7:52 a.m. UTC | #5 
Tue, Aug 18, 2015 at 09:21:15AM CEST, sfeldma@gmail.com wrote:
>On Mon, Aug 17, 2015 at 11:48 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Tue, Aug 18, 2015 at 08:14:48AM CEST, sfeldma@gmail.com wrote:
>>>On Mon, Aug 17, 2015 at 10:55 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>>> Tue, Aug 18, 2015 at 12:36:17AM CEST, sfeldma@gmail.com wrote:
>>>>>From: Scott Feldman <sfeldma@gmail.com>
>>>>
>>>> <snip>
>>>>
>>>>>+      rocker->dbg_dir = debugfs_create_dir(dbg_dir_name, rocker_dbg_root);
>>>>>+      if (!rocker->dbg_dir)
>>>>
>>>> You still check the retval here and
>>>
>>>It's OK.  See include/linux/debugfs.h when CONFIG_DEBUG_FS=n.
>>>debugfs_create_dir() returns ERR_PTR(-ENODEV), which is !NULL.
>>
>> Does not make sense to check it for NULL then. It is never NULL...
>
>It can be NULL when CONFIG_DEBUG_FS=y.

You are right, this inconsistency looks odd to me though.

Anyway,

Acked-by: Jiri Pirko <jiri@resnulli.us>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Lunn Aug. 18, 2015, 2:01 p.m. UTC | #6 
On Mon, Aug 17, 2015 at 03:36:17PM -0700, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
> 
> > tree /sys/kernel/debug/rocker
> /sys/kernel/debug/rocker
> ????????? 5254001235010000
>     ????????? fdb_tbl
>     ????????? internal_vlan_tbl
>     ????????? neigh_tbl
>     ????????? of_dpa_flow_tbl
>     ????????? of_dpa_group_tbl
> 
> 1 directory, 5 files
> 
> > cat /sys/kernel/debug/rocker/5254001235010000/internal_vlan_tbl
> ifindex 5 ref_count 1 vlan 3843
> ifindex 7 ref_count 2 vlan 3840
> ifindex 4 ref_count 1 vlan 3842
> 
> > cat /sys/kernel/debug/rocker/5254001235010000/fdb_tbl
> learned 1 pport 1 addr 00:02:00:00:02:00 vlan 3840
> learned 1 pport 2 addr 00:02:00:00:03:00 vlan 3840
> 
> > cat /sys/kernel/debug/rocker/5254001235010000/neigh_tbl
> 11.0.0.9 dev sw1p2 ref_count 3 index 1 dst 00:02:00:00:01:00 ttl_check 1
> 11.0.0.1 dev sw1p1 ref_count 3 index 0 dst 00:02:00:00:00:00 ttl_check 1

Hi Scott

David is not so keen no debugfs stuff. He already NACKed adding more
than what is currently in DSA:

https://lkml.org/lkml/2015/7/11/8

I think the key thing from David's message is:

> Fetching information should be done by well typed, generic,
> interfaces that apply to any similar device or object.

So rather than splattering rocker stuff in debugfs, we should really
be thinking about an interface which works well for both Rocker and
DSA, i.e. generic, and keeps David happy by being typed.

No idea what that actually is, not thought about it yet.

     Andrew
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Feldman Aug. 18, 2015, 5:18 p.m. UTC | #7 
On Tue, Aug 18, 2015 at 7:01 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, Aug 17, 2015 at 03:36:17PM -0700, sfeldma@gmail.com wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> > tree /sys/kernel/debug/rocker
>> /sys/kernel/debug/rocker
>> ????????? 5254001235010000
>>     ????????? fdb_tbl
>>     ????????? internal_vlan_tbl
>>     ????????? neigh_tbl
>>     ????????? of_dpa_flow_tbl
>>     ????????? of_dpa_group_tbl
>>
>> 1 directory, 5 files
>>
>> > cat /sys/kernel/debug/rocker/5254001235010000/internal_vlan_tbl
>> ifindex 5 ref_count 1 vlan 3843
>> ifindex 7 ref_count 2 vlan 3840
>> ifindex 4 ref_count 1 vlan 3842
>>
>> > cat /sys/kernel/debug/rocker/5254001235010000/fdb_tbl
>> learned 1 pport 1 addr 00:02:00:00:02:00 vlan 3840
>> learned 1 pport 2 addr 00:02:00:00:03:00 vlan 3840
>>
>> > cat /sys/kernel/debug/rocker/5254001235010000/neigh_tbl
>> 11.0.0.9 dev sw1p2 ref_count 3 index 1 dst 00:02:00:00:01:00 ttl_check 1
>> 11.0.0.1 dev sw1p1 ref_count 3 index 0 dst 00:02:00:00:00:00 ttl_check 1
>
> Hi Scott
>
> David is not so keen no debugfs stuff. He already NACKed adding more
> than what is currently in DSA:
>
> https://lkml.org/lkml/2015/7/11/8

That patch added writable debugfs files, which I can see might be used
as a back-door to program hardware.  That does seem bad.

> I think the key thing from David's message is:
>
>> Fetching information should be done by well typed, generic,
>> interfaces that apply to any similar device or object.
>
> So rather than splattering rocker stuff in debugfs, we should really
> be thinking about an interface which works well for both Rocker and
> DSA, i.e. generic, and keeps David happy by being typed.
>
> No idea what that actually is, not thought about it yet.

Rocker debugfs files are read-only and dump internal tables that are
very specific to the (rocker) device.  I don't see how these could be
generic as each device/driver will have it's own unique data
structures.

I'm OK with not including debugfs in rocker even for these read-only
files, but I find them very useful during development/debugging, and I
could see their value in troubleshooting issues with others using
rocker.

Writable debugfs files is another story.

-scott
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Lunn Aug. 18, 2015, 6:19 p.m. UTC | #8 
On Tue, Aug 18, 2015 at 10:18:51AM -0700, Scott Feldman wrote:
> On Tue, Aug 18, 2015 at 7:01 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Mon, Aug 17, 2015 at 03:36:17PM -0700, sfeldma@gmail.com wrote:
> >> From: Scott Feldman <sfeldma@gmail.com>
> >>
> >> > tree /sys/kernel/debug/rocker
> >> /sys/kernel/debug/rocker
> >> ????????? 5254001235010000
> >>     ????????? fdb_tbl
> >>     ????????? internal_vlan_tbl
> >>     ????????? neigh_tbl
> >>     ????????? of_dpa_flow_tbl
> >>     ????????? of_dpa_group_tbl
> >>
> >> 1 directory, 5 files
> >>
> >> > cat /sys/kernel/debug/rocker/5254001235010000/internal_vlan_tbl
> >> ifindex 5 ref_count 1 vlan 3843
> >> ifindex 7 ref_count 2 vlan 3840
> >> ifindex 4 ref_count 1 vlan 3842
> >>
> >> > cat /sys/kernel/debug/rocker/5254001235010000/fdb_tbl
> >> learned 1 pport 1 addr 00:02:00:00:02:00 vlan 3840
> >> learned 1 pport 2 addr 00:02:00:00:03:00 vlan 3840
> >>
> >> > cat /sys/kernel/debug/rocker/5254001235010000/neigh_tbl
> >> 11.0.0.9 dev sw1p2 ref_count 3 index 1 dst 00:02:00:00:01:00 ttl_check 1
> >> 11.0.0.1 dev sw1p1 ref_count 3 index 0 dst 00:02:00:00:00:00 ttl_check 1
> >
> > Hi Scott
> >
> > David is not so keen no debugfs stuff. He already NACKed adding more
> > than what is currently in DSA:
> >
> > https://lkml.org/lkml/2015/7/11/8
> 
> That patch added writable debugfs files, which I can see might be used
> as a back-door to program hardware.  That does seem bad.

I fully agreed with respect to write. But if you read the whole
message, David is also not happy with read only.

I think before you spend too much more time on this, you need some
indication from David if he is going to merge it or not.

   Andrew
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Aug. 18, 2015, 6:42 p.m. UTC | #9 
From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 18 Aug 2015 07:55:55 +0200

> Tue, Aug 18, 2015 at 12:36:17AM CEST, sfeldma@gmail.com wrote:
>>+static int rocker_probe_debugfs_init(struct rocker *rocker)
>>+{
>>+	char dbg_dir_name[sizeof(rocker->hw.id) * 2 + 1];
>>+
>>+	sprintf(dbg_dir_name, "%*phN", (int)sizeof(rocker->hw.id),
>>+		&rocker->hw.id);
> 
> You can use PCI address here. Might be better.

This absolutely will not be unique in a multi-domain PCI configuration.

It is arguable whether that matters or not under qemu, but it's a bad
precedence to say that PCI addresses will be unique, they absolutely
are not unique between devices even of the same exact type.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Feldman Aug. 18, 2015, 8:37 p.m. UTC | #10 
On Tue, Aug 18, 2015 at 11:19 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Tue, Aug 18, 2015 at 10:18:51AM -0700, Scott Feldman wrote:
>> On Tue, Aug 18, 2015 at 7:01 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > On Mon, Aug 17, 2015 at 03:36:17PM -0700, sfeldma@gmail.com wrote:
>> >> From: Scott Feldman <sfeldma@gmail.com>
>> >>
>> >> > tree /sys/kernel/debug/rocker
>> >> /sys/kernel/debug/rocker
>> >> ????????? 5254001235010000
>> >>     ????????? fdb_tbl
>> >>     ????????? internal_vlan_tbl
>> >>     ????????? neigh_tbl
>> >>     ????????? of_dpa_flow_tbl
>> >>     ????????? of_dpa_group_tbl
>> >>
>> >> 1 directory, 5 files
>> >>
>> >> > cat /sys/kernel/debug/rocker/5254001235010000/internal_vlan_tbl
>> >> ifindex 5 ref_count 1 vlan 3843
>> >> ifindex 7 ref_count 2 vlan 3840
>> >> ifindex 4 ref_count 1 vlan 3842
>> >>
>> >> > cat /sys/kernel/debug/rocker/5254001235010000/fdb_tbl
>> >> learned 1 pport 1 addr 00:02:00:00:02:00 vlan 3840
>> >> learned 1 pport 2 addr 00:02:00:00:03:00 vlan 3840
>> >>
>> >> > cat /sys/kernel/debug/rocker/5254001235010000/neigh_tbl
>> >> 11.0.0.9 dev sw1p2 ref_count 3 index 1 dst 00:02:00:00:01:00 ttl_check 1
>> >> 11.0.0.1 dev sw1p1 ref_count 3 index 0 dst 00:02:00:00:00:00 ttl_check 1
>> >
>> > Hi Scott
>> >
>> > David is not so keen no debugfs stuff. He already NACKed adding more
>> > than what is currently in DSA:
>> >
>> > https://lkml.org/lkml/2015/7/11/8
>>
>> That patch added writable debugfs files, which I can see might be used
>> as a back-door to program hardware.  That does seem bad.
>
> I fully agreed with respect to write. But if you read the whole
> message, David is also not happy with read only.
>
> I think before you spend too much more time on this, you need some
> indication from David if he is going to merge it or not.

David, please give us guidance on debugfs in drivers/net.  Is there
some criteria we can define to know when it's OK to use debugfs?

-scott
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Aug. 18, 2015, 8:47 p.m. UTC | #11 
From: Scott Feldman <sfeldma@gmail.com>
Date: Tue, 18 Aug 2015 13:37:56 -0700

> On Tue, Aug 18, 2015 at 11:19 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>> On Tue, Aug 18, 2015 at 10:18:51AM -0700, Scott Feldman wrote:
>>> On Tue, Aug 18, 2015 at 7:01 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>>> > On Mon, Aug 17, 2015 at 03:36:17PM -0700, sfeldma@gmail.com wrote:
>>> >> From: Scott Feldman <sfeldma@gmail.com>
>>> >>
>>> >> > tree /sys/kernel/debug/rocker
>>> >> /sys/kernel/debug/rocker
>>> >> ????????? 5254001235010000
>>> >>     ????????? fdb_tbl
>>> >>     ????????? internal_vlan_tbl
>>> >>     ????????? neigh_tbl
>>> >>     ????????? of_dpa_flow_tbl
>>> >>     ????????? of_dpa_group_tbl
>>> >>
>>> >> 1 directory, 5 files
>>> >>
>>> >> > cat /sys/kernel/debug/rocker/5254001235010000/internal_vlan_tbl
>>> >> ifindex 5 ref_count 1 vlan 3843
>>> >> ifindex 7 ref_count 2 vlan 3840
>>> >> ifindex 4 ref_count 1 vlan 3842
>>> >>
>>> >> > cat /sys/kernel/debug/rocker/5254001235010000/fdb_tbl
>>> >> learned 1 pport 1 addr 00:02:00:00:02:00 vlan 3840
>>> >> learned 1 pport 2 addr 00:02:00:00:03:00 vlan 3840
>>> >>
>>> >> > cat /sys/kernel/debug/rocker/5254001235010000/neigh_tbl
>>> >> 11.0.0.9 dev sw1p2 ref_count 3 index 1 dst 00:02:00:00:01:00 ttl_check 1
>>> >> 11.0.0.1 dev sw1p1 ref_count 3 index 0 dst 00:02:00:00:00:00 ttl_check 1
>>> >
>>> > Hi Scott
>>> >
>>> > David is not so keen no debugfs stuff. He already NACKed adding more
>>> > than what is currently in DSA:
>>> >
>>> > https://lkml.org/lkml/2015/7/11/8
>>>
>>> That patch added writable debugfs files, which I can see might be used
>>> as a back-door to program hardware.  That does seem bad.
>>
>> I fully agreed with respect to write. But if you read the whole
>> message, David is also not happy with read only.
>>
>> I think before you spend too much more time on this, you need some
>> indication from David if he is going to merge it or not.
> 
> David, please give us guidance on debugfs in drivers/net.  Is there
> some criteria we can define to know when it's OK to use debugfs?

The less you use it the better, seriously.

I see some drivers where the foo_debugfs.c file is larger than the rest
of the driver.  Once people start using it, it's like crack, and they
dump every single debugging widget they found useful at some point into
there.

This is not what we want.  Most things I see in debugfs support was
probably useful for debugging one particular bug but then it was never
really useful again in the future.  Those kinds of things can be done
locally in someone's tree.

I often see various kinds of "statistics" ending up in these things,
or register dumps, both of which are 'ethtool' or similar material.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Feldman Aug. 19, 2015, 7:46 p.m. UTC | #12 
On Tue, Aug 18, 2015 at 1:47 PM, David Miller <davem@davemloft.net> wrote:
> From: Scott Feldman <sfeldma@gmail.com>
> Date: Tue, 18 Aug 2015 13:37:56 -0700
>

>>>> > Hi Scott
>>>> >
>>>> > David is not so keen no debugfs stuff. He already NACKed adding more
>>>> > than what is currently in DSA:
>>>> >
>>>> > https://lkml.org/lkml/2015/7/11/8
>>>>
>>>> That patch added writable debugfs files, which I can see might be used
>>>> as a back-door to program hardware.  That does seem bad.
>>>
>>> I fully agreed with respect to write. But if you read the whole
>>> message, David is also not happy with read only.
>>>
>>> I think before you spend too much more time on this, you need some
>>> indication from David if he is going to merge it or not.
>>
>> David, please give us guidance on debugfs in drivers/net.  Is there
>> some criteria we can define to know when it's OK to use debugfs?
>
> The less you use it the better, seriously.
>
> I see some drivers where the foo_debugfs.c file is larger than the rest
> of the driver.  Once people start using it, it's like crack, and they
> dump every single debugging widget they found useful at some point into
> there.
>
> This is not what we want.  Most things I see in debugfs support was
> probably useful for debugging one particular bug but then it was never
> really useful again in the future.  Those kinds of things can be done
> locally in someone's tree.
>
> I often see various kinds of "statistics" ending up in these things,
> or register dumps, both of which are 'ethtool' or similar material.

# git grep debugfs_create drivers/net

^^^  this is scary.  I see some crazy things being done here.   The
writable nodes look like workaround driver/device bugs or to provide
backdoor interfaces that don't exist natively.

I say we clean up this mess.  Just eliminating the writable files
would force bugs to get fixed and get new interfaces defined.  And
replace readable files when interface exist (stats/reg).  Finally,
look for readable files that can be converted to new shared common
interfaces.  What's left should be read-only (S_IRUGO) files (no
binary blobs) containing data unique for driver/device useful for
field troubleshooting.

I'm motivated.  Next net-next cycle I'm going to go down the list with
a big eraser.  I'm sure I'll be a popular guy.

-scott
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Murali Karicheri March 17, 2016, 8:10 p.m. UTC | #13 
David,

On 08/18/2015 04:47 PM, David Miller wrote:
> I see some drivers where the foo_debugfs.c file is larger than the rest
> of the driver.  Once people start using it, it's like crack, and they
> dump every single debugging widget they found useful at some point into
> there.
> 
> This is not what we want.  Most things I see in debugfs support was
> probably useful for debugging one particular bug but then it was never
> really useful again in the future.  Those kinds of things can be done
> locally in someone's tree.
> 
> I often see various kinds of "statistics" ending up in these things,
> or register dumps, both of which are 'ethtool' or similar material.
Very late to this discussion, but I need to port some of the internal code
to display the content of a ALE (Address Learning Engine) table maintained
in hardwareat L2 layer. Currently I have a sysfs implementation that dumps
information like below.

root@k2e-evm:~# cat /sys/devices/platform/soc/2620110.netcp/ale_table
index 0, raw: 000007fc d000ffff ffffffff, type: addr(1), addr: ff:ff:ff:ff:ff:ff, mcstate: f(3), port mask: 1ff, no super
index 1, raw: 00000000 10000800 28329a1c, type: addr(1), addr: 08:00:28:32:9a:1c, uctype: persistent(0), port: 0
index 2, raw: 000007fc d0000100 5e000001, type: addr(1), addr: 01:00:5e:00:00:01, mcstate: f(3), port mask: 1ff, no super
index 19, raw: 00000004 d000d4be d93db6c1, type: addr(1), addr: d4:be:d9:3d:b6:c1, uctype: touched(3), port: 1

What is the available interface in kernel to expose this information
to user space as debugfs is not suggested based on this thread?
Andrew Lunn March 17, 2016, 8:25 p.m. UTC | #14 
On Thu, Mar 17, 2016 at 04:10:31PM -0400, Murali Karicheri wrote:
> David,
> 
> On 08/18/2015 04:47 PM, David Miller wrote:
> > I see some drivers where the foo_debugfs.c file is larger than the rest
> > of the driver.  Once people start using it, it's like crack, and they
> > dump every single debugging widget they found useful at some point into
> > there.
> > 
> > This is not what we want.  Most things I see in debugfs support was
> > probably useful for debugging one particular bug but then it was never
> > really useful again in the future.  Those kinds of things can be done
> > locally in someone's tree.
> > 
> > I often see various kinds of "statistics" ending up in these things,
> > or register dumps, both of which are 'ethtool' or similar material.
> Very late to this discussion, but I need to port some of the internal code
> to display the content of a ALE (Address Learning Engine) table maintained
> in hardwareat L2 layer. Currently I have a sysfs implementation that dumps
> information like below.
> 
> root@k2e-evm:~# cat /sys/devices/platform/soc/2620110.netcp/ale_table
> index 0, raw: 000007fc d000ffff ffffffff, type: addr(1), addr: ff:ff:ff:ff:ff:ff, mcstate: f(3), port mask: 1ff, no super
> index 1, raw: 00000000 10000800 28329a1c, type: addr(1), addr: 08:00:28:32:9a:1c, uctype: persistent(0), port: 0
> index 2, raw: 000007fc d0000100 5e000001, type: addr(1), addr: 01:00:5e:00:00:01, mcstate: f(3), port mask: 1ff, no super
> index 19, raw: 00000004 d000d4be d93db6c1, type: addr(1), addr: d4:be:d9:3d:b6:c1, uctype: touched(3), port: 1
> 
> What is the available interface in kernel to expose this information
> to user space as debugfs is not suggested based on this thread?

This looks a lot like what the mv88e6xxx_port_fdb_dump() callback
returns to DSA when SWITCHDEV_OBJ_ID_PORT_FDB is passed to
switchdev_port_obj_dump() in the switchdev ops.

	  Andrew
Murali Karicheri March 17, 2016, 8:48 p.m. UTC | #15 
On 03/17/2016 04:25 PM, Andrew Lunn wrote:
> On Thu, Mar 17, 2016 at 04:10:31PM -0400, Murali Karicheri wrote:
>> David,
>>
>> On 08/18/2015 04:47 PM, David Miller wrote:
>>> I see some drivers where the foo_debugfs.c file is larger than the rest
>>> of the driver.  Once people start using it, it's like crack, and they
>>> dump every single debugging widget they found useful at some point into
>>> there.
>>>
>>> This is not what we want.  Most things I see in debugfs support was
>>> probably useful for debugging one particular bug but then it was never
>>> really useful again in the future.  Those kinds of things can be done
>>> locally in someone's tree.
>>>
>>> I often see various kinds of "statistics" ending up in these things,
>>> or register dumps, both of which are 'ethtool' or similar material.
>> Very late to this discussion, but I need to port some of the internal code
>> to display the content of a ALE (Address Learning Engine) table maintained
>> in hardwareat L2 layer. Currently I have a sysfs implementation that dumps
>> information like below.
>>
>> root@k2e-evm:~# cat /sys/devices/platform/soc/2620110.netcp/ale_table
>> index 0, raw: 000007fc d000ffff ffffffff, type: addr(1), addr: ff:ff:ff:ff:ff:ff, mcstate: f(3), port mask: 1ff, no super
>> index 1, raw: 00000000 10000800 28329a1c, type: addr(1), addr: 08:00:28:32:9a:1c, uctype: persistent(0), port: 0
>> index 2, raw: 000007fc d0000100 5e000001, type: addr(1), addr: 01:00:5e:00:00:01, mcstate: f(3), port mask: 1ff, no super
>> index 19, raw: 00000004 d000d4be d93db6c1, type: addr(1), addr: d4:be:d9:3d:b6:c1, uctype: touched(3), port: 1
>>
>> What is the available interface in kernel to expose this information
>> to user space as debugfs is not suggested based on this thread?
> 
> This looks a lot like what the mv88e6xxx_port_fdb_dump() callback
> returns to DSA when SWITCHDEV_OBJ_ID_PORT_FDB is passed to
> switchdev_port_obj_dump() in the switchdev ops.
> 
Andrew,

Which tool user has to use to get this dump once implemented?

Murali

> 	  Andrew
>
Florian Fainelli March 17, 2016, 8:56 p.m. UTC | #16 
On March 17, 2016 1:10:31 PM PDT, Murali Karicheri <m-karicheri2@ti.com> wrote:
>David,
>
>On 08/18/2015 04:47 PM, David Miller wrote:
>> I see some drivers where the foo_debugfs.c file is larger than the
>rest
>> of the driver.  Once people start using it, it's like crack, and they
>> dump every single debugging widget they found useful at some point
>into
>> there.
>> 
>> This is not what we want.  Most things I see in debugfs support was
>> probably useful for debugging one particular bug but then it was
>never
>> really useful again in the future.  Those kinds of things can be done
>> locally in someone's tree.
>> 
>> I often see various kinds of "statistics" ending up in these things,
>> or register dumps, both of which are 'ethtool' or similar material.
>Very late to this discussion, but I need to port some of the internal
>code
>to display the content of a ALE (Address Learning Engine) table
>maintained
>in hardwareat L2 layer. Currently I have a sysfs implementation that
>dumps
>information like below.
>
>root@k2e-evm:~# cat /sys/devices/platform/soc/2620110.netcp/ale_table
>index 0, raw: 000007fc d000ffff ffffffff, type: addr(1), addr:
>ff:ff:ff:ff:ff:ff, mcstate: f(3), port mask: 1ff, no super
>index 1, raw: 00000000 10000800 28329a1c, type: addr(1), addr:
>08:00:28:32:9a:1c, uctype: persistent(0), port: 0
>index 2, raw: 000007fc d0000100 5e000001, type: addr(1), addr:
>01:00:5e:00:00:01, mcstate: f(3), port mask: 1ff, no super
>index 19, raw: 00000004 d000d4be d93db6c1, type: addr(1), addr:
>d4:be:d9:3d:b6:c1, uctype: touched(3), port: 1
>
>What is the available interface in kernel to expose this information
>to user space as debugfs is not suggested based on this thread

Using something like bridge fdb show (bridge sub command part of iproute2) would give you most of what you need here, except the port mask (since that is semi implied by which interface you use for dumping) and internal flags about the address validity.

How frequently is this code used once you have a proper switch driver which supports the FDB add/del/dump operations?
Ido Schimmel March 17, 2016, 9:53 p.m. UTC | #17 
Thu, Mar 17, 2016 at 10:25:19PM IST, andrew@lunn.ch wrote:
>On Thu, Mar 17, 2016 at 04:10:31PM -0400, Murali Karicheri wrote:
>> David,
>> 
>> On 08/18/2015 04:47 PM, David Miller wrote:
>> > I see some drivers where the foo_debugfs.c file is larger than the rest
>> > of the driver.  Once people start using it, it's like crack, and they
>> > dump every single debugging widget they found useful at some point into
>> > there.
>> > 
>> > This is not what we want.  Most things I see in debugfs support was
>> > probably useful for debugging one particular bug but then it was never
>> > really useful again in the future.  Those kinds of things can be done
>> > locally in someone's tree.
>> > 
>> > I often see various kinds of "statistics" ending up in these things,
>> > or register dumps, both of which are 'ethtool' or similar material.
>> Very late to this discussion, but I need to port some of the internal code
>> to display the content of a ALE (Address Learning Engine) table maintained
>> in hardwareat L2 layer. Currently I have a sysfs implementation that dumps
>> information like below.
>> 
>> root@k2e-evm:~# cat /sys/devices/platform/soc/2620110.netcp/ale_table
>> index 0, raw: 000007fc d000ffff ffffffff, type: addr(1), addr: ff:ff:ff:ff:ff:ff, mcstate: f(3), port mask: 1ff, no super
>> index 1, raw: 00000000 10000800 28329a1c, type: addr(1), addr: 08:00:28:32:9a:1c, uctype: persistent(0), port: 0
>> index 2, raw: 000007fc d0000100 5e000001, type: addr(1), addr: 01:00:5e:00:00:01, mcstate: f(3), port mask: 1ff, no super
>> index 19, raw: 00000004 d000d4be d93db6c1, type: addr(1), addr: d4:be:d9:3d:b6:c1, uctype: touched(3), port: 1
>> 
>> What is the available interface in kernel to expose this information
>> to user space as debugfs is not suggested based on this thread?
>
>This looks a lot like what the mv88e6xxx_port_fdb_dump() callback
>returns to DSA when SWITCHDEV_OBJ_ID_PORT_FDB is passed to
>switchdev_port_obj_dump() in the switchdev ops.

+1

Also, Murali, using standard interfaces instead of debugfs will allow
you to:

1) Upstream your code
2) Use existing tests for your code. In particular, the following
(which is used for mlxsw testing):

https://github.com/jpirko/lnst/blob/master/recipes/switchdev/l2-002-bridge_fdb.py

There are a bunch of others there which you'll probably find useful.

BTW, are you familiar with the following document?
https://www.kernel.org/doc/Documentation/networking/switchdev.txt
I believe it answers your question.

Good luck!

>
>	  Andrew
Florian Fainelli March 17, 2016, 11:18 p.m. UTC | #18 
On 17/03/16 13:48, Murali Karicheri wrote:
> On 03/17/2016 04:25 PM, Andrew Lunn wrote:
>> On Thu, Mar 17, 2016 at 04:10:31PM -0400, Murali Karicheri wrote:
>>> David,
>>>
>>> On 08/18/2015 04:47 PM, David Miller wrote:
>>>> I see some drivers where the foo_debugfs.c file is larger than the rest
>>>> of the driver.  Once people start using it, it's like crack, and they
>>>> dump every single debugging widget they found useful at some point into
>>>> there.
>>>>
>>>> This is not what we want.  Most things I see in debugfs support was
>>>> probably useful for debugging one particular bug but then it was never
>>>> really useful again in the future.  Those kinds of things can be done
>>>> locally in someone's tree.
>>>>
>>>> I often see various kinds of "statistics" ending up in these things,
>>>> or register dumps, both of which are 'ethtool' or similar material.
>>> Very late to this discussion, but I need to port some of the internal code
>>> to display the content of a ALE (Address Learning Engine) table maintained
>>> in hardwareat L2 layer. Currently I have a sysfs implementation that dumps
>>> information like below.
>>>
>>> root@k2e-evm:~# cat /sys/devices/platform/soc/2620110.netcp/ale_table
>>> index 0, raw: 000007fc d000ffff ffffffff, type: addr(1), addr: ff:ff:ff:ff:ff:ff, mcstate: f(3), port mask: 1ff, no super
>>> index 1, raw: 00000000 10000800 28329a1c, type: addr(1), addr: 08:00:28:32:9a:1c, uctype: persistent(0), port: 0
>>> index 2, raw: 000007fc d0000100 5e000001, type: addr(1), addr: 01:00:5e:00:00:01, mcstate: f(3), port mask: 1ff, no super
>>> index 19, raw: 00000004 d000d4be d93db6c1, type: addr(1), addr: d4:be:d9:3d:b6:c1, uctype: touched(3), port: 1
>>>
>>> What is the available interface in kernel to expose this information
>>> to user space as debugfs is not suggested based on this thread?
>>
>> This looks a lot like what the mv88e6xxx_port_fdb_dump() callback
>> returns to DSA when SWITCHDEV_OBJ_ID_PORT_FDB is passed to
>> switchdev_port_obj_dump() in the switchdev ops.
>>
> Andrew,
> 
> Which tool user has to use to get this dump once implemented?

iproute2's bridge command with bridge fdb show <interface>.
Murali Karicheri March 18, 2016, 5:25 p.m. UTC | #19 
On 03/17/2016 05:53 PM, Ido Schimmel wrote:
> Thu, Mar 17, 2016 at 10:25:19PM IST, andrew@lunn.ch wrote:
>> On Thu, Mar 17, 2016 at 04:10:31PM -0400, Murali Karicheri wrote:
>>> David,
>>>
>>> On 08/18/2015 04:47 PM, David Miller wrote:
>>>> I see some drivers where the foo_debugfs.c file is larger than the rest
>>>> of the driver.  Once people start using it, it's like crack, and they
>>>> dump every single debugging widget they found useful at some point into
>>>> there.
>>>>
>>>> This is not what we want.  Most things I see in debugfs support was
>>>> probably useful for debugging one particular bug but then it was never
>>>> really useful again in the future.  Those kinds of things can be done
>>>> locally in someone's tree.
>>>>
>>>> I often see various kinds of "statistics" ending up in these things,
>>>> or register dumps, both of which are 'ethtool' or similar material.
>>> Very late to this discussion, but I need to port some of the internal code
>>> to display the content of a ALE (Address Learning Engine) table maintained
>>> in hardwareat L2 layer. Currently I have a sysfs implementation that dumps
>>> information like below.
>>>
>>> root@k2e-evm:~# cat /sys/devices/platform/soc/2620110.netcp/ale_table
>>> index 0, raw: 000007fc d000ffff ffffffff, type: addr(1), addr: ff:ff:ff:ff:ff:ff, mcstate: f(3), port mask: 1ff, no super
>>> index 1, raw: 00000000 10000800 28329a1c, type: addr(1), addr: 08:00:28:32:9a:1c, uctype: persistent(0), port: 0
>>> index 2, raw: 000007fc d0000100 5e000001, type: addr(1), addr: 01:00:5e:00:00:01, mcstate: f(3), port mask: 1ff, no super
>>> index 19, raw: 00000004 d000d4be d93db6c1, type: addr(1), addr: d4:be:d9:3d:b6:c1, uctype: touched(3), port: 1
>>>
>>> What is the available interface in kernel to expose this information
>>> to user space as debugfs is not suggested based on this thread?
>>
>> This looks a lot like what the mv88e6xxx_port_fdb_dump() callback
>> returns to DSA when SWITCHDEV_OBJ_ID_PORT_FDB is passed to
>> switchdev_port_obj_dump() in the switchdev ops.
> 
> +1
> 
> Also, Murali, using standard interfaces instead of debugfs will allow
> you to:
> 
> 1) Upstream your code
> 2) Use existing tests for your code. In particular, the following
> (which is used for mlxsw testing):
> 
> https://github.com/jpirko/lnst/blob/master/recipes/switchdev/l2-002-bridge_fdb.py
> 
> There are a bunch of others there which you'll probably find useful.
> 
> BTW, are you familiar with the following document?
> https://www.kernel.org/doc/Documentation/networking/switchdev.txt
> I believe it answers your question.

Yes. I have support for DSA and switchdev for netcp driver in my TODO list.
Will post more questions on this once I start work on this.

Thanks

Murali
> 
> Good luck!
> 
>>
>> 	  Andrew
>
Murali Karicheri March 18, 2016, 5:29 p.m. UTC | #20 
On 03/17/2016 07:18 PM, Florian Fainelli wrote:
> On 17/03/16 13:48, Murali Karicheri wrote:
>> On 03/17/2016 04:25 PM, Andrew Lunn wrote:
>>> On Thu, Mar 17, 2016 at 04:10:31PM -0400, Murali Karicheri wrote:
>>>> David,
>>>>
>>>> On 08/18/2015 04:47 PM, David Miller wrote:
>>>>> I see some drivers where the foo_debugfs.c file is larger than the rest
>>>>> of the driver.  Once people start using it, it's like crack, and they
>>>>> dump every single debugging widget they found useful at some point into
>>>>> there.
>>>>>
>>>>> This is not what we want.  Most things I see in debugfs support was
>>>>> probably useful for debugging one particular bug but then it was never
>>>>> really useful again in the future.  Those kinds of things can be done
>>>>> locally in someone's tree.
>>>>>
>>>>> I often see various kinds of "statistics" ending up in these things,
>>>>> or register dumps, both of which are 'ethtool' or similar material.
>>>> Very late to this discussion, but I need to port some of the internal code
>>>> to display the content of a ALE (Address Learning Engine) table maintained
>>>> in hardwareat L2 layer. Currently I have a sysfs implementation that dumps
>>>> information like below.
>>>>
>>>> root@k2e-evm:~# cat /sys/devices/platform/soc/2620110.netcp/ale_table
>>>> index 0, raw: 000007fc d000ffff ffffffff, type: addr(1), addr: ff:ff:ff:ff:ff:ff, mcstate: f(3), port mask: 1ff, no super
>>>> index 1, raw: 00000000 10000800 28329a1c, type: addr(1), addr: 08:00:28:32:9a:1c, uctype: persistent(0), port: 0
>>>> index 2, raw: 000007fc d0000100 5e000001, type: addr(1), addr: 01:00:5e:00:00:01, mcstate: f(3), port mask: 1ff, no super
>>>> index 19, raw: 00000004 d000d4be d93db6c1, type: addr(1), addr: d4:be:d9:3d:b6:c1, uctype: touched(3), port: 1
>>>>
>>>> What is the available interface in kernel to expose this information
>>>> to user space as debugfs is not suggested based on this thread?
>>>
>>> This looks a lot like what the mv88e6xxx_port_fdb_dump() callback
>>> returns to DSA when SWITCHDEV_OBJ_ID_PORT_FDB is passed to
>>> switchdev_port_obj_dump() in the switchdev ops.
>>>
>> Andrew,
>>
>> Which tool user has to use to get this dump once implemented?
> 
> iproute2's bridge command with bridge fdb show <interface>.
> 
Thanks
diff mbox

Patch

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index a7cb74a..c14f99f 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -18,6 +18,8 @@ 
 #include <linux/spinlock.h>
 #include <linux/hashtable.h>
 #include <linux/crc32.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
 #include <linux/sort.h>
 #include <linux/random.h>
 #include <linux/netdevice.h>
@@ -43,6 +45,8 @@ 
 
 static const char rocker_driver_name[] = "rocker";
 
+static struct dentry *rocker_dbg_root;
+
 static const struct pci_device_id rocker_pci_id_table[] = {
 	{PCI_VDEVICE(REDHAT, PCI_DEVICE_ID_REDHAT_ROCKER), 0},
 	{0, }
@@ -238,6 +242,7 @@  struct rocker {
 	struct {
 		u64 id;
 	} hw;
+	struct dentry *dbg_dir;
 	spinlock_t cmd_ring_lock;		/* for cmd ring accesses */
 	struct rocker_dma_ring_info cmd_ring;
 	struct rocker_dma_ring_info event_ring;
@@ -2363,22 +2368,351 @@  static int rocker_cmd_group_tbl_del(const struct rocker_port *rocker_port,
  * Flow, group, FDB, internal VLAN and neigh tables
  ***************************************************/
 
+static int dbg_of_dpa_flow_tbl_read(struct seq_file *file, void *data)
+{
+	struct rocker *rocker = file->private;
+	struct rocker_flow_tbl_entry *entry;
+	unsigned long flags;
+	int bkt;
+
+	char *tbl_id_to_str(enum rocker_of_dpa_table_id tbl_id)
+	{
+		switch (tbl_id) {
+		case ROCKER_OF_DPA_TABLE_ID_INGRESS_PORT:
+			return "ig_port";
+		case ROCKER_OF_DPA_TABLE_ID_VLAN:
+			return "vlan";
+		case ROCKER_OF_DPA_TABLE_ID_TERMINATION_MAC:
+			return "term_mac";
+		case ROCKER_OF_DPA_TABLE_ID_UNICAST_ROUTING:
+			return "ucast_routing";
+		case ROCKER_OF_DPA_TABLE_ID_MULTICAST_ROUTING:
+			return "mcast_routing";
+		case ROCKER_OF_DPA_TABLE_ID_BRIDGING:
+			return "bridge";
+		case ROCKER_OF_DPA_TABLE_ID_ACL_POLICY:
+			return "acl";
+		}
+		return "unknown";
+	}
+
+	spin_lock_irqsave(&rocker->flow_tbl_lock, flags);
+
+	hash_for_each(rocker->flow_tbl, bkt, entry, entry) {
+		struct rocker_flow_tbl_key *key = &entry->key;
+
+		seq_printf(file, "cmd %d cookie %llx\tpriority %-3d tbl %-15s",
+			   entry->cmd, entry->cookie,
+			   key->priority,
+			   tbl_id_to_str(key->tbl_id));
+		switch (key->tbl_id) {
+		case ROCKER_OF_DPA_TABLE_ID_INGRESS_PORT:
+			seq_printf(file, "in_pport %d",
+				   key->ig_port.in_pport);
+			if (key->ig_port.in_pport_mask != 0xffffffff)
+				seq_printf(file, "/0x%08x",
+					   key->ig_port.in_pport_mask);
+			seq_printf(file, " goto_tbl %s\n",
+				   tbl_id_to_str(key->ig_port.goto_tbl));
+			break;
+		case ROCKER_OF_DPA_TABLE_ID_VLAN:
+			seq_printf(file, "in_pport %d vlan_id %d",
+				   key->vlan.in_pport,
+				   be16_to_cpu(key->vlan.vlan_id));
+			if (key->vlan.vlan_id_mask != 0xffff)
+				seq_printf(file, "/0x%04x",
+					   be16_to_cpu(key->vlan.vlan_id_mask));
+			seq_printf(file, " goto_tbl %s untagged %d new_vlan_id %d\n",
+				   tbl_id_to_str(key->vlan.goto_tbl),
+				   key->vlan.untagged,
+				   be16_to_cpu(key->vlan.new_vlan_id));
+			break;
+		case ROCKER_OF_DPA_TABLE_ID_TERMINATION_MAC:
+			seq_printf(file, "in_pport %d",
+				   key->term_mac.in_pport);
+			if (key->term_mac.in_pport_mask != 0xffffffff)
+				seq_printf(file, "/0x%08x",
+					   key->term_mac.in_pport_mask);
+			seq_printf(file, " eth_type 0x%04x %pM",
+				   be16_to_cpu(key->term_mac.eth_type),
+				   key->term_mac.eth_dst);
+			if (!is_broadcast_ether_addr(
+			    key->term_mac.eth_dst_mask))
+				seq_printf(file, "/%pM",
+					   key->term_mac.eth_dst_mask);
+			seq_printf(file, " vlan_id %d",
+				   be16_to_cpu(key->term_mac.vlan_id));
+			if (key->term_mac.vlan_id_mask != 0xffff)
+				seq_printf(file, "/0x%04x",
+					   be16_to_cpu(
+					   key->term_mac.vlan_id_mask));
+			seq_printf(file, " goto_tbl %s copy_to_cpu %d\n",
+				   tbl_id_to_str(key->term_mac.goto_tbl),
+				   key->term_mac.copy_to_cpu);
+			break;
+		case ROCKER_OF_DPA_TABLE_ID_UNICAST_ROUTING:
+			seq_printf(file, "eth_type 0x%04x %pI4",
+				   be16_to_cpu(key->ucast_routing.eth_type),
+				   &key->ucast_routing.dst4);
+			if (key->ucast_routing.dst4_mask != 0xffffffff)
+				seq_printf(file, "/%pI4",
+					   &key->ucast_routing.dst4_mask);
+			seq_printf(file, " goto_tbl %s group_id 0x%08x\n",
+				   tbl_id_to_str(key->ucast_routing.goto_tbl),
+				   key->ucast_routing.group_id);
+			break;
+		case ROCKER_OF_DPA_TABLE_ID_BRIDGING:
+			if (key->bridge.has_eth_dst) {
+				seq_printf(file, "%pM",
+					   key->bridge.eth_dst);
+				if (key->bridge.has_eth_dst_mask)
+					seq_printf(file, "/%pM ",
+						   key->bridge.eth_dst_mask);
+				else
+					seq_puts(file, " ");
+			}
+			seq_printf(file, "vlan_id %d tunnel_id %d goto_tbl %s group_id 0x%08x copy_to_cpu %d\n",
+				   be16_to_cpu(key->bridge.vlan_id),
+				   key->bridge.tunnel_id,
+				   tbl_id_to_str(key->bridge.goto_tbl),
+				   key->bridge.group_id,
+				   key->bridge.copy_to_cpu);
+			break;
+		case ROCKER_OF_DPA_TABLE_ID_ACL_POLICY:
+			seq_printf(file, "in_pport %d",
+				   key->acl.in_pport);
+			if (key->acl.in_pport_mask != 0xffffffff)
+				seq_printf(file, "/0x%08x",
+					   key->acl.in_pport_mask);
+			if (!is_zero_ether_addr(key->acl.eth_src)) {
+				seq_printf(file, " %pM",
+					   key->acl.eth_src);
+				if (!is_broadcast_ether_addr(
+				    key->acl.eth_src_mask))
+					seq_printf(file, "/%pM",
+						   key->acl.eth_src_mask);
+			}
+			if (!is_zero_ether_addr(key->acl.eth_dst)) {
+				seq_printf(file, " %pM",
+					   key->acl.eth_dst);
+				if (!is_broadcast_ether_addr(
+				    key->acl.eth_dst_mask))
+					seq_printf(file, "/%pM",
+						   key->acl.eth_dst_mask);
+			}
+			seq_printf(file, " eth_type 0x%04x",
+				   be16_to_cpu(key->acl.eth_type));
+			seq_printf(file, " vlan_id %d",
+				   be16_to_cpu(key->acl.vlan_id));
+			if (key->acl.vlan_id_mask != 0xffff)
+				seq_printf(file, "/0x%04x",
+					   be16_to_cpu(key->acl.vlan_id_mask));
+			seq_printf(file, " ip proto %d", key->acl.ip_proto);
+			if (key->acl.ip_proto_mask != 0xff)
+				seq_printf(file, "/%d", key->acl.ip_proto_mask);
+			seq_printf(file, " ip tos %d", key->acl.ip_tos);
+			if (key->acl.ip_tos_mask != 0xff)
+				seq_printf(file, "/%d", key->acl.ip_tos_mask);
+			seq_printf(file, " group_id 0x%08x\n",
+				   key->acl.group_id);
+			break;
+		default:
+			seq_puts(file, "\n");
+			break;
+		}
+	}
+
+	spin_unlock_irqrestore(&rocker->flow_tbl_lock, flags);
+
+	return 0;
+}
+
+static int dbg_of_dpa_group_tbl_read(struct seq_file *file, void *data)
+{
+	struct rocker *rocker = file->private;
+	struct rocker_group_tbl_entry *entry;
+	unsigned long flags;
+	u32 *group_id;
+	int bkt;
+	int i;
+
+	spin_lock_irqsave(&rocker->group_tbl_lock, flags);
+
+	hash_for_each(rocker->group_tbl, bkt, entry, entry) {
+		seq_printf(file, "cmd %d group_id 0x%08x",
+			   entry->cmd, entry->group_id);
+		switch (ROCKER_GROUP_TYPE_GET(entry->group_id)) {
+		case ROCKER_OF_DPA_GROUP_TYPE_L2_INTERFACE:
+			seq_printf(file, " (L2 interface vlan %d port %d)",
+				   ROCKER_GROUP_VLAN_GET(entry->group_id),
+				   ROCKER_GROUP_PORT_GET(entry->group_id));
+			seq_printf(file, " pop_vlan %d",
+				   entry->l2_interface.pop_vlan);
+			break;
+		case ROCKER_OF_DPA_GROUP_TYPE_L2_REWRITE:
+			seq_printf(file, " (L2 rewrite index %d)",
+				   ROCKER_GROUP_INDEX_LONG_GET(
+				   entry->group_id));
+			if (!is_zero_ether_addr(entry->l2_rewrite.eth_src))
+				seq_printf(file, " src %pM",
+					   entry->l2_rewrite.eth_src);
+			if (!is_zero_ether_addr(entry->l2_rewrite.eth_dst))
+				seq_printf(file, " dst %pM",
+					   entry->l2_rewrite.eth_dst);
+			if (entry->l2_rewrite.vlan_id)
+				seq_printf(file, " vlan %d",
+					   be16_to_cpu(
+					   entry->l2_rewrite.vlan_id));
+			seq_printf(file, " group_id 0x%08x",
+				   entry->l2_rewrite.group_id);
+			break;
+		case ROCKER_OF_DPA_GROUP_TYPE_L2_FLOOD:
+			seq_printf(file, " (L2 flood vlan %d index %d)",
+				   ROCKER_GROUP_VLAN_GET(entry->group_id),
+				   ROCKER_GROUP_INDEX_GET(entry->group_id));
+			break;
+		case ROCKER_OF_DPA_GROUP_TYPE_L2_MCAST:
+			seq_printf(file, " (L2 multicast vlan %d index %d)",
+				   ROCKER_GROUP_VLAN_GET(entry->group_id),
+				   ROCKER_GROUP_INDEX_GET(entry->group_id));
+			break;
+		case ROCKER_OF_DPA_GROUP_TYPE_L3_UCAST:
+			seq_printf(file, " (L3 unicast index %d)",
+				   ROCKER_GROUP_INDEX_LONG_GET(
+				   entry->group_id));
+			if (!is_zero_ether_addr(entry->l3_unicast.eth_src))
+				seq_printf(file, " src %pM",
+					   entry->l3_unicast.eth_src);
+			if (!is_zero_ether_addr(entry->l3_unicast.eth_dst))
+				seq_printf(file, " dst %pM",
+					   entry->l3_unicast.eth_dst);
+			if (entry->l3_unicast.vlan_id)
+				seq_printf(file, " vlan %d",
+					   be16_to_cpu(
+					   entry->l3_unicast.vlan_id));
+			seq_printf(file, " ttl_check %d",
+				   entry->l3_unicast.ttl_check);
+			seq_printf(file, " group_id 0x%08x",
+				   entry->l3_unicast.group_id);
+			break;
+		}
+		for (i = 0, group_id = entry->group_ids;
+		     i < entry->group_count;
+		     i++, group_id++)
+			seq_printf(file, "%s0x%08x%s",
+				   i == 0 ? " {" : " ",
+				   *group_id,
+				   i + 1 == entry->group_count ? "}" : "");
+		seq_puts(file, "\n");
+	}
+
+	spin_unlock_irqrestore(&rocker->group_tbl_lock, flags);
+
+	return 0;
+}
+
+static int dbg_fdb_tbl_read(struct seq_file *file, void *data)
+{
+	struct rocker *rocker = file->private;
+	struct rocker_fdb_tbl_entry *entry;
+	unsigned long flags;
+	int bkt;
+
+	spin_lock_irqsave(&rocker->fdb_tbl_lock, flags);
+
+	hash_for_each(rocker->fdb_tbl, bkt, entry, entry)
+		seq_printf(file, "learned %d pport %d addr %pM vlan %d\n",
+			   entry->learned, entry->key.pport,
+			   entry->key.addr, be16_to_cpu(entry->key.vlan_id));
+
+	spin_unlock_irqrestore(&rocker->fdb_tbl_lock, flags);
+
+	return 0;
+}
+
+static int dbg_internal_vlan_tbl_read(struct seq_file *file, void *data)
+{
+	struct rocker *rocker = file->private;
+	struct rocker_internal_vlan_tbl_entry *entry;
+	unsigned long flags;
+	int bkt;
+
+	spin_lock_irqsave(&rocker->internal_vlan_tbl_lock, flags);
+
+	hash_for_each(rocker->internal_vlan_tbl, bkt, entry, entry)
+		seq_printf(file, "ifindex %d ref_count %d vlan %d\n",
+			   entry->ifindex, entry->ref_count,
+			   be16_to_cpu(entry->vlan_id));
+
+	spin_unlock_irqrestore(&rocker->internal_vlan_tbl_lock, flags);
+
+	return 0;
+}
+
+static int dbg_neigh_tbl_read(struct seq_file *file, void *data)
+{
+	struct rocker *rocker = file->private;
+	struct rocker_neigh_tbl_entry *entry;
+	unsigned long flags;
+	int bkt;
+
+	spin_lock_irqsave(&rocker->neigh_tbl_lock, flags);
+
+	hash_for_each(rocker->neigh_tbl, bkt, entry, entry)
+		seq_printf(file, "%pI4 dev %s ref_count %d index %d dst %pM ttl_check %d\n",
+			   &entry->ip_addr, entry->dev->name, entry->ref_count,
+			   entry->index, entry->eth_dst, entry->ttl_check);
+
+	spin_unlock_irqrestore(&rocker->neigh_tbl_lock, flags);
+
+	return 0;
+}
+
+#define ROCKER_DBG_TBL(name) \
+	static int dbg_##name##_open(struct inode *inode, struct file *f) \
+	{                                                                 \
+		struct rocker *rocker = inode->i_private;                 \
+		return single_open(f, dbg_##name##_read, rocker);         \
+	}                                                                 \
+	static const struct file_operations dbg_##name##_ops = {          \
+		.owner = THIS_MODULE,                                     \
+		.open = dbg_##name##_open,                                \
+		.release = single_release,                                \
+		.read = seq_read,                                         \
+		.llseek = seq_lseek                                       \
+	}
+
+ROCKER_DBG_TBL(of_dpa_flow_tbl);
+ROCKER_DBG_TBL(of_dpa_group_tbl);
+ROCKER_DBG_TBL(fdb_tbl);
+ROCKER_DBG_TBL(internal_vlan_tbl);
+ROCKER_DBG_TBL(neigh_tbl);
+
 static int rocker_init_tbls(struct rocker *rocker)
 {
+#define ROCKER_CREATE_DBG_FILE(name)                                      \
+	debugfs_create_file(#name, S_IRUGO, rocker->dbg_dir, rocker,      \
+			    &dbg_##name##_ops)
+
 	hash_init(rocker->flow_tbl);
 	spin_lock_init(&rocker->flow_tbl_lock);
+	ROCKER_CREATE_DBG_FILE(of_dpa_flow_tbl);
 
 	hash_init(rocker->group_tbl);
 	spin_lock_init(&rocker->group_tbl_lock);
+	ROCKER_CREATE_DBG_FILE(of_dpa_group_tbl);
 
 	hash_init(rocker->fdb_tbl);
 	spin_lock_init(&rocker->fdb_tbl_lock);
+	ROCKER_CREATE_DBG_FILE(fdb_tbl);
 
 	hash_init(rocker->internal_vlan_tbl);
 	spin_lock_init(&rocker->internal_vlan_tbl_lock);
+	ROCKER_CREATE_DBG_FILE(internal_vlan_tbl);
 
 	hash_init(rocker->neigh_tbl);
 	spin_lock_init(&rocker->neigh_tbl_lock);
+	ROCKER_CREATE_DBG_FILE(neigh_tbl);
 
 	return 0;
 }
@@ -5090,6 +5424,23 @@  static void rocker_msix_fini(const struct rocker *rocker)
 	kfree(rocker->msix_entries);
 }
 
+static int rocker_probe_debugfs_init(struct rocker *rocker)
+{
+	char dbg_dir_name[sizeof(rocker->hw.id) * 2 + 1];
+
+	sprintf(dbg_dir_name, "%*phN", (int)sizeof(rocker->hw.id),
+		&rocker->hw.id);
+	rocker->dbg_dir = debugfs_create_dir(dbg_dir_name, rocker_dbg_root);
+	if (!rocker->dbg_dir)
+		return -ENOMEM;
+	return 0;
+}
+
+static void rocker_probe_debugfs_fini(struct rocker *rocker)
+{
+	debugfs_remove_recursive(rocker->dbg_dir);
+}
+
 static int rocker_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	struct rocker *rocker;
@@ -5182,6 +5533,12 @@  static int rocker_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	rocker->hw.id = rocker_read64(rocker, SWITCH_ID);
 
+	err = rocker_probe_debugfs_init(rocker);
+	if (err) {
+		dev_err(&pdev->dev, "cannot init debugfs\n");
+		goto err_dbg_dir;
+	}
+
 	err = rocker_init_tbls(rocker);
 	if (err) {
 		dev_err(&pdev->dev, "cannot init rocker tables\n");
@@ -5202,6 +5559,8 @@  static int rocker_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 err_probe_ports:
 	rocker_free_tbls(rocker);
 err_init_tbls:
+	rocker_probe_debugfs_fini(rocker);
+err_dbg_dir:
 	free_irq(rocker_msix_vector(rocker, ROCKER_MSIX_VEC_EVENT), rocker);
 err_request_event_irq:
 	free_irq(rocker_msix_vector(rocker, ROCKER_MSIX_VEC_CMD), rocker);
@@ -5228,6 +5587,7 @@  static void rocker_remove(struct pci_dev *pdev)
 	struct rocker *rocker = pci_get_drvdata(pdev);
 
 	rocker_free_tbls(rocker);
+	rocker_probe_debugfs_fini(rocker);
 	rocker_write32(rocker, CONTROL, ROCKER_CONTROL_RESET);
 	rocker_remove_ports(rocker);
 	free_irq(rocker_msix_vector(rocker, ROCKER_MSIX_VEC_EVENT), rocker);
@@ -5426,18 +5786,36 @@  static struct notifier_block rocker_netevent_nb __read_mostly = {
  * Module init and exit
  ***********************/
 
+static int rocker_debugfs_init(void)
+{
+	rocker_dbg_root = debugfs_create_dir(rocker_driver_name, NULL);
+	if (!rocker_dbg_root)
+		return -ENOMEM;
+	return 0;
+}
+
+static void rocker_debugfs_fini(void)
+{
+	debugfs_remove_recursive(rocker_dbg_root);
+}
+
 static int __init rocker_module_init(void)
 {
 	int err;
 
 	register_netdevice_notifier(&rocker_netdevice_nb);
 	register_netevent_notifier(&rocker_netevent_nb);
+	err = rocker_debugfs_init();
+	if (err)
+		goto err_debugfs;
 	err = pci_register_driver(&rocker_pci_driver);
 	if (err)
 		goto err_pci_register_driver;
 	return 0;
 
 err_pci_register_driver:
+	rocker_debugfs_fini();
+err_debugfs:
 	unregister_netevent_notifier(&rocker_netevent_nb);
 	unregister_netdevice_notifier(&rocker_netdevice_nb);
 	return err;
@@ -5445,6 +5823,7 @@  err_pci_register_driver:
 
 static void __exit rocker_module_exit(void)
 {
+	rocker_debugfs_fini();
 	unregister_netevent_notifier(&rocker_netevent_nb);
 	unregister_netdevice_notifier(&rocker_netdevice_nb);
 	pci_unregister_driver(&rocker_pci_driver);
diff --git a/drivers/net/ethernet/rocker/rocker.h b/drivers/net/ethernet/rocker/rocker.h
index 12490b2..0eacc53 100644
--- a/drivers/net/ethernet/rocker/rocker.h
+++ b/drivers/net/ethernet/rocker/rocker.h
@@ -419,7 +419,7 @@  enum rocker_of_dpa_overlay_type {
 #define ROCKER_GROUP_TYPE_SET(type) \
 	(((type) << ROCKER_GROUP_TYPE_SHIFT) & ROCKER_GROUP_TYPE_MASK)
 #define ROCKER_GROUP_VLAN_GET(group_id) \
-	(((group_id) & ROCKER_GROUP_VLAN_ID_MASK) >> ROCKER_GROUP_VLAN_ID_SHIFT)
+	(((group_id) & ROCKER_GROUP_VLAN_MASK) >> ROCKER_GROUP_VLAN_SHIFT)
 #define ROCKER_GROUP_VLAN_SET(vlan_id) \
 	(((vlan_id) << ROCKER_GROUP_VLAN_SHIFT) & ROCKER_GROUP_VLAN_MASK)
 #define ROCKER_GROUP_PORT_GET(group_id) \