| Message ID | 20200728231507.426387-1-kuba@kernel.org |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] devlink: ignore -EOPNOTSUPP errors on dumpit | expand |
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++; > } >
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>
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.
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++; }
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(-)