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? -- Florian
