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

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

[Li Zhong]

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

Signed-off-by: Li Zhong <floridsleeves@gmail.com>
---
 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

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

[Tony Nguyen]

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 <floridsleeves@gmail.com>
> ---
>   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;
>   	}
>   }

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

[Li Zhong]

On Mon, Sep 12, 2022 at 3:08 PM Tony Nguyen <anthony.l.nguyen@intel.com> 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 <floridsleeves@gmail.com>
> > ---
> >   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;
> >       }
> >   }