Re: [PATCH net-next v8 1/2] net/sched: cls_flower: remove unions from fl_flow_key

[Simon Horman <horms@kernel.org>]

On Wed, Mar 11, 2026 at 11:46:18AM +0100, Cedric Jehasse via B4 Relay wrote:
> From: Cedric Jehasse <cedric.jehasse@luminex.be>
> 
> When creating a flower classifier with an ipv4 address the
> flow_dissector has both FLOW_DISSECTOR_KEY_IPV4_ADDRS and
> FLOW_DISSECTOR_KEY_IPV6_ADDRS bits set in used_keys.
> This happens because ipv4/ipv6 fields are a union and
> FL_KEY_SET_IF_MASKED() will interpret either being set as both.
> 
> Removing the unions fixes this behavior without needing special handling
> for union fields.
> 
> Example of a command that caused FLOW_DISSECTOR_KEY_IPV4_ADDRS and
> FLOW_DISSECTOR_KEY_IPV6_ADDRS to be set:
> tc filter add dev p1 ingress protocol ip flower skip_sw \
>     dst_ip 224.0.1.129 action trap
> 
> Signed-off-by: Cedric Jehasse <cedric.jehasse@luminex.be>

Hi Cedric,

This doesn't seem to be the right approach to me.

It seems to me that the use of a union is intentional here, as either IPv4
or IPv6 addresses can be present in each case - never both.  And that
control.addr_type and enc_control.addr_type are intended to allow
differentiation of the address type in use for each of these unions.

> ---
>  net/sched/cls_flower.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index 7669371c1354..b95dbe847dde 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -59,18 +59,14 @@ struct fl_flow_key {
>  	struct flow_dissector_key_eth_addrs eth;
>  	struct flow_dissector_key_vlan vlan;
>  	struct flow_dissector_key_vlan cvlan;
> -	union {
> -		struct flow_dissector_key_ipv4_addrs ipv4;
> -		struct flow_dissector_key_ipv6_addrs ipv6;
> -	};
> +	struct flow_dissector_key_ipv4_addrs ipv4;
> +	struct flow_dissector_key_ipv6_addrs ipv6;
>  	struct flow_dissector_key_ports tp;
>  	struct flow_dissector_key_icmp icmp;
>  	struct flow_dissector_key_arp arp;
>  	struct flow_dissector_key_keyid enc_key_id;
> -	union {
> -		struct flow_dissector_key_ipv4_addrs enc_ipv4;
> -		struct flow_dissector_key_ipv6_addrs enc_ipv6;
> -	};
> +	struct flow_dissector_key_ipv4_addrs enc_ipv4;
> +	struct flow_dissector_key_ipv6_addrs enc_ipv6;
>  	struct flow_dissector_key_ports enc_tp;
>  	struct flow_dissector_key_mpls mpls;
>  	struct flow_dissector_key_tcp tcp;
> 
> -- 
> 2.43.0
> 
>