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 | 11 ++++
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, 90 insertions(+), 5 deletions(-)
diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 837112da6738..1f41d934dc5b 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -867,6 +867,9 @@ 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 +1292,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 580985025f49..8fbe0417ab55 100644
--- a/net/devlink/netlink_gen.c
+++ b/net/devlink/netlink_gen.c
@@ -38,6 +38,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 09cc6f264ccf..94566cab1734 100644
--- a/net/devlink/netlink_gen.h
+++ b/net/devlink/netlink_gen.h
@@ -12,6 +12,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];
@@ -28,12 +29,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.31.1
On Tue, Nov 25, 2025 at 10:06:05PM +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> > --- > Documentation/netlink/specs/devlink.yaml | 11 ++++ > 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 +++ Hi, I think that the updates to netlink_gen.[ch] belong in the following patch rather than this one. You can observe this using tools/net/ynl/ynl-regen.sh -f && git diff
On Thu, 2025-11-27 at 15:28 +0000, Simon Horman wrote:
> On Tue, Nov 25, 2025 at 10:06:05PM +0200, Tariq Toukan wrote:
> > net/devlink/netlink_gen.c | 5 ++
> > net/devlink/netlink_gen.h | 8 +++
>
> Hi,
>
> I think that the updates to netlink_gen.[ch] belong in
> the following patch rather than this one.
>
> You can observe this using
>
> tools/net/ynl/ynl-regen.sh -f && git diff
Hi,
You are right, some of these changes belong to the next patch. We will
fix in the next submission.
But running ynl-regen.sh results in unbuildable code, due to missing
devlink_dl_parent_dev_nl_policy. It seems it is not added unless the
attribute is referenced in at least one attribute set.
Additionally, the devlink_nl_pre_doit_parent_dev_optional needs to be
manually added to netlink-gen.{h,c}, as it is not automatically
generated.
Cosmin.
© 2016 - 2025 Red Hat, Inc.