[PATCH v2] bus/vmbus: add device_order field to rte_vmbus_dev

[PATCH v2] bus/vmbus: add device_order field to rte_vmbus_dev

[Vladimir Ratnikov]

Field can be used to have static interface naming
in DPDK based applications instead of uuid.
RTE_FOREACH_DEV won't return the correct order of
such devices due to vmbus_ignore_device in case of
blocklisting/allowlisting. So device_order field
will be helpful in that case.

Signed-off-by: Vladimir Ratnikov <vratnikov@netgate.com>
---
 drivers/bus/vmbus/bus_vmbus_driver.h | 1 +
 drivers/bus/vmbus/vmbus_common.c     | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/vmbus/bus_vmbus_driver.h b/drivers/bus/vmbus/bus_vmbus_driver.h
index e2475a642d..6b010cbe41 100644
--- a/drivers/bus/vmbus/bus_vmbus_driver.h
+++ b/drivers/bus/vmbus/bus_vmbus_driver.h
@@ -37,6 +37,7 @@ struct rte_vmbus_device {
 	rte_uuid_t device_id;		       /**< VMBUS device id */
 	rte_uuid_t class_id;		       /**< VMBUS device type */
 	uint32_t relid;			       /**< id for primary */
+	uint16_t device_order;		       /**< Device order after probing */
 	uint8_t monitor_id;		       /**< monitor page */
 	int uio_num;			       /**< UIO device number */
 	uint32_t *int_page;		       /**< VMBUS interrupt page */
diff --git a/drivers/bus/vmbus/vmbus_common.c b/drivers/bus/vmbus/vmbus_common.c
index b9139c6e6c..1a64ab2712 100644
--- a/drivers/bus/vmbus/vmbus_common.c
+++ b/drivers/bus/vmbus/vmbus_common.c
@@ -199,7 +199,7 @@ rte_vmbus_probe(void)
 	char ubuf[RTE_UUID_STRLEN];
 
 	FOREACH_DEVICE_ON_VMBUS(dev) {
-		probed++;
+		dev->device_order = probed++;
 
 		rte_uuid_unparse(dev->device_id, ubuf, sizeof(ubuf));
 
-- 
2.34.1

[PATCH v2] bus/vmbus: add device_order field to rte_vmbus_dev

[Vladimir Ratnikov]

Field can be used to have static interface naming
in DPDK based applications instead of uuid.
RTE_FOREACH_DEV won't return the correct order of
such devices due to vmbus_ignore_device in case of
blocklisting/allowlisting. So device_order field
will be helpful in that case.

Signed-off-by: Vladimir Ratnikov <vratnikov@netgate.com>
---
 drivers/bus/vmbus/bus_vmbus_driver.h | 1 +
 drivers/bus/vmbus/vmbus_common.c     | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/vmbus/bus_vmbus_driver.h b/drivers/bus/vmbus/bus_vmbus_driver.h
index e2475a642d..6b010cbe41 100644
--- a/drivers/bus/vmbus/bus_vmbus_driver.h
+++ b/drivers/bus/vmbus/bus_vmbus_driver.h
@@ -37,6 +37,7 @@ struct rte_vmbus_device {
 	rte_uuid_t device_id;		       /**< VMBUS device id */
 	rte_uuid_t class_id;		       /**< VMBUS device type */
 	uint32_t relid;			       /**< id for primary */
+	uint16_t device_order;		       /**< Device order after probing */
 	uint8_t monitor_id;		       /**< monitor page */
 	int uio_num;			       /**< UIO device number */
 	uint32_t *int_page;		       /**< VMBUS interrupt page */
diff --git a/drivers/bus/vmbus/vmbus_common.c b/drivers/bus/vmbus/vmbus_common.c
index b9139c6e6c..1a64ab2712 100644
--- a/drivers/bus/vmbus/vmbus_common.c
+++ b/drivers/bus/vmbus/vmbus_common.c
@@ -199,7 +199,7 @@ rte_vmbus_probe(void)
 	char ubuf[RTE_UUID_STRLEN];
 
 	FOREACH_DEVICE_ON_VMBUS(dev) {
-		probed++;
+		dev->device_order = probed++;
 
 		rte_uuid_unparse(dev->device_id, ubuf, sizeof(ubuf));
 
-- 
2.34.1

Re: [PATCH v2] bus/vmbus: add device_order field to rte_vmbus_dev

[Stephen Hemminger]

On Mon, 24 Jun 2024 11:15:21 +0000
Vladimir Ratnikov <vratnikov@netgate.com> wrote:

> Field can be used to have static interface naming
> in DPDK based applications instead of uuid.
> RTE_FOREACH_DEV won't return the correct order of
> such devices due to vmbus_ignore_device in case of
> blocklisting/allowlisting. So device_order field
> will be helpful in that case.
> 
> Signed-off-by: Vladimir Ratnikov <vratnikov@netgate.com>

The patch only has the device_order field, but it is set
but not used anywhere in the DPDK. If you want to use it for naming
then please add that patch as well.

Re: [PATCH v2] bus/vmbus: add device_order field to rte_vmbus_dev

[Stephen Hemminger]

On Mon, 24 Jun 2024 11:15:21 +0000
Vladimir Ratnikov <vratnikov@netgate.com> wrote:

> Field can be used to have static interface naming
> in DPDK based applications instead of uuid.
> RTE_FOREACH_DEV won't return the correct order of
> such devices due to vmbus_ignore_device in case of
> blocklisting/allowlisting. So device_order field
> will be helpful in that case.
> 
> Signed-off-by: Vladimir Ratnikov <vratnikov@netgate.com>
> ---

Not sure the order of device discovery is predictable.
The order of devices on vmbus is when they were reported by the host OS.
That is why Linux driver use other ways to have persistent naming.

Re: [PATCH v2] bus/vmbus: add device_order field to rte_vmbus_dev

[Vladimir Ratnikov]

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]

Thanks for review!

Regarding to

> The patch only has the device_order field, but it is set but not used
> anywhere in the DPDK.

Unfortunately, It's not for DPDK usage, it's for DPDK based applications.
After some time of testing, if nothing's changed in the host OS(no new
devices, no removed devices) - the device list is always the same with the
same order.

On Tue, Jun 25, 2024 at 2:02 AM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Mon, 24 Jun 2024 11:15:21 +0000
> Vladimir Ratnikov <vratnikov@netgate.com> wrote:
>
> > Field can be used to have static interface naming
> > in DPDK based applications instead of uuid.
> > RTE_FOREACH_DEV won't return the correct order of
> > such devices due to vmbus_ignore_device in case of
> > blocklisting/allowlisting. So device_order field
> > will be helpful in that case.
> >
> > Signed-off-by: Vladimir Ratnikov <vratnikov@netgate.com>
> > ---
>
> Not sure the order of device discovery is predictable.
> The order of devices on vmbus is when they were reported by the host OS.
> That is why Linux driver use other ways to have persistent naming.
>

[-- Attachment #2: Type: text/html, Size: 1822 bytes --]

RE: [PATCH v2] bus/vmbus: add device_order field to rte_vmbus_dev

[Long Li]

[-- Attachment #1: Type: text/plain, Size: 1791 bytes --]

Hi Vladimir,

Is there another way that you can determine the device probe order from your application? (like going through the /sys/class/uio devices)

Long

From: Vladimir Ratnikov <vratnikov@netgate.com>
Sent: Tuesday, June 25, 2024 4:50 AM
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: Long Li <longli@microsoft.com>; dev@dpdk.org
Subject: Re: [PATCH v2] bus/vmbus: add device_order field to rte_vmbus_dev

You don't often get email from vratnikov@netgate.com<mailto:vratnikov@netgate.com>. Learn why this is important<https://aka.ms/LearnAboutSenderIdentification>
Thanks for review!

Regarding to
The patch only has the device_order field, but it is set but not used anywhere in the DPDK.
Unfortunately, It's not for DPDK usage, it's for DPDK based applications.
After some time of testing, if nothing's changed in the host OS(no new devices, no removed devices) - the device list is always the same with the same order.

On Tue, Jun 25, 2024 at 2:02 AM Stephen Hemminger <stephen@networkplumber.org<mailto:stephen@networkplumber.org>> wrote:
On Mon, 24 Jun 2024 11:15:21 +0000
Vladimir Ratnikov <vratnikov@netgate.com<mailto:vratnikov@netgate.com>> wrote:

> Field can be used to have static interface naming
> in DPDK based applications instead of uuid.
> RTE_FOREACH_DEV won't return the correct order of
> such devices due to vmbus_ignore_device in case of
> blocklisting/allowlisting. So device_order field
> will be helpful in that case.
>
> Signed-off-by: Vladimir Ratnikov <vratnikov@netgate.com<mailto:vratnikov@netgate.com>>
> ---

Not sure the order of device discovery is predictable.
The order of devices on vmbus is when they were reported by the host OS.
That is why Linux driver use other ways to have persistent naming.

[-- Attachment #2: Type: text/html, Size: 6371 bytes --]

Re: [PATCH v2] bus/vmbus: add device_order field to rte_vmbus_dev

[Stephen Hemminger]

On Mon, 24 Jun 2024 11:04:15 +0000
Vladimir Ratnikov <vratnikov@netgate.com> wrote:

> diff --git a/drivers/bus/vmbus/bus_vmbus_driver.h b/drivers/bus/vmbus/bus_vmbus_driver.h
> index e2475a642d..6b010cbe41 100644
> --- a/drivers/bus/vmbus/bus_vmbus_driver.h
> +++ b/drivers/bus/vmbus/bus_vmbus_driver.h
> @@ -37,6 +37,7 @@ struct rte_vmbus_device {
>  	rte_uuid_t device_id;		       /**< VMBUS device id */
>  	rte_uuid_t class_id;		       /**< VMBUS device type */
>  	uint32_t relid;			       /**< id for primary */
> +	uint16_t device_order;		       /**< Device order after probing */
>  	uint8_t monitor_id;		       /**< monitor page */
>  	int uio_num;			       /**< UIO device number */
>  	uint32_t *int_page;		       /**< VMBUS interrupt page */
> diff --git a/drivers/bus/vmbus/vmbus_common.c b/drivers/bus/vmbus/vmbus_common.c

Is this an ABI change?

Re: [PATCH v2] bus/vmbus: add device_order field to rte_vmbus_dev

[David Marchand]

On Mon, Jun 24, 2024 at 5:14 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 24 Jun 2024 11:04:15 +0000
> Vladimir Ratnikov <vratnikov@netgate.com> wrote:
>
> > diff --git a/drivers/bus/vmbus/bus_vmbus_driver.h b/drivers/bus/vmbus/bus_vmbus_driver.h
> > index e2475a642d..6b010cbe41 100644
> > --- a/drivers/bus/vmbus/bus_vmbus_driver.h
> > +++ b/drivers/bus/vmbus/bus_vmbus_driver.h
> > @@ -37,6 +37,7 @@ struct rte_vmbus_device {
> >       rte_uuid_t device_id;                  /**< VMBUS device id */
> >       rte_uuid_t class_id;                   /**< VMBUS device type */
> >       uint32_t relid;                        /**< id for primary */
> > +     uint16_t device_order;                 /**< Device order after probing */
> >       uint8_t monitor_id;                    /**< monitor page */
> >       int uio_num;                           /**< UIO device number */
> >       uint32_t *int_page;                    /**< VMBUS interrupt page */
> > diff --git a/drivers/bus/vmbus/vmbus_common.c b/drivers/bus/vmbus/vmbus_common.c
>
> Is this an ABI change?

drivers/bus/vmbus/meson.build:driver_sdk_headers = files('bus_vmbus_driver.h')

Only drivers of this bus know the rte_vmbus_device object.
So this patch does not impact the public ABI.

Yet, I fail to see what this patch is trying to achieve.


-- 
David Marchand