Re: [PATCH nf] netfilter: nft_flow_offload: do not offload multicast

[Fernando Fernandez Mancera]

On 10/28/26 9:59 PM, Koral Ilgun wrote:
> From: Koral Ilgun <koral.ilgun@xxxxxxxxxx>
>
> A flow holds one output per direction, which it resolves when it is
> added. The outputs of a multicast route change when listeners join
> or leave. So a multicast flow in the flowtable continues to send to
> the output that it resolved, also after the last listener left, and
> the counters of the multicast routing stop.
>
> IPv4 multicast stays out of the flowtable only because the reverse
> route lookup in nft_flow_route() refuses a multicast source address.
> The IPv6 route lookup has no such check, so IPv6 multicast that
> ip6mr forwards goes into the flowtable.
>
> Skip a multicast destination for both families, so that the result
> no longer depends on the route lookup.
>
> Fixes: a3c90f7a2323 ("netfilter: nf_tables: flow offload expression")
> Signed-off-by: Koral Ilgun <koral.ilgun@xxxxxxxxxx>
> ---
> Tested on an arm64 router with Linux 6.18.54, with an inet flowtable
> that offloads forwarded flows, and IPv4 and IPv6 multicast routed from
> the WAN to one VLAN. Without this change, the IPv6 flow went into the
> flowtable after its first packets: the ip6mr vif counters showed 25 of
> 1,500 packets, and the receiver still got all 400 packets after its
> leave. With it, the vif counters showed 1,500 of 1,500, the flow stayed
> out of the flowtable, and the stream stopped about 2.5 s after the
> leave. IPv4 multicast stayed out of the flowtable before and after.
> Unicast IPv4 and IPv6 flows still went into the flowtable.
>
>   net/netfilter/nft_flow_offload.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
> index d3c5651dd699..1fc755e594b0 100644
> --- a/net/netfilter/nft_flow_offload.c
> +++ b/net/netfilter/nft_flow_offload.c
> @@ -10,6 +10,7 @@
>   #include <linux/netfilter/nf_conntrack_common.h>
>   #include <linux/netfilter/nf_tables.h>
>   #include <net/ip.h>
> +#include <net/ipv6.h>
>   #include <net/flow.h>
>   #include <net/netfilter/nf_tables.h>
>   #include <net/netfilter/nf_tables_core.h>
> @@ -35,6 +36,16 @@ static bool nft_flow_offload_skip(struct sk_buff *skb, int family)
>   			return true;
>   	}
>   
> +	/* A flow keeps the output it resolved at offload time, but the
> +	 * outputs of a multicast route change when listeners join or leave.
> +	 */
> +	switch (family) {
> +	case NFPROTO_IPV4:
> +		return ipv4_is_multicast(ip_hdr(skb)->daddr);
> +	case NFPROTO_IPV6:
> +		return ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr);
> +	}
> +
>   	return false;
>   }

The patch is sound and I think it is better skip the flow earlier here. 
I believe there is a bigger problem with IPv6 route engine not rejecting 
a routing request for a saddr that is multicast, I am already looking 
into that. In any case, couldn't we simplify this code as:

diff --git a/net/netfilter/nft_flow_offload.c 
b/net/netfilter/nft_flow_offload.c
index d3c5651dd699..7d0b87a1bca1 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -33,6 +33,10 @@ static bool nft_flow_offload_skip(struct sk_buff 
*skb, int family)

                if (unlikely(opt->optlen))
                        return true;
+
+               return ipv4_is_multicast(ip_hdr(skb)->daddr);
+       } else if (family == NFPROTO_IPV6) {
+               return ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr);
        }

        return false;


Thanks,
Fernando.

>   
>
> base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b