RE: [PATCH net-next 12/19] devlink: Introduce mdev port flavour

[Parav Pandit <parav@mellanox.com>]

> -----Original Message-----
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Sent: Thursday, November 7, 2019 8:20 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; davem@davemloft.net;
> kvm@vger.kernel.org; netdev@vger.kernel.org; Saeed Mahameed
> <saeedm@mellanox.com>; kwankhede@nvidia.com; leon@kernel.org;
> cohuck@redhat.com; Jiri Pirko <jiri@mellanox.com>; linux-
> rdma@vger.kernel.org
> Subject: Re: [PATCH net-next 12/19] devlink: Introduce mdev port flavour
> 
> On Fri, 8 Nov 2019 01:44:53 +0000, Parav Pandit wrote:
> > > I'm talking about netlink attributes. I'm not suggesting to sprintf
> > > it all into the phys_port_name.
> > >
> > I didn't follow your comment. For devlink port show command output you
> > said,
> >
> > "Surely those devices are anchored in on of the PF (or possibly VFs)
> > that should be exposed here from the start."
> > So I was trying to explain why we don't expose PF/VF detail in the
> > port attributes which contains
> > (a) flavour
> > (b) netdev representor (name derived from phys_port_name)
> > (c) mdev alias
> >
> > Can you please describe which netlink attribute I missed?
> 
> Identification of the PCI device. The PCI devices are not linked to devlink
> ports, so the sysfs hierarchy (a) is irrelevant, (b) may not be visible in multi-
> host (or SmartNIC).
>

It's the unique mdev device alias. It is not right to attach to the PCI device.
Mdev is bus in itself where devices are identified uniquely. So an alias suffice that identity.

> > > > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > >
> > > > > > @@ -6649,6 +6678,9 @@ static int
> > > > > __devlink_port_phys_port_name_get(struct devlink_port
> > > > > *devlink_port,
> > > > > >  		n = snprintf(name, len, "pf%uvf%u",
> > > > > >  			     attrs->pci_vf.pf, attrs->pci_vf.vf);
> > > > > >  		break;
> > > > > > +	case DEVLINK_PORT_FLAVOUR_MDEV:
> > > > > > +		n = snprintf(name, len, "p%s", attrs-
> >mdev.mdev_alias);
> > > > >
> > > > > Didn't you say m$alias in the cover letter? Not p$alias?
> > > > >
> > > > In cover letter I described the naming scheme for the netdevice of
> > > > the mdev device (not the representor). Representor follows current
> > > > unique phys_port_name method.
> > >
> > > So we're reusing the letter that normal ports use?
> > >
> > I initially had 'm' as prefix to make it easy to recognize as mdev's port,
> instead of 'p', but during internal review Jiri's input was to just use 'p'.
> 
> Let's way for Jiri to weigh in then.

Yeah.
I remember his point was to not confuse the <en><m> prefix in the persistent device name with 'm' prefix in phys_port_name.
Hence, his input was just 'p'.

> 
> > > Why does it matter to name the virtualized device? In case of other
> > > reprs its the repr that has the canonical name, in case of
> > > containers and VMs they will not care at all what hypervisor identifier
> the device has.
> > >
> > Well, many orchestration framework probably won't care of what name is
> picked up.
> > And such name will likely get renamed to eth0 in VM or container.
> > Unlike vxlan, macvlan interfaces, user explicitly specify the netdevice name,
> and when newlink() netlink command completes with success, user know the
> device to use.
> > If we don't have persistent name for mdev, if a random name ethX is
> picked up, user needs refer to sysfs device hierarchy to know its netdev.
> > Its super easy to do refer that, but having persistent name based out of
> alias makes things aligned like naming device on PCI bus.
> > This way devices can be used without VM/container use cases too, for
> example user is interested in only 4 or 8 mdev devices in system and its
> setup is done through systemd.service.