[SRU][F:linux-bluefield][PATCH] net/sched: act_ct: Fix flow table lookup failure with no originating ifindex

[Kleber Souza]

On 05.04.22 17:45, Bodong Wang wrote:
> On 4/5/2022 10:38 AM, Bodong Wang wrote:
>> From: Paul Blakey <paulb at nvidia.com>
>>
>> BugLink: https://bugs.launchpad.net/bugs/1967892
>>
>> After cited commit optimizted hw insertion, flow table entries are
>> populated with ifindex information which was intended to only be used
>> for HW offload. This tuple ifindex is hashed in the flow table key, so
>> it must be filled for lookup to be successful. But tuple ifindex is only
>> relevant for the netfilter flowtables (nft), so it's not filled in
>> act_ct flow table lookup, resulting in lookup failure, and no SW
>> offload and no offload teardown for TCP connection FIN/RST packets.
>>
>> To fix this, add new tc ifindex field to tuple, which will
>> only be used for offloading, not for lookup, as it will not be
>> part of the tuple hash.
>>
>> Fixes: 9795ded7f924 ("net/sched: act_ct: Fill offloading tuple iifidx")
>> Signed-off-by: Paul Blakey <paulb at nvidia.com>
>> Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org>
>> (backported from commit db6140e5e35a48405e669353bd54042c1d4c3841)
>> [Oz: Add missing enum ]
>> Signed-off-by: Oz Shlomo <ozsh at nvidia.com>
>> Signed-off-by: Bodong Wang <bodong at nvidia.com>
>> ---
>>    include/net/netfilter/nf_flow_table.h | 16 ++++++++++++++++
>>    net/netfilter/nf_flow_table_offload.c |  6 +++++-
>>    net/sched/act_ct.c                    | 13 +++++++++----
>>    3 files changed, 30 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
>> index b40772f..a0c11bc 100644
>> --- a/include/net/netfilter/nf_flow_table.h
>> +++ b/include/net/netfilter/nf_flow_table.h
>> @@ -88,6 +88,14 @@ enum flow_offload_tuple_dir {
>>    	FLOW_OFFLOAD_DIR_MAX = IP_CT_DIR_MAX
>>    };
>>    
>> +enum flow_offload_xmit_type {
>> +	FLOW_OFFLOAD_XMIT_UNSPEC	= 0,
>> +	FLOW_OFFLOAD_XMIT_NEIGH,
>> +	FLOW_OFFLOAD_XMIT_XFRM,
>> +	FLOW_OFFLOAD_XMIT_DIRECT,
>> +	FLOW_OFFLOAD_XMIT_TC,
>> +};
>> +
>>    struct flow_offload_tuple {
>>    	union {
>>    		struct in_addr		src_v4;
>> @@ -111,6 +119,14 @@ struct flow_offload_tuple {
>>    	u16				mtu;
>>    
>>    	struct dst_entry		*dst_cache;
>> +
>> +	/* fix conflicting upstream commit db6140e5e35a48405e669353bd54042c1d4c3841 */
>> +	u8				xmit_type;
>> +	union {
>> +		struct {
>> +			u32		iifidx;
>> +		} tc;
>> +	};
>>    };
>>    
>>    struct flow_offload_tuple_rhash {
>> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
>> index b6421a8..e41b5c5 100644
>> --- a/net/netfilter/nf_flow_table_offload.c
>> +++ b/net/netfilter/nf_flow_table_offload.c
>> @@ -103,7 +103,11 @@ static int nf_flow_rule_match(struct nf_flow_match *match,
>>    		nf_flow_rule_lwt_match(match, tun_info);
>>    	}
>>    
>> -	key->meta.ingress_ifindex = tuple->iifidx;
>> +	if (tuple->xmit_type == FLOW_OFFLOAD_XMIT_TC)
>> +		key->meta.ingress_ifindex = tuple->tc.iifidx;
>> +	else
>> +		key->meta.ingress_ifindex = tuple->iifidx;
>> +
>>    	mask->meta.ingress_ifindex = 0xffffffff;
>>    
>>    	switch (tuple->l3proto) {
>> diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
>> index a54ba2e..ed310be 100644
>> --- a/net/sched/act_ct.c
>> +++ b/net/sched/act_ct.c
>> @@ -356,6 +356,13 @@ static void tcf_ct_flow_table_put(struct tcf_ct_params *params)
>>    	}
>>    }
>>    
>> +static void tcf_ct_flow_tc_ifidx(struct flow_offload *entry,
>> +				 struct nf_conn_act_ct_ext *act_ct_ext, u8 dir)
>> +{
>> +	entry->tuplehash[dir].tuple.xmit_type = FLOW_OFFLOAD_XMIT_TC;
>> +	entry->tuplehash[dir].tuple.tc.iifidx = act_ct_ext->ifindex[dir];
>> +}
>> +
>>    static void tcf_ct_flow_table_add(struct tcf_ct_flow_table *ct_ft,
>>    				  struct nf_conn *ct,
>>    				  bool tcp)
>> @@ -380,10 +387,8 @@ static void tcf_ct_flow_table_add(struct tcf_ct_flow_table *ct_ft,
>>    
>>    	act_ct_ext = nf_conn_act_ct_ext_find(ct);
>>    	if (act_ct_ext) {
>> -		entry->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.iifidx =
>> -			act_ct_ext->ifindex[IP_CT_DIR_ORIGINAL];
>> -		entry->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.iifidx =
>> -			act_ct_ext->ifindex[IP_CT_DIR_REPLY];
>> +		tcf_ct_flow_tc_ifidx(entry, act_ct_ext, FLOW_OFFLOAD_DIR_ORIGINAL);
>> +		tcf_ct_flow_tc_ifidx(entry, act_ct_ext, FLOW_OFFLOAD_DIR_REPLY);
>>    	}
>>    
>>    	err = flow_offload_add(&ct_ft->nf_ft, entry);
> 
> Tim, can we apply this critical fix asap? We're closing the release but
> current SRU cycle(April) is too late for us.
> 
> 

Hi Bodong,

The commit that this fix claims to address (9795ded7f924 "net/sched: act_ct: Fill offloading tuple iifidx")
is applied and queued to be added to a build in the current SRU cycle (2022.03.21 with release date scheduled
for Apr-18). So if this release date is too late we will need to actually pull that commit and other prerequisites
for a re-spin in the previous SRU cycle (2022.02.21) to be released asap, but the release wouldn't likely happen
more than a week earlier than Apr-18. Is the second option what you need?

Thank you,
Kleber