Inter-revision diff: patch 9

Comparing v4 (message) to v3 (message)

--- v4
+++ v3
@@ -1,208 +1,153 @@
-This adds transport callback and it's logic for SEQPACKET dequeue.
-Callback fetches RW packets from rx queue of socket until whole record
-is copied(if user's buffer is full, user is not woken up). This is done
-to not stall sender, because if we wake up user and it leaves syscall,
-nobody will send credit update for rest of record, and sender will wait
-for next enter of read syscall at receiver's side. So if user buffer is
-full, we just send credit update and drop data. If during copy SEQ_BEGIN
-was found(and not all data was copied), copying is restarted by reset
-user's iov iterator(previous unfinished data is dropped).
+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@kaspersky.com>
 ---
- include/linux/virtio_vsock.h            |   5 +
- include/uapi/linux/virtio_vsock.h       |  16 ++++
- net/vmw_vsock/virtio_transport_common.c | 120 ++++++++++++++++++++++++
- 3 files changed, 141 insertions(+)
+ net/vmw_vsock/virtio_transport_common.c | 79 ++++++++++++++++++-------
+ 1 file changed, 58 insertions(+), 21 deletions(-)
 
-diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
-index dc636b727179..4d0de3dee9a4 100644
---- a/include/linux/virtio_vsock.h
-+++ b/include/linux/virtio_vsock.h
-@@ -36,6 +36,11 @@ struct virtio_vsock_sock {
- 	u32 rx_bytes;
- 	u32 buf_alloc;
- 	struct list_head rx_queue;
-+
-+	/* For SOCK_SEQPACKET */
-+	u32 user_read_seq_len;
-+	u32 user_read_copied;
-+	u32 curr_rx_msg_cnt;
- };
- 
- struct virtio_vsock_pkt {
-diff --git a/include/uapi/linux/virtio_vsock.h b/include/uapi/linux/virtio_vsock.h
-index 1d57ed3d84d2..cf9c165e5cca 100644
---- a/include/uapi/linux/virtio_vsock.h
-+++ b/include/uapi/linux/virtio_vsock.h
-@@ -63,8 +63,14 @@ struct virtio_vsock_hdr {
- 	__le32	fwd_cnt;
- } __attribute__((packed));
- 
-+struct virtio_vsock_seq_hdr {
-+	__le32  msg_cnt;
-+	__le32  msg_len;
-+} __attribute__((packed));
-+
- enum virtio_vsock_type {
- 	VIRTIO_VSOCK_TYPE_STREAM = 1,
-+	VIRTIO_VSOCK_TYPE_SEQPACKET = 2,
- };
- 
- enum virtio_vsock_op {
-@@ -83,6 +89,11 @@ enum virtio_vsock_op {
- 	VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6,
- 	/* Request the peer to send the credit info to us */
- 	VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7,
-+
-+	/* Record begin for SOCK_SEQPACKET */
-+	VIRTIO_VSOCK_OP_SEQ_BEGIN = 8,
-+	/* Record end for SOCK_SEQPACKET */
-+	VIRTIO_VSOCK_OP_SEQ_END = 9,
- };
- 
- /* VIRTIO_VSOCK_OP_SHUTDOWN flags values */
-@@ -91,4 +102,9 @@ enum virtio_vsock_shutdown {
- 	VIRTIO_VSOCK_SHUTDOWN_SEND = 2,
- };
- 
-+/* VIRTIO_VSOCK_OP_RW flags values */
-+enum virtio_vsock_rw {
-+	VIRTIO_VSOCK_RW_EOR = 1,
-+};
-+
- #endif /* _UAPI_LINUX_VIRTIO_VSOCK_H */
 diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
-index 5956939eebb7..4572d01c8ea5 100644
+index dcce35d7b462..90f9feef9d8f 100644
 --- a/net/vmw_vsock/virtio_transport_common.c
 +++ b/net/vmw_vsock/virtio_transport_common.c
-@@ -397,6 +397,126 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
+@@ -397,6 +397,14 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
  	return err;
  }
  
-+static inline void virtio_transport_remove_pkt(struct virtio_vsock_pkt *pkt)
++static u16 virtio_transport_get_type(struct sock *sk)
 +{
-+	list_del(&pkt->list);
-+	virtio_transport_free_pkt(pkt);
++	if (sk->sk_type == SOCK_STREAM)
++		return VIRTIO_VSOCK_TYPE_STREAM;
++	else
++		return VIRTIO_VSOCK_TYPE_SEQPACKET;
 +}
 +
-+static size_t virtio_transport_drop_until_seq_begin(struct virtio_vsock_sock *vvs)
+ 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)) {
+ 		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: {
++		/* 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;
++	}
+ 	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)
 +{
-+	struct virtio_vsock_pkt *pkt, *n;
-+	size_t bytes_dropped = 0;
++	return (type == VIRTIO_VSOCK_TYPE_STREAM) ||
++	       (type == VIRTIO_VSOCK_TYPE_SEQPACKET);
++}
 +
-+	list_for_each_entry_safe(pkt, n, &vvs->rx_queue, list) {
-+		if (le16_to_cpu(pkt->hdr.op) == VIRTIO_VSOCK_OP_SEQ_BEGIN)
-+			break;
-+
-+		bytes_dropped += le32_to_cpu(pkt->hdr.len);
-+		virtio_transport_dec_rx_pkt(vvs, pkt);
-+		virtio_transport_remove_pkt(pkt);
+ /* 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;
 +	}
 +
-+	return bytes_dropped;
-+}
-+
-+static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
-+						 struct msghdr *msg,
-+						 bool *msg_ready)
-+{
-+	struct virtio_vsock_sock *vvs = vsk->trans;
-+	struct virtio_vsock_pkt *pkt;
-+	int err = 0;
-+	size_t user_buf_len = msg->msg_iter.count;
-+
-+	*msg_ready = false;
-+	spin_lock_bh(&vvs->rx_lock);
-+
-+	while (!*msg_ready && !list_empty(&vvs->rx_queue) && !err) {
-+		pkt = list_first_entry(&vvs->rx_queue, struct virtio_vsock_pkt, list);
-+
-+		switch (le16_to_cpu(pkt->hdr.op)) {
-+		case VIRTIO_VSOCK_OP_SEQ_BEGIN: {
-+			/* Unexpected 'SEQ_BEGIN' during record copy:
-+			 * Leave receive loop, 'EAGAIN' will restart it from
-+			 * outer receive loop, packet is still in queue and
-+			 * counters are cleared. So in next loop enter,
-+			 * 'SEQ_BEGIN' will be dequeued first. User's iov
-+			 * iterator will be reset in outer loop. Also
-+			 * send credit update, because some bytes could be
-+			 * copied. User will never see unfinished record.
-+			 */
-+			err = -EAGAIN;
-+			break;
-+		}
-+		case VIRTIO_VSOCK_OP_SEQ_END: {
-+			struct virtio_vsock_seq_hdr *seq_hdr;
-+
-+			seq_hdr = (struct virtio_vsock_seq_hdr *)pkt->buf;
-+			/* First check that whole record is received. */
-+
-+			if (vvs->user_read_copied != vvs->user_read_seq_len ||
-+			    (le32_to_cpu(seq_hdr->msg_cnt) - vvs->curr_rx_msg_cnt) != 1) {
-+				/* Tail of current record and head of next missed,
-+				 * so this EOR is from next record. Restart receive.
-+				 * Current record will be dropped, next headless will
-+				 * be dropped on next attempt to get record length.
-+				 */
-+				err = -EAGAIN;
-+			} else {
-+				/* Success. */
-+				*msg_ready = true;
-+			}
-+
-+			break;
-+		}
-+		case VIRTIO_VSOCK_OP_RW: {
-+			size_t bytes_to_copy;
-+			size_t pkt_len;
-+
-+			pkt_len = (size_t)le32_to_cpu(pkt->hdr.len);
-+			bytes_to_copy = min(user_buf_len, pkt_len);
-+
-+			/* sk_lock is held by caller so no one else can dequeue.
-+			 * Unlock rx_lock since memcpy_to_msg() may sleep.
-+			 */
-+			spin_unlock_bh(&vvs->rx_lock);
-+
-+			if (memcpy_to_msg(msg, pkt->buf, bytes_to_copy)) {
-+				spin_lock_bh(&vvs->rx_lock);
-+				err = -EINVAL;
-+				break;
-+			}
-+
-+			spin_lock_bh(&vvs->rx_lock);
-+			user_buf_len -= bytes_to_copy;
-+			vvs->user_read_copied += pkt_len;
-+
-+			if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_RW_EOR)
-+				msg->msg_flags |= MSG_EOR;
-+			break;
-+		}
-+		default:
-+			;
-+		}
-+
-+		/* For unexpected 'SEQ_BEGIN', keep such packet in queue,
-+		 * but drop any other type of packet.
-+		 */
-+		if (le16_to_cpu(pkt->hdr.op) != VIRTIO_VSOCK_OP_SEQ_BEGIN) {
-+			virtio_transport_dec_rx_pkt(vvs, pkt);
-+			virtio_transport_remove_pkt(pkt);
-+		}
-+	}
-+
-+	spin_unlock_bh(&vvs->rx_lock);
-+
-+	virtio_transport_send_credit_update(vsk, VIRTIO_VSOCK_TYPE_SEQPACKET,
-+					    NULL);
-+
-+	return err;
-+}
-+
- ssize_t
- virtio_transport_stream_dequeue(struct vsock_sock *vsk,
- 				struct msghdr *msg,
+ 	vsk = vsock_sk(sk);
+ 
+ 	space_available = virtio_transport_space_update(sk, pkt);
 -- 
 2.25.1
 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help