lkml.org 
[lkml]   [2022]   [Mar]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.15 079/110] net: macb: Fix lost RX packet wakeup race in NAPI receive
    Date
    From: Robert Hancock <robert.hancock@calian.com>

    commit 0bf476fc3624e3a72af4ba7340d430a91c18cd67 upstream.

    There is an oddity in the way the RSR register flags propagate to the
    ISR register (and the actual interrupt output) on this hardware: it
    appears that RSR register bits only result in ISR being asserted if the
    interrupt was actually enabled at the time, so enabling interrupts with
    RSR bits already set doesn't trigger an interrupt to be raised. There
    was already a partial fix for this race in the macb_poll function where
    it checked for RSR bits being set and re-triggered NAPI receive.
    However, there was a still a race window between checking RSR and
    actually enabling interrupts, where a lost wakeup could happen. It's
    necessary to check again after enabling interrupts to see if RSR was set
    just prior to the interrupt being enabled, and re-trigger receive in that
    case.

    This issue was noticed in a point-to-point UDP request-response protocol
    which periodically saw timeouts or abnormally high response times due to
    received packets not being processed in a timely fashion. In many
    applications, more packets arriving, including TCP retransmissions, would
    cause the original packet to be processed, thus masking the issue.

    Fixes: 02f7a34f34e3 ("net: macb: Re-enable RX interrupt only when RX is done")
    Cc: stable@vger.kernel.org
    Co-developed-by: Scott McNutt <scott.mcnutt@siriusxm.com>
    Signed-off-by: Scott McNutt <scott.mcnutt@siriusxm.com>
    Signed-off-by: Robert Hancock <robert.hancock@calian.com>
    Tested-by: Claudiu Beznea <claudiu.beznea@microchip.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/net/ethernet/cadence/macb_main.c | 25 ++++++++++++++++++++++++-
    1 file changed, 24 insertions(+), 1 deletion(-)

    --- a/drivers/net/ethernet/cadence/macb_main.c
    +++ b/drivers/net/ethernet/cadence/macb_main.c
    @@ -1606,7 +1606,14 @@ static int macb_poll(struct napi_struct
    if (work_done < budget) {
    napi_complete_done(napi, work_done);

    - /* Packets received while interrupts were disabled */
    + /* RSR bits only seem to propagate to raise interrupts when
    + * interrupts are enabled at the time, so if bits are already
    + * set due to packets received while interrupts were disabled,
    + * they will not cause another interrupt to be generated when
    + * interrupts are re-enabled.
    + * Check for this case here. This has been seen to happen
    + * around 30% of the time under heavy network load.
    + */
    status = macb_readl(bp, RSR);
    if (status) {
    if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
    @@ -1614,6 +1621,22 @@ static int macb_poll(struct napi_struct
    napi_reschedule(napi);
    } else {
    queue_writel(queue, IER, bp->rx_intr_mask);
    +
    + /* In rare cases, packets could have been received in
    + * the window between the check above and re-enabling
    + * interrupts. Therefore, a double-check is required
    + * to avoid losing a wakeup. This can potentially race
    + * with the interrupt handler doing the same actions
    + * if an interrupt is raised just after enabling them,
    + * but this should be harmless.
    + */
    + status = macb_readl(bp, RSR);
    + if (unlikely(status)) {
    + queue_writel(queue, IDR, bp->rx_intr_mask);
    + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
    + queue_writel(queue, ISR, MACB_BIT(RCOMP));
    + napi_schedule(napi);
    + }
    }
    }


    \
     
     \ /
      Last update: 2022-03-14 13:26    [W:2.787 / U:24.488 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site