Skip to contentSkip to conversation

vhost/net: Fix receive queue test in busy poll

PATCH In review raoxu 3 participants 3 messages Nov 10 – Nov 11

Subsystems (5)
Networking Network drivers KVM virtio core +1 more subsystems
lore
Share & tools
lore

raoxu

author
#0 lore raw

PATCH 1/1 vhost/net: Fix receive queue test in busy poll

From: Xu Rao <[email protected]>

sock_has_rx_data() returns a positive result when receive data is available. Socket implementations with a peek_len() callback follow that convention, but the fallback for sockets without one returns skb_queue_empty(), which has the opposite meaning.

This makes busy polling stop when a raw packet socket has no queued data and fail to recognize queued data through the fallback path. Return the negated queue-empty result so the fallback matches the callback and its callers.

Signed-off-by Xu Rao <[email protected]>
---
 drivers/vhost/net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/vhost/net.c +1 -1
index 4570a4ffbab2..10ed2d55ff07 100644
@@ -538,7 +538,7 @@ static int sock_has_rx_data(struct socket *sock)
 	if (sock->ops->peek_len)
 		return sock->ops->peek_len(sock);
 
-	return skb_queue_empty(&sock->sk->sk_receive_queue);
+	return !skb_queue_empty(&sock->sk->sk_receive_queue);
 }
 
 static void vhost_net_busy_poll_try_queue(struct vhost_net *net,

--
2.50.1

AI review
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Low] vhost/net: missing Fixes trailer for sock_has_rx_data() bug fix --

Thank you for your contribution! Sashiko AI review found 1 potential issue to consider:
- [Low] vhost/net: missing Fixes trailer for sock_has_rx_data() bug fix --

commit e704f9e65e5ab760d5f0c63a3cc9fef9370b05b7
Author: Xu Rao <[email protected]>

Quoted 7 lines

vhost/net: Fix receive queue test in busy poll

sock_has_rx_data() returns a positive result when receive data is available. Socket implementations with a peek_len() callback follow that convention, but the fallback for sockets without one returns skb_queue_empty(), which has the opposite meaning.

Quoted 6 lines

This makes busy polling stop when a raw packet socket has no queued data and fail to recognize queued data through the fallback path. Return the negated queue-empty result so the fallback matches the callback and its callers.

Signed-off-by Xu Rao <[email protected]>

[Severity: Low]
The inverted skb_queue_empty() return value was introduced by 1576d9860599 ("tun: switch to use skb array for tx") and later moved to sock_has_rx_data() by dc151282bbda ("net: vhost: factor out busy polling logic to vhost_net_busy_poll()"). Should this include a Fixes tag, such as:

Fixes 1576d9860599 ("tun: switch to use skb array for tx")
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#2 lore raw
In reply to raoxu
Quoted 7 lines · raoxu

From: Xu Rao <[email protected]>

sock_has_rx_data() returns a positive result when receive data is available. Socket implementations with a peek_len() callback follow that convention, but the fallback for sockets without one returns skb_queue_empty(), which has the opposite meaning.

Quoted 6 lines

This makes busy polling stop when a raw packet socket has no queued data and fail to recognize queued data through the fallback path. Return the negated queue-empty result so the fallback matches the callback and its callers.

Signed-off-by Xu Rao <[email protected]>

This fix is valid, but you need to carry a Fixes: tag. You should also state what netdev tree you're targeting. Please see Documentation/process/maintainer-netdev.rst

Could the changelog also state how the bug was found and confirmed, and how the fix was tested?

Björn