[PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion

[PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion

[Pablo Neira Ayuso]

Retrieve stats too when deleting a flow from hardware. Apparently, the
existing approach is missing last stats sync with the conntrack entry.
Pass the stats object to the driver so it has a chance to populate it
at deletion time. Drivers need to be updated to use this new approach.

Reported-by: Valentin Berezhnoy <dreyksmail@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_flow_table_offload.c | 43 ++++++++++++++++-----------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index 4365859220e6..53182323f73c 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -934,6 +934,27 @@ static int nf_flow_offload_tuple(struct nf_flowtable *flowtable,
 	return i;
 }
 
+static void flow_offload_update_stats(struct flow_offload_work *offload,
+				      struct flow_stats *stats)
+{
+	u64 lastused;
+
+	lastused = max_t(u64, stats[0].lastused, stats[1].lastused);
+	offload->flow->timeout = max_t(u64, offload->flow->timeout,
+				       lastused + flow_offload_get_timeout(offload->flow));
+
+	if (offload->flowtable->flags & NF_FLOWTABLE_COUNTER) {
+		if (stats[0].pkts)
+			nf_ct_acct_add(offload->flow->ct,
+				       FLOW_OFFLOAD_DIR_ORIGINAL,
+				       stats[0].pkts, stats[0].bytes);
+		if (stats[1].pkts)
+			nf_ct_acct_add(offload->flow->ct,
+				       FLOW_OFFLOAD_DIR_REPLY,
+				       stats[1].pkts, stats[1].bytes);
+	}
+}
+
 static int flow_offload_tuple_add(struct flow_offload_work *offload,
 				  struct nf_flow_rule *flow_rule,
 				  enum flow_offload_tuple_dir dir)
@@ -948,10 +969,13 @@ static int flow_offload_tuple_add(struct flow_offload_work *offload,
 static void flow_offload_tuple_del(struct flow_offload_work *offload,
 				   enum flow_offload_tuple_dir dir)
 {
+	struct flow_stats stats[FLOW_OFFLOAD_DIR_MAX] = {};
+
 	nf_flow_offload_tuple(offload->flowtable, offload->flow, NULL, dir,
 			      offload->flowtable->priority,
-			      FLOW_CLS_DESTROY, NULL,
+			      FLOW_CLS_DESTROY, stats,
 			      &offload->flowtable->flow_block.cb_list);
+	flow_offload_update_stats(offload, stats);
 }
 
 static int flow_offload_rule_add(struct flow_offload_work *offload,
@@ -1011,27 +1035,12 @@ static void flow_offload_tuple_stats(struct flow_offload_work *offload,
 static void flow_offload_work_stats(struct flow_offload_work *offload)
 {
 	struct flow_stats stats[FLOW_OFFLOAD_DIR_MAX] = {};
-	u64 lastused;
 
 	flow_offload_tuple_stats(offload, FLOW_OFFLOAD_DIR_ORIGINAL, &stats[0]);
 	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
 		flow_offload_tuple_stats(offload, FLOW_OFFLOAD_DIR_REPLY,
 					 &stats[1]);
-
-	lastused = max_t(u64, stats[0].lastused, stats[1].lastused);
-	offload->flow->timeout = max_t(u64, offload->flow->timeout,
-				       lastused + flow_offload_get_timeout(offload->flow));
-
-	if (offload->flowtable->flags & NF_FLOWTABLE_COUNTER) {
-		if (stats[0].pkts)
-			nf_ct_acct_add(offload->flow->ct,
-				       FLOW_OFFLOAD_DIR_ORIGINAL,
-				       stats[0].pkts, stats[0].bytes);
-		if (stats[1].pkts)
-			nf_ct_acct_add(offload->flow->ct,
-				       FLOW_OFFLOAD_DIR_REPLY,
-				       stats[1].pkts, stats[1].bytes);
-	}
+	flow_offload_update_stats(offload, stats);
 }
 
 static void flow_offload_work_handler(struct work_struct *work)
-- 
2.47.3

[PATCH nf-next 2/2] net: ethernet: mtk_ppe: update stats on flow deletion

[Pablo Neira Ayuso]

Update this driver to use the new approach to fetch stats on flow
deletion, introduced by ("netfilter: flowtable: retrieve stats on flow
deletion").

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 .../net/ethernet/mediatek/mtk_ppe_offload.c   | 32 ++++++++++++-------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
index 99b28aaa7cc4..8ca694bb5e1b 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
+++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
@@ -526,6 +526,24 @@ mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f,
 	return err;
 }
 
+static void
+mtk_flow_offload_update_stats(struct mtk_eth *eth, struct flow_cls_offload *f,
+			      struct mtk_flow_entry *entry)
+{
+	struct mtk_foe_accounting diff;
+	u32 idle;
+
+	idle = mtk_foe_entry_idle_time(eth->ppe[entry->ppe_index], entry);
+	f->stats.lastused = jiffies - idle * HZ;
+
+	if (entry->hash != 0xFFFF &&
+	    mtk_foe_entry_get_mib(eth->ppe[entry->ppe_index], entry->hash,
+				  &diff)) {
+		f->stats.pkts += diff.packets;
+		f->stats.bytes += diff.bytes;
+	}
+}
+
 static int
 mtk_flow_offload_destroy(struct mtk_eth *eth, struct flow_cls_offload *f)
 {
@@ -536,6 +554,8 @@ mtk_flow_offload_destroy(struct mtk_eth *eth, struct flow_cls_offload *f)
 	if (!entry)
 		return -ENOENT;
 
+	mtk_flow_offload_update_stats(eth, f, entry);
+
 	mtk_foe_entry_clear(eth->ppe[entry->ppe_index], entry);
 	rhashtable_remove_fast(&eth->flow_table, &entry->node,
 			       mtk_flow_ht_params);
@@ -550,23 +570,13 @@ static int
 mtk_flow_offload_stats(struct mtk_eth *eth, struct flow_cls_offload *f)
 {
 	struct mtk_flow_entry *entry;
-	struct mtk_foe_accounting diff;
-	u32 idle;
 
 	entry = rhashtable_lookup(&eth->flow_table, &f->cookie,
 				  mtk_flow_ht_params);
 	if (!entry)
 		return -ENOENT;
 
-	idle = mtk_foe_entry_idle_time(eth->ppe[entry->ppe_index], entry);
-	f->stats.lastused = jiffies - idle * HZ;
-
-	if (entry->hash != 0xFFFF &&
-	    mtk_foe_entry_get_mib(eth->ppe[entry->ppe_index], entry->hash,
-				  &diff)) {
-		f->stats.pkts += diff.packets;
-		f->stats.bytes += diff.bytes;
-	}
+	mtk_flow_offload_update_stats(eth, f, entry);
 
 	return 0;
 }
-- 
2.47.3

Re: [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 3900 bytes --]

> Retrieve stats too when deleting a flow from hardware. Apparently, the
> existing approach is missing last stats sync with the conntrack entry.
> Pass the stats object to the driver so it has a chance to populate it
> at deletion time. Drivers need to be updated to use this new approach.
> 
> Reported-by: Valentin Berezhnoy <dreyksmail@gmail.com>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  net/netfilter/nf_flow_table_offload.c | 43 ++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> index 4365859220e6..53182323f73c 100644
> --- a/net/netfilter/nf_flow_table_offload.c
> +++ b/net/netfilter/nf_flow_table_offload.c
> @@ -934,6 +934,27 @@ static int nf_flow_offload_tuple(struct nf_flowtable *flowtable,
>  	return i;
>  }
>  
> +static void flow_offload_update_stats(struct flow_offload_work *offload,
> +				      struct flow_stats *stats)
> +{
> +	u64 lastused;
> +
> +	lastused = max_t(u64, stats[0].lastused, stats[1].lastused);
> +	offload->flow->timeout = max_t(u64, offload->flow->timeout,
> +				       lastused + flow_offload_get_timeout(offload->flow));
> +
> +	if (offload->flowtable->flags & NF_FLOWTABLE_COUNTER) {
> +		if (stats[0].pkts)
> +			nf_ct_acct_add(offload->flow->ct,
> +				       FLOW_OFFLOAD_DIR_ORIGINAL,
> +				       stats[0].pkts, stats[0].bytes);
> +		if (stats[1].pkts)
> +			nf_ct_acct_add(offload->flow->ct,
> +				       FLOW_OFFLOAD_DIR_REPLY,
> +				       stats[1].pkts, stats[1].bytes);
> +	}
> +}
> +
>  static int flow_offload_tuple_add(struct flow_offload_work *offload,
>  				  struct nf_flow_rule *flow_rule,
>  				  enum flow_offload_tuple_dir dir)
> @@ -948,10 +969,13 @@ static int flow_offload_tuple_add(struct flow_offload_work *offload,
>  static void flow_offload_tuple_del(struct flow_offload_work *offload,
>  				   enum flow_offload_tuple_dir dir)
>  {
> +	struct flow_stats stats[FLOW_OFFLOAD_DIR_MAX] = {};
> +
>  	nf_flow_offload_tuple(offload->flowtable, offload->flow, NULL, dir,
>  			      offload->flowtable->priority,
> -			      FLOW_CLS_DESTROY, NULL,
> +			      FLOW_CLS_DESTROY, stats,
>  			      &offload->flowtable->flow_block.cb_list);

according to my understanding nf_flow_offload_tuple() copies the stats just if
cmd is set to FLOW_CLS_STATS, right? Should we just check if stats is not NULL
in nf_flow_offload_tuple() instead?

Regards,
Lorenzo

> +	flow_offload_update_stats(offload, stats);
>  }
>  
>  static int flow_offload_rule_add(struct flow_offload_work *offload,
> @@ -1011,27 +1035,12 @@ static void flow_offload_tuple_stats(struct flow_offload_work *offload,
>  static void flow_offload_work_stats(struct flow_offload_work *offload)
>  {
>  	struct flow_stats stats[FLOW_OFFLOAD_DIR_MAX] = {};
> -	u64 lastused;
>  
>  	flow_offload_tuple_stats(offload, FLOW_OFFLOAD_DIR_ORIGINAL, &stats[0]);
>  	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
>  		flow_offload_tuple_stats(offload, FLOW_OFFLOAD_DIR_REPLY,
>  					 &stats[1]);
> -
> -	lastused = max_t(u64, stats[0].lastused, stats[1].lastused);
> -	offload->flow->timeout = max_t(u64, offload->flow->timeout,
> -				       lastused + flow_offload_get_timeout(offload->flow));
> -
> -	if (offload->flowtable->flags & NF_FLOWTABLE_COUNTER) {
> -		if (stats[0].pkts)
> -			nf_ct_acct_add(offload->flow->ct,
> -				       FLOW_OFFLOAD_DIR_ORIGINAL,
> -				       stats[0].pkts, stats[0].bytes);
> -		if (stats[1].pkts)
> -			nf_ct_acct_add(offload->flow->ct,
> -				       FLOW_OFFLOAD_DIR_REPLY,
> -				       stats[1].pkts, stats[1].bytes);
> -	}
> +	flow_offload_update_stats(offload, stats);
>  }
>  
>  static void flow_offload_work_handler(struct work_struct *work)
> -- 
> 2.47.3
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH nf-next 2/2] net: ethernet: mtk_ppe: update stats on flow deletion

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 2580 bytes --]

> Update this driver to use the new approach to fetch stats on flow
> deletion, introduced by ("netfilter: flowtable: retrieve stats on flow
> deletion").
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> ---
>  .../net/ethernet/mediatek/mtk_ppe_offload.c   | 32 ++++++++++++-------
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> index 99b28aaa7cc4..8ca694bb5e1b 100644
> --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> @@ -526,6 +526,24 @@ mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f,
>  	return err;
>  }
>  
> +static void
> +mtk_flow_offload_update_stats(struct mtk_eth *eth, struct flow_cls_offload *f,
> +			      struct mtk_flow_entry *entry)
> +{
> +	struct mtk_foe_accounting diff;
> +	u32 idle;
> +
> +	idle = mtk_foe_entry_idle_time(eth->ppe[entry->ppe_index], entry);
> +	f->stats.lastused = jiffies - idle * HZ;
> +
> +	if (entry->hash != 0xFFFF &&
> +	    mtk_foe_entry_get_mib(eth->ppe[entry->ppe_index], entry->hash,
> +				  &diff)) {
> +		f->stats.pkts += diff.packets;
> +		f->stats.bytes += diff.bytes;
> +	}
> +}
> +
>  static int
>  mtk_flow_offload_destroy(struct mtk_eth *eth, struct flow_cls_offload *f)
>  {
> @@ -536,6 +554,8 @@ mtk_flow_offload_destroy(struct mtk_eth *eth, struct flow_cls_offload *f)
>  	if (!entry)
>  		return -ENOENT;
>  
> +	mtk_flow_offload_update_stats(eth, f, entry);
> +
>  	mtk_foe_entry_clear(eth->ppe[entry->ppe_index], entry);
>  	rhashtable_remove_fast(&eth->flow_table, &entry->node,
>  			       mtk_flow_ht_params);
> @@ -550,23 +570,13 @@ static int
>  mtk_flow_offload_stats(struct mtk_eth *eth, struct flow_cls_offload *f)
>  {
>  	struct mtk_flow_entry *entry;
> -	struct mtk_foe_accounting diff;
> -	u32 idle;
>  
>  	entry = rhashtable_lookup(&eth->flow_table, &f->cookie,
>  				  mtk_flow_ht_params);
>  	if (!entry)
>  		return -ENOENT;
>  
> -	idle = mtk_foe_entry_idle_time(eth->ppe[entry->ppe_index], entry);
> -	f->stats.lastused = jiffies - idle * HZ;
> -
> -	if (entry->hash != 0xFFFF &&
> -	    mtk_foe_entry_get_mib(eth->ppe[entry->ppe_index], entry->hash,
> -				  &diff)) {
> -		f->stats.pkts += diff.packets;
> -		f->stats.bytes += diff.bytes;
> -	}
> +	mtk_flow_offload_update_stats(eth, f, entry);
>  
>  	return 0;
>  }
> -- 
> 2.47.3
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion

[Pablo Neira Ayuso]

On Thu, Oct 29, 2026 at 12:25:21PM +0100, Lorenzo Bianconi wrote:
> > Retrieve stats too when deleting a flow from hardware. Apparently, the
> > existing approach is missing last stats sync with the conntrack entry.
> > Pass the stats object to the driver so it has a chance to populate it
> > at deletion time. Drivers need to be updated to use this new approach.
> > 
> > Reported-by: Valentin Berezhnoy <dreyksmail@gmail.com>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> >  net/netfilter/nf_flow_table_offload.c | 43 ++++++++++++++++-----------
> >  1 file changed, 26 insertions(+), 17 deletions(-)
> > 
> > diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> > index 4365859220e6..53182323f73c 100644
> > --- a/net/netfilter/nf_flow_table_offload.c
> > +++ b/net/netfilter/nf_flow_table_offload.c
> > @@ -934,6 +934,27 @@ static int nf_flow_offload_tuple(struct nf_flowtable *flowtable,
> >  	return i;
> >  }
> >  
> > +static void flow_offload_update_stats(struct flow_offload_work *offload,
> > +				      struct flow_stats *stats)
> > +{
> > +	u64 lastused;
> > +
> > +	lastused = max_t(u64, stats[0].lastused, stats[1].lastused);
> > +	offload->flow->timeout = max_t(u64, offload->flow->timeout,
> > +				       lastused + flow_offload_get_timeout(offload->flow));
> > +
> > +	if (offload->flowtable->flags & NF_FLOWTABLE_COUNTER) {
> > +		if (stats[0].pkts)
> > +			nf_ct_acct_add(offload->flow->ct,
> > +				       FLOW_OFFLOAD_DIR_ORIGINAL,
> > +				       stats[0].pkts, stats[0].bytes);
> > +		if (stats[1].pkts)
> > +			nf_ct_acct_add(offload->flow->ct,
> > +				       FLOW_OFFLOAD_DIR_REPLY,
> > +				       stats[1].pkts, stats[1].bytes);
> > +	}
> > +}
> > +
> >  static int flow_offload_tuple_add(struct flow_offload_work *offload,
> >  				  struct nf_flow_rule *flow_rule,
> >  				  enum flow_offload_tuple_dir dir)
> > @@ -948,10 +969,13 @@ static int flow_offload_tuple_add(struct flow_offload_work *offload,
> >  static void flow_offload_tuple_del(struct flow_offload_work *offload,
> >  				   enum flow_offload_tuple_dir dir)
> >  {
> > +	struct flow_stats stats[FLOW_OFFLOAD_DIR_MAX] = {};
> > +
> >  	nf_flow_offload_tuple(offload->flowtable, offload->flow, NULL, dir,
> >  			      offload->flowtable->priority,
> > -			      FLOW_CLS_DESTROY, NULL,
> > +			      FLOW_CLS_DESTROY, stats,
> >  			      &offload->flowtable->flow_block.cb_list);
> 
> according to my understanding nf_flow_offload_tuple() copies the stats just if
> cmd is set to FLOW_CLS_STATS, right? Should we just check if stats is not NULL
> in nf_flow_offload_tuple() instead?

I posted a v2 with:

        if (cmd == FLOW_CLS_STATS ||
            cmd == FLOW_CLS_DESTROY)

but I can post a v3 with.

        if (stats)

instead as you suggest.

> Regards,
> Lorenzo
> 
> > +	flow_offload_update_stats(offload, stats);
> >  }
> >  
> >  static int flow_offload_rule_add(struct flow_offload_work *offload,
> > @@ -1011,27 +1035,12 @@ static void flow_offload_tuple_stats(struct flow_offload_work *offload,
> >  static void flow_offload_work_stats(struct flow_offload_work *offload)
> >  {
> >  	struct flow_stats stats[FLOW_OFFLOAD_DIR_MAX] = {};
> > -	u64 lastused;
> >  
> >  	flow_offload_tuple_stats(offload, FLOW_OFFLOAD_DIR_ORIGINAL, &stats[0]);
> >  	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
> >  		flow_offload_tuple_stats(offload, FLOW_OFFLOAD_DIR_REPLY,
> >  					 &stats[1]);
> > -
> > -	lastused = max_t(u64, stats[0].lastused, stats[1].lastused);
> > -	offload->flow->timeout = max_t(u64, offload->flow->timeout,
> > -				       lastused + flow_offload_get_timeout(offload->flow));
> > -
> > -	if (offload->flowtable->flags & NF_FLOWTABLE_COUNTER) {
> > -		if (stats[0].pkts)
> > -			nf_ct_acct_add(offload->flow->ct,
> > -				       FLOW_OFFLOAD_DIR_ORIGINAL,
> > -				       stats[0].pkts, stats[0].bytes);
> > -		if (stats[1].pkts)
> > -			nf_ct_acct_add(offload->flow->ct,
> > -				       FLOW_OFFLOAD_DIR_REPLY,
> > -				       stats[1].pkts, stats[1].bytes);
> > -	}
> > +	flow_offload_update_stats(offload, stats);
> >  }
> >  
> >  static void flow_offload_work_handler(struct work_struct *work)
> > -- 
> > 2.47.3
> > 

Re: [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 4708 bytes --]

On Oct 29, Pablo Neira Ayuso wrote:
> On Thu, Oct 29, 2026 at 12:25:21PM +0100, Lorenzo Bianconi wrote:
> > > Retrieve stats too when deleting a flow from hardware. Apparently, the
> > > existing approach is missing last stats sync with the conntrack entry.
> > > Pass the stats object to the driver so it has a chance to populate it
> > > at deletion time. Drivers need to be updated to use this new approach.
> > > 
> > > Reported-by: Valentin Berezhnoy <dreyksmail@gmail.com>
> > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > > ---
> > >  net/netfilter/nf_flow_table_offload.c | 43 ++++++++++++++++-----------
> > >  1 file changed, 26 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> > > index 4365859220e6..53182323f73c 100644
> > > --- a/net/netfilter/nf_flow_table_offload.c
> > > +++ b/net/netfilter/nf_flow_table_offload.c
> > > @@ -934,6 +934,27 @@ static int nf_flow_offload_tuple(struct nf_flowtable *flowtable,
> > >  	return i;
> > >  }
> > >  
> > > +static void flow_offload_update_stats(struct flow_offload_work *offload,
> > > +				      struct flow_stats *stats)
> > > +{
> > > +	u64 lastused;
> > > +
> > > +	lastused = max_t(u64, stats[0].lastused, stats[1].lastused);
> > > +	offload->flow->timeout = max_t(u64, offload->flow->timeout,
> > > +				       lastused + flow_offload_get_timeout(offload->flow));
> > > +
> > > +	if (offload->flowtable->flags & NF_FLOWTABLE_COUNTER) {
> > > +		if (stats[0].pkts)
> > > +			nf_ct_acct_add(offload->flow->ct,
> > > +				       FLOW_OFFLOAD_DIR_ORIGINAL,
> > > +				       stats[0].pkts, stats[0].bytes);
> > > +		if (stats[1].pkts)
> > > +			nf_ct_acct_add(offload->flow->ct,
> > > +				       FLOW_OFFLOAD_DIR_REPLY,
> > > +				       stats[1].pkts, stats[1].bytes);
> > > +	}
> > > +}
> > > +
> > >  static int flow_offload_tuple_add(struct flow_offload_work *offload,
> > >  				  struct nf_flow_rule *flow_rule,
> > >  				  enum flow_offload_tuple_dir dir)
> > > @@ -948,10 +969,13 @@ static int flow_offload_tuple_add(struct flow_offload_work *offload,
> > >  static void flow_offload_tuple_del(struct flow_offload_work *offload,
> > >  				   enum flow_offload_tuple_dir dir)
> > >  {
> > > +	struct flow_stats stats[FLOW_OFFLOAD_DIR_MAX] = {};
> > > +
> > >  	nf_flow_offload_tuple(offload->flowtable, offload->flow, NULL, dir,
> > >  			      offload->flowtable->priority,
> > > -			      FLOW_CLS_DESTROY, NULL,
> > > +			      FLOW_CLS_DESTROY, stats,
> > >  			      &offload->flowtable->flow_block.cb_list);
> > 
> > according to my understanding nf_flow_offload_tuple() copies the stats just if
> > cmd is set to FLOW_CLS_STATS, right? Should we just check if stats is not NULL
> > in nf_flow_offload_tuple() instead?
> 
> I posted a v2 with:
> 
>         if (cmd == FLOW_CLS_STATS ||
>             cmd == FLOW_CLS_DESTROY)
> 
> but I can post a v3 with.
> 
>         if (stats)
> 
> instead as you suggest.

up to you, same for me. I guess you can add even add the support for airoha.

Regards,
Lorenzo

> 
> > Regards,
> > Lorenzo
> > 
> > > +	flow_offload_update_stats(offload, stats);
> > >  }
> > >  
> > >  static int flow_offload_rule_add(struct flow_offload_work *offload,
> > > @@ -1011,27 +1035,12 @@ static void flow_offload_tuple_stats(struct flow_offload_work *offload,
> > >  static void flow_offload_work_stats(struct flow_offload_work *offload)
> > >  {
> > >  	struct flow_stats stats[FLOW_OFFLOAD_DIR_MAX] = {};
> > > -	u64 lastused;
> > >  
> > >  	flow_offload_tuple_stats(offload, FLOW_OFFLOAD_DIR_ORIGINAL, &stats[0]);
> > >  	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
> > >  		flow_offload_tuple_stats(offload, FLOW_OFFLOAD_DIR_REPLY,
> > >  					 &stats[1]);
> > > -
> > > -	lastused = max_t(u64, stats[0].lastused, stats[1].lastused);
> > > -	offload->flow->timeout = max_t(u64, offload->flow->timeout,
> > > -				       lastused + flow_offload_get_timeout(offload->flow));
> > > -
> > > -	if (offload->flowtable->flags & NF_FLOWTABLE_COUNTER) {
> > > -		if (stats[0].pkts)
> > > -			nf_ct_acct_add(offload->flow->ct,
> > > -				       FLOW_OFFLOAD_DIR_ORIGINAL,
> > > -				       stats[0].pkts, stats[0].bytes);
> > > -		if (stats[1].pkts)
> > > -			nf_ct_acct_add(offload->flow->ct,
> > > -				       FLOW_OFFLOAD_DIR_REPLY,
> > > -				       stats[1].pkts, stats[1].bytes);
> > > -	}
> > > +	flow_offload_update_stats(offload, stats);
> > >  }
> > >  
> > >  static void flow_offload_work_handler(struct work_struct *work)
> > > -- 
> > > 2.47.3
> > > 
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 5131 bytes --]

On Oct 29, Lorenzo Bianconi wrote:
> On Oct 29, Pablo Neira Ayuso wrote:
> > On Thu, Oct 29, 2026 at 12:25:21PM +0100, Lorenzo Bianconi wrote:
> > > > Retrieve stats too when deleting a flow from hardware. Apparently, the
> > > > existing approach is missing last stats sync with the conntrack entry.
> > > > Pass the stats object to the driver so it has a chance to populate it
> > > > at deletion time. Drivers need to be updated to use this new approach.
> > > > 
> > > > Reported-by: Valentin Berezhnoy <dreyksmail@gmail.com>
> > > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > > > ---
> > > >  net/netfilter/nf_flow_table_offload.c | 43 ++++++++++++++++-----------
> > > >  1 file changed, 26 insertions(+), 17 deletions(-)
> > > > 
> > > > diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> > > > index 4365859220e6..53182323f73c 100644
> > > > --- a/net/netfilter/nf_flow_table_offload.c
> > > > +++ b/net/netfilter/nf_flow_table_offload.c
> > > > @@ -934,6 +934,27 @@ static int nf_flow_offload_tuple(struct nf_flowtable *flowtable,
> > > >  	return i;
> > > >  }
> > > >  
> > > > +static void flow_offload_update_stats(struct flow_offload_work *offload,
> > > > +				      struct flow_stats *stats)
> > > > +{
> > > > +	u64 lastused;
> > > > +
> > > > +	lastused = max_t(u64, stats[0].lastused, stats[1].lastused);
> > > > +	offload->flow->timeout = max_t(u64, offload->flow->timeout,
> > > > +				       lastused + flow_offload_get_timeout(offload->flow));
> > > > +
> > > > +	if (offload->flowtable->flags & NF_FLOWTABLE_COUNTER) {
> > > > +		if (stats[0].pkts)
> > > > +			nf_ct_acct_add(offload->flow->ct,
> > > > +				       FLOW_OFFLOAD_DIR_ORIGINAL,
> > > > +				       stats[0].pkts, stats[0].bytes);
> > > > +		if (stats[1].pkts)
> > > > +			nf_ct_acct_add(offload->flow->ct,
> > > > +				       FLOW_OFFLOAD_DIR_REPLY,
> > > > +				       stats[1].pkts, stats[1].bytes);
> > > > +	}
> > > > +}
> > > > +
> > > >  static int flow_offload_tuple_add(struct flow_offload_work *offload,
> > > >  				  struct nf_flow_rule *flow_rule,
> > > >  				  enum flow_offload_tuple_dir dir)
> > > > @@ -948,10 +969,13 @@ static int flow_offload_tuple_add(struct flow_offload_work *offload,
> > > >  static void flow_offload_tuple_del(struct flow_offload_work *offload,
> > > >  				   enum flow_offload_tuple_dir dir)
> > > >  {
> > > > +	struct flow_stats stats[FLOW_OFFLOAD_DIR_MAX] = {};
> > > > +
> > > >  	nf_flow_offload_tuple(offload->flowtable, offload->flow, NULL, dir,
> > > >  			      offload->flowtable->priority,
> > > > -			      FLOW_CLS_DESTROY, NULL,
> > > > +			      FLOW_CLS_DESTROY, stats,
> > > >  			      &offload->flowtable->flow_block.cb_list);
> > > 
> > > according to my understanding nf_flow_offload_tuple() copies the stats just if
> > > cmd is set to FLOW_CLS_STATS, right? Should we just check if stats is not NULL
> > > in nf_flow_offload_tuple() instead?
> > 
> > I posted a v2 with:
> > 
> >         if (cmd == FLOW_CLS_STATS ||
> >             cmd == FLOW_CLS_DESTROY)
> > 
> > but I can post a v3 with.
> > 
> >         if (stats)
> > 
> > instead as you suggest.
> 
> up to you, same for me. I guess you can add even add the support for airoha.
> 
> Regards,
> Lorenzo

off-topic:

Can I post new revision for flowtable sit support for nf-next even if net-next is
closed at the moment?

Regards,
Lorenzo

> 
> > 
> > > Regards,
> > > Lorenzo
> > > 
> > > > +	flow_offload_update_stats(offload, stats);
> > > >  }
> > > >  
> > > >  static int flow_offload_rule_add(struct flow_offload_work *offload,
> > > > @@ -1011,27 +1035,12 @@ static void flow_offload_tuple_stats(struct flow_offload_work *offload,
> > > >  static void flow_offload_work_stats(struct flow_offload_work *offload)
> > > >  {
> > > >  	struct flow_stats stats[FLOW_OFFLOAD_DIR_MAX] = {};
> > > > -	u64 lastused;
> > > >  
> > > >  	flow_offload_tuple_stats(offload, FLOW_OFFLOAD_DIR_ORIGINAL, &stats[0]);
> > > >  	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
> > > >  		flow_offload_tuple_stats(offload, FLOW_OFFLOAD_DIR_REPLY,
> > > >  					 &stats[1]);
> > > > -
> > > > -	lastused = max_t(u64, stats[0].lastused, stats[1].lastused);
> > > > -	offload->flow->timeout = max_t(u64, offload->flow->timeout,
> > > > -				       lastused + flow_offload_get_timeout(offload->flow));
> > > > -
> > > > -	if (offload->flowtable->flags & NF_FLOWTABLE_COUNTER) {
> > > > -		if (stats[0].pkts)
> > > > -			nf_ct_acct_add(offload->flow->ct,
> > > > -				       FLOW_OFFLOAD_DIR_ORIGINAL,
> > > > -				       stats[0].pkts, stats[0].bytes);
> > > > -		if (stats[1].pkts)
> > > > -			nf_ct_acct_add(offload->flow->ct,
> > > > -				       FLOW_OFFLOAD_DIR_REPLY,
> > > > -				       stats[1].pkts, stats[1].bytes);
> > > > -	}
> > > > +	flow_offload_update_stats(offload, stats);
> > > >  }
> > > >  
> > > >  static void flow_offload_work_handler(struct work_struct *work)
> > > > -- 
> > > > 2.47.3
> > > > 
> > 
> > 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion

[Pablo Neira Ayuso]

On Thu, Oct 29, 2026 at 12:44:20PM +0100, Lorenzo Bianconi wrote:
> off-topic:
> 
> Can I post new revision for flowtable sit support for nf-next even if net-next is
> closed at the moment?

Please, go ahead.