[PATCH net-next v3 0/3] net/mlx5: Expose additional devlink dev info

[PATCH net-next v3 0/3] net/mlx5: Expose additional devlink dev info

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

This patchset aims to expose couple of already defined serial numbers
for mlx5 driver.

On top of that, it introduces new field, "function.uid" and exposes
that for mlx5 driver.

Example:

$ devlink dev info
pci/0000:08:00.0:
  driver mlx5_core
  serial_number e4397f872caeed218000846daa7d2f49
  board.serial_number MT2314XZ00YA
  function.uid MT2314XZ00YAMLNXS0D0F0
  versions:
      fixed:
        fw.psid MT_0000000894
      running:
        fw.version 28.41.1000
        fw 28.41.1000
      stored:
        fw.version 28.41.1000
        fw 28.41.1000
auxiliary/mlx5_core.eth.0:
  driver mlx5_core.eth
pci/0000:08:00.1:
  driver mlx5_core
  serial_number e4397f872caeed218000846daa7d2f49
  board.serial_number MT2314XZ00YA
  function.uid MT2314XZ00YAMLNXS0D0F1
  versions:
      fixed:
        fw.psid MT_0000000894
      running:
        fw.version 28.41.1000
        fw 28.41.1000
      stored:
        fw.version 28.41.1000
        fw 28.41.1000
auxiliary/mlx5_core.eth.1:
  driver mlx5_core.eth

---
v2->v3:
- patch#1 from v2 was separatelly accepted
- patch#1:
  - do not continue in case devlink_info_*serial_number_put() returns
    error
- patch#2:
  - extended patch description
  - extended documentation
- patch#3:
  - do not continue in case devlink_info_function_uid_put() returns error
v1->v2:
- patch#2:
  - fixed possibly uninitialized variable "err"

Jiri Pirko (3):
  net/mlx5: Expose serial numbers in devlink info
  devlink: add function unique identifier to devlink dev info
  net/mlx5: Expose function UID in devlink info

 Documentation/netlink/specs/devlink.yaml      |  4 ++
 .../networking/devlink/devlink-info.rst       | 20 ++++++
 .../net/ethernet/mellanox/mlx5/core/devlink.c | 68 +++++++++++++++++++
 include/net/devlink.h                         |  2 +
 include/uapi/linux/devlink.h                  |  2 +
 net/devlink/dev.c                             |  9 +++
 6 files changed, 105 insertions(+)

-- 
2.49.0

[PATCH net-next v3 1/3] net/mlx5: Expose serial numbers in devlink info

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Devlink info allows to expose serial number and board serial number
Get the values from PCI VPD and expose it.

$ devlink dev info
pci/0000:08:00.0:
  driver mlx5_core
  serial_number e4397f872caeed218000846daa7d2f49
  board.serial_number MT2314XZ00YA
  versions:
      fixed:
        fw.psid MT_0000000894
      running:
        fw.version 28.41.1000
        fw 28.41.1000
      stored:
        fw.version 28.41.1000
        fw 28.41.1000
auxiliary/mlx5_core.eth.0:
  driver mlx5_core.eth
pci/0000:08:00.1:
  driver mlx5_core
  serial_number e4397f872caeed218000846daa7d2f49
  board.serial_number MT2314XZ00YA
  versions:
      fixed:
        fw.psid MT_0000000894
      running:
        fw.version 28.41.1000
        fw 28.41.1000
      stored:
        fw.version 28.41.1000
        fw 28.41.1000
auxiliary/mlx5_core.eth.1:
  driver mlx5_core.eth

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Acked-by: Tariq Toukan <tariqt@nvidia.com>
---
v2->v3:
- do not continue in case devlink_info_*serial_number_put() returns
  error
v1->v2:
- fixed possibly uninitialized variable "err"
---
 .../net/ethernet/mellanox/mlx5/core/devlink.c | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 73cd74644378..42218834183a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -35,6 +35,55 @@ static u16 mlx5_fw_ver_subminor(u32 version)
 	return version & 0xffff;
 }
 
+static int mlx5_devlink_serial_numbers_put(struct mlx5_core_dev *dev,
+					   struct devlink_info_req *req,
+					   struct netlink_ext_ack *extack)
+{
+	struct pci_dev *pdev = dev->pdev;
+	unsigned int vpd_size, kw_len;
+	char *str, *end;
+	u8 *vpd_data;
+	int err = 0;
+	int start;
+
+	vpd_data = pci_vpd_alloc(pdev, &vpd_size);
+	if (IS_ERR(vpd_data))
+		return 0;
+
+	start = pci_vpd_find_ro_info_keyword(vpd_data, vpd_size,
+					     PCI_VPD_RO_KEYWORD_SERIALNO, &kw_len);
+	if (start >= 0) {
+		str = kstrndup(vpd_data + start, kw_len, GFP_KERNEL);
+		if (!str) {
+			err = -ENOMEM;
+			goto end;
+		}
+		end = strchrnul(str, ' ');
+		*end = '\0';
+		err = devlink_info_board_serial_number_put(req, str);
+		kfree(str);
+		if (err)
+			goto end;
+	}
+
+	start = pci_vpd_find_ro_info_keyword(vpd_data, vpd_size, "V3", &kw_len);
+	if (start >= 0) {
+		str = kstrndup(vpd_data + start, kw_len, GFP_KERNEL);
+		if (!str) {
+			err = -ENOMEM;
+			goto end;
+		}
+		err = devlink_info_serial_number_put(req, str);
+		kfree(str);
+		if (err)
+			goto end;
+	}
+
+end:
+	kfree(vpd_data);
+	return err;
+}
+
 #define DEVLINK_FW_STRING_LEN 32
 
 static int
@@ -49,6 +98,10 @@ mlx5_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	if (!mlx5_core_is_pf(dev))
 		return 0;
 
+	err = mlx5_devlink_serial_numbers_put(dev, req, extack);
+	if (err)
+		return err;
+
 	err = devlink_info_version_fixed_put(req, "fw.psid", dev->board_id);
 	if (err)
 		return err;
-- 
2.49.0

[PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

A physical device may consists of several PCI physical functions.
Each of this PCI function's "serial_number" is same because they are
part of single board. From this serial number, PCI function cannot be
uniquely referenced in a system.

Expanding this in slightly more complex system of multi-host
"board.serial_number" is not even now unique across two hosts.

Further expanding this for DPU based board, a DPU board has PCI
functions on the external host as well as DPU internal host.
Such DPU side PCI physical functions also have the same "serial_number".

There is a need to identify each PCI function uniquely in a factory.
We are presently missing this function unique identifier.

Hence, introduce a function unique identifier, which is uniquely
identifies a function across one or multiple hosts, also has unique
identifier with/without DPU based NICs.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
---
v2->v3:
- extended patch description
- extended documentation
---
 Documentation/netlink/specs/devlink.yaml      |  4 ++++
 .../networking/devlink/devlink-info.rst       | 20 +++++++++++++++++++
 include/net/devlink.h                         |  2 ++
 include/uapi/linux/devlink.h                  |  2 ++
 net/devlink/dev.c                             |  9 +++++++++
 5 files changed, 37 insertions(+)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index bd9726269b4f..5d39eb68aefb 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -820,6 +820,9 @@ attribute-sets:
       -
         name: region-direct
         type: flag
+      -
+        name: info-function-uid
+        type: string
 
   -
     name: dl-dev-stats
@@ -1869,6 +1872,7 @@ operations:
             - info-version-running
             - info-version-stored
             - info-board-serial-number
+            - info-function-uid
       dump:
         reply: *info-get-reply
 
diff --git a/Documentation/networking/devlink/devlink-info.rst b/Documentation/networking/devlink/devlink-info.rst
index dd6adc4d0559..d0d4e17df148 100644
--- a/Documentation/networking/devlink/devlink-info.rst
+++ b/Documentation/networking/devlink/devlink-info.rst
@@ -50,6 +50,26 @@ versions is generally discouraged - here, and via any other Linux API.
        This is usually the serial number of the board, often available in
        PCI *Vital Product Data*.
 
+   * - ``function.uid``
+     - Function uniqueue identifier.
+
+       A physical device may consists of several PCI physical functions.
+       Each of this PCI function's ``serial_number`` is same because they are
+       part of single board. From this serial number, PCI function cannot be
+       uniquely referenced in a system.
+
+       Expanding this in slightly more complex system of multi-host
+       ``board.serial_number`` is not even now unique across two hosts.
+
+       Further expanding this for DPU based board, a DPU board has PCI
+       functions on the external host as well as DPU internal host.
+       Such DPU side PCI physical functions also have the same
+       ``serial_number``.
+
+       The function unique identifier uniquely identifies a function
+       across one or multiple hosts, also has unique identifier
+       with/without DPU based NICs.
+
    * - ``fixed``
      - Group for hardware identifiers, and versions of components
        which are not field-updatable.
diff --git a/include/net/devlink.h b/include/net/devlink.h
index b8783126c1ed..a0b84efd4740 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1846,6 +1846,8 @@ int devlink_info_serial_number_put(struct devlink_info_req *req,
 				   const char *sn);
 int devlink_info_board_serial_number_put(struct devlink_info_req *req,
 					 const char *bsn);
+int devlink_info_function_uid_put(struct devlink_info_req *req,
+				  const char *fuid);
 
 enum devlink_info_version_type {
 	DEVLINK_INFO_VERSION_TYPE_NONE,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 9401aa343673..816339790409 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -614,6 +614,8 @@ enum devlink_attr {
 
 	DEVLINK_ATTR_REGION_DIRECT,		/* flag */
 
+	DEVLINK_ATTR_INFO_FUNCTION_UID,		/* string */
+
 	/* Add new attributes above here, update the spec in
 	 * Documentation/netlink/specs/devlink.yaml and re-generate
 	 * net/devlink/netlink_gen.c.
diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index 02602704bdea..581a8ad7a568 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -763,6 +763,15 @@ int devlink_info_board_serial_number_put(struct devlink_info_req *req,
 }
 EXPORT_SYMBOL_GPL(devlink_info_board_serial_number_put);
 
+int devlink_info_function_uid_put(struct devlink_info_req *req,
+				  const char *fuid)
+{
+	if (!req->msg)
+		return 0;
+	return nla_put_string(req->msg, DEVLINK_ATTR_INFO_FUNCTION_UID, fuid);
+}
+EXPORT_SYMBOL_GPL(devlink_info_function_uid_put);
+
 static int devlink_info_version_put(struct devlink_info_req *req, int attr,
 				    const char *version_name,
 				    const char *version_value,
-- 
2.49.0

[PATCH net-next v3 3/3] net/mlx5: Expose function UID in devlink info

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Devlink info allows to expose function UID.
Get the value from PCI VPD and expose it.

$ devlink dev info
pci/0000:08:00.0:
  driver mlx5_core
  serial_number e4397f872caeed218000846daa7d2f49
  board.serial_number MT2314XZ00YA
  function.uid MT2314XZ00YAMLNXS0D0F0
  versions:
      fixed:
        fw.psid MT_0000000894
      running:
        fw.version 28.41.1000
        fw 28.41.1000
      stored:
        fw.version 28.41.1000
        fw 28.41.1000
auxiliary/mlx5_core.eth.0:
  driver mlx5_core.eth
pci/0000:08:00.1:
  driver mlx5_core
  serial_number e4397f872caeed218000846daa7d2f49
  board.serial_number MT2314XZ00YA
  function.uid MT2314XZ00YAMLNXS0D0F1
  versions:
      fixed:
        fw.psid MT_0000000894
      running:
        fw.version 28.41.1000
        fw 28.41.1000
      stored:
        fw.version 28.41.1000
        fw 28.41.1000
auxiliary/mlx5_core.eth.1:
  driver mlx5_core.eth

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Acked-by: Tariq Toukan <tariqt@nvidia.com>
---
v2->v3:
- do not continue in case devlink_info_function_uid_put() returns error
---
 drivers/net/ethernet/mellanox/mlx5/core/devlink.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 42218834183a..403c11694fa9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -79,6 +79,21 @@ static int mlx5_devlink_serial_numbers_put(struct mlx5_core_dev *dev,
 			goto end;
 	}
 
+	start = pci_vpd_find_ro_info_keyword(vpd_data, vpd_size, "VU", &kw_len);
+	if (start >= 0) {
+		str = kstrndup(vpd_data + start, kw_len, GFP_KERNEL);
+		if (!str) {
+			err = -ENOMEM;
+			goto end;
+		}
+		end = strchrnul(str, ' ');
+		*end = '\0';
+		err = devlink_info_function_uid_put(req, str);
+		kfree(str);
+		if (err)
+			goto end;
+	}
+
 end:
 	kfree(vpd_data);
 	return err;
-- 
2.49.0

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jakub Kicinski]

On Wed, 16 Apr 2025 23:41:32 +0200 Jiri Pirko wrote:
> A physical device may consists of several PCI physical functions.
> Each of this PCI function's "serial_number" is same because they are
> part of single board. From this serial number, PCI function cannot be
> uniquely referenced in a system.
> 
> Expanding this in slightly more complex system of multi-host
> "board.serial_number" is not even now unique across two hosts.
> 
> Further expanding this for DPU based board, a DPU board has PCI
> functions on the external host as well as DPU internal host.
> Such DPU side PCI physical functions also have the same "serial_number".
> 
> There is a need to identify each PCI function uniquely in a factory.
> We are presently missing this function unique identifier.
> 
> Hence, introduce a function unique identifier, which is uniquely
> identifies a function across one or multiple hosts, also has unique
> identifier with/without DPU based NICs.

Why do you think this should be a property of the instance?
We have PF ports.

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jiri Pirko]

Fri, Apr 18, 2025 at 03:38:22AM +0200, kuba@kernel.org wrote:
>On Wed, 16 Apr 2025 23:41:32 +0200 Jiri Pirko wrote:
>> A physical device may consists of several PCI physical functions.
>> Each of this PCI function's "serial_number" is same because they are
>> part of single board. From this serial number, PCI function cannot be
>> uniquely referenced in a system.
>> 
>> Expanding this in slightly more complex system of multi-host
>> "board.serial_number" is not even now unique across two hosts.
>> 
>> Further expanding this for DPU based board, a DPU board has PCI
>> functions on the external host as well as DPU internal host.
>> Such DPU side PCI physical functions also have the same "serial_number".
>> 
>> There is a need to identify each PCI function uniquely in a factory.
>> We are presently missing this function unique identifier.
>> 
>> Hence, introduce a function unique identifier, which is uniquely
>> identifies a function across one or multiple hosts, also has unique
>> identifier with/without DPU based NICs.
>
>Why do you think this should be a property of the instance?
>We have PF ports.

Ports does not look suitable to me. In case of a function with multiple
physical ports, would the same id be listed for multiple ports? What
about representors?

This is a function propertly, therefore it makes sense to me to put it
on devlink instance as devlink instance represents the function.

Another patchset that is most probably follow-up on this by one of my
colleagues will introduce fuid propertly on "devlink port function".
By that and the info exposed by this patch, you would be able to identify
which representor relates to which function cross-hosts. I think that
your question is actually aiming at this, isn't it?

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jakub Kicinski]

On Fri, 18 Apr 2025 12:15:01 +0200 Jiri Pirko wrote:
> Ports does not look suitable to me. In case of a function with multiple
> physical ports, would the same id be listed for multiple ports? What
> about representors?

You're stuck in nVidia thinking. PF port != Ethernet port.
I said PF port.

> This is a function propertly, therefore it makes sense to me to put it
> on devlink instance as devlink instance represents the function.
> 
> Another patchset that is most probably follow-up on this by one of my
> colleagues will introduce fuid propertly on "devlink port function".
> By that and the info exposed by this patch, you would be able to identify
> which representor relates to which function cross-hosts. I think that
> your question is actually aiming at this, isn't it?

Maybe it's time to pay off some technical debt instead of solving all
problems with yet another layer of new attributes :(

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jiri Pirko]

Sat, Apr 19, 2025 at 02:20:15AM +0200, kuba@kernel.org wrote:
>On Fri, 18 Apr 2025 12:15:01 +0200 Jiri Pirko wrote:
>> Ports does not look suitable to me. In case of a function with multiple
>> physical ports, would the same id be listed for multiple ports? What
>> about representors?
>
>You're stuck in nVidia thinking. PF port != Ethernet port.
>I said PF port.

PF port representor represents the eswitch side of the link to the
actual PF. The PF may or may not be on the same host.

Ethernet port is physical port.

Why this is nVidia thinking? How others understand it?


>
>> This is a function propertly, therefore it makes sense to me to put it
>> on devlink instance as devlink instance represents the function.
>> 
>> Another patchset that is most probably follow-up on this by one of my
>> colleagues will introduce fuid propertly on "devlink port function".
>> By that and the info exposed by this patch, you would be able to identify
>> which representor relates to which function cross-hosts. I think that
>> your question is actually aiming at this, isn't it?
>
>Maybe it's time to pay off some technical debt instead of solving all
>problems with yet another layer of new attributes :(

What do you mean by this? We need a way to identify port representor
(PF/VF/SF, does not matter which) and the other side of the wire that
may be on a different host. How else do you imagine to do the
identification of these 2 sides?

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jakub Kicinski]

On Tue, 22 Apr 2025 11:18:23 +0200 Jiri Pirko wrote:
> Sat, Apr 19, 2025 at 02:20:15AM +0200, kuba@kernel.org wrote:
> >On Fri, 18 Apr 2025 12:15:01 +0200 Jiri Pirko wrote:  
> >> Ports does not look suitable to me. In case of a function with multiple
> >> physical ports, would the same id be listed for multiple ports? What
> >> about representors?  
> >
> >You're stuck in nVidia thinking. PF port != Ethernet port.
> >I said PF port.  
> 
> PF port representor represents the eswitch side of the link to the
> actual PF. The PF may or may not be on the same host.
> 
> Ethernet port is physical port.
> 
> Why this is nVidia thinking? How others understand it?

Because you don't have a PF port for local PF.

The information you want to convey is which of the PF ports is "local".
I believe we discussed this >5 years ago when I was trying to solve
this exact problem for the NFP.

The topology information belongs on the ports, not the main instance.

> >> This is a function propertly, therefore it makes sense to me to put it
> >> on devlink instance as devlink instance represents the function.
> >> 
> >> Another patchset that is most probably follow-up on this by one of my
> >> colleagues will introduce fuid propertly on "devlink port function".
> >> By that and the info exposed by this patch, you would be able to identify
> >> which representor relates to which function cross-hosts. I think that
> >> your question is actually aiming at this, isn't it?  
> >
> >Maybe it's time to pay off some technical debt instead of solving all
> >problems with yet another layer of new attributes :(  
> 
> What do you mean by this? 

I keep saying that the devlink instance should represent the chip /
data processing pipeline.

> We need a way to identify port representor
> (PF/VF/SF, does not matter which) and the other side of the wire that
> may be on a different host. How else do you imagine to do the
> identification of these 2 sides?

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jiri Pirko]

Tue, Apr 22, 2025 at 05:02:38PM +0200, kuba@kernel.org wrote:
>On Tue, 22 Apr 2025 11:18:23 +0200 Jiri Pirko wrote:
>> Sat, Apr 19, 2025 at 02:20:15AM +0200, kuba@kernel.org wrote:
>> >On Fri, 18 Apr 2025 12:15:01 +0200 Jiri Pirko wrote:  
>> >> Ports does not look suitable to me. In case of a function with multiple
>> >> physical ports, would the same id be listed for multiple ports? What
>> >> about representors?  
>> >
>> >You're stuck in nVidia thinking. PF port != Ethernet port.
>> >I said PF port.  
>> 
>> PF port representor represents the eswitch side of the link to the
>> actual PF. The PF may or may not be on the same host.
>> 
>> Ethernet port is physical port.
>> 
>> Why this is nVidia thinking? How others understand it?
>
>Because you don't have a PF port for local PF.
>
>The information you want to convey is which of the PF ports is "local".
>I believe we discussed this >5 years ago when I was trying to solve
>this exact problem for the NFP.

If you instantiate a VF devlink instance, you would also like to see
"local" VF port? Does not make any sense to me honestly.

Why PF needs to have "local" PF port, isn't it a bit like Uroboros? The
PF devlink instance exists, the ports are links to other entities.
What's the reason to have a like to itself?


>
>The topology information belongs on the ports, not the main instance.

It's not a topology information. It's an entity property. Take VF for
example. VF also exposes FunctionUID under devlink info, same as PF.
There is no port instance under VF devlink instance. Same for SF.
Do you want to create dummy ports here just to have the "local" link?

I have to be missing something, the drawing as I see it fits 100%.


>
>> >> This is a function propertly, therefore it makes sense to me to put it
>> >> on devlink instance as devlink instance represents the function.
>> >> 
>> >> Another patchset that is most probably follow-up on this by one of my
>> >> colleagues will introduce fuid propertly on "devlink port function".
>> >> By that and the info exposed by this patch, you would be able to identify
>> >> which representor relates to which function cross-hosts. I think that
>> >> your question is actually aiming at this, isn't it?  
>> >
>> >Maybe it's time to pay off some technical debt instead of solving all
>> >problems with yet another layer of new attributes :(  
>> 
>> What do you mean by this? 
>
>I keep saying that the devlink instance should represent the chip /
>data processing pipeline.

It's instantiated per-PF/VF/SF. For VF and SF it represents the
function. For PF it should also represent a function. There is a concept
of per-asic faux device and related devlink instance I sent as RFC
patchset that is on top of PF devlink instances. That is the entity to
represent the chip and missing piece. Does that make sense?

https://lore.kernel.org/netdev/20250318124706.94156-1-jiri@resnulli.us/


>
>> We need a way to identify port representor
>> (PF/VF/SF, does not matter which) and the other side of the wire that
>> may be on a different host. How else do you imagine to do the
>> identification of these 2 sides?

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jakub Kicinski]

On Wed, 23 Apr 2025 13:23:46 +0200 Jiri Pirko wrote:
> >Because you don't have a PF port for local PF.
> >
> >The information you want to convey is which of the PF ports is "local".
> >I believe we discussed this >5 years ago when I was trying to solve
> >this exact problem for the NFP.  
> 
> If you instantiate a VF devlink instance, you would also like to see
> "local" VF port? Does not make any sense to me honestly.
> 
> Why PF needs to have "local" PF port, isn't it a bit like Uroboros? The
> PF devlink instance exists, the ports are links to other entities.
> What's the reason to have a like to itself?

Neither do VF devlink instances in the first place.

> >The topology information belongs on the ports, not the main instance.  
> 
> It's not a topology information. It's an entity property. Take VF for
> example. VF also exposes FunctionUID under devlink info, same as PF.
> There is no port instance under VF devlink instance. Same for SF.
> Do you want to create dummy ports here just to have the "local" link?
> 
> I have to be missing something, the drawing as I see it fits 100%.

Very hard to understand where you're coming from since you haven't
explained why the user has to suddenly care about this new property
you're adding.

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jiri Pirko]

Thu, Apr 24, 2025 at 12:17:45AM +0200, kuba@kernel.org wrote:
>On Wed, 23 Apr 2025 13:23:46 +0200 Jiri Pirko wrote:
>> >Because you don't have a PF port for local PF.
>> >
>> >The information you want to convey is which of the PF ports is "local".
>> >I believe we discussed this >5 years ago when I was trying to solve
>> >this exact problem for the NFP.  
>> 
>> If you instantiate a VF devlink instance, you would also like to see
>> "local" VF port? Does not make any sense to me honestly.
>> 
>> Why PF needs to have "local" PF port, isn't it a bit like Uroboros? The
>> PF devlink instance exists, the ports are links to other entities.
>> What's the reason to have a like to itself?
>
>Neither do VF devlink instances in the first place.

Sorry, I don't understand your point.

In VF instance, when there is no eswitch (in theory there could be one).
There is one devlink instance for VF and within that, one devlink port
instance with flavour "virtual". No other port. Simple.

If VF would manage nested eswitch, there would be another ports as
representors to links. This is more theoretical scenario, but I can
imagine VF managing eswitch and spawning SFs under it.

Same for PF. In case PF does not manage eswitch, there is one
devlink instance for the PF and one devlink port with flavour "physical".
In case the PF manages eswitch, there are other devlink ports, each
represents a link to another PF or VF or SF.

What are we missing here? What should be different?


>
>> >The topology information belongs on the ports, not the main instance.  
>> 
>> It's not a topology information. It's an entity property. Take VF for
>> example. VF also exposes FunctionUID under devlink info, same as PF.
>> There is no port instance under VF devlink instance. Same for SF.
>> Do you want to create dummy ports here just to have the "local" link?
>> 
>> I have to be missing something, the drawing as I see it fits 100%.
>
>Very hard to understand where you're coming from since you haven't
>explained why the user has to suddenly care about this new property
>you're adding.

That is simple. Same as we have serial_number and board.serial_number to
provide identification of NIC entities (asic/board), what I'm
introducing is a way to identify function - a function UID.

So 2 PF that belong under same asic would be distinquished by fUID.
Also, if you create couple of VFs, each will have different fUID.

Consider this patchset:
https://lore.kernel.org/netdev/1745416242-1162653-1-git-send-email-moshe@nvidia.com/

That adds a counterpart. PF devlink port function property:
$ devlink port show pci/0000:03:00.0/327680 -jp
{
    "port": {
        "pci/0000:03:00.0/327680": {
            "type": "eth",
            "netdev": "pf0hpf",
            "flavour": "pcipf",
            "controller": 1,
            "pfnum": 0,
            "external": true,
            "splittable": false,
            "function": {
                "hw_addr": "5c:25:73:37:70:5a",
                "roce": "enable",
                "max_io_eqs": 120,
                "uid":
"C6A76AD20605BE026D23C14E70B90704F4A5F5B3F304D83B37000732BF861D48MLNXS0D0F0"
            }
        }
    }
}

This you see on the PF that is managing the eswitch. This devlink port
is a representor of another PF, let's call it PFx. PFx may or may not be
on a different host. It's a link from PF managed eswitch to PFx.

So you have 2 PFs that are in hierarchy (one is on downlink of another),
each on different host.

To find out how these 2 are connected together, you need to know some
identification, on both sides. On PF side, that is port.function.uid the
patchset mentioned above introduces. On PFx, the same value is listed
under devlink info, which is what my patch we are discussing here is
adding.

Is there another way to see how things are connected together, if we
don't expose these identifiers? I don't see it :/

Sorry for not being clear from the beginning. I can extend the docs and
patch description with this info if needed.

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jakub Kicinski]

On Thu, 24 Apr 2025 11:42:09 +0200 Jiri Pirko wrote:
> This you see on the PF that is managing the eswitch. This devlink port
> is a representor of another PF, let's call it PFx. PFx may or may not be
> on a different host. It's a link from PF managed eswitch to PFx.
> 
> So you have 2 PFs that are in hierarchy (one is on downlink of another),
> each on different host.
> 
> To find out how these 2 are connected together, you need to know some
> identification, on both sides. On PF side, that is port.function.uid the
> patchset mentioned above introduces. On PFx, the same value is listed
> under devlink info, which is what my patch we are discussing here is
> adding.

Still not clear, sorry, can you show a diagram?
"Downsteam" makes me think the NIC device itself is a PCIe switch?

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jiri Pirko]

Fri, Apr 25, 2025 at 12:06:29AM +0200, kuba@kernel.org wrote:
>On Thu, 24 Apr 2025 11:42:09 +0200 Jiri Pirko wrote:
>> This you see on the PF that is managing the eswitch. This devlink port
>> is a representor of another PF, let's call it PFx. PFx may or may not be
>> on a different host. It's a link from PF managed eswitch to PFx.
>> 
>> So you have 2 PFs that are in hierarchy (one is on downlink of another),
>> each on different host.
>> 
>> To find out how these 2 are connected together, you need to know some
>> identification, on both sides. On PF side, that is port.function.uid the
>> patchset mentioned above introduces. On PFx, the same value is listed
>> under devlink info, which is what my patch we are discussing here is
>> adding.
>
>Still not clear, sorry, can you show a diagram?
>"Downsteam" makes me think the NIC device itself is a PCIe switch?

No, in theory, the PF can manage a nested eswitch. That is not the case
atm if I'm not mistaken for any device.

Here's the diagram:

               ┌──────────────────────────────────────────────────┐     ┌────────────────────────────────────────────────┐
               │                 host A (DPU)                     │     │  host B (host)                                 │
               │                                                  │     │                                                │
      ┌────────┼──────────────────────────────────────────────────┼─────┼──────────────────────────────────────────────┐ │
      │        │                                                  │     │                                              │ │
      │ NIC    │   ┌──────────────────────────────────────────┐   │     │                                              │ │
      │        │   │               PF0_devlink  info_fuid:A   │   │     │  ┌──────────────────────────────────┐        │ │
──────┼────────┼───┼─phys_dl_port                             │   │     │  │        PFx_devlink  info_fuid:C  │        │ │
      │        │   │                            PFx_dl_port───┼───┼─────┼──┼─virt_dl_port                     │        │ │
      │        │   │                              fuid:C      │   │     │  │                                  │        │ │
      │        │   │                                          │   │     │  └──────────────────────────────────┘        │ │
      │        │   │                            VFx1_dl_port──┼───┼──┐  │  ┌──────────────────────────────────┐        │ │
      │        │   │                              fuid:D      │   │  │  │  │       VFx1_devlink  info_fuid:D  │        │ │
      │        │   └──────────────────────────────────────────┘   │  └──┼──┼─virt_dl_port                     │        │ │
      │        │   ┌──────────────────────────────────────────┐   │     │  │                                  │        │ │
      │        │   │               PF1_devlink  info_fuid:B   │   │     │  └──────────────────────────────────┘        │ │
──────┼────────┼───┼─phys_dl_port                             │   │     │                                              │ │
      │        │   │                                          │   │     │                                              │ │
      │        │   └──────────────────────────────────────────┘   │     │                                              │ │
      └────────┼──────────────────────────────────────────────────┼─────┼──────────────────────────────────────────────┘ │
               │                                                  │     │                                                │
               └──────────────────────────────────────────────────┘     └────────────────────────────────────────────────┘



info_fuid is the devlink info function.uid I'm introducing.
the "fuid" under port is the port function uid attr from the RFC
patchset.

Is it clearer now? Should I extend the diagram by something you miss?

Thanks!

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jakub Kicinski]

On Fri, 25 Apr 2025 09:27:19 +0200 Jiri Pirko wrote:
> info_fuid is the devlink info function.uid I'm introducing.
> the "fuid" under port is the port function uid attr from the RFC
> patchset.
> 
> Is it clearer now? Should I extend the diagram by something you miss?

Yes, it is clear. The eswitch side makes sense.
You just need to find a better place to expose the client side.

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jiri Pirko]

Fri, Apr 25, 2025 at 10:45:29PM +0200, kuba@kernel.org wrote:
>On Fri, 25 Apr 2025 09:27:19 +0200 Jiri Pirko wrote:
>> info_fuid is the devlink info function.uid I'm introducing.
>> the "fuid" under port is the port function uid attr from the RFC
>> patchset.
>> 
>> Is it clearer now? Should I extend the diagram by something you miss?
>
>Yes, it is clear. The eswitch side makes sense.

Good.

>You just need to find a better place to expose the client side.

Okay. Why exactly you find it wrong to put it under devlink dev info?
I mean, to me it is perfect fit. It is basically another serial number,
uniqueue identification of devlink device.

If other place is better fit, I don't see it. Do you have some ideas?

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jakub Kicinski]

On Mon, 28 Apr 2025 18:28:24 +0200 Jiri Pirko wrote:
> >You just need to find a better place to expose the client side.  
> 
> Okay. Why exactly you find it wrong to put it under devlink dev info?
> I mean, to me it is perfect fit. It is basically another serial number,
> uniqueue identification of devlink device.
> 
> If other place is better fit, I don't see it.

devlink instance should represent the device, not a PF / port;
and then having the attribute on the instance does not fit
an eswitch controller with 2 PFs connected.

> Do you have some ideas?

Either subsystem specific (like a netdev attr) or not subsystem
specific (bus dev attr, so PCI). Forcing every endpoint to expose
a devlink instance just for one attribute is too much of a hack.

Re: [PATCH net-next v3 2/3] devlink: add function unique identifier to devlink dev info

[Jiri Pirko]

Mon, Apr 28, 2025 at 08:12:52PM +0200, kuba@kernel.org wrote:
>On Mon, 28 Apr 2025 18:28:24 +0200 Jiri Pirko wrote:
>> >You just need to find a better place to expose the client side.  
>> 
>> Okay. Why exactly you find it wrong to put it under devlink dev info?
>> I mean, to me it is perfect fit. It is basically another serial number,
>> uniqueue identification of devlink device.
>> 
>> If other place is better fit, I don't see it.
>
>devlink instance should represent the device, not a PF / port;
>and then having the attribute on the instance does not fit
>an eswitch controller with 2 PFs connected.

Why do you say so? In reality, devlink device is created per PF/VF/SF.
It actually makes sense as it is always based-on/backed-by parcticular
pci function or other device with address (like i2c).

The faux parent in the RFC I linked to is the per-device/asic instance
you are probably looking for. There is no real pci function backing it.
It is a parent of multiple PF devlink instances.


>
>> Do you have some ideas?
>
>Either subsystem specific (like a netdev attr) or not subsystem

I don't see how subsystem specific would make sense here, as the
function uuid is subsystem-agnostic.


>specific (bus dev attr, so PCI). Forcing every endpoint to expose
>a devlink instance just for one attribute is too much of a hack.

Wait, that's a misunderstanding. There is no forcing. Any driver has a
benevolency to either instantiate devlink instance for PF/VF/SF,
according to its needs, or not.

In mlx5, there is a need to do that. So if one has a need to expose
serial number of function id over devlink, he instantiates devlink
instance. What's even remotely hacky about that?