Re: [PATCH net-next 2/3] act_ct: Support GRE offload

[Paul Blakey <paulb@nvidia.com>]

On Thu, 3 Feb 2022, Toshiaki Makita wrote:

> Support GREv0 without NAT.
> 
> Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com>
> ---
>  net/sched/act_ct.c | 101 +++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 79 insertions(+), 22 deletions(-)
> 
> diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
> index f99247f..a5f47d5 100644
> --- a/net/sched/act_ct.c
> +++ b/net/sched/act_ct.c
> @@ -421,6 +421,19 @@ static void tcf_ct_flow_table_process_conn(struct tcf_ct_flow_table *ct_ft,
>  		break;
>  	case IPPROTO_UDP:
>  		break;
> +#ifdef CONFIG_NF_CT_PROTO_GRE
> +	case IPPROTO_GRE: {
> +		struct nf_conntrack_tuple *tuple;
> +
> +		if (ct->status & IPS_NAT_MASK)
> +			return;
> +		tuple = &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple;
> +		/* No support for GRE v1 */
> +		if (tuple->src.u.gre.key || tuple->dst.u.gre.key)
> +			return;
> +		break;
> +	}
> +#endif
>  	default:
>  		return;
>  	}
> @@ -440,6 +453,8 @@ static void tcf_ct_flow_table_process_conn(struct tcf_ct_flow_table *ct_ft,
>  	struct flow_ports *ports;
>  	unsigned int thoff;
>  	struct iphdr *iph;
> +	size_t hdrsize;
> +	u8 ipproto;
>  
>  	if (!pskb_network_may_pull(skb, sizeof(*iph)))
>  		return false;
> @@ -451,29 +466,49 @@ static void tcf_ct_flow_table_process_conn(struct tcf_ct_flow_table *ct_ft,
>  	    unlikely(thoff != sizeof(struct iphdr)))
>  		return false;
>  
> -	if (iph->protocol != IPPROTO_TCP &&
> -	    iph->protocol != IPPROTO_UDP)
> +	ipproto = iph->protocol;
> +	switch (ipproto) {
> +	case IPPROTO_TCP:
> +		hdrsize = sizeof(struct tcphdr);
> +		break;
> +	case IPPROTO_UDP:
> +		hdrsize = sizeof(*ports);
> +		break;
> +#ifdef CONFIG_NF_CT_PROTO_GRE
> +	case IPPROTO_GRE:
> +		hdrsize = sizeof(struct gre_base_hdr);
> +		break;
> +#endif
> +	default:
>  		return false;
> +	}
>  
>  	if (iph->ttl <= 1)
>  		return false;
>  
> -	if (!pskb_network_may_pull(skb, iph->protocol == IPPROTO_TCP ?
> -					thoff + sizeof(struct tcphdr) :
> -					thoff + sizeof(*ports)))
> +	if (!pskb_network_may_pull(skb, thoff + hdrsize))
>  		return false;
>  
>  	iph = ip_hdr(skb);
> -	if (iph->protocol == IPPROTO_TCP)
> +	if (ipproto == IPPROTO_TCP) {
>  		*tcph = (void *)(skb_network_header(skb) + thoff);
> +	} else if (ipproto == IPPROTO_GRE) {
> +		struct gre_base_hdr *greh;
> +
> +		greh = (struct gre_base_hdr *)(skb_network_header(skb) + thoff);
> +		if ((greh->flags & GRE_VERSION) != GRE_VERSION_0)
> +			return false;
> +	}
>  
> -	ports = (struct flow_ports *)(skb_network_header(skb) + thoff);
>  	tuple->src_v4.s_addr = iph->saddr;
>  	tuple->dst_v4.s_addr = iph->daddr;
> -	tuple->src_port = ports->source;
> -	tuple->dst_port = ports->dest;
> +	if (ipproto == IPPROTO_TCP || ipproto == IPPROTO_UDP) {
> +		ports = (struct flow_ports *)(skb_network_header(skb) + thoff);
> +		tuple->src_port = ports->source;
> +		tuple->dst_port = ports->dest;
> +	}
>  	tuple->l3proto = AF_INET;
> -	tuple->l4proto = iph->protocol;
> +	tuple->l4proto = ipproto;
>  
>  	return true;
>  }
> @@ -486,36 +521,58 @@ static void tcf_ct_flow_table_process_conn(struct tcf_ct_flow_table *ct_ft,
>  	struct flow_ports *ports;
>  	struct ipv6hdr *ip6h;
>  	unsigned int thoff;
> +	size_t hdrsize;
> +	u8 nexthdr;
>  
>  	if (!pskb_network_may_pull(skb, sizeof(*ip6h)))
>  		return false;
>  
>  	ip6h = ipv6_hdr(skb);
> +	thoff = sizeof(*ip6h);
>  
> -	if (ip6h->nexthdr != IPPROTO_TCP &&
> -	    ip6h->nexthdr != IPPROTO_UDP)
> -		return false;
> +	nexthdr = ip6h->nexthdr;
> +	switch (nexthdr) {
> +	case IPPROTO_TCP:
> +		hdrsize = sizeof(struct tcphdr);
> +		break;
> +	case IPPROTO_UDP:
> +		hdrsize = sizeof(*ports);
> +		break;
> +#ifdef CONFIG_NF_CT_PROTO_GRE
> +	case IPPROTO_GRE:
> +		hdrsize = sizeof(struct gre_base_hdr);
> +		break;
> +#endif
> +	default:
> +		return -1;
> +	}
>  
>  	if (ip6h->hop_limit <= 1)
>  		return false;
>  
> -	thoff = sizeof(*ip6h);
> -	if (!pskb_network_may_pull(skb, ip6h->nexthdr == IPPROTO_TCP ?
> -					thoff + sizeof(struct tcphdr) :
> -					thoff + sizeof(*ports)))
> +	if (!pskb_network_may_pull(skb, thoff + hdrsize))
>  		return false;
>  
>  	ip6h = ipv6_hdr(skb);
> -	if (ip6h->nexthdr == IPPROTO_TCP)
> +	if (nexthdr == IPPROTO_TCP) {
>  		*tcph = (void *)(skb_network_header(skb) + thoff);
> +	} else if (nexthdr == IPPROTO_GRE) {
> +		struct gre_base_hdr *greh;
> +
> +		greh = (struct gre_base_hdr *)(skb_network_header(skb) + thoff);
> +		if ((greh->flags & GRE_VERSION) != GRE_VERSION_0)
> +			return false;
> +	}
>  
> -	ports = (struct flow_ports *)(skb_network_header(skb) + thoff);
>  	tuple->src_v6 = ip6h->saddr;
>  	tuple->dst_v6 = ip6h->daddr;
> -	tuple->src_port = ports->source;
> -	tuple->dst_port = ports->dest;
> +	if (nexthdr == IPPROTO_TCP || nexthdr == IPPROTO_UDP) {
> +		ports = (struct flow_ports *)(skb_network_header(skb) + thoff);
> +		tuple->src_port = ports->source;
> +		tuple->dst_port = ports->dest;
> +	}
>  	tuple->l3proto = AF_INET6;
> -	tuple->l4proto = ip6h->nexthdr;
> +	tuple->l4proto = nexthdr;
>  
>  	return true;
>  }
> -- 
> 1.8.3.1
> 
> 

The GRE ifdefs I assume are for the same reason you mentioned in other 
patch, If so, looks good to me.

Acked-by: Paul Blakey <paulb@nvidia.com