Re: [PATCH nf-next v9 2/3] net: netfilter: Add IPIP flowtable tx sw acceleration

[Pablo Neira Ayuso <pablo@netfilter.org>]

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

Hi Lorenzo,

I found one more issue: caching the ip6 daddr does not work because
skb_cow_head() can reallocate the skb header, invalidating all
pointers.

I went back to use the other_tuple, it is safer, new branch:

        flowtable-consolidate-xmit+ipip3

Hopefully, this is the last iteration for this series.

I am attaching a diff that compares flowtable-consolidate-xmit+ipip2
vs. flowtable-consolidate-xmit+ipip3 branches.

I also made a few cosmetic edits.

[-- Attachment #2: diff-v2-v3.patch --]
[-- Type: text/x-diff, Size: 3099 bytes --]

diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index ee81ee3a5110..e128b0fe9a7b 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -512,13 +512,14 @@ static int nf_flow_pppoe_push(struct sk_buff *skb, u16 id)
 }
 
 static int nf_flow_tunnel_ipip_push(struct net *net, struct sk_buff *skb,
-				    struct flow_offload_tuple *tuple)
+				    struct flow_offload_tuple *tuple,
+				    __be32 *ip_daddr)
 {
 	struct iphdr *iph = (struct iphdr *)skb_network_header(skb);
 	struct rtable *rt = dst_rtable(tuple->dst_cache);
-	__be16	frag_off = iph->frag_off;
-	u32 headroom = sizeof(*iph);
 	u8 tos = iph->tos, ttl = iph->ttl;
+	__be16 frag_off = iph->frag_off;
+	u32 headroom = sizeof(*iph);
 	int err;
 
 	err = iptunnel_handle_offloads(skb, SKB_GSO_IPXIP4);
@@ -551,14 +552,17 @@ static int nf_flow_tunnel_ipip_push(struct net *net, struct sk_buff *skb,
 	__ip_select_ident(net, iph, skb_shinfo(skb)->gso_segs ?: 1);
 	ip_send_check(iph);
 
+	*ip_daddr = tuple->tun.src_v4.s_addr;
+
 	return 0;
 }
 
 static int nf_flow_tunnel_v4_push(struct net *net, struct sk_buff *skb,
-				  struct flow_offload_tuple *tuple)
+				  struct flow_offload_tuple *tuple,
+				  __be32 *ip_daddr)
 {
 	if (tuple->tun_num)
-		return nf_flow_tunnel_ipip_push(net, skb, tuple);
+		return nf_flow_tunnel_ipip_push(net, skb, tuple, ip_daddr);
 
 	return 0;
 }
@@ -572,7 +576,8 @@ static int nf_flow_encap_push(struct sk_buff *skb,
 		switch (tuple->encap[i].proto) {
 		case htons(ETH_P_8021Q):
 		case htons(ETH_P_8021AD):
-			if (skb_vlan_push(skb, tuple->encap[i].proto, tuple->encap[i].id) < 0)
+			if (skb_vlan_push(skb, tuple->encap[i].proto,
+					  tuple->encap[i].id) < 0)
 				return -1;
 			break;
 		case htons(ETH_P_PPP_SES):
@@ -624,12 +629,11 @@ nf_flow_offload_ip_hook(void *priv, struct sk_buff *skb,
 	dir = tuplehash->tuple.dir;
 	flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
 	other_tuple = &flow->tuplehash[!dir].tuple;
+	ip_daddr = other_tuple->src_v4.s_addr;
 
-	if (nf_flow_tunnel_v4_push(state->net, skb, other_tuple) < 0)
+	if (nf_flow_tunnel_v4_push(state->net, skb, other_tuple, &ip_daddr) < 0)
 		return NF_DROP;
 
-	ip_daddr = ip_hdr(skb)->daddr;
-
 	if (nf_flow_encap_push(skb, other_tuple) < 0)
 		return NF_DROP;
 
@@ -906,6 +910,7 @@ nf_flow_offload_ipv6_hook(void *priv, struct sk_buff *skb,
 {
 	struct flow_offload_tuple_rhash *tuplehash;
 	struct nf_flowtable *flow_table = priv;
+	struct flow_offload_tuple *other_tuple;
 	enum flow_offload_tuple_dir dir;
 	struct nf_flowtable_ctx ctx = {
 		.in	= state->in,
@@ -937,9 +942,10 @@ nf_flow_offload_ipv6_hook(void *priv, struct sk_buff *skb,
 
 	dir = tuplehash->tuple.dir;
 	flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
-	ip6_daddr = &ipv6_hdr(skb)->daddr;
+	other_tuple = &flow->tuplehash[!dir].tuple;
+	ip6_daddr = &other_tuple->src_v6;
 
-	if (nf_flow_encap_push(skb, &flow->tuplehash[!dir].tuple) < 0)
+	if (nf_flow_encap_push(skb, other_tuple) < 0)
 		return NF_DROP;
 
 	switch (tuplehash->tuple.xmit_type) {