| Message ID | 1620305213-166700-1-git-send-email-danielj@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,F:linux-bluefield] UBUNTU: SAUCE: netfilter: flowtable: Do flow offload refresh when requested | expand |
Is this a behavior unique to bluefield ? In other words, should this patch go upstream as well ? This kind of change looks like it impacts the offload logic for all NICs. rtg On 5/6/21 6:46 AM, Daniel Jurgens wrote: > BugLink: https://bugs.launchpad.net/bugs/1927374 > > Offload could fail for multiple reasons and a hw refresh bit is > set to try to reoffload it in next sw packet. > But sometimes the hw refresh bit is not set but a refresh could succeed. > Remove the hw refresh bit and do offload refresh if requested. > There won't be a new work entry if a work is already pending > anyway as there is the hw pending bit. > > Fixes: 8b3646d6e0c4 ("net/sched: act_ct: Support refreshing the flow table entries") > Signed-off-by: Roi Dayan <roid@nvidia.com> > Signed-off-by: Daniel Jurgens <danielj@nvidia.com> > --- > include/net/netfilter/nf_flow_table.h | 1 - > net/netfilter/nf_flow_table_core.c | 3 +-- > net/netfilter/nf_flow_table_offload.c | 7 ++++--- > 3 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h > index ff14b39..ba3b58a 100644 > --- a/include/net/netfilter/nf_flow_table.h > +++ b/include/net/netfilter/nf_flow_table.h > @@ -126,7 +126,6 @@ enum nf_flow_flags { > NF_FLOW_HW, > NF_FLOW_HW_DYING, > NF_FLOW_HW_DEAD, > - NF_FLOW_HW_REFRESH, > NF_FLOW_HW_PENDING, > }; > > diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c > index 64df3f8..2405eac 100644 > --- a/net/netfilter/nf_flow_table_core.c > +++ b/net/netfilter/nf_flow_table_core.c > @@ -261,8 +261,7 @@ void flow_offload_refresh(struct nf_flowtable *flow_table, > flow->timeout = nf_flowtable_time_stamp + > nf_flow_offload_timeout(flow_table); > > - if (likely(!nf_flowtable_hw_offload(flow_table) || > - !test_and_clear_bit(NF_FLOW_HW_REFRESH, &flow->flags))) > + if (likely(!nf_flowtable_hw_offload(flow_table))) > return; > > nf_flow_offload_add(flow_table, flow); > diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c > index 28a2983..ad705ca 100644 > --- a/net/netfilter/nf_flow_table_offload.c > +++ b/net/netfilter/nf_flow_table_offload.c > @@ -759,10 +759,11 @@ static void flow_offload_work_add(struct flow_offload_work *offload) > > err = flow_offload_rule_add(offload, flow_rule); > if (err < 0) > - set_bit(NF_FLOW_HW_REFRESH, &offload->flow->flags); > - else > - set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > + goto out; > + > + set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > > +out: > nf_flow_offload_destroy(flow_rule); > } > >
> From: Tim Gardner <tim.gardner@canonical.com> > Sent: Friday, May 7, 2021 9:26 AM > Is this a behavior unique to bluefield ? In other words, should this patch go > upstream as well ? This kind of change looks like it impacts the offload logic > for all NICs. > > rtg It's not unique to bluefield, it will go upstream. It's just not there yet. > > On 5/6/21 6:46 AM, Daniel Jurgens wrote: > > BugLink: https://bugs.launchpad.net/bugs/1927374 > > > > Offload could fail for multiple reasons and a hw refresh bit is set to > > try to reoffload it in next sw packet. > > But sometimes the hw refresh bit is not set but a refresh could succeed. > > Remove the hw refresh bit and do offload refresh if requested. > > There won't be a new work entry if a work is already pending anyway as > > there is the hw pending bit. > > > > Fixes: 8b3646d6e0c4 ("net/sched: act_ct: Support refreshing the flow > > table entries") > > Signed-off-by: Roi Dayan <roid@nvidia.com> > > Signed-off-by: Daniel Jurgens <danielj@nvidia.com> > > --- > > include/net/netfilter/nf_flow_table.h | 1 - > > net/netfilter/nf_flow_table_core.c | 3 +-- > > net/netfilter/nf_flow_table_offload.c | 7 ++++--- > > 3 files changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/include/net/netfilter/nf_flow_table.h > > b/include/net/netfilter/nf_flow_table.h > > index ff14b39..ba3b58a 100644 > > --- a/include/net/netfilter/nf_flow_table.h > > +++ b/include/net/netfilter/nf_flow_table.h > > @@ -126,7 +126,6 @@ enum nf_flow_flags { > > NF_FLOW_HW, > > NF_FLOW_HW_DYING, > > NF_FLOW_HW_DEAD, > > - NF_FLOW_HW_REFRESH, > > NF_FLOW_HW_PENDING, > > }; > > > > diff --git a/net/netfilter/nf_flow_table_core.c > > b/net/netfilter/nf_flow_table_core.c > > index 64df3f8..2405eac 100644 > > --- a/net/netfilter/nf_flow_table_core.c > > +++ b/net/netfilter/nf_flow_table_core.c > > @@ -261,8 +261,7 @@ void flow_offload_refresh(struct nf_flowtable > *flow_table, > > flow->timeout = nf_flowtable_time_stamp + > > nf_flow_offload_timeout(flow_table); > > > > - if (likely(!nf_flowtable_hw_offload(flow_table) || > > - !test_and_clear_bit(NF_FLOW_HW_REFRESH, &flow- > >flags))) > > + if (likely(!nf_flowtable_hw_offload(flow_table))) > > return; > > > > nf_flow_offload_add(flow_table, flow); diff --git > > a/net/netfilter/nf_flow_table_offload.c > > b/net/netfilter/nf_flow_table_offload.c > > index 28a2983..ad705ca 100644 > > --- a/net/netfilter/nf_flow_table_offload.c > > +++ b/net/netfilter/nf_flow_table_offload.c > > @@ -759,10 +759,11 @@ static void flow_offload_work_add(struct > > flow_offload_work *offload) > > > > err = flow_offload_rule_add(offload, flow_rule); > > if (err < 0) > > - set_bit(NF_FLOW_HW_REFRESH, &offload->flow->flags); > > - else > > - set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > > + goto out; > > + > > + set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > > > > +out: > > nf_flow_offload_destroy(flow_rule); > > } > > > > > > -- > ----------- > Tim Gardner > Canonical, Inc
Acked-by: Tim Gardner <tim.gardner@canonical.com> If I'm reading this right, this patch is dealing with offload request responses from NIC firmware, right ? I'm not convinced this will pass an upstream review because it seems like a bit a of a hack, but then the patch this is fixing 8b3646d6e0c4 ("net/sched: act_ct: Support refreshing the flow table entries") seems like a bit of a hack too. Sometimes you just have to work around firmware behaviors. Feel free to tell me if I'm completely wrong :) On 5/6/21 6:46 AM, Daniel Jurgens wrote: > BugLink: https://bugs.launchpad.net/bugs/1927374 > > Offload could fail for multiple reasons and a hw refresh bit is > set to try to reoffload it in next sw packet. > But sometimes the hw refresh bit is not set but a refresh could succeed. > Remove the hw refresh bit and do offload refresh if requested. > There won't be a new work entry if a work is already pending > anyway as there is the hw pending bit. > > Fixes: 8b3646d6e0c4 ("net/sched: act_ct: Support refreshing the flow table entries") > Signed-off-by: Roi Dayan <roid@nvidia.com> > Signed-off-by: Daniel Jurgens <danielj@nvidia.com> > --- > include/net/netfilter/nf_flow_table.h | 1 - > net/netfilter/nf_flow_table_core.c | 3 +-- > net/netfilter/nf_flow_table_offload.c | 7 ++++--- > 3 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h > index ff14b39..ba3b58a 100644 > --- a/include/net/netfilter/nf_flow_table.h > +++ b/include/net/netfilter/nf_flow_table.h > @@ -126,7 +126,6 @@ enum nf_flow_flags { > NF_FLOW_HW, > NF_FLOW_HW_DYING, > NF_FLOW_HW_DEAD, > - NF_FLOW_HW_REFRESH, > NF_FLOW_HW_PENDING, > }; > > diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c > index 64df3f8..2405eac 100644 > --- a/net/netfilter/nf_flow_table_core.c > +++ b/net/netfilter/nf_flow_table_core.c > @@ -261,8 +261,7 @@ void flow_offload_refresh(struct nf_flowtable *flow_table, > flow->timeout = nf_flowtable_time_stamp + > nf_flow_offload_timeout(flow_table); > > - if (likely(!nf_flowtable_hw_offload(flow_table) || > - !test_and_clear_bit(NF_FLOW_HW_REFRESH, &flow->flags))) > + if (likely(!nf_flowtable_hw_offload(flow_table))) > return; > > nf_flow_offload_add(flow_table, flow); > diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c > index 28a2983..ad705ca 100644 > --- a/net/netfilter/nf_flow_table_offload.c > +++ b/net/netfilter/nf_flow_table_offload.c > @@ -759,10 +759,11 @@ static void flow_offload_work_add(struct flow_offload_work *offload) > > err = flow_offload_rule_add(offload, flow_rule); > if (err < 0) > - set_bit(NF_FLOW_HW_REFRESH, &offload->flow->flags); > - else > - set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > + goto out; > + > + set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > > +out: > nf_flow_offload_destroy(flow_rule); > } > >
> -----Original Message----- > From: Tim Gardner <tim.gardner@canonical.com> > Sent: Friday, May 7, 2021 10:12 AM > > Acked-by: Tim Gardner <tim.gardner@canonical.com> > > If I'm reading this right, this patch is dealing with offload request responses > from NIC firmware, right ? > > I'm not convinced this will pass an upstream review because it seems like a > bit a of a hack, but then the patch this is fixing 8b3646d6e0c4 > ("net/sched: act_ct: Support refreshing the flow table entries") seems like a > bit of a hack too. Sometimes you just have to work around firmware > behaviors. > > Feel free to tell me if I'm completely wrong :) I'll let Roi answer this one. > > On 5/6/21 6:46 AM, Daniel Jurgens wrote: > > BugLink: https://bugs.launchpad.net/bugs/1927374 > > > > Offload could fail for multiple reasons and a hw refresh bit is set to > > try to reoffload it in next sw packet. > > But sometimes the hw refresh bit is not set but a refresh could succeed. > > Remove the hw refresh bit and do offload refresh if requested. > > There won't be a new work entry if a work is already pending anyway as > > there is the hw pending bit. > > > > Fixes: 8b3646d6e0c4 ("net/sched: act_ct: Support refreshing the flow > > table entries") > > Signed-off-by: Roi Dayan <roid@nvidia.com> > > Signed-off-by: Daniel Jurgens <danielj@nvidia.com> > > --- > > include/net/netfilter/nf_flow_table.h | 1 - > > net/netfilter/nf_flow_table_core.c | 3 +-- > > net/netfilter/nf_flow_table_offload.c | 7 ++++--- > > 3 files changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/include/net/netfilter/nf_flow_table.h > > b/include/net/netfilter/nf_flow_table.h > > index ff14b39..ba3b58a 100644 > > --- a/include/net/netfilter/nf_flow_table.h > > +++ b/include/net/netfilter/nf_flow_table.h > > @@ -126,7 +126,6 @@ enum nf_flow_flags { > > NF_FLOW_HW, > > NF_FLOW_HW_DYING, > > NF_FLOW_HW_DEAD, > > - NF_FLOW_HW_REFRESH, > > NF_FLOW_HW_PENDING, > > }; > > > > diff --git a/net/netfilter/nf_flow_table_core.c > > b/net/netfilter/nf_flow_table_core.c > > index 64df3f8..2405eac 100644 > > --- a/net/netfilter/nf_flow_table_core.c > > +++ b/net/netfilter/nf_flow_table_core.c > > @@ -261,8 +261,7 @@ void flow_offload_refresh(struct nf_flowtable > *flow_table, > > flow->timeout = nf_flowtable_time_stamp + > > nf_flow_offload_timeout(flow_table); > > > > - if (likely(!nf_flowtable_hw_offload(flow_table) || > > - !test_and_clear_bit(NF_FLOW_HW_REFRESH, &flow- > >flags))) > > + if (likely(!nf_flowtable_hw_offload(flow_table))) > > return; > > > > nf_flow_offload_add(flow_table, flow); diff --git > > a/net/netfilter/nf_flow_table_offload.c > > b/net/netfilter/nf_flow_table_offload.c > > index 28a2983..ad705ca 100644 > > --- a/net/netfilter/nf_flow_table_offload.c > > +++ b/net/netfilter/nf_flow_table_offload.c > > @@ -759,10 +759,11 @@ static void flow_offload_work_add(struct > > flow_offload_work *offload) > > > > err = flow_offload_rule_add(offload, flow_rule); > > if (err < 0) > > - set_bit(NF_FLOW_HW_REFRESH, &offload->flow->flags); > > - else > > - set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > > + goto out; > > + > > + set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > > > > +out: > > nf_flow_offload_destroy(flow_rule); > > } > > > > > > -- > ----------- > Tim Gardner > Canonical, Inc
On 2021-05-07 9:30 PM, Daniel Jurgens wrote: >> -----Original Message----- >> From: Tim Gardner <tim.gardner@canonical.com> >> Sent: Friday, May 7, 2021 10:12 AM >> >> Acked-by: Tim Gardner <tim.gardner@canonical.com> >> >> If I'm reading this right, this patch is dealing with offload request responses >> from NIC firmware, right ? >> >> I'm not convinced this will pass an upstream review because it seems like a >> bit a of a hack, but then the patch this is fixing 8b3646d6e0c4 >> ("net/sched: act_ct: Support refreshing the flow table entries") seems like a >> bit of a hack too. Sometimes you just have to work around firmware >> behaviors. >> >> Feel free to tell me if I'm completely wrong :) > > I'll let Roi answer this one. > Hi, I started to discuss about this with Pablo, netfilter maintainer. explaining my thoughts that this bit is redundant really. I do hope to do get this hw refresh bit removed. as tried to explain in the commit message, the later added hw pending bit already protects from trying to create a new work if not needed and we do check if the flowtable has hw offload bit anyway if tuples in this flowtable should be offloaded. there is no point for this bit and it causing issue today and can cause issue later if/when more conditions will stop the current offload but wont set the refresh bit actively. even today the single place that set the refresh bit has a race that the hw pending bit was not removed so new skb could clear the refresh bit and stop because of the hw pending bit still exists and no refresh will happen ever again. ending with connections that never gets offloaded even though possible. Thanks, Roi >> >> On 5/6/21 6:46 AM, Daniel Jurgens wrote: >>> BugLink: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.launchpad.net%2Fbugs%2F1927374&data=04%7C01%7Croid%40nvidia.com%7Cae611c26cea24e1ca6d608d9118638a6%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637560090442320882%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=anO2XFv6KZQSBR%2BEMIiI1UUI4TgazJrzg%2FwZmxsj%2Fr8%3D&reserved=0 >>> >>> Offload could fail for multiple reasons and a hw refresh bit is set to >>> try to reoffload it in next sw packet. >>> But sometimes the hw refresh bit is not set but a refresh could succeed. >>> Remove the hw refresh bit and do offload refresh if requested. >>> There won't be a new work entry if a work is already pending anyway as >>> there is the hw pending bit. >>> >>> Fixes: 8b3646d6e0c4 ("net/sched: act_ct: Support refreshing the flow >>> table entries") >>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>> Signed-off-by: Daniel Jurgens <danielj@nvidia.com> >>> --- >>> include/net/netfilter/nf_flow_table.h | 1 - >>> net/netfilter/nf_flow_table_core.c | 3 +-- >>> net/netfilter/nf_flow_table_offload.c | 7 ++++--- >>> 3 files changed, 5 insertions(+), 6 deletions(-) >>> >>> diff --git a/include/net/netfilter/nf_flow_table.h >>> b/include/net/netfilter/nf_flow_table.h >>> index ff14b39..ba3b58a 100644 >>> --- a/include/net/netfilter/nf_flow_table.h >>> +++ b/include/net/netfilter/nf_flow_table.h >>> @@ -126,7 +126,6 @@ enum nf_flow_flags { >>> NF_FLOW_HW, >>> NF_FLOW_HW_DYING, >>> NF_FLOW_HW_DEAD, >>> - NF_FLOW_HW_REFRESH, >>> NF_FLOW_HW_PENDING, >>> }; >>> >>> diff --git a/net/netfilter/nf_flow_table_core.c >>> b/net/netfilter/nf_flow_table_core.c >>> index 64df3f8..2405eac 100644 >>> --- a/net/netfilter/nf_flow_table_core.c >>> +++ b/net/netfilter/nf_flow_table_core.c >>> @@ -261,8 +261,7 @@ void flow_offload_refresh(struct nf_flowtable >> *flow_table, >>> flow->timeout = nf_flowtable_time_stamp + >>> nf_flow_offload_timeout(flow_table); >>> >>> - if (likely(!nf_flowtable_hw_offload(flow_table) || >>> - !test_and_clear_bit(NF_FLOW_HW_REFRESH, &flow- >>> flags))) >>> + if (likely(!nf_flowtable_hw_offload(flow_table))) >>> return; >>> >>> nf_flow_offload_add(flow_table, flow); diff --git >>> a/net/netfilter/nf_flow_table_offload.c >>> b/net/netfilter/nf_flow_table_offload.c >>> index 28a2983..ad705ca 100644 >>> --- a/net/netfilter/nf_flow_table_offload.c >>> +++ b/net/netfilter/nf_flow_table_offload.c >>> @@ -759,10 +759,11 @@ static void flow_offload_work_add(struct >>> flow_offload_work *offload) >>> >>> err = flow_offload_rule_add(offload, flow_rule); >>> if (err < 0) >>> - set_bit(NF_FLOW_HW_REFRESH, &offload->flow->flags); >>> - else >>> - set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); >>> + goto out; >>> + >>> + set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); >>> >>> +out: >>> nf_flow_offload_destroy(flow_rule); >>> } >>> >>> >> >> -- >> ----------- >> Tim Gardner >> Canonical, Inc
On 06.05.21 14:46, Daniel Jurgens wrote: > BugLink: https://bugs.launchpad.net/bugs/1927374 > > Offload could fail for multiple reasons and a hw refresh bit is > set to try to reoffload it in next sw packet. > But sometimes the hw refresh bit is not set but a refresh could succeed. > Remove the hw refresh bit and do offload refresh if requested. > There won't be a new work entry if a work is already pending > anyway as there is the hw pending bit. > > Fixes: 8b3646d6e0c4 ("net/sched: act_ct: Support refreshing the flow table entries") > Signed-off-by: Roi Dayan <roid@nvidia.com> > Signed-off-by: Daniel Jurgens <danielj@nvidia.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > include/net/netfilter/nf_flow_table.h | 1 - > net/netfilter/nf_flow_table_core.c | 3 +-- > net/netfilter/nf_flow_table_offload.c | 7 ++++--- > 3 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h > index ff14b39..ba3b58a 100644 > --- a/include/net/netfilter/nf_flow_table.h > +++ b/include/net/netfilter/nf_flow_table.h > @@ -126,7 +126,6 @@ enum nf_flow_flags { > NF_FLOW_HW, > NF_FLOW_HW_DYING, > NF_FLOW_HW_DEAD, > - NF_FLOW_HW_REFRESH, > NF_FLOW_HW_PENDING, > }; > > diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c > index 64df3f8..2405eac 100644 > --- a/net/netfilter/nf_flow_table_core.c > +++ b/net/netfilter/nf_flow_table_core.c > @@ -261,8 +261,7 @@ void flow_offload_refresh(struct nf_flowtable *flow_table, > flow->timeout = nf_flowtable_time_stamp + > nf_flow_offload_timeout(flow_table); > > - if (likely(!nf_flowtable_hw_offload(flow_table) || > - !test_and_clear_bit(NF_FLOW_HW_REFRESH, &flow->flags))) > + if (likely(!nf_flowtable_hw_offload(flow_table))) > return; > > nf_flow_offload_add(flow_table, flow); > diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c > index 28a2983..ad705ca 100644 > --- a/net/netfilter/nf_flow_table_offload.c > +++ b/net/netfilter/nf_flow_table_offload.c > @@ -759,10 +759,11 @@ static void flow_offload_work_add(struct flow_offload_work *offload) > > err = flow_offload_rule_add(offload, flow_rule); > if (err < 0) > - set_bit(NF_FLOW_HW_REFRESH, &offload->flow->flags); > - else > - set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > + goto out; > + > + set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > > +out: > nf_flow_offload_destroy(flow_rule); > } > >
On 06.05.21 14:46, Daniel Jurgens wrote: > BugLink: https://bugs.launchpad.net/bugs/1927374 > > Offload could fail for multiple reasons and a hw refresh bit is > set to try to reoffload it in next sw packet. > But sometimes the hw refresh bit is not set but a refresh could succeed. > Remove the hw refresh bit and do offload refresh if requested. > There won't be a new work entry if a work is already pending > anyway as there is the hw pending bit. > > Fixes: 8b3646d6e0c4 ("net/sched: act_ct: Support refreshing the flow table entries") > Signed-off-by: Roi Dayan <roid@nvidia.com> > Signed-off-by: Daniel Jurgens <danielj@nvidia.com> > --- > include/net/netfilter/nf_flow_table.h | 1 - > net/netfilter/nf_flow_table_core.c | 3 +-- > net/netfilter/nf_flow_table_offload.c | 7 ++++--- > 3 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h > index ff14b39..ba3b58a 100644 > --- a/include/net/netfilter/nf_flow_table.h > +++ b/include/net/netfilter/nf_flow_table.h > @@ -126,7 +126,6 @@ enum nf_flow_flags { > NF_FLOW_HW, > NF_FLOW_HW_DYING, > NF_FLOW_HW_DEAD, > - NF_FLOW_HW_REFRESH, > NF_FLOW_HW_PENDING, > }; > > diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c > index 64df3f8..2405eac 100644 > --- a/net/netfilter/nf_flow_table_core.c > +++ b/net/netfilter/nf_flow_table_core.c > @@ -261,8 +261,7 @@ void flow_offload_refresh(struct nf_flowtable *flow_table, > flow->timeout = nf_flowtable_time_stamp + > nf_flow_offload_timeout(flow_table); > > - if (likely(!nf_flowtable_hw_offload(flow_table) || > - !test_and_clear_bit(NF_FLOW_HW_REFRESH, &flow->flags))) > + if (likely(!nf_flowtable_hw_offload(flow_table))) > return; > > nf_flow_offload_add(flow_table, flow); > diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c > index 28a2983..ad705ca 100644 > --- a/net/netfilter/nf_flow_table_offload.c > +++ b/net/netfilter/nf_flow_table_offload.c > @@ -759,10 +759,11 @@ static void flow_offload_work_add(struct flow_offload_work *offload) > > err = flow_offload_rule_add(offload, flow_rule); > if (err < 0) > - set_bit(NF_FLOW_HW_REFRESH, &offload->flow->flags); > - else > - set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > + goto out; > + > + set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); > > +out: > nf_flow_offload_destroy(flow_rule); > } > > Applied to focal:linux-bluefield/master-next. Thanks. -Stefan
diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h index ff14b39..ba3b58a 100644 --- a/include/net/netfilter/nf_flow_table.h +++ b/include/net/netfilter/nf_flow_table.h @@ -126,7 +126,6 @@ enum nf_flow_flags { NF_FLOW_HW, NF_FLOW_HW_DYING, NF_FLOW_HW_DEAD, - NF_FLOW_HW_REFRESH, NF_FLOW_HW_PENDING, }; diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c index 64df3f8..2405eac 100644 --- a/net/netfilter/nf_flow_table_core.c +++ b/net/netfilter/nf_flow_table_core.c @@ -261,8 +261,7 @@ void flow_offload_refresh(struct nf_flowtable *flow_table, flow->timeout = nf_flowtable_time_stamp + nf_flow_offload_timeout(flow_table); - if (likely(!nf_flowtable_hw_offload(flow_table) || - !test_and_clear_bit(NF_FLOW_HW_REFRESH, &flow->flags))) + if (likely(!nf_flowtable_hw_offload(flow_table))) return; nf_flow_offload_add(flow_table, flow); diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c index 28a2983..ad705ca 100644 --- a/net/netfilter/nf_flow_table_offload.c +++ b/net/netfilter/nf_flow_table_offload.c @@ -759,10 +759,11 @@ static void flow_offload_work_add(struct flow_offload_work *offload) err = flow_offload_rule_add(offload, flow_rule); if (err < 0) - set_bit(NF_FLOW_HW_REFRESH, &offload->flow->flags); - else - set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); + goto out; + + set_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status); +out: nf_flow_offload_destroy(flow_rule); }
BugLink: https://bugs.launchpad.net/bugs/1927374 Offload could fail for multiple reasons and a hw refresh bit is set to try to reoffload it in next sw packet. But sometimes the hw refresh bit is not set but a refresh could succeed. Remove the hw refresh bit and do offload refresh if requested. There won't be a new work entry if a work is already pending anyway as there is the hw pending bit. Fixes: 8b3646d6e0c4 ("net/sched: act_ct: Support refreshing the flow table entries") Signed-off-by: Roi Dayan <roid@nvidia.com> Signed-off-by: Daniel Jurgens <danielj@nvidia.com> --- include/net/netfilter/nf_flow_table.h | 1 - net/netfilter/nf_flow_table_core.c | 3 +-- net/netfilter/nf_flow_table_offload.c | 7 ++++--- 3 files changed, 5 insertions(+), 6 deletions(-)