* [PATCH net] devlink: ignore -EOPNOTSUPP errors on dumpit
@ 2020-07-28 23:15 Jakub Kicinski
2020-07-28 23:21 ` Jacob Keller
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Jakub Kicinski @ 2020-07-28 23:15 UTC (permalink / raw)
To: davem; +Cc: netdev, andrew, jiri, kernel-team, Jakub Kicinski
Number of .dumpit functions try to ignore -EOPNOTSUPP errors.
Recent change missed that, and started reporting all errors
but -EMSGSIZE back from dumps. This leads to situation like
this:
$ devlink dev info
devlink answers: Operation not supported
Dump should not report an error just because the last device
to be queried could not provide an answer.
To fix this and avoid similar confusion make sure we clear
err properly, and not leave it set to an error if we don't
terminate the iteration.
Fixes: c62c2cfb801b ("net: devlink: don't ignore errors during dumpit")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
net/core/devlink.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 2cafbc808b09..1d38b6651b23 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1065,7 +1065,9 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
devlink_sb,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq);
- if (err && err != -EOPNOTSUPP) {
+ if (err == -EOPNOTSUPP) {
+ err = 0;
+ } else if (err) {
mutex_unlock(&devlink->lock);
goto out;
}
@@ -1266,7 +1268,9 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
devlink, devlink_sb,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq);
- if (err && err != -EOPNOTSUPP) {
+ if (err == -EOPNOTSUPP) {
+ err = 0;
+ } else if (err) {
mutex_unlock(&devlink->lock);
goto out;
}
@@ -1498,7 +1502,9 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
devlink_sb,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq);
- if (err && err != -EOPNOTSUPP) {
+ if (err == -EOPNOTSUPP) {
+ err = 0;
+ } else if (err) {
mutex_unlock(&devlink->lock);
goto out;
}
@@ -3299,7 +3305,9 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
NLM_F_MULTI);
- if (err && err != -EOPNOTSUPP) {
+ if (err == -EOPNOTSUPP) {
+ err = 0;
+ } else if (err) {
mutex_unlock(&devlink->lock);
goto out;
}
@@ -3569,7 +3577,9 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
NLM_F_MULTI);
- if (err && err != -EOPNOTSUPP) {
+ if (err == -EOPNOTSUPP) {
+ err = 0;
+ } else if (err) {
mutex_unlock(&devlink->lock);
goto out;
}
@@ -4518,7 +4528,9 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
cb->nlh->nlmsg_seq, NLM_F_MULTI,
cb->extack);
mutex_unlock(&devlink->lock);
- if (err && err != -EOPNOTSUPP)
+ if (err == -EOPNOTSUPP)
+ err = 0;
+ else if (err)
break;
idx++;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net] devlink: ignore -EOPNOTSUPP errors on dumpit
2020-07-28 23:15 [PATCH net] devlink: ignore -EOPNOTSUPP errors on dumpit Jakub Kicinski
@ 2020-07-28 23:21 ` Jacob Keller
2020-07-29 8:21 ` Jiri Pirko
2020-07-30 23:52 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Jacob Keller @ 2020-07-28 23:21 UTC (permalink / raw)
To: Jakub Kicinski, davem; +Cc: netdev, andrew, jiri, kernel-team
On 7/28/2020 4:15 PM, Jakub Kicinski wrote:
> Number of .dumpit functions try to ignore -EOPNOTSUPP errors.
> Recent change missed that, and started reporting all errors
> but -EMSGSIZE back from dumps. This leads to situation like
> this:
>
> $ devlink dev info
> devlink answers: Operation not supported
>
> Dump should not report an error just because the last device
> to be queried could not provide an answer.
>
> To fix this and avoid similar confusion make sure we clear
> err properly, and not leave it set to an error if we don't
> terminate the iteration.
>
Makes sense to me.
> Fixes: c62c2cfb801b ("net: devlink: don't ignore errors during dumpit")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> net/core/devlink.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 2cafbc808b09..1d38b6651b23 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -1065,7 +1065,9 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
> devlink_sb,
> NETLINK_CB(cb->skb).portid,
> cb->nlh->nlmsg_seq);
> - if (err && err != -EOPNOTSUPP) {
> + if (err == -EOPNOTSUPP) {
> + err = 0;
> + } else if (err) {
> mutex_unlock(&devlink->lock);
> goto out;
> }
> @@ -1266,7 +1268,9 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
> devlink, devlink_sb,
> NETLINK_CB(cb->skb).portid,
> cb->nlh->nlmsg_seq);
> - if (err && err != -EOPNOTSUPP) {
> + if (err == -EOPNOTSUPP) {
> + err = 0;
> + } else if (err) {
> mutex_unlock(&devlink->lock);
> goto out;
> }
> @@ -1498,7 +1502,9 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
> devlink_sb,
> NETLINK_CB(cb->skb).portid,
> cb->nlh->nlmsg_seq);
> - if (err && err != -EOPNOTSUPP) {
> + if (err == -EOPNOTSUPP) {
> + err = 0;
> + } else if (err) {
> mutex_unlock(&devlink->lock);
> goto out;
> }
> @@ -3299,7 +3305,9 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
> NETLINK_CB(cb->skb).portid,
> cb->nlh->nlmsg_seq,
> NLM_F_MULTI);
> - if (err && err != -EOPNOTSUPP) {
> + if (err == -EOPNOTSUPP) {
> + err = 0;
> + } else if (err) {
> mutex_unlock(&devlink->lock);
> goto out;
> }
> @@ -3569,7 +3577,9 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
> NETLINK_CB(cb->skb).portid,
> cb->nlh->nlmsg_seq,
> NLM_F_MULTI);
> - if (err && err != -EOPNOTSUPP) {
> + if (err == -EOPNOTSUPP) {
> + err = 0;
> + } else if (err) {
> mutex_unlock(&devlink->lock);
> goto out;
> }
> @@ -4518,7 +4528,9 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
> cb->nlh->nlmsg_seq, NLM_F_MULTI,
> cb->extack);
> mutex_unlock(&devlink->lock);
> - if (err && err != -EOPNOTSUPP)
> + if (err == -EOPNOTSUPP)
> + err = 0;
> + else if (err)
> break;
> idx++;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] devlink: ignore -EOPNOTSUPP errors on dumpit
2020-07-28 23:15 [PATCH net] devlink: ignore -EOPNOTSUPP errors on dumpit Jakub Kicinski
2020-07-28 23:21 ` Jacob Keller
@ 2020-07-29 8:21 ` Jiri Pirko
2020-07-30 23:52 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Jiri Pirko @ 2020-07-29 8:21 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, netdev, andrew, jiri, kernel-team
Wed, Jul 29, 2020 at 01:15:07AM CEST, kuba@kernel.org wrote:
>Number of .dumpit functions try to ignore -EOPNOTSUPP errors.
>Recent change missed that, and started reporting all errors
>but -EMSGSIZE back from dumps. This leads to situation like
>this:
>
>$ devlink dev info
>devlink answers: Operation not supported
>
>Dump should not report an error just because the last device
>to be queried could not provide an answer.
>
>To fix this and avoid similar confusion make sure we clear
>err properly, and not leave it set to an error if we don't
>terminate the iteration.
>
>Fixes: c62c2cfb801b ("net: devlink: don't ignore errors during dumpit")
>Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Yeah, that makes perfect sense. Thanks for the fix Kuba!
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] devlink: ignore -EOPNOTSUPP errors on dumpit
2020-07-28 23:15 [PATCH net] devlink: ignore -EOPNOTSUPP errors on dumpit Jakub Kicinski
2020-07-28 23:21 ` Jacob Keller
2020-07-29 8:21 ` Jiri Pirko
@ 2020-07-30 23:52 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2020-07-30 23:52 UTC (permalink / raw)
To: kuba; +Cc: netdev, andrew, jiri, kernel-team
From: Jakub Kicinski <kuba@kernel.org>
Date: Tue, 28 Jul 2020 16:15:07 -0700
> Number of .dumpit functions try to ignore -EOPNOTSUPP errors.
> Recent change missed that, and started reporting all errors
> but -EMSGSIZE back from dumps. This leads to situation like
> this:
>
> $ devlink dev info
> devlink answers: Operation not supported
>
> Dump should not report an error just because the last device
> to be queried could not provide an answer.
>
> To fix this and avoid similar confusion make sure we clear
> err properly, and not leave it set to an error if we don't
> terminate the iteration.
>
> Fixes: c62c2cfb801b ("net: devlink: don't ignore errors during dumpit")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-30 23:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-28 23:15 [PATCH net] devlink: ignore -EOPNOTSUPP errors on dumpit Jakub Kicinski
2020-07-28 23:21 ` Jacob Keller
2020-07-29 8:21 ` Jiri Pirko
2020-07-30 23:52 ` 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).