| Message ID | 20180322105522.8186-1-jiri@resnulli.us |
|---|---|
| Headers | show |
| Series | devlink: introduce port flavours and common phys_port_name generation | expand |
On Thu, Mar 22, 2018 at 3:55 AM, Jiri Pirko <jiri@resnulli.us> wrote: > From: Jiri Pirko <jiri@mellanox.com> > > This patchset resolves 2 issues we have right now: > 1) There are many netdevices / ports in the system, for port, pf, vf > represenatation but the user has no way to see which is which > 2) The ndo_get_phys_port_name is implemented in each driver separatelly, > which may lead to inconsistent names between drivers. > > This patchset introduces port flavours which should address the first > problem. I'm testing this with Netronome nfp hardware. When the user > has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: > # devlink port > pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 > pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0 > pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1 > pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008 > pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0 > pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1 > pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2 > pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3 > > The indexes are weird numbers now. That needs to be fixed. Also, netdev > renaming does not work correctly for me now for some reason. > Also, there is one extra port that I don't understand what > is the purpose for it - something nfp specific perhaps. > > The desired output should look like this: > # devlink port > pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 > pci/0000:05:00.0/1: type eth netdev enp5s0np1 flavour physical number 1 > pci/0000:05:00.0/2: type eth netdev enp5s0npf0 flavour pf_rep number 0 > pci/0000:05:00.0/3: type eth netdev enp5s0nvf0 flavour vf_rep number 0 > pci/0000:05:00.0/4: type eth netdev enp5s0nvf1 flavour vf_rep number 1 > pci/0000:05:00.0/5: type eth netdev enp5s0nvf2 flavour vf_rep number 2 > pci/0000:05:00.0/6: type eth netdev enp5s0nvf3 flavour vf_rep number 3 > > As you can see, the netdev names are generated according to the flavour > and port number. In case the port is split, the split subnumber is also > included. > > 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. Thanks! > nice series, I like that the user can query a ports flavor (I get this ask all the time). thanks
Thu, Mar 22, 2018 at 03:40:02PM CET, roopa@cumulusnetworks.com wrote: >On Thu, Mar 22, 2018 at 3:55 AM, Jiri Pirko <jiri@resnulli.us> wrote: >> From: Jiri Pirko <jiri@mellanox.com> >> >> This patchset resolves 2 issues we have right now: >> 1) There are many netdevices / ports in the system, for port, pf, vf >> represenatation but the user has no way to see which is which >> 2) The ndo_get_phys_port_name is implemented in each driver separatelly, >> which may lead to inconsistent names between drivers. >> >> This patchset introduces port flavours which should address the first >> problem. I'm testing this with Netronome nfp hardware. When the user >> has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: >> # devlink port >> pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 >> pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0 >> pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1 >> pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008 >> pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0 >> pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1 >> pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2 >> pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3 >> >> The indexes are weird numbers now. That needs to be fixed. Also, netdev >> renaming does not work correctly for me now for some reason. >> Also, there is one extra port that I don't understand what >> is the purpose for it - something nfp specific perhaps. >> >> The desired output should look like this: >> # devlink port >> pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 >> pci/0000:05:00.0/1: type eth netdev enp5s0np1 flavour physical number 1 >> pci/0000:05:00.0/2: type eth netdev enp5s0npf0 flavour pf_rep number 0 >> pci/0000:05:00.0/3: type eth netdev enp5s0nvf0 flavour vf_rep number 0 >> pci/0000:05:00.0/4: type eth netdev enp5s0nvf1 flavour vf_rep number 1 >> pci/0000:05:00.0/5: type eth netdev enp5s0nvf2 flavour vf_rep number 2 >> pci/0000:05:00.0/6: type eth netdev enp5s0nvf3 flavour vf_rep number 3 >> >> As you can see, the netdev names are generated according to the flavour >> and port number. In case the port is split, the split subnumber is also >> included. >> >> 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. Thanks! >> > >nice series, I like that the user can query a ports flavor (I get this >ask all the time). Yeah, it is really needed. I would like to fix this jungle so all drivers behave the same. Started with nfp as they are leading with what they have implemented. But I expect others to join in (please). Many drivers just create devlink instance without any ports. Odd. I will write some Documentation file as a part of this patchset. Also, I'm thinking about adding some warnings in care driver does some crippled implementation.
On 3/22/18 4:55 AM, Jiri Pirko wrote: > From: Jiri Pirko <jiri@mellanox.com> > > This patchset resolves 2 issues we have right now: > 1) There are many netdevices / ports in the system, for port, pf, vf > represenatation but the user has no way to see which is which > 2) The ndo_get_phys_port_name is implemented in each driver separatelly, > which may lead to inconsistent names between drivers. Similar to ndo_get_phys_port_{name,id}, devlink requires drivers to opt in with an implementation right, so you can't really force a solution to the consistent naming. > > This patchset introduces port flavours which should address the first > problem. I'm testing this with Netronome nfp hardware. When the user > has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: > # devlink port > pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 > pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0 > pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1 > pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008 > pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0 > pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1 > pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2 > pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3 How about 'kind' instead of flavo{u}r?
On Thu, Mar 22, 2018 at 8:34 AM, David Ahern <dsahern@gmail.com> wrote: > On 3/22/18 4:55 AM, Jiri Pirko wrote: >> From: Jiri Pirko <jiri@mellanox.com> >> >> This patchset resolves 2 issues we have right now: >> 1) There are many netdevices / ports in the system, for port, pf, vf >> represenatation but the user has no way to see which is which >> 2) The ndo_get_phys_port_name is implemented in each driver separatelly, >> which may lead to inconsistent names between drivers. > > Similar to ndo_get_phys_port_{name,id}, devlink requires drivers to opt > in with an implementation right, so you can't really force a solution to > the consistent naming. > >> >> This patchset introduces port flavours which should address the first >> problem. I'm testing this with Netronome nfp hardware. When the user >> has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: >> # devlink port >> pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 >> pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0 >> pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1 >> pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008 >> pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0 >> pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1 >> pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2 >> pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3 > > How about 'kind' instead of flavo{u}r? > 'kind' is good too...but to not confuse it with logical device rtnetlink 'kind', maybe 'physical_kind' or 'port_kind' or 'phys_kind' or any-other-better--prefix-to-kind would be preferable
Thu, Mar 22, 2018 at 04:34:07PM CET, dsahern@gmail.com wrote: >On 3/22/18 4:55 AM, Jiri Pirko wrote: >> From: Jiri Pirko <jiri@mellanox.com> >> >> This patchset resolves 2 issues we have right now: >> 1) There are many netdevices / ports in the system, for port, pf, vf >> represenatation but the user has no way to see which is which >> 2) The ndo_get_phys_port_name is implemented in each driver separatelly, >> which may lead to inconsistent names between drivers. > >Similar to ndo_get_phys_port_{name,id}, devlink requires drivers to opt >in with an implementation right, so you can't really force a solution to >the consistent naming. Yeah, drivers would still have free choice to implemen the ndo themselves. But most of them, like all sriov switch drivers should use the devlink helper to have consistent naming. In other words, devlink helper should be the standard way, in weird cases (like rocker), driver implements it himself. > >> >> This patchset introduces port flavours which should address the first >> problem. I'm testing this with Netronome nfp hardware. When the user >> has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: >> # devlink port >> pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 >> pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0 >> pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1 >> pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008 >> pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0 >> pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1 >> pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2 >> pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3 > >How about 'kind' instead of flavo{u}r? Yeah, kind is often used in kernel already with different meaning git grep kind net/core I wanted to avoid confusions >
On 3/22/18 11:49 AM, Jiri Pirko wrote: > Thu, Mar 22, 2018 at 04:34:07PM CET, dsahern@gmail.com wrote: >> On 3/22/18 4:55 AM, Jiri Pirko wrote: >>> From: Jiri Pirko <jiri@mellanox.com> >>> >>> This patchset resolves 2 issues we have right now: >>> 1) There are many netdevices / ports in the system, for port, pf, vf >>> represenatation but the user has no way to see which is which >>> 2) The ndo_get_phys_port_name is implemented in each driver separatelly, >>> which may lead to inconsistent names between drivers. >> >> Similar to ndo_get_phys_port_{name,id}, devlink requires drivers to opt >> in with an implementation right, so you can't really force a solution to >> the consistent naming. > > Yeah, drivers would still have free choice to implemen the ndo > themselves. But most of them, like all sriov switch drivers should use > the devlink helper to have consistent naming. In other words, devlink > helper should be the standard way, in weird cases (like rocker), driver > implements it himself. That's an assumption that somehow the devlink API will be better supported than ndo_get_phys_port_{name,id}. Don't get me wrong -- an API to show the kind of device is needed, but I do not think this enforces any kind of consistency in naming. > > >> >>> >>> This patchset introduces port flavours which should address the first >>> problem. I'm testing this with Netronome nfp hardware. When the user >>> has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: >>> # devlink port >>> pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 >>> pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0 >>> pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1 >>> pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008 >>> pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0 >>> pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1 >>> pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2 >>> pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3 >> >> How about 'kind' instead of flavo{u}r? > > Yeah, kind is often used in kernel already with different meaning > git grep kind net/core > I wanted to avoid confusions Roopa's amendment works as well; I just think flavor / flavour is the wrong word. Make me thinks of food ... ice cream vs netdevices.
On Thu, Mar 22, 2018 at 01:10:38PM -0600, David Ahern wrote: > On 3/22/18 11:49 AM, Jiri Pirko wrote: > > Thu, Mar 22, 2018 at 04:34:07PM CET, dsahern@gmail.com wrote: > >> On 3/22/18 4:55 AM, Jiri Pirko wrote: > >>> From: Jiri Pirko <jiri@mellanox.com> > >>> > >>> This patchset resolves 2 issues we have right now: > >>> 1) There are many netdevices / ports in the system, for port, pf, vf > >>> represenatation but the user has no way to see which is which > >>> 2) The ndo_get_phys_port_name is implemented in each driver separatelly, > >>> which may lead to inconsistent names between drivers. > >> > >> Similar to ndo_get_phys_port_{name,id}, devlink requires drivers to opt > >> in with an implementation right, so you can't really force a solution to > >> the consistent naming. > > > > Yeah, drivers would still have free choice to implemen the ndo > > themselves. But most of them, like all sriov switch drivers should use > > the devlink helper to have consistent naming. In other words, devlink > > helper should be the standard way, in weird cases (like rocker), driver > > implements it himself. > > That's an assumption that somehow the devlink API will be better > supported than ndo_get_phys_port_{name,id}. Don't get me wrong -- an API > to show the kind of device is needed, but I do not think this enforces > any kind of consistency in naming. > > > > > > >> > >>> > >>> This patchset introduces port flavours which should address the first > >>> problem. I'm testing this with Netronome nfp hardware. When the user > >>> has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: > >>> # devlink port > >>> pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 > >>> pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0 > >>> pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1 > >>> pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008 > >>> pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0 > >>> pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1 > >>> pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2 > >>> pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3 > >> > >> How about 'kind' instead of flavo{u}r? > > > > Yeah, kind is often used in kernel already with different meaning > > git grep kind net/core > > I wanted to avoid confusions > > Roopa's amendment works as well; I just think flavor / flavour is the > wrong word. Make me thinks of food ... ice cream vs netdevices. Naming it a 'subtype' could also work. It's a bit longer than 'kind' (no longer than 'flavour') and accurately describes the characteristic of this port. It also avoids the namespace collision of 'kind' that Jiri points out. It also fits with the names used in the PCI world with vendor:device and subsystem vendor:subsystem device naming used there for further granularity.
On Thu, Mar 22, 2018 at 11:55:10AM +0100, Jiri Pirko wrote: > From: Jiri Pirko <jiri@mellanox.com> > > This patchset resolves 2 issues we have right now: > 1) There are many netdevices / ports in the system, for port, pf, vf > represenatation but the user has no way to see which is which > 2) The ndo_get_phys_port_name is implemented in each driver separatelly, > which may lead to inconsistent names between drivers. > > This patchset introduces port flavours which should address the first > problem. I'm testing this with Netronome nfp hardware. When the user > has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: > # devlink port > pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 > pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0 > pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1 > pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008 > pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0 > pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1 > pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2 > pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3 Hi Jiri Can you point me to towards your iproute2 patch to make use of the new properties. > The indexes are weird numbers now. That needs to be fixed. Also, netdev > renaming does not work correctly for me now for some reason. > Also, there is one extra port that I don't understand what > is the purpose for it - something nfp specific perhaps. > > The desired output should look like this: > # devlink port > pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 > pci/0000:05:00.0/1: type eth netdev enp5s0np1 flavour physical number 1 > pci/0000:05:00.0/2: type eth netdev enp5s0npf0 flavour pf_rep number 0 > pci/0000:05:00.0/3: type eth netdev enp5s0nvf0 flavour vf_rep number 0 > pci/0000:05:00.0/4: type eth netdev enp5s0nvf1 flavour vf_rep number 1 > pci/0000:05:00.0/5: type eth netdev enp5s0nvf2 flavour vf_rep number 2 > pci/0000:05:00.0/6: type eth netdev enp5s0nvf3 flavour vf_rep number 3 > > As you can see, the netdev names are generated according to the flavour > and port number. In case the port is split, the split subnumber is also > included. This naming is something we won't want with DSA. We get the port names from device tree, and they are supposed to reflect the name of the port printed on the label on the case. So this is often wan, lan0, lan1, lan2, lan3, etc. That is much more intuitive than enp5s0. Andrew
On Thu, 22 Mar 2018 11:55:10 +0100, Jiri Pirko wrote: > Also, there is one extra port that I don't understand what > is the purpose for it - something nfp specific perhaps. Do you mean the PF netdev? There can be multiple of those on multi-host cards. There is one pf_repr from ASIC's perspective and a full-blown PF netdev which should be used by applications. pf_repr is only for switch config.
Thu, Mar 22, 2018 at 08:10:38PM CET, dsahern@gmail.com wrote: >On 3/22/18 11:49 AM, Jiri Pirko wrote: >> Thu, Mar 22, 2018 at 04:34:07PM CET, dsahern@gmail.com wrote: >>> On 3/22/18 4:55 AM, Jiri Pirko wrote: >>>> From: Jiri Pirko <jiri@mellanox.com> >>>> >>>> This patchset resolves 2 issues we have right now: >>>> 1) There are many netdevices / ports in the system, for port, pf, vf >>>> represenatation but the user has no way to see which is which >>>> 2) The ndo_get_phys_port_name is implemented in each driver separatelly, >>>> which may lead to inconsistent names between drivers. >>> >>> Similar to ndo_get_phys_port_{name,id}, devlink requires drivers to opt >>> in with an implementation right, so you can't really force a solution to >>> the consistent naming. >> >> Yeah, drivers would still have free choice to implemen the ndo >> themselves. But most of them, like all sriov switch drivers should use >> the devlink helper to have consistent naming. In other words, devlink >> helper should be the standard way, in weird cases (like rocker), driver >> implements it himself. > >That's an assumption that somehow the devlink API will be better >supported than ndo_get_phys_port_{name,id}. Don't get me wrong -- an API >to show the kind of device is needed, but I do not think this enforces >any kind of consistency in naming. So you say that we need to enforce it somehow? > >> >> >>> >>>> >>>> This patchset introduces port flavours which should address the first >>>> problem. I'm testing this with Netronome nfp hardware. When the user >>>> has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: >>>> # devlink port >>>> pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 >>>> pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0 >>>> pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1 >>>> pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008 >>>> pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0 >>>> pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1 >>>> pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2 >>>> pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3 >>> >>> How about 'kind' instead of flavo{u}r? >> >> Yeah, kind is often used in kernel already with different meaning >> git grep kind net/core >> I wanted to avoid confusions > >Roopa's amendment works as well; I just think flavor / flavour is the >wrong word. Make me thinks of food ... ice cream vs netdevices. Ok
Fri, Mar 23, 2018 at 04:34:47AM CET, jakub.kicinski@netronome.com wrote: >On Thu, 22 Mar 2018 11:55:10 +0100, Jiri Pirko wrote: >> Also, there is one extra port that I don't understand what >> is the purpose for it - something nfp specific perhaps. > >Do you mean the PF netdev? There can be multiple of those on >multi-host cards. There is one pf_repr from ASIC's perspective and a >full-blown PF netdev which should be used by applications. pf_repr is >only for switch config. Got it.
> 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 This is on a board with a DSA cluster of three switches. Some of the switch ports are not connected to anything, so are plain 'notset'. What is the "number X" meant to mean? Andrew
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 > >This is on a board with a DSA cluster of three switches. Some of the >switch ports are not connected to anything, so are plain 'notset'. Okay. That looks fine. I wonder if it would make sense to have another flavour for "unused" ports. > >What is the "number X" meant to mean? That is basically front panel number for physical ports. It is used for generating phys_port_name. It should have separate numbering for cpu ports and dsa ports most probably. Although, since they have no netdevice associated, the number is not used and only shown here. In case of mlxsw switch port 1, the netdev name is then for example: "enp3s0np1".
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. Andrew
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? > > Andrew
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).
> >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? Hi Jiri Isn't the same true for all devices? It is not just DSA devices where the hardware engineer is free to wire up the front panel however they want, it can happen for any device. Andrew
Sat, Mar 24, 2018 at 03:40:02PM CET, andrew@lunn.ch wrote: >> >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? > >Hi Jiri > >Isn't the same true for all devices? It is not just DSA devices where >the hardware engineer is free to wire up the front panel however they >want, it can happen for any device. In mlxsw, driver queries the FW to get this info.
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
On March 24, 2018 9:07:49 AM PDT, Jiri Pirko <jiri@resnulli.us> 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 <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. 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?
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 <jiri@resnulli.us> 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 <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. > >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.
> >>>>>> >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 > >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. Florian, do you need it to be unique per switch, or DSA cluster. The current solution is clearly not unique across a cluster. Andrew
On 24/03/2018 12:00, Andrew Lunn wrote: >>>>>>>>> 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 > >>> 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. > > Florian, do you need it to be unique per switch, or DSA cluster. The > current solution is clearly not unique across a cluster. It needs to be unique per switch, we have the switch identifier through phys_switch_id for higher level "uniqueness".
On Thu, 22 Mar 2018 11:55:10 +0100 Jiri Pirko <jiri@resnulli.us> wrote: > From: Jiri Pirko <jiri@mellanox.com> > > This patchset resolves 2 issues we have right now: > 1) There are many netdevices / ports in the system, for port, pf, vf > represenatation but the user has no way to see which is which There already are a lot of attributes, adding more doesn't necessarily help make things clearer. > 2) The ndo_get_phys_port_name is implemented in each driver separatelly, > which may lead to inconsistent names between drivers. Why not address that problem. My concern is that your new attribute will have the same problem. Also adding pf and vfNNN on the name will make the already tightly squeezed interface name length a real problem. I have had arguments with people trying use VLAN 4000 and standard naming policy. Which means you really can't go that long.
Wed, Mar 28, 2018 at 07:02:34AM CEST, stephen@networkplumber.org wrote: >On Thu, 22 Mar 2018 11:55:10 +0100 >Jiri Pirko <jiri@resnulli.us> wrote: > >> From: Jiri Pirko <jiri@mellanox.com> >> >> This patchset resolves 2 issues we have right now: >> 1) There are many netdevices / ports in the system, for port, pf, vf >> represenatation but the user has no way to see which is which > >There already are a lot of attributes, adding more doesn't necessarily >help make things clearer. How elso you distinguish pfrep/vfrep/cpuport/etc? > >> 2) The ndo_get_phys_port_name is implemented in each driver separatelly, >> which may lead to inconsistent names between drivers. > >Why not address that problem. My concern is that your new attribute >will have the same problem. I try to address that... > >Also adding pf and vfNNN on the name will make the already tightly squeezed >interface name length a real problem. I have had arguments with people >trying use VLAN 4000 and standard naming policy. Which means you really >can't go that long. Understood. However, I just do what is already done in nfp for example.
On Thu, Mar 22, 2018 at 1:55 PM, Jiri Pirko <jiri@resnulli.us> wrote: > From: Jiri Pirko <jiri@mellanox.com> > > This patchset resolves 2 issues we have right now: > 1) There are many netdevices / ports in the system, for port, pf, vf > represenatation but the user has no way to see which is which > 2) The ndo_get_phys_port_name is implemented in each driver separatelly, > which may lead to inconsistent names between drivers. > > This patchset introduces port flavours which should address the first > problem. I'm testing this with Netronome nfp hardware. When the user > has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: J/J (Jiri/Jakub) -- re "2 physical ports, 1 pf, and 4 vfs" --- does NFP exposes one PF for both physical ports? FWIW note that in mlx5 and AFAIK any other device except for mlx4 (...) folks have FPP (Function Per Port) scheme. [..] > The desired output should look like this: > # devlink port > pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 > pci/0000:05:00.0/1: type eth netdev enp5s0np1 flavour physical number 1 > pci/0000:05:00.0/2: type eth netdev enp5s0npf0 flavour pf_rep number 0 > pci/0000:05:00.0/3: type eth netdev enp5s0nvf0 flavour vf_rep number 0 > pci/0000:05:00.0/4: type eth netdev enp5s0nvf1 flavour vf_rep number 1 > pci/0000:05:00.0/5: type eth netdev enp5s0nvf2 flavour vf_rep number 2 > pci/0000:05:00.0/6: type eth netdev enp5s0nvf3 flavour vf_rep number 3 > As you can see, the netdev names are generated according to the flavour > and port number. In case the port is split, the split subnumber is also included. What is the purpose/role in getting dev link ports here? is it such that @ the end of the day the driver would do a devlink_port_get_phys_port_name() call in their get phys port name ndo? or we buy more advantages out of doing so? Or.
On Tue, 17 Apr 2018 16:23:48 +0300, Or Gerlitz wrote: > On Thu, Mar 22, 2018 at 1:55 PM, Jiri Pirko <jiri@resnulli.us> wrote: > > From: Jiri Pirko <jiri@mellanox.com> > > > > This patchset resolves 2 issues we have right now: > > 1) There are many netdevices / ports in the system, for port, pf, vf > > represenatation but the user has no way to see which is which > > 2) The ndo_get_phys_port_name is implemented in each driver separatelly, > > which may lead to inconsistent names between drivers. > > > > This patchset introduces port flavours which should address the first > > problem. I'm testing this with Netronome nfp hardware. When the user > > has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: > > J/J (Jiri/Jakub) -- > > re "2 physical ports, 1 pf, and 4 vfs" --- does NFP exposes one PF for > both physical ports? Yes there are multiple PCIe PFs on the card, but the basic CX card just uses one for all uplinks (like mlx4). > FWIW note that in mlx5 and AFAIK any other device except for mlx4 (...) > folks have FPP (Function Per Port) scheme. > > [..] > > > The desired output should look like this: > > # devlink port > > pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 > > pci/0000:05:00.0/1: type eth netdev enp5s0np1 flavour physical number 1 > > pci/0000:05:00.0/2: type eth netdev enp5s0npf0 flavour pf_rep number 0 > > pci/0000:05:00.0/3: type eth netdev enp5s0nvf0 flavour vf_rep number 0 > > pci/0000:05:00.0/4: type eth netdev enp5s0nvf1 flavour vf_rep number 1 > > pci/0000:05:00.0/5: type eth netdev enp5s0nvf2 flavour vf_rep number 2 > > pci/0000:05:00.0/6: type eth netdev enp5s0nvf3 flavour vf_rep number 3 > > As you can see, the netdev names are generated according to the flavour > > and port number. In case the port is split, the split subnumber is also included. > > What is the purpose/role in getting dev link ports here? is it such > that @ the end > of the day the driver would do a devlink_port_get_phys_port_name() call in their > get phys port name ndo? or we buy more advantages out of doing so? IMHO having way to get all netdevs and the netdev type from devlink is quite user friendly. As of today we also use the devlink ports for port splitting on 40/100G parts. Hopefully more functionality migrates over from ethtool over time.
Thu, Mar 22, 2018 at 08:25:46PM CET, andrew.gospodarek@broadcom.com wrote: >On Thu, Mar 22, 2018 at 01:10:38PM -0600, David Ahern wrote: >> On 3/22/18 11:49 AM, Jiri Pirko wrote: >> > Thu, Mar 22, 2018 at 04:34:07PM CET, dsahern@gmail.com wrote: >> >> On 3/22/18 4:55 AM, Jiri Pirko wrote: >> >>> From: Jiri Pirko <jiri@mellanox.com> >> >>> >> >>> This patchset resolves 2 issues we have right now: >> >>> 1) There are many netdevices / ports in the system, for port, pf, vf >> >>> represenatation but the user has no way to see which is which >> >>> 2) The ndo_get_phys_port_name is implemented in each driver separatelly, >> >>> which may lead to inconsistent names between drivers. >> >> >> >> Similar to ndo_get_phys_port_{name,id}, devlink requires drivers to opt >> >> in with an implementation right, so you can't really force a solution to >> >> the consistent naming. >> > >> > Yeah, drivers would still have free choice to implemen the ndo >> > themselves. But most of them, like all sriov switch drivers should use >> > the devlink helper to have consistent naming. In other words, devlink >> > helper should be the standard way, in weird cases (like rocker), driver >> > implements it himself. >> >> That's an assumption that somehow the devlink API will be better >> supported than ndo_get_phys_port_{name,id}. Don't get me wrong -- an API >> to show the kind of device is needed, but I do not think this enforces >> any kind of consistency in naming. >> >> > >> > >> >> >> >>> >> >>> This patchset introduces port flavours which should address the first >> >>> problem. I'm testing this with Netronome nfp hardware. When the user >> >>> has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: >> >>> # devlink port >> >>> pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 >> >>> pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0 >> >>> pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1 >> >>> pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008 >> >>> pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0 >> >>> pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1 >> >>> pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2 >> >>> pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3 >> >> >> >> How about 'kind' instead of flavo{u}r? >> > >> > Yeah, kind is often used in kernel already with different meaning >> > git grep kind net/core >> > I wanted to avoid confusions >> >> Roopa's amendment works as well; I just think flavor / flavour is the >> wrong word. Make me thinks of food ... ice cream vs netdevices. > >Naming it a 'subtype' could also work. It's a bit longer than 'kind' >(no longer than 'flavour') and accurately describes the characteristic >of this port. It also avoids the namespace collision of 'kind' that >Jiri points out. > >It also fits with the names used in the PCI world with vendor:device and >subsystem vendor:subsystem device naming used there for further >granularity. Problem with "subtype" is that it indicates some relation with type. We have type: enum devlink_port_type { DEVLINK_PORT_TYPE_NOTSET, DEVLINK_PORT_TYPE_AUTO, DEVLINK_PORT_TYPE_ETH, DEVLINK_PORT_TYPE_IB, }; Does not feel correct to have subtypes VF/PF/VFREP/etc, which really has no relation to ETH/IB What about "role"? Also does not sound good to me, as the "role" indicates that the port can "act" like something. For me the "flavour/flavor" is still a favourite. Tells how the port tastes like, in a semi-fun way :) Also, there is a precedence to this in particle physics: https://en.wikipedia.org/wiki/Color%E2%80%93flavor_locking I guess they also had troubles to find the right name :) >
From: Jiri Pirko <jiri@mellanox.com> This patchset resolves 2 issues we have right now: 1) There are many netdevices / ports in the system, for port, pf, vf represenatation but the user has no way to see which is which 2) The ndo_get_phys_port_name is implemented in each driver separatelly, which may lead to inconsistent names between drivers. This patchset introduces port flavours which should address the first problem. I'm testing this with Netronome nfp hardware. When the user has 2 physical ports, 1 pf, and 4 vfs, he should see something like this: # devlink port pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0 pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1 pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008 pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0 pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1 pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2 pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3 The indexes are weird numbers now. That needs to be fixed. Also, netdev renaming does not work correctly for me now for some reason. Also, there is one extra port that I don't understand what is the purpose for it - something nfp specific perhaps. The desired output should look like this: # devlink port pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0 pci/0000:05:00.0/1: type eth netdev enp5s0np1 flavour physical number 1 pci/0000:05:00.0/2: type eth netdev enp5s0npf0 flavour pf_rep number 0 pci/0000:05:00.0/3: type eth netdev enp5s0nvf0 flavour vf_rep number 0 pci/0000:05:00.0/4: type eth netdev enp5s0nvf1 flavour vf_rep number 1 pci/0000:05:00.0/5: type eth netdev enp5s0nvf2 flavour vf_rep number 2 pci/0000:05:00.0/6: type eth netdev enp5s0nvf3 flavour vf_rep number 3 As you can see, the netdev names are generated according to the flavour and port number. In case the port is split, the split subnumber is also included. 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. Thanks! Jiri Pirko (12): devlink: introduce devlink_port_attrs_set devlink: extend attrs_set for setting port flavours devlink: introduce a helper to generate physical port names dsa: set devlink port attrs for dsa ports dsa: use devlink helper to generate physical port name mlxsw: use devlink helper to generate physical port name nfp: flower: fix error path during representor creation nfp: set eth_id for representors to avoid port index conflict nfp: register devlink port for VF/PF representors nfp: flower: create port for flower vnic nfp: use devlink helper to generate physical port name nfp: flower: set sysfs link to device for representors drivers/net/ethernet/mellanox/mlxsw/core.c | 18 ++++- drivers/net/ethernet/mellanox/mlxsw/core.h | 5 +- drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 21 ++---- drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 11 +-- drivers/net/ethernet/netronome/nfp/flower/main.c | 17 ++++- drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 45 +++++++++-- drivers/net/ethernet/netronome/nfp/nfp_net_repr.c | 19 ++++- drivers/net/ethernet/netronome/nfp/nfp_net_repr.h | 1 + drivers/net/ethernet/netronome/nfp/nfp_port.c | 30 +------- include/net/devlink.h | 32 ++++++-- include/uapi/linux/devlink.h | 22 ++++++ net/core/devlink.c | 92 ++++++++++++++++++++--- net/dsa/dsa2.c | 23 ++++++ net/dsa/slave.c | 6 +- 14 files changed, 252 insertions(+), 90 deletions(-)