public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v3 0/3] ioctl support for AF_VSOCK and virtio-based transports
@ 2024-06-26 12:08 Luigi Leonardi via B4 Relay
  2024-06-26 12:08 ` [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types Luigi Leonardi via B4 Relay
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Luigi Leonardi via B4 Relay @ 2024-06-26 12:08 UTC (permalink / raw)
  To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
	Eugenio Pérez, Xuan Zhuo
  Cc: virtualization, netdev, linux-kernel, kvm, Luigi Leonardi,
	Daan De Meyer

This patch series introduce the support for ioctl(s) in AF_VSOCK.
The only ioctl currently available is SIOCOUTQ, which returns
the number of unsent or unacked packets. It is available for
SOCK_STREAM, SOCK_SEQPACKET and SOCK_DGRAM.

As this information is transport-dependent, a new optional callback
is introduced: unsent_bytes.

The first patch add ioctl support in AF_VSOCK, while the second
patch introduce support for SOCK_STREAM and SOCK_SEQPACKET
in all virtio-based transports: virtio_transport (G2H),
vhost-vsock (H2G) and vsock-loopback.

The latest patch introduce two tests for this new feature.
More details can be found in each patch changelog.

v2->v3
Applied all reviewers' suggetions:
    - Minor style and code changes
    - atomic_int replaced with an existing spin_lock.
Introduced lock_sock on ioctl call.
Rebased to latest net-next

v1->v2
Applied all Stefano's suggestions:
    - vsock_do_ioctl has been rewritten
    - ioctl(SIOCOUTQ) test is skipped when it is not supported
    - Minor variable/function name changes
    - rebased to latest net-next

Signed-off-by: Luigi Leonardi <luigi.leonardi@outlook.com>
---
Luigi Leonardi (3):
      vsock: add support for SIOCOUTQ ioctl for all vsock socket types.
      vsock/virtio: add SIOCOUTQ support for all virtio based transports
      test/vsock: add ioctl unsent bytes test

 drivers/vhost/vsock.c                   |  4 +-
 include/linux/virtio_vsock.h            |  7 +++
 include/net/af_vsock.h                  |  3 ++
 net/vmw_vsock/af_vsock.c                | 60 +++++++++++++++++++++--
 net/vmw_vsock/virtio_transport.c        |  4 +-
 net/vmw_vsock/virtio_transport_common.c | 35 ++++++++++++++
 net/vmw_vsock/vsock_loopback.c          |  7 +++
 tools/testing/vsock/util.c              |  6 +--
 tools/testing/vsock/util.h              |  3 ++
 tools/testing/vsock/vsock_test.c        | 85 +++++++++++++++++++++++++++++++++
 10 files changed, 206 insertions(+), 8 deletions(-)
---
base-commit: 50b70845fc5c22cf7e7d25b57d57b3dca1725aa5
change-id: 20240626-ioctl_next-fbbcdd596063

Best regards,
-- 
Luigi Leonardi <luigi.leonardi@outlook.com>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types.
  2024-06-26 12:08 [PATCH net-next v3 0/3] ioctl support for AF_VSOCK and virtio-based transports Luigi Leonardi via B4 Relay
@ 2024-06-26 12:08 ` Luigi Leonardi via B4 Relay
  2024-06-27 10:34   ` kernel test robot
                     ` (3 more replies)
  2024-06-26 12:08 ` [PATCH net-next v3 2/3] vsock/virtio: add SIOCOUTQ support for all virtio based transports Luigi Leonardi via B4 Relay
  2024-06-26 12:08 ` [PATCH net-next v3 3/3] test/vsock: add ioctl unsent bytes test Luigi Leonardi via B4 Relay
  2 siblings, 4 replies; 10+ messages in thread
From: Luigi Leonardi via B4 Relay @ 2024-06-26 12:08 UTC (permalink / raw)
  To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
	Eugenio Pérez, Xuan Zhuo
  Cc: virtualization, netdev, linux-kernel, kvm, Luigi Leonardi,
	Daan De Meyer

From: Luigi Leonardi <luigi.leonardi@outlook.com>

Add support for ioctl(s) for SOCK_STREAM SOCK_SEQPACKET and SOCK_DGRAM
in AF_VSOCK.
The only ioctl available is SIOCOUTQ/TIOCOUTQ, which returns the number
of unsent bytes in the socket. This information is transport-specific
and is delegated to them using a callback.

Suggested-by: Daan De Meyer <daan.j.demeyer@gmail.com>
Signed-off-by: Luigi Leonardi <luigi.leonardi@outlook.com>
---
 include/net/af_vsock.h   |  3 +++
 net/vmw_vsock/af_vsock.c | 60 +++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index 535701efc1e5..7b5375ae7827 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -169,6 +169,9 @@ struct vsock_transport {
 	void (*notify_buffer_size)(struct vsock_sock *, u64 *);
 	int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val);
 
+	/* SIOCOUTQ ioctl */
+	size_t (*unsent_bytes)(struct vsock_sock *vsk);
+
 	/* Shutdown. */
 	int (*shutdown)(struct vsock_sock *, int);
 
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 4b040285aa78..d6140d73d122 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -112,6 +112,7 @@
 #include <net/sock.h>
 #include <net/af_vsock.h>
 #include <uapi/linux/vm_sockets.h>
+#include <uapi/asm-generic/ioctls.h>
 
 static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr);
 static void vsock_sk_destruct(struct sock *sk);
@@ -1292,6 +1293,59 @@ int vsock_dgram_recvmsg(struct socket *sock, struct msghdr *msg,
 }
 EXPORT_SYMBOL_GPL(vsock_dgram_recvmsg);
 
+static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
+			  int __user *arg)
+{
+	struct sock *sk = sock->sk;
+	struct vsock_sock *vsk;
+	int retval;
+
+	vsk = vsock_sk(sk);
+
+	switch (cmd) {
+	case SIOCOUTQ: {
+		size_t n_bytes;
+
+		if (!vsk->transport || !vsk->transport->unsent_bytes) {
+			retval = -EOPNOTSUPP;
+			break;
+		}
+
+		if (vsk->transport->unsent_bytes) {
+			if (sock_type_connectible(sk->sk_type) && sk->sk_state == TCP_LISTEN) {
+				retval = -EINVAL;
+				break;
+			}
+
+			n_bytes = vsk->transport->unsent_bytes(vsk);
+			if (n_bytes < 0) {
+				retval = n_bytes;
+				break;
+			}
+
+			retval = put_user(n_bytes, arg);
+		}
+		break;
+	}
+	default:
+		retval = -ENOIOCTLCMD;
+	}
+
+	return retval;
+}
+
+static int vsock_ioctl(struct socket *sock, unsigned int cmd,
+		       unsigned long arg)
+{
+	int ret;
+
+	lock_sock(sock->sk);
+	ret = vsock_do_ioctl(sock, cmd, (int __user *)arg);
+	release_sock(sock->sk);
+
+	return ret;
+}
+
 static const struct proto_ops vsock_dgram_ops = {
 	.family = PF_VSOCK,
 	.owner = THIS_MODULE,
@@ -1302,7 +1356,7 @@ static const struct proto_ops vsock_dgram_ops = {
 	.accept = sock_no_accept,
 	.getname = vsock_getname,
 	.poll = vsock_poll,
-	.ioctl = sock_no_ioctl,
+	.ioctl = vsock_ioctl,
 	.listen = sock_no_listen,
 	.shutdown = vsock_shutdown,
 	.sendmsg = vsock_dgram_sendmsg,
@@ -2286,7 +2340,7 @@ static const struct proto_ops vsock_stream_ops = {
 	.accept = vsock_accept,
 	.getname = vsock_getname,
 	.poll = vsock_poll,
-	.ioctl = sock_no_ioctl,
+	.ioctl = vsock_ioctl,
 	.listen = vsock_listen,
 	.shutdown = vsock_shutdown,
 	.setsockopt = vsock_connectible_setsockopt,
@@ -2308,7 +2362,7 @@ static const struct proto_ops vsock_seqpacket_ops = {
 	.accept = vsock_accept,
 	.getname = vsock_getname,
 	.poll = vsock_poll,
-	.ioctl = sock_no_ioctl,
+	.ioctl = vsock_ioctl,
 	.listen = vsock_listen,
 	.shutdown = vsock_shutdown,
 	.setsockopt = vsock_connectible_setsockopt,

-- 
2.45.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next v3 2/3] vsock/virtio: add SIOCOUTQ support for all virtio based transports
  2024-06-26 12:08 [PATCH net-next v3 0/3] ioctl support for AF_VSOCK and virtio-based transports Luigi Leonardi via B4 Relay
  2024-06-26 12:08 ` [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types Luigi Leonardi via B4 Relay
@ 2024-06-26 12:08 ` Luigi Leonardi via B4 Relay
  2024-06-28 11:16   ` Stefano Garzarella
  2024-06-26 12:08 ` [PATCH net-next v3 3/3] test/vsock: add ioctl unsent bytes test Luigi Leonardi via B4 Relay
  2 siblings, 1 reply; 10+ messages in thread
From: Luigi Leonardi via B4 Relay @ 2024-06-26 12:08 UTC (permalink / raw)
  To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
	Eugenio Pérez, Xuan Zhuo
  Cc: virtualization, netdev, linux-kernel, kvm, Luigi Leonardi

From: Luigi Leonardi <luigi.leonardi@outlook.com>

Introduce support for stream_bytes_unsent and seqpacket_bytes_unsent
ioctl for virtio_transport, vhost_vsock and vsock_loopback.

For all transports the unsent bytes counter is incremented
in virtio_transport_get_credit.

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                   |  4 +++-
 include/linux/virtio_vsock.h            |  7 +++++++
 net/vmw_vsock/virtio_transport.c        |  4 +++-
 net/vmw_vsock/virtio_transport_common.c | 35 +++++++++++++++++++++++++++++++++
 net/vmw_vsock/vsock_loopback.c          |  7 +++++++
 5 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index ec20ecff85c7..dba8b3ea37bf 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)
@@ -451,6 +451,8 @@ static struct virtio_transport vhost_transport = {
 		.notify_buffer_size       = virtio_transport_notify_buffer_size,
 		.notify_set_rcvlowat      = virtio_transport_notify_set_rcvlowat,
 
+		.unsent_bytes             = virtio_transport_bytes_unsent,
+
 		.read_skb = virtio_transport_read_skb,
 	},
 
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index c82089dee0c8..e74c12878213 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;
 
+	size_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);
 
+size_t virtio_transport_bytes_unsent(struct vsock_sock *vsk);
+
+void virtio_transport_consume_skb_sent(struct sk_buff *skb,
+				       bool consume);
+
 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 43d405298857..fc62d2818c2c 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -311,7 +311,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));
@@ -540,6 +540,8 @@ static struct virtio_transport virtio_transport = {
 		.notify_buffer_size       = virtio_transport_notify_buffer_size,
 		.notify_set_rcvlowat      = virtio_transport_notify_set_rcvlowat,
 
+		.unsent_bytes             = virtio_transport_bytes_unsent,
+
 		.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 16ff976a86e3..3a7fa36f306b 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -463,6 +463,26 @@ 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 consume)
+{
+	struct sock *s = skb->sk;
+
+	if (s && skb->len) {
+		struct vsock_sock *vs = vsock_sk(s);
+		struct virtio_vsock_sock *vvs;
+
+		vvs = vs->trans;
+
+		spin_lock_bh(&vvs->tx_lock);
+		vvs->bytes_unsent -= skb->len;
+		spin_unlock_bh(&vvs->tx_lock);
+	}
+
+	if (consume)
+		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;
@@ -475,6 +495,7 @@ u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 credit)
 	if (ret > credit)
 		ret = credit;
 	vvs->tx_cnt += ret;
+	vvs->bytes_unsent += ret;
 	spin_unlock_bh(&vvs->tx_lock);
 
 	return ret;
@@ -488,6 +509,7 @@ void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit)
 
 	spin_lock_bh(&vvs->tx_lock);
 	vvs->tx_cnt -= credit;
+	vvs->bytes_unsent -= credit;
 	spin_unlock_bh(&vvs->tx_lock);
 }
 EXPORT_SYMBOL_GPL(virtio_transport_put_credit);
@@ -1090,6 +1112,19 @@ void virtio_transport_destruct(struct vsock_sock *vsk)
 }
 EXPORT_SYMBOL_GPL(virtio_transport_destruct);
 
+size_t virtio_transport_bytes_unsent(struct vsock_sock *vsk)
+{
+	struct virtio_vsock_sock *vvs = vsk->trans;
+	size_t ret;
+
+	spin_lock_bh(&vvs->tx_lock);
+	ret = vvs->bytes_unsent;
+	spin_unlock_bh(&vvs->tx_lock);
+
+	return ret;
+}
+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..9098613561e3 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,
 		.notify_set_rcvlowat      = virtio_transport_notify_set_rcvlowat,
 
+		.unsent_bytes             = virtio_transport_bytes_unsent,
+
 		.read_skb = virtio_transport_read_skb,
 	},
 
@@ -123,6 +125,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.45.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next v3 3/3] test/vsock: add ioctl unsent bytes test
  2024-06-26 12:08 [PATCH net-next v3 0/3] ioctl support for AF_VSOCK and virtio-based transports Luigi Leonardi via B4 Relay
  2024-06-26 12:08 ` [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types Luigi Leonardi via B4 Relay
  2024-06-26 12:08 ` [PATCH net-next v3 2/3] vsock/virtio: add SIOCOUTQ support for all virtio based transports Luigi Leonardi via B4 Relay
@ 2024-06-26 12:08 ` Luigi Leonardi via B4 Relay
  2024-07-15 16:03   ` Stefano Garzarella
  2 siblings, 1 reply; 10+ messages in thread
From: Luigi Leonardi via B4 Relay @ 2024-06-26 12:08 UTC (permalink / raw)
  To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
	Eugenio Pérez, Xuan Zhuo
  Cc: virtualization, netdev, linux-kernel, kvm, Luigi Leonardi

From: Luigi Leonardi <luigi.leonardi@outlook.com>

Introduce two tests, one for SOCK_STREAM and one for SOCK_SEQPACKET, which checks
after a packet is delivered, that the number of unsent bytes is zero,
using ioctl SIOCOUTQ.

Signed-off-by: Luigi Leonardi <luigi.leonardi@outlook.com>
---
 tools/testing/vsock/util.c       |  6 +--
 tools/testing/vsock/util.h       |  3 ++
 tools/testing/vsock/vsock_test.c | 85 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 3 deletions(-)

diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 554b290fefdc..a3d448a075e3 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -139,7 +139,7 @@ int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_po
 }
 
 /* Connect to <cid, port> and return the file descriptor. */
-static int vsock_connect(unsigned int cid, unsigned int port, int type)
+int vsock_connect(unsigned int cid, unsigned int port, int type)
 {
 	union {
 		struct sockaddr sa;
@@ -226,8 +226,8 @@ static int vsock_listen(unsigned int cid, unsigned int port, int type)
 /* Listen on <cid, port> and return the first incoming connection.  The remote
  * address is stored to clientaddrp.  clientaddrp may be NULL.
  */
-static int vsock_accept(unsigned int cid, unsigned int port,
-			struct sockaddr_vm *clientaddrp, int type)
+int vsock_accept(unsigned int cid, unsigned int port,
+		 struct sockaddr_vm *clientaddrp, int type)
 {
 	union {
 		struct sockaddr sa;
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index e95e62485959..fff22d4a14c0 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -39,6 +39,9 @@ struct test_case {
 void init_signals(void);
 unsigned int parse_cid(const char *str);
 unsigned int parse_port(const char *str);
+int vsock_connect(unsigned int cid, unsigned int port, int type);
+int vsock_accept(unsigned int cid, unsigned int port,
+		 struct sockaddr_vm *clientaddrp, int type);
 int vsock_stream_connect(unsigned int cid, unsigned int port);
 int vsock_bind_connect(unsigned int cid, unsigned int port,
 		       unsigned int bind_port, int type);
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index f851f8961247..76bd17b4b291 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -20,6 +20,8 @@
 #include <sys/mman.h>
 #include <poll.h>
 #include <signal.h>
+#include <sys/ioctl.h>
+#include <linux/sockios.h>
 
 #include "vsock_test_zerocopy.h"
 #include "timeout.h"
@@ -1238,6 +1240,79 @@ static void test_double_bind_connect_client(const struct test_opts *opts)
 	}
 }
 
+#define MSG_BUF_IOCTL_LEN 64
+static void test_unsent_bytes_server(const struct test_opts *opts, int type)
+{
+	unsigned char buf[MSG_BUF_IOCTL_LEN];
+	int client_fd;
+
+	client_fd = vsock_accept(VMADDR_CID_ANY, 1234, NULL, type);
+	if (client_fd < 0) {
+		perror("accept");
+		exit(EXIT_FAILURE);
+	}
+
+	recv_buf(client_fd, buf, sizeof(buf), 0, sizeof(buf));
+	control_writeln("RECEIVED");
+
+	close(client_fd);
+}
+
+static void test_unsent_bytes_client(const struct test_opts *opts, int type)
+{
+	unsigned char buf[MSG_BUF_IOCTL_LEN];
+	int ret, fd, sock_bytes_unsent;
+
+	fd = vsock_connect(opts->peer_cid, 1234, type);
+	if (fd < 0) {
+		perror("connect");
+		exit(EXIT_FAILURE);
+	}
+
+	for (int i = 0; i < sizeof(buf); i++)
+		buf[i] = rand() & 0xFF;
+
+	send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
+	control_expectln("RECEIVED");
+
+	ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
+	if (ret < 0) {
+		if (errno == EOPNOTSUPP) {
+			fprintf(stderr, "Test skipped\n");
+		} else {
+			perror("ioctl");
+			exit(EXIT_FAILURE);
+		}
+	} else if (ret == 0 && sock_bytes_unsent != 0) {
+		fprintf(stderr,
+			"Unexpected 'SIOCOUTQ' value, expected 0, got %i\n",
+			sock_bytes_unsent);
+		exit(EXIT_FAILURE);
+	}
+
+	close(fd);
+}
+
+static void test_stream_unsent_bytes_client(const struct test_opts *opts)
+{
+	test_unsent_bytes_client(opts, SOCK_STREAM);
+}
+
+static void test_stream_unsent_bytes_server(const struct test_opts *opts)
+{
+	test_unsent_bytes_server(opts, SOCK_STREAM);
+}
+
+static void test_seqpacket_unsent_bytes_client(const struct test_opts *opts)
+{
+	test_unsent_bytes_client(opts, SOCK_SEQPACKET);
+}
+
+static void test_seqpacket_unsent_bytes_server(const struct test_opts *opts)
+{
+	test_unsent_bytes_server(opts, SOCK_SEQPACKET);
+}
+
 #define RCVLOWAT_CREDIT_UPD_BUF_SIZE	(1024 * 128)
 /* This define is the same as in 'include/linux/virtio_vsock.h':
  * it is used to decide when to send credit update message during
@@ -1523,6 +1598,16 @@ static struct test_case test_cases[] = {
 		.run_client = test_stream_rcvlowat_def_cred_upd_client,
 		.run_server = test_stream_cred_upd_on_low_rx_bytes,
 	},
+	{
+		.name = "SOCK_STREAM ioctl(SIOCOUTQ) 0 unsent bytes",
+		.run_client = test_stream_unsent_bytes_client,
+		.run_server = test_stream_unsent_bytes_server,
+	},
+	{
+		.name = "SOCK_SEQPACKET ioctl(SIOCOUTQ) 0 unsent bytes",
+		.run_client = test_seqpacket_unsent_bytes_client,
+		.run_server = test_seqpacket_unsent_bytes_server,
+	},
 	{},
 };
 

-- 
2.45.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types.
  2024-06-26 12:08 ` [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types Luigi Leonardi via B4 Relay
@ 2024-06-27 10:34   ` kernel test robot
  2024-06-28  6:15   ` kernel test robot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2024-06-27 10:34 UTC (permalink / raw)
  To: Luigi Leonardi via B4 Relay, Stefano Garzarella, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Stefan Hajnoczi,
	Michael S. Tsirkin, Jason Wang, Eugenio Pérez, Xuan Zhuo
  Cc: llvm, oe-kbuild-all, netdev, virtualization, linux-kernel, kvm,
	Luigi Leonardi, Daan De Meyer

Hi Luigi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 50b70845fc5c22cf7e7d25b57d57b3dca1725aa5]

url:    https://github.com/intel-lab-lkp/linux/commits/Luigi-Leonardi-via-B4-Relay/vsock-add-support-for-SIOCOUTQ-ioctl-for-all-vsock-socket-types/20240627-023902
base:   50b70845fc5c22cf7e7d25b57d57b3dca1725aa5
patch link:    https://lore.kernel.org/r/20240626-ioctl_next-v3-1-63be5bf19a40%40outlook.com
patch subject: [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types.
config: i386-buildonly-randconfig-001-20240627 (https://download.01.org/0day-ci/archive/20240627/202406271827.aQ9ZYlCh-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240627/202406271827.aQ9ZYlCh-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406271827.aQ9ZYlCh-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> net/vmw_vsock/af_vsock.c:1314:7: warning: variable 'retval' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
    1314 |                 if (vsk->transport->unsent_bytes) {
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/vmw_vsock/af_vsock.c:1334:9: note: uninitialized use occurs here
    1334 |         return retval;
         |                ^~~~~~
   net/vmw_vsock/af_vsock.c:1314:3: note: remove the 'if' if its condition is always true
    1314 |                 if (vsk->transport->unsent_bytes) {
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/vmw_vsock/af_vsock.c:1301:12: note: initialize the variable 'retval' to silence this warning
    1301 |         int retval;
         |                   ^
         |                    = 0
   1 warning generated.


vim +1314 net/vmw_vsock/af_vsock.c

  1295	
  1296	static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
  1297				  int __user *arg)
  1298	{
  1299		struct sock *sk = sock->sk;
  1300		struct vsock_sock *vsk;
  1301		int retval;
  1302	
  1303		vsk = vsock_sk(sk);
  1304	
  1305		switch (cmd) {
  1306		case SIOCOUTQ: {
  1307			size_t n_bytes;
  1308	
  1309			if (!vsk->transport || !vsk->transport->unsent_bytes) {
  1310				retval = -EOPNOTSUPP;
  1311				break;
  1312			}
  1313	
> 1314			if (vsk->transport->unsent_bytes) {
  1315				if (sock_type_connectible(sk->sk_type) && sk->sk_state == TCP_LISTEN) {
  1316					retval = -EINVAL;
  1317					break;
  1318				}
  1319	
  1320				n_bytes = vsk->transport->unsent_bytes(vsk);
  1321				if (n_bytes < 0) {
  1322					retval = n_bytes;
  1323					break;
  1324				}
  1325	
  1326				retval = put_user(n_bytes, arg);
  1327			}
  1328			break;
  1329		}
  1330		default:
  1331			retval = -ENOIOCTLCMD;
  1332		}
  1333	
  1334		return retval;
  1335	}
  1336	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types.
  2024-06-26 12:08 ` [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types Luigi Leonardi via B4 Relay
  2024-06-27 10:34   ` kernel test robot
@ 2024-06-28  6:15   ` kernel test robot
  2024-06-28  9:09   ` Stefano Garzarella
  2024-06-28 13:42   ` kernel test robot
  3 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2024-06-28  6:15 UTC (permalink / raw)
  To: Luigi Leonardi via B4 Relay, Stefano Garzarella, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Stefan Hajnoczi,
	Michael S. Tsirkin, Jason Wang, Eugenio Pérez, Xuan Zhuo
  Cc: oe-kbuild-all, netdev, virtualization, linux-kernel, kvm,
	Luigi Leonardi, Daan De Meyer

Hi Luigi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 50b70845fc5c22cf7e7d25b57d57b3dca1725aa5]

url:    https://github.com/intel-lab-lkp/linux/commits/Luigi-Leonardi-via-B4-Relay/vsock-add-support-for-SIOCOUTQ-ioctl-for-all-vsock-socket-types/20240627-023902
base:   50b70845fc5c22cf7e7d25b57d57b3dca1725aa5
patch link:    https://lore.kernel.org/r/20240626-ioctl_next-v3-1-63be5bf19a40%40outlook.com
patch subject: [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types.
config: i386-randconfig-141-20240628 (https://download.01.org/0day-ci/archive/20240628/202406281355.d1jNVGBc-lkp@intel.com/config)
compiler: gcc-8 (Ubuntu 8.4.0-3ubuntu2) 8.4.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406281355.d1jNVGBc-lkp@intel.com/

smatch warnings:
net/vmw_vsock/af_vsock.c:1321 vsock_do_ioctl() warn: unsigned 'n_bytes' is never less than zero.

vim +/n_bytes +1321 net/vmw_vsock/af_vsock.c

  1295	
  1296	static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
  1297				  int __user *arg)
  1298	{
  1299		struct sock *sk = sock->sk;
  1300		struct vsock_sock *vsk;
  1301		int retval;
  1302	
  1303		vsk = vsock_sk(sk);
  1304	
  1305		switch (cmd) {
  1306		case SIOCOUTQ: {
  1307			size_t n_bytes;
  1308	
  1309			if (!vsk->transport || !vsk->transport->unsent_bytes) {
  1310				retval = -EOPNOTSUPP;
  1311				break;
  1312			}
  1313	
  1314			if (vsk->transport->unsent_bytes) {
  1315				if (sock_type_connectible(sk->sk_type) && sk->sk_state == TCP_LISTEN) {
  1316					retval = -EINVAL;
  1317					break;
  1318				}
  1319	
  1320				n_bytes = vsk->transport->unsent_bytes(vsk);
> 1321				if (n_bytes < 0) {
  1322					retval = n_bytes;
  1323					break;
  1324				}
  1325	
  1326				retval = put_user(n_bytes, arg);
  1327			}
  1328			break;
  1329		}
  1330		default:
  1331			retval = -ENOIOCTLCMD;
  1332		}
  1333	
  1334		return retval;
  1335	}
  1336	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types.
  2024-06-26 12:08 ` [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types Luigi Leonardi via B4 Relay
  2024-06-27 10:34   ` kernel test robot
  2024-06-28  6:15   ` kernel test robot
@ 2024-06-28  9:09   ` Stefano Garzarella
  2024-06-28 13:42   ` kernel test robot
  3 siblings, 0 replies; 10+ messages in thread
From: Stefano Garzarella @ 2024-06-28  9:09 UTC (permalink / raw)
  To: luigi.leonardi
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
	Eugenio Pérez, Xuan Zhuo, virtualization, netdev,
	linux-kernel, kvm, Daan De Meyer

nit: in theory in this patch we don't support it for any of the 
transports, so I wouldn't confuse and take that part out of the title.

WDYT with someting like:

     vsock: add support for SIOCOUTQ ioctl

On Wed, Jun 26, 2024 at 02:08:35PM GMT, Luigi Leonardi via B4 Relay 
wrote:
>From: Luigi Leonardi <luigi.leonardi@outlook.com>
>
>Add support for ioctl(s) for SOCK_STREAM SOCK_SEQPACKET and SOCK_DGRAM
>in AF_VSOCK.
>The only ioctl available is SIOCOUTQ/TIOCOUTQ, which returns the number
>of unsent bytes in the socket. This information is transport-specific
>and is delegated to them using a callback.
>
>Suggested-by: Daan De Meyer <daan.j.demeyer@gmail.com>
>Signed-off-by: Luigi Leonardi <luigi.leonardi@outlook.com>
>---
> include/net/af_vsock.h   |  3 +++
> net/vmw_vsock/af_vsock.c | 60 +++++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 60 insertions(+), 3 deletions(-)
>
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index 535701efc1e5..7b5375ae7827 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -169,6 +169,9 @@ struct vsock_transport {
> 	void (*notify_buffer_size)(struct vsock_sock *, u64 *);
> 	int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val);
>
>+	/* SIOCOUTQ ioctl */
>+	size_t (*unsent_bytes)(struct vsock_sock *vsk);

If you want to return also errors, maybe better returning ssize_t.
This should fix one of the error reported by kernel bots.

>+
> 	/* Shutdown. */
> 	int (*shutdown)(struct vsock_sock *, int);
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 4b040285aa78..d6140d73d122 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -112,6 +112,7 @@
> #include <net/sock.h>
> #include <net/af_vsock.h>
> #include <uapi/linux/vm_sockets.h>
>+#include <uapi/asm-generic/ioctls.h>
>
> static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr);
> static void vsock_sk_destruct(struct sock *sk);
>@@ -1292,6 +1293,59 @@ int vsock_dgram_recvmsg(struct socket *sock, struct msghdr *msg,
> }
> EXPORT_SYMBOL_GPL(vsock_dgram_recvmsg);
>
>+static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
>+			  int __user *arg)
>+{
>+	struct sock *sk = sock->sk;
>+	struct vsock_sock *vsk;
>+	int retval;
>+
>+	vsk = vsock_sk(sk);
>+
>+	switch (cmd) {
>+	case SIOCOUTQ: {
>+		size_t n_bytes;
>+
>+		if (!vsk->transport || !vsk->transport->unsent_bytes) {
>+			retval = -EOPNOTSUPP;
>+			break;
>+		}
>+
>+		if (vsk->transport->unsent_bytes) {

This if is not necessary after the check we did earlier, right?

Removing it should fix the other issue reported by the bot.

>+			if (sock_type_connectible(sk->sk_type) && sk->sk_state == TCP_LISTEN) {
>+				retval = -EINVAL;
>+				break;
>+			}
>+
>+			n_bytes = vsk->transport->unsent_bytes(vsk);
>+			if (n_bytes < 0) {
>+				retval = n_bytes;
>+				break;
>+			}
>+
>+			retval = put_user(n_bytes, arg);
>+		}
>+		break;
>+	}
>+	default:
>+		retval = -ENOIOCTLCMD;
>+	}
>+
>+	return retval;
>+}
>+
>+static int vsock_ioctl(struct socket *sock, unsigned int cmd,
>+		       unsigned long arg)
>+{
>+	int ret;
>+
>+	lock_sock(sock->sk);
>+	ret = vsock_do_ioctl(sock, cmd, (int __user *)arg);
>+	release_sock(sock->sk);
>+
>+	return ret;
>+}
>+
> static const struct proto_ops vsock_dgram_ops = {
> 	.family = PF_VSOCK,
> 	.owner = THIS_MODULE,
>@@ -1302,7 +1356,7 @@ static const struct proto_ops vsock_dgram_ops = {
> 	.accept = sock_no_accept,
> 	.getname = vsock_getname,
> 	.poll = vsock_poll,
>-	.ioctl = sock_no_ioctl,
>+	.ioctl = vsock_ioctl,
> 	.listen = sock_no_listen,
> 	.shutdown = vsock_shutdown,
> 	.sendmsg = vsock_dgram_sendmsg,
>@@ -2286,7 +2340,7 @@ static const struct proto_ops vsock_stream_ops = {
> 	.accept = vsock_accept,
> 	.getname = vsock_getname,
> 	.poll = vsock_poll,
>-	.ioctl = sock_no_ioctl,
>+	.ioctl = vsock_ioctl,
> 	.listen = vsock_listen,
> 	.shutdown = vsock_shutdown,
> 	.setsockopt = vsock_connectible_setsockopt,
>@@ -2308,7 +2362,7 @@ static const struct proto_ops vsock_seqpacket_ops = {
> 	.accept = vsock_accept,
> 	.getname = vsock_getname,
> 	.poll = vsock_poll,
>-	.ioctl = sock_no_ioctl,
>+	.ioctl = vsock_ioctl,
> 	.listen = vsock_listen,
> 	.shutdown = vsock_shutdown,
> 	.setsockopt = vsock_connectible_setsockopt,
>
>-- 
>2.45.2
>
>
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v3 2/3] vsock/virtio: add SIOCOUTQ support for all virtio based transports
  2024-06-26 12:08 ` [PATCH net-next v3 2/3] vsock/virtio: add SIOCOUTQ support for all virtio based transports Luigi Leonardi via B4 Relay
@ 2024-06-28 11:16   ` Stefano Garzarella
  0 siblings, 0 replies; 10+ messages in thread
From: Stefano Garzarella @ 2024-06-28 11:16 UTC (permalink / raw)
  To: luigi.leonardi
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
	Eugenio Pérez, Xuan Zhuo, virtualization, netdev,
	linux-kernel, kvm

On Wed, Jun 26, 2024 at 02:08:36PM GMT, Luigi Leonardi via B4 Relay wrote:
>From: Luigi Leonardi <luigi.leonardi@outlook.com>
>
>Introduce support for stream_bytes_unsent and seqpacket_bytes_unsent
>ioctl for virtio_transport, vhost_vsock and vsock_loopback.
>
>For all transports the unsent bytes counter is incremented
>in virtio_transport_get_credit.
>
>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                   |  4 +++-
> include/linux/virtio_vsock.h            |  7 +++++++
> net/vmw_vsock/virtio_transport.c        |  4 +++-
> net/vmw_vsock/virtio_transport_common.c | 35 +++++++++++++++++++++++++++++++++
> net/vmw_vsock/vsock_loopback.c          |  7 +++++++
> 5 files changed, 55 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index ec20ecff85c7..dba8b3ea37bf 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)
>@@ -451,6 +451,8 @@ static struct virtio_transport vhost_transport = {
> 		.notify_buffer_size       = virtio_transport_notify_buffer_size,
> 		.notify_set_rcvlowat      = virtio_transport_notify_set_rcvlowat,
>
>+		.unsent_bytes             = virtio_transport_bytes_unsent,

The callback is named `unsent_bytes`, I'd use something similar also
in the function name, so `virtio_transport_unsent_bytes`, or the
opposite renaming the callback, as you prefer, but I'd use the same
for both.

>+
> 		.read_skb = virtio_transport_read_skb,
> 	},
>
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index c82089dee0c8..e74c12878213 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;
>

Can you remove this extra empty line, so it's clear that it is
protected by tx_lock?

>+	size_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);
>
>+size_t virtio_transport_bytes_unsent(struct vsock_sock *vsk);
>+
>+void virtio_transport_consume_skb_sent(struct sk_buff *skb,
>+				       bool consume);
>+
> 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 43d405298857..fc62d2818c2c 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -311,7 +311,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));
>@@ -540,6 +540,8 @@ static struct virtio_transport virtio_transport = {
> 		.notify_buffer_size       = virtio_transport_notify_buffer_size,
> 		.notify_set_rcvlowat      = virtio_transport_notify_set_rcvlowat,
>
>+		.unsent_bytes             = virtio_transport_bytes_unsent,
>+
> 		.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 16ff976a86e3..3a7fa36f306b 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -463,6 +463,26 @@ 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 consume)
>+{
>+	struct sock *s = skb->sk;
>+
>+	if (s && skb->len) {
>+		struct vsock_sock *vs = vsock_sk(s);
>+		struct virtio_vsock_sock *vvs;
>+
>+		vvs = vs->trans;
>+
>+		spin_lock_bh(&vvs->tx_lock);
>+		vvs->bytes_unsent -= skb->len;
>+		spin_unlock_bh(&vvs->tx_lock);
>+	}
>+
>+	if (consume)
>+		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;
>@@ -475,6 +495,7 @@ u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 credit)
> 	if (ret > credit)
> 		ret = credit;
> 	vvs->tx_cnt += ret;
>+	vvs->bytes_unsent += ret;
> 	spin_unlock_bh(&vvs->tx_lock);
>
> 	return ret;
>@@ -488,6 +509,7 @@ void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit)
>
> 	spin_lock_bh(&vvs->tx_lock);
> 	vvs->tx_cnt -= credit;
>+	vvs->bytes_unsent -= credit;
> 	spin_unlock_bh(&vvs->tx_lock);
> }
> EXPORT_SYMBOL_GPL(virtio_transport_put_credit);
>@@ -1090,6 +1112,19 @@ void virtio_transport_destruct(struct vsock_sock *vsk)
> }
> EXPORT_SYMBOL_GPL(virtio_transport_destruct);
>
>+size_t virtio_transport_bytes_unsent(struct vsock_sock *vsk)
>+{
>+	struct virtio_vsock_sock *vvs = vsk->trans;
>+	size_t ret;
>+
>+	spin_lock_bh(&vvs->tx_lock);
>+	ret = vvs->bytes_unsent;
>+	spin_unlock_bh(&vvs->tx_lock);
>+
>+	return ret;
>+}
>+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..9098613561e3 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,
> 		.notify_set_rcvlowat      = virtio_transport_notify_set_rcvlowat,
>
>+		.unsent_bytes             = virtio_transport_bytes_unsent,
>+
> 		.read_skb = virtio_transport_read_skb,
> 	},
>
>@@ -123,6 +125,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
                                  ^
Should be `bytes_unsent` ?

>+		 * It is freed by the receiver.
>+		 */
>+		virtio_transport_consume_skb_sent(skb, false);
>+

nit: no need for this new empty line.

> 		virtio_transport_deliver_tap_pkt(skb);
> 		virtio_transport_recv_pkt(&loopback_transport, skb);
> 	}
>
>-- 
>2.45.2
>
>
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types.
  2024-06-26 12:08 ` [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types Luigi Leonardi via B4 Relay
                     ` (2 preceding siblings ...)
  2024-06-28  9:09   ` Stefano Garzarella
@ 2024-06-28 13:42   ` kernel test robot
  3 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2024-06-28 13:42 UTC (permalink / raw)
  To: Luigi Leonardi via B4 Relay, Stefano Garzarella, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Stefan Hajnoczi,
	Michael S. Tsirkin, Jason Wang, Eugenio Pérez, Xuan Zhuo
  Cc: oe-kbuild-all, netdev, virtualization, linux-kernel, kvm,
	Luigi Leonardi, Daan De Meyer

Hi Luigi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 50b70845fc5c22cf7e7d25b57d57b3dca1725aa5]

url:    https://github.com/intel-lab-lkp/linux/commits/Luigi-Leonardi-via-B4-Relay/vsock-add-support-for-SIOCOUTQ-ioctl-for-all-vsock-socket-types/20240627-023902
base:   50b70845fc5c22cf7e7d25b57d57b3dca1725aa5
patch link:    https://lore.kernel.org/r/20240626-ioctl_next-v3-1-63be5bf19a40%40outlook.com
patch subject: [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types.
config: i386-randconfig-141-20240628 (https://download.01.org/0day-ci/archive/20240628/202406282144.DxR5KwIu-lkp@intel.com/config)
compiler: gcc-8 (Ubuntu 8.4.0-3ubuntu2) 8.4.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406282144.DxR5KwIu-lkp@intel.com/

smatch warnings:
net/vmw_vsock/af_vsock.c:1321 vsock_do_ioctl() warn: unsigned 'n_bytes' is never less than zero.

vim +/n_bytes +1321 net/vmw_vsock/af_vsock.c

  1295	
  1296	static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
  1297				  int __user *arg)
  1298	{
  1299		struct sock *sk = sock->sk;
  1300		struct vsock_sock *vsk;
  1301		int retval;
  1302	
  1303		vsk = vsock_sk(sk);
  1304	
  1305		switch (cmd) {
  1306		case SIOCOUTQ: {
  1307			size_t n_bytes;
  1308	
  1309			if (!vsk->transport || !vsk->transport->unsent_bytes) {
  1310				retval = -EOPNOTSUPP;
  1311				break;
  1312			}
  1313	
  1314			if (vsk->transport->unsent_bytes) {
  1315				if (sock_type_connectible(sk->sk_type) && sk->sk_state == TCP_LISTEN) {
  1316					retval = -EINVAL;
  1317					break;
  1318				}
  1319	
  1320				n_bytes = vsk->transport->unsent_bytes(vsk);
> 1321				if (n_bytes < 0) {
  1322					retval = n_bytes;
  1323					break;
  1324				}
  1325	
  1326				retval = put_user(n_bytes, arg);
  1327			}
  1328			break;
  1329		}
  1330		default:
  1331			retval = -ENOIOCTLCMD;
  1332		}
  1333	
  1334		return retval;
  1335	}
  1336	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v3 3/3] test/vsock: add ioctl unsent bytes test
  2024-06-26 12:08 ` [PATCH net-next v3 3/3] test/vsock: add ioctl unsent bytes test Luigi Leonardi via B4 Relay
@ 2024-07-15 16:03   ` Stefano Garzarella
  0 siblings, 0 replies; 10+ messages in thread
From: Stefano Garzarella @ 2024-07-15 16:03 UTC (permalink / raw)
  To: luigi.leonardi
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
	Eugenio Pérez, Xuan Zhuo, virtualization, netdev,
	linux-kernel, kvm

On Wed, Jun 26, 2024 at 02:08:37PM GMT, Luigi Leonardi via B4 Relay wrote:
>From: Luigi Leonardi <luigi.leonardi@outlook.com>
>
>Introduce two tests, one for SOCK_STREAM and one for SOCK_SEQPACKET, which checks
>after a packet is delivered, that the number of unsent bytes is zero,
>using ioctl SIOCOUTQ.
>
>Signed-off-by: Luigi Leonardi <luigi.leonardi@outlook.com>
>---
> tools/testing/vsock/util.c       |  6 +--
> tools/testing/vsock/util.h       |  3 ++
> tools/testing/vsock/vsock_test.c | 85 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 91 insertions(+), 3 deletions(-)
>
>diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>index 554b290fefdc..a3d448a075e3 100644
>--- a/tools/testing/vsock/util.c
>+++ b/tools/testing/vsock/util.c
>@@ -139,7 +139,7 @@ int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_po
> }
>
> /* Connect to <cid, port> and return the file descriptor. */
>-static int vsock_connect(unsigned int cid, unsigned int port, int type)
>+int vsock_connect(unsigned int cid, unsigned int port, int type)
> {
> 	union {
> 		struct sockaddr sa;
>@@ -226,8 +226,8 @@ static int vsock_listen(unsigned int cid, unsigned int port, int type)
> /* Listen on <cid, port> and return the first incoming connection.  The remote
>  * address is stored to clientaddrp.  clientaddrp may be NULL.
>  */
>-static int vsock_accept(unsigned int cid, unsigned int port,
>-			struct sockaddr_vm *clientaddrp, int type)
>+int vsock_accept(unsigned int cid, unsigned int port,
>+		 struct sockaddr_vm *clientaddrp, int type)
> {
> 	union {
> 		struct sockaddr sa;
>diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
>index e95e62485959..fff22d4a14c0 100644
>--- a/tools/testing/vsock/util.h
>+++ b/tools/testing/vsock/util.h
>@@ -39,6 +39,9 @@ struct test_case {
> void init_signals(void);
> unsigned int parse_cid(const char *str);
> unsigned int parse_port(const char *str);
>+int vsock_connect(unsigned int cid, unsigned int port, int type);
>+int vsock_accept(unsigned int cid, unsigned int port,
>+		 struct sockaddr_vm *clientaddrp, int type);

I'd mention in the commit description that you need these functions to 
be more generic. Maybe in the future we can re-use them where we share 
the same test for both SEQPACKET and STREAM.

The rest LGTM.

Thanks,
Stefano

> int vsock_stream_connect(unsigned int cid, unsigned int port);
> int vsock_bind_connect(unsigned int cid, unsigned int port,
> 		       unsigned int bind_port, int type);
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index f851f8961247..76bd17b4b291 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -20,6 +20,8 @@
> #include <sys/mman.h>
> #include <poll.h>
> #include <signal.h>
>+#include <sys/ioctl.h>
>+#include <linux/sockios.h>
>
> #include "vsock_test_zerocopy.h"
> #include "timeout.h"
>@@ -1238,6 +1240,79 @@ static void test_double_bind_connect_client(const struct test_opts *opts)
> 	}
> }
>
>+#define MSG_BUF_IOCTL_LEN 64
>+static void test_unsent_bytes_server(const struct test_opts *opts, int type)
>+{
>+	unsigned char buf[MSG_BUF_IOCTL_LEN];
>+	int client_fd;
>+
>+	client_fd = vsock_accept(VMADDR_CID_ANY, 1234, NULL, type);
>+	if (client_fd < 0) {
>+		perror("accept");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	recv_buf(client_fd, buf, sizeof(buf), 0, sizeof(buf));
>+	control_writeln("RECEIVED");
>+
>+	close(client_fd);
>+}
>+
>+static void test_unsent_bytes_client(const struct test_opts *opts, int type)
>+{
>+	unsigned char buf[MSG_BUF_IOCTL_LEN];
>+	int ret, fd, sock_bytes_unsent;
>+
>+	fd = vsock_connect(opts->peer_cid, 1234, type);
>+	if (fd < 0) {
>+		perror("connect");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	for (int i = 0; i < sizeof(buf); i++)
>+		buf[i] = rand() & 0xFF;
>+
>+	send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
>+	control_expectln("RECEIVED");
>+
>+	ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
>+	if (ret < 0) {
>+		if (errno == EOPNOTSUPP) {
>+			fprintf(stderr, "Test skipped\n");
>+		} else {
>+			perror("ioctl");
>+			exit(EXIT_FAILURE);
>+		}
>+	} else if (ret == 0 && sock_bytes_unsent != 0) {
>+		fprintf(stderr,
>+			"Unexpected 'SIOCOUTQ' value, expected 0, got %i\n",
>+			sock_bytes_unsent);
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	close(fd);
>+}
>+
>+static void test_stream_unsent_bytes_client(const struct test_opts *opts)
>+{
>+	test_unsent_bytes_client(opts, SOCK_STREAM);
>+}
>+
>+static void test_stream_unsent_bytes_server(const struct test_opts *opts)
>+{
>+	test_unsent_bytes_server(opts, SOCK_STREAM);
>+}
>+
>+static void test_seqpacket_unsent_bytes_client(const struct test_opts *opts)
>+{
>+	test_unsent_bytes_client(opts, SOCK_SEQPACKET);
>+}
>+
>+static void test_seqpacket_unsent_bytes_server(const struct test_opts *opts)
>+{
>+	test_unsent_bytes_server(opts, SOCK_SEQPACKET);
>+}
>+
> #define RCVLOWAT_CREDIT_UPD_BUF_SIZE	(1024 * 128)
> /* This define is the same as in 'include/linux/virtio_vsock.h':
>  * it is used to decide when to send credit update message during
>@@ -1523,6 +1598,16 @@ static struct test_case test_cases[] = {
> 		.run_client = test_stream_rcvlowat_def_cred_upd_client,
> 		.run_server = test_stream_cred_upd_on_low_rx_bytes,
> 	},
>+	{
>+		.name = "SOCK_STREAM ioctl(SIOCOUTQ) 0 unsent bytes",
>+		.run_client = test_stream_unsent_bytes_client,
>+		.run_server = test_stream_unsent_bytes_server,
>+	},
>+	{
>+		.name = "SOCK_SEQPACKET ioctl(SIOCOUTQ) 0 unsent bytes",
>+		.run_client = test_seqpacket_unsent_bytes_client,
>+		.run_server = test_seqpacket_unsent_bytes_server,
>+	},
> 	{},
> };
>
>
>-- 
>2.45.2
>
>
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-07-15 16:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-26 12:08 [PATCH net-next v3 0/3] ioctl support for AF_VSOCK and virtio-based transports Luigi Leonardi via B4 Relay
2024-06-26 12:08 ` [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types Luigi Leonardi via B4 Relay
2024-06-27 10:34   ` kernel test robot
2024-06-28  6:15   ` kernel test robot
2024-06-28  9:09   ` Stefano Garzarella
2024-06-28 13:42   ` kernel test robot
2024-06-26 12:08 ` [PATCH net-next v3 2/3] vsock/virtio: add SIOCOUTQ support for all virtio based transports Luigi Leonardi via B4 Relay
2024-06-28 11:16   ` Stefano Garzarella
2024-06-26 12:08 ` [PATCH net-next v3 3/3] test/vsock: add ioctl unsent bytes test Luigi Leonardi via B4 Relay
2024-07-15 16:03   ` Stefano Garzarella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox