2022-09-09 04:21:15

by Li Zhong

Subject: [PATCH net-next v1] drivers/net/ethernet/intel/e100: check the return value of e100_exec_cmd()

Check the return value of e100_exec_cmd() which could return error code
when execution fails.

Signed-off-by: Li Zhong <[email protected]>
---
drivers/net/ethernet/intel/e100.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index 11a884aa5082..3b84745376fe 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -1911,7 +1911,8 @@ static inline void e100_start_receiver(struct nic *nic, struct rx *rx)

/* (Re)start RU if suspended or idle and RFA is non-NULL */
if (rx->skb) {
- e100_exec_cmd(nic, ruc_start, rx->dma_addr);
+ if (!e100_exec_cmd(nic, ruc_start, rx->dma_addr))
+ return;
nic->ru_running = RU_RUNNING;
}
}
--
2.25.1

2022-09-12 22:16:06

by Tony Nguyen

Subject: Re: [PATCH net-next v1] drivers/net/ethernet/intel/e100: check the return value of e100_exec_cmd()

On 9/8/2022 9:16 PM, Li Zhong wrote:
> Check the return value of e100_exec_cmd() which could return error code
> when execution fails.

Are you coming across this as a real bug or as something reported by
static analysis? If the latter, I suggest checking the return value and
reporting it as debug, however, not changing existing behavior. We don't
have validation on this driver so there is limited ability to check for
regressions and the code has been like this for a long time without
reported issues.

Thanks,
Tony

> Signed-off-by: Li Zhong <[email protected]>
> ---
> drivers/net/ethernet/intel/e100.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
> index 11a884aa5082..3b84745376fe 100644
> --- a/drivers/net/ethernet/intel/e100.c
> +++ b/drivers/net/ethernet/intel/e100.c
> @@ -1911,7 +1911,8 @@ static inline void e100_start_receiver(struct nic *nic, struct rx *rx)
>
> /* (Re)start RU if suspended or idle and RFA is non-NULL */
> if (rx->skb) {
> - e100_exec_cmd(nic, ruc_start, rx->dma_addr);
> + if (!e100_exec_cmd(nic, ruc_start, rx->dma_addr))
> + return;
> nic->ru_running = RU_RUNNING;
> }
> }

2022-09-17 00:09:22

by Li Zhong

Subject: Re: [PATCH net-next v1] drivers/net/ethernet/intel/e100: check the return value of e100_exec_cmd()

On Mon, Sep 12, 2022 at 3:08 PM Tony Nguyen <[email protected]> wrote:
>
> On 9/8/2022 9:16 PM, Li Zhong wrote:
> > Check the return value of e100_exec_cmd() which could return error code
> > when execution fails.
>
> Are you coming across this as a real bug or as something reported by
> static analysis? If the latter, I suggest checking the return value and
> reporting it as debug, however, not changing existing behavior. We don't
> have validation on this driver so there is limited ability to check for
> regressions and the code has been like this for a long time without
> reported issues.

Thanks for replying and suggestions! This is detected by static analysis.
I submit a v2 patch that fixes it as debug printk.

>
> Thanks,
> Tony
>
> > Signed-off-by: Li Zhong <[email protected]>
> > ---
> > drivers/net/ethernet/intel/e100.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
> > index 11a884aa5082..3b84745376fe 100644
> > --- a/drivers/net/ethernet/intel/e100.c
> > +++ b/drivers/net/ethernet/intel/e100.c
> > @@ -1911,7 +1911,8 @@ static inline void e100_start_receiver(struct nic *nic, struct rx *rx)
> >
> > /* (Re)start RU if suspended or idle and RFA is non-NULL */
> > if (rx->skb) {
> > - e100_exec_cmd(nic, ruc_start, rx->dma_addr);
> > + if (!e100_exec_cmd(nic, ruc_start, rx->dma_addr))
> > + return;
> > nic->ru_running = RU_RUNNING;
> > }
> > }