RE: [PATCH net-next v2 4/7] devlink: allow subports on devlink PCI ports

[Parav Pandit <parav@mellanox.com>]

Hi Jiri, Jakub, Samudrala Sridhar,

> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Parav Pandit
> Sent: Monday, March 18, 2019 5:12 PM
> To: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Jiri Pirko <jiri@resnulli.us>; Samudrala, Sridhar
> <sridhar.samudrala@intel.com>; davem@davemloft.net;
> netdev@vger.kernel.org; oss-drivers@netronome.com
> Subject: RE: [PATCH net-next v2 4/7] devlink: allow subports on devlink PCI
> ports
> 
> 
> 
> > -----Original Message-----
> > From: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Sent: Monday, March 18, 2019 4:30 PM
> > To: Parav Pandit <parav@mellanox.com>
> > Cc: Jiri Pirko <jiri@resnulli.us>; Samudrala, Sridhar
> > <sridhar.samudrala@intel.com>; davem@davemloft.net;
> > netdev@vger.kernel.org; oss-drivers@netronome.com
> > Subject: Re: [PATCH net-next v2 4/7] devlink: allow subports on
> > devlink PCI ports
> >
> > On Mon, 18 Mar 2019 20:35:02 +0000, Parav Pandit wrote:
> > > > > > > physical and pf has some overlapping definitions.
> > > > > >
> > > > > > What "overlapping definitions" do physical and PF have?
> > > > > PF has physically user facing port.
> > > >
> > > > PF doesn't "have a user facing port" in switchdev mode.
> > >
> > > Physical port described in include/uapi/linux/devlink.h as
> > > DEVLINK_PORT_FLAVOUR_PHYSICAL is not related to switchdev or legacy
> > > mode.
> >
> > I said "PF doesn't ...", you're now talking about physical?
> >
> > > As the comment block describe it is 'any kind of port physical
> > > facing user'.
> >
> > Are you saying PCI function is physical?  Just because PF stands for
> > Physical Function?
> >
> > Physical port in devlink means a port in the front panel where
> > networking cable goes.
> >
> > > Current mlx5 driver doesn't expose ports as physical regardless of
> > > switchdev/legacy mode.
> >
> > Today mlx5 doesn't expose devlink ports at all.
> >
> > > > It's a limitation of Mellanox HW that you have some strong
> > > > association there.
> > > Not sure why you keep saying that. Any code reference that I should
> > > look at? Or maybe you can explain what is that limitation, because I
> > > am not aware of any.
> >
> > NIC designs originating from traditional NICs were build as pipelines
> > from PCI to wire or from wire to PCI.  Reportedly it makes it hard to
> > completely divorce the PCI PF from the wire port (physical port).
> >
> > Which is why you may think that "PF has physically user facing port".
> >
> > > > > And physical port in include/uapi/linux/devlink.h also describe
> > > > > that.
> > > >
> > > > By "that" you must mean that the physical is a user facing port.
> > >
> > > Can you please describe the difference between 'PF port' and
> > > 'physical port of include/uapi/linux/devlink.h'? I must have missed
> > > this crisp definition in discussion between you and Jiri. I am in
> > > meantime checking the thread.
> >
> > Perhaps start with the cover letter which includes an ASCII drawing?
> >
> > Using Mellanox nomenclature - PF port is a "representor" for the PF
> > which may be on another Host (SmartNIC or multihost).  It's pretty
> > much the same thing as a VF port/"representor".
> >
> Yes. We are aligned here. :-)
> I see your point, where in multi-host scenario, a physical port may be 1, but
> PF ports are 4, because of 4 PFs for 4 hosts.
> (just an example of 4 hosts with their own mac address sharing 1 physical
> port).
> 
> When there is no multihost and one to one mapping between a PF and
> physical links, there is some overlap between PF port and physical port
> attributes.
> I believe, such overlap is fine as long as we have unique indices for the ports.
> 
> So I am ok to have flavours as physical/cpu/dsa/pf/vf/mdev/switchport.
> (last 4 as new port flavours).
> 
> > Physical port is the hole on the panel of the adapter where cable goes.

So my take away from above discussion are:
1. Following new port flavours should be added pci_pf/pci_vf/mdev/switchport.
a. Switchport indicates port on the eswitch. Normally this port has rep-netdev attached to it.
b. host side port flavours are pci_pf/pci_vf/mdev which may be connected to switchport

2. host side port flavours are not limited to Ethernet, as it is for devlink's port instance.

3. Each port is continue to be accessed using unique port index.

4. host side ports and switchport are control objects.
a. switch side ports reside where current eswitch object of devlink instance reside
b. for a given VF/PF/mdev such host side ports may be in hypervisor or VM or both 
depending on the privilege

5. eth.mac_address, rdma.port_guid can be programmed at 
host port flavours by extending as $ devlink port param set...
(similar to devlink dev param set)

6. more host port params can be added in future when user need arise

7. rep-netdev continue to be eswitch (switchport) representor at the switch side.
a. Hence rep-netdev cannot be used for programming host port's parameters.

8. eswitch devlink instance knows when VF/PF/mdev's switchport are created/removed.
Hence, those will be created/deleted by eswitch.
Similarly for host port flavours too.

Does it look fine? Did I miss something?
We would like to progress on incremental patches for item-4 and 
any prep work needed to reach to item-4.