Stefano Garzarella
2021-Jan-29 10:55 UTC
[RFC PATCH v3 09/13] virtio/vsock: add SEQPACKET receive logic
On Mon, Jan 25, 2021 at 02:14:41PM +0300, Arseny Krasnov wrote:>This modifies current receive logic for SEQPACKET support: >1) Inserts 'SEQ_BEGIN' packet to socket's rx queue. >2) Inserts 'RW' packet to socket's rx queue, but without merging with > buffer of last packet in queue. >3) Performs check for packet and socket types on receive(if mismatch, > then reset connection). > >Signed-off-by: Arseny Krasnov <arseny.krasnov at kaspersky.com> >--- > net/vmw_vsock/virtio_transport_common.c | 79 ++++++++++++++++++------- > 1 file changed, 58 insertions(+), 21 deletions(-) > >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >index dcce35d7b462..90f9feef9d8f 100644 >--- a/net/vmw_vsock/virtio_transport_common.c >+++ b/net/vmw_vsock/virtio_transport_common.c >@@ -397,6 +397,14 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, > return err; > } > >+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; >+} >+ > static inline void virtio_transport_del_n_free_pkt(struct virtio_vsock_pkt *pkt) > { > list_del(&pkt->list); >@@ -1050,39 +1058,49 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk, > struct virtio_vsock_pkt *pkt) > { > struct virtio_vsock_sock *vvs = vsk->trans; >- bool can_enqueue, free_pkt = false; >+ bool free_pkt = false; > > pkt->len = le32_to_cpu(pkt->hdr.len); > pkt->off = 0; > > spin_lock_bh(&vvs->rx_lock); > >- can_enqueue = virtio_transport_inc_rx_pkt(vvs, pkt); >- if (!can_enqueue) { >+ if (!virtio_transport_inc_rx_pkt(vvs, pkt)) {This is an unnecessary change, it should go in another patch.> free_pkt = true; > goto out; > } > >- /* Try to copy small packets into the buffer of last packet queued, >- * to avoid wasting memory queueing the entire buffer with a small >- * payload. >- */ >- if (pkt->len <= GOOD_COPY_LEN && !list_empty(&vvs->rx_queue)) { >- struct virtio_vsock_pkt *last_pkt; >+ switch (le16_to_cpu(pkt->hdr.type)) { >+ case VIRTIO_VSOCK_TYPE_STREAM: {We already checked the 'type' in virtio_transport_recv_pkt(), so there is no need to this switch, I think you can simply add if (le16_to_cpu(pkt->hdr.type) == VIRTIO_VSOCK_TYPE_STREAM) {>+ /* Try to copy small packets into the buffer of last packet queued, >+ * to avoid wasting memory queueing the entire buffer with a small >+ * payload. >+ */ >+ if (pkt->len <= GOOD_COPY_LEN && !list_empty(&vvs->rx_queue)) { >+ struct virtio_vsock_pkt *last_pkt; > >- last_pkt = list_last_entry(&vvs->rx_queue, >- struct virtio_vsock_pkt, list); >+ last_pkt = list_last_entry(&vvs->rx_queue, >+ struct virtio_vsock_pkt, list); > >- /* If there is space in the last packet queued, we copy the >- * new packet in its buffer. >- */ >- if (pkt->len <= last_pkt->buf_len - last_pkt->len) { >- memcpy(last_pkt->buf + last_pkt->len, pkt->buf, >- pkt->len); >- last_pkt->len += pkt->len; >- free_pkt = true; >- goto out; >+ /* If there is space in the last packet queued, we copy the >+ * new packet in its buffer. >+ */ >+ if (pkt->len <= last_pkt->buf_len - last_pkt->len) { >+ memcpy(last_pkt->buf + last_pkt->len, pkt->buf, >+ pkt->len); >+ last_pkt->len += pkt->len; >+ free_pkt = true; >+ goto out; >+ } > } >+ >+ break; >+ } >+ case VIRTIO_VSOCK_TYPE_SEQPACKET: { >+ break; >+ } >+ default: >+ goto out; > } > > list_add_tail(&pkt->list, &vvs->rx_queue); >@@ -1101,6 +1119,14 @@ virtio_transport_recv_connected(struct sock *sk, > int err = 0; > > switch (le16_to_cpu(pkt->hdr.op)) { >+ case VIRTIO_VSOCK_OP_SEQ_BEGIN: { >+ struct virtio_vsock_sock *vvs = vsk->trans; >+ >+ spin_lock_bh(&vvs->rx_lock); >+ list_add_tail(&pkt->list, &vvs->rx_queue); >+ spin_unlock_bh(&vvs->rx_lock); >+ return err; >+ }Are there any problems if we use virtio_transport_recv_enqueue() in this case?> case VIRTIO_VSOCK_OP_RW: > virtio_transport_recv_enqueue(vsk, pkt); > sk->sk_data_ready(sk); >@@ -1247,6 +1273,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. > */ >@@ -1272,7 +1304,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; > } >@@ -1289,6 +1321,11 @@ 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); >+ goto free_pkt; >+ } >+ > vsk = vsock_sk(sk); > > space_available = virtio_transport_space_update(sk, pkt); >-- >2.25.1 >