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

[Joe Stringer <joe@xxxxxxxxxxx>]

On Mon, Mar 16, 2020 at 4:03 PM Martin KaFai Lau <kafai@xxxxxx> 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@xxxxxxxxxxxxx>
> > Signed-off-by: Joe Stringer <joe@xxxxxxxxxxx>
> > ---
> >  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;
        }