From: Cosmin Ratiu <[email protected]> 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->ctx.
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 <[email protected]> Reviewed-by: Carolina Jubran <[email protected]> Reviewed-by: Jiri Pirko <[email protected]> Signed-off-by: Tariq Toukan <[email protected]> --- Documentation/netlink/specs/devlink.yaml | 14 +++++++++ include/uapi/linux/devlink.h | 2 ++ net/devlink/devl_internal.h | 3 ++ net/devlink/netlink.c | 36 ++++++++++++++++++++---- 4 files changed, 50 insertions(+), 5 deletions(-) diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml index b495d56b9137..43cc0abf7235 100644 --- a/Documentation/netlink/specs/devlink.yaml +++ b/Documentation/netlink/specs/devlink.yaml @@ -873,6 +873,10 @@ attribute-sets: doc: Unique devlink instance index. checks: max: u32-max + - + name: parent-dev + type: nest + nested-attributes: dl-parent-dev - name: dl-dev-stats subset-of: devlink @@ -1295,6 +1299,16 @@ 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 + - + name: index operations: enum-model: directional diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h index 7de2d8cc862f..01b7a4fcfedd 100644 --- a/include/uapi/linux/devlink.h +++ b/include/uapi/linux/devlink.h @@ -646,6 +646,8 @@ enum devlink_attr { DEVLINK_ATTR_INDEX, /* uint */ + 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 1af445f044e5..414b3d8f70a5 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -153,6 +153,7 @@ int devlink_rel_devlink_handle_put(struct sk_buff *msg, struct devlink *devlink, struct devlink_nl_ctx { struct devlink *devlink; struct devlink_port *devlink_port; + struct devlink *parent_devlink; }; static inline struct devlink_nl_ctx * @@ -191,6 +192,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 5624cf71592f..21a34e4d2a49 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 }, @@ -239,19 +240,39 @@ 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) +{ + return ERR_PTR(-EOPNOTSUPP); +} + 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); + devlink_nl_ctx(info)->parent_devlink = 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; + } devlink_nl_ctx(info)->devlink = devlink; if (flags & DEVLINK_NL_FLAG_NEED_PORT) { devlink_port = devlink_port_get_from_info(devlink, info); @@ -270,6 +291,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; } @@ -307,6 +331,8 @@ static void __devlink_nl_post_doit(struct sk_buff *skb, struct genl_info *info, devlink = devlink_nl_ctx(info)->devlink; devl_dev_unlock(devlink, dev_lock); devlink_put(devlink); + if (devlink_nl_ctx(info)->parent_devlink) + devlink_put(devlink_nl_ctx(info)->parent_devlink); } void devlink_nl_post_doit(const struct genl_split_ops *ops, -- 2.44.0
