Re: [RFC PATCH net-next v6 07/14] virtio/vsock: add common datagram send path
From: Stefano Garzarella <sgarzare@redhat.com>
Date: 2024-07-23 14:42:14
Also in:
bpf, kvm, linux-hyperv, lkml, virtualization
On Wed, Jul 10, 2024 at 09:25:48PM GMT, Amery Hung wrote:
quoted hunk ↗ jump to hunk
From: Bobby Eshleman <redacted> This commit implements the common function virtio_transport_dgram_enqueue for enqueueing datagrams. It does not add usage in either vhost or virtio yet. Signed-off-by: Bobby Eshleman <redacted> Signed-off-by: Amery Hung <redacted> --- include/linux/virtio_vsock.h | 1 + include/net/af_vsock.h | 2 + net/vmw_vsock/af_vsock.c | 2 +- net/vmw_vsock/virtio_transport_common.c | 87 ++++++++++++++++++++++++- 4 files changed, 90 insertions(+), 2 deletions(-)diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h index f749a066af46..4408749febd2 100644 --- a/include/linux/virtio_vsock.h +++ b/include/linux/virtio_vsock.h@@ -152,6 +152,7 @@ struct virtio_vsock_pkt_info {u16 op; u32 flags; bool reply; + u8 remote_flags; }; struct virtio_transport {diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h index 44db8f2c507d..6e97d344ac75 100644 --- a/include/net/af_vsock.h +++ b/include/net/af_vsock.h@@ -216,6 +216,8 @@ void vsock_for_each_connected_socket(struct vsock_transport *transport,void (*fn)(struct sock *sk)); int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk); bool vsock_find_cid(unsigned int cid); +const struct vsock_transport *vsock_dgram_lookup_transport(unsigned int cid, + __u8 flags);
Why __u8 and not just u8?
quoted hunk ↗ jump to hunk
struct vsock_skb_cb { unsigned int src_cid;diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index ab08cd81720e..f83b655fdbe9 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c@@ -487,7 +487,7 @@ vsock_connectible_lookup_transport(unsigned int cid, __u8 flags)return transport; } -static const struct vsock_transport * +const struct vsock_transport * vsock_dgram_lookup_transport(unsigned int cid, __u8 flags) { const struct vsock_transport *transport;diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index a1c76836d798..46cd1807f8e3 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c@@ -1040,13 +1040,98 @@ int virtio_transport_shutdown(struct vsock_sock *vsk, int mode)} EXPORT_SYMBOL_GPL(virtio_transport_shutdown); +static int virtio_transport_dgram_send_pkt_info(struct vsock_sock *vsk, + struct virtio_vsock_pkt_info *info) +{ + u32 src_cid, src_port, dst_cid, dst_port; + const struct vsock_transport *transport; + const struct virtio_transport *t_ops; + struct sock *sk = sk_vsock(vsk); + struct virtio_vsock_hdr *hdr; + struct sk_buff *skb; + void *payload; + int noblock = 0; + int err; + + info->type = virtio_transport_get_type(sk_vsock(vsk)); + + if (info->pkt_len > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) + return -EMSGSIZE; + + transport = vsock_dgram_lookup_transport(info->remote_cid, info->remote_flags);
Can `transport` be null? I don't understand why we are calling vsock_dgram_lookup_transport() again. Didn't we already do that in vsock_dgram_sendmsg()? Also should we add a comment mentioning that we can't use virtio_transport_get_ops()? IIUC becuase the vsk can be not assigned to a specific transport, right?
+ t_ops = container_of(transport, struct virtio_transport, transport);
+ if (unlikely(!t_ops))
+ return -EFAULT;
+
+ if (info->msg)
+ noblock = info->msg->msg_flags & MSG_DONTWAIT;
+
+ /* Use sock_alloc_send_skb to throttle by sk_sndbuf. This helps avoid
+ * triggering the OOM.
+ */
+ skb = sock_alloc_send_skb(sk, info->pkt_len + VIRTIO_VSOCK_SKB_HEADROOM,
+ noblock, &err);
+ if (!skb)
+ return err;
+
+ skb_reserve(skb, VIRTIO_VSOCK_SKB_HEADROOM);
+
+ src_cid = t_ops->transport.get_local_cid();
+ src_port = vsk->local_addr.svm_port;
+ dst_cid = info->remote_cid;
+ dst_port = info->remote_port;
+
+ hdr = virtio_vsock_hdr(skb);
+ hdr->type = cpu_to_le16(info->type);
+ hdr->op = cpu_to_le16(info->op);
+ hdr->src_cid = cpu_to_le64(src_cid);
+ hdr->dst_cid = cpu_to_le64(dst_cid);
+ hdr->src_port = cpu_to_le32(src_port);
+ hdr->dst_port = cpu_to_le32(dst_port);
+ hdr->flags = cpu_to_le32(info->flags);
+ hdr->len = cpu_to_le32(info->pkt_len);
+
+ if (info->msg && info->pkt_len > 0) {
+ payload = skb_put(skb, info->pkt_len);
+ err = memcpy_from_msg(payload, info->msg, info->pkt_len);
+ if (err)
+ goto out;
+ }
+
+ trace_virtio_transport_alloc_pkt(src_cid, src_port,
+ dst_cid, dst_port,
+ info->pkt_len,
+ info->type,
+ info->op,
+ info->flags,
+ false);
+
+ return t_ops->send_pkt(skb);
+out:
+ kfree_skb(skb);
+ return err;
+}
+
int
virtio_transport_dgram_enqueue(struct vsock_sock *vsk,
struct sockaddr_vm *remote_addr,
struct msghdr *msg,
size_t dgram_len)
{
- return -EOPNOTSUPP;
+ /* Here we are only using the info struct to retain style uniformity
+ * and to ease future refactoring and merging.
+ */
+ struct virtio_vsock_pkt_info info = {
+ .op = VIRTIO_VSOCK_OP_RW,
+ .remote_cid = remote_addr->svm_cid,
+ .remote_port = remote_addr->svm_port,
+ .remote_flags = remote_addr->svm_flags,
+ .msg = msg,
+ .vsk = vsk,
+ .pkt_len = dgram_len,
+ };
+
+ return virtio_transport_dgram_send_pkt_info(vsk, &info);
}
EXPORT_SYMBOL_GPL(virtio_transport_dgram_enqueue);
--
2.20.1