Re: [PATCH RFC net-next v2 2/4] virtio/vsock: add VIRTIO_VSOCK_F_DGRAM feature bit

From: Bobby Eshleman <bobbyeshleman@gmail.com>
To: Stefano Garzarella <sgarzare@redhat.com>
Cc: Bobby Eshleman <bobby.eshleman@bytedance.com>,
	Vishnu Dasa <vdasa@vmware.com>, Wei Liu <wei.liu@kernel.org>,
	Jiang Wang <jiang.wang@bytedance.com>,
	kvm@vger.kernel.org, "Michael S. Tsirkin" <mst@redhat.com>,
	VMware PV-Drivers Reviewers <pv-drivers@vmware.com>,
	Dexuan Cui <decui@microsoft.com>,
	Haiyang Zhang <haiyangz@microsoft.com>,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	Bryan Tan <bryantan@vmware.com>,
	Eric Dumazet <edumazet@google.com>,
	linux-hyperv@vger.kernel.org,
	Stefan Hajnoczi <stefanha@redhat.com>,
	netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH RFC net-next v2 2/4] virtio/vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
Date: Sat, 15 Apr 2023 09:51:37 +0000	[thread overview]
Message-ID: <ZDpzqTewbtuxV/Sk@bullseye> (raw)
In-Reply-To: <nbuuohh72i4n27rzlg7sj7bwsrsrnnxxcxj6w5yotw5bhpcznt@ormwl5d6jiuw>

On Wed, Apr 19, 2023 at 11:30:21AM +0200, Stefano Garzarella wrote:
> On Fri, Apr 14, 2023 at 12:25:58AM +0000, Bobby Eshleman wrote:
> > This commit adds a feature bit for virtio vsock to support datagrams.
> > This commit should not be applied without first applying the commit
> > that implements datagrams for virtio.
> > 
> > Signed-off-by: Jiang Wang <jiang.wang@bytedance.com>
> > Signed-off-by: Bobby Eshleman <bobby.eshleman@bytedance.com>
> > ---
> > drivers/vhost/vsock.c             | 3 ++-
> > include/uapi/linux/virtio_vsock.h | 1 +
> > net/vmw_vsock/virtio_transport.c  | 8 ++++++--
> > 3 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > index dff6ee1c479b..028cf079225e 100644
> > --- a/drivers/vhost/vsock.c
> > +++ b/drivers/vhost/vsock.c
> > @@ -32,7 +32,8 @@
> > enum {
> > 	VHOST_VSOCK_FEATURES = VHOST_FEATURES |
> > 			       (1ULL << VIRTIO_F_ACCESS_PLATFORM) |
> > -			       (1ULL << VIRTIO_VSOCK_F_SEQPACKET)
> > +			       (1ULL << VIRTIO_VSOCK_F_SEQPACKET) |
> > +			       (1ULL << VIRTIO_VSOCK_F_DGRAM)
> > };
> > 
> > enum {
> > diff --git a/include/uapi/linux/virtio_vsock.h b/include/uapi/linux/virtio_vsock.h
> > index 331be28b1d30..0975b9c88292 100644
> > --- a/include/uapi/linux/virtio_vsock.h
> > +++ b/include/uapi/linux/virtio_vsock.h
> > @@ -40,6 +40,7 @@
> > 
> > /* The feature bitmap for virtio vsock */
> > #define VIRTIO_VSOCK_F_SEQPACKET	1	/* SOCK_SEQPACKET supported */
> > +#define VIRTIO_VSOCK_F_DGRAM		2	/* Host support dgram vsock */
> > 
> > struct virtio_vsock_config {
> > 	__le64 guest_cid;
> > diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> > index 582c6c0f788f..bb43eea9a6f9 100644
> > --- a/net/vmw_vsock/virtio_transport.c
> > +++ b/net/vmw_vsock/virtio_transport.c
> > @@ -29,6 +29,7 @@ static struct virtio_transport virtio_transport; /* forward declaration */
> > struct virtio_vsock {
> > 	struct virtio_device *vdev;
> > 	struct virtqueue *vqs[VSOCK_VQ_MAX];
> > +	bool has_dgram;
> > 
> > 	/* Virtqueue processing is deferred to a workqueue */
> > 	struct work_struct tx_work;
> > @@ -640,7 +641,6 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
> > 	}
> > 
> > 	vsock->vdev = vdev;
> > -
> > 	vsock->rx_buf_nr = 0;
> > 	vsock->rx_buf_max_nr = 0;
> > 	atomic_set(&vsock->queued_replies, 0);
> > @@ -657,6 +657,9 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
> > 	if (virtio_has_feature(vdev, VIRTIO_VSOCK_F_SEQPACKET))
> > 		vsock->seqpacket_allow = true;
> > 
> > +	if (virtio_has_feature(vdev, VIRTIO_VSOCK_F_DGRAM))
> > +		vsock->has_dgram = true;
> 
> This is unused for now, but I think the idea is to use in
> virtio_transport_dgram_allow(), right?
> 
> I would follow `seqpacket_allow` in this case.
> 

Got it, thanks.

> Thanks,
> Stefano
> 
> > +
> > 	vdev->priv = vsock;
> > 
> > 	ret = virtio_vsock_vqs_init(vsock);
> > @@ -749,7 +752,8 @@ static struct virtio_device_id id_table[] = {
> > };
> > 
> > static unsigned int features[] = {
> > -	VIRTIO_VSOCK_F_SEQPACKET
> > +	VIRTIO_VSOCK_F_SEQPACKET,
> > +	VIRTIO_VSOCK_F_DGRAM
> > };
> > 
> > static struct virtio_driver virtio_vsock_driver = {
> > 
> > -- 
> > 2.30.2
> > 
> 
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization