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

[Stefan Bader]

On 01.03.19 09:06, Juerg Haefliger wrote:
> 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.

Were told (maybe you were not close by at that point) that git send-email was
not set up on laptop. So used ...<something else> and not only messed up the
patch but also had the cover totally separated from the actual patch.

We might have better chances re-constructing this by taking the upstream patch
and modify the file paths. Given where we are in time...

-Stefan
> 
> 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: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20190301/afd316ce/attachment.sig>