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

[Eric Dumazet]

On Mon, Sep 4, 2023 at 5:45 PM Quan Tian <qtian@xxxxxxxxxx> 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@xxxxxxxxx>
> Closes: https://github.com/antrea-io/antrea/issues/5457
> Signed-off-by: Quan Tian <qtian@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>

Thanks.