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
Hi Pablo,
thx for fixing it. I tested the branch above and it works fine with IPIP tunnel flowtable offloading.
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.
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;@@ -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) {
ack, the above changes are fine to me.
Regards, Lorenzo