Re: [PATCH net-next v2 01/10] net: dsa: add debugfs interface

From: Jiri Pirko
Date: Mon Aug 28 2017 - 16:05:09 EST


Mon, Aug 28, 2017 at 09:58:12PM CEST, f.fainelli@xxxxxxxxx wrote:
>On 08/28/2017 12:50 PM, Jiri Pirko wrote:
>> Mon, Aug 28, 2017 at 09:17:39PM CEST, vivien.didelot@xxxxxxxxxxxxxxxxxxxx wrote:
>>> This commit adds a DEBUG_FS dependent DSA core file creating a generic
>>> debug filesystem interface for the DSA switch devices.
>>>
>>> The interface can be mounted with:
>>>
>>> # mount -t debugfs none /sys/kernel/debug
>>>
>>> The dsa directory contains one directory per switch chip:
>>>
>>> # cd /sys/kernel/debug/dsa/
>>> # ls
>>> switch0 switch1 switch2
>>>
>>> Each chip directory contains one directory per port:
>>>
>>> # ls -l switch0/
>>> drwxr-xr-x 2 root root 0 Jan 1 00:00 port0
>>> drwxr-xr-x 2 root root 0 Jan 1 00:00 port1
>>> drwxr-xr-x 2 root root 0 Jan 1 00:00 port2
>>> drwxr-xr-x 2 root root 0 Jan 1 00:00 port5
>>> drwxr-xr-x 2 root root 0 Jan 1 00:00 port6
>>>
>>> Future patches will add entry files to these directories.
>>>
>>> Signed-off-by: Vivien Didelot <vivien.didelot@xxxxxxxxxxxxxxxxxxxx>
>>
>> Oh no, no debugfs please!
>>
>> What do you need to expose? I'm sure we can find out some generic, well
>> defined and reusable way.
>
>We have no CPU or DSA (cross switches) net_device reprensentors because
>those would be two ends of the same pipe so it would be both confusing

So? That is certainly not an argument for debugfs. Just have all ports
as devlink port, and you can introduce special new kind of port for cpu
port. Note that devlink port does not have to have netdev association.


>and a duplication. For a CPU interface, one side goes to the switch, the
>other one is the master net_device (normal Ethernet MAC). For a DSA
>interface, one interface is on one switch, and the other is on the other
>switch.
>
>If you look at the patch series it's pretty obvious what is being exposed :)

Sure. But lets use existing interfaces and extend them if needed. Please
don't use some made-up debugfs mess. That is never the correct answer :/