| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20250521020613.3218651-1-niuxuewei.nxw@antgroup.com>
Date: Wed, 21 May 2025 10:06:13 +0800
From: Xuewei Niu <niuxuewei97@...il.com>
To: sgarzare@...hat.com
Cc: davem@...emloft.net,
fupan.lfp@...group.com,
jasowang@...hat.com,
kvm@...r.kernel.org,
linux-kernel@...r.kernel.org,
mst@...hat.com,
niuxuewei.nxw@...group.com,
niuxuewei97@...il.com,
pabeni@...hat.com,
stefanha@...hat.com,
virtualization@...ts.linux.dev,
xuanzhuo@...ux.alibaba.com
Subject: Re: [PATCH 2/3] vsock/virtio: Add SIOCINQ support for all virtio based transports
> On Mon, May 19, 2025 at 03:06:48PM +0800, Xuewei Niu wrote:
> >The virtio_vsock_sock has a new field called bytes_unread as the return
> >value of the SIOCINQ ioctl.
> >
> >Though the rx_bytes exists, we introduce a bytes_unread field to the
> >virtio_vsock_sock struct. The reason is that it will not be updated
> >until the skbuff is fully consumed, which causes inconsistency.
> >
> >The byte_unread is increased by the length of the skbuff when skbuff is
> >enqueued, and it is decreased when dequeued.
> >
> >Signed-off-by: Xuewei Niu <niuxuewei.nxw@...group.com>
> >---
> > drivers/vhost/vsock.c | 1 +
> > include/linux/virtio_vsock.h | 2 ++
> > net/vmw_vsock/virtio_transport.c | 1 +
> > net/vmw_vsock/virtio_transport_common.c | 17 +++++++++++++++++
> > net/vmw_vsock/vsock_loopback.c | 1 +
> > 5 files changed, 22 insertions(+)
> >
> >diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> >index 802153e23073..0f20af6e5036 100644
> >--- a/drivers/vhost/vsock.c
> >+++ b/drivers/vhost/vsock.c
> >@@ -452,6 +452,7 @@ static struct virtio_transport vhost_transport = {
> > .notify_set_rcvlowat = virtio_transport_notify_set_rcvlowat,
> >
> > .unsent_bytes = virtio_transport_unsent_bytes,
> >+ .unread_bytes = virtio_transport_unread_bytes,
> >
> > .read_skb = virtio_transport_read_skb,
> > },
> >diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> >index 0387d64e2c66..0a7bd240113a 100644
> >--- a/include/linux/virtio_vsock.h
> >+++ b/include/linux/virtio_vsock.h
> >@@ -142,6 +142,7 @@ struct virtio_vsock_sock {
> > u32 buf_alloc;
> > struct sk_buff_head rx_queue;
> > u32 msg_count;
> >+ size_t bytes_unread;
>
> Can we just use `rx_bytes` field we already have?
>
> Thanks,
> Stefano
I perfer not. The `rx_bytes` won't be updated until the skbuff is fully
consumed, causing inconsistency issues. If it is acceptable to you, I'll
reuse the field instead.
Thanks,
Xuewei
> > };
> >
> > struct virtio_vsock_pkt_info {
> >@@ -195,6 +196,7 @@ s64 virtio_transport_stream_has_space(struct vsock_sock *vsk);
> > u32 virtio_transport_seqpacket_has_data(struct vsock_sock *vsk);
> >
> > ssize_t virtio_transport_unsent_bytes(struct vsock_sock *vsk);
> >+ssize_t virtio_transport_unread_bytes(struct vsock_sock *vsk);
> >
> > void virtio_transport_consume_skb_sent(struct sk_buff *skb,
> > bool consume);
> >diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> >index f0e48e6911fc..917881537b63 100644
> >--- a/net/vmw_vsock/virtio_transport.c
> >+++ b/net/vmw_vsock/virtio_transport.c
> >@@ -585,6 +585,7 @@ static struct virtio_transport virtio_transport = {
> > .notify_set_rcvlowat = virtio_transport_notify_set_rcvlowat,
> >
> > .unsent_bytes = virtio_transport_unsent_bytes,
> >+ .unread_bytes = virtio_transport_unread_bytes,
> >
> > .read_skb = virtio_transport_read_skb,
> > },
> >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> >index 7f7de6d88096..11eae88c60fc 100644
> >--- a/net/vmw_vsock/virtio_transport_common.c
> >+++ b/net/vmw_vsock/virtio_transport_common.c
> >@@ -632,6 +632,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
> > free_space = vvs->buf_alloc - fwd_cnt_delta;
> > low_rx_bytes = (vvs->rx_bytes <
> > sock_rcvlowat(sk_vsock(vsk), 0, INT_MAX));
> >+ vvs->bytes_unread -= total;
> >
> > spin_unlock_bh(&vvs->rx_lock);
> >
> >@@ -782,6 +783,7 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
> > }
> >
> > virtio_transport_dec_rx_pkt(vvs, pkt_len);
> >+ vvs->bytes_unread -= pkt_len;
> > kfree_skb(skb);
> > }
> >
> >@@ -1132,6 +1134,19 @@ ssize_t virtio_transport_unsent_bytes(struct vsock_sock *vsk)
> > }
> > EXPORT_SYMBOL_GPL(virtio_transport_unsent_bytes);
> >
> >+ssize_t virtio_transport_unread_bytes(struct vsock_sock *vsk)
> >+{
> >+ struct virtio_vsock_sock *vvs = vsk->trans;
> >+ size_t ret;
> >+
> >+ spin_lock_bh(&vvs->rx_lock);
> >+ ret = vvs->bytes_unread;
> >+ spin_unlock_bh(&vvs->rx_lock);
> >+
> >+ return ret;
> >+}
> >+EXPORT_SYMBOL_GPL(virtio_transport_unread_bytes);
> >+
> > static int virtio_transport_reset(struct vsock_sock *vsk,
> > struct sk_buff *skb)
> > {
> >@@ -1365,6 +1380,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > goto out;
> > }
> >
> >+ vvs->bytes_unread += len;
> >+
> > if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
> > vvs->msg_count++;
> >
> >diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
> >index 6e78927a598e..13a77db2a76f 100644
> >--- a/net/vmw_vsock/vsock_loopback.c
> >+++ b/net/vmw_vsock/vsock_loopback.c
> >@@ -99,6 +99,7 @@ static struct virtio_transport loopback_transport = {
> > .notify_set_rcvlowat = virtio_transport_notify_set_rcvlowat,
> >
> > .unsent_bytes = virtio_transport_unsent_bytes,
> >+ .unread_bytes = virtio_transport_unread_bytes,
> >
> > .read_skb = virtio_transport_read_skb,
> > },
> >--
> >2.34.1
> >
Powered by blists - more mailing lists