[PATCH] net: remove local_bh_enable during busy poll

[PATCH] net: remove local_bh_enable during busy poll

[Longjun Tang]

From: Longjun Tang <tanglongjun@kylinos.cn>

When CONFIG_NET_RX_BUSY_POLL==Y and net.core.busy_read > 0,
the __napi_busy_loop function calls napi_poll to perform busy polling,
such as in the case of virtio_net's virnet_poll. If interrupts are enabled
during the busy polling process, it is possible that data has already been
received and that last_used_idx is updated before the interrupt is handled.
This can lead to the vring_interrupt returning IRQ_NONE in response to the
interrupt because used_idx == last_used_idx, which is considered a spurious
interrupt.Once certain conditions are met, this interrupt can be disabled.

The local_bh_enable during the busy polling process allows softirq to be
executed and interrupt notification to be enabled. Removing local_bh_enable
will significantly reduce the number of unhandled interrupts.

Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
---
 net/core/dev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 5a3c0f40a93f..f5737c551666 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6763,12 +6763,12 @@ static void __napi_busy_loop(unsigned int napi_id,
 					LINUX_MIB_BUSYPOLLRXPACKETS, work);
 		skb_defer_free_flush(this_cpu_ptr(&softnet_data));
 		bpf_net_ctx_clear(bpf_net_ctx);
-		local_bh_enable();
 
 		if (!loop_end || loop_end(loop_end_arg, start_time))
 			break;
 
 		if (unlikely(need_resched())) {
+			local_bh_enable();
 			if (flags & NAPI_F_END_ON_RESCHED)
 				break;
 			if (napi_poll)
@@ -6784,6 +6784,7 @@ static void __napi_busy_loop(unsigned int napi_id,
 		}
 		cpu_relax();
 	}
+	local_bh_enable();
 	if (napi_poll)
 		busy_poll_stop(napi, have_poll_lock, flags, budget);
 	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
-- 
2.48.1

Re: [PATCH] net: remove local_bh_enable during busy poll

[Jakub Kicinski]

On Fri, 29 Aug 2025 11:04:56 +0800 Longjun Tang wrote:
> When CONFIG_NET_RX_BUSY_POLL==Y and net.core.busy_read > 0,
> the __napi_busy_loop function calls napi_poll to perform busy polling,
> such as in the case of virtio_net's virnet_poll. If interrupts are enabled
> during the busy polling process, it is possible that data has already been
> received and that last_used_idx is updated before the interrupt is handled.
> This can lead to the vring_interrupt returning IRQ_NONE in response to the
> interrupt because used_idx == last_used_idx, which is considered a spurious
> interrupt.Once certain conditions are met, this interrupt can be disabled.

I'm not sure this patch completely fixes the issue you're describing.
It just makes it less likely to happen. Really, it feels like the onus
for fixing this is on the driver that can't discern its own IRQ sources.
-- 
pw-bot: cr

Re:Re: [PATCH] net: remove local_bh_enable during busy poll

[Lange Tang]

Thanks for your reply!

I've done some testing, pps=350000，net.core.busy_read=50.

Before apply this patch: unhandled ≈ 6400/s
After apply this patch: unhandled < 10/s

As you said, the driver needs to discern spurious interrupts in above describing situation, which I strongly agree with. and I also think that it's necessary to remove local_bh_enable during busy polling, as it causes interrupts to be enabled during the busy poll.

I think fix this issue requires two patches, in addition to this patch, another patch is needed from the driver side to discern spurious interrupts.






At 2025-09-02 04:23:30, "Jakub Kicinski" <kuba@kernel.org> wrote:
>On Fri, 29 Aug 2025 11:04:56 +0800 Longjun Tang wrote:
>> When CONFIG_NET_RX_BUSY_POLL==Y and net.core.busy_read > 0,
>> the __napi_busy_loop function calls napi_poll to perform busy polling,
>> such as in the case of virtio_net's virnet_poll. If interrupts are enabled
>> during the busy polling process, it is possible that data has already been
>> received and that last_used_idx is updated before the interrupt is handled.
>> This can lead to the vring_interrupt returning IRQ_NONE in response to the
>> interrupt because used_idx == last_used_idx, which is considered a spurious
>> interrupt.Once certain conditions are met, this interrupt can be disabled.
>
>I'm not sure this patch completely fixes the issue you're describing.
>It just makes it less likely to happen. Really, it feels like the onus
>for fixing this is on the driver that can't discern its own IRQ sources.
>-- 
>pw-bot: cr

Re: Re: [PATCH] net: remove local_bh_enable during busy poll

[Eric Dumazet]

On Tue, Sep 2, 2025 at 1:53 AM Lange Tang <lange_tang@163.com> wrote:
>
> Thanks for your reply!
>
> I've done some testing, pps=350000，net.core.busy_read=50.

We can not let __napi_busy_loop() run for thousands of usec while blocking BH,
if net.core.busy_read=5000

To me, your patch is a no go.

Something is wrong in virtqueue_napi_complete() I think.


>
> Before apply this patch: unhandled ≈ 6400/s
> After apply this patch: unhandled < 10/s
>
> As you said, the driver needs to discern spurious interrupts in above describing situation, which I strongly agree with. and I also think that it's necessary to remove local_bh_enable during busy polling, as it causes interrupts to be enabled during the busy poll.
>
> I think fix this issue requires two patches, in addition to this patch, another patch is needed from the driver side to discern spurious interrupts.
>
>
>
>
>
>
> At 2025-09-02 04:23:30, "Jakub Kicinski" <kuba@kernel.org> wrote:
> >On Fri, 29 Aug 2025 11:04:56 +0800 Longjun Tang wrote:
> >> When CONFIG_NET_RX_BUSY_POLL==Y and net.core.busy_read > 0,
> >> the __napi_busy_loop function calls napi_poll to perform busy polling,
> >> such as in the case of virtio_net's virnet_poll. If interrupts are enabled
> >> during the busy polling process, it is possible that data has already been
> >> received and that last_used_idx is updated before the interrupt is handled.
> >> This can lead to the vring_interrupt returning IRQ_NONE in response to the
> >> interrupt because used_idx == last_used_idx, which is considered a spurious
> >> interrupt.Once certain conditions are met, this interrupt can be disabled.
> >
> >I'm not sure this patch completely fixes the issue you're describing.
> >It just makes it less likely to happen. Really, it feels like the onus
> >for fixing this is on the driver that can't discern its own IRQ sources.
> >--
> >pw-bot: cr