Re: [PATCH net-next 0/4] net: tcp: make txhash use consistent for IPv4

[Antoine Tenart <atenart@kernel.org>]

Quoting Eric Dumazet (2023-05-11 12:24:15)
> On Thu, May 11, 2023 at 11:35 AM Antoine Tenart <atenart@kernel.org> wrote:
> >
> > Series is divided in two parts. First two commits make the txhash (used
> > for the skb hash in TCP) to be consistent for all IPv4/TCP packets (IPv6
> > doesn't have the same issue). Last two commits improve doc/comment
> > hash-related parts.
> >
> > One example is when using OvS with dp_hash, which uses skb->hash, to
> > select a path. We'd like packets from the same flow to be consistent, as
> > well as the hash being stable over time when using net.core.txrehash=0.
> > Same applies for kernel ECMP which also can use skb->hash.
> >
> 
> SGTM, thanks.
> 
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> 
> FYI while reviewing your patches, I found that I have to send this fix:
> 
> I suggest we hold your patch series a bit before this reaches net-next tree,
> to avoid merge conflicts.

Sure, no problem. Thanks for the review!

> Bug was added in commit f6c0f5d209fa ("tcp: honor SO_PRIORITY in
> TIME_WAIT state")
> 
> 
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 39bda2b1066e1d607a59fb79c6305d0ca30cb28d..06d2573685ca993a3a0a89807f09d7b5c153cc72
> 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -829,6 +829,9 @@ static void tcp_v4_send_reset(const struct sock
> *sk, struct sk_buff *skb)
>                                    inet_twsk(sk)->tw_priority : sk->sk_priority;
>                 transmit_time = tcp_transmit_time(sk);
>                 xfrm_sk_clone_policy(ctl_sk, sk);
> +       } else {
> +               ctl_sk->sk_mark = 0;
> +               ctl_sk->sk_priority = 0;
>         }
>         ip_send_unicast_reply(ctl_sk,
>                               skb, &TCP_SKB_CB(skb)->header.h4.opt,
> @@ -836,7 +839,6 @@ static void tcp_v4_send_reset(const struct sock
> *sk, struct sk_buff *skb)
>                               &arg, arg.iov[0].iov_len,
>                               transmit_time);
> 
> -       ctl_sk->sk_mark = 0;
>         xfrm_sk_free_policy(ctl_sk);
>         sock_net_set(ctl_sk, &init_net);
>         __TCP_INC_STATS(net, TCP_MIB_OUTSEGS);
> @@ -935,7 +937,6 @@ static void tcp_v4_send_ack(const struct sock *sk,
>                               &arg, arg.iov[0].iov_len,
>                               transmit_time);
> 
> -       ctl_sk->sk_mark = 0;
>         sock_net_set(ctl_sk, &init_net);
>         __TCP_INC_STATS(net, TCP_MIB_OUTSEGS);
>         local_bh_enable();
>