* [PATCH net 0/3] ethtool: fail with error if request has unknown flags
@ 2020-03-15 17:17 Michal Kubecek
2020-03-15 17:17 ` [PATCH net 1/3] netlink: allow extack cookie also for error messages Michal Kubecek
` (3 more replies)
0 siblings, 4 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
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).
Michal Kubecek (3):
netlink: allow extack cookie also for error messages
netlink: add nl_set_extack_cookie_u32()
ethtool: reject unrecognized request flags
include/linux/netlink.h | 9 +++++++++
net/ethtool/netlink.c | 16 +++++++++++----
net/netlink/af_netlink.c | 43 ++++++++++++++++------------------------
3 files changed, 38 insertions(+), 30 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [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 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
* 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
end of thread, other threads:[~2020-03-16 9:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH net 3/3] ethtool: reject unrecognized request flags 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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).