On 15.11.2023 14:08, Stefano Garzarella wrote: > On Wed, Nov 08, 2023 at 10:20:03AM +0300, Arseniy Krasnov wrote: >> This adds sending credit update message when SO_RCVLOWAT is updated and >> it is bigger than number of bytes in rx queue. It is needed, because >> 'poll()' will wait until number of bytes in rx queue will be not smaller >> than SO_RCVLOWAT, so kick sender to send more data. Otherwise mutual >> hungup for tx/rx is possible: sender waits for free space and receiver >> is waiting data in 'poll()'. >> >> Signed-off-by: Arseniy Krasnov <avkrasnov@xxxxxxxxxxxxxxxxx> >> --- >> drivers/vhost/vsock.c | 2 ++ >> include/linux/virtio_vsock.h | 1 + >> net/vmw_vsock/virtio_transport.c | 2 ++ >> net/vmw_vsock/virtio_transport_common.c | 31 +++++++++++++++++++++++++ >> net/vmw_vsock/vsock_loopback.c | 2 ++ >> 5 files changed, 38 insertions(+) >> >> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c >> index f75731396b7e..ecfa5c11f5ee 100644 >> --- a/drivers/vhost/vsock.c >> +++ b/drivers/vhost/vsock.c >> @@ -451,6 +451,8 @@ static struct virtio_transport vhost_transport = { >> .notify_buffer_size = virtio_transport_notify_buffer_size, >> >> .read_skb = virtio_transport_read_skb, >> + >> + .set_rcvlowat = virtio_transport_set_rcvlowat >> }, >> >> .send_pkt = vhost_transport_send_pkt, >> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h >> index ebb3ce63d64d..97dc1bebc69c 100644 >> --- a/include/linux/virtio_vsock.h >> +++ b/include/linux/virtio_vsock.h >> @@ -256,4 +256,5 @@ void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit); >> void virtio_transport_deliver_tap_pkt(struct sk_buff *skb); >> int virtio_transport_purge_skbs(void *vsk, struct sk_buff_head *list); >> int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t read_actor); >> +int virtio_transport_set_rcvlowat(struct vsock_sock *vsk, int val); >> #endif /* _LINUX_VIRTIO_VSOCK_H */ >> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c >> index af5bab1acee1..cf3431189d0c 100644 >> --- a/net/vmw_vsock/virtio_transport.c >> +++ b/net/vmw_vsock/virtio_transport.c >> @@ -539,6 +539,8 @@ static struct virtio_transport virtio_transport = { >> .notify_buffer_size = virtio_transport_notify_buffer_size, >> >> .read_skb = virtio_transport_read_skb, >> + >> + .set_rcvlowat = virtio_transport_set_rcvlowat >> }, >> >> .send_pkt = virtio_transport_send_pkt, >> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >> index e22c81435ef7..88a58163046e 100644 >> --- a/net/vmw_vsock/virtio_transport_common.c >> +++ b/net/vmw_vsock/virtio_transport_common.c >> @@ -1676,6 +1676,37 @@ int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t recv_acto >> } >> EXPORT_SYMBOL_GPL(virtio_transport_read_skb); >> >> +int virtio_transport_set_rcvlowat(struct vsock_sock *vsk, int val) >> +{ >> + struct virtio_vsock_sock *vvs = vsk->trans; >> + bool send_update = false; > > I'd declare this not initialized. > >> + >> + spin_lock_bh(&vvs->rx_lock); >> + >> + /* If number of available bytes is less than new >> + * SO_RCVLOWAT value, kick sender to send more >> + * data, because sender may sleep in its 'send()' >> + * syscall waiting for enough space at our side. >> + */ >> + if (vvs->rx_bytes < val) >> + send_update = true; > > Then here just: > send_update = vvs->rx_bytes < val; > >> + >> + spin_unlock_bh(&vvs->rx_lock); >> + >> + if (send_update) { >> + int err; >> + >> + err = virtio_transport_send_credit_update(vsk); >> + if (err < 0) >> + return err; >> + } >> + >> + WRITE_ONCE(sk_vsock(vsk)->sk_rcvlowat, val ? : 1); > > Not in this patch, but what about doing this in vsock_set_rcvlowat() in af_vsock.c? > > I mean avoid to return if `transport->set_rcvlowat(vsk, val)` is > successfully, so set sk_rcvlowat in a single point. Yes, we can do it, I'll include new patch as 0001 in v2, don't remember why it wasn't implemented in this way before. Thanks, Arseniy > > The rest LGTM! > > Stefano > >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(virtio_transport_set_rcvlowat); >> + >> MODULE_LICENSE("GPL v2"); >> MODULE_AUTHOR("Asias He"); >> MODULE_DESCRIPTION("common code for virtio vsock"); >> diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c >> index 048640167411..388c157f6633 100644 >> --- a/net/vmw_vsock/vsock_loopback.c >> +++ b/net/vmw_vsock/vsock_loopback.c >> @@ -98,6 +98,8 @@ static struct virtio_transport loopback_transport = { >> .notify_buffer_size = virtio_transport_notify_buffer_size, >> >> .read_skb = virtio_transport_read_skb, >> + >> + .set_rcvlowat = virtio_transport_set_rcvlowat >> }, >> >> .send_pkt = vsock_loopback_send_pkt, >> -- 2.25.1 >> >