Thursday, June 28, 2018 11:46 AM, Adrien Mazarguil:

Subject: Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors

On Thu, Jun 28, 2018 at 06:01:54AM +0000, Shahaf Shuler wrote:

quoted

Wednesday, June 27, 2018 4:32 PM, Adrien Mazarguil:

quoted

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:

quoted

Hi Adrien,

Saturday, June 16, 2018 11:58 AM, Xueming(Steven) Li:

quoted

Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port
representors

quoted

-----Original Message-----
From: dev <redacted> On Behalf Of Adrien

Mazarguil

quoted

quoted

quoted

quoted

quoted

Sent: Thursday, June 14, 2018 4:35 PM
To: Shahaf Shuler <redacted>
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.

quoted

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:

quoted

- "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).

quoted

Before this patch the naming scheme for non-representor port is

"{DBDF}".

quoted

quoted

quoted

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.

quoted

quoted

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.

quoted

quoted

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.

quoted

quoted

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.

quoted

quoted

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.

quoted

- 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.

For example the command to add new port to the ovs bridge is done on the following syntax
ovs-vsctl add-port ovsbr0 dpdk0 \        
    -- set interface dpdk0 type=dpdk \   
    options:dpdk-devargs="0000:81:00.0" \
    ofport_request=1                    

OVS users/automation are using the PCI address for Mellanox PMDs. w/ your change they will need to use net_0000:81:00.0 to attach the port. 

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.

quoted

 then when no

quoted

  "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.

quoted

quoted

- In both cases, representors if any, will be named according to the

format

quoted

quoted

  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