Re: [PATCH net v2] bnxt_en: Fix TX timeout during netpoll.
On Wed, Sep 26, 2018 at 8:49 PM Song Liu wrote: > > We also need this patch from Eric: > > https://marc.info/?l=linux-netdev&m=153780304905946 > I will submit this formally tomorrow, thanks.
Re: [PATCH net v2] bnxt_en: Fix TX timeout during netpoll.
> On Sep 26, 2018, at 8:33 PM, David Miller wrote: > > From: Michael Chan > Date: Wed, 26 Sep 2018 00:41:04 -0400 > >> 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 >> >> Reported-by: Song Liu >> Signed-off-by: Michael Chan > > Applied and queued up for -stable, thanks Michael. Hi David, We also need this patch from Eric: https://marc.info/?l=linux-netdev&m=153780304905946 Thanks, Song
Re: [PATCH net v2] bnxt_en: Fix TX timeout during netpoll.
From: Michael Chan Date: Wed, 26 Sep 2018 00:41:04 -0400 > 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 > > Reported-by: Song Liu > Signed-off-by: Michael Chan Applied and queued up for -stable, thanks Michael.
Re: [PATCH net v2] bnxt_en: Fix TX timeout during netpoll.
> On Sep 25, 2018, at 9:41 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.
>
> 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
>
> Reported-by: Song Liu
> Signed-off-by: Michael Chan
Reviewed-and-tested-by: Song Liu
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 61957b0..0478e56 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -1884,8 +1884,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);
> @@ -1913,7 +1916,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 +2030,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))
> --
> 2.5.1
>
[PATCH net v2] bnxt_en: Fix TX timeout during netpoll.
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
Reported-by: Song Liu
Signed-off-by: Michael Chan
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 61957b0..0478e56 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1884,8 +1884,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);
@@ -1913,7 +1916,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 +2030,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))
--
2.5.1
