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 | 67 ++++++++++++++++++++++--
net/devlink/netlink_gen.c | 5 ++
net/devlink/netlink_gen.h | 8 +++
6 files changed, 91 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..781758b8632c 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],
+ devlink_dl_parent_dev_nl_policy, 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;
}
@@ -265,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)
{
@@ -274,6 +318,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,
@@ -289,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..6b691bdbf037 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),
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.40.1On Sun, Jan 25, 2026 at 01:31:56PM +0200, Tariq Toukan wrote:
> 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>
...
Hi Cosmin, all,
The netlink_gen.[ch] and spec changes in this patch do not seem consistent.
$ tools/net/ynl/ynl-regen.sh -f
$ git diff
diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
index 6b691bdbf037..f4c61c2b4f22 100644
--- a/net/devlink/netlink_gen.c
+++ b/net/devlink/netlink_gen.c
@@ -39,11 +39,6 @@ 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),
diff --git a/net/devlink/netlink_gen.h b/net/devlink/netlink_gen.h
index d4db82a00522..2817d53a0eba 100644
--- a/net/devlink/netlink_gen.h
+++ b/net/devlink/netlink_gen.h
@@ -13,7 +13,6 @@
#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];
@@ -30,19 +29,12 @@ 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);
;
On Tue, 2026-01-27 at 13:49 +0000, Simon Horman wrote:
> On Sun, Jan 25, 2026 at 01:31:56PM +0200, Tariq Toukan wrote:
> > 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>
>
> ...
>
> Hi Cosmin, all,
>
> The netlink_gen.[ch] and spec changes in this patch do not seem
> consistent.
>
> $ tools/net/ynl/ynl-regen.sh -f
> $ git diff
> diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
> index 6b691bdbf037..f4c61c2b4f22 100644
> --- a/net/devlink/netlink_gen.c
> +++ b/net/devlink/netlink_gen.c
> @@ -39,11 +39,6 @@ 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),
> diff --git a/net/devlink/netlink_gen.h b/net/devlink/netlink_gen.h
> index d4db82a00522..2817d53a0eba 100644
> --- a/net/devlink/netlink_gen.h
> +++ b/net/devlink/netlink_gen.h
> @@ -13,7 +13,6 @@
> #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];
> @@ -30,19 +29,12 @@ 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);
> ;
Hi Simon,
We had this conversation during v4, I replied then [1].
But thinking about it a bit more, I think it's indeed slightly cleaner
to move the policy and the new pre/post doit handlers to the next
patch, where they are actually used. The only bit is that the policy is
used from devlink_get_parent_from_attrs_lock from this function, but it
appears safe to use NULL there until next patch (the underlying parse
functions tolerate NULL policies).
So I'll do that in the next submission.
[1]
https://lore.kernel.org/netdev/3ec956ea1d0a1c6e56865b2ded6d83ed773ccd4d.camel@nvidia.com/
On Tue, Jan 27, 2026 at 02:25:51PM +0000, Cosmin Ratiu wrote: > On Tue, 2026-01-27 at 13:49 +0000, Simon Horman wrote: > > On Sun, Jan 25, 2026 at 01:31:56PM +0200, Tariq Toukan wrote: > > > From: Cosmin Ratiu <cratiu@nvidia.com> ... Hi Cosmin, > Hi Simon, > > We had this conversation during v4, I replied then [1]. Sorry, I had forgotten about that. > But thinking about it a bit more, I think it's indeed slightly cleaner > to move the policy and the new pre/post doit handlers to the next > patch, where they are actually used. The only bit is that the policy is > used from devlink_get_parent_from_attrs_lock from this function, but it > appears safe to use NULL there until next patch (the underlying parse > functions tolerate NULL policies). > > So I'll do that in the next submission. Thanks, that makes sense. And I'll try to remember this conversation when looking at the next version :) > > [1] > https://lore.kernel.org/netdev/3ec956ea1d0a1c6e56865b2ded6d83ed773ccd4d.camel@nvidia.com/
© 2016 - 2026 Red Hat, Inc.