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

From: Parav Pandit <parav@mellanox.com>
To: Jiri Pirko <jiri@resnulli.us>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Saeed Mahameed <saeedm@mellanox.com>,
	"kwankhede@nvidia.com" <kwankhede@nvidia.com>,
	"leon@kernel.org" <leon@kernel.org>,
	"cohuck@redhat.com" <cohuck@redhat.com>,
	Jiri Pirko <jiri@mellanox.com>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>
Subject: RE: [PATCH net-next 12/19] devlink: Introduce mdev port flavour
Date: Fri, 8 Nov 2019 16:43:43 +0000	[thread overview]
Message-ID: <AM0PR05MB48669A9AE494CCCE8E07C367D17B0@AM0PR05MB4866.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <20191108163139.GQ6990@nanopsycho>


> -----Original Message-----
> From: Jiri Pirko <jiri@resnulli.us>
> >> >> 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.
> >>
> >> Wait a sec. For mdev, what you say is correct. But here we talk about
> >> devlink_port which is representing this mdev. And this devlink_port
> >> is very similar to VF devlink_port. It is bound to specific PF (in
> >> case of mdev it could be PF-VF).
> >>
> >But mdev port has unique phys_port_name in system, it incorrect to use
> PF/VF prefix.
> 
> Why incorrect? It is always bound to pf/vf?
> 
Because mdev device already identified using its unique alias. Why does it need prefix?
Mdev core generating the alias is not aware of the prefixes applied devlink. it shouldn't be.
We want more letters towards uniqueness of the alias and filling it up with such prefixes doesn't make sense.

> >What in hypothetical case, mdev is not on top of PCI...
> 
> Okay, let's go hypothetical. In that case, it is going to be on top of something
> else, wouldn't it?
Yes, it will be. But just because it is on top of something, doesn't mean we include the whole parent dev, its bridge, its rc hierarchy here.
There should be a need.
It was needed in PF/VF case due to overlapping numbers of VFs via single devlink instance. You probably missed my reply to Jakub.
Here it is no overlap.