netdev - Re: [PATCH bpf-next 4/7] dst: Prefetch established socket destinations

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: <CAOftzPjMO2Y=efeqYZCzYd2JX8JZULfvCNw2mc-io1anxNYgGA@mail.gmail.com>
Date:   Mon, 16 Mar 2020 20:17:59 -0700
From:   Joe Stringer <joe@...d.net.nz>
To:     Martin KaFai Lau <kafai@...com>
Cc:     Joe Stringer <joe@...d.net.nz>, bpf@...r.kernel.org,
        netdev <netdev@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Lorenz Bauer <lmb@...udflare.com>
Subject: Re: [PATCH bpf-next 4/7] dst: Prefetch established socket destinations

On Mon, Mar 16, 2020 at 4:03 PM Martin KaFai Lau <kafai@...com> wrote:
>
> On Thu, Mar 12, 2020 at 04:36:45PM -0700, Joe Stringer wrote:
> > Enhance the dst_sk_prefetch logic to temporarily store the socket
> > receive destination, to save the route lookup later on. The dst
> > reference is kept alive by the caller's socket reference.
> >
> > Suggested-by: Daniel Borkmann <daniel@...earbox.net>
> > Signed-off-by: Joe Stringer <joe@...d.net.nz>
> > ---
> >  include/net/dst_metadata.h |  2 +-
> >  net/core/dst.c             | 20 +++++++++++++++++---
> >  net/core/filter.c          |  2 +-
> >  3 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h
> > index 31574c553a07..4f16322b08d5 100644
> > --- a/include/net/dst_metadata.h
> > +++ b/include/net/dst_metadata.h
> > @@ -230,7 +230,7 @@ static inline bool skb_dst_is_sk_prefetch(const struct sk_buff *skb)
> >       return dst_is_sk_prefetch(skb_dst(skb));
> >  }
> >
> > -void dst_sk_prefetch_store(struct sk_buff *skb);
> > +void dst_sk_prefetch_store(struct sk_buff *skb, struct sock *sk);
> >  void dst_sk_prefetch_fetch(struct sk_buff *skb);
> >
> >  /**
> > diff --git a/net/core/dst.c b/net/core/dst.c
> > index cf1a1d5b6b0a..5068d127d9c2 100644
> > --- a/net/core/dst.c
> > +++ b/net/core/dst.c
> > @@ -346,11 +346,25 @@ EXPORT_SYMBOL(dst_sk_prefetch);
> >
> >  DEFINE_PER_CPU(unsigned long, dst_sk_prefetch_dst);
> >
> > -void dst_sk_prefetch_store(struct sk_buff *skb)
> > +void dst_sk_prefetch_store(struct sk_buff *skb, struct sock *sk)
> >  {
> > -     unsigned long refdst;
> > +     unsigned long refdst = 0L;
> > +
> > +     WARN_ON(!rcu_read_lock_held() &&
> > +             !rcu_read_lock_bh_held());
> > +     if (sk_fullsock(sk)) {
> > +             struct dst_entry *dst = READ_ONCE(sk->sk_rx_dst);
> > +
> > +             if (dst)
> > +                     dst = dst_check(dst, 0);
> v6 requires a cookie.  tcp_v6_early_demux() could be a good example.

Nice catch. I plan to roll in the following incremental for v2:

diff --git a/net/core/dst.c b/net/core/dst.c
index 5068d127d9c2..b60f85227247 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -354,9 +354,14 @@ void dst_sk_prefetch_store(struct sk_buff *skb,
struct sock *sk)
                !rcu_read_lock_bh_held());
        if (sk_fullsock(sk)) {
                struct dst_entry *dst = READ_ONCE(sk->sk_rx_dst);
+               u32 cookie = 0;

+#if IS_ENABLED(CONFIG_IPV6)
+               if (sk->sk_family == AF_INET6)
+                       cookie = inet6_sk(sk)->rx_dst_cookie;
+#endif
                if (dst)
-                       dst = dst_check(dst, 0);
+                       dst = dst_check(dst, cookie);
                if (dst)
                        refdst = (unsigned long)dst | SKB_DST_NOREF;
        }