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

[Tim Gardner]

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 :)

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