From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f170.google.com ([209.85.128.170]:36141 "EHLO mail-wr0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752569AbeCXRmv (ORCPT ); Sat, 24 Mar 2018 13:42:51 -0400 Received: by mail-wr0-f170.google.com with SMTP id d10so15018568wrf.3 for ; Sat, 24 Mar 2018 10:42:50 -0700 (PDT) Date: Sat, 24 Mar 2018 18:42:48 +0100 From: Jiri Pirko To: Florian Fainelli Cc: Andrew Lunn , netdev@vger.kernel.org, davem@davemloft.net, idosch@mellanox.com, jakub.kicinski@netronome.com, mlxsw@mellanox.com, vivien.didelot@savoirfairelinux.com, michael.chan@broadcom.com, ganeshgr@chelsio.com, saeedm@mellanox.com, simon.horman@netronome.com, pieter.jansenvanvuuren@netronome.com, john.hurley@netronome.com, dirk.vandermerwe@netronome.com, alexander.h.duyck@intel.com, ogerlitz@mellanox.com, dsahern@gmail.com, vijaya.guvva@cavium.com, satananda.burla@cavium.com, raghu.vatsavayi@cavium.com, felix.manlunas@cavium.com, gospo@broadcom.com, sathya.perla@broadcom.com, vasundhara-v.volam@broadcom.com, tariqt@mellanox.com, eranbe@mellanox.com, jeffrey.t.kirsher@intel.com Subject: Re: [patch net-next RFC 00/12] devlink: introduce port flavours and common phys_port_name generation Message-ID: <20180324174248.GH1891@nanopsycho> References: <20180322105522.8186-1-jiri@resnulli.us> <20180323134357.GG5145@lunn.ch> <20180323145935.GC2125@nanopsycho> <20180323152412.GC24361@lunn.ch> <20180324074551.GD1891@nanopsycho> <0B8BC202-9067-4D34-AC4F-79DC33B4E6F9@gmail.com> <20180324160749.GG1891@nanopsycho> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Sat, Mar 24, 2018 at 06:07:27PM CET, f.fainelli@gmail.com wrote: >On March 24, 2018 9:07:49 AM PDT, Jiri Pirko wrote: >>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 >>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. > >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.