Re: [patch net-next RFC 00/12] devlink: introduce port flavours and common phys_port_name generation

[Jiri Pirko <jiri@resnulli.us>]

Sat, Mar 24, 2018 at 03:35:09PM CET, f.fainelli@gmail.com wrote:
>On March 24, 2018 12:45:51 AM PDT, Jiri Pirko <jiri@resnulli.us> wrote:
>>Fri, Mar 23, 2018 at 04:24:12PM CET, andrew@lunn.ch wrote:
>>>On Fri, Mar 23, 2018 at 03:59:35PM +0100, Jiri Pirko wrote:
>>>> Fri, Mar 23, 2018 at 02:43:57PM CET, andrew@lunn.ch wrote:
>>>> >> I tested this for mlxsw and nfp. I have no way to test this on
>>DSA hw,
>>>> >> I would really appretiate DSA guys to test this.
>>>> >
>>>> >Hi Jiri
>>>> >
>>>> >With the missing break added, i get:
>>>> >
>>>> >root@zii-devel-b:~# ./iproute2/devlink/devlink port 
>>>> >mdio_bus/0.1:00/0: type eth netdev lan0 flavour physical number 0
>>>> >mdio_bus/0.1:00/1: type eth netdev lan1 flavour physical number 1
>>>> >mdio_bus/0.1:00/2: type eth netdev lan2 flavour physical number 2
>>>> >mdio_bus/0.1:00/3: type notset
>>>> >mdio_bus/0.1:00/4: type notset
>>>> >mdio_bus/0.1:00/5: type notset flavour dsa number 5
>>>> >mdio_bus/0.1:00/6: type notset flavour cpu number 6
>>>> >mdio_bus/0.2:00/0: type eth netdev lan3 flavour physical number 0
>>>> >mdio_bus/0.2:00/1: type eth netdev lan4 flavour physical number 1
>>>> >mdio_bus/0.2:00/2: type eth netdev lan5 flavour physical number 2
>>>> >mdio_bus/0.2:00/3: type notset
>>>> >mdio_bus/0.2:00/4: type notset
>>>> >mdio_bus/0.2:00/5: type notset flavour dsa number 5
>>>> >mdio_bus/0.2:00/6: type notset flavour dsa number 6
>>>> >mdio_bus/0.4:00/0: type eth netdev lan6 flavour physical number 0
>>>> >mdio_bus/0.4:00/1: type eth netdev lan7 flavour physical number 1
>>>> >mdio_bus/0.4:00/2: type eth netdev lan8 flavour physical number 2
>>>> >mdio_bus/0.4:00/3: type eth netdev optical3 flavour physical number
>>3
>>>> >mdio_bus/0.4:00/4: type eth netdev optical4 flavour physical number
>>4
>>>> >mdio_bus/0.4:00/5: type notset
>>>> >mdio_bus/0.4:00/6: type notset
>>>> >mdio_bus/0.4:00/7: type notset
>>>> >mdio_bus/0.4:00/8: type notset
>>>> >mdio_bus/0.4:00/9: type notset flavour dsa number 9
>>>
>>>> That is basically front panel number for physical ports.
>>>
>>>You cannot make that assumption. As you can see here, we have 3 ports
>>>with the number 0.
>>>
>>>Look at clearfog, armada-388-clearfog.dts. port 0=lan5, port 1=lan4
>>>port 2=lan3, port 3=lan2, port 4=lan1, port 5=cpu, port 6=lan6.
>>>
>>>The hardware and mechanical engineer is free to wire switch ports to
>>>the front panel however they want. That is why we put the netdev name
>>>in device tree.
>>
>>Got it. Hmm, so I think that the port number can be made optional and
>>when it is present, it would be used to generate phys_port_name. If
>>not, perhaps devlink port index could be used instead.
>>
>>So iiuc, you don't really need phys_port_name in dsa, as it provides
>>misleading names, right? Why is it implemented then?
>
>We do need phys_port_name because there are switch configuration operations, e.g: ethtool::rxnfc which take a port number and queue number as part of the action to redirect a packet for instance. There is no way to obtain this physical port number other than either knowing it and hard coding it (not great) or scanning the device tree and look for the "reg" property value. phys_port_name gets you that and it is easy for an application to scan /sys/class/net/ on startup to get to know that (and other stuff as well).

Hmm. That sounds like misuse of phys_port_name. The original purpose was
to provide names as they are actually written on the front panel.


>
>-- 
>Florian