virtio_vsock_skb_rx_put() only calls skb_put() if the length in the
packet header is not zero even though skb_put() handles this case
gracefully.

Remove the functionally redundant check from virtio_vsock_skb_rx_put()
and, on the assumption that this is a worthwhile optimisation for
handling credit messages, augment the existing length checks in
virtio_transport_rx_work() to elide the call for zero-length payloads.
Note that the vhost code already has similar logic in
vhost_vsock_alloc_skb().

Signed-off-by: Will Deacon <will@kernel.org>
---
 include/linux/virtio_vsock.h     | 4 +---
 net/vmw_vsock/virtio_transport.c | 4 +++-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 36fb3edfa403..eb6980aa19fd 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -52,9 +52,7 @@ static inline void virtio_vsock_skb_rx_put(struct sk_buff *skb)
 	u32 len;
 
 	len = le32_to_cpu(virtio_vsock_hdr(skb)->len);
-
-	if (len > 0)
-		skb_put(skb, len);
+	skb_put(skb, len);
 }
 
 static inline struct sk_buff *virtio_vsock_alloc_skb(unsigned int size, gfp_t mask)
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index bd2c6aaa1a93..488e6ddc6ffa 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -656,7 +656,9 @@ static void virtio_transport_rx_work(struct work_struct *work)
 				continue;
 			}
 
-			virtio_vsock_skb_rx_put(skb);
+			if (payload_len)
+				virtio_vsock_skb_rx_put(skb);
+
 			virtio_transport_deliver_tap_pkt(skb);
 			virtio_transport_recv_pkt(&virtio_transport, skb);
 		}
-- 
2.50.0.727.gbf7dc18ff4-goog

On Tue, Jul 01, 2025 at 05:45:02PM +0100, Will Deacon wrote:
>virtio_vsock_skb_rx_put() only calls skb_put() if the length in the
>packet header is not zero even though skb_put() handles this case
>gracefully.
>
>Remove the functionally redundant check from virtio_vsock_skb_rx_put()
>and, on the assumption that this is a worthwhile optimisation for
>handling credit messages, augment the existing length checks in
>virtio_transport_rx_work() to elide the call for zero-length payloads.
>Note that the vhost code already has similar logic in
>vhost_vsock_alloc_skb().
>
>Signed-off-by: Will Deacon <will@kernel.org>
>---
> include/linux/virtio_vsock.h     | 4 +---
> net/vmw_vsock/virtio_transport.c | 4 +++-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index 36fb3edfa403..eb6980aa19fd 100644
>--- a/include/linux/virtio_vsock.h
>+++ b/include/linux/virtio_vsock.h
>@@ -52,9 +52,7 @@ static inline void virtio_vsock_skb_rx_put(struct sk_buff *skb)
> 	u32 len;
>
> 	len = le32_to_cpu(virtio_vsock_hdr(skb)->len);
>-
>-	if (len > 0)
>-		skb_put(skb, len);
>+	skb_put(skb, len);

Since the caller is supposed to check the len, can we just pass it as 
parameter?

So we can avoid the `le32_to_cpu(virtio_vsock_hdr(skb)->len)` here.

Thanks,
Stefano

> }
>
> static inline struct sk_buff *virtio_vsock_alloc_skb(unsigned int size, gfp_t mask)
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index bd2c6aaa1a93..488e6ddc6ffa 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -656,7 +656,9 @@ static void virtio_transport_rx_work(struct work_struct *work)
> 				continue;
> 			}
>
>-			virtio_vsock_skb_rx_put(skb);
>+			if (payload_len)
>+				virtio_vsock_skb_rx_put(skb);
>+
> 			virtio_transport_deliver_tap_pkt(skb);
> 			virtio_transport_recv_pkt(&virtio_transport, skb);
> 		}
>-- 
>2.50.0.727.gbf7dc18ff4-goog
>

On Wed, Jul 02, 2025 at 06:28:37PM +0200, Stefano Garzarella wrote:
> On Tue, Jul 01, 2025 at 05:45:02PM +0100, Will Deacon wrote:
> > virtio_vsock_skb_rx_put() only calls skb_put() if the length in the
> > packet header is not zero even though skb_put() handles this case
> > gracefully.
> > 
> > Remove the functionally redundant check from virtio_vsock_skb_rx_put()
> > and, on the assumption that this is a worthwhile optimisation for
> > handling credit messages, augment the existing length checks in
> > virtio_transport_rx_work() to elide the call for zero-length payloads.
> > Note that the vhost code already has similar logic in
> > vhost_vsock_alloc_skb().
> > 
> > Signed-off-by: Will Deacon <will@kernel.org>
> > ---
> > include/linux/virtio_vsock.h     | 4 +---
> > net/vmw_vsock/virtio_transport.c | 4 +++-
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> > index 36fb3edfa403..eb6980aa19fd 100644
> > --- a/include/linux/virtio_vsock.h
> > +++ b/include/linux/virtio_vsock.h
> > @@ -52,9 +52,7 @@ static inline void virtio_vsock_skb_rx_put(struct sk_buff *skb)
> > 	u32 len;
> > 
> > 	len = le32_to_cpu(virtio_vsock_hdr(skb)->len);
> > -
> > -	if (len > 0)
> > -		skb_put(skb, len);
> > +	skb_put(skb, len);
> 
> Since the caller is supposed to check the len, can we just pass it as
> parameter?
> 
> So we can avoid the `le32_to_cpu(virtio_vsock_hdr(skb)->len)` here.

Sure, I'll do that. It means that virtio_vsock_skb_rx_put() will briefly
be a simple wrapper around skb_put() but once the non-linear handling
comes in then it becomes useful again.

Will