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

On Mon, Mar 16, 2020 at 4:03 PM Martin KaFai Lau [off-list ref] wrote:

On Thu, Mar 12, 2020 at 04:36:45PM -0700, Joe Stringer wrote:

quoted

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@iogearbox.net>
Signed-off-by: Joe Stringer <redacted>
---
 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;
        }