Commit 11bc150d authored by Paolo Abeni's avatar Paolo Abeni
Browse files

Merge branch 'netlink-support-reporting-missing-attributes' 

Jakub Kicinski says:

====================
netlink: support reporting missing attributes

This series adds support for reporting missing attributes
in a structured way. We communicate the type of the missing
attribute and if it was missing inside a nest the offset
of that nest.

Example of (YAML-based) user space reporting ethtool header
missing:

 Kernel error: missing attribute: .header

I was tempted to integrate the check with the policy
but it seems tricky without doing a full scan, and there
may be a ton of attrs in the policy. So leaving that
for later.
====================

Link: https://lore.kernel.org/r/20220826030935.2165661-1-kuba@kernel.org


Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parents 47cf8899 4f5059e6

Documentation/userspace-api/netlink/intro.rst

+5 −2
Original line number Diff line number Diff line
@@ -359,8 +359,8 @@ compatibility this feature has to be explicitly enabled by setting 
the ``NETLINK_EXT_ACK`` setsockopt() to ``1``.



Types of extended ack attributes are defined in enum nlmsgerr_attrs.

The two most commonly used attributes are ``NLMSGERR_ATTR_MSG``

and ``NLMSGERR_ATTR_OFFS``.

The most commonly used attributes are ``NLMSGERR_ATTR_MSG``,

``NLMSGERR_ATTR_OFFS`` and ``NLMSGERR_ATTR_MISS_*``.



``NLMSGERR_ATTR_MSG`` carries a message in English describing

the encountered problem. These messages are far more detailed
@@ -368,6 +368,9 @@ than what can be expressed thru standard UNIX error codes. 


``NLMSGERR_ATTR_OFFS`` points to the attribute which caused the problem.



``NLMSGERR_ATTR_MISS_TYPE`` and ``NLMSGERR_ATTR_MISS_NEST``

inform about a missing attribute.



Extended ACKs can be reported on errors as well as in case of success.

The latter should be treated as a warning.

include/linux/netlink.h

+24 −0
Original line number Diff line number Diff line
@@ -71,6 +71,8 @@ netlink_kernel_create(struct net *net, int unit, struct netlink_kernel_cfg *cfg) 
 *	%NL_SET_ERR_MSG

 * @bad_attr: attribute with error

 * @policy: policy for a bad attribute

 * @miss_type: attribute type which was missing

 * @miss_nest: nest missing an attribute (%NULL if missing top level attr)

 * @cookie: cookie data to return to userspace (for success)

 * @cookie_len: actual cookie data length

 */
@@ -78,6 +80,8 @@ struct netlink_ext_ack { 
	const char *_msg;

	const struct nlattr *bad_attr;

	const struct nla_policy *policy;

	const struct nlattr *miss_nest;

	u16 miss_type;

	u8 cookie[NETLINK_MAX_COOKIE_LEN];

	u8 cookie_len;

};
@@ -126,6 +130,26 @@ struct netlink_ext_ack { 
#define NL_SET_ERR_MSG_ATTR(extack, attr, msg)		\

	NL_SET_ERR_MSG_ATTR_POL(extack, attr, NULL, msg)



#define NL_SET_ERR_ATTR_MISS(extack, nest, type)  do {	\

	struct netlink_ext_ack *__extack = (extack);	\

							\

	if (__extack) {					\

		__extack->miss_nest = (nest);		\

		__extack->miss_type = (type);		\

	}						\

} while (0)



#define NL_REQ_ATTR_CHECK(extack, nest, tb, type) ({		\

	struct nlattr **__tb = (tb);				\

	u32 __attr = (type);					\

	int __retval;						\

								\

	__retval = !__tb[__attr];				\

	if (__retval)						\

		NL_SET_ERR_ATTR_MISS((extack), (nest), __attr);	\

	__retval;						\

})



static inline void nl_set_extack_cookie_u64(struct netlink_ext_ack *extack,

					    u64 cookie)

{

include/net/genetlink.h

+7 −0
Original line number Diff line number Diff line
@@ -110,6 +110,13 @@ static inline void genl_info_net_set(struct genl_info *info, struct net *net) 


#define GENL_SET_ERR_MSG(info, msg) NL_SET_ERR_MSG((info)->extack, msg)



/* Report that a root attribute is missing */

#define GENL_REQ_ATTR_CHECK(info, attr) ({				\

	struct genl_info *__info = (info);				\

									\

	NL_REQ_ATTR_CHECK(__info->extack, NULL, __info->attrs, (attr)); \

})



enum genl_validate_flags {

	GENL_DONT_VALIDATE_STRICT		= BIT(0),

	GENL_DONT_VALIDATE_DUMP			= BIT(1),

include/uapi/linux/netlink.h

+6 −0
Original line number Diff line number Diff line
@@ -140,6 +140,10 @@ struct nlmsgerr { 
 *	be used - in the success case - to identify a created

 *	object or operation or similar (binary)

 * @NLMSGERR_ATTR_POLICY: policy for a rejected attribute

 * @NLMSGERR_ATTR_MISS_TYPE: type of a missing required attribute,

 *	%NLMSGERR_ATTR_MISS_NEST will not be present if the attribute was

 *	missing at the message level

 * @NLMSGERR_ATTR_MISS_NEST: offset of the nest where attribute was missing

 * @__NLMSGERR_ATTR_MAX: number of attributes

 * @NLMSGERR_ATTR_MAX: highest attribute number

 */
@@ -149,6 +153,8 @@ enum nlmsgerr_attrs { 
	NLMSGERR_ATTR_OFFS,

	NLMSGERR_ATTR_COOKIE,

	NLMSGERR_ATTR_POLICY,

	NLMSGERR_ATTR_MISS_TYPE,

	NLMSGERR_ATTR_MISS_NEST,



	__NLMSGERR_ATTR_MAX,

	NLMSGERR_ATTR_MAX = __NLMSGERR_ATTR_MAX - 1

net/core/devlink.c

+20 −21
Original line number Diff line number Diff line
@@ -1710,7 +1710,7 @@ static int devlink_nl_cmd_port_split_doit(struct sk_buff *skb, 
	struct devlink *devlink = info->user_ptr[0];

	u32 count;



	if (!info->attrs[DEVLINK_ATTR_PORT_SPLIT_COUNT])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_PORT_SPLIT_COUNT))

		return -EINVAL;

	if (!devlink->ops->port_split)

		return -EOPNOTSUPP;
@@ -1838,7 +1838,7 @@ static int devlink_nl_cmd_port_del_doit(struct sk_buff *skb, 
	if (!devlink->ops->port_del)

		return -EOPNOTSUPP;



	if (!info->attrs[DEVLINK_ATTR_PORT_INDEX]) {

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_PORT_INDEX)) {

		NL_SET_ERR_MSG_MOD(extack, "Port index is not specified");

		return -EINVAL;

	}
@@ -2690,7 +2690,7 @@ static int devlink_nl_cmd_sb_pool_set_doit(struct sk_buff *skb, 
	if (err)

		return err;



	if (!info->attrs[DEVLINK_ATTR_SB_POOL_SIZE])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_SB_POOL_SIZE))

		return -EINVAL;



	size = nla_get_u32(info->attrs[DEVLINK_ATTR_SB_POOL_SIZE]);
@@ -2900,7 +2900,7 @@ static int devlink_nl_cmd_sb_port_pool_set_doit(struct sk_buff *skb, 
	if (err)

		return err;



	if (!info->attrs[DEVLINK_ATTR_SB_THRESHOLD])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_SB_THRESHOLD))

		return -EINVAL;



	threshold = nla_get_u32(info->attrs[DEVLINK_ATTR_SB_THRESHOLD]);
@@ -3156,7 +3156,7 @@ static int devlink_nl_cmd_sb_tc_pool_bind_set_doit(struct sk_buff *skb, 
	if (err)

		return err;



	if (!info->attrs[DEVLINK_ATTR_SB_THRESHOLD])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_SB_THRESHOLD))

		return -EINVAL;



	threshold = nla_get_u32(info->attrs[DEVLINK_ATTR_SB_THRESHOLD]);
@@ -3845,7 +3845,7 @@ static int devlink_nl_cmd_dpipe_entries_get(struct sk_buff *skb, 
	struct devlink_dpipe_table *table;

	const char *table_name;



	if (!info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_DPIPE_TABLE_NAME))

		return -EINVAL;



	table_name = nla_data(info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME]);
@@ -4029,8 +4029,9 @@ static int devlink_nl_cmd_dpipe_table_counters_set(struct sk_buff *skb, 
	const char *table_name;

	bool counters_enable;



	if (!info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME] ||

	    !info->attrs[DEVLINK_ATTR_DPIPE_TABLE_COUNTERS_ENABLED])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_DPIPE_TABLE_NAME) ||

	    GENL_REQ_ATTR_CHECK(info,

				DEVLINK_ATTR_DPIPE_TABLE_COUNTERS_ENABLED))

		return -EINVAL;



	table_name = nla_data(info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME]);
@@ -4119,8 +4120,8 @@ static int devlink_nl_cmd_resource_set(struct sk_buff *skb, 
	u64 size;

	int err;



	if (!info->attrs[DEVLINK_ATTR_RESOURCE_ID] ||

	    !info->attrs[DEVLINK_ATTR_RESOURCE_SIZE])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_RESOURCE_ID) ||

	    GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_RESOURCE_SIZE))

		return -EINVAL;

	resource_id = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_ID]);
@@ -4821,7 +4822,7 @@ static int devlink_nl_cmd_flash_update(struct sk_buff *skb, 
	if (!devlink->ops->flash_update)

		return -EOPNOTSUPP;



	if (!info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME))

		return -EINVAL;



	ret = devlink_flash_component_get(devlink,
@@ -4998,10 +4999,8 @@ static int devlink_nl_cmd_selftests_run(struct sk_buff *skb, 
	if (!devlink->ops->selftest_run || !devlink->ops->selftest_check)

		return -EOPNOTSUPP;



	if (!info->attrs[DEVLINK_ATTR_SELFTESTS]) {

		NL_SET_ERR_MSG_MOD(info->extack, "selftest required");

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_SELFTESTS))

		return -EINVAL;

	}



	attrs = info->attrs[DEVLINK_ATTR_SELFTESTS];
@@ -5455,7 +5454,7 @@ static int 
devlink_param_type_get_from_info(struct genl_info *info,

				 enum devlink_param_type *param_type)

{

	if (!info->attrs[DEVLINK_ATTR_PARAM_TYPE])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_PARAM_TYPE))

		return -EINVAL;



	switch (nla_get_u8(info->attrs[DEVLINK_ATTR_PARAM_TYPE])) {
@@ -5532,7 +5531,7 @@ devlink_param_get_from_info(struct list_head *param_list, 
{

	char *param_name;



	if (!info->attrs[DEVLINK_ATTR_PARAM_NAME])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_PARAM_NAME))

		return NULL;



	param_name = nla_data(info->attrs[DEVLINK_ATTR_PARAM_NAME]);
@@ -5598,7 +5597,7 @@ static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink, 
			return err;

	}



	if (!info->attrs[DEVLINK_ATTR_PARAM_VALUE_CMODE])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_PARAM_VALUE_CMODE))

		return -EINVAL;

	cmode = nla_get_u8(info->attrs[DEVLINK_ATTR_PARAM_VALUE_CMODE]);

	if (!devlink_param_cmode_is_supported(param, cmode))
@@ -6118,7 +6117,7 @@ static int devlink_nl_cmd_region_get_doit(struct sk_buff *skb, 
	unsigned int index;

	int err;



	if (!info->attrs[DEVLINK_ATTR_REGION_NAME])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_REGION_NAME))

		return -EINVAL;



	if (info->attrs[DEVLINK_ATTR_PORT_INDEX]) {
@@ -6251,8 +6250,8 @@ static int devlink_nl_cmd_region_del(struct sk_buff *skb, 
	unsigned int index;

	u32 snapshot_id;



	if (!info->attrs[DEVLINK_ATTR_REGION_NAME] ||

	    !info->attrs[DEVLINK_ATTR_REGION_SNAPSHOT_ID])

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_REGION_NAME) ||

	    GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_REGION_SNAPSHOT_ID))

		return -EINVAL;



	region_name = nla_data(info->attrs[DEVLINK_ATTR_REGION_NAME]);
@@ -6300,7 +6299,7 @@ devlink_nl_cmd_region_new(struct sk_buff *skb, struct genl_info *info) 
	u8 *data;

	int err;



	if (!info->attrs[DEVLINK_ATTR_REGION_NAME]) {

	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_REGION_NAME)) {

		NL_SET_ERR_MSG_MOD(info->extack, "No region name provided");

		return -EINVAL;

	}