netdev - Re: [Patch bpf-next 08/19] udp: implement ->read

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: <CACAyw9-is-sTBUyJnNsQBKga9eyKA8m6+qfS-hWBNipwqgaafg@mail.gmail.com>
Date:   Mon, 8 Feb 2021 09:48:30 +0000
From:   Lorenz Bauer <lmb@...udflare.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        duanxiongchun@...edance.com, wangdongdong.6@...edance.com,
        jiang.wang@...edance.com, Cong Wang <cong.wang@...edance.com>,
        John Fastabend <john.fastabend@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jakub Sitnicki <jakub@...udflare.com>
Subject: Re: [Patch bpf-next 08/19] udp: implement ->read_sock() for sockmap

On Wed, 3 Feb 2021 at 04:17, Cong Wang <xiyou.wangcong@...il.com> wrote:
>
> From: Cong Wang <cong.wang@...edance.com>
>
> Cc: John Fastabend <john.fastabend@...il.com>
> Cc: Daniel Borkmann <daniel@...earbox.net>
> Cc: Jakub Sitnicki <jakub@...udflare.com>
> Cc: Lorenz Bauer <lmb@...udflare.com>
> Signed-off-by: Cong Wang <cong.wang@...edance.com>
> ---
>  include/net/udp.h  |  2 ++
>  net/ipv4/af_inet.c |  1 +
>  net/ipv4/udp.c     | 34 ++++++++++++++++++++++++++++++++++
>  3 files changed, 37 insertions(+)
>
> diff --git a/include/net/udp.h b/include/net/udp.h
> index 13f9354dbd3e..b6b75cabf4e4 100644
> --- a/include/net/udp.h
> +++ b/include/net/udp.h
> @@ -327,6 +327,8 @@ struct sock *__udp6_lib_lookup(struct net *net,
>                                struct sk_buff *skb);
>  struct sock *udp6_lib_lookup_skb(const struct sk_buff *skb,
>                                  __be16 sport, __be16 dport);
> +int udp_read_sock(struct sock *sk, read_descriptor_t *desc,
> +                 sk_read_actor_t recv_actor);
>
>  /* UDP uses skb->dev_scratch to cache as much information as possible and avoid
>   * possibly multiple cache miss on dequeue()
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index d184d9379a92..4a4c6d3d2786 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1072,6 +1072,7 @@ const struct proto_ops inet_dgram_ops = {
>         .getsockopt        = sock_common_getsockopt,
>         .sendmsg           = inet_sendmsg,
>         .sendmsg_locked    = udp_sendmsg_locked,
> +       .read_sock         = udp_read_sock,
>         .recvmsg           = inet_recvmsg,
>         .mmap              = sock_no_mmap,
>         .sendpage          = inet_sendpage,
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 635e1e8b2968..6dffbcec0b51 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1792,6 +1792,40 @@ struct sk_buff *__skb_recv_udp(struct sock *sk, unsigned int flags,
>  }
>  EXPORT_SYMBOL(__skb_recv_udp);
>
> +int udp_read_sock(struct sock *sk, read_descriptor_t *desc,
> +                 sk_read_actor_t recv_actor)
> +{
> +       struct sk_buff *skb;
> +       int copied = 0, err;
> +
> +       while (1) {
> +               int offset = 0;
> +
> +               skb = __skb_recv_udp(sk, 0, 1, &offset, &err);

Seems like err isn't used outside of the loop, is that on purpose? If
yes, how about moving the declaration of err to be with offset. Maybe
rename to ignored?

> +               if (!skb)
> +                       break;
> +               if (offset < skb->len) {
> +                       int used;
> +                       size_t len;
> +
> +                       len = skb->len - offset;
> +                       used = recv_actor(desc, skb, offset, len);
> +                       if (used <= 0) {
> +                               if (!copied)
> +                                       copied = used;
> +                               break;
> +                       } else if (used <= len) {

In which case can used be > len?


> +                               copied += used;
> +                               offset += used;
> +                       }
> +               }
> +               if (!desc->count)
> +                       break;
> +       }
> +
> +       return copied;
> +}
> +
>  /*
>   *     This should be easy, if there is something there we
>   *     return it, otherwise we block.
> --
> 2.25.1
>


--
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com