On 03/09/2018 03:59 PM, William Tu wrote:
On Thu, Feb 8, 2018 at 10:42 AM, Eric Garver <[email protected]> wrote:
On Thu, Feb 08, 2018 at 04:40:38PM +0100, Eelco Chaudron wrote:
While OVS userspace datapath (OVS-DPDK) supports GREv6, it does not
inter-operate with a native Linux ip6gretap tunnel. This is because
the Linux driver uses IPv6 optional headers for the Tunnel
Encapsulation Limit (RFC 2473, section 6.6).
Maybe worth noting that the kernel started adding these headers in
4.12. Specifically, it was introduced by 89a23c8b528b ("ip6_tunnel: Fix
missing tunnel encapsulation limit option")
OVS userspace simply does not parse these IPv6 extension headers
inside netdev_tnl_ip_extract_tnl_md(), as such popping the tunnel
leaves extra bytes resulting in a mangled decapsulated frame.
The change below will parse the IPv6 "next header" chain and return
the offset to the upper layer protocol.
v1->v2
- Remove netdev_tnl_ip6_get_upperlayer_offset() and reused existing
parse_ipv6_ext_hdrs() function.
I also hit this issue recent. I tested this patch and it works OK.
However, do you think its simpler if we do below:
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -146,8 +146,8 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet
*packet, struct flow_tnl *tnl,
tnl->ip_tos = ntohl(tc_flow) >> 20;
tnl->ip_ttl = ip6->ip6_hlim;
- *hlen += IPV6_HEADER_LEN;
+ *hlen += packet->l4_ofs - packet->l3_ofs;
Since parse_ipv6_ext_hdrs() already called at packet parsing stage,
the l4_ofs has the extension header's length.
Are you sure this path is taken for all packets? Quickly looking at the
code I see this path only with connection tracking actions. Or did I
miss something?
Signed-off-by: Eelco Chaudron <[email protected]>
---
lib/netdev-native-tnl.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index fb5eab033..4f520a0f9 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -115,6 +115,10 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet,
struct flow_tnl *tnl,
*hlen += IP_HEADER_LEN;
} else if (IP_VER(ip->ip_ihl_ver) == 6) {
+ const void *ip6_data;
+ size_t ip6_size;
+ uint8_t nw_proto;
+ uint8_t nw_frag;
ovs_be32 tc_flow = get_16aligned_be32(&ip6->ip6_flow);
memcpy(tnl->ipv6_src.s6_addr, ip6->ip6_src.be16, sizeof ip6->ip6_src);
@@ -125,6 +129,20 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet,
struct flow_tnl *tnl,
*hlen += IPV6_HEADER_LEN;
+ ip6_data = ip6 + 1;
+ ip6_size = l3_size - IPV6_HEADER_LEN;
+ nw_proto = ip6->ip6_nxt;
+ nw_frag = 0;
+
+ if (!parse_ipv6_ext_hdrs(&ip6_data, &ip6_size, &nw_proto, &nw_frag) ||
+ nw_frag != 0) {
+ VLOG_WARN_RL(&err_rl,
+ "ipv6 packet has unsupported extension headers");
+ return NULL;
+ }
+
+ *hlen += l3_size - IPV6_HEADER_LEN - ip6_size;
+
} else {
VLOG_WARN_RL(&err_rl, "ipv4 packet has invalid version (%d)",
IP_VER(ip->ip_ihl_ver));
--
2.14.3
Thanks Eelco. I tested this by applying it on top of a non-upstream gre6
test case I had previously written [0]. If this gets applied I can post
that testcase to the list.
Tested-by: Eric Garver <[email protected]>
[0]
https://github.com/erig0/ovs/commits/ff2e39278b66b3fe937dba63b32e341098901c50
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
