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

[Tim Gardner]

Acked-by: Tim Gardner <tim.gardner at canonical.com>

Your backport note neglected to mention that you also changed the bit 
width of xmit_type from 3 to 8.

On 4/5/22 09:38, 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 Gardner
Canonical, Inc