Date:   Thu, 7 Jul 2022 15:59:10 +0200
From:   Matthias May <matthias.may@...termo.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     <netdev@...r.kernel.org>, <davem@...emloft.net>,
        <yoshfuji@...ux-ipv6.org>, <dsahern@...nel.org>,
        <edumazet@...gle.com>, <pabeni@...hat.com>
Subject: Re: [PATCH net] ip_tunnel: allow to inherit from VLAN encapsulated IP
 frames

On 7/6/22 22:17, Jakub Kicinski wrote:
> On Wed, 6 Jul 2022 09:07:36 +0200 Matthias May wrote:
>>>> The current code allows to inherit the TOS, TTL, DF from the payload
>>>> when skb->protocol is ETH_P_IP or ETH_P_IPV6.
>>>> However when the payload is VLAN encapsulated (e.g because the tunnel
>>>> is of type GRETAP), then this inheriting does not work, because the
>>>> visible skb->protocol is of type ETH_P_8021Q.
>>>>
>>>> Add a check on ETH_P_8021Q and subsequently check the payload protocol.
>>>
>>> Do we need to check for 8021AD as well?
>>
>> Yeah that would make sense.
>> I can add the check for ETH_P_8021AD in v2.
>> Will have to find some hardware that is AD capable to test.
> 
> Why HW, you should be able to test with two Linux endpoints, no?
> 
>>>> Signed-off-by: Matthias May <matthias.may@...termo.com>
>>>> ---
>>>>    net/ipv4/ip_tunnel.c | 21 +++++++++++++--------
>>>
>>> Does ipv6 need the same treatment?
>>
>> I don't think i changed anything regarding the behaviour for ipv6
>> by allowing to skip from the outer protocol to the payload protocol.
> 
> Sorry, to be clear what I meant - we try to enforce feature parity for
> IPv6 these days in Linux. So I was asking if ipv6 needs changes to be
> able to deal with VLANs. I think you got that but just in case.
> 
>> The previous code already
>> * got the TOS via ipv6_get_dsfield,
>> * the TTL was derived from the hop_limit,
>> * and DF does not exist for ipv6 so it doesn't check for ETH_P_IPV6.
> 
> Purely by looking at the code I thought that VLAN-enabled GRETAP frames
> would fall into ip6gre_xmit_other() which passes dsfield=0 into
> __gre6_xmit(). key->tos only overrides the field for "external" tunnels,
> not normal tunnels with a dedicated netdev per tunnel.
> 
> A selftest to check both ipv4 and ipv6 would be the ultimate win there.

I wrote the small test-script below.
Without looking at the code, to me it seems like setting the TOS when the outer
transport is IPv6 does not work.
This is on 5.19-rc5

When i create the tunnel with the script below:
outer=6
inner=4
and hardcode the TOS for the tunnel to 0xa0
ip link add name tep0 type ip6gretap local fdd1:ced0:5d88:3fce::1 remote fdd1:ced0:5d88:3fce::2 tos 0xa0
then i would expect the class of the resulting GRE frames to be 0xa0
however a ping through the tunnel shows me

$ tcpdump -i veth0 -n -v -e
15:24:01.395236 a2:f8:bb:1f:d3:b1 > 06:94:da:29:e0:0b, ethertype IPv6 (0x86dd), length 168: (flowlabel 0xb4c84, hlim 64, 
next-header unknown (60) payload length: 114) fdd1:ced0:5d88:3fce::1 > fdd1:ced0:5d88:3fce::2: DSTOPT (opt_type 0x04: 
len=1)(padn) GREv0, Flags [none], proto TEB (0x6558), length 106
	b2:d8:59:9d:ce:ca > b6:9e:a3:aa:f7:ae, ethertype 802.1Q (0x8100), length 102: vlan 99, p 0, ethertype IPv4 (0x0800), 
(tos 0x0, ttl 64, id 7056, offset 0, flags [DF], proto ICMP (1), length 84)
     198.19.0.1 > 198.19.0.2: ICMP echo request, id 59101, seq 5, length 64

whereas when i do a ping directly (not through the tunnel) with
ping fdd1:ced0:5d88:3fce::2 -Q 0xa0
then i see the class correctly

$ tcpdump -i veth0 -n -v -e
15:25:00.755188 a2:f8:bb:1f:d3:b1 > 06:94:da:29:e0:0b, ethertype IPv6 (0x86dd), length 118: (class 0xa0, flowlabel 
0x5f8c7, hlim 64, next-header ICMPv6 (58) payload length: 64) fdd1:ced0:5d88:3fce::1 > fdd1:ced0:5d88:3fce::2: [icmp6 
sum ok] ICMP6, echo request, id 46866, seq 2


Has setting the TOS for ip6gretap ever worked?
How should i go forward with this?
You state that your require feature parity between v4 and v6, but i am not sure i can provide that when
the underlying building blocks are missing.

BR
Matthias

---
#!/bin/sh

setup() {
	local outer inner test_namespace nsexec
	outer="$1"
	inner="$2"
	test_namespace="testing"
	nsexec="ip netns exec $test_namespace"

	# Create 'testing' netns, veth pair and connect main ns with testing ns
	ip netns add $test_namespace
	ip link add type veth
	ip link set veth1 netns testing
	ip link set veth0 up
	$nsexec ip link set veth1 up
	ip addr flush dev veth0
	$nsexec ip addr flush dev veth1

	# Create (ip6)gretap and assign outer IPv4/IPv6 addresses
	if [ "$outer" = "4" ]; then
		ip addr add 198.18.0.1/24 dev veth0
		$nsexec ip addr add 198.18.0.2/24 dev veth1
		ip link add name tep0 type gretap local 198.18.0.1 remote 198.18.0.2 tos inherit
		$nsexec ip link add name tep1 type gretap local 198.18.0.2 remote 198.18.0.1 tos inherit
	elif [ "$outer" = "6" ]; then
		ip addr add fdd1:ced0:5d88:3fce::1/64 dev veth0
		$nsexec ip addr add fdd1:ced0:5d88:3fce::2/64 dev veth1
		ip link add name tep0 type ip6gretap local fdd1:ced0:5d88:3fce::1 remote fdd1:ced0:5d88:3fce::2 tos inherit
		$nsexec ip link add name tep1 type ip6gretap local fdd1:ced0:5d88:3fce::2 remote fdd1:ced0:5d88:3fce::1 tos inherit
	else
		return -1
	fi

	# Bring (IP6)GRETAP link up and create VLAN on top
	ip link set tep0 up
	$nsexec ip link set tep1 up
	ip addr flush dev tep0
	$nsexec $nsexec ip addr flush dev tep1
	ip link add link tep0 name vlan99-0 type vlan id 99
	$nsexec ip link add link tep1 name vlan99-1 type vlan id 99
	ip link set vlan99-0 up
	$nsexec ip link set vlan99-1 up
	ip addr flush dev vlan99-0
	$nsexec ip addr flush dev vlan99-1

	# Assign inner IPv4/IPv6 addresses
	if [ "$inner" = "4" ]; then
		ip addr add 198.19.0.1/24 brd + dev vlan99-0
		$nsexec ip addr add 198.19.0.2/24 brd + dev vlan99-1
	elif [ "$inner" = "6" ]; then
		ip addr add fdd4:96cf:4eae:443b::1/64 dev vlan99-0
		$nsexec ip addr add fdd4:96cf:4eae:443b::2/64 dev vlan99-1
	else
		return -1
	fi
}

cleanup() {
	ip link del veth0
	ip netns del testing
	ip link del tep0
}
if [ "$1" != "start" ] && [ "$1" != "stop" ]; then
	echo "invalid first argument, valid is 'start' or 'stop'"
	exit 1
fi
if [ "$1" = "start" ] && [ "$2" != "4" ] && [ "$2" != "6" ]; then
	echo "invalid second argument (outer protocol), valid is '4' or '6'"
	exit 1
fi
if [ "$1" = "start" ] && [ "$3" != "4" ] && [ "$3" != "6" ]; then
	echo "invalid third argument (inner protocol), valid is '4' or '6'"
	exit 1
fi
if [ "$1" = "start" ]; then
	setup "$2" "$3"
elif [ "$1" = "stop" ]; then
	cleanup
fi


Download attachment "OpenPGP_0xDF76B604533C0DBE.asc" of type "application/pgp-keys" (670 bytes)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (237 bytes)

Powered by blists - more mailing lists