ACK/Cmnt: [SRU][F:linux-bluefield][PATCH] UBUNTU: SAUCE: netfilter: flowtable: Do flow offload refresh when requested

[Daniel Jurgens]

> -----Original Message-----
> From: Tim Gardner <tim.gardner at canonical.com>
> Sent: Friday, May 7, 2021 10:12 AM
> 
> Acked-by: Tim Gardner <tim.gardner at 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 at nvidia.com>
> > Signed-off-by: Daniel Jurgens <danielj at 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