| Message ID | 1569824541-5603-1-git-send-email-vasundhara-v.volam@broadcom.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2,net] devlink: Fix error handling in param and info_get dumpit cb | expand |
Mon, Sep 30, 2019 at 08:22:21AM CEST, vasundhara-v.volam@broadcom.com wrote: >If any of the param or info_get op returns error, dumpit cb is >skipping to dump remaining params or info_get ops for all the >drivers. > >Fix to not return if any of the param/info_get op returns error >as not supported and continue to dump remaining information. > >v2: Modify the patch to return error, except for params/info_get >op that return -EOPNOTSUPP as suggested by Andrew Lunn. Also, modify >commit message to reflect the same. > >Cc: Andrew Lunn <andrew@lunn.ch> >Cc: Jiri Pirko <jiri@mellanox.com> >Cc: Michael Chan <michael.chan@broadcom.com> >Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com> Acked-by: Jiri Pirko <jiri@mellanox.com>
On Mon, Sep 30, 2019 at 11:52:21AM +0530, Vasundhara Volam wrote: > If any of the param or info_get op returns error, dumpit cb is > skipping to dump remaining params or info_get ops for all the > drivers. > > Fix to not return if any of the param/info_get op returns error > as not supported and continue to dump remaining information. > > v2: Modify the patch to return error, except for params/info_get > op that return -EOPNOTSUPP as suggested by Andrew Lunn. Also, modify > commit message to reflect the same. > > Cc: Andrew Lunn <andrew@lunn.ch> > Cc: Jiri Pirko <jiri@mellanox.com> > Cc: Michael Chan <michael.chan@broadcom.com> > Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com> > --- > net/core/devlink.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/net/core/devlink.c b/net/core/devlink.c > index e48680e..f80151e 100644 > --- a/net/core/devlink.c > +++ b/net/core/devlink.c > @@ -3172,7 +3172,7 @@ 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) { > + if (err && err != -EOPNOTSUPP) { > mutex_unlock(&devlink->lock); > goto out; > } and out: is out: mutex_unlock(&devlink_mutex); cb->args[0] = idx; return msg->len; } Jiri: Is the intention really to throw away the error? Looking at the rest of devlink, all the other _get_dumpit() functions, except health_reporter_dump_get_dumpit(), do discard any errors. As for this patch Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
Mon, Sep 30, 2019 at 02:10:03PM CEST, andrew@lunn.ch wrote: >On Mon, Sep 30, 2019 at 11:52:21AM +0530, Vasundhara Volam wrote: >> If any of the param or info_get op returns error, dumpit cb is >> skipping to dump remaining params or info_get ops for all the >> drivers. >> >> Fix to not return if any of the param/info_get op returns error >> as not supported and continue to dump remaining information. >> >> v2: Modify the patch to return error, except for params/info_get >> op that return -EOPNOTSUPP as suggested by Andrew Lunn. Also, modify >> commit message to reflect the same. >> >> Cc: Andrew Lunn <andrew@lunn.ch> >> Cc: Jiri Pirko <jiri@mellanox.com> >> Cc: Michael Chan <michael.chan@broadcom.com> >> Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com> >> --- >> net/core/devlink.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/net/core/devlink.c b/net/core/devlink.c >> index e48680e..f80151e 100644 >> --- a/net/core/devlink.c >> +++ b/net/core/devlink.c >> @@ -3172,7 +3172,7 @@ 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) { >> + if (err && err != -EOPNOTSUPP) { >> mutex_unlock(&devlink->lock); >> goto out; >> } > >and out: is > >out: > mutex_unlock(&devlink_mutex); > > cb->args[0] = idx; > return msg->len; >} > >Jiri: Is the intention really to throw away the error? > >Looking at the rest of devlink, all the other _get_dumpit() functions, >except health_reporter_dump_get_dumpit(), do discard any errors. You are correct. The -EMSGSIZE dump errors should not be propagaged out and -EOPNOTSUPP, but the rest should. I'll look into it. Thanks! > >As for this patch > >Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > Andrew
From: Vasundhara Volam <vasundhara-v.volam@broadcom.com> Date: Mon, 30 Sep 2019 11:52:21 +0530 > If any of the param or info_get op returns error, dumpit cb is > skipping to dump remaining params or info_get ops for all the > drivers. > > Fix to not return if any of the param/info_get op returns error > as not supported and continue to dump remaining information. > > v2: Modify the patch to return error, except for params/info_get > op that return -EOPNOTSUPP as suggested by Andrew Lunn. Also, modify > commit message to reflect the same. > > Cc: Andrew Lunn <andrew@lunn.ch> > Cc: Jiri Pirko <jiri@mellanox.com> > Cc: Michael Chan <michael.chan@broadcom.com> > Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com> Applied, thanks.
diff --git a/net/core/devlink.c b/net/core/devlink.c index e48680e..f80151e 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -3172,7 +3172,7 @@ 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) { + if (err && err != -EOPNOTSUPP) { mutex_unlock(&devlink->lock); goto out; } @@ -3432,7 +3432,7 @@ 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) { + if (err && err != -EOPNOTSUPP) { mutex_unlock(&devlink->lock); goto out; } @@ -4088,7 +4088,7 @@ 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) + if (err && err != -EOPNOTSUPP) break; idx++; }
If any of the param or info_get op returns error, dumpit cb is skipping to dump remaining params or info_get ops for all the drivers. Fix to not return if any of the param/info_get op returns error as not supported and continue to dump remaining information. v2: Modify the patch to return error, except for params/info_get op that return -EOPNOTSUPP as suggested by Andrew Lunn. Also, modify commit message to reflect the same. Cc: Andrew Lunn <andrew@lunn.ch> Cc: Jiri Pirko <jiri@mellanox.com> Cc: Michael Chan <michael.chan@broadcom.com> Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com> --- net/core/devlink.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)