netdev - Re: [PATCH net RFT] bnxt_en: Fix TX timeout during netpoll.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c496aa4a-0fc1-9305-abfa-c287680e944a@gmail.com>
Date:   Tue, 25 Sep 2018 15:15:43 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Michael Chan <michael.chan@...adcom.com>, songliubraving@...com,
        edumazet@...gle.com, davem@...emloft.net
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net RFT] bnxt_en: Fix TX timeout during netpoll.



On 09/25/2018 02:31 PM, Michael Chan wrote:
> The current netpoll implementation in the bnxt_en driver has problems
> that may miss TX completion events.  bnxt_poll_work() in effect is
> only handling at most 1 TX packet before exiting.  In addition,
> there may be in flight TX completions that ->poll() may miss even
> after we fix bnxt_poll_work() to handle all visible TX completions.
> netpoll may not call ->poll() again and HW may not generate IRQ
> because the driver does not ARM the IRQ when the budget (0 for netpoll)
> is reached.
> 
> We fix it by handling all TX completions and to always ARM the IRQ
> when we exit ->poll() with 0 budget.
> 
> Reported-by: Song Liu <songliubraving@...com>
> Signed-off-by: Michael Chan <michael.chan@...adcom.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 61957b0..c981b53 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -1913,7 +1913,7 @@ static int bnxt_poll_work(struct bnxt *bp, struct bnxt_napi *bnapi, int budget)
>  		}
>  		raw_cons = NEXT_RAW_CMP(raw_cons);
>  
> -		if (rx_pkts == budget)
> +		if (rx_pkts && rx_pkts == budget)
>  			break;
>  	}
>  
> @@ -2027,8 +2027,12 @@ static int bnxt_poll(struct napi_struct *napi, int budget)
>  	while (1) {
>  		work_done += bnxt_poll_work(bp, bnapi, budget - work_done);
>  
> -		if (work_done >= budget)
> +		if (work_done >= budget) {
> +			if (!budget)
> +				BNXT_CP_DB_REARM(cpr->cp_doorbell,
> +						 cpr->cp_raw_cons);
>  			break;
> +		}
>  
>  		if (!bnxt_has_work(bp, cpr)) {
>  			if (napi_complete_done(napi, work_done))
> 

Hi Michael, thanks for the patch.

It seems bnx2 should have a similar issue ?