Sat, Mar 24, 2018 at 06:07:27PM CET, f.faine...@gmail.com wrote: >On March 24, 2018 9:07:49 AM PDT, Jiri Pirko <j...@resnulli.us> wrote: >>Sat, Mar 24, 2018 at 03:35:09PM CET, f.faine...@gmail.com wrote: >>>On March 24, 2018 12:45:51 AM PDT, Jiri Pirko <j...@resnulli.us> >>wrote: >>>>Fri, Mar 23, 2018 at 04:24:12PM CET, and...@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, and...@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. > >Ok, if we look back at the history of the changes I had implemented >ndo_phys_port_id() to return the port number initially, but this was wrong and >reverted based on your feedback, and then ndo_phys_port_name() was implemented >with your reviewed-by tag: > >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/net/dsa/slave.c?id=3a543ef479868e36c95935de320608a7e41466ca >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/net/dsa/slave.c?id=592050b2541407d033da18226d3644644832d082 >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/net/dsa/slave.c?id=592050b2541407d033da18226d3644644832d082 > >Now that this is reported through sysfs it unfortunately becomes ABI and we >should not be breaking user applications relying on that and there is at least >one I know of... > >What is an appropriate attribute to use to return the physical port number >within a given switch?
I don't think there's one out there. I tried to add it in this patchset.
