[PATCH nf,v2] netfilter: flowtable: tear down flow entries with stale dst from GC

[PATCH nf,v2] netfilter: flowtable: tear down flow entries with stale dst from GC

[Pablo Neira Ayuso]

In case of route updates, tear down flow entries with stale dst to give
them a chance to obtain a fresh route.

This is specifically useful for hardware offloaded entries, where the
flowtable software dataplane sees no packet, where the existing check
for stale dst entries does not help.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: - reuse nf_flow_dst_check(), move it to .h file
    - use correct logic in nf_flow_dst_check() from GC step

This patch has been repurposed to the nf.git tree, because net-next.git is
still missing a recent fix and I would like sashiko kicks it for review.
So I am still leaning towards including this in nf-next.

 include/net/netfilter/nf_flow_table.h | 8 ++++++++
 net/netfilter/nf_flow_table_core.c    | 2 ++
 net/netfilter/nf_flow_table_ip.c      | 8 --------
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index ce414118962f..a090ec3ffef2 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -310,6 +310,14 @@ int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow)
 void flow_offload_refresh(struct nf_flowtable *flow_table,
 			  struct flow_offload *flow, bool force);
 
+static inline bool nf_flow_dst_check(struct flow_offload_tuple *tuple)
+{
+	if (!tuple->dst_cache)
+		return true;
+
+	return dst_check(tuple->dst_cache, tuple->dst_cookie);
+}
+
 struct flow_offload_tuple_rhash *flow_offload_lookup(struct nf_flowtable *flow_table,
 						     struct flow_offload_tuple *tuple);
 void nf_flow_table_gc_run(struct nf_flowtable *flow_table);
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 2a829b5e8240..fe655e5e3b06 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -571,6 +571,8 @@ static void nf_flow_offload_gc_step(struct nf_flowtable *flow_table,
 
 	if (nf_flow_has_expired(flow) ||
 	    nf_ct_is_dying(flow->ct) ||
+	    !nf_flow_dst_check(&flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple) ||
+	    !nf_flow_dst_check(&flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple) ||
 	    nf_flow_custom_gc(flow_table, flow)) {
 		flow_offload_teardown(flow);
 		teardown = true;
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 44f4d74f2982..55f6a0dedb03 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -297,14 +297,6 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
 	return true;
 }
 
-static inline bool nf_flow_dst_check(struct flow_offload_tuple *tuple)
-{
-	if (!tuple->dst_cache)
-		return true;
-
-	return dst_check(tuple->dst_cache, tuple->dst_cookie);
-}
-
 static unsigned int nf_flow_xmit_xfrm(struct sk_buff *skb,
 				      const struct nf_hook_state *state,
 				      struct dst_entry *dst)
-- 
2.47.3

Re: [PATCH nf,v2] netfilter: flowtable: tear down flow entries with stale dst from GC

[Ahmed Zaki]

On Fri, Jul 10, 2026 at 1:54 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> In case of route updates, tear down flow entries with stale dst to give
> them a chance to obtain a fresh route.
>
> This is specifically useful for hardware offloaded entries, where the
> flowtable software dataplane sees no packet, where the existing check
> for stale dst entries does not help.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> v2: - reuse nf_flow_dst_check(), move it to .h file
>     - use correct logic in nf_flow_dst_check() from GC step
>
> This patch has been repurposed to the nf.git tree, because net-next.git is
> still missing a recent fix and I would like sashiko kicks it for review.
> So I am still leaning towards including this in nf-next.
>
>  include/net/netfilter/nf_flow_table.h | 8 ++++++++
>  net/netfilter/nf_flow_table_core.c    | 2 ++
>  net/netfilter/nf_flow_table_ip.c      | 8 --------
>  3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
> index ce414118962f..a090ec3ffef2 100644
> --- a/include/net/netfilter/nf_flow_table.h
> +++ b/include/net/netfilter/nf_flow_table.h
> @@ -310,6 +310,14 @@ int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow)
>  void flow_offload_refresh(struct nf_flowtable *flow_table,
>                           struct flow_offload *flow, bool force);
>
> +static inline bool nf_flow_dst_check(struct flow_offload_tuple *tuple)
> +{
> +       if (!tuple->dst_cache)
> +               return true;
> +
> +       return dst_check(tuple->dst_cache, tuple->dst_cookie);
> +}

I am testing this patch and keep getting some splats. I am testing
with a MTK7621 hw which
to my understanding, marks the tuple's xmit_type DIRECT (not neigh or XFRM).

In nft_dev_forward_path(), out.h_source is set and this overrides the
dst_cache (same union)
this seems to be causing the splat when the dst_cache is dereferenced.
(btw, not like his patch,
in the latest HEAD, nf_flow_dst_check() guards tuple->dst_****  by
checks on xmit_type)

So, to support DIRECT types, we can:
1 - go back to my v1 patch (no dependency on dst_cache)
2 - same patch but use dst_check() only for NEIGH/XFRM
3 - or maybe we can have the dst_cache out of the union and available
for all xmit_types.

I have some time to work on this, let me know if I can help.

Thanks.

Re: [PATCH nf,v2] netfilter: flowtable: tear down flow entries with stale dst from GC

[Pablo Neira Ayuso]

Hi,

On Fri, Jul 10, 2026 at 01:55:59PM -0600, Ahmed Zaki wrote:
> On Fri, Jul 10, 2026 at 1:54 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > In case of route updates, tear down flow entries with stale dst to give
> > them a chance to obtain a fresh route.
> >
> > This is specifically useful for hardware offloaded entries, where the
> > flowtable software dataplane sees no packet, where the existing check
> > for stale dst entries does not help.
> >
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> > v2: - reuse nf_flow_dst_check(), move it to .h file
> >     - use correct logic in nf_flow_dst_check() from GC step
> >
> > This patch has been repurposed to the nf.git tree, because net-next.git is
> > still missing a recent fix and I would like sashiko kicks it for review.
> > So I am still leaning towards including this in nf-next.
> >
> >  include/net/netfilter/nf_flow_table.h | 8 ++++++++
> >  net/netfilter/nf_flow_table_core.c    | 2 ++
> >  net/netfilter/nf_flow_table_ip.c      | 8 --------
> >  3 files changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
> > index ce414118962f..a090ec3ffef2 100644
> > --- a/include/net/netfilter/nf_flow_table.h
> > +++ b/include/net/netfilter/nf_flow_table.h
> > @@ -310,6 +310,14 @@ int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow)
> >  void flow_offload_refresh(struct nf_flowtable *flow_table,
> >                           struct flow_offload *flow, bool force);
> >
> > +static inline bool nf_flow_dst_check(struct flow_offload_tuple *tuple)
> > +{
> > +       if (!tuple->dst_cache)
> > +               return true;
> > +
> > +       return dst_check(tuple->dst_cache, tuple->dst_cookie);
> > +}
> 
> I am testing this patch and keep getting some splats. I am testing
> with a MTK7621 hw which
> to my understanding, marks the tuple's xmit_type DIRECT (not neigh or XFRM).
> 
> In nft_dev_forward_path(), out.h_source is set and this overrides the
> dst_cache (same union)
> this seems to be causing the splat when the dst_cache is dereferenced.
> (btw, not like his patch,
> in the latest HEAD, nf_flow_dst_check() guards tuple->dst_****  by
> checks on xmit_type)

We have move dst_cache out of the union quite recently so... (see below)

> So, to support DIRECT types, we can:
> 1 - go back to my v1 patch (no dependency on dst_cache)
> 2 - same patch but use dst_check() only for NEIGH/XFRM
> 3 - or maybe we can have the dst_cache out of the union and available
> for all xmit_types.
> 
> I have some time to work on this, let me know if I can help.

... probably you are missing this series? They got merged quite
recently, I am not sure what tree you are using as reference:

fa7395c02d95 netfilter: flowtable: support IPIP tunnel with direct xmit
6c5dcab95f4c netfilter: flowtable: IPIP tunnel hardware offload is not yet support
c328b90c17fc netfilter: flowtable: use dst in this direction when pushing IPIP header

There is also this patch which is needed:

https://patchwork.ozlabs.org/project/netfilter-devel/patch/20260709114025.1294044-1-pablo@netfilter.org/

which has been included in the last PR this Friday.

Re: [PATCH nf,v2] netfilter: flowtable: tear down flow entries with stale dst from GC

[Ahmed Zaki]

On Sat, Jul 11, 2026 at 3:02 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
.
.
.

> > I am testing this patch and keep getting some splats. I am testing
> > with a MTK7621 hw which
> > to my understanding, marks the tuple's xmit_type DIRECT (not neigh or XFRM).
> >
> > In nft_dev_forward_path(), out.h_source is set and this overrides the
> > dst_cache (same union)
> > this seems to be causing the splat when the dst_cache is dereferenced.
> > (btw, not like his patch,
> > in the latest HEAD, nf_flow_dst_check() guards tuple->dst_****  by
> > checks on xmit_type)
>
> We have move dst_cache out of the union quite recently so... (see below)
>
> > So, to support DIRECT types, we can:
> > 1 - go back to my v1 patch (no dependency on dst_cache)
> > 2 - same patch but use dst_check() only for NEIGH/XFRM
> > 3 - or maybe we can have the dst_cache out of the union and available
> > for all xmit_types.
> >
> > I have some time to work on this, let me know if I can help.
>
> ... probably you are missing this series? They got merged quite
> recently, I am not sure what tree you are using as reference:

yeah, I was looking at nf-next. These seem to be in nf only as of now.

>
> fa7395c02d95 netfilter: flowtable: support IPIP tunnel with direct xmit
> 6c5dcab95f4c netfilter: flowtable: IPIP tunnel hardware offload is not yet support
> c328b90c17fc netfilter: flowtable: use dst in this direction when pushing IPIP header
>
> There is also this patch which is needed:
>
> https://patchwork.ozlabs.org/project/netfilter-devel/patch/20260709114025.1294044-1-pablo@netfilter.org/
>
> which has been included in the last PR this Friday.

I have some limitations so I can only test with 6.6 or 6.12. I will
try to trim down these patches to
"move dst_cache out of union and use the gc to check all entries" and
will let you know if I see
any more problems.

The one thing I believe is still missing is to remove the if condition
in flow_offload_fill_route()
when setting  "flow_tuple->dst_cache/dst_cookie":

- if (route->tuple[!dir].in.num_tuns) {

We now need "flow_tuple->dst_cache/dst_cookie" for all DIRECT xmit
even with no tunnels
(quick look on nft_dev_path_info() shows "num_tuns" set for
"path->type == DEV_PATH_TUN" only).

Thank you.

Re: [PATCH nf,v2] netfilter: flowtable: tear down flow entries with stale dst from GC

[Pablo Neira Ayuso]

On Sun, Jul 12, 2026 at 07:39:25AM -0600, Ahmed Zaki wrote:
> On Sat, Jul 11, 2026 at 3:02 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> .
> 
> > > I am testing this patch and keep getting some splats. I am testing
> > > with a MTK7621 hw which
> > > to my understanding, marks the tuple's xmit_type DIRECT (not neigh or XFRM).
> > >
> > > In nft_dev_forward_path(), out.h_source is set and this overrides the
> > > dst_cache (same union)
> > > this seems to be causing the splat when the dst_cache is dereferenced.
> > > (btw, not like his patch,
> > > in the latest HEAD, nf_flow_dst_check() guards tuple->dst_****  by
> > > checks on xmit_type)
> >
> > We have move dst_cache out of the union quite recently so... (see below)
> >
> > > So, to support DIRECT types, we can:
> > > 1 - go back to my v1 patch (no dependency on dst_cache)
> > > 2 - same patch but use dst_check() only for NEIGH/XFRM
> > > 3 - or maybe we can have the dst_cache out of the union and available
> > > for all xmit_types.
> > >
> > > I have some time to work on this, let me know if I can help.
> >
> > ... probably you are missing this series? They got merged quite
> > recently, I am not sure what tree you are using as reference:
> 
> yeah, I was looking at nf-next. These seem to be in nf only as of now.

OK.

> > fa7395c02d95 netfilter: flowtable: support IPIP tunnel with direct xmit
> > 6c5dcab95f4c netfilter: flowtable: IPIP tunnel hardware offload is not yet support
> > c328b90c17fc netfilter: flowtable: use dst in this direction when pushing IPIP header
> >
> > There is also this patch which is needed:
> >
> > https://patchwork.ozlabs.org/project/netfilter-devel/patch/20260709114025.1294044-1-pablo@netfilter.org/
> >
> > which has been included in the last PR this Friday.
> 
> I have some limitations so I can only test with 6.6 or 6.12. I will
> try to trim down these patches to
> "move dst_cache out of union and use the gc to check all entries" and
> will let you know if I see
> any more problems.

You will have to backport the move of dst_cache out of the union.

> The one thing I believe is still missing is to remove the if condition
> in flow_offload_fill_route()
> when setting  "flow_tuple->dst_cache/dst_cookie":
> 
> - if (route->tuple[!dir].in.num_tuns) {
> 
> We now need "flow_tuple->dst_cache/dst_cookie" for all DIRECT xmit
> even with no tunnels

How so? ->dst_cache is not available when sending packets to a port
behind a master bridge device.

Re: [PATCH nf,v2] netfilter: flowtable: tear down flow entries with stale dst from GC

[Ahmed Zaki]

On Sun, Jul 12, 2026 at 1:27 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Sun, Jul 12, 2026 at 07:39:25AM -0600, Ahmed Zaki wrote:
> > On Sat, Jul 11, 2026 at 3:02 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > >
.
.
.
>
> > > fa7395c02d95 netfilter: flowtable: support IPIP tunnel with direct xmit
> > > 6c5dcab95f4c netfilter: flowtable: IPIP tunnel hardware offload is not yet support
> > > c328b90c17fc netfilter: flowtable: use dst in this direction when pushing IPIP header
> > >
> > > There is also this patch which is needed:
> > >
> > > https://patchwork.ozlabs.org/project/netfilter-devel/patch/20260709114025.1294044-1-pablo@netfilter.org/
> > >
> > > which has been included in the last PR this Friday.
> >
> > I have some limitations so I can only test with 6.6 or 6.12. I will
> > try to trim down these patches to
> > "move dst_cache out of union and use the gc to check all entries" and
> > will let you know if I see
> > any more problems.
>
> You will have to backport the move of dst_cache out of the union.
>

yes, and I tested for a couple of hours with no issues. GC was taking
care of the
expired dst_cache.

> > The one thing I believe is still missing is to remove the if condition
> > in flow_offload_fill_route()
> > when setting  "flow_tuple->dst_cache/dst_cookie":
> >
> > - if (route->tuple[!dir].in.num_tuns) {
> >
> > We now need "flow_tuple->dst_cache/dst_cookie" for all DIRECT xmit
> > even with no tunnels
>
> How so? ->dst_cache is not available when sending packets to a port
> behind a master bridge device.

Sorry I must be missing something. I do not have a bridge in my setup. I have a
Mediatek PPE that is offloading from one port to another and there are
no bridges:

# nft list ruleset
table ip fw4 {
    flowtable f1 {
        hook ingress priority filter
        devices = { eth0, eth1 }
        flags offload
    }

chain forward {
    type filter hook forward priority filter; policy accept;
    tcp dport 5201 flow add @f1
    }
}

My previous comment is that in this case, AFAIK, the "xmit_type =
DIRECT" and in order
to set dst_cache/cookie, the guard "if
(route->tuple[!dir].in.num_tuns)" needs to be
taken away (which I did in my 6.6 patch, let me know if you want me to
send this as ref).

Thank you.