* [PATCH net-next V7 01/14] documentation: networking: add shared devlink documentation
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-02-03 3:40 ` Jakub Kicinski
2026-01-28 11:25 ` [PATCH net-next V7 02/14] devlink: introduce shared devlink instance for PFs on same chip Tariq Toukan
` (13 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Jiri Pirko <jiri@nvidia.com>
Document shared devlink instances for multiple PFs on the same chip.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../networking/devlink/devlink-shared.rst | 95 +++++++++++++++++++
Documentation/networking/devlink/index.rst | 1 +
2 files changed, 96 insertions(+)
create mode 100644 Documentation/networking/devlink/devlink-shared.rst
diff --git a/Documentation/networking/devlink/devlink-shared.rst b/Documentation/networking/devlink/devlink-shared.rst
new file mode 100644
index 000000000000..74655dc671bc
--- /dev/null
+++ b/Documentation/networking/devlink/devlink-shared.rst
@@ -0,0 +1,95 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============================
+Devlink Shared Instances
+============================
+
+Overview
+========
+
+Shared devlink instances allow multiple physical functions (PFs) on the same
+chip to share an additional devlink instance for chip-wide operations. This
+is implemented within individual drivers alongside the individual PF devlink
+instances, not replacing them.
+
+Multiple PFs may reside on the same physical chip, running a single firmware.
+Some of the resources and configurations may be shared among these PFs. The
+shared devlink instance provides an object to pin configuration knobs on.
+
+The shared devlink instance is backed by a faux device and provides a common
+interface for operations that affect the entire chip rather than individual PFs.
+A faux device is used as a backing device for the 'entire chip' since there's no
+additional real device instantiated by hardware besides the PF devices.
+
+Implementation
+==============
+
+Architecture
+------------
+
+The implementation uses:
+
+* **Faux device**: Virtual device backing the shared devlink instance
+* **Chip identification**: PFs are grouped by chip using a driver-specific identifier
+* **Shared instance management**: Global list of shared instances with reference counting
+
+API Functions
+-------------
+
+The following functions are provided for managing shared devlink instances:
+
+* ``devlink_shd_get()``: Get or create a shared devlink instance identified by a string ID
+* ``devlink_shd_put()``: Release a reference on a shared devlink instance
+* ``devlink_shd_get_priv()``: Get private data from shared devlink instance
+
+Initialization Flow
+-------------------
+
+1. **PF calls shared devlink init** during driver probe
+2. **Chip identification** using driver-specific method to determine device identity
+3. **Get or create shared instance** using ``devlink_shd_get()``:
+
+ * The function looks up existing instance by identifier
+ * If none exists, creates new instance:
+ - Creates faux device with chip identifier as name
+ - Allocates and registers devlink instance
+ - Adds to global shared instances list
+ - Increments reference count
+
+4. **Set nested devlink instance** for the PF devlink instance using
+ ``devl_nested_devlink_set()`` before registering the PF devlink instance
+
+Cleanup Flow
+------------
+
+1. **Cleanup** when PF is removed
+2. **Call** ``devlink_shd_put()`` to release reference (decrements reference count)
+3. **Shared instance is automatically destroyed** when the last PF removes (device list becomes empty)
+
+Chip Identification
+-------------------
+
+PFs belonging to the same chip are identified using a driver-specific method.
+The driver is free to choose any identifier that is suitable for determining
+whether two PFs are part of the same device. Examples include:
+
+* **PCI VPD serial numbers**: Extract from PCI VPD
+* **Device tree properties**: Read chip identifier from device tree
+* **Other hardware-specific identifiers**: Any unique identifier that groups PFs by chip
+
+Locking
+-------
+
+A global mutex (``shd_mutex``) protects the shared instances list during
+registration/deregistration.
+
+Similarly to other nested devlink instance relationships, devlink lock of
+the shared instance should be always taken after the devlink lock of PF.
+
+Reference Counting
+------------------
+
+Each shared devlink instance maintains a reference count (``refcount_t refcount``).
+The reference count is incremented when ``devlink_shd_get()`` is called and
+decremented when ``devlink_shd_put()`` is called. When the reference count
+reaches zero, the shared instance is automatically destroyed.
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index 35b12a2bfeba..f7ba7dcf477d 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -68,6 +68,7 @@ general.
devlink-resource
devlink-selftests
devlink-trap
+ devlink-shared
Driver-specific documentation
-----------------------------
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next V7 01/14] documentation: networking: add shared devlink documentation
2026-01-28 11:25 ` [PATCH net-next V7 01/14] documentation: networking: add shared devlink documentation Tariq Toukan
@ 2026-02-03 3:40 ` Jakub Kicinski
2026-02-03 9:18 ` Jiri Pirko
0 siblings, 1 reply; 32+ messages in thread
From: Jakub Kicinski @ 2026-02-03 3:40 UTC (permalink / raw)
To: Tariq Toukan
Cc: Eric Dumazet, Paolo Abeni, Andrew Lunn, David S. Miller,
Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
On Wed, 28 Jan 2026 13:25:31 +0200 Tariq Toukan wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Document shared devlink instances for multiple PFs on the same chip.
> diff --git a/Documentation/networking/devlink/devlink-shared.rst b/Documentation/networking/devlink/devlink-shared.rst
> new file mode 100644
> index 000000000000..74655dc671bc
> --- /dev/null
> +++ b/Documentation/networking/devlink/devlink-shared.rst
> @@ -0,0 +1,95 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +============================
> +Devlink Shared Instances
> +============================
Shouldn't the length of the ==== lines match the title length?
> +Overview
> +========
> +
> +Shared devlink instances allow multiple physical functions (PFs) on the same
> +chip to share an additional devlink instance for chip-wide operations. This
> +is implemented within individual drivers alongside the individual PF devlink
> +instances, not replacing them.
> +
> +Multiple PFs may reside on the same physical chip, running a single firmware.
> +Some of the resources and configurations may be shared among these PFs. The
> +shared devlink instance provides an object to pin configuration knobs on.
> +
> +The shared devlink instance is backed by a faux device and provides a common
> +interface for operations that affect the entire chip rather than individual PFs.
> +A faux device is used as a backing device for the 'entire chip' since there's no
> +additional real device instantiated by hardware besides the PF devices.
There needs to be a note here clearly stating the the use of "shared
devlink instace" is a hack for legacy drivers, and new drivers should
have a single devlink instance for the entire device. The fact that
single instance is always preferred, and *more correct* must be made
very clear to the reader. Ideally the single instance multiple function
implementation would leverage the infra added here for collecting the
functions, however.
> +Implementation
> +==============
> +
> +Architecture
> +------------
> +
> +The implementation uses:
> +
> +* **Faux device**: Virtual device backing the shared devlink instance
"backing"? It isn't backing anything, its just another hack because we
made the mistake of tying devlink instances to $bus/$device as an id.
Now we need a fake device to have an identifier.
> +* **Chip identification**: PFs are grouped by chip using a driver-specific identifier
> +* **Shared instance management**: Global list of shared instances with reference counting
> +
> +API Functions
> +-------------
> +
> +The following functions are provided for managing shared devlink instances:
> +
> +* ``devlink_shd_get()``: Get or create a shared devlink instance identified by a string ID
> +* ``devlink_shd_put()``: Release a reference on a shared devlink instance
> +* ``devlink_shd_get_priv()``: Get private data from shared devlink instance
> +
> +Initialization Flow
> +-------------------
> +
> +1. **PF calls shared devlink init** during driver probe
> +2. **Chip identification** using driver-specific method to determine device identity
This isn't very clear.
> +3. **Get or create shared instance** using ``devlink_shd_get()``:
Just "Call ``devlink_shd_get()`` with the identifier constructed in
step 2" (?) and then have the points below explain that it gets or
recreates
> + * The function looks up existing instance by identifier
> + * If none exists, creates new instance:
> + - Creates faux device with chip identifier as name
> + - Allocates and registers devlink instance
> + - Adds to global shared instances list
> + - Increments reference count
> +
> +4. **Set nested devlink instance** for the PF devlink instance using
> + ``devl_nested_devlink_set()`` before registering the PF devlink instance
> +
> +Cleanup Flow
> +------------
> +
> +1. **Cleanup** when PF is removed
"``.remove()`` callback for a PCIe device is called"
> +2. **Call** ``devlink_shd_put()`` to release reference (decrements reference count)
> +3. **Shared instance is automatically destroyed** when the last PF removes (device list becomes empty)
> +
> +Chip Identification
> +-------------------
> +
> +PFs belonging to the same chip are identified using a driver-specific method.
> +The driver is free to choose any identifier that is suitable for determining
> +whether two PFs are part of the same device. Examples include:
> +
> +* **PCI VPD serial numbers**: Extract from PCI VPD
> +* **Device tree properties**: Read chip identifier from device tree
> +* **Other hardware-specific identifiers**: Any unique identifier that groups PFs by chip
> +
> +Locking
> +-------
> +
> +A global mutex (``shd_mutex``) protects the shared instances list during
> +registration/deregistration.
> +
> +Similarly to other nested devlink instance relationships, devlink lock of
> +the shared instance should be always taken after the devlink lock of PF.
of an instance, not a PF
> +
> +Reference Counting
> +------------------
> +
> +Each shared devlink instance maintains a reference count (``refcount_t refcount``).
> +The reference count is incremented when ``devlink_shd_get()`` is called and
> +decremented when ``devlink_shd_put()`` is called. When the reference count
> +reaches zero, the shared instance is automatically destroyed.
I think AI went too far with the text generation here, this is very
obvious from the previous sections.
--
pw-bot: cr
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next V7 01/14] documentation: networking: add shared devlink documentation
2026-02-03 3:40 ` Jakub Kicinski
@ 2026-02-03 9:18 ` Jiri Pirko
2026-02-04 3:01 ` Jakub Kicinski
0 siblings, 1 reply; 32+ messages in thread
From: Jiri Pirko @ 2026-02-03 9:18 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Tariq Toukan, Eric Dumazet, Paolo Abeni, Andrew Lunn,
David S. Miller, Donald Hunter, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
Tue, Feb 03, 2026 at 04:40:23AM +0100, kuba@kernel.org wrote:
>On Wed, 28 Jan 2026 13:25:31 +0200 Tariq Toukan wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> Document shared devlink instances for multiple PFs on the same chip.
>
>> diff --git a/Documentation/networking/devlink/devlink-shared.rst b/Documentation/networking/devlink/devlink-shared.rst
>> new file mode 100644
>> index 000000000000..74655dc671bc
>> --- /dev/null
>> +++ b/Documentation/networking/devlink/devlink-shared.rst
>> @@ -0,0 +1,95 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +============================
>> +Devlink Shared Instances
>> +============================
>
>Shouldn't the length of the ==== lines match the title length?
>
>> +Overview
>> +========
>> +
>> +Shared devlink instances allow multiple physical functions (PFs) on the same
>> +chip to share an additional devlink instance for chip-wide operations. This
>> +is implemented within individual drivers alongside the individual PF devlink
>> +instances, not replacing them.
>> +
>> +Multiple PFs may reside on the same physical chip, running a single firmware.
>> +Some of the resources and configurations may be shared among these PFs. The
>> +shared devlink instance provides an object to pin configuration knobs on.
>> +
>> +The shared devlink instance is backed by a faux device and provides a common
>> +interface for operations that affect the entire chip rather than individual PFs.
>> +A faux device is used as a backing device for the 'entire chip' since there's no
>> +additional real device instantiated by hardware besides the PF devices.
>
>There needs to be a note here clearly stating the the use of "shared
>devlink instace" is a hack for legacy drivers, and new drivers should
>have a single devlink instance for the entire device. The fact that
>single instance is always preferred, and *more correct* must be made
>very clear to the reader. Ideally the single instance multiple function
>implementation would leverage the infra added here for collecting the
>functions, however.
How exactly you can have a single devlink instance for multiple PFs of a
same device? I don't really understand how that could work, considering
dynamic binds/unbinds of the PFs within single host and/or multiple VMs
passing PFs to.
>
>> +Implementation
>> +==============
>> +
>> +Architecture
>> +------------
>> +
>> +The implementation uses:
>> +
>> +* **Faux device**: Virtual device backing the shared devlink instance
>
>"backing"? It isn't backing anything, its just another hack because we
>made the mistake of tying devlink instances to $bus/$device as an id.
>Now we need a fake device to have an identifier.
Okay. I originally wanted to use an id, similar to what we have in
the dpll. However I was forced by community to tie the instance to
bus/device. It is how it is, any idea how to relax this bond?
>
>> +* **Chip identification**: PFs are grouped by chip using a driver-specific identifier
>> +* **Shared instance management**: Global list of shared instances with reference counting
>> +
>> +API Functions
>> +-------------
>> +
>> +The following functions are provided for managing shared devlink instances:
>> +
>> +* ``devlink_shd_get()``: Get or create a shared devlink instance identified by a string ID
>> +* ``devlink_shd_put()``: Release a reference on a shared devlink instance
>> +* ``devlink_shd_get_priv()``: Get private data from shared devlink instance
>> +
>> +Initialization Flow
>> +-------------------
>> +
>> +1. **PF calls shared devlink init** during driver probe
>> +2. **Chip identification** using driver-specific method to determine device identity
>
>This isn't very clear.
Hmm, any suggestion to make it cleaner?
>
>> +3. **Get or create shared instance** using ``devlink_shd_get()``:
>
>Just "Call ``devlink_shd_get()`` with the identifier constructed in
>step 2" (?) and then have the points below explain that it gets or
>recreates
Okay.
>
>> + * The function looks up existing instance by identifier
>> + * If none exists, creates new instance:
>> + - Creates faux device with chip identifier as name
>> + - Allocates and registers devlink instance
>> + - Adds to global shared instances list
>> + - Increments reference count
>> +
>> +4. **Set nested devlink instance** for the PF devlink instance using
>> + ``devl_nested_devlink_set()`` before registering the PF devlink instance
>> +
>> +Cleanup Flow
>> +------------
>> +
>> +1. **Cleanup** when PF is removed
>
>"``.remove()`` callback for a PCIe device is called"
>
>> +2. **Call** ``devlink_shd_put()`` to release reference (decrements reference count)
>> +3. **Shared instance is automatically destroyed** when the last PF removes (device list becomes empty)
>> +
>> +Chip Identification
>> +-------------------
>> +
>> +PFs belonging to the same chip are identified using a driver-specific method.
>> +The driver is free to choose any identifier that is suitable for determining
>> +whether two PFs are part of the same device. Examples include:
>> +
>> +* **PCI VPD serial numbers**: Extract from PCI VPD
>> +* **Device tree properties**: Read chip identifier from device tree
>> +* **Other hardware-specific identifiers**: Any unique identifier that groups PFs by chip
>> +
>> +Locking
>> +-------
>> +
>> +A global mutex (``shd_mutex``) protects the shared instances list during
>> +registration/deregistration.
>> +
>> +Similarly to other nested devlink instance relationships, devlink lock of
>> +the shared instance should be always taken after the devlink lock of PF.
>
>of an instance, not a PF
lock of PF devlink instance. I think that is what the text says, no?
>
>> +
>> +Reference Counting
>> +------------------
>> +
>> +Each shared devlink instance maintains a reference count (``refcount_t refcount``).
>> +The reference count is incremented when ``devlink_shd_get()`` is called and
>> +decremented when ``devlink_shd_put()`` is called. When the reference count
>> +reaches zero, the shared instance is automatically destroyed.
>
>I think AI went too far with the text generation here, this is very
>obvious from the previous sections.
>--
>pw-bot: cr
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next V7 01/14] documentation: networking: add shared devlink documentation
2026-02-03 9:18 ` Jiri Pirko
@ 2026-02-04 3:01 ` Jakub Kicinski
2026-02-04 7:12 ` Jiri Pirko
0 siblings, 1 reply; 32+ messages in thread
From: Jakub Kicinski @ 2026-02-04 3:01 UTC (permalink / raw)
To: Jiri Pirko
Cc: Tariq Toukan, Eric Dumazet, Paolo Abeni, Andrew Lunn,
David S. Miller, Donald Hunter, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
On Tue, 3 Feb 2026 10:18:22 +0100 Jiri Pirko wrote:
> Tue, Feb 03, 2026 at 04:40:23AM +0100, kuba@kernel.org wrote:
> >There needs to be a note here clearly stating the the use of "shared
> >devlink instace" is a hack for legacy drivers, and new drivers should
> >have a single devlink instance for the entire device. The fact that
> >single instance is always preferred, and *more correct* must be made
> >very clear to the reader. Ideally the single instance multiple function
> >implementation would leverage the infra added here for collecting the
> >functions, however.
>
> How exactly you can have a single devlink instance for multiple PFs of a
> same device? I don't really understand how that could work, considering
> dynamic binds/unbinds of the PFs within single host and/or multiple VMs
> passing PFs to.
The same way you currently gather up the devlink instances to create
the shared instance.
> >> +The implementation uses:
> >> +
> >> +* **Faux device**: Virtual device backing the shared devlink instance
> >
> >"backing"? It isn't backing anything, its just another hack because we
> >made the mistake of tying devlink instances to $bus/$device as an id.
> >Now we need a fake device to have an identifier.
>
> Okay. I originally wanted to use an id, similar to what we have in
> the dpll. However I was forced by community to tie the instance to
> bus/device. It is how it is, any idea how to relax this bond?
Interesting! I was curious to research how we ended up here, found this:
https://lore.kernel.org/netdev/20160225225803.GA2191@nanopsycho.orion/
My reading is that Hannes was arguing against the _NAME attribute but
both _NAME and _INDEX were deleted? I think there's nothing wrong with
an index.
FWIW using devlink day to day, the bus/device is not at all useful as
an identifier. Most of code touching devlink at Meta either matches
on devlink dev info or assumes there's one instance on the system.
> >> +Similarly to other nested devlink instance relationships, devlink lock of
> >> +the shared instance should be always taken after the devlink lock of PF.
> >
> >of an instance, not a PF
>
> lock of PF devlink instance. I think that is what the text says, no?
Sorry, I was trying to flag that using PF is not necessary great cause
we may support this on other functions in the future.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next V7 01/14] documentation: networking: add shared devlink documentation
2026-02-04 3:01 ` Jakub Kicinski
@ 2026-02-04 7:12 ` Jiri Pirko
2026-02-05 2:02 ` Jakub Kicinski
0 siblings, 1 reply; 32+ messages in thread
From: Jiri Pirko @ 2026-02-04 7:12 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Tariq Toukan, Eric Dumazet, Paolo Abeni, Andrew Lunn,
David S. Miller, Donald Hunter, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
Wed, Feb 04, 2026 at 04:01:05AM +0100, kuba@kernel.org wrote:
>On Tue, 3 Feb 2026 10:18:22 +0100 Jiri Pirko wrote:
>> Tue, Feb 03, 2026 at 04:40:23AM +0100, kuba@kernel.org wrote:
>> >There needs to be a note here clearly stating the the use of "shared
>> >devlink instace" is a hack for legacy drivers, and new drivers should
>> >have a single devlink instance for the entire device. The fact that
>> >single instance is always preferred, and *more correct* must be made
>> >very clear to the reader. Ideally the single instance multiple function
>> >implementation would leverage the infra added here for collecting the
>> >functions, however.
>>
>> How exactly you can have a single devlink instance for multiple PFs of a
>> same device? I don't really understand how that could work, considering
>> dynamic binds/unbinds of the PFs within single host and/or multiple VMs
>> passing PFs to.
>
>The same way you currently gather up the devlink instances to create
>the shared instance.
What's the backing device / handle (busname/devname)? Best would be to
draw a picture, as always :)
>
>> >> +The implementation uses:
>> >> +
>> >> +* **Faux device**: Virtual device backing the shared devlink instance
>> >
>> >"backing"? It isn't backing anything, its just another hack because we
>> >made the mistake of tying devlink instances to $bus/$device as an id.
>> >Now we need a fake device to have an identifier.
>>
>> Okay. I originally wanted to use an id, similar to what we have in
>> the dpll. However I was forced by community to tie the instance to
>> bus/device. It is how it is, any idea how to relax this bond?
>
>Interesting! I was curious to research how we ended up here, found this:
>https://lore.kernel.org/netdev/20160225225803.GA2191@nanopsycho.orion/
>My reading is that Hannes was arguing against the _NAME attribute but
>both _NAME and _INDEX were deleted? I think there's nothing wrong with
>an index.
He argues for "stable topology indentifiers", which randomly assigned
index is not.
>
>FWIW using devlink day to day, the bus/device is not at all useful as
>an identifier. Most of code touching devlink at Meta either matches
>on devlink dev info or assumes there's one instance on the system.
Okay, what's your suggestion going foreward then?
>
>> >> +Similarly to other nested devlink instance relationships, devlink lock of
>> >> +the shared instance should be always taken after the devlink lock of PF.
>> >
>> >of an instance, not a PF
>>
>> lock of PF devlink instance. I think that is what the text says, no?
>
>Sorry, I was trying to flag that using PF is not necessary great cause
>we may support this on other functions in the future.
You are right, will fix this.
Thanks!
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next V7 01/14] documentation: networking: add shared devlink documentation
2026-02-04 7:12 ` Jiri Pirko
@ 2026-02-05 2:02 ` Jakub Kicinski
2026-02-06 10:52 ` Jiri Pirko
0 siblings, 1 reply; 32+ messages in thread
From: Jakub Kicinski @ 2026-02-05 2:02 UTC (permalink / raw)
To: Jiri Pirko
Cc: Tariq Toukan, Eric Dumazet, Paolo Abeni, Andrew Lunn,
David S. Miller, Donald Hunter, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
On Wed, 4 Feb 2026 08:12:00 +0100 Jiri Pirko wrote:
> Wed, Feb 04, 2026 at 04:01:05AM +0100, kuba@kernel.org wrote:
> >On Tue, 3 Feb 2026 10:18:22 +0100 Jiri Pirko wrote:
> >> How exactly you can have a single devlink instance for multiple PFs of a
> >> same device? I don't really understand how that could work, considering
> >> dynamic binds/unbinds of the PFs within single host and/or multiple VMs
> >> passing PFs to.
> >
> >The same way you currently gather up the devlink instances to create
> >the shared instance.
>
> What's the backing device / handle (busname/devname)? Best would be to
> draw a picture, as always :)
Either the bus/dev that shows up first or we go back to index.
(My main point being that the single instance is strictly better
than shared, ie. no problem exists in single instance multi func
which does not exist in multi instance + extra instance multi func.
But some problems do exist in multi instance which do not in single
like the locking)
> >> Okay. I originally wanted to use an id, similar to what we have in
> >> the dpll. However I was forced by community to tie the instance to
> >> bus/device. It is how it is, any idea how to relax this bond?
> >
> >Interesting! I was curious to research how we ended up here, found this:
> >https://lore.kernel.org/netdev/20160225225803.GA2191@nanopsycho.orion/
> >My reading is that Hannes was arguing against the _NAME attribute but
> >both _NAME and _INDEX were deleted? I think there's nothing wrong with
> >an index.
>
> He argues for "stable topology indentifiers", which randomly assigned
> index is not.
Agreed, I love me a stable identifier myself! :) That does not mean
we can't have ID _as well_ as the identifiers. Which lets us add
more stable identifiers and/or making some optional.
I think I was trying to sell you on "more stable identifiers"
as a alternative to ALT_NAMEs for netdevs at some point ;)
Maybe I'm projecting that conversation onto what Hannes said.
> >FWIW using devlink day to day, the bus/device is not at all useful as
> >an identifier. Most of code touching devlink at Meta either matches
> >on devlink dev info or assumes there's one instance on the system.
>
> Okay, what's your suggestion going foreward then?
Add the ID back, make bus/dev optional, forgo the faux dev?
Would that work? Would exiting CLI become very unhappy? :S
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next V7 01/14] documentation: networking: add shared devlink documentation
2026-02-05 2:02 ` Jakub Kicinski
@ 2026-02-06 10:52 ` Jiri Pirko
2026-02-07 1:50 ` Jakub Kicinski
0 siblings, 1 reply; 32+ messages in thread
From: Jiri Pirko @ 2026-02-06 10:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Tariq Toukan, Eric Dumazet, Paolo Abeni, Andrew Lunn,
David S. Miller, Donald Hunter, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
Thu, Feb 05, 2026 at 03:02:56AM +0100, kuba@kernel.org wrote:
>On Wed, 4 Feb 2026 08:12:00 +0100 Jiri Pirko wrote:
>> Wed, Feb 04, 2026 at 04:01:05AM +0100, kuba@kernel.org wrote:
>> >On Tue, 3 Feb 2026 10:18:22 +0100 Jiri Pirko wrote:
>> >> How exactly you can have a single devlink instance for multiple PFs of a
>> >> same device? I don't really understand how that could work, considering
>> >> dynamic binds/unbinds of the PFs within single host and/or multiple VMs
>> >> passing PFs to.
>> >
>> >The same way you currently gather up the devlink instances to create
>> >the shared instance.
>>
>> What's the backing device / handle (busname/devname)? Best would be to
>> draw a picture, as always :)
>
>Either the bus/dev that shows up first or we go back to index.
That may be tricky in case you bind/unbind the PFs randomly. You may and
up with devlink handle of PF1 with only member PF2. Confusing at least.
You need to expose the members to the user anyway somehow. That is
exactly the list of nested instances these patchset is adding.
>(My main point being that the single instance is strictly better
>than shared, ie. no problem exists in single instance multi func
>which does not exist in multi instance + extra instance multi func.
>But some problems do exist in multi instance which do not in single
>like the locking)
I think that my shared and your shared are basically the same as far as
the nested PF instances are not really used for anything except the
modelling purposes. That should be up to the driver how he wants to play
it, shouldn't it?
>
>> >> Okay. I originally wanted to use an id, similar to what we have in
>> >> the dpll. However I was forced by community to tie the instance to
>> >> bus/device. It is how it is, any idea how to relax this bond?
>> >
>> >Interesting! I was curious to research how we ended up here, found this:
>> >https://lore.kernel.org/netdev/20160225225803.GA2191@nanopsycho.orion/
>> >My reading is that Hannes was arguing against the _NAME attribute but
>> >both _NAME and _INDEX were deleted? I think there's nothing wrong with
>> >an index.
>>
>> He argues for "stable topology indentifiers", which randomly assigned
>> index is not.
>
>Agreed, I love me a stable identifier myself! :) That does not mean
>we can't have ID _as well_ as the identifiers. Which lets us add
>more stable identifiers and/or making some optional.
Got it.
>
>I think I was trying to sell you on "more stable identifiers"
>as a alternative to ALT_NAMEs for netdevs at some point ;)
I don't recall that. Anyway, everyone loves ALT_NAMEs at this point :)
>Maybe I'm projecting that conversation onto what Hannes said.
>
>> >FWIW using devlink day to day, the bus/device is not at all useful as
>> >an identifier. Most of code touching devlink at Meta either matches
>> >on devlink dev info or assumes there's one instance on the system.
>>
>> Okay, what's your suggestion going foreward then?
>
>Add the ID back, make bus/dev optional, forgo the faux dev?
>Would that work? Would exiting CLI become very unhappy? :S
Ha, that would break so many things, everything is based on
bus/dev on UAPI level :/
I was thinking about having some sort of *special-bus-name* for indexes,
like:
none/1
none/2
or
devlink_id/1
devlink_id/2
or something like that.
But it would be either that or original bus/dev, not both :/
Perhaps we can add ID attr as optional/alternative handle? Then legacy
userspace can use existing bus/dev handle and new can use the ID attr?
Then the example output would look something like:
$ devlink dev
pci/0000:08:00.0: id 1
nested_devlink:
auxiliary/mlx5_core.eth.0
devlink_id/2: id 2
nested_devlink:
pci/0000:08:00.0
pci/0000:08:00.1
auxiliary/mlx5_core.eth.0: id 3
pci/0000:08:00.1: id 4
nested_devlink:
auxiliary/mlx5_core.eth.1
auxiliary/mlx5_core.eth.1: id 5
Makes sense?
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next V7 01/14] documentation: networking: add shared devlink documentation
2026-02-06 10:52 ` Jiri Pirko
@ 2026-02-07 1:50 ` Jakub Kicinski
0 siblings, 0 replies; 32+ messages in thread
From: Jakub Kicinski @ 2026-02-07 1:50 UTC (permalink / raw)
To: Jiri Pirko
Cc: Tariq Toukan, Eric Dumazet, Paolo Abeni, Andrew Lunn,
David S. Miller, Donald Hunter, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
On Fri, 6 Feb 2026 11:52:21 +0100 Jiri Pirko wrote:
> Thu, Feb 05, 2026 at 03:02:56AM +0100, kuba@kernel.org wrote:
> >On Wed, 4 Feb 2026 08:12:00 +0100 Jiri Pirko wrote:
> >> What's the backing device / handle (busname/devname)? Best would be to
> >> draw a picture, as always :)
> >
> >Either the bus/dev that shows up first or we go back to index.
>
> That may be tricky in case you bind/unbind the PFs randomly. You may and
> up with devlink handle of PF1 with only member PF2. Confusing at least.
> You need to expose the members to the user anyway somehow. That is
> exactly the list of nested instances these patchset is adding.
Yes, simpler setup would likely be to have one function designated
as "main". This could get fairly complicated, OTOH most current use
cases are fairly rigid, with the function config being per device SKU.
From uAPI perspective having the list of currently present functions
and ports should be flexible for current and future use cases.
> >(My main point being that the single instance is strictly better
> >than shared, ie. no problem exists in single instance multi func
> >which does not exist in multi instance + extra instance multi func.
> >But some problems do exist in multi instance which do not in single
> >like the locking)
>
> I think that my shared and your shared are basically the same as far as
> the nested PF instances are not really used for anything except the
> modelling purposes. That should be up to the driver how he wants to play
> it, shouldn't it?
Man, seeing the locking shenanigans that the driver developers end up
doing just in the last two weeks makes me question the "leave it to
the driver" attitude. I'd much rather have code that works than code
that is adaptive to multiple levels of incompetence.
> >I think I was trying to sell you on "more stable identifiers"
> >as a alternative to ALT_NAMEs for netdevs at some point ;)
>
> I don't recall that. Anyway, everyone loves ALT_NAMEs at this point :)
:)
> >> Okay, what's your suggestion going foreward then?
> >
> >Add the ID back, make bus/dev optional, forgo the faux dev?
> >Would that work? Would exiting CLI become very unhappy? :S
>
> Ha, that would break so many things, everything is based on
> bus/dev on UAPI level :/
>
> I was thinking about having some sort of *special-bus-name* for indexes,
> like:
>
> none/1
> none/2
> or
> devlink_id/1
> devlink_id/2
> or something like that.
>
> But it would be either that or original bus/dev, not both :/
>
> Perhaps we can add ID attr as optional/alternative handle? Then legacy
> userspace can use existing bus/dev handle and new can use the ID attr?
> Then the example output would look something like:
>
> $ devlink dev
> pci/0000:08:00.0: id 1
> nested_devlink:
> auxiliary/mlx5_core.eth.0
> devlink_id/2: id 2
> nested_devlink:
> pci/0000:08:00.0
> pci/0000:08:00.1
> auxiliary/mlx5_core.eth.0: id 3
> pci/0000:08:00.1: id 4
> nested_devlink:
> auxiliary/mlx5_core.eth.1
> auxiliary/mlx5_core.eth.1: id 5
>
> Makes sense?
Yup, seems reasonable.
Alternatively we could add a socket or request flag to dumps that
indicates that user space is aware that some instances will not
have the bus/dev identifier?
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next V7 02/14] devlink: introduce shared devlink instance for PFs on same chip
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
2026-01-28 11:25 ` [PATCH net-next V7 01/14] documentation: networking: add shared devlink documentation Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-02-03 3:49 ` Jakub Kicinski
2026-01-28 11:25 ` [PATCH net-next V7 03/14] devlink: Reverse locking order for nested instances Tariq Toukan
` (12 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Jiri Pirko <jiri@nvidia.com>
Multiple PFs may reside on the same physical chip, running a single
firmware. Some of the resources and configurations may be shared among
these PFs. Currently, there is no good object to pin the configuration
knobs on.
Introduce a shared devlink instance, instantiated upon probe of the
first PF and removed during remove of the last PF. The shared devlink
instance is backed by a faux device, as there is no PCI device related
to it. The implementation uses reference counting to manage the
lifecycle: each PF that probes calls devlink_shd_get() to get or create
the shared instance, and calls devlink_shd_put() when it removes. The
shared instance is automatically destroyed when the last PF removes.
Example:
$ devlink dev
pci/0000:08:00.0:
nested_devlink:
auxiliary/mlx5_core.eth.0
faux/mlx5_core_83013c12b77faa1a30000c82a1045c91:
nested_devlink:
pci/0000:08:00.0
pci/0000:08:00.1
auxiliary/mlx5_core.eth.0
pci/0000:08:00.1:
nested_devlink:
auxiliary/mlx5_core.eth.1
auxiliary/mlx5_core.eth.1
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
include/net/devlink.h | 6 ++
net/devlink/Makefile | 2 +-
net/devlink/sh_dev.c | 163 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 170 insertions(+), 1 deletion(-)
create mode 100644 net/devlink/sh_dev.c
diff --git a/include/net/devlink.h b/include/net/devlink.h
index cb839e0435a1..c453faec8ebf 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1644,6 +1644,12 @@ void devlink_register(struct devlink *devlink);
void devlink_unregister(struct devlink *devlink);
void devlink_free(struct devlink *devlink);
+struct devlink *devlink_shd_get(const char *id,
+ const struct devlink_ops *ops,
+ size_t priv_size);
+void devlink_shd_put(struct devlink *devlink);
+void *devlink_shd_get_priv(struct devlink *devlink);
+
/**
* struct devlink_port_ops - Port operations
* @port_split: Callback used to split the port into multiple ones.
diff --git a/net/devlink/Makefile b/net/devlink/Makefile
index 000da622116a..8f2adb5e5836 100644
--- a/net/devlink/Makefile
+++ b/net/devlink/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-y := core.o netlink.o netlink_gen.o dev.o port.o sb.o dpipe.o \
- resource.o param.o region.o health.o trap.o rate.o linecard.o
+ resource.o param.o region.o health.o trap.o rate.o linecard.o sh_dev.o
diff --git a/net/devlink/sh_dev.c b/net/devlink/sh_dev.c
new file mode 100644
index 000000000000..acc8d549aaae
--- /dev/null
+++ b/net/devlink/sh_dev.c
@@ -0,0 +1,163 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. */
+
+#include <linux/device/faux.h>
+#include <net/devlink.h>
+
+#include "devl_internal.h"
+
+static LIST_HEAD(shd_list);
+static DEFINE_MUTEX(shd_mutex); /* Protects shd_list and shd->list */
+
+/* This structure represents a shared devlink instance,
+ * there is one created per identifier (e.g., serial number).
+ */
+struct devlink_shd {
+ struct list_head list; /* Node in shd list */
+ const char *id; /* Identifier string (e.g., serial number) */
+ struct faux_device *faux_dev; /* Related faux device */
+ refcount_t refcount; /* Reference count */
+ char priv[] __aligned(NETDEV_ALIGN); /* Driver private data */
+};
+
+static struct devlink_shd *devlink_shd_lookup(const char *id)
+{
+ struct devlink_shd *shd;
+
+ list_for_each_entry(shd, &shd_list, list) {
+ if (!strcmp(shd->id, id))
+ return shd;
+ }
+
+ return NULL;
+}
+
+static struct devlink_shd *devlink_shd_create(const char *id,
+ const struct devlink_ops *ops,
+ size_t priv_size)
+{
+ struct faux_device *faux_dev;
+ struct devlink_shd *shd;
+ struct devlink *devlink;
+
+ /* Create faux device - probe will be called synchronously */
+ faux_dev = faux_device_create(id, NULL, NULL);
+ if (!faux_dev)
+ return NULL;
+
+ devlink = devlink_alloc(ops, sizeof(struct devlink_shd) + priv_size,
+ &faux_dev->dev);
+ if (!devlink)
+ goto err_devlink_alloc;
+ shd = devlink_priv(devlink);
+
+ shd->id = kstrdup(id, GFP_KERNEL);
+ if (!shd->id)
+ goto err_kstrdup_id;
+ shd->faux_dev = faux_dev;
+ refcount_set(&shd->refcount, 1);
+
+ devl_lock(devlink);
+ devl_register(devlink);
+ devl_unlock(devlink);
+
+ list_add_tail(&shd->list, &shd_list);
+
+ return shd;
+
+err_kstrdup_id:
+ devlink_free(devlink);
+
+err_devlink_alloc:
+ faux_device_destroy(faux_dev);
+ return NULL;
+}
+
+static void devlink_shd_destroy(struct devlink_shd *shd)
+{
+ struct devlink *devlink = priv_to_devlink(shd);
+ struct faux_device *faux_dev = shd->faux_dev;
+
+ list_del(&shd->list);
+ devl_lock(devlink);
+ devl_unregister(devlink);
+ devl_unlock(devlink);
+ kfree(shd->id);
+ devlink_free(devlink);
+ faux_device_destroy(faux_dev);
+}
+
+/**
+ * devlink_shd_get - Get or create a shared devlink instance
+ * @id: Identifier string (e.g., serial number) for the shared instance
+ * @ops: Devlink operations structure
+ * @priv_size: Size of private data structure
+ *
+ * Get an existing shared devlink instance identified by @id, or create
+ * a new one if it doesn't exist. The device is automatically added to
+ * the shared instance's device list. Return the devlink instance
+ * with a reference held. The caller must call devlink_shd_put() when done.
+ *
+ * Return: Pointer to the shared devlink instance on success,
+ * NULL on failure
+ */
+struct devlink *devlink_shd_get(const char *id,
+ const struct devlink_ops *ops,
+ size_t priv_size)
+{
+ struct devlink_shd *shd;
+
+ if (WARN_ON(!id || !ops))
+ return NULL;
+
+ mutex_lock(&shd_mutex);
+
+ shd = devlink_shd_lookup(id);
+ if (!shd)
+ shd = devlink_shd_create(id, ops, priv_size);
+ else
+ refcount_inc(&shd->refcount);
+
+ mutex_unlock(&shd_mutex);
+ return shd ? priv_to_devlink(shd) : NULL;
+}
+EXPORT_SYMBOL_GPL(devlink_shd_get);
+
+/**
+ * devlink_shd_put - Release a reference on a shared devlink instance
+ * @devlink: Shared devlink instance
+ *
+ * Release a reference on a shared devlink instance obtained via
+ * devlink_shd_get().
+ */
+void devlink_shd_put(struct devlink *devlink)
+{
+ struct devlink_shd *shd;
+
+ if (WARN_ON(!devlink))
+ return;
+
+ mutex_lock(&shd_mutex);
+ shd = devlink_priv(devlink);
+ if (refcount_dec_and_test(&shd->refcount))
+ devlink_shd_destroy(shd);
+ mutex_unlock(&shd_mutex);
+}
+EXPORT_SYMBOL_GPL(devlink_shd_put);
+
+/**
+ * devlink_shd_get_priv - Get private data from shared devlink instance
+ * @devlink: Devlink instance
+ *
+ * Returns a pointer to the driver's private data structure within
+ * the shared devlink instance.
+ *
+ * Return: Pointer to private data
+ */
+void *devlink_shd_get_priv(struct devlink *devlink)
+{
+ struct devlink_shd *shd = devlink_priv(devlink);
+
+ return shd->priv;
+}
+EXPORT_SYMBOL_GPL(devlink_shd_get_priv);
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next V7 02/14] devlink: introduce shared devlink instance for PFs on same chip
2026-01-28 11:25 ` [PATCH net-next V7 02/14] devlink: introduce shared devlink instance for PFs on same chip Tariq Toukan
@ 2026-02-03 3:49 ` Jakub Kicinski
2026-02-03 9:44 ` Jiri Pirko
0 siblings, 1 reply; 32+ messages in thread
From: Jakub Kicinski @ 2026-02-03 3:49 UTC (permalink / raw)
To: Tariq Toukan
Cc: Eric Dumazet, Paolo Abeni, Andrew Lunn, David S. Miller,
Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
On Wed, 28 Jan 2026 13:25:32 +0200 Tariq Toukan wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Multiple PFs may reside on the same physical chip, running a single
> firmware. Some of the resources and configurations may be shared among
> these PFs. Currently, there is no good object to pin the configuration
> knobs on.
>
> Introduce a shared devlink instance, instantiated upon probe of the
> first PF and removed during remove of the last PF. The shared devlink
> instance is backed by a faux device, as there is no PCI device related
> to it. The implementation uses reference counting to manage the
> lifecycle: each PF that probes calls devlink_shd_get() to get or create
> the shared instance, and calls devlink_shd_put() when it removes. The
> shared instance is automatically destroyed when the last PF removes.
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index cb839e0435a1..c453faec8ebf 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -1644,6 +1644,12 @@ void devlink_register(struct devlink *devlink);
> void devlink_unregister(struct devlink *devlink);
> void devlink_free(struct devlink *devlink);
>
> +struct devlink *devlink_shd_get(const char *id,
> + const struct devlink_ops *ops,
> + size_t priv_size);
> +void devlink_shd_put(struct devlink *devlink);
> +void *devlink_shd_get_priv(struct devlink *devlink);
Would Cosmin or someone else be willing to take on co-maintainership
of this API (including reviews of other drivers using it)?
We could add a maintainers entry with:
K: devlink_shd_
So y'all get CCed.
> +#include <linux/device/faux.h>
> +#include <net/devlink.h>
> +/* This structure represents a shared devlink instance,
> + * there is one created per identifier (e.g., serial number).
> + */
> +struct devlink_shd {
> + struct list_head list; /* Node in shd list */
> + const char *id; /* Identifier string (e.g., serial number) */
Why does this have to be a string? The identifier should be irrelevant,
and if something like serial number exists it can be reported in dev
info for the shared instance?
> + struct faux_device *faux_dev; /* Related faux device */
> + refcount_t refcount; /* Reference count */
> + char priv[] __aligned(NETDEV_ALIGN); /* Driver private data */
size member annotated with __counted_by() is missing here
> +};
> +static struct devlink_shd *devlink_shd_create(const char *id,
> + const struct devlink_ops *ops,
> + size_t priv_size)
> +{
> + struct faux_device *faux_dev;
> + struct devlink_shd *shd;
> + struct devlink *devlink;
> +
> + /* Create faux device - probe will be called synchronously */
> + faux_dev = faux_device_create(id, NULL, NULL);
> + if (!faux_dev)
> + return NULL;
> +
> + devlink = devlink_alloc(ops, sizeof(struct devlink_shd) + priv_size,
> + &faux_dev->dev);
> + if (!devlink)
> + goto err_devlink_alloc;
error labels should be named after the target not the source in new code
> + shd = devlink_priv(devlink);
> +
> + shd->id = kstrdup(id, GFP_KERNEL);
> + if (!shd->id)
> + goto err_kstrdup_id;
> + shd->faux_dev = faux_dev;
> + refcount_set(&shd->refcount, 1);
> +
> + devl_lock(devlink);
> + devl_register(devlink);
> + devl_unlock(devlink);
> +
> + list_add_tail(&shd->list, &shd_list);
> +
> + return shd;
> +
> +err_kstrdup_id:
> + devlink_free(devlink);
> +
> +err_devlink_alloc:
> + faux_device_destroy(faux_dev);
> + return NULL;
> +}
> +struct devlink *devlink_shd_get(const char *id,
> + const struct devlink_ops *ops,
> + size_t priv_size)
> +{
> + struct devlink_shd *shd;
> +
> + if (WARN_ON(!id || !ops))
> + return NULL;
Seems a little too defensive to check input attrs against NULL.
Let the kernel crash if someone is foolish enough..
> + mutex_lock(&shd_mutex);
> +
> + shd = devlink_shd_lookup(id);
> + if (!shd)
> + shd = devlink_shd_create(id, ops, priv_size);
> + else
> + refcount_inc(&shd->refcount);
> +
> + mutex_unlock(&shd_mutex);
> + return shd ? priv_to_devlink(shd) : NULL;
> +}
> +EXPORT_SYMBOL_GPL(devlink_shd_get);
> +
> +/**
> + * devlink_shd_put - Release a reference on a shared devlink instance
> + * @devlink: Shared devlink instance
> + *
> + * Release a reference on a shared devlink instance obtained via
> + * devlink_shd_get().
> + */
> +void devlink_shd_put(struct devlink *devlink)
> +{
> + struct devlink_shd *shd;
> +
> + if (WARN_ON(!devlink))
> + return;
ditto
> + mutex_lock(&shd_mutex);
> + shd = devlink_priv(devlink);
> + if (refcount_dec_and_test(&shd->refcount))
> + devlink_shd_destroy(shd);
> + mutex_unlock(&shd_mutex);
> +}
> +EXPORT_SYMBOL_GPL(devlink_shd_put);
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next V7 02/14] devlink: introduce shared devlink instance for PFs on same chip
2026-02-03 3:49 ` Jakub Kicinski
@ 2026-02-03 9:44 ` Jiri Pirko
2026-02-04 2:42 ` Jakub Kicinski
0 siblings, 1 reply; 32+ messages in thread
From: Jiri Pirko @ 2026-02-03 9:44 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Tariq Toukan, Eric Dumazet, Paolo Abeni, Andrew Lunn,
David S. Miller, Donald Hunter, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
Tue, Feb 03, 2026 at 04:49:46AM +0100, kuba@kernel.org wrote:
>On Wed, 28 Jan 2026 13:25:32 +0200 Tariq Toukan wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> Multiple PFs may reside on the same physical chip, running a single
>> firmware. Some of the resources and configurations may be shared among
>> these PFs. Currently, there is no good object to pin the configuration
>> knobs on.
>>
>> Introduce a shared devlink instance, instantiated upon probe of the
>> first PF and removed during remove of the last PF. The shared devlink
>> instance is backed by a faux device, as there is no PCI device related
>> to it. The implementation uses reference counting to manage the
>> lifecycle: each PF that probes calls devlink_shd_get() to get or create
>> the shared instance, and calls devlink_shd_put() when it removes. The
>> shared instance is automatically destroyed when the last PF removes.
>
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index cb839e0435a1..c453faec8ebf 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -1644,6 +1644,12 @@ void devlink_register(struct devlink *devlink);
>> void devlink_unregister(struct devlink *devlink);
>> void devlink_free(struct devlink *devlink);
>>
>> +struct devlink *devlink_shd_get(const char *id,
>> + const struct devlink_ops *ops,
>> + size_t priv_size);
>> +void devlink_shd_put(struct devlink *devlink);
>> +void *devlink_shd_get_priv(struct devlink *devlink);
>
>Would Cosmin or someone else be willing to take on co-maintainership
>of this API (including reviews of other drivers using it)?
>We could add a maintainers entry with:
>
>K: devlink_shd_
>
>So y'all get CCed.
>
>> +#include <linux/device/faux.h>
>> +#include <net/devlink.h>
>
>> +/* This structure represents a shared devlink instance,
>> + * there is one created per identifier (e.g., serial number).
>> + */
>> +struct devlink_shd {
>> + struct list_head list; /* Node in shd list */
>> + const char *id; /* Identifier string (e.g., serial number) */
>
>Why does this have to be a string? The identifier should be irrelevant,
>and if something like serial number exists it can be reported in dev
>info for the shared instance?
String gives drivers flexibility to use anything. Perhaps I'm missing
your point. Are you againts free-form or just string and buf+buf_len
would be fine?
>
>> + struct faux_device *faux_dev; /* Related faux device */
>> + refcount_t refcount; /* Reference count */
>> + char priv[] __aligned(NETDEV_ALIGN); /* Driver private data */
>
>size member annotated with __counted_by() is missing here
Will add.
>
>> +};
>
>> +static struct devlink_shd *devlink_shd_create(const char *id,
>> + const struct devlink_ops *ops,
>> + size_t priv_size)
>> +{
>> + struct faux_device *faux_dev;
>> + struct devlink_shd *shd;
>> + struct devlink *devlink;
>> +
>> + /* Create faux device - probe will be called synchronously */
>> + faux_dev = faux_device_create(id, NULL, NULL);
>> + if (!faux_dev)
>> + return NULL;
>> +
>> + devlink = devlink_alloc(ops, sizeof(struct devlink_shd) + priv_size,
>> + &faux_dev->dev);
>> + if (!devlink)
>> + goto err_devlink_alloc;
>
>error labels should be named after the target not the source in new code
Okay. Tried to be consistent with the rest of the code. But as you wish.
>
>> + shd = devlink_priv(devlink);
>> +
>> + shd->id = kstrdup(id, GFP_KERNEL);
>> + if (!shd->id)
>> + goto err_kstrdup_id;
>> + shd->faux_dev = faux_dev;
>> + refcount_set(&shd->refcount, 1);
>> +
>> + devl_lock(devlink);
>> + devl_register(devlink);
>> + devl_unlock(devlink);
>> +
>> + list_add_tail(&shd->list, &shd_list);
>> +
>> + return shd;
>> +
>> +err_kstrdup_id:
>> + devlink_free(devlink);
>> +
>> +err_devlink_alloc:
>> + faux_device_destroy(faux_dev);
>> + return NULL;
>> +}
>
>> +struct devlink *devlink_shd_get(const char *id,
>> + const struct devlink_ops *ops,
>> + size_t priv_size)
>> +{
>> + struct devlink_shd *shd;
>> +
>> + if (WARN_ON(!id || !ops))
>> + return NULL;
>
>Seems a little too defensive to check input attrs against NULL.
>Let the kernel crash if someone is foolish enough..
Okay.
>
>> + mutex_lock(&shd_mutex);
>> +
>> + shd = devlink_shd_lookup(id);
>> + if (!shd)
>> + shd = devlink_shd_create(id, ops, priv_size);
>> + else
>> + refcount_inc(&shd->refcount);
>> +
>> + mutex_unlock(&shd_mutex);
>> + return shd ? priv_to_devlink(shd) : NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_shd_get);
>> +
>> +/**
>> + * devlink_shd_put - Release a reference on a shared devlink instance
>> + * @devlink: Shared devlink instance
>> + *
>> + * Release a reference on a shared devlink instance obtained via
>> + * devlink_shd_get().
>> + */
>> +void devlink_shd_put(struct devlink *devlink)
>> +{
>> + struct devlink_shd *shd;
>> +
>> + if (WARN_ON(!devlink))
>> + return;
>
>ditto
Okay.
>
>> + mutex_lock(&shd_mutex);
>> + shd = devlink_priv(devlink);
>> + if (refcount_dec_and_test(&shd->refcount))
>> + devlink_shd_destroy(shd);
>> + mutex_unlock(&shd_mutex);
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_shd_put);
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next V7 02/14] devlink: introduce shared devlink instance for PFs on same chip
2026-02-03 9:44 ` Jiri Pirko
@ 2026-02-04 2:42 ` Jakub Kicinski
2026-02-04 7:15 ` Jiri Pirko
0 siblings, 1 reply; 32+ messages in thread
From: Jakub Kicinski @ 2026-02-04 2:42 UTC (permalink / raw)
To: Jiri Pirko
Cc: Tariq Toukan, Eric Dumazet, Paolo Abeni, Andrew Lunn,
David S. Miller, Donald Hunter, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
On Tue, 3 Feb 2026 10:44:16 +0100 Jiri Pirko wrote:
> >> +/* This structure represents a shared devlink instance,
> >> + * there is one created per identifier (e.g., serial number).
> >> + */
> >> +struct devlink_shd {
> >> + struct list_head list; /* Node in shd list */
> >> + const char *id; /* Identifier string (e.g., serial number) */
> >
> >Why does this have to be a string? The identifier should be irrelevant,
> >and if something like serial number exists it can be reported in dev
> >info for the shared instance?
>
> String gives drivers flexibility to use anything. Perhaps I'm missing
> your point. Are you againts free-form or just string and buf+buf_len
> would be fine?
I was thinking binary buf+len is fine, and we shouldn't really expose
this to user space in any shape or form (hence no concern about free
form).
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next V7 02/14] devlink: introduce shared devlink instance for PFs on same chip
2026-02-04 2:42 ` Jakub Kicinski
@ 2026-02-04 7:15 ` Jiri Pirko
2026-02-05 2:06 ` Jakub Kicinski
0 siblings, 1 reply; 32+ messages in thread
From: Jiri Pirko @ 2026-02-04 7:15 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Tariq Toukan, Eric Dumazet, Paolo Abeni, Andrew Lunn,
David S. Miller, Donald Hunter, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
Wed, Feb 04, 2026 at 03:42:00AM +0100, kuba@kernel.org wrote:
>On Tue, 3 Feb 2026 10:44:16 +0100 Jiri Pirko wrote:
>> >> +/* This structure represents a shared devlink instance,
>> >> + * there is one created per identifier (e.g., serial number).
>> >> + */
>> >> +struct devlink_shd {
>> >> + struct list_head list; /* Node in shd list */
>> >> + const char *id; /* Identifier string (e.g., serial number) */
>> >
>> >Why does this have to be a string? The identifier should be irrelevant,
>> >and if something like serial number exists it can be reported in dev
>> >info for the shared instance?
>>
>> String gives drivers flexibility to use anything. Perhaps I'm missing
>> your point. Are you againts free-form or just string and buf+buf_len
>> would be fine?
>
>I was thinking binary buf+len is fine, and we shouldn't really expose
>this to user space in any shape or form (hence no concern about free
>form).
How you imagine to name faux device then? I'm sensing that you want to
get rid of busname/devname handle for things like this and rely on some
randomly generated index. But the whole ecosystem is bases on
busname/devname handle. Any idea how to overcome that?
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next V7 02/14] devlink: introduce shared devlink instance for PFs on same chip
2026-02-04 7:15 ` Jiri Pirko
@ 2026-02-05 2:06 ` Jakub Kicinski
0 siblings, 0 replies; 32+ messages in thread
From: Jakub Kicinski @ 2026-02-05 2:06 UTC (permalink / raw)
To: Jiri Pirko
Cc: Tariq Toukan, Eric Dumazet, Paolo Abeni, Andrew Lunn,
David S. Miller, Donald Hunter, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
On Wed, 4 Feb 2026 08:15:21 +0100 Jiri Pirko wrote:
> >> String gives drivers flexibility to use anything. Perhaps I'm missing
> >> your point. Are you againts free-form or just string and buf+buf_len
> >> would be fine?
> >
> >I was thinking binary buf+len is fine, and we shouldn't really expose
> >this to user space in any shape or form (hence no concern about free
> >form).
>
> How you imagine to name faux device then? I'm sensing that you want to
> get rid of busname/devname handle for things like this and rely on some
> randomly generated index. But the whole ecosystem is bases on
> busname/devname handle. Any idea how to overcome that?
Quoting myself form the other sub-thread:
FWIW using devlink day to day, the bus/device is not at all useful as
an identifier. Most of code touching devlink at Meta either matches
on devlink dev info or assumes there's one instance on the system.
IOW I think you over-estimate the value of the bus/dev in real life
systems. And that's for instances that have a real bus/dev. Having
a faux bus/dev is completely pointless, right? The only question is
whether some code will straight up crash when seeing a device without
bus/dev.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next V7 03/14] devlink: Reverse locking order for nested instances
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
2026-01-28 11:25 ` [PATCH net-next V7 01/14] documentation: networking: add shared devlink documentation Tariq Toukan
2026-01-28 11:25 ` [PATCH net-next V7 02/14] devlink: introduce shared devlink instance for PFs on same chip Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-01-28 11:25 ` [PATCH net-next V7 04/14] devlink: Add helpers to lock nested-in instances Tariq Toukan
` (11 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Cosmin Ratiu <cratiu@nvidia.com>
Commit [1] defined the locking expectations for nested devlink
instances: the nested-in devlink instance lock needs to be acquired
before the nested devlink instance lock. The code handling devlink rels
was architected with that assumption in mind.
There are no actual users of double locking yet but that is about to
change in the upcoming patches in the series.
Code operating on nested devlink instances will require also obtaining
the nested-in instance lock, but such code may already be called from a
variety of places with the nested devlink instance lock. Then, there's
no way to acquire the nested-in lock other than making sure that all
callers acquire it first.
Reversing the nested lock order allows incrementally acquiring the
nested-in instance lock when needed (perhaps even a chain of locks up to
the root) without affecting any caller.
The only affected use of nesting is devlink_nl_nested_fill(), which
iterates over nested devlink instances with the RCU lock, without
locking them, so there's no possibility of deadlock.
So this commit just updates a comment regarding the nested locks.
[1] commit c137743bce02b ("devlink: introduce object and nested devlink
relationship infra")
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
net/devlink/core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/devlink/core.c b/net/devlink/core.c
index 58093f49c090..6ae62c7f2a80 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -178,9 +178,7 @@ int devlink_rel_nested_in_add(u32 *rel_index, u32 devlink_index,
* a notification of a change of this object should be sent
* over netlink. The parent devlink instance lock needs to be
* taken during the notification preparation.
- * However, since the devlink lock of nested instance is held here,
- * we would end with wrong devlink instance lock ordering and
- * deadlock. Therefore the work is utilized to avoid that.
+ * Since the parent may or may not be locked, 'work' is utilized.
*/
void devlink_rel_nested_in_notify(struct devlink *devlink)
{
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH net-next V7 04/14] devlink: Add helpers to lock nested-in instances
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
` (2 preceding siblings ...)
2026-01-28 11:25 ` [PATCH net-next V7 03/14] devlink: Reverse locking order for nested instances Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-01-28 11:25 ` [PATCH net-next V7 05/14] devlink: Refactor devlink_rate_nodes_check Tariq Toukan
` (10 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Cosmin Ratiu <cratiu@nvidia.com>
Upcoming code will need to obtain a reference to locked nested-in
devlink instances. Add helpers to lock, obtain an already locked
reference and unlock/unref the nested-in instance.
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
net/devlink/core.c | 42 +++++++++++++++++++++++++++++++++++++
net/devlink/devl_internal.h | 3 +++
2 files changed, 45 insertions(+)
diff --git a/net/devlink/core.c b/net/devlink/core.c
index 6ae62c7f2a80..f228190df346 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -67,6 +67,48 @@ static void __devlink_rel_put(struct devlink_rel *rel)
devlink_rel_free(rel);
}
+struct devlink *devlink_nested_in_get_lock(struct devlink_rel *rel)
+{
+ struct devlink *devlink;
+
+ if (!rel)
+ return NULL;
+ devlink = devlinks_xa_get(rel->nested_in.devlink_index);
+ if (!devlink)
+ return NULL;
+ devl_lock(devlink);
+ if (devl_is_registered(devlink))
+ return devlink;
+ devl_unlock(devlink);
+ devlink_put(devlink);
+ return NULL;
+}
+
+/* Returns the nested in devlink object and validates its lock is held. */
+struct devlink *devlink_nested_in_get_locked(struct devlink_rel *rel)
+{
+ struct devlink *devlink;
+ unsigned long index;
+
+ if (!rel)
+ return NULL;
+ index = rel->nested_in.devlink_index;
+ devlink = xa_find(&devlinks, &index, index, DEVLINK_REGISTERED);
+ if (devlink)
+ devl_assert_locked(devlink);
+ return devlink;
+}
+
+void devlink_nested_in_put_unlock(struct devlink_rel *rel)
+{
+ struct devlink *devlink = devlink_nested_in_get_locked(rel);
+
+ if (devlink) {
+ devl_unlock(devlink);
+ devlink_put(devlink);
+ }
+}
+
static void devlink_rel_nested_in_notify_work(struct work_struct *work)
{
struct devlink_rel *rel = container_of(work, struct devlink_rel,
diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index 14eaad9cfe35..aea43d750d23 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -120,6 +120,9 @@ typedef void devlink_rel_notify_cb_t(struct devlink *devlink, u32 obj_index);
typedef void devlink_rel_cleanup_cb_t(struct devlink *devlink, u32 obj_index,
u32 rel_index);
+struct devlink *devlink_nested_in_get_lock(struct devlink_rel *rel);
+struct devlink *devlink_nested_in_get_locked(struct devlink_rel *rel);
+void devlink_nested_in_put_unlock(struct devlink_rel *rel);
void devlink_rel_nested_in_clear(u32 rel_index);
int devlink_rel_nested_in_add(u32 *rel_index, u32 devlink_index,
u32 obj_index, devlink_rel_notify_cb_t *notify_cb,
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH net-next V7 05/14] devlink: Refactor devlink_rate_nodes_check
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
` (3 preceding siblings ...)
2026-01-28 11:25 ` [PATCH net-next V7 04/14] devlink: Add helpers to lock nested-in instances Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-01-28 11:25 ` [PATCH net-next V7 06/14] devlink: Decouple rate storage from associated devlink object Tariq Toukan
` (9 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Cosmin Ratiu <cratiu@nvidia.com>
devlink_rate_nodes_check() was used to verify there are no devlink rate
nodes created when switching the esw mode.
Rate management code is about to become more complex, so refactor this
function:
- remove unused param 'mode'.
- add a new 'rate_filter' param.
- rename to devlink_rates_check().
- expose devlink_rate_is_node() to be used as a rate filter.
This makes it more usable from multiple places, so use it from those
places as well.
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
net/devlink/core.c | 2 +-
net/devlink/dev.c | 7 ++++---
net/devlink/devl_internal.h | 6 ++++--
net/devlink/rate.c | 13 +++++++------
4 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/net/devlink/core.c b/net/devlink/core.c
index f228190df346..f72d8cc0d6dd 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -517,7 +517,7 @@ void devlink_free(struct devlink *devlink)
WARN_ON(!list_empty(&devlink->resource_list));
WARN_ON(!list_empty(&devlink->dpipe_table_list));
WARN_ON(!list_empty(&devlink->sb_list));
- WARN_ON(!list_empty(&devlink->rate_list));
+ WARN_ON(devlink_rates_check(devlink, NULL, NULL));
WARN_ON(!list_empty(&devlink->linecard_list));
WARN_ON(!xa_empty(&devlink->ports));
diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index 02602704bdea..e3a36de4f4ae 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -434,7 +434,7 @@ static void devlink_reload_reinit_sanity_check(struct devlink *devlink)
WARN_ON(!list_empty(&devlink->trap_list));
WARN_ON(!list_empty(&devlink->dpipe_table_list));
WARN_ON(!list_empty(&devlink->sb_list));
- WARN_ON(!list_empty(&devlink->rate_list));
+ WARN_ON(devlink_rates_check(devlink, NULL, NULL));
WARN_ON(!list_empty(&devlink->linecard_list));
WARN_ON(!xa_empty(&devlink->ports));
}
@@ -713,10 +713,11 @@ int devlink_nl_eswitch_set_doit(struct sk_buff *skb, struct genl_info *info)
if (info->attrs[DEVLINK_ATTR_ESWITCH_MODE]) {
if (!ops->eswitch_mode_set)
return -EOPNOTSUPP;
- mode = nla_get_u16(info->attrs[DEVLINK_ATTR_ESWITCH_MODE]);
- err = devlink_rate_nodes_check(devlink, mode, info->extack);
+ err = devlink_rates_check(devlink, devlink_rate_is_node,
+ info->extack);
if (err)
return err;
+ mode = nla_get_u16(info->attrs[DEVLINK_ATTR_ESWITCH_MODE]);
err = ops->eswitch_mode_set(devlink, mode, info->extack);
if (err)
return err;
diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index aea43d750d23..8374c9cab6ce 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -300,8 +300,10 @@ int devlink_resources_validate(struct devlink *devlink,
struct genl_info *info);
/* Rates */
-int devlink_rate_nodes_check(struct devlink *devlink, u16 mode,
- struct netlink_ext_ack *extack);
+bool devlink_rate_is_node(const struct devlink_rate *devlink_rate);
+int devlink_rates_check(struct devlink *devlink,
+ bool (*rate_filter)(const struct devlink_rate *),
+ struct netlink_ext_ack *extack);
/* Linecards */
unsigned int devlink_linecard_index(struct devlink_linecard *linecard);
diff --git a/net/devlink/rate.c b/net/devlink/rate.c
index d157a8419bca..0d68b5c477dc 100644
--- a/net/devlink/rate.c
+++ b/net/devlink/rate.c
@@ -12,8 +12,7 @@ devlink_rate_is_leaf(struct devlink_rate *devlink_rate)
return devlink_rate->type == DEVLINK_RATE_TYPE_LEAF;
}
-static inline bool
-devlink_rate_is_node(struct devlink_rate *devlink_rate)
+bool devlink_rate_is_node(const struct devlink_rate *devlink_rate)
{
return devlink_rate->type == DEVLINK_RATE_TYPE_NODE;
}
@@ -688,14 +687,16 @@ int devlink_nl_rate_del_doit(struct sk_buff *skb, struct genl_info *info)
return err;
}
-int devlink_rate_nodes_check(struct devlink *devlink, u16 mode,
- struct netlink_ext_ack *extack)
+int devlink_rates_check(struct devlink *devlink,
+ bool (*rate_filter)(const struct devlink_rate *),
+ struct netlink_ext_ack *extack)
{
struct devlink_rate *devlink_rate;
list_for_each_entry(devlink_rate, &devlink->rate_list, list)
- if (devlink_rate_is_node(devlink_rate)) {
- NL_SET_ERR_MSG(extack, "Rate node(s) exists.");
+ if (!rate_filter || rate_filter(devlink_rate)) {
+ if (extack)
+ NL_SET_ERR_MSG(extack, "Rate node(s) exists.");
return -EBUSY;
}
return 0;
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH net-next V7 06/14] devlink: Decouple rate storage from associated devlink object
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
` (4 preceding siblings ...)
2026-01-28 11:25 ` [PATCH net-next V7 05/14] devlink: Refactor devlink_rate_nodes_check Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-01-28 11:25 ` [PATCH net-next V7 07/14] devlink: Add parent dev to devlink API Tariq Toukan
` (8 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Cosmin Ratiu <cratiu@nvidia.com>
Devlink rate leafs and nodes were stored in their respective devlink
objects pointed to by devlink_rate->devlink.
This patch removes that association by introducing the concept of
'rate node devlink', which is where all rates that could link to each
other are stored. For now this is the same as devlink_rate->devlink.
After this patch, the devlink rates stored in this devlink instance
could potentially be from multiple other devlink instances. So all rate
node manipulation code was updated to:
- correctly compare the actual devlink object during iteration.
- maybe acquire additional locks (noop for now).
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
include/net/devlink.h | 2 +
net/devlink/rate.c | 192 +++++++++++++++++++++++++++++++-----------
2 files changed, 143 insertions(+), 51 deletions(-)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index c453faec8ebf..fbb434185a67 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1820,6 +1820,8 @@ void devlink_port_attrs_pci_sf_set(struct devlink_port *devlink_port,
bool external);
int devl_port_fn_devlink_set(struct devlink_port *devlink_port,
struct devlink *fn_devlink);
+struct devlink *devl_rate_lock(struct devlink *devlink);
+void devl_rate_unlock(struct devlink *devlink);
struct devlink_rate *
devl_rate_node_create(struct devlink *devlink, void *priv, char *node_name,
struct devlink_rate *parent);
diff --git a/net/devlink/rate.c b/net/devlink/rate.c
index 0d68b5c477dc..c062fd8a6c36 100644
--- a/net/devlink/rate.c
+++ b/net/devlink/rate.c
@@ -30,13 +30,31 @@ devlink_rate_leaf_get_from_info(struct devlink *devlink, struct genl_info *info)
return devlink_rate ?: ERR_PTR(-ENODEV);
}
+struct devlink *devl_rate_lock(struct devlink *devlink)
+{
+ return devlink;
+}
+
+static struct devlink *
+devl_get_rate_node_instance_locked(struct devlink *devlink)
+{
+ return devlink;
+}
+
+void devl_rate_unlock(struct devlink *devlink)
+{
+}
+
static struct devlink_rate *
devlink_rate_node_get_by_name(struct devlink *devlink, const char *node_name)
{
struct devlink_rate *devlink_rate;
+ struct devlink *rate_devlink;
- list_for_each_entry(devlink_rate, &devlink->rate_list, list) {
- if (devlink_rate_is_node(devlink_rate) &&
+ rate_devlink = devl_get_rate_node_instance_locked(devlink);
+ list_for_each_entry(devlink_rate, &rate_devlink->rate_list, list) {
+ if (devlink_rate->devlink == devlink &&
+ devlink_rate_is_node(devlink_rate) &&
!strcmp(node_name, devlink_rate->name))
return devlink_rate;
}
@@ -190,17 +208,25 @@ static void devlink_rate_notify(struct devlink_rate *devlink_rate,
void devlink_rates_notify_register(struct devlink *devlink)
{
struct devlink_rate *rate_node;
+ struct devlink *rate_devlink;
- list_for_each_entry(rate_node, &devlink->rate_list, list)
- devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW);
+ rate_devlink = devl_rate_lock(devlink);
+ list_for_each_entry(rate_node, &rate_devlink->rate_list, list)
+ if (rate_node->devlink == devlink)
+ devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW);
+ devl_rate_unlock(devlink);
}
void devlink_rates_notify_unregister(struct devlink *devlink)
{
struct devlink_rate *rate_node;
+ struct devlink *rate_devlink;
- list_for_each_entry_reverse(rate_node, &devlink->rate_list, list)
- devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_DEL);
+ rate_devlink = devl_rate_lock(devlink);
+ list_for_each_entry_reverse(rate_node, &rate_devlink->rate_list, list)
+ if (rate_node->devlink == devlink)
+ devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_DEL);
+ devl_rate_unlock(devlink);
}
static int
@@ -209,17 +235,20 @@ devlink_nl_rate_get_dump_one(struct sk_buff *msg, struct devlink *devlink,
{
struct devlink_nl_dump_state *state = devlink_dump_state(cb);
struct devlink_rate *devlink_rate;
+ struct devlink *rate_devlink;
int idx = 0;
int err = 0;
- list_for_each_entry(devlink_rate, &devlink->rate_list, list) {
+ rate_devlink = devl_rate_lock(devlink);
+ list_for_each_entry(devlink_rate, &rate_devlink->rate_list, list) {
enum devlink_command cmd = DEVLINK_CMD_RATE_NEW;
u32 id = NETLINK_CB(cb->skb).portid;
- if (idx < state->idx) {
+ if (idx < state->idx || devlink_rate->devlink != devlink) {
idx++;
continue;
}
+
err = devlink_nl_rate_fill(msg, devlink_rate, cmd, id,
cb->nlh->nlmsg_seq, flags, NULL);
if (err) {
@@ -228,6 +257,7 @@ devlink_nl_rate_get_dump_one(struct sk_buff *msg, struct devlink *devlink,
}
idx++;
}
+ devl_rate_unlock(devlink);
return err;
}
@@ -244,23 +274,33 @@ int devlink_nl_rate_get_doit(struct sk_buff *skb, struct genl_info *info)
struct sk_buff *msg;
int err;
+ devl_rate_lock(devlink);
devlink_rate = devlink_rate_get_from_info(devlink, info);
- if (IS_ERR(devlink_rate))
- return PTR_ERR(devlink_rate);
+ if (IS_ERR(devlink_rate)) {
+ err = PTR_ERR(devlink_rate);
+ goto unlock;
+ }
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
+ if (!msg) {
+ err = -ENOMEM;
+ goto unlock;
+ }
err = devlink_nl_rate_fill(msg, devlink_rate, DEVLINK_CMD_RATE_NEW,
info->snd_portid, info->snd_seq, 0,
info->extack);
- if (err) {
- nlmsg_free(msg);
- return err;
- }
+ if (err)
+ goto err_fill;
+ devl_rate_unlock(devlink);
return genlmsg_reply(msg, info);
+
+err_fill:
+ nlmsg_free(msg);
+unlock:
+ devl_rate_unlock(devlink);
+ return err;
}
static bool
@@ -590,24 +630,32 @@ int devlink_nl_rate_set_doit(struct sk_buff *skb, struct genl_info *info)
const struct devlink_ops *ops;
int err;
+ devl_rate_lock(devlink);
devlink_rate = devlink_rate_get_from_info(devlink, info);
- if (IS_ERR(devlink_rate))
- return PTR_ERR(devlink_rate);
+ if (IS_ERR(devlink_rate)) {
+ err = PTR_ERR(devlink_rate);
+ goto unlock;
+ }
ops = devlink->ops;
- if (!ops || !devlink_rate_set_ops_supported(ops, info, devlink_rate->type))
- return -EOPNOTSUPP;
+ if (!ops ||
+ !devlink_rate_set_ops_supported(ops, info, devlink_rate->type)) {
+ err = -EOPNOTSUPP;
+ goto unlock;
+ }
err = devlink_nl_rate_set(devlink_rate, ops, info);
if (!err)
devlink_rate_notify(devlink_rate, DEVLINK_CMD_RATE_NEW);
+unlock:
+ devl_rate_unlock(devlink);
return err;
}
int devlink_nl_rate_new_doit(struct sk_buff *skb, struct genl_info *info)
{
- struct devlink *devlink = info->user_ptr[0];
+ struct devlink *rate_devlink, *devlink = info->user_ptr[0];
struct devlink_rate *rate_node;
const struct devlink_ops *ops;
int err;
@@ -621,15 +669,21 @@ int devlink_nl_rate_new_doit(struct sk_buff *skb, struct genl_info *info)
if (!devlink_rate_set_ops_supported(ops, info, DEVLINK_RATE_TYPE_NODE))
return -EOPNOTSUPP;
+ rate_devlink = devl_rate_lock(devlink);
rate_node = devlink_rate_node_get_from_attrs(devlink, info->attrs);
- if (!IS_ERR(rate_node))
- return -EEXIST;
- else if (rate_node == ERR_PTR(-EINVAL))
- return -EINVAL;
+ if (!IS_ERR(rate_node)) {
+ err = -EEXIST;
+ goto unlock;
+ } else if (rate_node == ERR_PTR(-EINVAL)) {
+ err = -EINVAL;
+ goto unlock;
+ }
rate_node = kzalloc(sizeof(*rate_node), GFP_KERNEL);
- if (!rate_node)
- return -ENOMEM;
+ if (!rate_node) {
+ err = -ENOMEM;
+ goto unlock;
+ }
rate_node->devlink = devlink;
rate_node->type = DEVLINK_RATE_TYPE_NODE;
@@ -648,8 +702,9 @@ int devlink_nl_rate_new_doit(struct sk_buff *skb, struct genl_info *info)
goto err_rate_set;
refcount_set(&rate_node->refcnt, 1);
- list_add(&rate_node->list, &devlink->rate_list);
+ list_add(&rate_node->list, &rate_devlink->rate_list);
devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW);
+ devl_rate_unlock(devlink);
return 0;
err_rate_set:
@@ -658,6 +713,8 @@ int devlink_nl_rate_new_doit(struct sk_buff *skb, struct genl_info *info)
kfree(rate_node->name);
err_strdup:
kfree(rate_node);
+unlock:
+ devl_rate_unlock(devlink);
return err;
}
@@ -667,13 +724,17 @@ int devlink_nl_rate_del_doit(struct sk_buff *skb, struct genl_info *info)
struct devlink_rate *rate_node;
int err;
+ devl_rate_lock(devlink);
rate_node = devlink_rate_node_get_from_info(devlink, info);
- if (IS_ERR(rate_node))
- return PTR_ERR(rate_node);
+ if (IS_ERR(rate_node)) {
+ err = PTR_ERR(rate_node);
+ goto unlock;
+ }
if (refcount_read(&rate_node->refcnt) > 1) {
NL_SET_ERR_MSG(info->extack, "Node has children. Cannot delete node.");
- return -EBUSY;
+ err = -EBUSY;
+ goto unlock;
}
devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_DEL);
@@ -684,6 +745,8 @@ int devlink_nl_rate_del_doit(struct sk_buff *skb, struct genl_info *info)
list_del(&rate_node->list);
kfree(rate_node->name);
kfree(rate_node);
+unlock:
+ devl_rate_unlock(devlink);
return err;
}
@@ -692,14 +755,20 @@ int devlink_rates_check(struct devlink *devlink,
struct netlink_ext_ack *extack)
{
struct devlink_rate *devlink_rate;
+ struct devlink *rate_devlink;
+ int err = 0;
- list_for_each_entry(devlink_rate, &devlink->rate_list, list)
- if (!rate_filter || rate_filter(devlink_rate)) {
+ rate_devlink = devl_rate_lock(devlink);
+ list_for_each_entry(devlink_rate, &rate_devlink->rate_list, list)
+ if (devlink_rate->devlink == devlink &&
+ (!rate_filter || rate_filter(devlink_rate))) {
if (extack)
NL_SET_ERR_MSG(extack, "Rate node(s) exists.");
- return -EBUSY;
+ err = -EBUSY;
+ break;
}
- return 0;
+ devl_rate_unlock(devlink);
+ return err;
}
/**
@@ -716,14 +785,20 @@ devl_rate_node_create(struct devlink *devlink, void *priv, char *node_name,
struct devlink_rate *parent)
{
struct devlink_rate *rate_node;
+ struct devlink *rate_devlink;
+ rate_devlink = devl_rate_lock(devlink);
rate_node = devlink_rate_node_get_by_name(devlink, node_name);
- if (!IS_ERR(rate_node))
- return ERR_PTR(-EEXIST);
+ if (!IS_ERR(rate_node)) {
+ rate_node = ERR_PTR(-EEXIST);
+ goto unlock;
+ }
rate_node = kzalloc(sizeof(*rate_node), GFP_KERNEL);
- if (!rate_node)
- return ERR_PTR(-ENOMEM);
+ if (!rate_node) {
+ rate_node = ERR_PTR(-ENOMEM);
+ goto unlock;
+ }
if (parent) {
rate_node->parent = parent;
@@ -737,12 +812,15 @@ devl_rate_node_create(struct devlink *devlink, void *priv, char *node_name,
rate_node->name = kstrdup(node_name, GFP_KERNEL);
if (!rate_node->name) {
kfree(rate_node);
- return ERR_PTR(-ENOMEM);
+ rate_node = ERR_PTR(-ENOMEM);
+ goto unlock;
}
refcount_set(&rate_node->refcnt, 1);
- list_add(&rate_node->list, &devlink->rate_list);
+ list_add(&rate_node->list, &rate_devlink->rate_list);
devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW);
+unlock:
+ devl_rate_unlock(devlink);
return rate_node;
}
EXPORT_SYMBOL_GPL(devl_rate_node_create);
@@ -758,10 +836,10 @@ EXPORT_SYMBOL_GPL(devl_rate_node_create);
int devl_rate_leaf_create(struct devlink_port *devlink_port, void *priv,
struct devlink_rate *parent)
{
- struct devlink *devlink = devlink_port->devlink;
+ struct devlink *rate_devlink, *devlink = devlink_port->devlink;
struct devlink_rate *devlink_rate;
- devl_assert_locked(devlink_port->devlink);
+ devl_assert_locked(devlink);
if (WARN_ON(devlink_port->devlink_rate))
return -EBUSY;
@@ -770,6 +848,7 @@ int devl_rate_leaf_create(struct devlink_port *devlink_port, void *priv,
if (!devlink_rate)
return -ENOMEM;
+ rate_devlink = devl_rate_lock(devlink);
if (parent) {
devlink_rate->parent = parent;
refcount_inc(&devlink_rate->parent->refcnt);
@@ -779,9 +858,10 @@ int devl_rate_leaf_create(struct devlink_port *devlink_port, void *priv,
devlink_rate->devlink = devlink;
devlink_rate->devlink_port = devlink_port;
devlink_rate->priv = priv;
- list_add_tail(&devlink_rate->list, &devlink->rate_list);
+ list_add_tail(&devlink_rate->list, &rate_devlink->rate_list);
devlink_port->devlink_rate = devlink_rate;
devlink_rate_notify(devlink_rate, DEVLINK_CMD_RATE_NEW);
+ devl_rate_unlock(devlink);
return 0;
}
@@ -797,16 +877,19 @@ EXPORT_SYMBOL_GPL(devl_rate_leaf_create);
void devl_rate_leaf_destroy(struct devlink_port *devlink_port)
{
struct devlink_rate *devlink_rate = devlink_port->devlink_rate;
+ struct devlink *devlink = devlink_port->devlink;
- devl_assert_locked(devlink_port->devlink);
+ devl_assert_locked(devlink);
if (!devlink_rate)
return;
+ devl_rate_lock(devlink);
devlink_rate_notify(devlink_rate, DEVLINK_CMD_RATE_DEL);
if (devlink_rate->parent)
refcount_dec(&devlink_rate->parent->refcnt);
list_del(&devlink_rate->list);
devlink_port->devlink_rate = NULL;
+ devl_rate_unlock(devlink);
kfree(devlink_rate);
}
EXPORT_SYMBOL_GPL(devl_rate_leaf_destroy);
@@ -815,18 +898,22 @@ EXPORT_SYMBOL_GPL(devl_rate_leaf_destroy);
* devl_rate_nodes_destroy - destroy all devlink rate nodes on device
* @devlink: devlink instance
*
- * Unset parent for all rate objects and destroy all rate nodes
- * on specified device.
+ * Unset parent for all rate objects involving this device and destroy all rate
+ * nodes on it.
*/
void devl_rate_nodes_destroy(struct devlink *devlink)
{
const struct devlink_ops *ops = devlink->ops;
struct devlink_rate *devlink_rate, *tmp;
+ struct devlink *rate_devlink;
devl_assert_locked(devlink);
+ rate_devlink = devl_rate_lock(devlink);
- list_for_each_entry(devlink_rate, &devlink->rate_list, list) {
- if (!devlink_rate->parent)
+ list_for_each_entry(devlink_rate, &rate_devlink->rate_list, list) {
+ if (!devlink_rate->parent ||
+ (devlink_rate->devlink != devlink &&
+ devlink_rate->parent->devlink != devlink))
continue;
if (devlink_rate_is_leaf(devlink_rate))
@@ -839,13 +926,16 @@ void devl_rate_nodes_destroy(struct devlink *devlink)
refcount_dec(&devlink_rate->parent->refcnt);
devlink_rate->parent = NULL;
}
- list_for_each_entry_safe(devlink_rate, tmp, &devlink->rate_list, list) {
- if (devlink_rate_is_node(devlink_rate)) {
+ list_for_each_entry_safe(devlink_rate, tmp, &rate_devlink->rate_list,
+ list) {
+ if (devlink_rate->devlink == devlink &&
+ devlink_rate_is_node(devlink_rate)) {
ops->rate_node_del(devlink_rate, devlink_rate->priv, NULL);
list_del(&devlink_rate->list);
kfree(devlink_rate->name);
kfree(devlink_rate);
}
}
+ devl_rate_unlock(devlink);
}
EXPORT_SYMBOL_GPL(devl_rate_nodes_destroy);
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH net-next V7 07/14] devlink: Add parent dev to devlink API
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
` (5 preceding siblings ...)
2026-01-28 11:25 ` [PATCH net-next V7 06/14] devlink: Decouple rate storage from associated devlink object Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-02-03 4:00 ` Jakub Kicinski
2026-01-28 11:25 ` [PATCH net-next V7 08/14] devlink: Allow parent dev for rate-set and rate-new Tariq Toukan
` (7 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Cosmin Ratiu <cratiu@nvidia.com>
Upcoming changes to the rate commands need the parent devlink specified.
This change adds a nested 'parent-dev' attribute to the API and helpers
to obtain and put a reference to the parent devlink instance in
info->user_ptr[1].
To avoid deadlocks, the parent devlink is unlocked before obtaining the
main devlink instance that is the target of the request.
A reference to the parent is kept until the end of the request to avoid
it suddenly disappearing.
This means that this reference is of limited use without additional
protection.
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
Documentation/netlink/specs/devlink.yaml | 12 ++++++
include/uapi/linux/devlink.h | 2 +
net/devlink/devl_internal.h | 2 +
net/devlink/netlink.c | 51 +++++++++++++++++++++---
4 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 837112da6738..a8fd0a815c0d 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -867,6 +867,10 @@ attribute-sets:
type: flag
doc: Request restoring parameter to its default value.
value: 183
+
+ - name: parent-dev
+ type: nest
+ nested-attributes: dl-parent-dev
-
name: dl-dev-stats
subset-of: devlink
@@ -1289,6 +1293,14 @@ attribute-sets:
Specifies the bandwidth share assigned to the Traffic Class.
The bandwidth for the traffic class is determined
in proportion to the sum of the shares of all configured classes.
+ -
+ name: dl-parent-dev
+ subset-of: devlink
+ attributes:
+ -
+ name: bus-name
+ -
+ name: dev-name
operations:
enum-model: directional
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index e7d6b6d13470..94b8a4437bac 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -642,6 +642,8 @@ enum devlink_attr {
DEVLINK_ATTR_PARAM_VALUE_DEFAULT, /* dynamic */
DEVLINK_ATTR_PARAM_RESET_DEFAULT, /* flag */
+ DEVLINK_ATTR_PARENT_DEV, /* nested */
+
/* 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/devl_internal.h b/net/devlink/devl_internal.h
index 8374c9cab6ce..3ca4cc8517cd 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -162,6 +162,8 @@ typedef int devlink_nl_dump_one_func_t(struct sk_buff *msg,
struct devlink *
devlink_get_from_attrs_lock(struct net *net, struct nlattr **attrs,
bool dev_lock);
+struct devlink *
+devlink_get_parent_from_attrs_lock(struct net *net, struct nlattr **attrs);
int devlink_nl_dumpit(struct sk_buff *msg, struct netlink_callback *cb,
devlink_nl_dump_one_func_t *dump_one);
diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c
index 593605c1b1ef..72897b19c372 100644
--- a/net/devlink/netlink.c
+++ b/net/devlink/netlink.c
@@ -12,6 +12,7 @@
#define DEVLINK_NL_FLAG_NEED_PORT BIT(0)
#define DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT BIT(1)
#define DEVLINK_NL_FLAG_NEED_DEV_LOCK BIT(2)
+#define DEVLINK_NL_FLAG_OPTIONAL_PARENT_DEV BIT(3)
static const struct genl_multicast_group devlink_nl_mcgrps[] = {
[DEVLINK_MCGRP_CONFIG] = { .name = DEVLINK_GENL_MCGRP_CONFIG_NAME },
@@ -206,19 +207,51 @@ devlink_get_from_attrs_lock(struct net *net, struct nlattr **attrs,
return ERR_PTR(-ENODEV);
}
+struct devlink *
+devlink_get_parent_from_attrs_lock(struct net *net, struct nlattr **attrs)
+{
+ struct nlattr *tb[DEVLINK_ATTR_DEV_NAME + 1];
+ int err;
+
+ if (!attrs[DEVLINK_ATTR_PARENT_DEV])
+ return ERR_PTR(-EINVAL);
+
+ err = nla_parse_nested(tb, DEVLINK_ATTR_DEV_NAME,
+ attrs[DEVLINK_ATTR_PARENT_DEV],
+ NULL, NULL);
+ if (err)
+ return ERR_PTR(err);
+
+ return devlink_get_from_attrs_lock(net, tb, false);
+}
+
static int __devlink_nl_pre_doit(struct sk_buff *skb, struct genl_info *info,
u8 flags)
{
+ bool parent_dev = flags & DEVLINK_NL_FLAG_OPTIONAL_PARENT_DEV;
bool dev_lock = flags & DEVLINK_NL_FLAG_NEED_DEV_LOCK;
+ struct devlink *devlink, *parent_devlink = NULL;
+ struct net *net = genl_info_net(info);
+ struct nlattr **attrs = info->attrs;
struct devlink_port *devlink_port;
- struct devlink *devlink;
int err;
- devlink = devlink_get_from_attrs_lock(genl_info_net(info), info->attrs,
- dev_lock);
- if (IS_ERR(devlink))
- return PTR_ERR(devlink);
+ if (parent_dev && attrs[DEVLINK_ATTR_PARENT_DEV]) {
+ parent_devlink = devlink_get_parent_from_attrs_lock(net, attrs);
+ if (IS_ERR(parent_devlink))
+ return PTR_ERR(parent_devlink);
+ info->user_ptr[1] = parent_devlink;
+ /* Drop the parent devlink lock but don't release the reference.
+ * This will keep it alive until the end of the request.
+ */
+ devl_unlock(parent_devlink);
+ }
+ devlink = devlink_get_from_attrs_lock(net, attrs, dev_lock);
+ if (IS_ERR(devlink)) {
+ err = PTR_ERR(devlink);
+ goto parent_put;
+ }
info->user_ptr[0] = devlink;
if (flags & DEVLINK_NL_FLAG_NEED_PORT) {
devlink_port = devlink_port_get_from_info(devlink, info);
@@ -237,6 +270,9 @@ static int __devlink_nl_pre_doit(struct sk_buff *skb, struct genl_info *info,
unlock:
devl_dev_unlock(devlink, dev_lock);
devlink_put(devlink);
+parent_put:
+ if (parent_dev && parent_devlink)
+ devlink_put(parent_devlink);
return err;
}
@@ -274,6 +310,11 @@ static void __devlink_nl_post_doit(struct sk_buff *skb, struct genl_info *info,
devlink = info->user_ptr[0];
devl_dev_unlock(devlink, dev_lock);
devlink_put(devlink);
+ if ((flags & DEVLINK_NL_FLAG_OPTIONAL_PARENT_DEV) &&
+ info->user_ptr[1]) {
+ devlink = info->user_ptr[1];
+ devlink_put(devlink);
+ }
}
void devlink_nl_post_doit(const struct genl_split_ops *ops,
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next V7 07/14] devlink: Add parent dev to devlink API
2026-01-28 11:25 ` [PATCH net-next V7 07/14] devlink: Add parent dev to devlink API Tariq Toukan
@ 2026-02-03 4:00 ` Jakub Kicinski
2026-02-11 16:28 ` Cosmin Ratiu
0 siblings, 1 reply; 32+ messages in thread
From: Jakub Kicinski @ 2026-02-03 4:00 UTC (permalink / raw)
To: Tariq Toukan
Cc: Eric Dumazet, Paolo Abeni, Andrew Lunn, David S. Miller,
Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
On Wed, 28 Jan 2026 13:25:37 +0200 Tariq Toukan wrote:
> static int __devlink_nl_pre_doit(struct sk_buff *skb, struct genl_info *info,
> u8 flags)
> {
> + bool parent_dev = flags & DEVLINK_NL_FLAG_OPTIONAL_PARENT_DEV;
> bool dev_lock = flags & DEVLINK_NL_FLAG_NEED_DEV_LOCK;
> + struct devlink *devlink, *parent_devlink = NULL;
> + struct net *net = genl_info_net(info);
> + struct nlattr **attrs = info->attrs;
> struct devlink_port *devlink_port;
> - struct devlink *devlink;
> int err;
>
> - devlink = devlink_get_from_attrs_lock(genl_info_net(info), info->attrs,
> - dev_lock);
> - if (IS_ERR(devlink))
> - return PTR_ERR(devlink);
> + if (parent_dev && attrs[DEVLINK_ATTR_PARENT_DEV]) {
> + parent_devlink = devlink_get_parent_from_attrs_lock(net, attrs);
> + if (IS_ERR(parent_devlink))
> + return PTR_ERR(parent_devlink);
> + info->user_ptr[1] = parent_devlink;
Let's convert devlink to use proper overlay struct over info->cb ?
The user_ptr array only has two entries so devlink stuffs all the
extra pointers into the second slot. But the cb is much larger - 48B
so we can easily give each of these values a dedicated pointer.
> + /* Drop the parent devlink lock but don't release the reference.
> + * This will keep it alive until the end of the request.
> + */
To be clear -- devlink instances do not behave like netdev instances.
netdev instances prevent unregistration of the netdev.
devlink refs are normal refs, they just keep the memory around.
If memory serves me..
> + devl_unlock(parent_devlink);
> + }
>
> + devlink = devlink_get_from_attrs_lock(net, attrs, dev_lock);
> + if (IS_ERR(devlink)) {
> + err = PTR_ERR(devlink);
> + goto parent_put;
> + }
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH net-next V7 07/14] devlink: Add parent dev to devlink API
2026-02-03 4:00 ` Jakub Kicinski
@ 2026-02-11 16:28 ` Cosmin Ratiu
2026-02-11 16:57 ` Jakub Kicinski
0 siblings, 1 reply; 32+ messages in thread
From: Cosmin Ratiu @ 2026-02-11 16:28 UTC (permalink / raw)
To: Tariq Toukan, kuba@kernel.org
Cc: corbet@lwn.net, andrew+netdev@lunn.ch, donald.hunter@gmail.com,
davem@davemloft.net, leon@kernel.org,
linux-kernel@vger.kernel.org, edumazet@google.com,
linux-rdma@vger.kernel.org, jiri@resnulli.us, pabeni@redhat.com,
Jiri Pirko, linux-doc@vger.kernel.org, Carolina Jubran,
horms@kernel.org, Mark Bloch, rdunlap@infradead.org,
Saeed Mahameed, netdev@vger.kernel.org, Gal Pressman,
Moshe Shemesh, krzk@kernel.org
On Mon, 2026-02-02 at 20:00 -0800, Jakub Kicinski wrote:
> > + info->user_ptr[1] = parent_devlink;
>
> Let's convert devlink to use proper overlay struct over info->cb ?
> The user_ptr array only has two entries so devlink stuffs all the
> extra pointers into the second slot. But the cb is much larger - 48B
> so we can easily give each of these values a dedicated pointer.
I assume you are referring to info->ctx, added by Paolo as part of the
netshaper series. If that's the case, sure, I'll convert devlink to
that as part of the next version of this series.
>
> > + /* Drop the parent devlink lock but don't release
> > the reference.
> > + * This will keep it alive until the end of the
> > request.
> > + */
>
> To be clear -- devlink instances do not behave like netdev instances.
> netdev instances prevent unregistration of the netdev.
> devlink refs are normal refs, they just keep the memory around.
> If memory serves me..
If no reference is held, a concurrent user op could release the parent
devlink instance altogether, and free its memory, that's the reason for
keeping a ref alive for the duration of this request.
Cosmin.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next V7 07/14] devlink: Add parent dev to devlink API
2026-02-11 16:28 ` Cosmin Ratiu
@ 2026-02-11 16:57 ` Jakub Kicinski
0 siblings, 0 replies; 32+ messages in thread
From: Jakub Kicinski @ 2026-02-11 16:57 UTC (permalink / raw)
To: Cosmin Ratiu
Cc: Tariq Toukan, corbet@lwn.net, andrew+netdev@lunn.ch,
donald.hunter@gmail.com, davem@davemloft.net, leon@kernel.org,
linux-kernel@vger.kernel.org, edumazet@google.com,
linux-rdma@vger.kernel.org, jiri@resnulli.us, pabeni@redhat.com,
Jiri Pirko, linux-doc@vger.kernel.org, Carolina Jubran,
horms@kernel.org, Mark Bloch, rdunlap@infradead.org,
Saeed Mahameed, netdev@vger.kernel.org, Gal Pressman,
Moshe Shemesh, krzk@kernel.org
On Wed, 11 Feb 2026 16:28:14 +0000 Cosmin Ratiu wrote:
> > > + /* Drop the parent devlink lock but don't release
> > > the reference.
> > > + * This will keep it alive until the end of the
> > > request.
> > > + */
> >
> > To be clear -- devlink instances do not behave like netdev instances.
> > netdev instances prevent unregistration of the netdev.
> > devlink refs are normal refs, they just keep the memory around.
> > If memory serves me..
>
> If no reference is held, a concurrent user op could release the parent
> devlink instance altogether, and free its memory, that's the reason for
> keeping a ref alive for the duration of this request.
I suppose my comment was more about using the word "alive"
in networking context. I associate "alive" with registered.
See dev_isalive(). Here you're just keeping the object
from being freed.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next V7 08/14] devlink: Allow parent dev for rate-set and rate-new
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
` (6 preceding siblings ...)
2026-01-28 11:25 ` [PATCH net-next V7 07/14] devlink: Add parent dev to devlink API Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-01-28 11:25 ` [PATCH net-next V7 09/14] devlink: Allow rate node parents from other devlinks Tariq Toukan
` (6 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Cosmin Ratiu <cratiu@nvidia.com>
Currently, a devlink rate's parent device is assumed to be the same as
the one where the devlink rate is created.
This patch changes that to allow rate commands to accept an additional
argument that specifies the parent dev. This will allow devlink rate
groups with leafs from other devices.
Example of the new usage with ynl:
Creating a group on pci/0000:08:00.1 with a parent to an already
existing pci/0000:08:00.1/group1:
./tools/net/ynl/pyynl/cli.py --spec \
Documentation/netlink/specs/devlink.yaml --do rate-new --json '{
"bus-name": "pci",
"dev-name": "0000:08:00.1",
"rate-node-name": "group2",
"rate-parent-node-name": "group1",
"parent-dev": {
"bus-name": "pci",
"dev-name": "0000:08:00.1"
}
}'
Setting the parent of leaf node pci/0000:08:00.1/65537 to
pci/0000:08:00.0/group1:
./tools/net/ynl/pyynl/cli.py --spec \
Documentation/netlink/specs/devlink.yaml --do rate-set --json '{
"bus-name": "pci",
"dev-name": "0000:08:00.1",
"port-index": 65537,
"parent-dev": {
"bus-name": "pci",
"dev-name": "0000:08:00.0"
},
"rate-parent-node-name": "group1"
}'
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
Documentation/netlink/specs/devlink.yaml | 10 ++++++----
net/devlink/netlink.c | 18 +++++++++++++++++-
net/devlink/netlink_gen.c | 23 +++++++++++++++--------
net/devlink/netlink_gen.h | 8 ++++++++
4 files changed, 46 insertions(+), 13 deletions(-)
diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index a8fd0a815c0d..c81c467f144f 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -2218,8 +2218,8 @@ operations:
dont-validate: [strict]
flags: [admin-perm]
do:
- pre: devlink-nl-pre-doit
- post: devlink-nl-post-doit
+ pre: devlink-nl-pre-doit-parent-dev-optional
+ post: devlink-nl-post-doit-parent-dev-optional
request:
attributes:
- bus-name
@@ -2231,6 +2231,7 @@ operations:
- rate-tx-weight
- rate-parent-node-name
- rate-tc-bws
+ - parent-dev
-
name: rate-new
@@ -2239,8 +2240,8 @@ operations:
dont-validate: [strict]
flags: [admin-perm]
do:
- pre: devlink-nl-pre-doit
- post: devlink-nl-post-doit
+ pre: devlink-nl-pre-doit-parent-dev-optional
+ post: devlink-nl-post-doit-parent-dev-optional
request:
attributes:
- bus-name
@@ -2252,6 +2253,7 @@ operations:
- rate-tx-weight
- rate-parent-node-name
- rate-tc-bws
+ - parent-dev
-
name: rate-del
diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c
index 72897b19c372..781758b8632c 100644
--- a/net/devlink/netlink.c
+++ b/net/devlink/netlink.c
@@ -218,7 +218,7 @@ devlink_get_parent_from_attrs_lock(struct net *net, struct nlattr **attrs)
err = nla_parse_nested(tb, DEVLINK_ATTR_DEV_NAME,
attrs[DEVLINK_ATTR_PARENT_DEV],
- NULL, NULL);
+ devlink_dl_parent_dev_nl_policy, NULL);
if (err)
return ERR_PTR(err);
@@ -301,6 +301,14 @@ int devlink_nl_pre_doit_port_optional(const struct genl_split_ops *ops,
return __devlink_nl_pre_doit(skb, info, DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT);
}
+int devlink_nl_pre_doit_parent_dev_optional(const struct genl_split_ops *ops,
+ struct sk_buff *skb,
+ struct genl_info *info)
+{
+ return __devlink_nl_pre_doit(skb, info,
+ DEVLINK_NL_FLAG_OPTIONAL_PARENT_DEV);
+}
+
static void __devlink_nl_post_doit(struct sk_buff *skb, struct genl_info *info,
u8 flags)
{
@@ -330,6 +338,14 @@ devlink_nl_post_doit_dev_lock(const struct genl_split_ops *ops,
__devlink_nl_post_doit(skb, info, DEVLINK_NL_FLAG_NEED_DEV_LOCK);
}
+void
+devlink_nl_post_doit_parent_dev_optional(const struct genl_split_ops *ops,
+ struct sk_buff *skb,
+ struct genl_info *info)
+{
+ __devlink_nl_post_doit(skb, info, DEVLINK_NL_FLAG_OPTIONAL_PARENT_DEV);
+}
+
static int devlink_nl_inst_single_dumpit(struct sk_buff *msg,
struct netlink_callback *cb, int flags,
devlink_nl_dump_one_func_t *dump_one,
diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
index f4c61c2b4f22..f82656d6d7c1 100644
--- a/net/devlink/netlink_gen.c
+++ b/net/devlink/netlink_gen.c
@@ -39,6 +39,11 @@ devlink_attr_param_type_validate(const struct nlattr *attr,
}
/* Common nested types */
+const struct nla_policy devlink_dl_parent_dev_nl_policy[DEVLINK_ATTR_DEV_NAME + 1] = {
+ [DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, },
+ [DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, },
+};
+
const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_CAPS + 1] = {
[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] = { .type = NLA_BINARY, },
[DEVLINK_PORT_FN_ATTR_STATE] = NLA_POLICY_MAX(NLA_U8, 1),
@@ -531,7 +536,7 @@ static const struct nla_policy devlink_rate_get_dump_nl_policy[DEVLINK_ATTR_DEV_
};
/* DEVLINK_CMD_RATE_SET - do */
-static const struct nla_policy devlink_rate_set_nl_policy[DEVLINK_ATTR_RATE_TC_BWS + 1] = {
+static const struct nla_policy devlink_rate_set_nl_policy[DEVLINK_ATTR_PARENT_DEV + 1] = {
[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, },
[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, },
[DEVLINK_ATTR_RATE_NODE_NAME] = { .type = NLA_NUL_STRING, },
@@ -541,10 +546,11 @@ static const struct nla_policy devlink_rate_set_nl_policy[DEVLINK_ATTR_RATE_TC_B
[DEVLINK_ATTR_RATE_TX_WEIGHT] = { .type = NLA_U32, },
[DEVLINK_ATTR_RATE_PARENT_NODE_NAME] = { .type = NLA_NUL_STRING, },
[DEVLINK_ATTR_RATE_TC_BWS] = NLA_POLICY_NESTED(devlink_dl_rate_tc_bws_nl_policy),
+ [DEVLINK_ATTR_PARENT_DEV] = NLA_POLICY_NESTED(devlink_dl_parent_dev_nl_policy),
};
/* DEVLINK_CMD_RATE_NEW - do */
-static const struct nla_policy devlink_rate_new_nl_policy[DEVLINK_ATTR_RATE_TC_BWS + 1] = {
+static const struct nla_policy devlink_rate_new_nl_policy[DEVLINK_ATTR_PARENT_DEV + 1] = {
[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, },
[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, },
[DEVLINK_ATTR_RATE_NODE_NAME] = { .type = NLA_NUL_STRING, },
@@ -554,6 +560,7 @@ static const struct nla_policy devlink_rate_new_nl_policy[DEVLINK_ATTR_RATE_TC_B
[DEVLINK_ATTR_RATE_TX_WEIGHT] = { .type = NLA_U32, },
[DEVLINK_ATTR_RATE_PARENT_NODE_NAME] = { .type = NLA_NUL_STRING, },
[DEVLINK_ATTR_RATE_TC_BWS] = NLA_POLICY_NESTED(devlink_dl_rate_tc_bws_nl_policy),
+ [DEVLINK_ATTR_PARENT_DEV] = NLA_POLICY_NESTED(devlink_dl_parent_dev_nl_policy),
};
/* DEVLINK_CMD_RATE_DEL - do */
@@ -1197,21 +1204,21 @@ const struct genl_split_ops devlink_nl_ops[74] = {
{
.cmd = DEVLINK_CMD_RATE_SET,
.validate = GENL_DONT_VALIDATE_STRICT,
- .pre_doit = devlink_nl_pre_doit,
+ .pre_doit = devlink_nl_pre_doit_parent_dev_optional,
.doit = devlink_nl_rate_set_doit,
- .post_doit = devlink_nl_post_doit,
+ .post_doit = devlink_nl_post_doit_parent_dev_optional,
.policy = devlink_rate_set_nl_policy,
- .maxattr = DEVLINK_ATTR_RATE_TC_BWS,
+ .maxattr = DEVLINK_ATTR_PARENT_DEV,
.flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
},
{
.cmd = DEVLINK_CMD_RATE_NEW,
.validate = GENL_DONT_VALIDATE_STRICT,
- .pre_doit = devlink_nl_pre_doit,
+ .pre_doit = devlink_nl_pre_doit_parent_dev_optional,
.doit = devlink_nl_rate_new_doit,
- .post_doit = devlink_nl_post_doit,
+ .post_doit = devlink_nl_post_doit_parent_dev_optional,
.policy = devlink_rate_new_nl_policy,
- .maxattr = DEVLINK_ATTR_RATE_TC_BWS,
+ .maxattr = DEVLINK_ATTR_PARENT_DEV,
.flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
},
{
diff --git a/net/devlink/netlink_gen.h b/net/devlink/netlink_gen.h
index 2817d53a0eba..d4db82a00522 100644
--- a/net/devlink/netlink_gen.h
+++ b/net/devlink/netlink_gen.h
@@ -13,6 +13,7 @@
#include <uapi/linux/devlink.h>
/* Common nested types */
+extern const struct nla_policy devlink_dl_parent_dev_nl_policy[DEVLINK_ATTR_DEV_NAME + 1];
extern const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_CAPS + 1];
extern const struct nla_policy devlink_dl_rate_tc_bws_nl_policy[DEVLINK_RATE_TC_ATTR_BW + 1];
extern const struct nla_policy devlink_dl_selftest_id_nl_policy[DEVLINK_ATTR_SELFTEST_ID_FLASH + 1];
@@ -29,12 +30,19 @@ int devlink_nl_pre_doit_dev_lock(const struct genl_split_ops *ops,
int devlink_nl_pre_doit_port_optional(const struct genl_split_ops *ops,
struct sk_buff *skb,
struct genl_info *info);
+int devlink_nl_pre_doit_parent_dev_optional(const struct genl_split_ops *ops,
+ struct sk_buff *skb,
+ struct genl_info *info);
void
devlink_nl_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
struct genl_info *info);
void
devlink_nl_post_doit_dev_lock(const struct genl_split_ops *ops,
struct sk_buff *skb, struct genl_info *info);
+void
+devlink_nl_post_doit_parent_dev_optional(const struct genl_split_ops *ops,
+ struct sk_buff *skb,
+ struct genl_info *info);
int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info);
int devlink_nl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH net-next V7 09/14] devlink: Allow rate node parents from other devlinks
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
` (7 preceding siblings ...)
2026-01-28 11:25 ` [PATCH net-next V7 08/14] devlink: Allow parent dev for rate-set and rate-new Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-02-03 4:04 ` Jakub Kicinski
2026-01-28 11:25 ` [PATCH net-next V7 10/14] net/mlx5: Add a shared devlink instance for PFs on same chip Tariq Toukan
` (5 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Cosmin Ratiu <cratiu@nvidia.com>
This commit makes use of the building blocks previously added to
implement cross-device rate nodes.
A new 'supported_cross_device_rate_nodes' bool is added to devlink_ops
which lets drivers advertise support for cross-device rate objects.
If enabled and if there is a common shared devlink instance, then:
- all rate objects will be stored in the top-most common nested instance
and
- rate objects can have parents from other devices sharing the same
common instance.
The parent devlink from info->user_ptr[1] is not locked, so none of its
mutable fields can be used. But parent setting only requires comparing
devlink pointer comparisons. Additionally, since the shared devlink is
locked, other rate operations cannot concurrently happen.
The rate lock/unlock functions are now exported, so that drivers
implementing this can protect against concurrent modifications on any
shared device structures.
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../networking/devlink/devlink-port.rst | 2 +
include/net/devlink.h | 5 +
net/devlink/rate.c | 91 +++++++++++++++++--
3 files changed, 90 insertions(+), 8 deletions(-)
diff --git a/Documentation/networking/devlink/devlink-port.rst b/Documentation/networking/devlink/devlink-port.rst
index 5e397798a402..976bc5ca0962 100644
--- a/Documentation/networking/devlink/devlink-port.rst
+++ b/Documentation/networking/devlink/devlink-port.rst
@@ -417,6 +417,8 @@ API allows to configure following rate object's parameters:
Parent node name. Parent node rate limits are considered as additional limits
to all node children limits. ``tx_max`` is an upper limit for children.
``tx_share`` is a total bandwidth distributed among children.
+ If the device supports cross-function scheduling, the parent can be from a
+ different function of the same underlying device.
``tc_bw``
Allow users to set the bandwidth allocation per traffic class on rate
diff --git a/include/net/devlink.h b/include/net/devlink.h
index fbb434185a67..1165dc1ae165 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1585,6 +1585,11 @@ struct devlink_ops {
struct devlink_rate *parent,
void *priv_child, void *priv_parent,
struct netlink_ext_ack *extack);
+ /* Indicates if cross-device rate nodes are supported.
+ * This also requires a shared common ancestor object all devices that
+ * could share rate nodes are nested in.
+ */
+ bool supported_cross_device_rate_nodes;
/**
* selftests_check() - queries if selftest is supported
* @devlink: devlink instance
diff --git a/net/devlink/rate.c b/net/devlink/rate.c
index c062fd8a6c36..3d28a4c5bb5f 100644
--- a/net/devlink/rate.c
+++ b/net/devlink/rate.c
@@ -30,20 +30,56 @@ devlink_rate_leaf_get_from_info(struct devlink *devlink, struct genl_info *info)
return devlink_rate ?: ERR_PTR(-ENODEV);
}
+/* Repeatedly locks the nested-in devlink instances while cross device rate
+ * nodes are supported. Returns the devlink instance where rates should be
+ * stored.
+ */
struct devlink *devl_rate_lock(struct devlink *devlink)
{
- return devlink;
+ struct devlink *rate_devlink = devlink;
+
+ while (rate_devlink->ops &&
+ rate_devlink->ops->supported_cross_device_rate_nodes) {
+ devlink = devlink_nested_in_get_lock(rate_devlink->rel);
+ if (!devlink)
+ break;
+ rate_devlink = devlink;
+ }
+ return rate_devlink;
}
+EXPORT_SYMBOL_GPL(devl_rate_lock);
+/* Variant of the above for when the nested-in devlink instances are already
+ * locked.
+ */
static struct devlink *
devl_get_rate_node_instance_locked(struct devlink *devlink)
{
- return devlink;
+ struct devlink *rate_devlink = devlink;
+
+ while (rate_devlink->ops &&
+ rate_devlink->ops->supported_cross_device_rate_nodes) {
+ devlink = devlink_nested_in_get_locked(rate_devlink->rel);
+ if (!devlink)
+ break;
+ rate_devlink = devlink;
+ }
+ return rate_devlink;
}
+/* Repeatedly unlocks the nested-in devlink instances of 'devlink' while cross
+ * device nodes are supported.
+ */
void devl_rate_unlock(struct devlink *devlink)
{
+ if (!devlink || !devlink->ops ||
+ !devlink->ops->supported_cross_device_rate_nodes)
+ return;
+
+ devl_rate_unlock(devlink_nested_in_get_locked(devlink->rel));
+ devlink_nested_in_put_unlock(devlink->rel);
}
+EXPORT_SYMBOL_GPL(devl_rate_unlock);
static struct devlink_rate *
devlink_rate_node_get_by_name(struct devlink *devlink, const char *node_name)
@@ -120,6 +156,24 @@ static int devlink_rate_put_tc_bws(struct sk_buff *msg, u32 *tc_bw)
return -EMSGSIZE;
}
+static int devlink_nl_rate_parent_fill(struct sk_buff *msg,
+ struct devlink_rate *devlink_rate)
+{
+ struct devlink_rate *parent = devlink_rate->parent;
+ struct devlink *devlink = parent->devlink;
+
+ if (nla_put_string(msg, DEVLINK_ATTR_RATE_PARENT_NODE_NAME,
+ parent->name))
+ return -EMSGSIZE;
+
+ if (devlink != devlink_rate->devlink &&
+ devlink_nl_put_nested_handle(msg, devlink_net(devlink), devlink,
+ DEVLINK_ATTR_PARENT_DEV))
+ return -EMSGSIZE;
+
+ return 0;
+}
+
static int devlink_nl_rate_fill(struct sk_buff *msg,
struct devlink_rate *devlink_rate,
enum devlink_command cmd, u32 portid, u32 seq,
@@ -164,10 +218,9 @@ static int devlink_nl_rate_fill(struct sk_buff *msg,
devlink_rate->tx_weight))
goto nla_put_failure;
- if (devlink_rate->parent)
- if (nla_put_string(msg, DEVLINK_ATTR_RATE_PARENT_NODE_NAME,
- devlink_rate->parent->name))
- goto nla_put_failure;
+ if (devlink_rate->parent &&
+ devlink_nl_rate_parent_fill(msg, devlink_rate))
+ goto nla_put_failure;
if (devlink_rate_put_tc_bws(msg, devlink_rate->tc_bw))
goto nla_put_failure;
@@ -320,13 +373,14 @@ devlink_nl_rate_parent_node_set(struct devlink_rate *devlink_rate,
struct genl_info *info,
struct nlattr *nla_parent)
{
- struct devlink *devlink = devlink_rate->devlink;
+ struct devlink *devlink = devlink_rate->devlink, *parent_devlink;
const char *parent_name = nla_data(nla_parent);
const struct devlink_ops *ops = devlink->ops;
size_t len = strlen(parent_name);
struct devlink_rate *parent;
int err = -EOPNOTSUPP;
+ parent_devlink = info->user_ptr[1] ? : devlink;
parent = devlink_rate->parent;
if (parent && !len) {
@@ -344,7 +398,13 @@ devlink_nl_rate_parent_node_set(struct devlink_rate *devlink_rate,
refcount_dec(&parent->refcnt);
devlink_rate->parent = NULL;
} else if (len) {
- parent = devlink_rate_node_get_by_name(devlink, parent_name);
+ /* parent_devlink (when different than devlink) isn't locked,
+ * but the rate node devlink instance is, so nobody from the
+ * same group of devices sharing rates could change the used
+ * fields or unregister the parent.
+ */
+ parent = devlink_rate_node_get_by_name(parent_devlink,
+ parent_name);
if (IS_ERR(parent))
return -ENODEV;
@@ -644,6 +704,14 @@ int devlink_nl_rate_set_doit(struct sk_buff *skb, struct genl_info *info)
goto unlock;
}
+ if (info->user_ptr[1] && info->user_ptr[1] != devlink &&
+ !ops->supported_cross_device_rate_nodes) {
+ NL_SET_ERR_MSG(info->extack,
+ "Cross-device rate parents aren't supported");
+ err = -EOPNOTSUPP;
+ goto unlock;
+ }
+
err = devlink_nl_rate_set(devlink_rate, ops, info);
if (!err)
@@ -669,6 +737,13 @@ int devlink_nl_rate_new_doit(struct sk_buff *skb, struct genl_info *info)
if (!devlink_rate_set_ops_supported(ops, info, DEVLINK_RATE_TYPE_NODE))
return -EOPNOTSUPP;
+ if (info->user_ptr[1] && info->user_ptr[1] != devlink &&
+ !ops->supported_cross_device_rate_nodes) {
+ NL_SET_ERR_MSG(info->extack,
+ "Cross-device rate parents aren't supported");
+ return -EOPNOTSUPP;
+ }
+
rate_devlink = devl_rate_lock(devlink);
rate_node = devlink_rate_node_get_from_attrs(devlink, info->attrs);
if (!IS_ERR(rate_node)) {
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next V7 09/14] devlink: Allow rate node parents from other devlinks
2026-01-28 11:25 ` [PATCH net-next V7 09/14] devlink: Allow rate node parents from other devlinks Tariq Toukan
@ 2026-02-03 4:04 ` Jakub Kicinski
0 siblings, 0 replies; 32+ messages in thread
From: Jakub Kicinski @ 2026-02-03 4:04 UTC (permalink / raw)
To: Tariq Toukan
Cc: Eric Dumazet, Paolo Abeni, Andrew Lunn, David S. Miller,
Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
On Wed, 28 Jan 2026 13:25:39 +0200 Tariq Toukan wrote:
> +EXPORT_SYMBOL_GPL(devl_rate_lock);
Why is this exported? There is no caller outside of core..
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next V7 10/14] net/mlx5: Add a shared devlink instance for PFs on same chip
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
` (8 preceding siblings ...)
2026-01-28 11:25 ` [PATCH net-next V7 09/14] devlink: Allow rate node parents from other devlinks Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-01-28 11:25 ` [PATCH net-next V7 11/14] net/mlx5: Expose a function to clear a vport's parent Tariq Toukan
` (4 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Jiri Pirko <jiri@nvidia.com>
Use the previously introduced shared devlink infrastructure to create
a shared devlink instance for mlx5 PFs that reside on the same physical
chip. The shared instance is identified by the chip's serial number
extracted from PCI VPD (V3 keyword, with fallback to serial number
for older devices).
Each PF that probes calls mlx5_shd_init() which extracts the chip serial
number and uses devlink_shd_get() to get or create the shared instance.
When a PF removes, mlx5_shd_uninit() calls devlink_shd_put() to release
the reference. The shared instance is automatically destroyed when the
last PF removes.
Make the PF devlink instances nested in this shared devlink instance,
allowing userspace to identify which PFs belong to the same physical
chip.
Example:
$ devlink dev
pci/0000:08:00.0:
nested_devlink:
auxiliary/mlx5_core.eth.0
faux/mlx5_core_83013c12b77faa1a30000c82a1045c91:
nested_devlink:
pci/0000:08:00.0
pci/0000:08:00.1
auxiliary/mlx5_core.eth.0
pci/0000:08:00.1:
nested_devlink:
auxiliary/mlx5_core.eth.1
auxiliary/mlx5_core.eth.1
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/Makefile | 5 +-
.../net/ethernet/mellanox/mlx5/core/main.c | 17 +++++
.../ethernet/mellanox/mlx5/core/sh_devlink.c | 71 +++++++++++++++++++
.../ethernet/mellanox/mlx5/core/sh_devlink.h | 12 ++++
include/linux/mlx5/driver.h | 1 +
5 files changed, 104 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.h
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
index 8ffa286a18f5..d39fe9c4a87c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
@@ -16,8 +16,9 @@ mlx5_core-y := main.o cmd.o debugfs.o fw.o eq.o uar.o pagealloc.o \
transobj.o vport.o sriov.o fs_cmd.o fs_core.o pci_irq.o \
fs_counters.o fs_ft_pool.o rl.o lag/debugfs.o lag/lag.o dev.o events.o wq.o lib/gid.o \
lib/devcom.o lib/pci_vsc.o lib/dm.o lib/fs_ttc.o diag/fs_tracepoint.o \
- diag/fw_tracer.o diag/crdump.o devlink.o diag/rsc_dump.o diag/reporter_vnic.o \
- fw_reset.o qos.o lib/tout.o lib/aso.o wc.o fs_pool.o lib/nv_param.o
+ diag/fw_tracer.o diag/crdump.o devlink.o sh_devlink.o diag/rsc_dump.o \
+ diag/reporter_vnic.o fw_reset.o qos.o lib/tout.o lib/aso.o wc.o fs_pool.o \
+ lib/nv_param.o
#
# Netdev basic
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 4209da722f9a..9cd8361ca00e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -74,6 +74,7 @@
#include "mlx5_irq.h"
#include "hwmon.h"
#include "lag/lag.h"
+#include "sh_devlink.h"
MODULE_AUTHOR("Eli Cohen <eli@mellanox.com>");
MODULE_DESCRIPTION("Mellanox 5th generation network adapters (ConnectX series) core driver");
@@ -1520,10 +1521,16 @@ int mlx5_init_one(struct mlx5_core_dev *dev)
int err;
devl_lock(devlink);
+ if (dev->shd) {
+ err = devl_nested_devlink_set(dev->shd, devlink);
+ if (err)
+ goto unlock;
+ }
devl_register(devlink);
err = mlx5_init_one_devl_locked(dev);
if (err)
devl_unregister(devlink);
+unlock:
devl_unlock(devlink);
return err;
}
@@ -2015,6 +2022,13 @@ static int probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
goto pci_init_err;
}
+ err = mlx5_shd_init(dev);
+ if (err) {
+ mlx5_core_err(dev, "mlx5_shd_init failed with error code %d\n",
+ err);
+ goto shd_init_err;
+ }
+
err = mlx5_init_one(dev);
if (err) {
mlx5_core_err(dev, "mlx5_init_one failed with error code %d\n",
@@ -2026,6 +2040,8 @@ static int probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
return 0;
err_init_one:
+ mlx5_shd_uninit(dev);
+shd_init_err:
mlx5_pci_close(dev);
pci_init_err:
mlx5_mdev_uninit(dev);
@@ -2047,6 +2063,7 @@ static void remove_one(struct pci_dev *pdev)
mlx5_drain_health_wq(dev);
mlx5_sriov_disable(pdev, false);
mlx5_uninit_one(dev);
+ mlx5_shd_uninit(dev);
mlx5_pci_close(dev);
mlx5_mdev_uninit(dev);
mlx5_adev_idx_free(dev->priv.adev_idx);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c
new file mode 100644
index 000000000000..202f4ae99fa9
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+/* Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. */
+
+#include <linux/mlx5/driver.h>
+#include <net/devlink.h>
+
+#include "sh_devlink.h"
+
+static const struct devlink_ops mlx5_shd_ops = {
+};
+
+int mlx5_shd_init(struct mlx5_core_dev *dev)
+{
+ struct pci_dev *pdev = dev->pdev;
+ unsigned int vpd_size, kw_len;
+ struct devlink *devlink;
+ const char *sn;
+ u8 *vpd_data;
+ int err = 0;
+ char *end;
+ int start;
+
+ if (!mlx5_core_is_pf(dev))
+ return 0;
+
+ vpd_data = pci_vpd_alloc(pdev, &vpd_size);
+ if (IS_ERR(vpd_data)) {
+ err = PTR_ERR(vpd_data);
+ return err == -ENODEV ? 0 : err;
+ }
+ start = pci_vpd_find_ro_info_keyword(vpd_data, vpd_size, "V3", &kw_len);
+ if (start < 0) {
+ /* Fall-back to SN for older devices. */
+ start = pci_vpd_find_ro_info_keyword(vpd_data, vpd_size,
+ PCI_VPD_RO_KEYWORD_SERIALNO,
+ &kw_len);
+ if (start < 0) {
+ err = -ENOENT;
+ goto out;
+ }
+ }
+ sn = kstrndup(vpd_data + start, kw_len, GFP_KERNEL);
+ if (!sn) {
+ err = -ENOMEM;
+ goto out;
+ }
+ /* Firmware may return spaces at the end of the string, strip it. */
+ end = strchrnul(sn, ' ');
+ *end = '\0';
+
+ /* Get or create shared devlink instance */
+ devlink = devlink_shd_get(sn, &mlx5_shd_ops, 0);
+ kfree(sn);
+ if (!devlink) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ dev->shd = devlink;
+out:
+ kfree(vpd_data);
+ return err;
+}
+
+void mlx5_shd_uninit(struct mlx5_core_dev *dev)
+{
+ if (!dev->shd)
+ return;
+
+ devlink_shd_put(dev->shd);
+}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.h b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.h
new file mode 100644
index 000000000000..8ab8d6940227
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
+/* Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. */
+
+#ifndef __MLX5_SH_DEVLINK_H__
+#define __MLX5_SH_DEVLINK_H__
+
+#include <linux/mlx5/driver.h>
+
+int mlx5_shd_init(struct mlx5_core_dev *dev);
+void mlx5_shd_uninit(struct mlx5_core_dev *dev);
+
+#endif /* __MLX5_SH_DEVLINK_H__ */
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index e2d067b1e67b..3657cedc89b1 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -798,6 +798,7 @@ struct mlx5_core_dev {
enum mlx5_wc_state wc_state;
/* sync write combining state */
struct mutex wc_state_lock;
+ struct devlink *shd;
};
struct mlx5_db {
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH net-next V7 11/14] net/mlx5: Expose a function to clear a vport's parent
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
` (9 preceding siblings ...)
2026-01-28 11:25 ` [PATCH net-next V7 10/14] net/mlx5: Add a shared devlink instance for PFs on same chip Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-01-28 11:25 ` [PATCH net-next V7 12/14] net/mlx5: Store QoS sched nodes in the sh_devlink Tariq Toukan
` (3 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Cosmin Ratiu <cratiu@nvidia.com>
Currently, clearing a vport's parent happens with a call that looks like
this:
mlx5_esw_qos_vport_update_parent(vport, NULL, NULL);
Change that to something nicer that looks like this:
mlx5_esw_qos_vport_clear_parent(vport);
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../ethernet/mellanox/mlx5/core/esw/devlink_port.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c | 11 +++++++++--
drivers/net/ethernet/mellanox/mlx5/core/eswitch.h | 3 +--
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
index 89a58dee50b3..31704ea9cdb4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
@@ -202,7 +202,7 @@ void mlx5_esw_offloads_devlink_port_unregister(struct mlx5_vport *vport)
return;
dl_port = vport->dl_port;
- mlx5_esw_qos_vport_update_parent(vport, NULL, NULL);
+ mlx5_esw_qos_vport_clear_parent(vport);
devl_rate_leaf_destroy(&dl_port->dl_port);
devl_port_unregister(&dl_port->dl_port);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
index 4278bcb04c72..8c3a026b8db4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
@@ -1896,8 +1896,10 @@ int mlx5_esw_devlink_rate_node_del(struct devlink_rate *rate_node, void *priv,
return 0;
}
-int mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport, struct mlx5_esw_sched_node *parent,
- struct netlink_ext_ack *extack)
+static int
+mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport,
+ struct mlx5_esw_sched_node *parent,
+ struct netlink_ext_ack *extack)
{
struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
int err = 0;
@@ -1922,6 +1924,11 @@ int mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport, struct mlx5_esw_s
return err;
}
+void mlx5_esw_qos_vport_clear_parent(struct mlx5_vport *vport)
+{
+ mlx5_esw_qos_vport_update_parent(vport, NULL, NULL);
+}
+
int mlx5_esw_devlink_rate_leaf_parent_set(struct devlink_rate *devlink_rate,
struct devlink_rate *parent,
void *priv, void *parent_priv,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index ad1073f7b79f..20cf9dd542a1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -452,8 +452,7 @@ int mlx5_eswitch_set_vport_trust(struct mlx5_eswitch *esw,
u16 vport_num, bool setting);
int mlx5_eswitch_set_vport_rate(struct mlx5_eswitch *esw, u16 vport,
u32 max_rate, u32 min_rate);
-int mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport, struct mlx5_esw_sched_node *node,
- struct netlink_ext_ack *extack);
+void mlx5_esw_qos_vport_clear_parent(struct mlx5_vport *vport);
int mlx5_eswitch_set_vepa(struct mlx5_eswitch *esw, u8 setting);
int mlx5_eswitch_get_vepa(struct mlx5_eswitch *esw, u8 *setting);
int mlx5_eswitch_get_vport_config(struct mlx5_eswitch *esw,
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH net-next V7 12/14] net/mlx5: Store QoS sched nodes in the sh_devlink
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
` (10 preceding siblings ...)
2026-01-28 11:25 ` [PATCH net-next V7 11/14] net/mlx5: Expose a function to clear a vport's parent Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-01-28 11:25 ` [PATCH net-next V7 13/14] net/mlx5: qos: Support cross-device tx scheduling Tariq Toukan
` (2 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Cosmin Ratiu <cratiu@nvidia.com>
In order to support cross-esw scheduling, the nodes for all devices need
to be accessible from the same place so that normalization can work.
Store them in the shared devlink instance and provide an accessor for
them.
Protecting against concurrent QoS modifications is now done with
the shd lock. Enable the supported_cross_device_rate_nodes devink ops
attribute so that all calls originating from devlink rate acquire the
shd lock. Only the additional entry points into QoS need to acquire the
shd lock.
Enabling supported_cross_device_rate_nodes now is safe, because
mlx5_esw_qos_vport_update_parent will reject cross-esw parent updates.
This will change in the next patch.
As part of this change, the E-Switch qos domain concept was removed.
E-Switch QoS domains were added with the intention of eventually
implementing shared qos domains to support cross-esw scheduling in the
previous approach ([1]), but they are no longer necessary in the new
approach.
[1] https://lore.kernel.org/netdev/20250213180134.323929-1-tariqt@nvidia.com/
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/devlink.c | 1 +
.../net/ethernet/mellanox/mlx5/core/esw/qos.c | 201 +++++-------------
.../net/ethernet/mellanox/mlx5/core/esw/qos.h | 3 -
.../net/ethernet/mellanox/mlx5/core/eswitch.c | 9 +-
.../net/ethernet/mellanox/mlx5/core/eswitch.h | 11 +-
.../ethernet/mellanox/mlx5/core/sh_devlink.c | 30 ++-
.../ethernet/mellanox/mlx5/core/sh_devlink.h | 2 +
7 files changed, 88 insertions(+), 169 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index ea77fbd98396..a040c81b10ce 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -385,6 +385,7 @@ static const struct devlink_ops mlx5_devlink_ops = {
.rate_node_del = mlx5_esw_devlink_rate_node_del,
.rate_leaf_parent_set = mlx5_esw_devlink_rate_leaf_parent_set,
.rate_node_parent_set = mlx5_esw_devlink_rate_node_parent_set,
+ .supported_cross_device_rate_nodes = true,
#endif
#ifdef CONFIG_MLX5_SF_MANAGER
.port_new = mlx5_devlink_sf_port_new,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
index 8c3a026b8db4..0d187399d846 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
@@ -5,60 +5,16 @@
#include "lib/mlx5.h"
#include "esw/qos.h"
#include "en/port.h"
+#include "sh_devlink.h"
#define CREATE_TRACE_POINTS
#include "diag/qos_tracepoint.h"
/* Minimum supported BW share value by the HW is 1 Mbit/sec */
#define MLX5_MIN_BW_SHARE 1
-/* Holds rate nodes associated with an E-Switch. */
-struct mlx5_qos_domain {
- /* Serializes access to all qos changes in the qos domain. */
- struct mutex lock;
- /* List of all mlx5_esw_sched_nodes. */
- struct list_head nodes;
-};
-
-static void esw_qos_lock(struct mlx5_eswitch *esw)
-{
- mutex_lock(&esw->qos.domain->lock);
-}
-
-static void esw_qos_unlock(struct mlx5_eswitch *esw)
-{
- mutex_unlock(&esw->qos.domain->lock);
-}
-
static void esw_assert_qos_lock_held(struct mlx5_eswitch *esw)
{
- lockdep_assert_held(&esw->qos.domain->lock);
-}
-
-static struct mlx5_qos_domain *esw_qos_domain_alloc(void)
-{
- struct mlx5_qos_domain *qos_domain;
-
- qos_domain = kzalloc(sizeof(*qos_domain), GFP_KERNEL);
- if (!qos_domain)
- return NULL;
-
- mutex_init(&qos_domain->lock);
- INIT_LIST_HEAD(&qos_domain->nodes);
-
- return qos_domain;
-}
-
-static int esw_qos_domain_init(struct mlx5_eswitch *esw)
-{
- esw->qos.domain = esw_qos_domain_alloc();
-
- return esw->qos.domain ? 0 : -ENOMEM;
-}
-
-static void esw_qos_domain_release(struct mlx5_eswitch *esw)
-{
- kfree(esw->qos.domain);
- esw->qos.domain = NULL;
+ devl_assert_locked(esw->dev->shd);
}
enum sched_node_type {
@@ -111,7 +67,8 @@ static void esw_qos_node_attach_to_parent(struct mlx5_esw_sched_node *node)
if (!node->parent) {
/* Root children are assigned a depth level of 2. */
node->level = 2;
- list_add_tail(&node->entry, &node->esw->qos.domain->nodes);
+ list_add_tail(&node->entry,
+ mlx5_shd_get_qos_nodes(node->esw->dev));
} else {
node->level = node->parent->level + 1;
list_add_tail(&node->entry, &node->parent->children);
@@ -324,14 +281,15 @@ static int esw_qos_create_rate_limit_element(struct mlx5_esw_sched_node *node,
static u32 esw_qos_calculate_min_rate_divider(struct mlx5_eswitch *esw,
struct mlx5_esw_sched_node *parent)
{
- struct list_head *nodes = parent ? &parent->children : &esw->qos.domain->nodes;
u32 fw_max_bw_share = MLX5_CAP_QOS(esw->dev, max_tsar_bw_share);
struct mlx5_esw_sched_node *node;
+ struct list_head *nodes;
u32 max_guarantee = 0;
/* Find max min_rate across all nodes.
* This will correspond to fw_max_bw_share in the final bw_share calculation.
*/
+ nodes = parent ? &parent->children : mlx5_shd_get_qos_nodes(esw->dev);
list_for_each_entry(node, nodes, entry) {
if (node->esw == esw && node->ix != esw->qos.root_tsar_ix &&
node->min_rate > max_guarantee)
@@ -372,10 +330,11 @@ static void esw_qos_normalize_min_rate(struct mlx5_eswitch *esw,
struct mlx5_esw_sched_node *parent,
struct netlink_ext_ack *extack)
{
- struct list_head *nodes = parent ? &parent->children : &esw->qos.domain->nodes;
u32 divider = esw_qos_calculate_min_rate_divider(esw, parent);
struct mlx5_esw_sched_node *node;
+ struct list_head *nodes;
+ nodes = parent ? &parent->children : mlx5_shd_get_qos_nodes(esw->dev);
list_for_each_entry(node, nodes, entry) {
if (node->esw != esw || node->ix == esw->qos.root_tsar_ix)
continue;
@@ -715,7 +674,7 @@ __esw_qos_create_vports_sched_node(struct mlx5_eswitch *esw, struct mlx5_esw_sch
goto err_alloc_node;
}
- list_add_tail(&node->entry, &esw->qos.domain->nodes);
+ list_add_tail(&node->entry, mlx5_shd_get_qos_nodes(esw->dev));
esw_qos_normalize_min_rate(esw, NULL, extack);
trace_mlx5_esw_node_qos_create(esw->dev, node, node->ix);
@@ -1108,7 +1067,8 @@ static int mlx5_esw_qos_vport_enable(struct mlx5_vport *vport, enum sched_node_t
return -ENOMEM;
}
if (!parent)
- list_add_tail(&sched_node->entry, &esw->qos.domain->nodes);
+ list_add_tail(&sched_node->entry,
+ mlx5_shd_get_qos_nodes(esw->dev));
sched_node->max_rate = max_rate;
sched_node->min_rate = min_rate;
@@ -1143,7 +1103,7 @@ void mlx5_esw_qos_vport_disable(struct mlx5_vport *vport)
struct mlx5_esw_sched_node *parent;
lockdep_assert_held(&esw->state_lock);
- esw_qos_lock(esw);
+ devl_lock(esw->dev->shd);
if (!vport->qos.sched_node)
goto unlock;
@@ -1152,7 +1112,7 @@ void mlx5_esw_qos_vport_disable(struct mlx5_vport *vport)
mlx5_esw_qos_vport_disable_locked(vport);
unlock:
- esw_qos_unlock(esw);
+ devl_unlock(esw->dev->shd);
}
static int mlx5_esw_qos_set_vport_max_rate(struct mlx5_vport *vport, u32 max_rate,
@@ -1191,26 +1151,25 @@ int mlx5_esw_qos_set_vport_rate(struct mlx5_vport *vport, u32 max_rate, u32 min_
struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
int err;
- esw_qos_lock(esw);
+ devl_lock(esw->dev->shd);
err = mlx5_esw_qos_set_vport_min_rate(vport, min_rate, NULL);
if (!err)
err = mlx5_esw_qos_set_vport_max_rate(vport, max_rate, NULL);
- esw_qos_unlock(esw);
+ devl_unlock(esw->dev->shd);
return err;
}
bool mlx5_esw_qos_get_vport_rate(struct mlx5_vport *vport, u32 *max_rate, u32 *min_rate)
{
- struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
bool enabled;
- esw_qos_lock(esw);
+ devl_lock(vport->dev->shd);
enabled = !!vport->qos.sched_node;
if (enabled) {
*max_rate = vport->qos.sched_node->max_rate;
*min_rate = vport->qos.sched_node->min_rate;
}
- esw_qos_unlock(esw);
+ devl_unlock(vport->dev->shd);
return enabled;
}
@@ -1576,9 +1535,9 @@ int mlx5_esw_qos_modify_vport_rate(struct mlx5_eswitch *esw, u16 vport_num, u32
return err;
}
- esw_qos_lock(esw);
+ devl_lock(esw->dev->shd);
err = mlx5_esw_qos_set_vport_max_rate(vport, rate_mbps, NULL);
- esw_qos_unlock(esw);
+ devl_unlock(esw->dev->shd);
return err;
}
@@ -1667,44 +1626,24 @@ static void esw_vport_qos_prune_empty(struct mlx5_vport *vport)
mlx5_esw_qos_vport_disable_locked(vport);
}
-int mlx5_esw_qos_init(struct mlx5_eswitch *esw)
-{
- if (esw->qos.domain)
- return 0; /* Nothing to change. */
-
- return esw_qos_domain_init(esw);
-}
-
-void mlx5_esw_qos_cleanup(struct mlx5_eswitch *esw)
-{
- if (esw->qos.domain)
- esw_qos_domain_release(esw);
-}
-
/* Eswitch devlink rate API */
int mlx5_esw_devlink_rate_leaf_tx_share_set(struct devlink_rate *rate_leaf, void *priv,
u64 tx_share, struct netlink_ext_ack *extack)
{
struct mlx5_vport *vport = priv;
- struct mlx5_eswitch *esw;
int err;
- esw = vport->dev->priv.eswitch;
- if (!mlx5_esw_allowed(esw))
+ if (!mlx5_esw_allowed(vport->dev->priv.eswitch))
return -EPERM;
err = esw_qos_devlink_rate_to_mbps(vport->dev, "tx_share", &tx_share, extack);
if (err)
return err;
- esw_qos_lock(esw);
err = mlx5_esw_qos_set_vport_min_rate(vport, tx_share, extack);
- if (err)
- goto out;
- esw_vport_qos_prune_empty(vport);
-out:
- esw_qos_unlock(esw);
+ if (!err)
+ esw_vport_qos_prune_empty(vport);
return err;
}
@@ -1712,24 +1651,18 @@ int mlx5_esw_devlink_rate_leaf_tx_max_set(struct devlink_rate *rate_leaf, void *
u64 tx_max, struct netlink_ext_ack *extack)
{
struct mlx5_vport *vport = priv;
- struct mlx5_eswitch *esw;
int err;
- esw = vport->dev->priv.eswitch;
- if (!mlx5_esw_allowed(esw))
+ if (!mlx5_esw_allowed(vport->dev->priv.eswitch))
return -EPERM;
err = esw_qos_devlink_rate_to_mbps(vport->dev, "tx_max", &tx_max, extack);
if (err)
return err;
- esw_qos_lock(esw);
err = mlx5_esw_qos_set_vport_max_rate(vport, tx_max, extack);
- if (err)
- goto out;
- esw_vport_qos_prune_empty(vport);
-out:
- esw_qos_unlock(esw);
+ if (!err)
+ esw_vport_qos_prune_empty(vport);
return err;
}
@@ -1740,34 +1673,30 @@ int mlx5_esw_devlink_rate_leaf_tc_bw_set(struct devlink_rate *rate_leaf,
{
struct mlx5_esw_sched_node *vport_node;
struct mlx5_vport *vport = priv;
- struct mlx5_eswitch *esw;
bool disable;
int err = 0;
- esw = vport->dev->priv.eswitch;
- if (!mlx5_esw_allowed(esw))
+ if (!mlx5_esw_allowed(vport->dev->priv.eswitch))
return -EPERM;
disable = esw_qos_tc_bw_disabled(tc_bw);
- esw_qos_lock(esw);
if (!esw_qos_vport_validate_unsupported_tc_bw(vport, tc_bw)) {
NL_SET_ERR_MSG_MOD(extack,
"E-Switch traffic classes number is not supported");
- err = -EOPNOTSUPP;
- goto unlock;
+ return -EOPNOTSUPP;
}
vport_node = vport->qos.sched_node;
if (disable && !vport_node)
- goto unlock;
+ return 0;
if (disable) {
if (vport_node->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR)
err = esw_qos_vport_update(vport, SCHED_NODE_TYPE_VPORT,
vport_node->parent, extack);
esw_vport_qos_prune_empty(vport);
- goto unlock;
+ return err;
}
if (!vport_node) {
@@ -1782,8 +1711,6 @@ int mlx5_esw_devlink_rate_leaf_tc_bw_set(struct devlink_rate *rate_leaf,
}
if (!err)
esw_qos_set_tc_arbiter_bw_shares(vport_node, tc_bw, extack);
-unlock:
- esw_qos_unlock(esw);
return err;
}
@@ -1793,28 +1720,22 @@ int mlx5_esw_devlink_rate_node_tc_bw_set(struct devlink_rate *rate_node,
struct netlink_ext_ack *extack)
{
struct mlx5_esw_sched_node *node = priv;
- struct mlx5_eswitch *esw = node->esw;
bool disable;
int err;
- if (!esw_qos_validate_unsupported_tc_bw(esw, tc_bw)) {
+ if (!esw_qos_validate_unsupported_tc_bw(node->esw, tc_bw)) {
NL_SET_ERR_MSG_MOD(extack,
"E-Switch traffic classes number is not supported");
return -EOPNOTSUPP;
}
disable = esw_qos_tc_bw_disabled(tc_bw);
- esw_qos_lock(esw);
- if (disable) {
- err = esw_qos_node_disable_tc_arbitration(node, extack);
- goto unlock;
- }
+ if (disable)
+ return esw_qos_node_disable_tc_arbitration(node, extack);
err = esw_qos_node_enable_tc_arbitration(node, extack);
if (!err)
esw_qos_set_tc_arbiter_bw_shares(node, tc_bw, extack);
-unlock:
- esw_qos_unlock(esw);
return err;
}
@@ -1822,17 +1743,14 @@ int mlx5_esw_devlink_rate_node_tx_share_set(struct devlink_rate *rate_node, void
u64 tx_share, struct netlink_ext_ack *extack)
{
struct mlx5_esw_sched_node *node = priv;
- struct mlx5_eswitch *esw = node->esw;
int err;
- err = esw_qos_devlink_rate_to_mbps(esw->dev, "tx_share", &tx_share, extack);
+ err = esw_qos_devlink_rate_to_mbps(node->esw->dev, "tx_share",
+ &tx_share, extack);
if (err)
return err;
- esw_qos_lock(esw);
- err = esw_qos_set_node_min_rate(node, tx_share, extack);
- esw_qos_unlock(esw);
- return err;
+ return esw_qos_set_node_min_rate(node, tx_share, extack);
}
int mlx5_esw_devlink_rate_node_tx_max_set(struct devlink_rate *rate_node, void *priv,
@@ -1846,10 +1764,7 @@ int mlx5_esw_devlink_rate_node_tx_max_set(struct devlink_rate *rate_node, void *
if (err)
return err;
- esw_qos_lock(esw);
- err = esw_qos_sched_elem_config(node, tx_max, node->bw_share, extack);
- esw_qos_unlock(esw);
- return err;
+ return esw_qos_sched_elem_config(node, tx_max, node->bw_share, extack);
}
int mlx5_esw_devlink_rate_node_new(struct devlink_rate *rate_node, void **priv,
@@ -1857,30 +1772,23 @@ int mlx5_esw_devlink_rate_node_new(struct devlink_rate *rate_node, void **priv,
{
struct mlx5_esw_sched_node *node;
struct mlx5_eswitch *esw;
- int err = 0;
esw = mlx5_devlink_eswitch_get(rate_node->devlink);
if (IS_ERR(esw))
return PTR_ERR(esw);
- esw_qos_lock(esw);
if (esw->mode != MLX5_ESWITCH_OFFLOADS) {
NL_SET_ERR_MSG_MOD(extack,
"Rate node creation supported only in switchdev mode");
- err = -EOPNOTSUPP;
- goto unlock;
+ return -EOPNOTSUPP;
}
node = esw_qos_create_vports_sched_node(esw, extack);
- if (IS_ERR(node)) {
- err = PTR_ERR(node);
- goto unlock;
- }
+ if (IS_ERR(node))
+ return PTR_ERR(node);
*priv = node;
-unlock:
- esw_qos_unlock(esw);
- return err;
+ return 0;
}
int mlx5_esw_devlink_rate_node_del(struct devlink_rate *rate_node, void *priv,
@@ -1889,10 +1797,9 @@ int mlx5_esw_devlink_rate_node_del(struct devlink_rate *rate_node, void *priv,
struct mlx5_esw_sched_node *node = priv;
struct mlx5_eswitch *esw = node->esw;
- esw_qos_lock(esw);
__esw_qos_destroy_node(node, extack);
esw_qos_put(esw);
- esw_qos_unlock(esw);
+
return 0;
}
@@ -1909,7 +1816,6 @@ mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport,
return -EOPNOTSUPP;
}
- esw_qos_lock(esw);
if (!vport->qos.sched_node && parent) {
enum sched_node_type type;
@@ -1920,13 +1826,15 @@ mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport,
} else if (vport->qos.sched_node) {
err = esw_qos_vport_update_parent(vport, parent, extack);
}
- esw_qos_unlock(esw);
+
return err;
}
void mlx5_esw_qos_vport_clear_parent(struct mlx5_vport *vport)
{
+ devl_lock(vport->dev->shd);
mlx5_esw_qos_vport_update_parent(vport, NULL, NULL);
+ devl_unlock(vport->dev->shd);
}
int mlx5_esw_devlink_rate_leaf_parent_set(struct devlink_rate *devlink_rate,
@@ -1939,13 +1847,8 @@ int mlx5_esw_devlink_rate_leaf_parent_set(struct devlink_rate *devlink_rate,
int err;
err = mlx5_esw_qos_vport_update_parent(vport, node, extack);
- if (!err) {
- struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
-
- esw_qos_lock(esw);
+ if (!err)
esw_vport_qos_prune_empty(vport);
- esw_qos_unlock(esw);
- }
return err;
}
@@ -2071,14 +1974,12 @@ static int mlx5_esw_qos_node_update_parent(struct mlx5_esw_sched_node *node,
struct netlink_ext_ack *extack)
{
struct mlx5_esw_sched_node *curr_parent;
- struct mlx5_eswitch *esw = node->esw;
int err;
err = mlx5_esw_qos_node_validate_set_parent(node, parent, extack);
if (err)
return err;
- esw_qos_lock(esw);
curr_parent = node->parent;
if (node->type == SCHED_NODE_TYPE_TC_ARBITER_TSAR) {
err = esw_qos_tc_arbiter_node_update_parent(node, parent,
@@ -2088,15 +1989,11 @@ static int mlx5_esw_qos_node_update_parent(struct mlx5_esw_sched_node *node,
}
if (err)
- goto out;
-
- esw_qos_normalize_min_rate(esw, curr_parent, extack);
- esw_qos_normalize_min_rate(esw, parent, extack);
-
-out:
- esw_qos_unlock(esw);
+ return err;
- return err;
+ esw_qos_normalize_min_rate(node->esw, curr_parent, extack);
+ esw_qos_normalize_min_rate(node->esw, parent, extack);
+ return 0;
}
int mlx5_esw_devlink_rate_node_parent_set(struct devlink_rate *devlink_rate,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.h b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.h
index 0a50982b0e27..f275e850d2c9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.h
@@ -6,9 +6,6 @@
#ifdef CONFIG_MLX5_ESWITCH
-int mlx5_esw_qos_init(struct mlx5_eswitch *esw);
-void mlx5_esw_qos_cleanup(struct mlx5_eswitch *esw);
-
int mlx5_esw_qos_set_vport_rate(struct mlx5_vport *evport, u32 max_rate, u32 min_rate);
bool mlx5_esw_qos_get_vport_rate(struct mlx5_vport *vport, u32 *max_rate, u32 *min_rate);
void mlx5_esw_qos_vport_disable(struct mlx5_vport *vport);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index 4b7a1ce7f406..51eacc286cbb 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -49,6 +49,7 @@
#include "ecpf.h"
#include "en/mod_hdr.h"
#include "en_accel/ipsec.h"
+#include "sh_devlink.h"
enum {
MLX5_ACTION_NONE = 0,
@@ -1618,10 +1619,6 @@ int mlx5_eswitch_enable_locked(struct mlx5_eswitch *esw, int num_vfs)
MLX5_NB_INIT(&esw->nb, eswitch_vport_event, NIC_VPORT_CHANGE);
mlx5_eq_notifier_register(esw->dev, &esw->nb);
- err = mlx5_esw_qos_init(esw);
- if (err)
- goto err_esw_init;
-
if (esw->mode == MLX5_ESWITCH_LEGACY) {
err = esw_legacy_enable(esw);
} else {
@@ -2028,9 +2025,6 @@ int mlx5_eswitch_init(struct mlx5_core_dev *dev)
goto reps_err;
esw->mode = MLX5_ESWITCH_LEGACY;
- err = mlx5_esw_qos_init(esw);
- if (err)
- goto reps_err;
mutex_init(&esw->offloads.encap_tbl_lock);
hash_init(esw->offloads.encap_tbl);
@@ -2080,7 +2074,6 @@ void mlx5_eswitch_cleanup(struct mlx5_eswitch *esw)
esw_info(esw->dev, "cleanup\n");
- mlx5_esw_qos_cleanup(esw);
destroy_workqueue(esw->work_queue);
WARN_ON(refcount_read(&esw->qos.refcnt));
mutex_destroy(&esw->state_lock);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index 20cf9dd542a1..d145591b3434 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -224,8 +224,9 @@ struct mlx5_vport {
struct mlx5_vport_info info;
- /* Protected with the E-Switch qos domain lock. The Vport QoS can
- * either be disabled (sched_node is NULL) or in one of three states:
+ /* Protected by mlx5_shd_lock().
+ * The Vport QoS can either be disabled (sched_node is NULL) or in one
+ * of three states:
* 1. Regular QoS (sched_node is a vport node).
* 2. TC QoS enabled on the vport (sched_node is a TC arbiter).
* 3. TC QoS enabled on the vport's parent node
@@ -356,7 +357,6 @@ enum {
};
struct dentry;
-struct mlx5_qos_domain;
struct mlx5_eswitch {
struct mlx5_core_dev *dev;
@@ -383,12 +383,13 @@ struct mlx5_eswitch {
struct rw_semaphore mode_lock;
atomic64_t user_count;
- /* Protected with the E-Switch qos domain lock. */
+ /* The QoS tree is stored in mlx5_shd.
+ * QoS changes are serialized with mlx5_shd_lock().
+ */
struct {
/* Initially 0, meaning no QoS users and QoS is disabled. */
refcount_t refcnt;
u32 root_tsar_ix;
- struct mlx5_qos_domain *domain;
} qos;
struct mlx5_esw_bridge_offloads *br_offloads;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c
index 202f4ae99fa9..22fd6e0e4b7a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c
@@ -9,6 +9,16 @@
static const struct devlink_ops mlx5_shd_ops = {
};
+struct mlx5_shd_priv {
+ struct list_head qos_nodes;
+};
+
+static int mlx5_shd_priv_init(struct mlx5_shd_priv *shd_priv)
+{
+ INIT_LIST_HEAD(&shd_priv->qos_nodes);
+ return 0;
+}
+
int mlx5_shd_init(struct mlx5_core_dev *dev)
{
struct pci_dev *pdev = dev->pdev;
@@ -49,13 +59,20 @@ int mlx5_shd_init(struct mlx5_core_dev *dev)
*end = '\0';
/* Get or create shared devlink instance */
- devlink = devlink_shd_get(sn, &mlx5_shd_ops, 0);
+ devlink = devlink_shd_get(sn, &mlx5_shd_ops,
+ sizeof(struct mlx5_shd_priv));
kfree(sn);
if (!devlink) {
err = -ENOMEM;
goto out;
}
+ err = mlx5_shd_priv_init(devlink_shd_get_priv(devlink));
+ if (err < 0) {
+ devlink_shd_put(devlink);
+ return err;
+ }
+
dev->shd = devlink;
out:
kfree(vpd_data);
@@ -69,3 +86,14 @@ void mlx5_shd_uninit(struct mlx5_core_dev *dev)
devlink_shd_put(dev->shd);
}
+
+struct list_head *mlx5_shd_get_qos_nodes(struct mlx5_core_dev *dev)
+{
+ struct mlx5_shd_priv *shd_priv;
+
+ if (!dev->shd)
+ return NULL;
+ devl_assert_locked(dev->shd);
+ shd_priv = devlink_shd_get_priv(dev->shd);
+ return &shd_priv->qos_nodes;
+}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.h b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.h
index 8ab8d6940227..ec6a03471204 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.h
@@ -9,4 +9,6 @@
int mlx5_shd_init(struct mlx5_core_dev *dev);
void mlx5_shd_uninit(struct mlx5_core_dev *dev);
+struct list_head *mlx5_shd_get_qos_nodes(struct mlx5_core_dev *dev);
+
#endif /* __MLX5_SH_DEVLINK_H__ */
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH net-next V7 13/14] net/mlx5: qos: Support cross-device tx scheduling
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
` (11 preceding siblings ...)
2026-01-28 11:25 ` [PATCH net-next V7 12/14] net/mlx5: Store QoS sched nodes in the sh_devlink Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-01-28 11:25 ` [PATCH net-next V7 14/14] net/mlx5: Document devlink rates Tariq Toukan
2026-02-03 4:09 ` [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Jakub Kicinski
14 siblings, 0 replies; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Cosmin Ratiu <cratiu@nvidia.com>
Up to now, rate groups could only contain vports from the same E-Switch.
This patch relaxes that restriction if the device supports it
(HCA_CAP.esw_cross_esw_sched == true) and the right conditions are met:
- Link Aggregation (LAG) is enabled.
- The E-Switches are from the same shared devlink device.
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/esw/qos.c | 122 ++++++++++++------
1 file changed, 86 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
index 0d187399d846..b4abb6fa2168 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
@@ -50,7 +50,9 @@ struct mlx5_esw_sched_node {
enum sched_node_type type;
/* The eswitch this node belongs to. */
struct mlx5_eswitch *esw;
- /* The children nodes of this node, empty list for leaf nodes. */
+ /* The children nodes of this node, empty list for leaf nodes.
+ * Can be from multiple E-Switches.
+ */
struct list_head children;
/* Valid only if this node is associated with a vport. */
struct mlx5_vport *vport;
@@ -419,6 +421,7 @@ esw_qos_vport_create_sched_element(struct mlx5_esw_sched_node *vport_node,
struct mlx5_esw_sched_node *parent = vport_node->parent;
u32 sched_ctx[MLX5_ST_SZ_DW(scheduling_context)] = {};
struct mlx5_core_dev *dev = vport_node->esw->dev;
+ struct mlx5_vport *vport = vport_node->vport;
void *attr;
if (!mlx5_qos_element_type_supported(
@@ -430,11 +433,18 @@ esw_qos_vport_create_sched_element(struct mlx5_esw_sched_node *vport_node,
MLX5_SET(scheduling_context, sched_ctx, element_type,
SCHEDULING_CONTEXT_ELEMENT_TYPE_VPORT);
attr = MLX5_ADDR_OF(scheduling_context, sched_ctx, element_attributes);
- MLX5_SET(vport_element, attr, vport_number, vport_node->vport->vport);
+ MLX5_SET(vport_element, attr, vport_number, vport->vport);
MLX5_SET(scheduling_context, sched_ctx, parent_element_id,
parent ? parent->ix : vport_node->esw->qos.root_tsar_ix);
MLX5_SET(scheduling_context, sched_ctx, max_average_bw,
vport_node->max_rate);
+ if (vport->dev != dev) {
+ /* The port is assigned to a node on another eswitch. */
+ MLX5_SET(vport_element, attr, eswitch_owner_vhca_id_valid,
+ true);
+ MLX5_SET(vport_element, attr, eswitch_owner_vhca_id,
+ MLX5_CAP_GEN(vport->dev, vhca_id));
+ }
return esw_qos_node_create_sched_element(vport_node, sched_ctx, extack);
}
@@ -446,6 +456,7 @@ esw_qos_vport_tc_create_sched_element(struct mlx5_esw_sched_node *vport_tc_node,
{
u32 sched_ctx[MLX5_ST_SZ_DW(scheduling_context)] = {};
struct mlx5_core_dev *dev = vport_tc_node->esw->dev;
+ struct mlx5_vport *vport = vport_tc_node->vport;
void *attr;
if (!mlx5_qos_element_type_supported(
@@ -457,8 +468,7 @@ esw_qos_vport_tc_create_sched_element(struct mlx5_esw_sched_node *vport_tc_node,
MLX5_SET(scheduling_context, sched_ctx, element_type,
SCHEDULING_CONTEXT_ELEMENT_TYPE_VPORT_TC);
attr = MLX5_ADDR_OF(scheduling_context, sched_ctx, element_attributes);
- MLX5_SET(vport_tc_element, attr, vport_number,
- vport_tc_node->vport->vport);
+ MLX5_SET(vport_tc_element, attr, vport_number, vport->vport);
MLX5_SET(vport_tc_element, attr, traffic_class, vport_tc_node->tc);
MLX5_SET(scheduling_context, sched_ctx, max_bw_obj_id,
rate_limit_elem_ix);
@@ -466,6 +476,13 @@ esw_qos_vport_tc_create_sched_element(struct mlx5_esw_sched_node *vport_tc_node,
vport_tc_node->parent->ix);
MLX5_SET(scheduling_context, sched_ctx, bw_share,
vport_tc_node->bw_share);
+ if (vport->dev != dev) {
+ /* The port is assigned to a node on another eswitch. */
+ MLX5_SET(vport_tc_element, attr, eswitch_owner_vhca_id_valid,
+ true);
+ MLX5_SET(vport_tc_element, attr, eswitch_owner_vhca_id,
+ MLX5_CAP_GEN(vport->dev, vhca_id));
+ }
return esw_qos_node_create_sched_element(vport_tc_node, sched_ctx,
extack);
@@ -1194,6 +1211,29 @@ static int esw_qos_vport_tc_check_type(enum sched_node_type curr_type,
return 0;
}
+static bool esw_qos_validate_unsupported_tc_bw(struct mlx5_eswitch *esw,
+ u32 *tc_bw)
+{
+ int i, num_tcs = esw_qos_num_tcs(esw->dev);
+
+ for (i = num_tcs; i < DEVLINK_RATE_TCS_MAX; i++)
+ if (tc_bw[i])
+ return false;
+
+ return true;
+}
+
+static bool esw_qos_vport_validate_unsupported_tc_bw(struct mlx5_vport *vport,
+ u32 *tc_bw)
+{
+ struct mlx5_esw_sched_node *node = vport->qos.sched_node;
+ struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
+
+ esw = (node && node->parent) ? node->parent->esw : esw;
+
+ return esw_qos_validate_unsupported_tc_bw(esw, tc_bw);
+}
+
static int esw_qos_vport_update(struct mlx5_vport *vport,
enum sched_node_type type,
struct mlx5_esw_sched_node *parent,
@@ -1213,8 +1253,17 @@ static int esw_qos_vport_update(struct mlx5_vport *vport,
if (err)
return err;
- if (curr_type == SCHED_NODE_TYPE_TC_ARBITER_TSAR && curr_type == type)
+ if (curr_type == SCHED_NODE_TYPE_TC_ARBITER_TSAR && curr_type == type) {
+ struct mlx5_eswitch *esw = parent ?
+ parent->esw : vport->dev->priv.eswitch;
+
esw_qos_tc_arbiter_get_bw_shares(vport_node, curr_tc_bw);
+ if (!esw_qos_validate_unsupported_tc_bw(esw, curr_tc_bw)) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Unsupported traffic classes on the new device");
+ return -EOPNOTSUPP;
+ }
+ }
esw_qos_vport_disable(vport, extack);
@@ -1224,10 +1273,9 @@ static int esw_qos_vport_update(struct mlx5_vport *vport,
extack = NULL;
}
- if (curr_type == SCHED_NODE_TYPE_TC_ARBITER_TSAR && curr_type == type) {
+ if (curr_type == SCHED_NODE_TYPE_TC_ARBITER_TSAR && curr_type == type)
esw_qos_set_tc_arbiter_bw_shares(vport_node, curr_tc_bw,
extack);
- }
return err;
}
@@ -1575,30 +1623,6 @@ static int esw_qos_devlink_rate_to_mbps(struct mlx5_core_dev *mdev, const char *
return 0;
}
-static bool esw_qos_validate_unsupported_tc_bw(struct mlx5_eswitch *esw,
- u32 *tc_bw)
-{
- int i, num_tcs = esw_qos_num_tcs(esw->dev);
-
- for (i = num_tcs; i < DEVLINK_RATE_TCS_MAX; i++) {
- if (tc_bw[i])
- return false;
- }
-
- return true;
-}
-
-static bool esw_qos_vport_validate_unsupported_tc_bw(struct mlx5_vport *vport,
- u32 *tc_bw)
-{
- struct mlx5_esw_sched_node *node = vport->qos.sched_node;
- struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
-
- esw = (node && node->parent) ? node->parent->esw : esw;
-
- return esw_qos_validate_unsupported_tc_bw(esw, tc_bw);
-}
-
static bool esw_qos_tc_bw_disabled(u32 *tc_bw)
{
int i;
@@ -1803,18 +1827,44 @@ int mlx5_esw_devlink_rate_node_del(struct devlink_rate *rate_node, void *priv,
return 0;
}
+static int
+mlx5_esw_validate_cross_esw_scheduling(struct mlx5_eswitch *esw,
+ struct mlx5_esw_sched_node *parent,
+ struct netlink_ext_ack *extack)
+{
+ if (!parent || esw == parent->esw)
+ return 0;
+
+ if (!MLX5_CAP_QOS(esw->dev, esw_cross_esw_sched)) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Cross E-Switch scheduling is not supported");
+ return -EOPNOTSUPP;
+ }
+ if (esw->dev->shd != parent->esw->dev->shd) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Cannot add vport to a parent belonging to a different device");
+ return -EOPNOTSUPP;
+ }
+ if (!mlx5_lag_is_active(esw->dev)) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Cross E-Switch scheduling requires LAG to be activated");
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
static int
mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport,
struct mlx5_esw_sched_node *parent,
struct netlink_ext_ack *extack)
{
struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
- int err = 0;
+ int err;
- if (parent && parent->esw != esw) {
- NL_SET_ERR_MSG_MOD(extack, "Cross E-Switch scheduling is not supported");
- return -EOPNOTSUPP;
- }
+ err = mlx5_esw_validate_cross_esw_scheduling(esw, parent, extack);
+ if (err)
+ return err;
if (!vport->qos.sched_node && parent) {
enum sched_node_type type;
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH net-next V7 14/14] net/mlx5: Document devlink rates
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
` (12 preceding siblings ...)
2026-01-28 11:25 ` [PATCH net-next V7 13/14] net/mlx5: qos: Support cross-device tx scheduling Tariq Toukan
@ 2026-01-28 11:25 ` Tariq Toukan
2026-02-03 4:09 ` [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Jakub Kicinski
14 siblings, 0 replies; 32+ messages in thread
From: Tariq Toukan @ 2026-01-28 11:25 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Mark Bloch, netdev, linux-kernel,
linux-doc, linux-rdma, Gal Pressman, Moshe Shemesh,
Carolina Jubran, Cosmin Ratiu, Jiri Pirko, Randy Dunlap,
Simon Horman, Krzysztof Kozlowski
From: Cosmin Ratiu <cratiu@nvidia.com>
It seems rates were not documented in the mlx5-specific file, so add
examples on how to limit VFs and groups and also provide an example of
the intended way to achieve cross-esw scheduling.
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
Documentation/networking/devlink/mlx5.rst | 33 +++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/Documentation/networking/devlink/mlx5.rst b/Documentation/networking/devlink/mlx5.rst
index 4bba4d780a4a..62c4d7bf0877 100644
--- a/Documentation/networking/devlink/mlx5.rst
+++ b/Documentation/networking/devlink/mlx5.rst
@@ -419,3 +419,36 @@ User commands examples:
.. note::
This command can run over all interfaces such as PF/VF and representor ports.
+
+Rates
+=====
+
+mlx5 devices can limit transmission of individual VFs or a group of them via
+the devlink-rate API in switchdev mode.
+
+User commands examples:
+
+- Print the existing rates::
+
+ $ devlink port function rate show
+
+- Set a max tx limit on traffic from VF0::
+
+ $ devlink port function rate set pci/0000:82:00.0/1 tx_max 10Gbit
+
+- Create a rate group with a max tx limit and adding two VFs to it::
+
+ $ devlink port function rate add pci/0000:82:00.0/group1 tx_max 10Gbit
+ $ devlink port function rate set pci/0000:82:00.0/1 parent group1
+ $ devlink port function rate set pci/0000:82:00.0/2 parent group1
+
+- Same scenario, with a min guarantee of 20% of the bandwidth for the first VFs::
+
+ $ devlink port function rate add pci/0000:82:00.0/group1 tx_max 10Gbit
+ $ devlink port function rate set pci/0000:82:00.0/1 parent group1 tx_share 2Gbit
+ $ devlink port function rate set pci/0000:82:00.0/2 parent group1
+
+- Cross-device scheduling::
+
+ $ devlink port function rate add pci/0000:82:00.0/group1 tx_max 10Gbit
+ $ devlink port function rate set pci/0000:82:00.1/32769 parent pci/0000:82:00.0/group1
--
2.44.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling
2026-01-28 11:25 [PATCH net-next V7 00/14] devlink and mlx5: Support cross-function rate scheduling Tariq Toukan
` (13 preceding siblings ...)
2026-01-28 11:25 ` [PATCH net-next V7 14/14] net/mlx5: Document devlink rates Tariq Toukan
@ 2026-02-03 4:09 ` Jakub Kicinski
14 siblings, 0 replies; 32+ messages in thread
From: Jakub Kicinski @ 2026-02-03 4:09 UTC (permalink / raw)
To: Tariq Toukan
Cc: Eric Dumazet, Paolo Abeni, Andrew Lunn, David S. Miller,
Donald Hunter, Jiri Pirko, Jonathan Corbet, Saeed Mahameed,
Leon Romanovsky, Mark Bloch, netdev, linux-kernel, linux-doc,
linux-rdma, Gal Pressman, Moshe Shemesh, Carolina Jubran,
Cosmin Ratiu, Jiri Pirko, Randy Dunlap, Simon Horman,
Krzysztof Kozlowski
On Wed, 28 Jan 2026 13:25:30 +0200 Tariq Toukan wrote:
> This series by Cosmin and Jiri adds support for cross-function rate
> scheduling in devlink and mlx5.
> See detailed explanation by Cosmin below [1].
I'll apply the trivial patches (3, 5) to avoid the reposts.
Please add driver tests exercising this code, there's high risk
of regressions with all this complexity and nested locking.
What devices will support this? Is CX7 enough to test?
^ permalink raw reply [flat|nested] 32+ messages in thread