[PATCH nf] netfilter: flowtable: fetch hardware counters before removing a flow

[Valentin Berezhnoy <dreyksmail@gmail.com>]

The counters of hardware offloaded flows are added to the conntrack
entry by the stats work, which nf_flow_offload_stats() queues only once
a tenth of the flow timeout has passed since the last update, i.e.
about every 3 seconds with the default nf_flowtable_tcp_timeout of 30
seconds. When a flow is torn down, e.g. after a TCP FIN or RST,
flow_offload_work_del() removes the hardware entries without fetching
their counters, so the traffic since the last update never reaches
conntrack.

At gigabit speeds that can be several hundred megabytes per connection.
On a MediaTek MT7622 router with hardware flow offloading, a 300 MiB
download that closed its connection right after the transfer was
accounted at 69-84% by conntrack, but at 105% (payload plus headers)
when the connection stayed open for 10 seconds before closing, i.e.
after one more stats update.

Fetch the counters once more in the delete work, before the hardware
entries are removed. The flow and its conntrack entry are still
referenced at that point: the garbage collector only releases the flow
once NF_FLOW_HW_DEAD is set at the end of the delete work.

Fixes: ef803b3cf96a ("netfilter: flowtable: add counter support in HW offload")
Assisted-by: LLM
Signed-off-by: Valentin Berezhnoy <dreyksmail@gmail.com>
---

Notes:
    This patch and its changelog were written with an AI coding assistant
    (Claude), which also ran the tests below; I reviewed the change and the
    results.
    
    Testing:
    - Runtime: OpenWrt main (Linux 6.18.55) on a Redmi AX6S (MediaTek
      MT7622) with flowtable hardware offloading and WED, counters read
      through nlbwmon (conntrack). 300 MiB downloads that close their
      connection right after the transfer are now accounted at 105.0 %
      (3 of 3 runs; payload plus TCP/IP headers), 30 downloads of 10 MiB
      on separate connections at 104.9 %, the same as a download whose
      connection stays open 10 s before closing (105.0 %). That kernel also
      carries a fix for OpenWrt's out-of-tree mtk_ppe accounting patch,
      which is needed on that driver for the final poll to read the
      counters of an entry the PPE already unbound after the FIN.
    - Build: net/netfilter/nf_flow_table_offload.o on this tree with W=1
      (x86_64 defconfig plus netfilter), no warnings. Not runtime-tested on
      this tree itself.

 net/netfilter/nf_flow_table_offload.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index 436585922..056c000a6 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -989,15 +989,6 @@ static void flow_offload_work_add(struct flow_offload_work *offload)
 	nf_flow_offload_destroy(flow_rule);
 }
 
-static void flow_offload_work_del(struct flow_offload_work *offload)
-{
-	clear_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status);
-	flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_ORIGINAL);
-	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
-		flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_REPLY);
-	set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
-}
-
 static void flow_offload_tuple_stats(struct flow_offload_work *offload,
 				     enum flow_offload_tuple_dir dir,
 				     struct flow_stats *stats)
@@ -1034,6 +1025,22 @@ static void flow_offload_work_stats(struct flow_offload_work *offload)
 	}
 }
 
+static void flow_offload_work_del(struct flow_offload_work *offload)
+{
+	/* Stats are only polled every few seconds: fetch the counters of the
+	 * hardware entries one last time before they are removed, so that the
+	 * traffic since the last poll is not lost for conntrack accounting.
+	 */
+	if (offload->flowtable->flags & NF_FLOWTABLE_COUNTER)
+		flow_offload_work_stats(offload);
+
+	clear_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status);
+	flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_ORIGINAL);
+	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
+		flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_REPLY);
+	set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
+}
+
 static void flow_offload_work_handler(struct work_struct *work)
 {
 	struct flow_offload_work *offload;
-- 
2.53.0