| Message ID | 20231023101347.564898-1-donald.hunter@gmail.com |
|---|---|
| State | RFC |
| Headers | show |
| Series | [netfilter] Fix hw flow offload from nftables | expand |
On Mon, Oct 23, 2023 at 11:13:47AM +0100, Donald Hunter wrote: > The NF_FLOW_HW_ESTABLISHED bit was not getting set in any nftables code > paths. It seems that the state was never correctly maintained but there > was no negative side-effect until commit 41f2c7c342d3 ("net/sched: > act_ct: Fix promotion of offloaded unreplied tuple") which uses it as > part of a flow outdated check. The net result is repeated cycles of > FLOW_CLS_REPLACE / FLOW_CLS_DESTROY commands and never getting any > FLOW_CLS_STATS commands for a flow. > > This patch sets and clears the NF_FLOW_HW_ESTABLISHED bit for nftables. > > Note that I don't have hardware to test this with. I have observed > the behaviour and verified the fix with modified veth code. Very strange, this bit did not exists before: commit 1a441a9b8be8849957a01413a144f84932c324cb Author: Vlad Buslov <vladbu@nvidia.com> Date: Wed Feb 1 17:30:57 2023 +0100 netfilter: flowtable: cache info of last offload I have no idea why this is needed. > Signed-off-by: Donald Hunter <donald.hunter@gmail.com> > Fixes: 41f2c7c342d3 ("net/sched: act_ct: Fix promotion of offloaded unreplied tuple") > --- > net/netfilter/nf_flow_table_offload.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c > index 1c26f03fc661..1d017191af80 100644 > --- a/net/netfilter/nf_flow_table_offload.c > +++ b/net/netfilter/nf_flow_table_offload.c > @@ -918,6 +918,7 @@ static void flow_offload_work_add(struct flow_offload_work *offload) > goto out; > > set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > + set_bit(NF_FLOW_HW_ESTABLISHED, &offload->flow->flags); > > out: > nf_flow_offload_destroy(flow_rule); > @@ -925,6 +926,7 @@ static void flow_offload_work_add(struct flow_offload_work *offload) > > static void flow_offload_work_del(struct flow_offload_work *offload) > { > + clear_bit(NF_FLOW_HW_ESTABLISHED, &offload->flow->flags); > 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)) > -- > 2.41.0 >
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c index 1c26f03fc661..1d017191af80 100644 --- a/net/netfilter/nf_flow_table_offload.c +++ b/net/netfilter/nf_flow_table_offload.c @@ -918,6 +918,7 @@ static void flow_offload_work_add(struct flow_offload_work *offload) goto out; set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); + set_bit(NF_FLOW_HW_ESTABLISHED, &offload->flow->flags); out: nf_flow_offload_destroy(flow_rule); @@ -925,6 +926,7 @@ static void flow_offload_work_add(struct flow_offload_work *offload) static void flow_offload_work_del(struct flow_offload_work *offload) { + clear_bit(NF_FLOW_HW_ESTABLISHED, &offload->flow->flags); 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))
The NF_FLOW_HW_ESTABLISHED bit was not getting set in any nftables code paths. It seems that the state was never correctly maintained but there was no negative side-effect until commit 41f2c7c342d3 ("net/sched: act_ct: Fix promotion of offloaded unreplied tuple") which uses it as part of a flow outdated check. The net result is repeated cycles of FLOW_CLS_REPLACE / FLOW_CLS_DESTROY commands and never getting any FLOW_CLS_STATS commands for a flow. This patch sets and clears the NF_FLOW_HW_ESTABLISHED bit for nftables. Note that I don't have hardware to test this with. I have observed the behaviour and verified the fix with modified veth code. Signed-off-by: Donald Hunter <donald.hunter@gmail.com> Fixes: 41f2c7c342d3 ("net/sched: act_ct: Fix promotion of offloaded unreplied tuple") --- net/netfilter/nf_flow_table_offload.c | 2 ++ 1 file changed, 2 insertions(+)