| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4141e1c0-618a-978f-0188-7bebeb2c4e70@redhat.com>
Date: Fri, 7 Sep 2018 11:35:13 +0800
From: Jason Wang <jasowang@...hat.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
kvm@...r.kernel.org, virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH net-next 08/11] tun: switch to new type of msg_control
On 2018年09月07日 00:54, Michael S. Tsirkin wrote:
> On Thu, Sep 06, 2018 at 12:05:23PM +0800, Jason Wang wrote:
>> This patch introduces to a new tun/tap specific msg_control:
>>
>> #define TUN_MSG_UBUF 1
>> #define TUN_MSG_PTR 2
>> struct tun_msg_ctl {
>> int type;
>> void *ptr;
>> };
>>
>> This allows us to pass different kinds of msg_control through
>> sendmsg(). The first supported type is ubuf (TUN_MSG_UBUF) which will
>> be used by the existed vhost_net zerocopy code. The second is XDP
>> buff, which allows vhost_net to pass XDP buff to TUN. This could be
>> used to implement accepting an array of XDP buffs from vhost_net in
>> the following patches.
>>
>> Signed-off-by: Jason Wang <jasowang@...hat.com>
> At this point, do we want to just add a new sock opt for tap's
> benefit? Seems cleaner than (ab)using sendmsg.
I think it won't be much difference, we still need a void pointer.
>> ---
>> drivers/net/tap.c | 18 ++++++++++++------
>> drivers/net/tun.c | 6 +++++-
>> drivers/vhost/net.c | 7 +++++--
>> include/linux/if_tun.h | 7 +++++++
>> 4 files changed, 29 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
>> index f0f7cd977667..7996ed7cbf18 100644
>> --- a/drivers/net/tap.c
>> +++ b/drivers/net/tap.c
>> @@ -619,7 +619,7 @@ static inline struct sk_buff *tap_alloc_skb(struct sock *sk, size_t prepad,
>> #define TAP_RESERVE HH_DATA_OFF(ETH_HLEN)
>>
>> /* Get packet from user space buffer */
>> -static ssize_t tap_get_user(struct tap_queue *q, struct msghdr *m,
>> +static ssize_t tap_get_user(struct tap_queue *q, void *msg_control,
>> struct iov_iter *from, int noblock)
>> {
>> int good_linear = SKB_MAX_HEAD(TAP_RESERVE);
>> @@ -663,7 +663,7 @@ static ssize_t tap_get_user(struct tap_queue *q, struct msghdr *m,
>> if (unlikely(len < ETH_HLEN))
>> goto err;
>>
>> - if (m && m->msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY)) {
>> + if (msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY)) {
>> struct iov_iter i;
>>
>> copylen = vnet_hdr.hdr_len ?
>> @@ -724,11 +724,11 @@ static ssize_t tap_get_user(struct tap_queue *q, struct msghdr *m,
>> tap = rcu_dereference(q->tap);
>> /* copy skb_ubuf_info for callback when skb has no error */
>> if (zerocopy) {
>> - skb_shinfo(skb)->destructor_arg = m->msg_control;
>> + skb_shinfo(skb)->destructor_arg = msg_control;
>> skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
>> skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
>> - } else if (m && m->msg_control) {
>> - struct ubuf_info *uarg = m->msg_control;
>> + } else if (msg_control) {
>> + struct ubuf_info *uarg = msg_control;
>> uarg->callback(uarg, false);
>> }
>>
>> @@ -1150,7 +1150,13 @@ static int tap_sendmsg(struct socket *sock, struct msghdr *m,
>> size_t total_len)
>> {
>> struct tap_queue *q = container_of(sock, struct tap_queue, sock);
>> - return tap_get_user(q, m, &m->msg_iter, m->msg_flags & MSG_DONTWAIT);
>> + struct tun_msg_ctl *ctl = m->msg_control;
>> +
>> + if (ctl && ctl->type != TUN_MSG_UBUF)
>> + return -EINVAL;
>> +
>> + return tap_get_user(q, ctl ? ctl->ptr : NULL, &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 ff1cbf3ebd50..c839a4bdcbd9 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -2429,11 +2429,15 @@ static int tun_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len)
>> int ret;
>> struct tun_file *tfile = container_of(sock, struct tun_file, socket);
>> struct tun_struct *tun = tun_get(tfile);
>> + struct tun_msg_ctl *ctl = m->msg_control;
>>
>> if (!tun)
>> return -EBADFD;
>>
>> - ret = tun_get_user(tun, tfile, m->msg_control, &m->msg_iter,
>> + if (ctl && ctl->type != TUN_MSG_UBUF)
>> + return -EINVAL;
>> +
>> + ret = tun_get_user(tun, tfile, ctl ? ctl->ptr : NULL, &m->msg_iter,
>> m->msg_flags & MSG_DONTWAIT,
>> m->msg_flags & MSG_MORE);
>> tun_put(tun);
>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> index 4e656f89cb22..fb01ce6d981c 100644
>> --- a/drivers/vhost/net.c
>> +++ b/drivers/vhost/net.c
>> @@ -620,6 +620,7 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock)
>> .msg_controllen = 0,
>> .msg_flags = MSG_DONTWAIT,
>> };
>> + struct tun_msg_ctl ctl;
>> size_t len, total_len = 0;
>> int err;
>> struct vhost_net_ubuf_ref *uninitialized_var(ubufs);
>> @@ -664,8 +665,10 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock)
>> ubuf->ctx = nvq->ubufs;
>> ubuf->desc = nvq->upend_idx;
>> refcount_set(&ubuf->refcnt, 1);
>> - msg.msg_control = ubuf;
>> - msg.msg_controllen = sizeof(ubuf);
>> + msg.msg_control = &ctl;
>> + ctl.type = TUN_MSG_UBUF;
>> + ctl.ptr = ubuf;
>> + msg.msg_controllen = sizeof(ctl);
>> ubufs = nvq->ubufs;
>> atomic_inc(&ubufs->refcount);
>> nvq->upend_idx = (nvq->upend_idx + 1) % UIO_MAXIOV;
>> diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
>> index 3d2996dc7d85..ba46dced1f38 100644
>> --- a/include/linux/if_tun.h
>> +++ b/include/linux/if_tun.h
>> @@ -19,6 +19,13 @@
>>
>> #define TUN_XDP_FLAG 0x1UL
>>
>> +#define TUN_MSG_UBUF 1
>> +#define TUN_MSG_PTR 2
> Looks like TUN_MSG_PTR should be pushed out to a follow-up patch?
Ok.
>
>> +struct tun_msg_ctl {
>> + int type;
>> + void *ptr;
>> +};
>> +
> type actually includes a size. Why not two short fields then?
Yes, this sounds better.
Thanks
>
>> #if defined(CONFIG_TUN) || defined(CONFIG_TUN_MODULE)
>> struct socket *tun_get_socket(struct file *);
>> struct ptr_ring *tun_get_tx_ring(struct file *file);
>> --
>> 2.17.1
Powered by blists - more mailing lists