Re: [PATCHv2 bpf-next 2/5] bpf: Prefetch established socket destinations

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Tue, Mar 24, 2020 at 10:57:42PM -0700, Joe Stringer wrote:
> Enhance the sk_assign 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>
> ---
> v2: Provide cookie to dst_check() for IPv6 case
> v1: Initial version
> ---
>  net/core/filter.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/net/core/filter.c b/net/core/filter.c
> index f7f9b6631f75..0fada7fe9b75 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5876,6 +5876,21 @@ BPF_CALL_3(bpf_sk_assign, struct sk_buff *, skb, struct sock *, sk, u64, flags)
>  	skb_orphan(skb);
>  	skb->sk = sk;
>  	skb->destructor = sock_pfree;
> +	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, cookie);
> +		if (dst) {
> +			skb_dst_drop(skb);
> +			skb_dst_set_noref(skb, dst);
> +		}

I think the rest of the feedback for the patches can be addressed quickly and
overall the set is imo ready to land within this cycle. My only concern is
above dst_set().
Since it's an optimization may be drop this patch? we can land
the rest and this one can be introduced in the next cycle?
I'm happy to be convinced otherwise, but would like a better explanation
why it's safe to do so in this context.