Message-ID: <20210125110903.597155-1-arseny.krasnov@kaspersky.com>
Date:   Mon, 25 Jan 2021 14:09:00 +0300
From:   Arseny Krasnov <arseny.krasnov@...persky.com>
To:     Stefan Hajnoczi <stefanha@...hat.com>,
        Stefano Garzarella <sgarzare@...hat.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Arseny Krasnov <arseny.krasnov@...persky.com>,
        Colin Ian King <colin.king@...onical.com>,
        Andra Paraschiv <andraprs@...zon.com>,
        Jeff Vander Stoep <jeffv@...gle.com>
CC:     <kvm@...r.kernel.org>, <virtualization@...ts.linux-foundation.org>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <stsp2@...dex.ru>, <oxffffaa@...il.com>
Subject: [RFC PATCH v3 00/13] virtio/vsock: introduce SOCK_SEQPACKET support

	This patchset impelements support of SOCK_SEQPACKET for virtio
transport.
	As SOCK_SEQPACKET guarantees to save record boundaries, so to
do it, new packet operation was added: it marks start of record (with
record length in header), such packet doesn't carry any data.  To send
record, packet with start marker is sent first, then all data is sent
as usual 'RW' packets. On receiver's side, length of record is known
from packet with start record marker. Now as  packets of one socket
are not reordered neither on vsock nor on vhost transport layers, such
marker allows to restore original record on receiver's side. If user's
buffer is smaller that record length, when all out of size data is
dropped.
	Maximum length of datagram is not limited as in stream socket,
because same credit logic is used. Difference with stream socket is
that user is not woken up until whole record is received or error
occurred. Implementation also supports 'MSG_EOR' and 'MSG_TRUNC' flags.
	Tests also implemented.

 Arseny Krasnov (13):
  af_vsock: prepare for SOCK_SEQPACKET support
  af_vsock: prepare 'vsock_connectible_recvmsg()'
  af_vsock: implement SEQPACKET rx loop
  af_vsock: implement send logic for SOCK_SEQPACKET
  af_vsock: rest of SEQPACKET support
  af_vsock: update comments for stream sockets
  virtio/vsock: dequeue callback for SOCK_SEQPACKET
  virtio/vsock: fetch length for SEQPACKET record
  virtio/vsock: add SEQPACKET receive logic
  virtio/vsock: rest of SOCK_SEQPACKET support
  virtio/vsock: setup SEQPACKET ops for transport
  vhost/vsock: setup SEQPACKET ops for transport
  vsock_test: add SOCK_SEQPACKET tests

 drivers/vhost/vsock.c                   |   7 +-
 include/linux/virtio_vsock.h            |  12 +
 include/net/af_vsock.h                  |   6 +
 include/uapi/linux/virtio_vsock.h       |   9 +
 net/vmw_vsock/af_vsock.c                | 543 ++++++++++++++++------
 net/vmw_vsock/virtio_transport.c        |   4 +
 net/vmw_vsock/virtio_transport_common.c | 295 ++++++++++--
 tools/testing/vsock/util.c              |  32 +-
 tools/testing/vsock/util.h              |   3 +
 tools/testing/vsock/vsock_test.c        | 126 +++++
 10 files changed, 862 insertions(+), 175 deletions(-)

 TODO:
 - Support for record integrity control. As transport could drop some
   packets, something like "record-id" and record end marker need to
   be implemented. Idea is that SEQ_BEGIN packet carries both record
   length and record id, end marker(let it be SEQ_END) carries only
   record id. To be sure that no one packet was lost, receiver checks
   length of data between SEQ_BEGIN and SEQ_END(it must be same with
   value in SEQ_BEGIN) and record ids of SEQ_BEGIN and SEQ_END(this
   means that both markers were not dropped. I think that easiest way
   to implement record id for SEQ_BEGIN is to reuse another field of
   packet header(SEQ_BEGIN already uses 'flags' as record length).For
   SEQ_END record id could be stored in 'flags'.
     Another way to implement it, is to move metadata of both SEQ_END
   and SEQ_BEGIN to payload. But this approach has problem, because
   if we move something to payload, such payload is accounted by
   credit logic, which fragments payload, while payload with record
   length and id couldn't be fragmented. One way to overcome it is to
   ignore credit update for SEQ_BEGIN/SEQ_END packet.Another solution
   is to update 'stream_has_space()' function: current implementation
   return non-zero when at least 1 byte is allowed to use,but updated
   version will have extra argument, which is needed length. For 'RW'
   packet this argument is 1, for SEQ_BEGIN it is sizeof(record len +
   record id) and for SEQ_END it is sizeof(record id).

 - What to do, when server doesn't support SOCK_SEQPACKET. In current
   implementation RST is replied in the same way when listening port
   is not found. I think that current RST is enough,because case when
   server doesn't support SEQ_PACKET is same when listener missed(e.g.
   no listener in both cases).

 v2 -> v3:
 - patches reorganized: split for prepare and implementation patches
 - local variables are declared in "Reverse Christmas tree" manner
 - virtio_transport_common.c: valid leXX_to_cpu() for vsock header
   fields access
 - af_vsock.c: 'vsock_connectible_*sockopt()' added as shared code
   between stream and seqpacket sockets.
 - af_vsock.c: loops in '__vsock_*_recvmsg()' refactored.
 - af_vsock.c: 'vsock_wait_data()' refactored.

 v1 -> v2:
 - patches reordered: af_vsock.c related changes now before virtio vsock
 - patches reorganized: more small patches, where +/- are not mixed
 - tests for SOCK_SEQPACKET added
 - all commit messages updated
 - af_vsock.c: 'vsock_pre_recv_check()' inlined to
   'vsock_connectible_recvmsg()'
 - af_vsock.c: 'vsock_assign_transport()' returns ENODEV if transport
   was not found
 - virtio_transport_common.c: transport callback for seqpacket dequeue
 - virtio_transport_common.c: simplified
   'virtio_transport_recv_connected()'
 - virtio_transport_common.c: send reset on socket and packet type
			      mismatch.

Signed-off-by: Arseny Krasnov <arseny.krasnov@...persky.com>

-- 
2.25.1

Powered by blists - more mailing lists