diff mbox series

[net-next,1/2] net/sched: act_ct: Fix ipv6 lookup of offloaded connections

Message ID 1583322579-11558-2-git-send-email-paulb@mellanox.com
State Accepted
Delegated to: David Miller
Headers show
Series Fixes for tc act_ct software offload of established flows (diff v4->v6) | expand

Commit Message

Paul Blakey March 4, 2020, 11:49 a.m. UTC 
When checking the protocol number tcf_ct_flow_table_lookup() handles
the flow as if it's always ipv4, while it can be ipv6.

Instead, refactor the code to fetch the tcp header, if available,
in the relevant family (ipv4/ipv6) filler function, and do the
check on the returned tcp header.

Fixes: 46475bb20f4b ("net/sched: act_ct: Software offload of established flows")
Signed-off-by: Paul Blakey <paulb@mellanox.com>
---
 net/sched/act_ct.c | 60 +++++++++++++++++++++++-------------------------------
 1 file changed, 26 insertions(+), 34 deletions(-)

Comments

Marcelo Ricardo Leitner March 4, 2020, 1:17 p.m. UTC | #1 
On Wed, Mar 04, 2020 at 01:49:38PM +0200, Paul Blakey wrote:
> When checking the protocol number tcf_ct_flow_table_lookup() handles
> the flow as if it's always ipv4, while it can be ipv6.
> 
> Instead, refactor the code to fetch the tcp header, if available,
> in the relevant family (ipv4/ipv6) filler function, and do the
> check on the returned tcp header.
> 
> Fixes: 46475bb20f4b ("net/sched: act_ct: Software offload of established flows")
> Signed-off-by: Paul Blakey <paulb@mellanox.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Jiri Pirko March 4, 2020, 2:06 p.m. UTC | #2 
Wed, Mar 04, 2020 at 12:49:38PM CET, paulb@mellanox.com wrote:
>When checking the protocol number tcf_ct_flow_table_lookup() handles
>the flow as if it's always ipv4, while it can be ipv6.
>
>Instead, refactor the code to fetch the tcp header, if available,
>in the relevant family (ipv4/ipv6) filler function, and do the
>check on the returned tcp header.
>
>Fixes: 46475bb20f4b ("net/sched: act_ct: Software offload of established flows")
>Signed-off-by: Paul Blakey <paulb@mellanox.com>

Reviewed-by: Jiri Pirko <jiri@mellanox.com>
diff mbox series

Patch

diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index a2d5582..f434db7 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -188,7 +188,8 @@  static void tcf_ct_flow_table_process_conn(struct tcf_ct_flow_table *ct_ft,
 
 static bool
 tcf_ct_flow_table_fill_tuple_ipv4(struct sk_buff *skb,
-				  struct flow_offload_tuple *tuple)
+				  struct flow_offload_tuple *tuple,
+				  struct tcphdr **tcph)
 {
 	struct flow_ports *ports;
 	unsigned int thoff;
@@ -211,11 +212,16 @@  static void tcf_ct_flow_table_process_conn(struct tcf_ct_flow_table *ct_ft,
 	if (iph->ttl <= 1)
 		return false;
 
-	if (!pskb_may_pull(skb, thoff + sizeof(*ports)))
+	if (!pskb_may_pull(skb, iph->protocol == IPPROTO_TCP ?
+			   thoff + sizeof(struct tcphdr) :
+			   thoff + sizeof(*ports)))
 		return false;
 
-	ports = (struct flow_ports *)(skb_network_header(skb) + thoff);
+	iph = ip_hdr(skb);
+	if (iph->protocol == IPPROTO_TCP)
+		*tcph = (void *)(skb_network_header(skb) + thoff);
 
+	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;
@@ -228,7 +234,8 @@  static void tcf_ct_flow_table_process_conn(struct tcf_ct_flow_table *ct_ft,
 
 static bool
 tcf_ct_flow_table_fill_tuple_ipv6(struct sk_buff *skb,
-				  struct flow_offload_tuple *tuple)
+				  struct flow_offload_tuple *tuple,
+				  struct tcphdr **tcph)
 {
 	struct flow_ports *ports;
 	struct ipv6hdr *ip6h;
@@ -247,11 +254,16 @@  static void tcf_ct_flow_table_process_conn(struct tcf_ct_flow_table *ct_ft,
 		return false;
 
 	thoff = sizeof(*ip6h);
-	if (!pskb_may_pull(skb, thoff + sizeof(*ports)))
+	if (!pskb_may_pull(skb, ip6h->nexthdr == IPPROTO_TCP ?
+			   thoff + sizeof(struct tcphdr) :
+			   thoff + sizeof(*ports)))
 		return false;
 
-	ports = (struct flow_ports *)(skb_network_header(skb) + thoff);
+	ip6h = ipv6_hdr(skb);
+	if (ip6h->nexthdr == IPPROTO_TCP)
+		*tcph = (void *)(skb_network_header(skb) + thoff);
 
+	ports = (struct flow_ports *)(skb_network_header(skb) + thoff);
 	tuple->src_v6 = ip6h->saddr;
 	tuple->dst_v6 = ip6h->daddr;
 	tuple->src_port = ports->source;
@@ -262,24 +274,6 @@  static void tcf_ct_flow_table_process_conn(struct tcf_ct_flow_table *ct_ft,
 	return true;
 }
 
-static bool tcf_ct_flow_table_check_tcp(struct flow_offload *flow,
-					struct sk_buff *skb,
-					unsigned int thoff)
-{
-	struct tcphdr *tcph;
-
-	if (!pskb_may_pull(skb, thoff + sizeof(*tcph)))
-		return false;
-
-	tcph = (void *)(skb_network_header(skb) + thoff);
-	if (unlikely(tcph->fin || tcph->rst)) {
-		flow_offload_teardown(flow);
-		return false;
-	}
-
-	return true;
-}
-
 static bool tcf_ct_flow_table_lookup(struct tcf_ct_params *p,
 				     struct sk_buff *skb,
 				     u8 family)
@@ -288,10 +282,9 @@  static bool tcf_ct_flow_table_lookup(struct tcf_ct_params *p,
 	struct flow_offload_tuple_rhash *tuplehash;
 	struct flow_offload_tuple tuple = {};
 	enum ip_conntrack_info ctinfo;
+	struct tcphdr *tcph = NULL;
 	struct flow_offload *flow;
 	struct nf_conn *ct;
-	unsigned int thoff;
-	int ip_proto;
 	u8 dir;
 
 	/* Previously seen or loopback */
@@ -301,11 +294,11 @@  static bool tcf_ct_flow_table_lookup(struct tcf_ct_params *p,
 
 	switch (family) {
 	case NFPROTO_IPV4:
-		if (!tcf_ct_flow_table_fill_tuple_ipv4(skb, &tuple))
+		if (!tcf_ct_flow_table_fill_tuple_ipv4(skb, &tuple, &tcph))
 			return false;
 		break;
 	case NFPROTO_IPV6:
-		if (!tcf_ct_flow_table_fill_tuple_ipv6(skb, &tuple))
+		if (!tcf_ct_flow_table_fill_tuple_ipv6(skb, &tuple, &tcph))
 			return false;
 		break;
 	default:
@@ -320,15 +313,14 @@  static bool tcf_ct_flow_table_lookup(struct tcf_ct_params *p,
 	flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
 	ct = flow->ct;
 
+	if (tcph && (unlikely(tcph->fin || tcph->rst))) {
+		flow_offload_teardown(flow);
+		return false;
+	}
+
 	ctinfo = dir == FLOW_OFFLOAD_DIR_ORIGINAL ? IP_CT_ESTABLISHED :
 						    IP_CT_ESTABLISHED_REPLY;
 
-	thoff = ip_hdr(skb)->ihl * 4;
-	ip_proto = ip_hdr(skb)->protocol;
-	if (ip_proto == IPPROTO_TCP &&
-	    !tcf_ct_flow_table_check_tcp(flow, skb, thoff))
-		return false;
-
 	nf_conntrack_get(&ct->ct_general);
 	nf_ct_set(skb, ct, ctinfo);