* [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion
@ 2026-10-29 11:14 Pablo Neira Ayuso
2026-10-29 11:14 ` [PATCH nf-next 2/2] net: ethernet: mtk_ppe: update " Pablo Neira Ayuso
2026-10-29 11:25 ` [PATCH nf-next 1/2] netfilter: flowtable: retrieve " Lorenzo Bianconi
0 siblings, 2 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2026-10-29 11:14 UTC (permalink / raw)
To: netfilter-devel; +Cc: lorenzo, nbd
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH nf-next 2/2] net: ethernet: mtk_ppe: update stats on flow deletion
2026-10-29 11:14 [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion Pablo Neira Ayuso
@ 2026-10-29 11:14 ` Pablo Neira Ayuso
2026-10-29 11:29 ` Lorenzo Bianconi
2026-10-29 11:25 ` [PATCH nf-next 1/2] netfilter: flowtable: retrieve " Lorenzo Bianconi
1 sibling, 1 reply; 8+ messages in thread
From: Pablo Neira Ayuso @ 2026-10-29 11:14 UTC (permalink / raw)
To: netfilter-devel; +Cc: lorenzo, nbd
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(ð->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(ð->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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH nf-next 2/2] net: ethernet: mtk_ppe: update stats on flow deletion
2026-10-29 11:14 ` [PATCH nf-next 2/2] net: ethernet: mtk_ppe: update " Pablo Neira Ayuso
@ 2026-10-29 11:29 ` Lorenzo Bianconi
0 siblings, 0 replies; 8+ messages in thread
From: Lorenzo Bianconi @ 2026-10-29 11:29 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, nbd
[-- 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(ð->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(ð->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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion
2026-10-29 11:14 [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion Pablo Neira Ayuso
2026-10-29 11:14 ` [PATCH nf-next 2/2] net: ethernet: mtk_ppe: update " Pablo Neira Ayuso
@ 2026-10-29 11:25 ` Lorenzo Bianconi
2026-10-29 11:39 ` Pablo Neira Ayuso
1 sibling, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2026-10-29 11:25 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, nbd
[-- 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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion
2026-10-29 11:25 ` [PATCH nf-next 1/2] netfilter: flowtable: retrieve " Lorenzo Bianconi
@ 2026-10-29 11:39 ` Pablo Neira Ayuso
2026-10-29 11:41 ` Lorenzo Bianconi
0 siblings, 1 reply; 8+ messages in thread
From: Pablo Neira Ayuso @ 2026-10-29 11:39 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: netfilter-devel, nbd
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
> >
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion
2026-10-29 11:39 ` Pablo Neira Ayuso
@ 2026-10-29 11:41 ` Lorenzo Bianconi
2026-10-29 11:44 ` Lorenzo Bianconi
0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2026-10-29 11:41 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, nbd
[-- 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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion
2026-10-29 11:41 ` Lorenzo Bianconi
@ 2026-10-29 11:44 ` Lorenzo Bianconi
2026-10-29 11:45 ` Pablo Neira Ayuso
0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2026-10-29 11:44 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, nbd
[-- 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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-29 11:45 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-29 11:14 [PATCH nf-next 1/2] netfilter: flowtable: retrieve stats on flow deletion Pablo Neira Ayuso
2026-10-29 11:14 ` [PATCH nf-next 2/2] net: ethernet: mtk_ppe: update " Pablo Neira Ayuso
2026-10-29 11:29 ` Lorenzo Bianconi
2026-10-29 11:25 ` [PATCH nf-next 1/2] netfilter: flowtable: retrieve " Lorenzo Bianconi
2026-10-29 11:39 ` Pablo Neira Ayuso
2026-10-29 11:41 ` Lorenzo Bianconi
2026-10-29 11:44 ` Lorenzo Bianconi
2026-10-29 11:45 ` Pablo Neira Ayuso
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.