* [PATCH net 1/3] netlink: allow extack cookie also for error messages
2020-03-15 17:17 [PATCH net 0/3] ethtool: fail with error if request has unknown flags Michal Kubecek
@ 2020-03-15 17:17 ` Michal Kubecek
2020-03-15 17:17 ` [PATCH net 2/3] netlink: add nl_set_extack_cookie_u32() Michal Kubecek
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Michal Kubecek @ 2020-03-15 17:17 UTC (permalink / raw)
To: David Miller, Jakub Kicinski, netdev; +Cc: Johannes Berg, linux-kernel
Commit ba0dc5f6e0ba ("netlink: allow sending extended ACK with cookie on
success") introduced a cookie which can be sent to userspace as part of
extended ack message in the form of NLMSGERR_ATTR_COOKIE attribute.
Currently the cookie is ignored if error code is non-zero but there is
no technical reason for such limitation and it can be useful to provide
machine parseable information as part of an error message.
Include NLMSGERR_ATTR_COOKIE whenever the cookie has been set,
regardless of error code.
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
net/netlink/af_netlink.c | 43 ++++++++++++++++------------------------
1 file changed, 17 insertions(+), 26 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 5313f1cec170..2f234791b879 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2392,19 +2392,14 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
if (nlk_has_extack && extack && extack->_msg)
tlvlen += nla_total_size(strlen(extack->_msg) + 1);
- if (err) {
- if (!(nlk->flags & NETLINK_F_CAP_ACK))
- payload += nlmsg_len(nlh);
- else
- flags |= NLM_F_CAPPED;
- if (nlk_has_extack && extack && extack->bad_attr)
- tlvlen += nla_total_size(sizeof(u32));
- } else {
+ if (err && !(nlk->flags & NETLINK_F_CAP_ACK))
+ payload += nlmsg_len(nlh);
+ else
flags |= NLM_F_CAPPED;
-
- if (nlk_has_extack && extack && extack->cookie_len)
- tlvlen += nla_total_size(extack->cookie_len);
- }
+ if (err && nlk_has_extack && extack && extack->bad_attr)
+ tlvlen += nla_total_size(sizeof(u32));
+ if (nlk_has_extack && extack && extack->cookie_len)
+ tlvlen += nla_total_size(extack->cookie_len);
if (tlvlen)
flags |= NLM_F_ACK_TLVS;
@@ -2427,20 +2422,16 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG,
extack->_msg));
}
- if (err) {
- if (extack->bad_attr &&
- !WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
- (u8 *)extack->bad_attr >= in_skb->data +
- in_skb->len))
- WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS,
- (u8 *)extack->bad_attr -
- (u8 *)nlh));
- } else {
- if (extack->cookie_len)
- WARN_ON(nla_put(skb, NLMSGERR_ATTR_COOKIE,
- extack->cookie_len,
- extack->cookie));
- }
+ if (err && extack->bad_attr &&
+ !WARN_ON((u8 *)extack->bad_attr < in_skb->data ||
+ (u8 *)extack->bad_attr >= in_skb->data +
+ in_skb->len))
+ WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS,
+ (u8 *)extack->bad_attr -
+ (u8 *)nlh));
+ if (extack->cookie_len)
+ WARN_ON(nla_put(skb, NLMSGERR_ATTR_COOKIE,
+ extack->cookie_len, extack->cookie));
}
nlmsg_end(skb, rep);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 2/3] netlink: add nl_set_extack_cookie_u32()
2020-03-15 17:17 [PATCH net 0/3] ethtool: fail with error if request has unknown flags Michal Kubecek
2020-03-15 17:17 ` [PATCH net 1/3] netlink: allow extack cookie also for error messages Michal Kubecek
@ 2020-03-15 17:17 ` Michal Kubecek
2020-03-15 17:17 ` [PATCH net 3/3] ethtool: reject unrecognized request flags Michal Kubecek
2020-03-16 9:04 ` [PATCH net 0/3] ethtool: fail with error if request has unknown flags David Miller
3 siblings, 0 replies; 6+ messages in thread
From: Michal Kubecek @ 2020-03-15 17:17 UTC (permalink / raw)
To: David Miller, Jakub Kicinski, netdev; +Cc: Johannes Berg, linux-kernel
Similar to existing nl_set_extack_cookie_u64(), add new helper
nl_set_extack_cookie_u32() which sets extack cookie to a u32 value.
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
include/linux/netlink.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 205fa7b1f07a..4090524c3462 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -119,6 +119,15 @@ static inline void nl_set_extack_cookie_u64(struct netlink_ext_ack *extack,
extack->cookie_len = sizeof(__cookie);
}
+static inline void nl_set_extack_cookie_u32(struct netlink_ext_ack *extack,
+ u32 cookie)
+{
+ u32 __cookie = cookie;
+
+ memcpy(extack->cookie, &__cookie, sizeof(__cookie));
+ extack->cookie_len = sizeof(__cookie);
+}
+
void netlink_kernel_release(struct sock *sk);
int __netlink_change_ngroups(struct sock *sk, unsigned int groups);
int netlink_change_ngroups(struct sock *sk, unsigned int groups);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 3/3] ethtool: reject unrecognized request flags
2020-03-15 17:17 [PATCH net 0/3] ethtool: fail with error if request has unknown flags Michal Kubecek
2020-03-15 17:17 ` [PATCH net 1/3] netlink: allow extack cookie also for error messages Michal Kubecek
2020-03-15 17:17 ` [PATCH net 2/3] netlink: add nl_set_extack_cookie_u32() Michal Kubecek
@ 2020-03-15 17:17 ` Michal Kubecek
2020-03-16 9:39 ` Sergei Shtylyov
2020-03-16 9:04 ` [PATCH net 0/3] ethtool: fail with error if request has unknown flags David Miller
3 siblings, 1 reply; 6+ messages in thread
From: Michal Kubecek @ 2020-03-15 17:17 UTC (permalink / raw)
To: David Miller, Jakub Kicinski, netdev; +Cc: Johannes Berg, linux-kernel
As pointed out by Jakub Kicinski, we ethtool netlink code should respond
with an error if request head has flags set which are not recognized by
kernel, either as a mistake or because it expects functionality introduced
in later kernel versions.
To avoid unnecessary roundtrips, use extack cookie to provide the
information about supported request flags.
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
net/ethtool/netlink.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 180c194fab07..fc9e0b806889 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -40,6 +40,7 @@ int ethnl_parse_header(struct ethnl_req_info *req_info,
struct nlattr *tb[ETHTOOL_A_HEADER_MAX + 1];
const struct nlattr *devname_attr;
struct net_device *dev = NULL;
+ u32 flags = 0;
int ret;
if (!header) {
@@ -50,8 +51,17 @@ int ethnl_parse_header(struct ethnl_req_info *req_info,
ethnl_header_policy, extack);
if (ret < 0)
return ret;
- devname_attr = tb[ETHTOOL_A_HEADER_DEV_NAME];
+ if (tb[ETHTOOL_A_HEADER_FLAGS]) {
+ flags = nla_get_u32(tb[ETHTOOL_A_HEADER_FLAGS]);
+ if (flags & ~ETHTOOL_FLAG_ALL) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[ETHTOOL_A_HEADER_FLAGS],
+ "unrecognized request flags");
+ nl_set_extack_cookie_u32(extack, ETHTOOL_FLAG_ALL);
+ return -EOPNOTSUPP;
+ }
+ }
+ devname_attr = tb[ETHTOOL_A_HEADER_DEV_NAME];
if (tb[ETHTOOL_A_HEADER_DEV_INDEX]) {
u32 ifindex = nla_get_u32(tb[ETHTOOL_A_HEADER_DEV_INDEX]);
@@ -90,9 +100,7 @@ int ethnl_parse_header(struct ethnl_req_info *req_info,
}
req_info->dev = dev;
- if (tb[ETHTOOL_A_HEADER_FLAGS])
- req_info->flags = nla_get_u32(tb[ETHTOOL_A_HEADER_FLAGS]);
-
+ req_info->flags = flags;
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net 3/3] ethtool: reject unrecognized request flags
2020-03-15 17:17 ` [PATCH net 3/3] ethtool: reject unrecognized request flags Michal Kubecek
@ 2020-03-16 9:39 ` Sergei Shtylyov
0 siblings, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2020-03-16 9:39 UTC (permalink / raw)
To: Michal Kubecek, David Miller, Jakub Kicinski, netdev
Cc: Johannes Berg, linux-kernel
Hello!
On 15.03.2020 20:17, Michal Kubecek wrote:
> As pointed out by Jakub Kicinski, we ethtool netlink code should respond
s/we/the/?
> with an error if request head has flags set which are not recognized by
> kernel, either as a mistake or because it expects functionality introduced
> in later kernel versions.
>
> To avoid unnecessary roundtrips, use extack cookie to provide the
> information about supported request flags.
>
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 0/3] ethtool: fail with error if request has unknown flags
2020-03-15 17:17 [PATCH net 0/3] ethtool: fail with error if request has unknown flags Michal Kubecek
` (2 preceding siblings ...)
2020-03-15 17:17 ` [PATCH net 3/3] ethtool: reject unrecognized request flags Michal Kubecek
@ 2020-03-16 9:04 ` David Miller
3 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-03-16 9:04 UTC (permalink / raw)
To: mkubecek; +Cc: kuba, netdev, johannes, linux-kernel
From: Michal Kubecek <mkubecek@suse.cz>
Date: Sun, 15 Mar 2020 18:17:38 +0100 (CET)
> Jakub Kicinski pointed out that if unrecognized flags are set in netlink
> header request, kernel shoud fail with an error rather than silently
> ignore them so that we have more freedom in future flags semantics.
>
> To help userspace with handling such errors, inform the client which
> flags are supported by kernel. For that purpose, we need to allow
> passing cookies as part of extack also in case of error (they can be
> only passed on success now).
Series applied, thanks Michal.
^ permalink raw reply [flat|nested] 6+ messages in thread