Fri, Mar 29, 2019 at 11:34:14PM CET, jakub.kicin...@netronome.com wrote: >On Fri, Mar 29, 2019 at 2:21 PM Jiri Pirko <j...@resnulli.us> wrote: >> Fri, Mar 29, 2019 at 07:59:26PM CET, jakub.kicin...@netronome.com wrote: >> >On Fri, 29 Mar 2019 07:49:05 +0100, Jiri Pirko wrote: >> >> Thu, Mar 28, 2019 at 10:40:02PM CET, jakub.kicin...@netronome.com wrote: >> >> >On Thu, 28 Mar 2019 22:12:42 +0100, Jiri Pirko wrote: >> >> >> From: Jiri Pirko <j...@mellanox.com> >> >> >> >> >> >> To provide visibility of the ports, this patchset exposes switch ID >> >> >> for devlink ports, which are part of a switch. The rest of the ports >> >> >> if any (in case of sr-iov for example) do not set switch ID. >> >> > >> >> >I don't feel good about this patch set. There is no visibility >> >> >provided here. Should the port flavour should be a sufficient >> >> >> >> 1) this patch is mainly about avoiding need to define the ndo and moving >> >> the switch id definition to devlink port attr. >> > >> >Sure, that you could achieve by putting the data in the netdevice >> >structure as well.. >> > >> >What is the guiding principle here? I'm trying to argue for leaving >> >forwarding-related info in netdev code, and only have HW control in >> >devlink. I just don't see switch id being useful at devlink level in >> >any way. >> >> Well we have switchib driver which does not have any netdevice and still >> the ports are part of a switch. In other words, this is not >> ethernet-specific attribute, therefore devlink is the right fit. > >Ack, but switch id is a switchdev thing, controlling which ports are >considered to be part of the same device _for_ _switchdev_.
"Switchdev" is a bridge offload. Nothing else. > >Is switchib doing forwarding? Not long ago Parav was convincing us >that switchdev mode for IB is pretty much meaningless. Even though >there are apparently representors for IB (judging by the recent RDMA >patchset on netdev)... Switch id is simply identifier for ports in general that belong to some switch. No matter if ethernet or IB. > >> >> 2) along with that, switch id is added as attribute. It tells the user >> >> that some devlink port is part of a switch with certain id. If port >> >> is not part of a switch (like upcoming hostport, cpu, dsa, etc), >> >> switch id is not set on that port >> > >> >If the flavour already gives that information, why crowd the attributes >> >for ports with switch id? >> >> Hmm, we'll have multiple non-switch port flavours and once your vf/pf >> patchset hits the tree we'll have multipkle switch port flavours. >> So makes sense to have switch id. > >It's probably a matter of preference then. To me its very clear to say that >you can switch to physical/vf/pf ports and not cpu/dsa/host. I don't like >the duplication of dumpnig the same attribute both in rtnetlink and devlink. I want to remind you, that switch_id in rtnetlink was introduced before devlink existed. Back than, if was the only place to put it. However, as I explained, it is not ethernet speficic thing and rather belongs to devlink. This patchset is doing that. > >> Also, you can have multiple switches within one asic. > >That line is blurry. It is. So far, it is mlx CX hw that have basically 2 switches, one per PF. > >> >> >indication of whether netdev associated with that port can be >> >> >switched to or not? CPU, DSA, and Host flavours can't be switched >> >> >to. And the switchid can be an attribute of the devlink instance, >> >> >if we want to expose it via devlink. >> >> >> >> One devlink instance can have multiple switch ids in use as it may >> >> contain multiple switches. Take mlx5 as an instance. Currently every PF >> >> creates a separate devlink instance, however there are some features >> >> shared. In this example, with proposed idea of aliasing, there would be >> >> one devlink instance aliased between these 2 pf inctances, with 2 >> >> eswitches and 2 sets of switch ports each belonging to an eswitch - >> >> distinguished by switch id. >> > >> >Out of curiosity, what are the shared features? It seems mlx5 drives >> >a lot of our API design, it'd be good if the community had a better >> >understanding of it. >> >> I have to gather that info. Not so many things are shared. There is one >> extra switch to mix 2 pfs together. I know about some IB features that >> also mix 2 pfs. > >I see, so a dual port IB card would somehow have only one whatever the >equivalent of a netdev in IB world is? Yes. We have a patchset internally to manage that. > >> >The situation with pipelined devices is somewhat murky. Didn't Or add >> >some from of PCIe-side looped queue to forward between PFs? >> >> I have no clue. Ccing Or. Or? >> >> >Presumably DSA would lean the opposite way with multiple ASICs >> >reporting the same ID? >> >> Yes, sounds right. > >Right, so to me switch ID really is a forwarding attribute.
