From: Cosmin Ratiu 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 Reviewed-by: Carolina Jubran Reviewed-by: Jiri Pirko Signed-off-by: Tariq Toukan --- 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 /* 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