netdev - Re: [PATCH net] net/ipv6: SKB symmetric hash should incorporate transport ports

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iKpj81ZJSn3ksS_P7EBwRjbbk=TnvOW8YF=ogHo5TXcwQ@mail.gmail.com>
Date: Tue, 5 Sep 2023 06:28:24 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Quan Tian <qtian@...are.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Lars Ekman <uablrek@...il.com>
Subject: Re: [PATCH net] net/ipv6: SKB symmetric hash should incorporate
 transport ports

On Mon, Sep 4, 2023 at 5:45 PM Quan Tian <qtian@...are.com> wrote:
>
> __skb_get_hash_symmetric() was added to compute a symmetric hash over
> the protocol, addresses and transport ports, by commit eb70db875671
> ("packet: Use symmetric hash for PACKET_FANOUT_HASH."). It uses
> flow_keys_dissector_symmetric_keys as the flow_dissector to incorporate
> IPv4 addresses, IPv6 addresses and ports. However, it should not specify
> the flag as FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL, which stops further
> dissection when an IPv6 flow label is encountered, making transport
> ports not being incorporated in such case.
>
> As a consequence, the symmetric hash is based on 5-tuple for IPv4 but
> 3-tuple for IPv6 when flow label is present. It caused a few problems,
> e.g. when nft symhash and openvswitch l4_sym rely on the symmetric hash
> to perform load balancing as different L4 flows between two given IPv6
> addresses would always get the same symmetric hash, leading to uneven
> traffic distribution.
>
> Removing the use of FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL makes sure the
> symmetric hash is based on 5-tuple for both IPv4 and IPv6 consistently.
>
> Reported-by: Lars Ekman <uablrek@...il.com>
> Closes: https://github.com/antrea-io/antrea/issues/5457
> Signed-off-by: Quan Tian <qtian@...are.com>
> ---
>  net/core/flow_dissector.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 89d15ceaf9af..b3b3af0e7844 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -1831,8 +1831,7 @@ u32 __skb_get_hash_symmetric(const struct sk_buff *skb)
>
>         memset(&keys, 0, sizeof(keys));
>         __skb_flow_dissect(NULL, skb, &flow_keys_dissector_symmetric,
> -                          &keys, NULL, 0, 0, 0,
> -                          FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL);
> +                          &keys, NULL, 0, 0, 0, 0);
>
>         return __flow_hash_from_keys(&keys, &hashrnd);
>  }
> --
> 2.42.0
>

Networking patches for net branches must include a Fixes: tag, to help
automation.
(providing the sha1 in the changelog is requesting human investigation
for stable teams,
because a sha1 could be mentioned even if it is not the bug origin)

Fixes: eb70db875671 ("packet: Use symmetric hash for PACKET_FANOUT_HASH.")

Reviewed-by: Eric Dumazet <edumazet@...gle.com>

Thanks.