Stefano Garzarella
2021-Jun-03 14:56 UTC
[PATCH v10 12/18] virtio/vsock: add SEQPACKET receive logic
On Thu, May 20, 2021 at 10:18:21PM +0300, Arseny Krasnov wrote:>Update current receive logic for SEQPACKET support: performs >check for packet and socket types on receive(if mismatch, then >reset connection).We also copy the flags. Please check better your commit messages.> >Signed-off-by: Arseny Krasnov <arseny.krasnov at kaspersky.com> >--- > v9 -> v10: > 1) Commit message updated. > 2) Comment updated. > 3) Updated way to to set 'last_pkt' flags. > > net/vmw_vsock/virtio_transport_common.c | 30 ++++++++++++++++++++++--- > 1 file changed, 27 insertions(+), 3 deletions(-) > >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >index 61349b2ea7fe..a6f8b0f39775 100644 >--- a/net/vmw_vsock/virtio_transport_common.c >+++ b/net/vmw_vsock/virtio_transport_common.c >@@ -165,6 +165,14 @@ void virtio_transport_deliver_tap_pkt(struct virtio_vsock_pkt *pkt) > } > EXPORT_SYMBOL_GPL(virtio_transport_deliver_tap_pkt); > >+static u16 virtio_transport_get_type(struct sock *sk) >+{ >+ if (sk->sk_type == SOCK_STREAM) >+ return VIRTIO_VSOCK_TYPE_STREAM; >+ else >+ return VIRTIO_VSOCK_TYPE_SEQPACKET; >+} >+ > /* This function can only be used on connecting/connected sockets, > * since a socket assigned to a transport is required. > * >@@ -979,13 +987,17 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk, > struct virtio_vsock_pkt, list); > > /* If there is space in the last packet queued, we copy the >- * new packet in its buffer. >+ * new packet in its buffer(except SEQPACKET case, when we >+ * also check that last packet is not last packet of previous >+ * record).Is better to explain why we don't do this for SEQPACKET, something like this: /* If there is space in the last packet queued, we copy the * new packet in its buffer. * We avoid this if the last packet queued has * VIRTIO_VSOCK_SEQ_EOR set, because it is the delimiter * of SEQPACKET record, so `pkt` is the first packet * of a new record. */> */ >- if (pkt->len <= last_pkt->buf_len - last_pkt->len) { >+ if ((pkt->len <= last_pkt->buf_len - last_pkt->len) && >+ !(le32_to_cpu(last_pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOR)) { > memcpy(last_pkt->buf + last_pkt->len, pkt->buf, > pkt->len); > last_pkt->len += pkt->len; > free_pkt = true; >+ last_pkt->hdr.flags |= pkt->hdr.flags; > goto out; > } > } >@@ -1151,6 +1163,12 @@ virtio_transport_recv_listen(struct sock *sk, struct virtio_vsock_pkt *pkt, > return 0; > } > >+static bool virtio_transport_valid_type(u16 type) >+{ >+ return (type == VIRTIO_VSOCK_TYPE_STREAM) || >+ (type == VIRTIO_VSOCK_TYPE_SEQPACKET); >+} >+ > /* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex > * lock. > */ >@@ -1176,7 +1194,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t, > le32_to_cpu(pkt->hdr.buf_alloc), > le32_to_cpu(pkt->hdr.fwd_cnt)); > >- if (le16_to_cpu(pkt->hdr.type) != VIRTIO_VSOCK_TYPE_STREAM) { >+ if (!virtio_transport_valid_type(le16_to_cpu(pkt->hdr.type))) { > (void)virtio_transport_reset_no_sock(t, pkt); > goto free_pkt; > } >@@ -1193,6 +1211,12 @@ void virtio_transport_recv_pkt(struct virtio_transport *t, > } > } > >+ if (virtio_transport_get_type(sk) != le16_to_cpu(pkt->hdr.type)) { >+ (void)virtio_transport_reset_no_sock(t, pkt); >+ sock_put(sk); >+ goto free_pkt; >+ } >+ > vsk = vsock_sk(sk); > > lock_sock(sk); >-- >2.25.1 >The rest LGTM. Stefano