* [PATCH net] devlink: Fix error handling in param and info_get dumpit cb
@ 2019-09-26 9:35 Vasundhara Volam
2019-09-26 12:27 ` Andrew Lunn
0 siblings, 1 reply; 6+ messages in thread
From: Vasundhara Volam @ 2019-09-26 9:35 UTC (permalink / raw)
To: davem; +Cc: netdev, Vasundhara Volam, Jiri Pirko, Michael Chan
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.
Instead skip only for the param/info_get op which returned error
and continue to dump remaining information, except if the return
code is EMSGSIZE.
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..a1dd1b8 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 == -EMSGSIZE) {
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 == -EMSGSIZE) {
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 == -EMSGSIZE)
break;
idx++;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net] devlink: Fix error handling in param and info_get dumpit cb
2019-09-26 9:35 [PATCH net] devlink: Fix error handling in param and info_get dumpit cb Vasundhara Volam
@ 2019-09-26 12:27 ` Andrew Lunn
2019-09-27 4:58 ` Vasundhara Volam
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2019-09-26 12:27 UTC (permalink / raw)
To: Vasundhara Volam; +Cc: davem, netdev, Jiri Pirko, Michael Chan
On Thu, Sep 26, 2019 at 03:05:54PM +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.
>
> Instead skip only for the param/info_get op which returned error
> and continue to dump remaining information, except if the return
> code is EMSGSIZE.
Hi Vasundhara
How do we get to see something did fail? If it failed, it failed for a
reason, and we want to know.
What is your real use case here? What is failing, and why are you
O.K. to skip this failure?
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] devlink: Fix error handling in param and info_get dumpit cb
2019-09-26 12:27 ` Andrew Lunn
@ 2019-09-27 4:58 ` Vasundhara Volam
2019-09-27 12:31 ` Andrew Lunn
0 siblings, 1 reply; 6+ messages in thread
From: Vasundhara Volam @ 2019-09-27 4:58 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, Netdev, Jiri Pirko, Michael Chan
On Thu, Sep 26, 2019 at 5:57 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Thu, Sep 26, 2019 at 03:05:54PM +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.
> >
> > Instead skip only for the param/info_get op which returned error
> > and continue to dump remaining information, except if the return
> > code is EMSGSIZE.
>
> Hi Vasundhara
>
> How do we get to see something did fail? If it failed, it failed for a
> reason, and we want to know.
>
> What is your real use case here? What is failing, and why are you
> O.K. to skip this failure?
>
> Andrew
Hi Andrew,
Thank you for looking into the patch.
If any of the devlink parameter is returning error like EINVAL, then
current code is not displaying any further parameters for all the other
devices as well.
In bnxt_en driver case, some of the parameters are not supported in
certain configurations like if the parameter is not part of the
NVM configuration, driver returns EINVAL error to the stack. And devlink is
skipping to display all the remaining parameters for that device and others
as well.
I am trying to fix to skip only the error parameter and display the remaining
parameters.
Thanks,
Vasundhara
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] devlink: Fix error handling in param and info_get dumpit cb
2019-09-27 4:58 ` Vasundhara Volam
@ 2019-09-27 12:31 ` Andrew Lunn
2019-09-27 13:08 ` Jiri Pirko
2019-09-27 13:37 ` Vasundhara Volam
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Lunn @ 2019-09-27 12:31 UTC (permalink / raw)
To: Vasundhara Volam; +Cc: David Miller, Netdev, Jiri Pirko, Michael Chan
On Fri, Sep 27, 2019 at 10:28:36AM +0530, Vasundhara Volam wrote:
> On Thu, Sep 26, 2019 at 5:57 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Thu, Sep 26, 2019 at 03:05:54PM +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.
> > >
> > > Instead skip only for the param/info_get op which returned error
> > > and continue to dump remaining information, except if the return
> > > code is EMSGSIZE.
> >
> > Hi Vasundhara
> >
> > How do we get to see something did fail? If it failed, it failed for a
> > reason, and we want to know.
> >
> > What is your real use case here? What is failing, and why are you
> > O.K. to skip this failure?
> >
> > Andrew
> Hi Andrew,
>
> Thank you for looking into the patch.
>
> If any of the devlink parameter is returning error like EINVAL, then
> current code is not displaying any further parameters for all the other
> devices as well.
>
> In bnxt_en driver case, some of the parameters are not supported in
> certain configurations like if the parameter is not part of the
> NVM configuration, driver returns EINVAL error to the stack. And devlink is
> skipping to display all the remaining parameters for that device and others
> as well.
>
> I am trying to fix to skip only the error parameter and display the remaining
> parameters.
Hi Vasundhara
Thanks for explaining your use case. It sounds sensible. But i would
narrow this down.
Make the driver return EOPNOTSUP, not EINVAL. And then in dump, only
skip EOPNOTSUP. Any other errors cause the error to be returned, so we
get to see them.
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] devlink: Fix error handling in param and info_get dumpit cb
2019-09-27 12:31 ` Andrew Lunn
@ 2019-09-27 13:08 ` Jiri Pirko
2019-09-27 13:37 ` Vasundhara Volam
1 sibling, 0 replies; 6+ messages in thread
From: Jiri Pirko @ 2019-09-27 13:08 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vasundhara Volam, David Miller, Netdev, Jiri Pirko, Michael Chan
Fri, Sep 27, 2019 at 02:31:29PM CEST, andrew@lunn.ch wrote:
>On Fri, Sep 27, 2019 at 10:28:36AM +0530, Vasundhara Volam wrote:
>> On Thu, Sep 26, 2019 at 5:57 PM Andrew Lunn <andrew@lunn.ch> wrote:
>> >
>> > On Thu, Sep 26, 2019 at 03:05:54PM +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.
>> > >
>> > > Instead skip only for the param/info_get op which returned error
>> > > and continue to dump remaining information, except if the return
>> > > code is EMSGSIZE.
>> >
>> > Hi Vasundhara
>> >
>> > How do we get to see something did fail? If it failed, it failed for a
>> > reason, and we want to know.
>> >
>> > What is your real use case here? What is failing, and why are you
>> > O.K. to skip this failure?
>> >
>> > Andrew
>> Hi Andrew,
>>
>> Thank you for looking into the patch.
>>
>> If any of the devlink parameter is returning error like EINVAL, then
>> current code is not displaying any further parameters for all the other
>> devices as well.
>>
>> In bnxt_en driver case, some of the parameters are not supported in
>> certain configurations like if the parameter is not part of the
>> NVM configuration, driver returns EINVAL error to the stack. And devlink is
>> skipping to display all the remaining parameters for that device and others
>> as well.
>>
>> I am trying to fix to skip only the error parameter and display the remaining
>> parameters.
>
>Hi Vasundhara
>
>Thanks for explaining your use case. It sounds sensible. But i would
>narrow this down.
>
>Make the driver return EOPNOTSUP, not EINVAL. And then in dump, only
>skip EOPNOTSUP. Any other errors cause the error to be returned, so we
>get to see them.
Agreed, that would be more reasonable.
>
> Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] devlink: Fix error handling in param and info_get dumpit cb
2019-09-27 12:31 ` Andrew Lunn
2019-09-27 13:08 ` Jiri Pirko
@ 2019-09-27 13:37 ` Vasundhara Volam
1 sibling, 0 replies; 6+ messages in thread
From: Vasundhara Volam @ 2019-09-27 13:37 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, Netdev, Jiri Pirko, Michael Chan
On Fri, Sep 27, 2019 at 6:01 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Fri, Sep 27, 2019 at 10:28:36AM +0530, Vasundhara Volam wrote:
> > On Thu, Sep 26, 2019 at 5:57 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > On Thu, Sep 26, 2019 at 03:05:54PM +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.
> > > >
> > > > Instead skip only for the param/info_get op which returned error
> > > > and continue to dump remaining information, except if the return
> > > > code is EMSGSIZE.
> > >
> > > Hi Vasundhara
> > >
> > > How do we get to see something did fail? If it failed, it failed for a
> > > reason, and we want to know.
> > >
> > > What is your real use case here? What is failing, and why are you
> > > O.K. to skip this failure?
> > >
> > > Andrew
> > Hi Andrew,
> >
> > Thank you for looking into the patch.
> >
> > If any of the devlink parameter is returning error like EINVAL, then
> > current code is not displaying any further parameters for all the other
> > devices as well.
> >
> > In bnxt_en driver case, some of the parameters are not supported in
> > certain configurations like if the parameter is not part of the
> > NVM configuration, driver returns EINVAL error to the stack. And devlink is
> > skipping to display all the remaining parameters for that device and others
> > as well.
> >
> > I am trying to fix to skip only the error parameter and display the remaining
> > parameters.
>
> Hi Vasundhara
>
> Thanks for explaining your use case. It sounds sensible. But i would
> narrow this down.
>
> Make the driver return EOPNOTSUP, not EINVAL. And then in dump, only
> skip EOPNOTSUP. Any other errors cause the error to be returned, so we
> get to see them.
Thank you Andrew, I will modify the patch and resubmit.
>
> Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-27 13:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-26 9:35 [PATCH net] devlink: Fix error handling in param and info_get dumpit cb Vasundhara Volam
2019-09-26 12:27 ` Andrew Lunn
2019-09-27 4:58 ` Vasundhara Volam
2019-09-27 12:31 ` Andrew Lunn
2019-09-27 13:08 ` Jiri Pirko
2019-09-27 13:37 ` Vasundhara Volam
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).