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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+FuTSdYB4rDMH3gAAMwaRdbqN58f_SxLz=C3fcCACw_KosGXw@mail.gmail.com>
Date:	Wed, 26 Nov 2014 14:59:34 -0500
From:	Willem de Bruijn <willemb@...gle.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>
Cc:	Network Development <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	Eric Dumazet <eric.dumazet@...il.com>,
	Daniel Borkmann <dborkman@...hat.com>
Subject: Re: [PATCH rfc] packet: zerocopy packet_snd

> The main problem with zero copy ATM is with queueing disciplines
> which might keep the socket around essentially forever.
> The case was described here:
> https://lkml.org/lkml/2014/1/17/105
> and of course this will make it more serious now that
> more applications will be able to do this, so
> chances that an administrator enables this
> are higher.

The denial of service issue raised there, that a single queue can
block an entire virtio-net device, is less problematic in the case of
packet sockets. A socket can run out of sk_wmem_alloc, but a prudent
application can increase the limit or use separate sockets for
separate flows.

> One possible solution is some kind of timer orphaning frags
> for skbs that have been around for too long.

Perhaps this can be approximated without an explicit timer by calling
skb_copy_ubufs on enqueue whenever qlen exceeds a threshold value?

>
>> ---
>>  include/linux/skbuff.h        |   1 +
>>  include/linux/socket.h        |   1 +
>>  include/uapi/linux/errqueue.h |   1 +
>>  net/packet/af_packet.c        | 110 ++++++++++++++++++++++++++++++++++++++----
>>  4 files changed, 103 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index 78c299f..8e661d2 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -295,6 +295,7 @@ struct ubuf_info {
>>       void (*callback)(struct ubuf_info *, bool zerocopy_success);
>>       void *ctx;
>>       unsigned long desc;
>> +     void *callback_priv;
>>  };
>>
>>  /* This data is invariant across clones and lives at
>> diff --git a/include/linux/socket.h b/include/linux/socket.h
>> index de52228..0a2e0ea 100644
>> --- a/include/linux/socket.h
>> +++ b/include/linux/socket.h
>> @@ -265,6 +265,7 @@ struct ucred {
>>  #define MSG_SENDPAGE_NOTLAST 0x20000 /* sendpage() internal : not the last page */
>>  #define MSG_EOF         MSG_FIN
>>
>> +#define MSG_ZEROCOPY 0x4000000       /* Pin user pages */
>>  #define MSG_FASTOPEN 0x20000000      /* Send data in TCP SYN */
>>  #define MSG_CMSG_CLOEXEC 0x40000000  /* Set close_on_exec for file
>>                                          descriptor received through
>> diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
>> index 07bdce1..61bf318 100644
>> --- a/include/uapi/linux/errqueue.h
>> +++ b/include/uapi/linux/errqueue.h
>> @@ -19,6 +19,7 @@ struct sock_extended_err {
>>  #define SO_EE_ORIGIN_ICMP6   3
>>  #define SO_EE_ORIGIN_TXSTATUS        4
>>  #define SO_EE_ORIGIN_TIMESTAMPING SO_EE_ORIGIN_TXSTATUS
>> +#define SO_EE_ORIGIN_UPAGE   5
>>
>>  #define SO_EE_OFFENDER(ee)   ((struct sockaddr*)((ee)+1))
>>
>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>> index 58af5802..367c23a 100644
>> --- a/net/packet/af_packet.c
>> +++ b/net/packet/af_packet.c
>> @@ -2370,28 +2370,112 @@ out:
>>
>>  static struct sk_buff *packet_alloc_skb(struct sock *sk, size_t prepad,
>>                                       size_t reserve, size_t len,
>> -                                     size_t linear, int noblock,
>> +                                     size_t linear, int flags,
>>                                       int *err)
>>  {
>>       struct sk_buff *skb;
>> +     size_t data_len;
>>
>> -     /* Under a page?  Don't bother with paged skb. */
>> -     if (prepad + len < PAGE_SIZE || !linear)
>> -             linear = len;
>> +     if (flags & MSG_ZEROCOPY) {
>> +             /* Minimize linear, but respect header lower bound */
>> +             linear = min(len, max_t(size_t, linear, MAX_HEADER));
>> +             data_len = 0;
>> +     } else {
>> +             /* Under a page? Don't bother with paged skb. */
>> +             if (prepad + len < PAGE_SIZE || !linear)
>> +                     linear = len;
>> +             data_len = len - linear;
>> +     }
>>
>> -     skb = sock_alloc_send_pskb(sk, prepad + linear, len - linear, noblock,
>> -                                err, 0);
>> +     skb = sock_alloc_send_pskb(sk, prepad + linear, data_len,
>> +                                flags & MSG_DONTWAIT, err, 0);
>>       if (!skb)
>>               return NULL;
>>
>>       skb_reserve(skb, reserve);
>>       skb_put(skb, linear);
>> -     skb->data_len = len - linear;
>> -     skb->len += len - linear;
>> +     skb->data_len = data_len;
>> +     skb->len += data_len;
>>
>>       return skb;
>>  }
>>
>> +/* release zerocopy resources and avoid destructor callback on kfree */
>> +static void packet_snd_zerocopy_free(struct sk_buff *skb)
>> +{
>> +     struct ubuf_info *uarg = skb_shinfo(skb)->destructor_arg;
>> +
>> +     if (uarg) {
>> +             kfree_skb(uarg->callback_priv);
>> +             sock_put((struct sock *) uarg->ctx);
>> +             kfree(uarg);
>> +
>> +             skb_shinfo(skb)->destructor_arg = NULL;
>> +             skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
>> +     }
>> +}
>> +
>> +static void packet_snd_zerocopy_callback(struct ubuf_info *uarg,
>> +                                      bool zerocopy_success)
>> +{
>> +     struct sk_buff *err_skb;
>> +     struct sock *err_sk;
>> +     struct sock_exterr_skb *serr;
>> +
>> +     err_sk = uarg->ctx;
>> +     err_skb = uarg->callback_priv;
>> +
>> +     serr = SKB_EXT_ERR(err_skb);
>> +     memset(serr, 0, sizeof(*serr));
>> +     serr->ee.ee_errno = ENOMSG;
>> +     serr->ee.ee_origin = SO_EE_ORIGIN_UPAGE;
>> +     serr->ee.ee_data = uarg->desc & 0xFFFFFFFF;
>> +     serr->ee.ee_info = ((u64) uarg->desc) >> 32;
>> +     if (sock_queue_err_skb(err_sk, err_skb))
>> +             kfree_skb(err_skb);
>> +
>> +     kfree(uarg);
>> +     sock_put(err_sk);
>> +}
>> +
>> +static int packet_zerocopy_sg_from_iovec(struct sk_buff *skb,
>> +                                      struct msghdr *msg)
>> +{
>> +     struct ubuf_info *uarg = NULL;
>> +     int ret;
>> +
>> +     if (iov_pages(msg->msg_iov, 0, msg->msg_iovlen) > MAX_SKB_FRAGS)
>> +             return -EMSGSIZE;
>> +
>> +     uarg = kzalloc(sizeof(*uarg), GFP_KERNEL);
>> +     if (!uarg)
>> +             return -ENOMEM;
>> +
>> +     uarg->callback_priv = alloc_skb(0, GFP_KERNEL);
>> +     if (!uarg->callback_priv) {
>> +             kfree(uarg);
>> +             return -ENOMEM;
>> +     }
>> +
>> +     ret = zerocopy_sg_from_iovec(skb, msg->msg_iov, 0, msg->msg_iovlen);
>> +     if (ret) {
>> +             kfree_skb(uarg->callback_priv);
>> +             kfree(uarg);
>> +             return -EIO;
>> +     }
>> +
>> +     sock_hold(skb->sk);
>> +     uarg->ctx = skb->sk;
>> +     uarg->callback = packet_snd_zerocopy_callback;
>> +     uarg->desc = (unsigned long) msg->msg_iov[0].iov_base;
>> +
>> +     skb_shinfo(skb)->destructor_arg = uarg;
>> +     skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
>> +     skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
>> +
>> +     return 0;
>> +}
>> +
>>  static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
>>  {
>>       struct sock *sk = sock->sk;
>> @@ -2408,6 +2492,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
>>       unsigned short gso_type = 0;
>>       int hlen, tlen;
>>       int extra_len = 0;
>> +     bool zerocopy = msg->msg_flags & MSG_ZEROCOPY;
>>
>>       /*
>>        *      Get and verify the address.
>> @@ -2501,7 +2586,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
>>       hlen = LL_RESERVED_SPACE(dev);
>>       tlen = dev->needed_tailroom;
>>       skb = packet_alloc_skb(sk, hlen + tlen, hlen, len, vnet_hdr.hdr_len,
>> -                            msg->msg_flags & MSG_DONTWAIT, &err);
>> +                            msg->msg_flags, &err);
>>       if (skb == NULL)
>>               goto out_unlock;
>>
>> @@ -2518,7 +2603,11 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
>>       }
>>
>>       /* Returns -EFAULT on error */
>> -     err = skb_copy_datagram_from_iovec(skb, offset, msg->msg_iov, 0, len);
>> +     if (zerocopy)
>> +             err = packet_zerocopy_sg_from_iovec(skb, msg);
>> +     else
>> +             err = skb_copy_datagram_from_iovec(skb, offset, msg->msg_iov,
>> +                                                0, len);
>>       if (err)
>>               goto out_free;
>>
>> @@ -2578,6 +2667,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
>>       return len;
>>
>>  out_free:
>> +     packet_snd_zerocopy_free(skb);
>>       kfree_skb(skb);
>>  out_unlock:
>>       if (dev)
>> --
>> 2.1.0.rc2.206.gedb03e5
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ