Re: [PATCH net-next V4 10/12] devlink: Add resource scope filtering to resource dump

[Or Har-Toov]

On 03/04/2026 5:02, Jakub Kicinski wrote:
> On Wed, 1 Apr 2026 21:49:45 +0300 Tariq Toukan wrote:
> > @@ -873,6 +881,16 @@ attribute-sets:
> >           doc: Unique devlink instance index.
> >           checks:
> >             max: u32-max
> > +      -
> > +        name: resource-scope-mask
> > +        type: bitfield32
>
> no need for a bitfield here, this is a simpler selector
> bitfield is for cases when we need to update some persistent
> state, in that case we want to indicate which bits we intend
> to update:
>
>          cfg = (cfg & ~bf.mask) | bf.val
>
> scope is a straight attribute, there's no updating of anything.
>
> u32 or unit would do

ack, will use u32 for resource scope mask

> > +        enum: resource-scope
> > +        enum-as-flags: true
> > +        doc: |
> > +          Bitmask selecting which resource classes to include in a
> > +          resource-dump response. Bit 0 (dev) selects device-level
> > +          resources; bit 1 (port) selects port-level resources.
> > +          When absent all classes are returned.
> >     -
> >       name: dl-dev-stats
> >       subset-of: devlink
> > @@ -1775,7 +1793,11 @@ operations:
> >               - resource-list
> >         dump:
> >           request:
> > -          attributes: *dev-id-attrs
> > +          attributes:
> > +            - bus-name
> > +            - dev-name
> > +            - index
> > +            - resource-scope-mask
> >           reply: *resource-dump-reply
> >
> >       -
> > diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
> > index 7de2d8cc862f..e0a0b523ce5c 100644
> > --- a/include/uapi/linux/devlink.h
> > +++ b/include/uapi/linux/devlink.h
> > @@ -645,6 +645,7 @@ enum devlink_attr {
> >        DEVLINK_ATTR_PARAM_RESET_DEFAULT,       /* flag */
> >
> >        DEVLINK_ATTR_INDEX,                     /* uint */
> > +     DEVLINK_ATTR_RESOURCE_SCOPE_MASK,       /* bitfield32 */
> >
> >        /* Add new attributes above here, update the spec in
> >         * Documentation/netlink/specs/devlink.yaml and re-generate
> > @@ -704,6 +705,22 @@ enum devlink_resource_unit {
> >        DEVLINK_RESOURCE_UNIT_ENTRY,
> >   };
> >
> > +enum devlink_resource_scope {
> > +     DEVLINK_RESOURCE_SCOPE_DEV_BIT,
> > +     DEVLINK_RESOURCE_SCOPE_PORT_BIT,
> > +
> > +     __DEVLINK_RESOURCE_SCOPE_MAX_BIT,
> > +     DEVLINK_RESOURCE_SCOPE_MAX_BIT =
>
> do we need this? it's not an attr enum all we care about here is
> the mask, really so just a trailing value which is max real value + 1
> is enough for all users?

ok, can manage without max at all, can manage also without 
DEVLINK_RESOURCE_SCOPE_VALID_MASK

> > +             __DEVLINK_RESOURCE_SCOPE_MAX_BIT - 1
> > +};
> > +
> > +#define DEVLINK_RESOURCE_SCOPE_DEV \
> > +     _BITUL(DEVLINK_RESOURCE_SCOPE_DEV_BIT)
> > +#define DEVLINK_RESOURCE_SCOPE_PORT \
> > +     _BITUL(DEVLINK_RESOURCE_SCOPE_PORT_BIT)
> > +#define DEVLINK_RESOURCE_SCOPE_VALID_MASK \
> > +     (_BITUL(__DEVLINK_RESOURCE_SCOPE_MAX_BIT) - 1)
> > +
> >   enum devlink_port_fn_attr_cap {
> >        DEVLINK_PORT_FN_ATTR_CAP_ROCE_BIT,
> >        DEVLINK_PORT_FN_ATTR_CAP_MIGRATABLE_BIT,
>
> > +static u32 devlink_resource_scope_get(struct nlattr **attrs, int *flags)
> > +{
> > +     struct nla_bitfield32 scope;
> > +     u32 value;
> > +
> > +     if (!attrs || !attrs[DEVLINK_ATTR_RESOURCE_SCOPE_MASK])
> > +             return DEVLINK_RESOURCE_SCOPE_VALID_MASK;
> > +
> > +     scope = nla_get_bitfield32(attrs[DEVLINK_ATTR_RESOURCE_SCOPE_MASK]);
> > +     value = scope.value & scope.selector;
> > +     if (value != DEVLINK_RESOURCE_SCOPE_VALID_MASK)
> > +             *flags |= NLM_F_DUMP_FILTERED;
> > +
> > +     return value;
> > +}
> > +
> >   static int
> >   devlink_resource_dump_fill_one(struct sk_buff *skb, struct devlink *devlink,
> >                               struct devlink_port *devlink_port,
> > @@ -400,16 +416,27 @@ devlink_nl_resource_dump_one(struct sk_buff *skb, struct devlink *devlink,
> >        struct devlink_nl_dump_state *state = devlink_dump_state(cb);
> >        struct devlink_port *devlink_port;
> >        unsigned long port_idx;
> > +     u32 scope;
> >        int err;
> >
> > -     if (!state->port_number) {
> > +     scope = devlink_resource_scope_get(genl_info_dump(cb)->attrs, &flags);
> > +     if (!scope) {
> > +             NL_SET_ERR_MSG_ATTR(genl_info_dump(cb)->extack,
> > +                                 genl_info_dump(cb)->attrs[DEVLINK_ATTR_RESOURCE_SCOPE_MASK],
>
> we have genl_info_dump(cb) 3 times here, let's save the pointer
> on the stack to make the lines shorter.

ack

> > +                                 "empty resource scope selection");
> > +             return -EINVAL;
> > +     }
> > +     if (!state->port_number && (scope & DEVLINK_RESOURCE_SCOPE_DEV)) {
> >                err = devlink_resource_dump_fill_one(skb, devlink, NULL,
> > -                                                  cb, flags, &state->idx);
> > +                                                  cb, flags,
> > +                                                  &state->idx);
> >                if (err)
> >                        return err;
> >                state->idx = 0;
> >        }
> >
> > +     if (!(scope & DEVLINK_RESOURCE_SCOPE_PORT))
> > +             goto out;
> >        xa_for_each_start(&devlink->ports, port_idx, devlink_port,
> >                          state->port_number ? state->port_number - 1 : 0) {
> >                err = devlink_resource_dump_fill_one(skb, devlink, devlink_port,
> > @@ -420,6 +447,7 @@ devlink_nl_resource_dump_one(struct sk_buff *skb, struct devlink *devlink,
> >                }
> >                state->idx = 0;
> >        }
> > +out:
> >        state->port_number = 0;
> >        return 0;
> >   }