ACK/Cmnt: [SRU X] [PATCH 1/1] UBUNTU: SAUCE: bnxt_en_bpo: Fix TX timeout during netpoll

[Stefan Bader]

On 22.02.19 11:20, Nivedita Singhvi wrote:
> From: Michael Chan <michael.chan at broadcom.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1814095
> 
> 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. Also, the logic to ACK the
> completion ring in case it is almost filled with TX completions need
> to be adjusted to take care of the 0 budget case, as discussed with
> Eric Dumazet <edumazet at google.com>
> 
> Reported-by: Song Liu <songliubraving at fb.com>
> Reviewed-by: Song Liu <songliubraving at fb.com>
> Tested-by: Song Liu <songliubraving at fb.com>
> Signed-off-by: Michael Chan <michael.chan at broadcom.com>
> Signed-off-by: David S. Miller <davem at davemloft.net>
> (backported from commit 73f21c653f930f438d53eed29b5e4c65c8a0f906
>  upstream)
> Signed-off-by: Nivedita Singhvi <nivedita.singhvi at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> 
--

So lets users have this confirmed with the backport. We just need to be careful
when ever pulling in a new backported driver.

Nivedita, the bug report has the devel (disco) kernel set to confirmed, still.
Is that true (as in 4.19 or later affected)? Otherwise the main task could be
set to invalid, keeping only the xenial task open.

-Stefan

> 
> diff --git a/ubuntu/bnxt/bnxt.c b/ubuntu/bnxt/bnxt.c
> index f04db69..9b51120 100644
> --- a/ubuntu/bnxt/bnxt.c
> +++ b/ubuntu/bnxt/bnxt.c
> @@ -2006,8 +2006,11 @@ static int bnxt_poll_work(struct bnxt *bp, struct
> bnxt_napi *bnapi, int budget)
>  		if (TX_CMP_TYPE(txcmp) == CMP_TYPE_TX_L2_CMP) {
>  			tx_pkts++;
>  			/* return full budget so NAPI will complete. */
> -			if (unlikely(tx_pkts > bp->tx_wake_thresh))
> +			if (unlikely(tx_pkts > bp->tx_wake_thresh)) {
>  				rx_pkts = budget;
> +				raw_cons = NEXT_RAW_CMP(raw_cons);
> +				break;
> +			}
>  		} else if ((TX_CMP_TYPE(txcmp) & 0x30) == 0x10) {
>  			if (likely(budget))
>  				rc = bnxt_rx_pkt(bp, bnapi, &raw_cons, &event);
> @@ -2028,7 +2031,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;
>  	}
> 
> @@ -2156,8 +2159,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)) {
>  #ifdef HAVE_NEW_NAPI_COMPLETE_DONE
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20190226/1798497b/attachment.sig>