[ovs-dev] [PATCH v2 1/1] userspace: Add IPv6 extension header parsing for tunnels

[William Tu]

On Thu, Feb 8, 2018 at 10:42 AM, Eric Garver <e at erig.me> 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.

Regards,
William

>> Signed-off-by: Eelco Chaudron <echaudro at redhat.com>
>> ---
>>  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 <e at erig.me>
>
> [0] https://github.com/erig0/ovs/commits/ff2e39278b66b3fe937dba63b32e341098901c50
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev