On Thu, Jun 28, 2018 at 06:01:54AM +0000, Shahaf Shuler wrote:
> Wednesday, June 27, 2018 4:32 PM, Adrien Mazarguil:
> > Subject: Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors
> >
> > On Sun, Jun 17, 2018 at 10:15:07AM +0000, Shahaf Shuler wrote:
> > > Hi Adrien,
> > >
> > > Saturday, June 16, 2018 11:58 AM, Xueming(Steven) Li:
> > > > Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port
> > > > representors
> > > >
> > > > > -----Original Message-----
> > > > > From: dev <dev-boun...@dpdk.org> On Behalf Of Adrien Mazarguil
> > > > > Sent: Thursday, June 14, 2018 4:35 PM
> > > > > To: Shahaf Shuler <shah...@mellanox.com>
> > > > > Cc: dev@dpdk.org
> > > > > Subject: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port
> > > > > representors
> > > > >
> > > > > Probe existing port representors in addition to their master
> > > > > device and
> > > > associate them automatically.
> > > > >
> > > > > To avoid name collision between Ethernet devices, their names use
> > > > > the same convention as ixgbe and i40e PMDs, that is, instead of
> > > > > only a PCI
> > > > address in DBDF notation:
> > > > >
> > > > > - "net_{DBDF}_0" for master/switch devices.
> > >
> > > This is breaking compatibility for application using the device names in
> > order to attach them to the application (e.g. OVS-DPDK).
> > > Before this patch the naming scheme for non-representor port is "{DBDF}".
> > >
> > > Can we preserve the compatibility and add appropriate suffix for the
> > representor case?
> >
> > There's one issue if representors are hot-plugged. The name of the master
> > device, which happens to be that of the switch domain, cannot be updated.
> > The form "net_{DBDF}_0" seems expected for PMDs that support
> > representors (see ixgbe and i40e).
> >
> > Now since representor hot-plugging is not supported yet, I guess we could
> > postpone this problem by keeping the old format in the meantime, however
> > ideally, these applications should not rely on it. The only safe assumption
> > they can make is the uniqueness of any given name among ethdevs.
> >
> > PCI bus addresses, if needed, should be retrieved by looking at the
> > underlying bus object.
>
> Am not sure I understand. Those application attach the device to the
> application based on its name, which happens to be the PCI address in case of
> mlx5.
I'm only saying it's not future-proof seeing what happens when they rely on
it. Moreover this forces them to convert the name to some binary form
instead of e.g. simply checking RTE_DEV_TO_PCI(dev->device)->addr where
needed and only use the name as some kind of opaque identifier.
> > By the way, while thinking again about a past comment from Xueming [1],
> > maybe it's finally time to remove support for multiple Verbs ports on mlx5
> > after all. This should drop another unnecessary loop and the need for the
> > unused "port %u" suffix at all while naming the device.
> >
> > So how about the following plan for v3:
> >
> > - Adding a patch that drops support for multiple Verbs ports (note for
> > Xueming, yes I changed my mind *again* :)
>
> I am OK w/ that.
>
> >
> > - If you really think this will break OVS (please confirm),
>
> It will.
Out of curiosity, can you point me to the relevant code in OVS? Maybe
something can be done on their side.
Either ixgbe and i40e are unaffected by the very same change, or they also
break OVS, in which case there's an issue we need to solve with the
representor interface in DPDK before it's too late.
> then when no
> > "representor" parameter is provided (regardless of the presence of any
> > representors), name format will use the usual "{DBDF}" notation as you
> > suggested.
> >
> > - Otherwise as soon as a "representor" is found on the command line, the
> > new
> > format will be used, again regardless of the presence of any representors.
> >
> > - In both cases, representors if any, will be named according to the format
> > specified in this patch.
>
> Can we do the following?
> In case representor is found the naming will be DBDF_representor_%d
> In case no-representor naming will be DBDF
>
> Just removing the net prefix.
Yes, I'll remove it. We'll standardize on the naming used for ixgbe/i40e
only once the above concerns are addressed.
--
Adrien Mazarguil
6WIND
