| Message ID | AS2P194MB2170BA1D5A32BDF70C4547B19A3E2@AS2P194MB2170.EURP194.PROD.OUTLOOK.COM (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ioctl support for AF_VSOCK and virtio-based transports | expand |
On Tue, Apr 02, 2024 at 05:05:38PM +0200, Luigi Leonardi wrote: >This patch introduce support for stream_bytes_unsent in all >virtio based transports: virtio-transport, vhost-vsock and >vsock-loopback > >For all transports the unsent bytes counter is incremented >in virtio_transport_send_pkt_info. > >In the virtio-transport (G2H) the counter is decremented each time the host >notifies the guest that it consumed the skbuffs. >In vhost-vsock (H2G) the counter is decremented after the skbuff is queued >in the virtqueue. >In vsock-loopback the counter is decremented after the skbuff is >dequeued. > >Signed-off-by: Luigi Leonardi <luigi.leonardi@outlook.com> >--- > drivers/vhost/vsock.c | 3 ++- > include/linux/virtio_vsock.h | 7 ++++++ > net/vmw_vsock/virtio_transport.c | 3 ++- > net/vmw_vsock/virtio_transport_common.c | 30 +++++++++++++++++++++++++ > net/vmw_vsock/vsock_loopback.c | 6 +++++ > 5 files changed, 47 insertions(+), 2 deletions(-) > >diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c >index ec20ecff85c7..9732ab944e5b 100644 >--- a/drivers/vhost/vsock.c >+++ b/drivers/vhost/vsock.c >@@ -244,7 +244,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock, > restart_tx = true; > } > >- consume_skb(skb); >+ virtio_transport_consume_skb_sent(skb, true); > } > } while(likely(!vhost_exceeds_weight(vq, ++pkts, total_len))); > if (added) >@@ -430,6 +430,7 @@ static struct virtio_transport vhost_transport = { > .stream_rcvhiwat = virtio_transport_stream_rcvhiwat, > .stream_is_active = virtio_transport_stream_is_active, > .stream_allow = virtio_transport_stream_allow, >+ .stream_bytes_unsent = virtio_transport_bytes_unsent, > > .seqpacket_dequeue = virtio_transport_seqpacket_dequeue, > .seqpacket_enqueue = virtio_transport_seqpacket_enqueue, >diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h >index c82089dee0c8..cdce8b051f98 100644 >--- a/include/linux/virtio_vsock.h >+++ b/include/linux/virtio_vsock.h >@@ -134,6 +134,8 @@ struct virtio_vsock_sock { > u32 peer_fwd_cnt; > u32 peer_buf_alloc; > >+ atomic_t bytes_unsent; >+ > /* Protected by rx_lock */ > u32 fwd_cnt; > u32 last_fwd_cnt; >@@ -193,6 +195,11 @@ s64 virtio_transport_stream_has_data(struct vsock_sock *vsk); > s64 virtio_transport_stream_has_space(struct vsock_sock *vsk); > u32 virtio_transport_seqpacket_has_data(struct vsock_sock *vsk); > >+int virtio_transport_bytes_unsent(struct vsock_sock *vsk); >+ >+void virtio_transport_consume_skb_sent(struct sk_buff *skb, >+ bool dealloc); >+ > int virtio_transport_do_socket_init(struct vsock_sock *vsk, > struct vsock_sock *psk); > int >diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c >index 1748268e0694..d3dd0d49c2b3 100644 >--- a/net/vmw_vsock/virtio_transport.c >+++ b/net/vmw_vsock/virtio_transport.c >@@ -310,7 +310,7 @@ static void virtio_transport_tx_work(struct work_struct *work) > > virtqueue_disable_cb(vq); > while ((skb = virtqueue_get_buf(vq, &len)) != NULL) { >- consume_skb(skb); >+ virtio_transport_consume_skb_sent(skb, true); > added = true; > } > } while (!virtqueue_enable_cb(vq)); >@@ -518,6 +518,7 @@ static struct virtio_transport virtio_transport = { > .stream_rcvhiwat = virtio_transport_stream_rcvhiwat, > .stream_is_active = virtio_transport_stream_is_active, > .stream_allow = virtio_transport_stream_allow, >+ .stream_bytes_unsent = virtio_transport_bytes_unsent, > > .seqpacket_dequeue = virtio_transport_seqpacket_dequeue, > .seqpacket_enqueue = virtio_transport_seqpacket_enqueue, >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >index 16ff976a86e3..3a08e720aa9c 100644 >--- a/net/vmw_vsock/virtio_transport_common.c >+++ b/net/vmw_vsock/virtio_transport_common.c >@@ -419,6 +419,9 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk, > */ > rest_len -= ret; > >+ if (info->op == VIRTIO_VSOCK_OP_RW) >+ atomic_add(ret, &vvs->bytes_unsent); >+ > if (WARN_ONCE(ret != skb_len, > "'send_pkt()' returns %i, but %zu expected\n", > ret, skb_len)) >@@ -463,6 +466,24 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff * > } > EXPORT_SYMBOL_GPL(virtio_transport_inc_tx_pkt); > >+void virtio_transport_consume_skb_sent(struct sk_buff *skb, bool dealloc) >+{ >+ struct sock *s = skb->sk; >+ >+ if (s) { >+ struct vsock_sock *vs = vsock_sk(s); >+ struct virtio_vsock_sock *vvs; >+ >+ vvs = vs->trans; >+ if (skb->len) >+ atomic_sub(skb->len, &vvs->bytes_unsent); We incremented it only for VIRTIO_VSOCK_OP_RW, should we check the same here? >+ } >+ >+ if (dealloc) ^ What about rename it in `consume`? The rest LGTM. Thanks, Stefano >+ consume_skb(skb); >+} >+EXPORT_SYMBOL_GPL(virtio_transport_consume_skb_sent); >+ > u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 credit) > { > u32 ret; >@@ -891,6 +912,7 @@ int virtio_transport_do_socket_init(struct vsock_sock *vsk, > vsk->buffer_size = VIRTIO_VSOCK_MAX_BUF_SIZE; > > vvs->buf_alloc = vsk->buffer_size; >+ atomic_set(&vvs->bytes_unsent, 0); > > spin_lock_init(&vvs->rx_lock); > spin_lock_init(&vvs->tx_lock); >@@ -1090,6 +1112,14 @@ void virtio_transport_destruct(struct vsock_sock *vsk) > } > EXPORT_SYMBOL_GPL(virtio_transport_destruct); > >+int virtio_transport_bytes_unsent(struct vsock_sock *vsk) >+{ >+ struct virtio_vsock_sock *vvs = vsk->trans; >+ >+ return atomic_read(&vvs->bytes_unsent); >+} >+EXPORT_SYMBOL_GPL(virtio_transport_bytes_unsent); >+ > static int virtio_transport_reset(struct vsock_sock *vsk, > struct sk_buff *skb) > { >diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c >index 6dea6119f5b2..35fd4e47b5bf 100644 >--- a/net/vmw_vsock/vsock_loopback.c >+++ b/net/vmw_vsock/vsock_loopback.c >@@ -77,6 +77,7 @@ static struct virtio_transport loopback_transport = { > .stream_rcvhiwat = virtio_transport_stream_rcvhiwat, > .stream_is_active = virtio_transport_stream_is_active, > .stream_allow = virtio_transport_stream_allow, >+ .stream_bytes_unsent = virtio_transport_bytes_unsent, > > .seqpacket_dequeue = virtio_transport_seqpacket_dequeue, > .seqpacket_enqueue = virtio_transport_seqpacket_enqueue, >@@ -123,6 +124,11 @@ static void vsock_loopback_work(struct work_struct *work) > spin_unlock_bh(&vsock->pkt_queue.lock); > > while ((skb = __skb_dequeue(&pkts))) { >+ /* Decrement the bytes_sent counter without deallocating skb >+ * It is freed by the receiver. >+ */ >+ virtio_transport_consume_skb_sent(skb, false); >+ > virtio_transport_deliver_tap_pkt(skb); > virtio_transport_recv_pkt(&loopback_transport, skb); > } >-- >2.34.1 > >
Hi Stefano, Thanks for the review. >We incremented it only for VIRTIO_VSOCK_OP_RW, should we check the >same here? Checking for the length of the skbuf or checking for VIRTIO_VSOCK_OP_RW AFAIK is equivalent. Since the only packet with payload is this one. I'll add a comment in v2 to make it more clear. >What about rename it in `consume`? Sounds good! Thanks, Luigi
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c index ec20ecff85c7..9732ab944e5b 100644 --- a/drivers/vhost/vsock.c +++ b/drivers/vhost/vsock.c @@ -244,7 +244,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock, restart_tx = true; } - consume_skb(skb); + virtio_transport_consume_skb_sent(skb, true); } } while(likely(!vhost_exceeds_weight(vq, ++pkts, total_len))); if (added) @@ -430,6 +430,7 @@ static struct virtio_transport vhost_transport = { .stream_rcvhiwat = virtio_transport_stream_rcvhiwat, .stream_is_active = virtio_transport_stream_is_active, .stream_allow = virtio_transport_stream_allow, + .stream_bytes_unsent = virtio_transport_bytes_unsent, .seqpacket_dequeue = virtio_transport_seqpacket_dequeue, .seqpacket_enqueue = virtio_transport_seqpacket_enqueue, diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h index c82089dee0c8..cdce8b051f98 100644 --- a/include/linux/virtio_vsock.h +++ b/include/linux/virtio_vsock.h @@ -134,6 +134,8 @@ struct virtio_vsock_sock { u32 peer_fwd_cnt; u32 peer_buf_alloc; + atomic_t bytes_unsent; + /* Protected by rx_lock */ u32 fwd_cnt; u32 last_fwd_cnt; @@ -193,6 +195,11 @@ s64 virtio_transport_stream_has_data(struct vsock_sock *vsk); s64 virtio_transport_stream_has_space(struct vsock_sock *vsk); u32 virtio_transport_seqpacket_has_data(struct vsock_sock *vsk); +int virtio_transport_bytes_unsent(struct vsock_sock *vsk); + +void virtio_transport_consume_skb_sent(struct sk_buff *skb, + bool dealloc); + int virtio_transport_do_socket_init(struct vsock_sock *vsk, struct vsock_sock *psk); int diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c index 1748268e0694..d3dd0d49c2b3 100644 --- a/net/vmw_vsock/virtio_transport.c +++ b/net/vmw_vsock/virtio_transport.c @@ -310,7 +310,7 @@ static void virtio_transport_tx_work(struct work_struct *work) virtqueue_disable_cb(vq); while ((skb = virtqueue_get_buf(vq, &len)) != NULL) { - consume_skb(skb); + virtio_transport_consume_skb_sent(skb, true); added = true; } } while (!virtqueue_enable_cb(vq)); @@ -518,6 +518,7 @@ static struct virtio_transport virtio_transport = { .stream_rcvhiwat = virtio_transport_stream_rcvhiwat, .stream_is_active = virtio_transport_stream_is_active, .stream_allow = virtio_transport_stream_allow, + .stream_bytes_unsent = virtio_transport_bytes_unsent, .seqpacket_dequeue = virtio_transport_seqpacket_dequeue, .seqpacket_enqueue = virtio_transport_seqpacket_enqueue, diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 16ff976a86e3..3a08e720aa9c 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -419,6 +419,9 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk, */ rest_len -= ret; + if (info->op == VIRTIO_VSOCK_OP_RW) + atomic_add(ret, &vvs->bytes_unsent); + if (WARN_ONCE(ret != skb_len, "'send_pkt()' returns %i, but %zu expected\n", ret, skb_len)) @@ -463,6 +466,24 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff * } EXPORT_SYMBOL_GPL(virtio_transport_inc_tx_pkt); +void virtio_transport_consume_skb_sent(struct sk_buff *skb, bool dealloc) +{ + struct sock *s = skb->sk; + + if (s) { + struct vsock_sock *vs = vsock_sk(s); + struct virtio_vsock_sock *vvs; + + vvs = vs->trans; + if (skb->len) + atomic_sub(skb->len, &vvs->bytes_unsent); + } + + if (dealloc) + consume_skb(skb); +} +EXPORT_SYMBOL_GPL(virtio_transport_consume_skb_sent); + u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 credit) { u32 ret; @@ -891,6 +912,7 @@ int virtio_transport_do_socket_init(struct vsock_sock *vsk, vsk->buffer_size = VIRTIO_VSOCK_MAX_BUF_SIZE; vvs->buf_alloc = vsk->buffer_size; + atomic_set(&vvs->bytes_unsent, 0); spin_lock_init(&vvs->rx_lock); spin_lock_init(&vvs->tx_lock); @@ -1090,6 +1112,14 @@ void virtio_transport_destruct(struct vsock_sock *vsk) } EXPORT_SYMBOL_GPL(virtio_transport_destruct); +int virtio_transport_bytes_unsent(struct vsock_sock *vsk) +{ + struct virtio_vsock_sock *vvs = vsk->trans; + + return atomic_read(&vvs->bytes_unsent); +} +EXPORT_SYMBOL_GPL(virtio_transport_bytes_unsent); + static int virtio_transport_reset(struct vsock_sock *vsk, struct sk_buff *skb) { diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c index 6dea6119f5b2..35fd4e47b5bf 100644 --- a/net/vmw_vsock/vsock_loopback.c +++ b/net/vmw_vsock/vsock_loopback.c @@ -77,6 +77,7 @@ static struct virtio_transport loopback_transport = { .stream_rcvhiwat = virtio_transport_stream_rcvhiwat, .stream_is_active = virtio_transport_stream_is_active, .stream_allow = virtio_transport_stream_allow, + .stream_bytes_unsent = virtio_transport_bytes_unsent, .seqpacket_dequeue = virtio_transport_seqpacket_dequeue, .seqpacket_enqueue = virtio_transport_seqpacket_enqueue, @@ -123,6 +124,11 @@ static void vsock_loopback_work(struct work_struct *work) spin_unlock_bh(&vsock->pkt_queue.lock); while ((skb = __skb_dequeue(&pkts))) { + /* Decrement the bytes_sent counter without deallocating skb + * It is freed by the receiver. + */ + virtio_transport_consume_skb_sent(skb, false); + virtio_transport_deliver_tap_pkt(skb); virtio_transport_recv_pkt(&loopback_transport, skb); }
This patch introduce support for stream_bytes_unsent in all virtio based transports: virtio-transport, vhost-vsock and vsock-loopback For all transports the unsent bytes counter is incremented in virtio_transport_send_pkt_info. In the virtio-transport (G2H) the counter is decremented each time the host notifies the guest that it consumed the skbuffs. In vhost-vsock (H2G) the counter is decremented after the skbuff is queued in the virtqueue. In vsock-loopback the counter is decremented after the skbuff is dequeued. Signed-off-by: Luigi Leonardi <luigi.leonardi@outlook.com> --- drivers/vhost/vsock.c | 3 ++- include/linux/virtio_vsock.h | 7 ++++++ net/vmw_vsock/virtio_transport.c | 3 ++- net/vmw_vsock/virtio_transport_common.c | 30 +++++++++++++++++++++++++ net/vmw_vsock/vsock_loopback.c | 6 +++++ 5 files changed, 47 insertions(+), 2 deletions(-)