Hi Vlad, On Tue, Oct 24, 2023 at 07:17:18PM +0200, Vlad Buslov wrote: > Current nf_flow_is_outdated() implementation considers any flow table flow > which state diverged from its underlying CT connection status for teardown > which can be problematic in the following cases: > > - Flow has never been offloaded to hardware in the first place either > because flow table has hardware offload disabled (flag > NF_FLOWTABLE_HW_OFFLOAD is not set) or because it is still pending on 'add' > workqueue to be offloaded for the first time. The former is incorrect, the > later generates excessive deletions and additions of flows. > > - Flow is already pending to be updated on the workqueue. Tearing down such > flows will also generate excessive removals from the flow table, especially > on highly loaded system where the latency to re-offload a flow via 'add' > workqueue can be quite high. > > When considering a flow for teardown as outdated verify that it is both > offloaded to hardware and doesn't have any pending updates. Thanks. I have posted an alternative patch to move the handling of NF_FLOW_HW_ESTABLISHED, which is specific for sched/act_ct: https://patchwork.ozlabs.org/project/netfilter-devel/patch/20231024193815.1987-1-pablo@xxxxxxxxxxxxx/ it is a bit more code, but it makes it easier to understand for the code reader that this bit is net/sched specific. > Fixes: 41f2c7c342d3 ("net/sched: act_ct: Fix promotion of offloaded unreplied tuple") > Reviewed-by: Paul Blakey <paulb@xxxxxxxxxx> > Signed-off-by: Vlad Buslov <vladbu@xxxxxxxxxx> > --- > net/netfilter/nf_flow_table_core.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c > index 1d34d700bd09..db404f89d3d7 100644 > --- a/net/netfilter/nf_flow_table_core.c > +++ b/net/netfilter/nf_flow_table_core.c > @@ -319,6 +319,8 @@ EXPORT_SYMBOL_GPL(flow_offload_refresh); > static bool nf_flow_is_outdated(const struct flow_offload *flow) > { > return test_bit(IPS_SEEN_REPLY_BIT, &flow->ct->status) && > + test_bit(IPS_HW_OFFLOAD_BIT, &flow->ct->status) && > + !test_bit(NF_FLOW_HW_PENDING, &flow->flags) && > !test_bit(NF_FLOW_HW_ESTABLISHED, &flow->flags); > } > > -- > 2.39.2 >
