| Message ID | 20200312233648.1767-5-joe@wand.net.nz |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Add bpf_sk_assign eBPF helper | expand |
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@iogearbox.net> > Signed-off-by: Joe Stringer <joe@wand.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. > + if (dst) > + refdst = (unsigned long)dst | SKB_DST_NOREF; > + } > + if (!refdst) > + refdst = skb->_skb_refdst; > + if (skb->_skb_refdst != refdst) > + skb_dst_drop(skb); > > - refdst = skb->_skb_refdst; > __this_cpu_write(dst_sk_prefetch_dst, refdst); > skb_dst_set_noref(skb, (struct dst_entry *)&dst_sk_prefetch.dst); > }
On Mon, Mar 16, 2020 at 4:03 PM Martin KaFai Lau <kafai@fb.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@iogearbox.net> > > Signed-off-by: Joe Stringer <joe@wand.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; }
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); + if (dst) + refdst = (unsigned long)dst | SKB_DST_NOREF; + } + if (!refdst) + refdst = skb->_skb_refdst; + if (skb->_skb_refdst != refdst) + skb_dst_drop(skb); - refdst = skb->_skb_refdst; __this_cpu_write(dst_sk_prefetch_dst, refdst); skb_dst_set_noref(skb, (struct dst_entry *)&dst_sk_prefetch.dst); } diff --git a/net/core/filter.c b/net/core/filter.c index bae0874289d8..db9b7b8b4a04 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -5858,7 +5858,7 @@ BPF_CALL_3(bpf_sk_assign, struct sk_buff *, skb, struct sock *, sk, u64, flags) skb_orphan(skb); skb->sk = sk; skb->destructor = sock_edemux; - dst_sk_prefetch_store(skb); + dst_sk_prefetch_store(skb, sk); return 0; }
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 <joe@wand.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(-)