* [PATCH net-next 0/3] vhost_net: access ptr ring using tap recvmsg
@ 2019-10-12 1:53 prashantbhole.linux
2019-10-12 1:53 ` [PATCH net-next 1/3] tuntap: reorganize tun_msg_ctl usage prashantbhole.linux
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: prashantbhole.linux @ 2019-10-12 1:53 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, David S . Miller
Cc: Prashant Bhole, David Ahern, kvm, netdev
From: Prashant Bhole <prashantbhole.linux@gmail.com>
vhost_net needs to peek tun packet sizes to allocate virtio buffers.
Currently it directly accesses tap ptr ring to do it. Jason Wang
suggested to achieve this using msghdr->msg_control and modifying the
behavior of tap recvmsg.
This change will be useful in future in case of virtio-net XDP
offload. Where packets will be XDP processed in tap recvmsg and vhost
will see only non XDP_DROP'ed packets.
Patch 1: reorganizes the tun_msg_ctl so that it can be extended by
the means of different commands. tap sendmsg recvmsg will behave
according to commands.
Patch 2: modifies recvmsg implementation to produce packet pointers.
vhost_net uses recvmsg API instead of ptr_ring_consume().
Patch 3: removes ptr ring usage in vhost and functions those export
ptr ring from tun/tap.
Prashant Bhole (3):
tuntap: reorganize tun_msg_ctl usage
vhost_net: user tap recvmsg api to access ptr ring
tuntap: remove usage of ptr ring in vhost_net
drivers/net/tap.c | 44 ++++++++++++++---------
drivers/net/tun.c | 45 +++++++++++++++---------
drivers/vhost/net.c | 79 ++++++++++++++++++++++--------------------
include/linux/if_tun.h | 9 +++--
4 files changed, 103 insertions(+), 74 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next 1/3] tuntap: reorganize tun_msg_ctl usage
2019-10-12 1:53 [PATCH net-next 0/3] vhost_net: access ptr ring using tap recvmsg prashantbhole.linux
@ 2019-10-12 1:53 ` prashantbhole.linux
2019-10-12 7:44 ` Jason Wang
2019-10-12 1:53 ` [PATCH net-next 2/3] vhost_net: user tap recvmsg api to access ptr ring prashantbhole.linux
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: prashantbhole.linux @ 2019-10-12 1:53 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, David S . Miller
Cc: Prashant Bhole, David Ahern, kvm, netdev
From: Prashant Bhole <prashantbhole.linux@gmail.com>
In order to extend the usage of tun_msg_ctl structure, this patch
changes the member name from type to cmd. Also following definitions
are changed:
TUN_MSG_PTR : TUN_CMD_BATCH
TUN_MSG_UBUF: TUN_CMD_PACKET
Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
---
drivers/net/tap.c | 9 ++++++---
drivers/net/tun.c | 8 ++++++--
drivers/vhost/net.c | 4 ++--
include/linux/if_tun.h | 6 +++---
4 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 3ae70c7e6860..01bd260ce60c 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -1213,9 +1213,10 @@ static int tap_sendmsg(struct socket *sock, struct msghdr *m,
struct tap_queue *q = container_of(sock, struct tap_queue, sock);
struct tun_msg_ctl *ctl = m->msg_control;
struct xdp_buff *xdp;
+ void *ptr = NULL;
int i;
- if (ctl && (ctl->type == TUN_MSG_PTR)) {
+ if (ctl && ctl->cmd == TUN_CMD_BATCH) {
for (i = 0; i < ctl->num; i++) {
xdp = &((struct xdp_buff *)ctl->ptr)[i];
tap_get_user_xdp(q, xdp);
@@ -1223,8 +1224,10 @@ static int tap_sendmsg(struct socket *sock, struct msghdr *m,
return 0;
}
- return tap_get_user(q, ctl ? ctl->ptr : NULL, &m->msg_iter,
- m->msg_flags & MSG_DONTWAIT);
+ if (ctl && ctl->cmd == TUN_CMD_PACKET)
+ ptr = ctl->ptr;
+
+ return tap_get_user(q, ptr, &m->msg_iter, m->msg_flags & MSG_DONTWAIT);
}
static int tap_recvmsg(struct socket *sock, struct msghdr *m,
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 0413d182d782..29711671959b 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2529,11 +2529,12 @@ static int tun_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len)
struct tun_struct *tun = tun_get(tfile);
struct tun_msg_ctl *ctl = m->msg_control;
struct xdp_buff *xdp;
+ void *ptr = NULL;
if (!tun)
return -EBADFD;
- if (ctl && (ctl->type == TUN_MSG_PTR)) {
+ if (ctl && ctl->cmd == TUN_CMD_BATCH) {
struct tun_page tpage;
int n = ctl->num;
int flush = 0;
@@ -2560,7 +2561,10 @@ static int tun_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len)
goto out;
}
- ret = tun_get_user(tun, tfile, ctl ? ctl->ptr : NULL, &m->msg_iter,
+ if (ctl && ctl->cmd == TUN_CMD_PACKET)
+ ptr = ctl->ptr;
+
+ ret = tun_get_user(tun, tfile, ptr, &m->msg_iter,
m->msg_flags & MSG_DONTWAIT,
m->msg_flags & MSG_MORE);
out:
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 1a2dd53caade..5946d2775bd0 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -462,7 +462,7 @@ static void vhost_tx_batch(struct vhost_net *net,
struct msghdr *msghdr)
{
struct tun_msg_ctl ctl = {
- .type = TUN_MSG_PTR,
+ .cmd = TUN_CMD_BATCH,
.num = nvq->batched_xdp,
.ptr = nvq->xdp,
};
@@ -902,7 +902,7 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock)
ubuf->desc = nvq->upend_idx;
refcount_set(&ubuf->refcnt, 1);
msg.msg_control = &ctl;
- ctl.type = TUN_MSG_UBUF;
+ ctl.cmd = TUN_CMD_PACKET;
ctl.ptr = ubuf;
msg.msg_controllen = sizeof(ctl);
ubufs = nvq->ubufs;
diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
index 5bda8cf457b6..bdfa671612db 100644
--- a/include/linux/if_tun.h
+++ b/include/linux/if_tun.h
@@ -11,10 +11,10 @@
#define TUN_XDP_FLAG 0x1UL
-#define TUN_MSG_UBUF 1
-#define TUN_MSG_PTR 2
+#define TUN_CMD_PACKET 1
+#define TUN_CMD_BATCH 2
struct tun_msg_ctl {
- unsigned short type;
+ unsigned short cmd;
unsigned short num;
void *ptr;
};
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next 2/3] vhost_net: user tap recvmsg api to access ptr ring
2019-10-12 1:53 [PATCH net-next 0/3] vhost_net: access ptr ring using tap recvmsg prashantbhole.linux
2019-10-12 1:53 ` [PATCH net-next 1/3] tuntap: reorganize tun_msg_ctl usage prashantbhole.linux
@ 2019-10-12 1:53 ` prashantbhole.linux
2019-10-12 7:54 ` Jason Wang
2019-10-12 20:41 ` Michael S. Tsirkin
2019-10-12 1:53 ` [PATCH net-next 3/3] tuntap: remove usage of ptr ring in vhost_net prashantbhole.linux
2019-10-12 7:57 ` [PATCH net-next 0/3] vhost_net: access ptr ring using tap recvmsg Jason Wang
3 siblings, 2 replies; 11+ messages in thread
From: prashantbhole.linux @ 2019-10-12 1:53 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, David S . Miller
Cc: Prashant Bhole, David Ahern, kvm, netdev
From: Prashant Bhole <prashantbhole.linux@gmail.com>
Currently vhost_net directly accesses ptr ring of tap driver to
fetch Rx packet pointers. In order to avoid it this patch modifies
tap driver's recvmsg api to do additional task of fetching Rx packet
pointers.
A special struct tun_msg_ctl is already being usedd via msg_control
for tun Rx XDP batching. This patch extends tun_msg_ctl usage to
send sub commands to recvmsg api. recvmsg can now produce/unproduce
pointers from ptr ring as an additional task.
This will be useful in future in implementation of virtio-net XDP
offload feature. Where packets will be XDP batch processed in
tun_recvmsg.
Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
---
drivers/net/tap.c | 22 +++++++++++++++++++-
drivers/net/tun.c | 24 +++++++++++++++++++++-
drivers/vhost/net.c | 46 +++++++++++++++++++++++++++++++++---------
include/linux/if_tun.h | 3 +++
4 files changed, 83 insertions(+), 12 deletions(-)
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 01bd260ce60c..3d0bf382dbbc 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -1234,8 +1234,28 @@ static int tap_recvmsg(struct socket *sock, struct msghdr *m,
size_t total_len, int flags)
{
struct tap_queue *q = container_of(sock, struct tap_queue, sock);
- struct sk_buff *skb = m->msg_control;
+ struct tun_msg_ctl *ctl = m->msg_control;
+ struct sk_buff *skb = NULL;
int ret;
+
+ if (ctl) {
+ switch (ctl->cmd) {
+ case TUN_CMD_PACKET:
+ skb = ctl->ptr;
+ break;
+ case TUN_CMD_PRODUCE_PTRS:
+ return ptr_ring_consume_batched(&q->ring,
+ ctl->ptr_array,
+ ctl->num);
+ case TUN_CMD_UNPRODUCE_PTRS:
+ ptr_ring_unconsume(&q->ring, ctl->ptr_array, ctl->num,
+ tun_ptr_free);
+ return 0;
+ default:
+ return -EINVAL;
+ }
+ }
+
if (flags & ~(MSG_DONTWAIT|MSG_TRUNC)) {
kfree_skb(skb);
return -EINVAL;
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 29711671959b..7d4886f53389 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2577,7 +2577,8 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
{
struct tun_file *tfile = container_of(sock, struct tun_file, socket);
struct tun_struct *tun = tun_get(tfile);
- void *ptr = m->msg_control;
+ struct tun_msg_ctl *ctl = m->msg_control;
+ void *ptr = NULL;
int ret;
if (!tun) {
@@ -2585,6 +2586,27 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
goto out_free;
}
+ if (ctl) {
+ switch (ctl->cmd) {
+ case TUN_CMD_PACKET:
+ ptr = ctl->ptr;
+ break;
+ case TUN_CMD_PRODUCE_PTRS:
+ ret = ptr_ring_consume_batched(&tfile->tx_ring,
+ ctl->ptr_array,
+ ctl->num);
+ goto out;
+ case TUN_CMD_UNPRODUCE_PTRS:
+ ptr_ring_unconsume(&tfile->tx_ring, ctl->ptr_array,
+ ctl->num, tun_ptr_free);
+ ret = 0;
+ goto out;
+ default:
+ ret = -EINVAL;
+ goto out_put_tun;
+ }
+ }
+
if (flags & ~(MSG_DONTWAIT|MSG_TRUNC|MSG_ERRQUEUE)) {
ret = -EINVAL;
goto out_put_tun;
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 5946d2775bd0..5e5c1063606c 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -175,24 +175,44 @@ static void *vhost_net_buf_consume(struct vhost_net_buf *rxq)
static int vhost_net_buf_produce(struct vhost_net_virtqueue *nvq)
{
+ struct vhost_virtqueue *vq = &nvq->vq;
+ struct socket *sock = vq->private_data;
struct vhost_net_buf *rxq = &nvq->rxq;
+ struct tun_msg_ctl ctl = {
+ .cmd = TUN_CMD_PRODUCE_PTRS,
+ .ptr_array = rxq->queue,
+ .num = VHOST_NET_BATCH,
+ };
+ struct msghdr msg = {
+ .msg_control = &ctl,
+ };
rxq->head = 0;
- rxq->tail = ptr_ring_consume_batched(nvq->rx_ring, rxq->queue,
- VHOST_NET_BATCH);
+ rxq->tail = sock->ops->recvmsg(sock, &msg, 0, 0);
+ if (rxq->tail < 0)
+ rxq->tail = 0;
+
return rxq->tail;
}
static void vhost_net_buf_unproduce(struct vhost_net_virtqueue *nvq)
{
+ struct vhost_virtqueue *vq = &nvq->vq;
+ struct socket *sock = vq->private_data;
struct vhost_net_buf *rxq = &nvq->rxq;
+ struct tun_msg_ctl ctl = {
+ .cmd = TUN_CMD_UNPRODUCE_PTRS,
+ .ptr_array = rxq->queue + rxq->head,
+ .num = vhost_net_buf_get_size(rxq),
+ };
+ struct msghdr msg = {
+ .msg_control = &ctl,
+ };
- if (nvq->rx_ring && !vhost_net_buf_is_empty(rxq)) {
- ptr_ring_unconsume(nvq->rx_ring, rxq->queue + rxq->head,
- vhost_net_buf_get_size(rxq),
- tun_ptr_free);
- rxq->head = rxq->tail = 0;
- }
+ if (!vhost_net_buf_is_empty(rxq))
+ sock->ops->recvmsg(sock, &msg, 0, 0);
+
+ rxq->head = rxq->tail = 0;
}
static int vhost_net_buf_peek_len(void *ptr)
@@ -1109,6 +1129,9 @@ static void handle_rx(struct vhost_net *net)
.flags = 0,
.gso_type = VIRTIO_NET_HDR_GSO_NONE
};
+ struct tun_msg_ctl ctl = {
+ .cmd = TUN_CMD_PACKET,
+ };
size_t total_len = 0;
int err, mergeable;
s16 headcount;
@@ -1166,8 +1189,11 @@ static void handle_rx(struct vhost_net *net)
goto out;
}
busyloop_intr = false;
- if (nvq->rx_ring)
- msg.msg_control = vhost_net_buf_consume(&nvq->rxq);
+ if (nvq->rx_ring) {
+ ctl.cmd = TUN_CMD_PACKET;
+ ctl.ptr = vhost_net_buf_consume(&nvq->rxq);
+ msg.msg_control = &ctl;
+ }
/* On overrun, truncate and discard */
if (unlikely(headcount > UIO_MAXIOV)) {
iov_iter_init(&msg.msg_iter, READ, vq->iov, 1, 1);
diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
index bdfa671612db..8608d4095143 100644
--- a/include/linux/if_tun.h
+++ b/include/linux/if_tun.h
@@ -13,10 +13,13 @@
#define TUN_CMD_PACKET 1
#define TUN_CMD_BATCH 2
+#define TUN_CMD_PRODUCE_PTRS 3
+#define TUN_CMD_UNPRODUCE_PTRS 4
struct tun_msg_ctl {
unsigned short cmd;
unsigned short num;
void *ptr;
+ void **ptr_array;
};
struct tun_xdp_hdr {
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next 3/3] tuntap: remove usage of ptr ring in vhost_net
2019-10-12 1:53 [PATCH net-next 0/3] vhost_net: access ptr ring using tap recvmsg prashantbhole.linux
2019-10-12 1:53 ` [PATCH net-next 1/3] tuntap: reorganize tun_msg_ctl usage prashantbhole.linux
2019-10-12 1:53 ` [PATCH net-next 2/3] vhost_net: user tap recvmsg api to access ptr ring prashantbhole.linux
@ 2019-10-12 1:53 ` prashantbhole.linux
2019-10-12 7:57 ` [PATCH net-next 0/3] vhost_net: access ptr ring using tap recvmsg Jason Wang
3 siblings, 0 replies; 11+ messages in thread
From: prashantbhole.linux @ 2019-10-12 1:53 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, David S . Miller
Cc: Prashant Bhole, David Ahern, kvm, netdev
From: Prashant Bhole <prashantbhole.linux@gmail.com>
Remove usage of ptr ring of tuntap in vhost_net and remove the
functions exported from tuntap drivers to get ptr ring.
Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
---
drivers/net/tap.c | 13 -------------
drivers/net/tun.c | 13 -------------
drivers/vhost/net.c | 31 ++++---------------------------
3 files changed, 4 insertions(+), 53 deletions(-)
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 3d0bf382dbbc..27ffd2210375 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -1298,19 +1298,6 @@ struct socket *tap_get_socket(struct file *file)
}
EXPORT_SYMBOL_GPL(tap_get_socket);
-struct ptr_ring *tap_get_ptr_ring(struct file *file)
-{
- struct tap_queue *q;
-
- if (file->f_op != &tap_fops)
- return ERR_PTR(-EINVAL);
- q = file->private_data;
- if (!q)
- return ERR_PTR(-EBADFD);
- return &q->ring;
-}
-EXPORT_SYMBOL_GPL(tap_get_ptr_ring);
-
int tap_queue_resize(struct tap_dev *tap)
{
struct net_device *dev = tap->dev;
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 7d4886f53389..75893921411b 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -3750,19 +3750,6 @@ struct socket *tun_get_socket(struct file *file)
}
EXPORT_SYMBOL_GPL(tun_get_socket);
-struct ptr_ring *tun_get_tx_ring(struct file *file)
-{
- struct tun_file *tfile;
-
- if (file->f_op != &tun_fops)
- return ERR_PTR(-EINVAL);
- tfile = file->private_data;
- if (!tfile)
- return ERR_PTR(-EBADFD);
- return &tfile->tx_ring;
-}
-EXPORT_SYMBOL_GPL(tun_get_tx_ring);
-
module_init(tun_init);
module_exit(tun_cleanup);
MODULE_DESCRIPTION(DRV_DESCRIPTION);
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 5e5c1063606c..0d302efadf44 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -122,7 +122,6 @@ struct vhost_net_virtqueue {
/* Reference counting for outstanding ubufs.
* Protected by vq mutex. Writers must also take device mutex. */
struct vhost_net_ubuf_ref *ubufs;
- struct ptr_ring *rx_ring;
struct vhost_net_buf rxq;
/* Batched XDP buffs */
struct xdp_buff *xdp;
@@ -997,8 +996,9 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk)
int len = 0;
unsigned long flags;
- if (rvq->rx_ring)
- return vhost_net_buf_peek(rvq);
+ len = vhost_net_buf_peek(rvq);
+ if (len)
+ return len;
spin_lock_irqsave(&sk->sk_receive_queue.lock, flags);
head = skb_peek(&sk->sk_receive_queue);
@@ -1189,7 +1189,7 @@ static void handle_rx(struct vhost_net *net)
goto out;
}
busyloop_intr = false;
- if (nvq->rx_ring) {
+ if (!vhost_net_buf_is_empty(&nvq->rxq)) {
ctl.cmd = TUN_CMD_PACKET;
ctl.ptr = vhost_net_buf_consume(&nvq->rxq);
msg.msg_control = &ctl;
@@ -1345,7 +1345,6 @@ static int vhost_net_open(struct inode *inode, struct file *f)
n->vqs[i].batched_xdp = 0;
n->vqs[i].vhost_hlen = 0;
n->vqs[i].sock_hlen = 0;
- n->vqs[i].rx_ring = NULL;
vhost_net_buf_init(&n->vqs[i].rxq);
}
vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX,
@@ -1374,7 +1373,6 @@ static struct socket *vhost_net_stop_vq(struct vhost_net *n,
vhost_net_disable_vq(n, vq);
vq->private_data = NULL;
vhost_net_buf_unproduce(nvq);
- nvq->rx_ring = NULL;
mutex_unlock(&vq->mutex);
return sock;
}
@@ -1470,25 +1468,6 @@ static struct socket *get_raw_socket(int fd)
return ERR_PTR(r);
}
-static struct ptr_ring *get_tap_ptr_ring(int fd)
-{
- struct ptr_ring *ring;
- struct file *file = fget(fd);
-
- if (!file)
- return NULL;
- ring = tun_get_tx_ring(file);
- if (!IS_ERR(ring))
- goto out;
- ring = tap_get_ptr_ring(file);
- if (!IS_ERR(ring))
- goto out;
- ring = NULL;
-out:
- fput(file);
- return ring;
-}
-
static struct socket *get_tap_socket(int fd)
{
struct file *file = fget(fd);
@@ -1572,8 +1551,6 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
r = vhost_net_enable_vq(n, vq);
if (r)
goto err_used;
- if (index == VHOST_NET_VQ_RX)
- nvq->rx_ring = get_tap_ptr_ring(fd);
oldubufs = nvq->ubufs;
nvq->ubufs = ubufs;
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 1/3] tuntap: reorganize tun_msg_ctl usage
2019-10-12 1:53 ` [PATCH net-next 1/3] tuntap: reorganize tun_msg_ctl usage prashantbhole.linux
@ 2019-10-12 7:44 ` Jason Wang
2019-10-15 0:33 ` Prashant Bhole
0 siblings, 1 reply; 11+ messages in thread
From: Jason Wang @ 2019-10-12 7:44 UTC (permalink / raw)
To: prashantbhole.linux, Michael S . Tsirkin, David S . Miller
Cc: David Ahern, kvm, netdev
On 2019/10/12 上午9:53, prashantbhole.linux@gmail.com wrote:
> From: Prashant Bhole <prashantbhole.linux@gmail.com>
>
> In order to extend the usage of tun_msg_ctl structure, this patch
> changes the member name from type to cmd. Also following definitions
> are changed:
> TUN_MSG_PTR : TUN_CMD_BATCH
> TUN_MSG_UBUF: TUN_CMD_PACKET
Not a native English speaker, but the conversion here looks not as
straightforward as it did.
For TUN_MSG_PTR, it means recvmsg() can do receiving from a pointer to
either XDP or skb instead of ptr_ring. TUN_CMD_BATCH sounds not related.
For TUN_MSG_UBUF, it means the packet is a zercopy (buffer pointers to
userspace). TUN_CMD_PACKET may bring confusion in this case.
Thanks
>
> Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
> ---
> drivers/net/tap.c | 9 ++++++---
> drivers/net/tun.c | 8 ++++++--
> drivers/vhost/net.c | 4 ++--
> include/linux/if_tun.h | 6 +++---
> 4 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index 3ae70c7e6860..01bd260ce60c 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -1213,9 +1213,10 @@ static int tap_sendmsg(struct socket *sock, struct msghdr *m,
> struct tap_queue *q = container_of(sock, struct tap_queue, sock);
> struct tun_msg_ctl *ctl = m->msg_control;
> struct xdp_buff *xdp;
> + void *ptr = NULL;
> int i;
>
> - if (ctl && (ctl->type == TUN_MSG_PTR)) {
> + if (ctl && ctl->cmd == TUN_CMD_BATCH) {
> for (i = 0; i < ctl->num; i++) {
> xdp = &((struct xdp_buff *)ctl->ptr)[i];
> tap_get_user_xdp(q, xdp);
> @@ -1223,8 +1224,10 @@ static int tap_sendmsg(struct socket *sock, struct msghdr *m,
> return 0;
> }
>
> - return tap_get_user(q, ctl ? ctl->ptr : NULL, &m->msg_iter,
> - m->msg_flags & MSG_DONTWAIT);
> + if (ctl && ctl->cmd == TUN_CMD_PACKET)
> + ptr = ctl->ptr;
> +
> + return tap_get_user(q, ptr, &m->msg_iter, m->msg_flags & MSG_DONTWAIT);
> }
>
> static int tap_recvmsg(struct socket *sock, struct msghdr *m,
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 0413d182d782..29711671959b 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -2529,11 +2529,12 @@ static int tun_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len)
> struct tun_struct *tun = tun_get(tfile);
> struct tun_msg_ctl *ctl = m->msg_control;
> struct xdp_buff *xdp;
> + void *ptr = NULL;
>
> if (!tun)
> return -EBADFD;
>
> - if (ctl && (ctl->type == TUN_MSG_PTR)) {
> + if (ctl && ctl->cmd == TUN_CMD_BATCH) {
> struct tun_page tpage;
> int n = ctl->num;
> int flush = 0;
> @@ -2560,7 +2561,10 @@ static int tun_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len)
> goto out;
> }
>
> - ret = tun_get_user(tun, tfile, ctl ? ctl->ptr : NULL, &m->msg_iter,
> + if (ctl && ctl->cmd == TUN_CMD_PACKET)
> + ptr = ctl->ptr;
> +
> + ret = tun_get_user(tun, tfile, ptr, &m->msg_iter,
> m->msg_flags & MSG_DONTWAIT,
> m->msg_flags & MSG_MORE);
> out:
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 1a2dd53caade..5946d2775bd0 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -462,7 +462,7 @@ static void vhost_tx_batch(struct vhost_net *net,
> struct msghdr *msghdr)
> {
> struct tun_msg_ctl ctl = {
> - .type = TUN_MSG_PTR,
> + .cmd = TUN_CMD_BATCH,
> .num = nvq->batched_xdp,
> .ptr = nvq->xdp,
> };
> @@ -902,7 +902,7 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock)
> ubuf->desc = nvq->upend_idx;
> refcount_set(&ubuf->refcnt, 1);
> msg.msg_control = &ctl;
> - ctl.type = TUN_MSG_UBUF;
> + ctl.cmd = TUN_CMD_PACKET;
> ctl.ptr = ubuf;
> msg.msg_controllen = sizeof(ctl);
> ubufs = nvq->ubufs;
> diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
> index 5bda8cf457b6..bdfa671612db 100644
> --- a/include/linux/if_tun.h
> +++ b/include/linux/if_tun.h
> @@ -11,10 +11,10 @@
>
> #define TUN_XDP_FLAG 0x1UL
>
> -#define TUN_MSG_UBUF 1
> -#define TUN_MSG_PTR 2
> +#define TUN_CMD_PACKET 1
> +#define TUN_CMD_BATCH 2
> struct tun_msg_ctl {
> - unsigned short type;
> + unsigned short cmd;
> unsigned short num;
> void *ptr;
> };
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 2/3] vhost_net: user tap recvmsg api to access ptr ring
2019-10-12 1:53 ` [PATCH net-next 2/3] vhost_net: user tap recvmsg api to access ptr ring prashantbhole.linux
@ 2019-10-12 7:54 ` Jason Wang
2019-10-12 20:41 ` Michael S. Tsirkin
1 sibling, 0 replies; 11+ messages in thread
From: Jason Wang @ 2019-10-12 7:54 UTC (permalink / raw)
To: prashantbhole.linux, Michael S . Tsirkin, David S . Miller
Cc: David Ahern, kvm, netdev
On 2019/10/12 上午9:53, prashantbhole.linux@gmail.com wrote:
> From: Prashant Bhole <prashantbhole.linux@gmail.com>
>
> Currently vhost_net directly accesses ptr ring of tap driver to
> fetch Rx packet pointers. In order to avoid it this patch modifies
> tap driver's recvmsg api to do additional task of fetching Rx packet
> pointers.
>
> A special struct tun_msg_ctl is already being usedd via msg_control
> for tun Rx XDP batching. This patch extends tun_msg_ctl usage to
> send sub commands to recvmsg api. recvmsg can now produce/unproduce
> pointers from ptr ring as an additional task.
>
> This will be useful in future in implementation of virtio-net XDP
> offload feature. Where packets will be XDP batch processed in
> tun_recvmsg.
>
> Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
> ---
> drivers/net/tap.c | 22 +++++++++++++++++++-
> drivers/net/tun.c | 24 +++++++++++++++++++++-
> drivers/vhost/net.c | 46 +++++++++++++++++++++++++++++++++---------
> include/linux/if_tun.h | 3 +++
> 4 files changed, 83 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index 01bd260ce60c..3d0bf382dbbc 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -1234,8 +1234,28 @@ static int tap_recvmsg(struct socket *sock, struct msghdr *m,
> size_t total_len, int flags)
> {
> struct tap_queue *q = container_of(sock, struct tap_queue, sock);
> - struct sk_buff *skb = m->msg_control;
> + struct tun_msg_ctl *ctl = m->msg_control;
> + struct sk_buff *skb = NULL;
> int ret;
> +
> + if (ctl) {
> + switch (ctl->cmd) {
> + case TUN_CMD_PACKET:
> + skb = ctl->ptr;
> + break;
> + case TUN_CMD_PRODUCE_PTRS:
I think we need a better name of the command to avoid uncovering the
implementation details of the lower socket.
> + return ptr_ring_consume_batched(&q->ring,
> + ctl->ptr_array,
> + ctl->num);
> + case TUN_CMD_UNPRODUCE_PTRS:
> + ptr_ring_unconsume(&q->ring, ctl->ptr_array, ctl->num,
> + tun_ptr_free);
> + return 0;
> + default:
> + return -EINVAL;
> + }
> + }
> +
> if (flags & ~(MSG_DONTWAIT|MSG_TRUNC)) {
> kfree_skb(skb);
> return -EINVAL;
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 29711671959b..7d4886f53389 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -2577,7 +2577,8 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
> {
> struct tun_file *tfile = container_of(sock, struct tun_file, socket);
> struct tun_struct *tun = tun_get(tfile);
> - void *ptr = m->msg_control;
> + struct tun_msg_ctl *ctl = m->msg_control;
> + void *ptr = NULL;
> int ret;
>
> if (!tun) {
> @@ -2585,6 +2586,27 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
> goto out_free;
> }
>
> + if (ctl) {
> + switch (ctl->cmd) {
> + case TUN_CMD_PACKET:
> + ptr = ctl->ptr;
> + break;
> + case TUN_CMD_PRODUCE_PTRS:
> + ret = ptr_ring_consume_batched(&tfile->tx_ring,
> + ctl->ptr_array,
> + ctl->num);
> + goto out;
> + case TUN_CMD_UNPRODUCE_PTRS:
> + ptr_ring_unconsume(&tfile->tx_ring, ctl->ptr_array,
> + ctl->num, tun_ptr_free);
> + ret = 0;
> + goto out;
> + default:
> + ret = -EINVAL;
> + goto out_put_tun;
> + }
> + }
> +
> if (flags & ~(MSG_DONTWAIT|MSG_TRUNC|MSG_ERRQUEUE)) {
> ret = -EINVAL;
> goto out_put_tun;
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 5946d2775bd0..5e5c1063606c 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -175,24 +175,44 @@ static void *vhost_net_buf_consume(struct vhost_net_buf *rxq)
>
> static int vhost_net_buf_produce(struct vhost_net_virtqueue *nvq)
> {
> + struct vhost_virtqueue *vq = &nvq->vq;
> + struct socket *sock = vq->private_data;
> struct vhost_net_buf *rxq = &nvq->rxq;
> + struct tun_msg_ctl ctl = {
> + .cmd = TUN_CMD_PRODUCE_PTRS,
> + .ptr_array = rxq->queue,
> + .num = VHOST_NET_BATCH,
> + };
> + struct msghdr msg = {
> + .msg_control = &ctl,
> + };
>
> rxq->head = 0;
> - rxq->tail = ptr_ring_consume_batched(nvq->rx_ring, rxq->queue,
> - VHOST_NET_BATCH);
> + rxq->tail = sock->ops->recvmsg(sock, &msg, 0, 0);
> + if (rxq->tail < 0)
> + rxq->tail = 0;
> +
> return rxq->tail;
> }
>
> static void vhost_net_buf_unproduce(struct vhost_net_virtqueue *nvq)
> {
> + struct vhost_virtqueue *vq = &nvq->vq;
> + struct socket *sock = vq->private_data;
> struct vhost_net_buf *rxq = &nvq->rxq;
> + struct tun_msg_ctl ctl = {
> + .cmd = TUN_CMD_UNPRODUCE_PTRS,
> + .ptr_array = rxq->queue + rxq->head,
> + .num = vhost_net_buf_get_size(rxq),
> + };
> + struct msghdr msg = {
> + .msg_control = &ctl,
> + };
>
> - if (nvq->rx_ring && !vhost_net_buf_is_empty(rxq)) {
> - ptr_ring_unconsume(nvq->rx_ring, rxq->queue + rxq->head,
> - vhost_net_buf_get_size(rxq),
> - tun_ptr_free);
> - rxq->head = rxq->tail = 0;
> - }
> + if (!vhost_net_buf_is_empty(rxq))
> + sock->ops->recvmsg(sock, &msg, 0, 0);
> +
> + rxq->head = rxq->tail = 0;
> }
>
> static int vhost_net_buf_peek_len(void *ptr)
> @@ -1109,6 +1129,9 @@ static void handle_rx(struct vhost_net *net)
> .flags = 0,
> .gso_type = VIRTIO_NET_HDR_GSO_NONE
> };
> + struct tun_msg_ctl ctl = {
> + .cmd = TUN_CMD_PACKET,
> + };
> size_t total_len = 0;
> int err, mergeable;
> s16 headcount;
> @@ -1166,8 +1189,11 @@ static void handle_rx(struct vhost_net *net)
> goto out;
> }
> busyloop_intr = false;
> - if (nvq->rx_ring)
> - msg.msg_control = vhost_net_buf_consume(&nvq->rxq);
> + if (nvq->rx_ring) {
> + ctl.cmd = TUN_CMD_PACKET;
Looks duplicated with the above initialization.
> + ctl.ptr = vhost_net_buf_consume(&nvq->rxq);
> + msg.msg_control = &ctl;
> + }
> /* On overrun, truncate and discard */
> if (unlikely(headcount > UIO_MAXIOV)) {
> iov_iter_init(&msg.msg_iter, READ, vq->iov, 1, 1);
> diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
> index bdfa671612db..8608d4095143 100644
> --- a/include/linux/if_tun.h
> +++ b/include/linux/if_tun.h
> @@ -13,10 +13,13 @@
>
> #define TUN_CMD_PACKET 1
> #define TUN_CMD_BATCH 2
> +#define TUN_CMD_PRODUCE_PTRS 3
> +#define TUN_CMD_UNPRODUCE_PTRS 4
> struct tun_msg_ctl {
> unsigned short cmd;
> unsigned short num;
> void *ptr;
> + void **ptr_array;
Using a union or just using void * is sufficient (rename it to void *data)?
Thanks
> };
>
> struct tun_xdp_hdr {
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 0/3] vhost_net: access ptr ring using tap recvmsg
2019-10-12 1:53 [PATCH net-next 0/3] vhost_net: access ptr ring using tap recvmsg prashantbhole.linux
` (2 preceding siblings ...)
2019-10-12 1:53 ` [PATCH net-next 3/3] tuntap: remove usage of ptr ring in vhost_net prashantbhole.linux
@ 2019-10-12 7:57 ` Jason Wang
2019-10-12 20:38 ` Michael S. Tsirkin
3 siblings, 1 reply; 11+ messages in thread
From: Jason Wang @ 2019-10-12 7:57 UTC (permalink / raw)
To: prashantbhole.linux, Michael S . Tsirkin, David S . Miller
Cc: David Ahern, kvm, netdev
On 2019/10/12 上午9:53, prashantbhole.linux@gmail.com wrote:
> From: Prashant Bhole <prashantbhole.linux@gmail.com>
>
> vhost_net needs to peek tun packet sizes to allocate virtio buffers.
> Currently it directly accesses tap ptr ring to do it. Jason Wang
> suggested to achieve this using msghdr->msg_control and modifying the
> behavior of tap recvmsg.
Note this may use more indirect calls, this could be optimized in the
future by doing XDP/skb receiving by vhost_net its own.
>
> This change will be useful in future in case of virtio-net XDP
> offload. Where packets will be XDP processed in tap recvmsg and vhost
> will see only non XDP_DROP'ed packets.
>
> Patch 1: reorganizes the tun_msg_ctl so that it can be extended by
> the means of different commands. tap sendmsg recvmsg will behave
> according to commands.
>
> Patch 2: modifies recvmsg implementation to produce packet pointers.
> vhost_net uses recvmsg API instead of ptr_ring_consume().
>
> Patch 3: removes ptr ring usage in vhost and functions those export
> ptr ring from tun/tap.
>
> Prashant Bhole (3):
> tuntap: reorganize tun_msg_ctl usage
> vhost_net: user tap recvmsg api to access ptr ring
> tuntap: remove usage of ptr ring in vhost_net
>
> drivers/net/tap.c | 44 ++++++++++++++---------
> drivers/net/tun.c | 45 +++++++++++++++---------
> drivers/vhost/net.c | 79 ++++++++++++++++++++++--------------------
> include/linux/if_tun.h | 9 +++--
> 4 files changed, 103 insertions(+), 74 deletions(-)
It would be helpful that if you can share some performance numbers here.
Thanks
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 0/3] vhost_net: access ptr ring using tap recvmsg
2019-10-12 7:57 ` [PATCH net-next 0/3] vhost_net: access ptr ring using tap recvmsg Jason Wang
@ 2019-10-12 20:38 ` Michael S. Tsirkin
0 siblings, 0 replies; 11+ messages in thread
From: Michael S. Tsirkin @ 2019-10-12 20:38 UTC (permalink / raw)
To: Jason Wang
Cc: prashantbhole.linux, David S . Miller, David Ahern, kvm, netdev
On Sat, Oct 12, 2019 at 03:57:21PM +0800, Jason Wang wrote:
>
> On 2019/10/12 上午9:53, prashantbhole.linux@gmail.com wrote:
> > From: Prashant Bhole <prashantbhole.linux@gmail.com>
> >
> > vhost_net needs to peek tun packet sizes to allocate virtio buffers.
> > Currently it directly accesses tap ptr ring to do it. Jason Wang
> > suggested to achieve this using msghdr->msg_control and modifying the
> > behavior of tap recvmsg.
>
>
> Note this may use more indirect calls, this could be optimized in the future
> by doing XDP/skb receiving by vhost_net its own.
So it looks like this is going in the reverse direction,
moving more data path code from vhost to tun.
What's the point of the patchset then?
>
> >
> > This change will be useful in future in case of virtio-net XDP
> > offload. Where packets will be XDP processed in tap recvmsg and vhost
> > will see only non XDP_DROP'ed packets.
> >
> > Patch 1: reorganizes the tun_msg_ctl so that it can be extended by
> > the means of different commands. tap sendmsg recvmsg will behave
> > according to commands.
> >
> > Patch 2: modifies recvmsg implementation to produce packet pointers.
> > vhost_net uses recvmsg API instead of ptr_ring_consume().
> >
> > Patch 3: removes ptr ring usage in vhost and functions those export
> > ptr ring from tun/tap.
> >
> > Prashant Bhole (3):
> > tuntap: reorganize tun_msg_ctl usage
> > vhost_net: user tap recvmsg api to access ptr ring
> > tuntap: remove usage of ptr ring in vhost_net
> >
> > drivers/net/tap.c | 44 ++++++++++++++---------
> > drivers/net/tun.c | 45 +++++++++++++++---------
> > drivers/vhost/net.c | 79 ++++++++++++++++++++++--------------------
> > include/linux/if_tun.h | 9 +++--
> > 4 files changed, 103 insertions(+), 74 deletions(-)
>
>
> It would be helpful that if you can share some performance numbers here.
>
> Thanks
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 2/3] vhost_net: user tap recvmsg api to access ptr ring
2019-10-12 1:53 ` [PATCH net-next 2/3] vhost_net: user tap recvmsg api to access ptr ring prashantbhole.linux
2019-10-12 7:54 ` Jason Wang
@ 2019-10-12 20:41 ` Michael S. Tsirkin
2019-10-15 0:57 ` Prashant Bhole
1 sibling, 1 reply; 11+ messages in thread
From: Michael S. Tsirkin @ 2019-10-12 20:41 UTC (permalink / raw)
To: prashantbhole.linux
Cc: Jason Wang, David S . Miller, David Ahern, kvm, netdev
On Sat, Oct 12, 2019 at 10:53:56AM +0900, prashantbhole.linux@gmail.com wrote:
> From: Prashant Bhole <prashantbhole.linux@gmail.com>
>
> Currently vhost_net directly accesses ptr ring of tap driver to
> fetch Rx packet pointers. In order to avoid it this patch modifies
> tap driver's recvmsg api to do additional task of fetching Rx packet
> pointers.
>
> A special struct tun_msg_ctl is already being usedd via msg_control
> for tun Rx XDP batching. This patch extends tun_msg_ctl usage to
> send sub commands to recvmsg api. recvmsg can now produce/unproduce
> pointers from ptr ring as an additional task.
>
> This will be useful in future in implementation of virtio-net XDP
> offload feature. Where packets will be XDP batch processed in
> tun_recvmsg.
I'd like to see that future patch, by itself this patchset
seems to be of limited usefulness.
> Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
> ---
> drivers/net/tap.c | 22 +++++++++++++++++++-
> drivers/net/tun.c | 24 +++++++++++++++++++++-
> drivers/vhost/net.c | 46 +++++++++++++++++++++++++++++++++---------
> include/linux/if_tun.h | 3 +++
> 4 files changed, 83 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index 01bd260ce60c..3d0bf382dbbc 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -1234,8 +1234,28 @@ static int tap_recvmsg(struct socket *sock, struct msghdr *m,
> size_t total_len, int flags)
> {
> struct tap_queue *q = container_of(sock, struct tap_queue, sock);
> - struct sk_buff *skb = m->msg_control;
> + struct tun_msg_ctl *ctl = m->msg_control;
> + struct sk_buff *skb = NULL;
> int ret;
> +
> + if (ctl) {
> + switch (ctl->cmd) {
> + case TUN_CMD_PACKET:
> + skb = ctl->ptr;
> + break;
> + case TUN_CMD_PRODUCE_PTRS:
> + return ptr_ring_consume_batched(&q->ring,
> + ctl->ptr_array,
> + ctl->num);
> + case TUN_CMD_UNPRODUCE_PTRS:
> + ptr_ring_unconsume(&q->ring, ctl->ptr_array, ctl->num,
> + tun_ptr_free);
> + return 0;
> + default:
> + return -EINVAL;
> + }
> + }
> +
> if (flags & ~(MSG_DONTWAIT|MSG_TRUNC)) {
> kfree_skb(skb);
> return -EINVAL;
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 29711671959b..7d4886f53389 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -2577,7 +2577,8 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
> {
> struct tun_file *tfile = container_of(sock, struct tun_file, socket);
> struct tun_struct *tun = tun_get(tfile);
> - void *ptr = m->msg_control;
> + struct tun_msg_ctl *ctl = m->msg_control;
> + void *ptr = NULL;
> int ret;
>
> if (!tun) {
> @@ -2585,6 +2586,27 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
> goto out_free;
> }
>
> + if (ctl) {
> + switch (ctl->cmd) {
> + case TUN_CMD_PACKET:
> + ptr = ctl->ptr;
> + break;
> + case TUN_CMD_PRODUCE_PTRS:
> + ret = ptr_ring_consume_batched(&tfile->tx_ring,
> + ctl->ptr_array,
> + ctl->num);
> + goto out;
> + case TUN_CMD_UNPRODUCE_PTRS:
> + ptr_ring_unconsume(&tfile->tx_ring, ctl->ptr_array,
> + ctl->num, tun_ptr_free);
> + ret = 0;
> + goto out;
> + default:
> + ret = -EINVAL;
> + goto out_put_tun;
> + }
> + }
> +
> if (flags & ~(MSG_DONTWAIT|MSG_TRUNC|MSG_ERRQUEUE)) {
> ret = -EINVAL;
> goto out_put_tun;
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 5946d2775bd0..5e5c1063606c 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -175,24 +175,44 @@ static void *vhost_net_buf_consume(struct vhost_net_buf *rxq)
>
> static int vhost_net_buf_produce(struct vhost_net_virtqueue *nvq)
> {
> + struct vhost_virtqueue *vq = &nvq->vq;
> + struct socket *sock = vq->private_data;
> struct vhost_net_buf *rxq = &nvq->rxq;
> + struct tun_msg_ctl ctl = {
> + .cmd = TUN_CMD_PRODUCE_PTRS,
> + .ptr_array = rxq->queue,
> + .num = VHOST_NET_BATCH,
> + };
> + struct msghdr msg = {
> + .msg_control = &ctl,
> + };
>
> rxq->head = 0;
> - rxq->tail = ptr_ring_consume_batched(nvq->rx_ring, rxq->queue,
> - VHOST_NET_BATCH);
> + rxq->tail = sock->ops->recvmsg(sock, &msg, 0, 0);
> + if (rxq->tail < 0)
> + rxq->tail = 0;
> +
> return rxq->tail;
> }
>
> static void vhost_net_buf_unproduce(struct vhost_net_virtqueue *nvq)
> {
> + struct vhost_virtqueue *vq = &nvq->vq;
> + struct socket *sock = vq->private_data;
> struct vhost_net_buf *rxq = &nvq->rxq;
> + struct tun_msg_ctl ctl = {
> + .cmd = TUN_CMD_UNPRODUCE_PTRS,
> + .ptr_array = rxq->queue + rxq->head,
> + .num = vhost_net_buf_get_size(rxq),
> + };
> + struct msghdr msg = {
> + .msg_control = &ctl,
> + };
>
> - if (nvq->rx_ring && !vhost_net_buf_is_empty(rxq)) {
> - ptr_ring_unconsume(nvq->rx_ring, rxq->queue + rxq->head,
> - vhost_net_buf_get_size(rxq),
> - tun_ptr_free);
> - rxq->head = rxq->tail = 0;
> - }
> + if (!vhost_net_buf_is_empty(rxq))
> + sock->ops->recvmsg(sock, &msg, 0, 0);
> +
> + rxq->head = rxq->tail = 0;
> }
>
> static int vhost_net_buf_peek_len(void *ptr)
> @@ -1109,6 +1129,9 @@ static void handle_rx(struct vhost_net *net)
> .flags = 0,
> .gso_type = VIRTIO_NET_HDR_GSO_NONE
> };
> + struct tun_msg_ctl ctl = {
> + .cmd = TUN_CMD_PACKET,
> + };
> size_t total_len = 0;
> int err, mergeable;
> s16 headcount;
> @@ -1166,8 +1189,11 @@ static void handle_rx(struct vhost_net *net)
> goto out;
> }
> busyloop_intr = false;
> - if (nvq->rx_ring)
> - msg.msg_control = vhost_net_buf_consume(&nvq->rxq);
> + if (nvq->rx_ring) {
> + ctl.cmd = TUN_CMD_PACKET;
> + ctl.ptr = vhost_net_buf_consume(&nvq->rxq);
> + msg.msg_control = &ctl;
> + }
> /* On overrun, truncate and discard */
> if (unlikely(headcount > UIO_MAXIOV)) {
> iov_iter_init(&msg.msg_iter, READ, vq->iov, 1, 1);
> diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
> index bdfa671612db..8608d4095143 100644
> --- a/include/linux/if_tun.h
> +++ b/include/linux/if_tun.h
> @@ -13,10 +13,13 @@
>
> #define TUN_CMD_PACKET 1
> #define TUN_CMD_BATCH 2
> +#define TUN_CMD_PRODUCE_PTRS 3
> +#define TUN_CMD_UNPRODUCE_PTRS 4
> struct tun_msg_ctl {
> unsigned short cmd;
> unsigned short num;
> void *ptr;
> + void **ptr_array;
> };
>
> struct tun_xdp_hdr {
> --
> 2.21.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 1/3] tuntap: reorganize tun_msg_ctl usage
2019-10-12 7:44 ` Jason Wang
@ 2019-10-15 0:33 ` Prashant Bhole
0 siblings, 0 replies; 11+ messages in thread
From: Prashant Bhole @ 2019-10-15 0:33 UTC (permalink / raw)
To: Jason Wang, Michael S . Tsirkin, David S . Miller
Cc: David Ahern, kvm, netdev
Hi Jason,
Thanks for reviewing.
On 10/12/19 4:44 PM, Jason Wang wrote:
>
> On 2019/10/12 上午9:53, prashantbhole.linux@gmail.com wrote:
>> From: Prashant Bhole <prashantbhole.linux@gmail.com>
>>
>> In order to extend the usage of tun_msg_ctl structure, this patch
>> changes the member name from type to cmd. Also following definitions
>> are changed:
>> TUN_MSG_PTR : TUN_CMD_BATCH
>> TUN_MSG_UBUF: TUN_CMD_PACKET
>
>
> Not a native English speaker, but the conversion here looks not as
> straightforward as it did.
>
> For TUN_MSG_PTR, it means recvmsg() can do receiving from a pointer to
> either XDP or skb instead of ptr_ring. TUN_CMD_BATCH sounds not related.
>
> For TUN_MSG_UBUF, it means the packet is a zercopy (buffer pointers to
> userspace). TUN_CMD_PACKET may bring confusion in this case.
Understood. Next time I will come up with better command names,
performance numbers and other changes suggested by you.
Thanks.
>
> Thanks
>
>
>>
>> Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
>> ---
>> drivers/net/tap.c | 9 ++++++---
>> drivers/net/tun.c | 8 ++++++--
>> drivers/vhost/net.c | 4 ++--
>> include/linux/if_tun.h | 6 +++---
>> 4 files changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
>> index 3ae70c7e6860..01bd260ce60c 100644
>> --- a/drivers/net/tap.c
>> +++ b/drivers/net/tap.c
>> @@ -1213,9 +1213,10 @@ static int tap_sendmsg(struct socket *sock,
>> struct msghdr *m,
>> struct tap_queue *q = container_of(sock, struct tap_queue, sock);
>> struct tun_msg_ctl *ctl = m->msg_control;
>> struct xdp_buff *xdp;
>> + void *ptr = NULL;
>> int i;
>> - if (ctl && (ctl->type == TUN_MSG_PTR)) {
>> + if (ctl && ctl->cmd == TUN_CMD_BATCH) {
>> for (i = 0; i < ctl->num; i++) {
>> xdp = &((struct xdp_buff *)ctl->ptr)[i];
>> tap_get_user_xdp(q, xdp);
>> @@ -1223,8 +1224,10 @@ static int tap_sendmsg(struct socket *sock,
>> struct msghdr *m,
>> return 0;
>> }
>> - return tap_get_user(q, ctl ? ctl->ptr : NULL, &m->msg_iter,
>> - m->msg_flags & MSG_DONTWAIT);
>> + if (ctl && ctl->cmd == TUN_CMD_PACKET)
>> + ptr = ctl->ptr;
>> +
>> + return tap_get_user(q, ptr, &m->msg_iter, m->msg_flags &
>> MSG_DONTWAIT);
>> }
>> static int tap_recvmsg(struct socket *sock, struct msghdr *m,
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index 0413d182d782..29711671959b 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -2529,11 +2529,12 @@ static int tun_sendmsg(struct socket *sock,
>> struct msghdr *m, size_t total_len)
>> struct tun_struct *tun = tun_get(tfile);
>> struct tun_msg_ctl *ctl = m->msg_control;
>> struct xdp_buff *xdp;
>> + void *ptr = NULL;
>> if (!tun)
>> return -EBADFD;
>> - if (ctl && (ctl->type == TUN_MSG_PTR)) {
>> + if (ctl && ctl->cmd == TUN_CMD_BATCH) {
>> struct tun_page tpage;
>> int n = ctl->num;
>> int flush = 0;
>> @@ -2560,7 +2561,10 @@ static int tun_sendmsg(struct socket *sock,
>> struct msghdr *m, size_t total_len)
>> goto out;
>> }
>> - ret = tun_get_user(tun, tfile, ctl ? ctl->ptr : NULL, &m->msg_iter,
>> + if (ctl && ctl->cmd == TUN_CMD_PACKET)
>> + ptr = ctl->ptr;
>> +
>> + ret = tun_get_user(tun, tfile, ptr, &m->msg_iter,
>> m->msg_flags & MSG_DONTWAIT,
>> m->msg_flags & MSG_MORE);
>> out:
>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> index 1a2dd53caade..5946d2775bd0 100644
>> --- a/drivers/vhost/net.c
>> +++ b/drivers/vhost/net.c
>> @@ -462,7 +462,7 @@ static void vhost_tx_batch(struct vhost_net *net,
>> struct msghdr *msghdr)
>> {
>> struct tun_msg_ctl ctl = {
>> - .type = TUN_MSG_PTR,
>> + .cmd = TUN_CMD_BATCH,
>> .num = nvq->batched_xdp,
>> .ptr = nvq->xdp,
>> };
>> @@ -902,7 +902,7 @@ static void handle_tx_zerocopy(struct vhost_net
>> *net, struct socket *sock)
>> ubuf->desc = nvq->upend_idx;
>> refcount_set(&ubuf->refcnt, 1);
>> msg.msg_control = &ctl;
>> - ctl.type = TUN_MSG_UBUF;
>> + ctl.cmd = TUN_CMD_PACKET;
>> ctl.ptr = ubuf;
>> msg.msg_controllen = sizeof(ctl);
>> ubufs = nvq->ubufs;
>> diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
>> index 5bda8cf457b6..bdfa671612db 100644
>> --- a/include/linux/if_tun.h
>> +++ b/include/linux/if_tun.h
>> @@ -11,10 +11,10 @@
>> #define TUN_XDP_FLAG 0x1UL
>> -#define TUN_MSG_UBUF 1
>> -#define TUN_MSG_PTR 2
>> +#define TUN_CMD_PACKET 1
>> +#define TUN_CMD_BATCH 2
>> struct tun_msg_ctl {
>> - unsigned short type;
>> + unsigned short cmd;
>> unsigned short num;
>> void *ptr;
>> };
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 2/3] vhost_net: user tap recvmsg api to access ptr ring
2019-10-12 20:41 ` Michael S. Tsirkin
@ 2019-10-15 0:57 ` Prashant Bhole
0 siblings, 0 replies; 11+ messages in thread
From: Prashant Bhole @ 2019-10-15 0:57 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Jason Wang, David S . Miller, David Ahern, kvm, netdev
Hi Michael,
Thanks for reviewing.
On 10/13/19 5:41 AM, Michael S. Tsirkin wrote:
> On Sat, Oct 12, 2019 at 10:53:56AM +0900, prashantbhole.linux@gmail.com wrote:
>> From: Prashant Bhole <prashantbhole.linux@gmail.com>
>>
>> Currently vhost_net directly accesses ptr ring of tap driver to
>> fetch Rx packet pointers. In order to avoid it this patch modifies
>> tap driver's recvmsg api to do additional task of fetching Rx packet
>> pointers.
>>
>> A special struct tun_msg_ctl is already being usedd via msg_control
>> for tun Rx XDP batching. This patch extends tun_msg_ctl usage to
>> send sub commands to recvmsg api. recvmsg can now produce/unproduce
>> pointers from ptr ring as an additional task.
>>
>> This will be useful in future in implementation of virtio-net XDP
>> offload feature. Where packets will be XDP batch processed in
>> tun_recvmsg.
>
> I'd like to see that future patch, by itself this patchset
> seems to be of limited usefulness.
Agree, this set is just a reorganization. Next time this will be a part
of set which actually uses it.
Thanks
>
>> Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
>> ---
>> drivers/net/tap.c | 22 +++++++++++++++++++-
>> drivers/net/tun.c | 24 +++++++++++++++++++++-
>> drivers/vhost/net.c | 46 +++++++++++++++++++++++++++++++++---------
>> include/linux/if_tun.h | 3 +++
>> 4 files changed, 83 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
>> index 01bd260ce60c..3d0bf382dbbc 100644
>> --- a/drivers/net/tap.c
>> +++ b/drivers/net/tap.c
>> @@ -1234,8 +1234,28 @@ static int tap_recvmsg(struct socket *sock, struct msghdr *m,
>> size_t total_len, int flags)
>> {
>> struct tap_queue *q = container_of(sock, struct tap_queue, sock);
>> - struct sk_buff *skb = m->msg_control;
>> + struct tun_msg_ctl *ctl = m->msg_control;
>> + struct sk_buff *skb = NULL;
>> int ret;
>> +
>> + if (ctl) {
>> + switch (ctl->cmd) {
>> + case TUN_CMD_PACKET:
>> + skb = ctl->ptr;
>> + break;
>> + case TUN_CMD_PRODUCE_PTRS:
>> + return ptr_ring_consume_batched(&q->ring,
>> + ctl->ptr_array,
>> + ctl->num);
>> + case TUN_CMD_UNPRODUCE_PTRS:
>> + ptr_ring_unconsume(&q->ring, ctl->ptr_array, ctl->num,
>> + tun_ptr_free);
>> + return 0;
>> + default:
>> + return -EINVAL;
>> + }
>> + }
>> +
>> if (flags & ~(MSG_DONTWAIT|MSG_TRUNC)) {
>> kfree_skb(skb);
>> return -EINVAL;
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index 29711671959b..7d4886f53389 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -2577,7 +2577,8 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
>> {
>> struct tun_file *tfile = container_of(sock, struct tun_file, socket);
>> struct tun_struct *tun = tun_get(tfile);
>> - void *ptr = m->msg_control;
>> + struct tun_msg_ctl *ctl = m->msg_control;
>> + void *ptr = NULL;
>> int ret;
>>
>> if (!tun) {
>> @@ -2585,6 +2586,27 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
>> goto out_free;
>> }
>>
>> + if (ctl) {
>> + switch (ctl->cmd) {
>> + case TUN_CMD_PACKET:
>> + ptr = ctl->ptr;
>> + break;
>> + case TUN_CMD_PRODUCE_PTRS:
>> + ret = ptr_ring_consume_batched(&tfile->tx_ring,
>> + ctl->ptr_array,
>> + ctl->num);
>> + goto out;
>> + case TUN_CMD_UNPRODUCE_PTRS:
>> + ptr_ring_unconsume(&tfile->tx_ring, ctl->ptr_array,
>> + ctl->num, tun_ptr_free);
>> + ret = 0;
>> + goto out;
>> + default:
>> + ret = -EINVAL;
>> + goto out_put_tun;
>> + }
>> + }
>> +
>> if (flags & ~(MSG_DONTWAIT|MSG_TRUNC|MSG_ERRQUEUE)) {
>> ret = -EINVAL;
>> goto out_put_tun;
>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> index 5946d2775bd0..5e5c1063606c 100644
>> --- a/drivers/vhost/net.c
>> +++ b/drivers/vhost/net.c
>> @@ -175,24 +175,44 @@ static void *vhost_net_buf_consume(struct vhost_net_buf *rxq)
>>
>> static int vhost_net_buf_produce(struct vhost_net_virtqueue *nvq)
>> {
>> + struct vhost_virtqueue *vq = &nvq->vq;
>> + struct socket *sock = vq->private_data;
>> struct vhost_net_buf *rxq = &nvq->rxq;
>> + struct tun_msg_ctl ctl = {
>> + .cmd = TUN_CMD_PRODUCE_PTRS,
>> + .ptr_array = rxq->queue,
>> + .num = VHOST_NET_BATCH,
>> + };
>> + struct msghdr msg = {
>> + .msg_control = &ctl,
>> + };
>>
>> rxq->head = 0;
>> - rxq->tail = ptr_ring_consume_batched(nvq->rx_ring, rxq->queue,
>> - VHOST_NET_BATCH);
>> + rxq->tail = sock->ops->recvmsg(sock, &msg, 0, 0);
>> + if (rxq->tail < 0)
>> + rxq->tail = 0;
>> +
>> return rxq->tail;
>> }
>>
>> static void vhost_net_buf_unproduce(struct vhost_net_virtqueue *nvq)
>> {
>> + struct vhost_virtqueue *vq = &nvq->vq;
>> + struct socket *sock = vq->private_data;
>> struct vhost_net_buf *rxq = &nvq->rxq;
>> + struct tun_msg_ctl ctl = {
>> + .cmd = TUN_CMD_UNPRODUCE_PTRS,
>> + .ptr_array = rxq->queue + rxq->head,
>> + .num = vhost_net_buf_get_size(rxq),
>> + };
>> + struct msghdr msg = {
>> + .msg_control = &ctl,
>> + };
>>
>> - if (nvq->rx_ring && !vhost_net_buf_is_empty(rxq)) {
>> - ptr_ring_unconsume(nvq->rx_ring, rxq->queue + rxq->head,
>> - vhost_net_buf_get_size(rxq),
>> - tun_ptr_free);
>> - rxq->head = rxq->tail = 0;
>> - }
>> + if (!vhost_net_buf_is_empty(rxq))
>> + sock->ops->recvmsg(sock, &msg, 0, 0);
>> +
>> + rxq->head = rxq->tail = 0;
>> }
>>
>> static int vhost_net_buf_peek_len(void *ptr)
>> @@ -1109,6 +1129,9 @@ static void handle_rx(struct vhost_net *net)
>> .flags = 0,
>> .gso_type = VIRTIO_NET_HDR_GSO_NONE
>> };
>> + struct tun_msg_ctl ctl = {
>> + .cmd = TUN_CMD_PACKET,
>> + };
>> size_t total_len = 0;
>> int err, mergeable;
>> s16 headcount;
>> @@ -1166,8 +1189,11 @@ static void handle_rx(struct vhost_net *net)
>> goto out;
>> }
>> busyloop_intr = false;
>> - if (nvq->rx_ring)
>> - msg.msg_control = vhost_net_buf_consume(&nvq->rxq);
>> + if (nvq->rx_ring) {
>> + ctl.cmd = TUN_CMD_PACKET;
>> + ctl.ptr = vhost_net_buf_consume(&nvq->rxq);
>> + msg.msg_control = &ctl;
>> + }
>> /* On overrun, truncate and discard */
>> if (unlikely(headcount > UIO_MAXIOV)) {
>> iov_iter_init(&msg.msg_iter, READ, vq->iov, 1, 1);
>> diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
>> index bdfa671612db..8608d4095143 100644
>> --- a/include/linux/if_tun.h
>> +++ b/include/linux/if_tun.h
>> @@ -13,10 +13,13 @@
>>
>> #define TUN_CMD_PACKET 1
>> #define TUN_CMD_BATCH 2
>> +#define TUN_CMD_PRODUCE_PTRS 3
>> +#define TUN_CMD_UNPRODUCE_PTRS 4
>> struct tun_msg_ctl {
>> unsigned short cmd;
>> unsigned short num;
>> void *ptr;
>> + void **ptr_array;
>> };
>>
>> struct tun_xdp_hdr {
>> --
>> 2.21.0
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-10-15 0:57 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-12 1:53 [PATCH net-next 0/3] vhost_net: access ptr ring using tap recvmsg prashantbhole.linux
2019-10-12 1:53 ` [PATCH net-next 1/3] tuntap: reorganize tun_msg_ctl usage prashantbhole.linux
2019-10-12 7:44 ` Jason Wang
2019-10-15 0:33 ` Prashant Bhole
2019-10-12 1:53 ` [PATCH net-next 2/3] vhost_net: user tap recvmsg api to access ptr ring prashantbhole.linux
2019-10-12 7:54 ` Jason Wang
2019-10-12 20:41 ` Michael S. Tsirkin
2019-10-15 0:57 ` Prashant Bhole
2019-10-12 1:53 ` [PATCH net-next 3/3] tuntap: remove usage of ptr ring in vhost_net prashantbhole.linux
2019-10-12 7:57 ` [PATCH net-next 0/3] vhost_net: access ptr ring using tap recvmsg Jason Wang
2019-10-12 20:38 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).