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

[Juerg Haefliger]

On Thu, 28 Feb 2019 20:33:06 -0500
Khaled Elmously <khalid.elmously at canonical.com> wrote:

> Nivedita, I'm having trouble working with this patch from my inbox. 
> Could you please use git-send-email instead of Thunderbird to re-send it? (or just send a pull-request)

Same here, something's funny about this email:

Applying: UBUNTU: SAUCE: bnxt_en_bpo: Fix TX timeout during netpoll
error: corrupt patch at line 6

How did you generate these emails? Also, the patch mails are usually
in-reply to the cover letter (patch 0/x) so that it's all contained within a
thread.

More comments below.


> Thanks
> 
> On 2019-02-22 11:20:29 , 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)

No newline please. That just makes parsing harder. And there is no need for
'upstream' if the commit is from Linus' tree. We only add a reference here if
the commit comes from a different tree, like linux-next or linux-stable.
And (not strictly mandatory) a quick note why this needed backporting and
doesn't cherry-pick cleanly would help. In the form of:
[<your nick>: <your comment>]

So something like:
(backported from commit 73f21c653f930f438d53eed29b5e4c65c8a0f906)
[niv: Patch is for ubuntu/bnxt driver, adjusted file paths accordingly.]
Signed-off-by: Nivedita Singhvi <nivedita.singhvi at canonical.com>

Thanks
...Juerg


> > Signed-off-by: Nivedita Singhvi <nivedita.singhvi at canonical.com>
> > 
> > 
> > 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
> > 
> > -- 
> > kernel-team mailing list
> > kernel-team at lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team  
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20190301/5b934a07/attachment.sig>