On Tue 24 Oct 2023 at 22:07, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > On Tue, Oct 24, 2023 at 10:45:31PM +0300, Vlad Buslov wrote: >> On Tue 24 Oct 2023 at 21:40, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: >> > 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. >> > >> >> Thanks for refactoring this, I agree that separating the act_ct-specific >> check makes it more obvious. >> >> How would you prefer to solve the conflict with my fix? Should I wait >> for your patch to be accepted to net, rebase my fix on top and submit >> V2? Or you can incorporate the checks from my fix together with my >> signoff and submit it as a single change? > > Rebased here as per your request: > > https://patchwork.ozlabs.org/project/netfilter-devel/patch/20231024200243.50784-1-pablo@xxxxxxxxxxxxx/ > > I took the freedom to take your Signed-off-by: and Paul's Reviewed-by: > which is not the best way to go, but please acknowledge this is fine > in this exceptional case. Ack. Replied to the patch with my signed-off-by. Thanks! > > We can handle this via nf.git tree, there were no plans to send a PR > to netdev, but I think these fixes are worth to (try to) get them > there in time for the 6.6 release. > > Thanks.
