Tue, Feb 03, 2026 at 08:10:29AM +0100, [email protected] wrote:
>From: Or Har-Toov <[email protected]>
>
>The current devlink resource infrastructure only supports device-level
>resources. Some hardware resources are associated with specific ports
>rather than the entire device, and today we have no way to show resource
>per-port.
>
>Add support for registering and querying resources at the port level,
>allowing drivers to expose per-port resource limits and usage.
>
>Example output:
>
> $ devlink port resource show
> pci/0000:03:00.0/196608:
> name max_SFs size 20 unit entry
> pci/0000:03:00.1/262144:
> name max_SFs size 20 unit entry
>
> $ devlink port resource show pci/0000:03:00.0/196608
> pci/0000:03:00.0/196608:
> name max_SFs size 20 unit entry
>
>Signed-off-by: Or Har-Toov <[email protected]>
>Reviewed-by: Shay Drori <[email protected]>
>Reviewed-by: Moshe Shemesh <[email protected]>
>Signed-off-by: Tariq Toukan <[email protected]>
>---
> Documentation/netlink/specs/devlink.yaml | 23 ++
> include/net/devlink.h | 8 +
> include/uapi/linux/devlink.h | 2 +
> net/devlink/netlink.c | 2 +-
> net/devlink/netlink_gen.c | 32 ++-
> net/devlink/netlink_gen.h | 6 +-
> net/devlink/port.c | 3 +
> net/devlink/resource.c | 282 ++++++++++++++++++-----
> 8 files changed, 301 insertions(+), 57 deletions(-)
Way too big for a single patch. Could you split it in logical chunks
please?
>
>diff --git a/Documentation/netlink/specs/devlink.yaml
>b/Documentation/netlink/specs/devlink.yaml
>index 837112da6738..0290db1b8393 100644
>--- a/Documentation/netlink/specs/devlink.yaml
>+++ b/Documentation/netlink/specs/devlink.yaml
>@@ -2336,3 +2336,26 @@ operations:
> - bus-name
> - dev-name
> - port-index
>+
>+ -
>+ name: port-resource-get
Why this is not aligned with DEVLINK_CMD_RESOURCE_* ?
$ git grep CMD_RESOURCE_ include/uapi/linux/devlink.h
include/uapi/linux/devlink.h: DEVLINK_CMD_RESOURCE_SET,
include/uapi/linux/devlink.h: DEVLINK_CMD_RESOURCE_DUMP,
I'm aware that DEVLINK_CMD_RESOURCE_DUMP only implements "do" now, but I
think both should be named the same, no?
>+ doc: Get port resources.
>+ attribute-set: devlink
>+ dont-validate: [strict]
>+ do:
>+ pre: devlink-nl-pre-doit-port
>+ post: devlink-nl-post-doit
>+ request:
>+ value: 85
>+ attributes: *port-id-attrs
>+ reply: &port-resource-get-reply
>+ value: 85
>+ attributes:
>+ - bus-name
>+ - dev-name
>+ - port-index
>+ - resource-list
>+ dump:
>+ request:
>+ attributes: *dev-id-attrs
>+ reply: *port-resource-get-reply
[...]
>-int devl_resource_register(struct devlink *devlink,
>- const char *resource_name,
>- u64 resource_size,
>- u64 resource_id,
>- u64 parent_resource_id,
>- const struct devlink_resource_size_params
>*size_params)
>+static int
>+devl_resource_reg_by_list(struct devlink *devlink,
can't this be "register"?
Also, why "by_list"? Sounds odd. Could you perhaps have it as
__devl_resource_register(). That's the usual pattern for similar
functions here, isn't it?
Also, could you do this "list" abstraction in a separate patch?
>+ struct list_head *res_list_head,
>+ const char *resource_name, u64 resource_size,
>+ u64 resource_id, u64 parent_res_id,
>+ const struct devlink_resource_size_params *params)
> {
> struct devlink_resource *resource;
> struct list_head *resource_list;
>@@ -341,9 +344,10 @@ int devl_resource_register(struct devlink *devlink,
>
> lockdep_assert_held(&devlink->lock);
>
>- top_hierarchy = parent_resource_id == DEVLINK_RESOURCE_ID_PARENT_TOP;
>+ top_hierarchy = parent_res_id == DEVLINK_RESOURCE_ID_PARENT_TOP;
>
>- resource = devlink_resource_find(devlink, NULL, resource_id);
>+ resource = devlink_resource_find_by_list(res_list_head, NULL,
>+ resource_id);
> if (resource)
> return -EEXIST;
>
>@@ -352,15 +356,15 @@ int devl_resource_register(struct devlink *devlink,
> return -ENOMEM;
>
> if (top_hierarchy) {
>- resource_list = &devlink->resource_list;
>+ resource_list = res_list_head;
> } else {
>- struct devlink_resource *parent_resource;
>+ struct devlink_resource *parent_res;
>
>- parent_resource = devlink_resource_find(devlink, NULL,
>- parent_resource_id);
>- if (parent_resource) {
>- resource_list = &parent_resource->resource_list;
>- resource->parent = parent_resource;
>+ parent_res = devlink_resource_find_by_list(res_list_head, NULL,
>+ parent_res_id);
>+ if (parent_res) {
>+ resource_list = &parent_res->resource_list;
>+ resource->parent = parent_res;
> } else {
> kfree(resource);
> return -EINVAL;
>@@ -372,46 +376,78 @@ int devl_resource_register(struct devlink *devlink,
> resource->size_new = resource_size;
> resource->id = resource_id;
> resource->size_valid = true;
>- memcpy(&resource->size_params, size_params,
>- sizeof(resource->size_params));
>+ memcpy(&resource->size_params, params, sizeof(resource->size_params));
> INIT_LIST_HEAD(&resource->resource_list);
> list_add_tail(&resource->list, resource_list);
>
> return 0;
> }
>+
>+/**
>+ * devl_resource_register - devlink resource register
>+ *
>+ * @devlink: devlink
>+ * @resource_name: resource's name
>+ * @resource_size: resource's size
>+ * @resource_id: resource's id
>+ * @parent_resource_id: resource's parent id
>+ * @params: size parameters
>+ *
>+ * Generic resources should reuse the same names across drivers.
>+ * Please see the generic resources list at:
>+ * Documentation/networking/devlink/devlink-resource.rst
>+ *
>+ * Return: 0 on success, negative error code otherwise.
>+ */
>+int devl_resource_register(struct devlink *devlink, const char *resource_name,
>+ u64 resource_size, u64 resource_id,
>+ u64 parent_resource_id,
>+ const struct devlink_resource_size_params *params)
>+{
>+ return devl_resource_reg_by_list(devlink, &devlink->resource_list,
>+ resource_name, resource_size,
>+ resource_id, parent_resource_id,
>+ params);
>+}
[...]
