2022-07-24 01:24:23

by Matthias May

[permalink] [raw]
Subject: [PATCH 0/2 net-next] geneve: fix TOS inheriting

Currently when the TOS of an encapsulated frame is inherited,
the 6 DSCP bits are cut down to 3 original TOS bits.
Compare to other L2 tunneling protocols (gretap, vxlan) this
is unexpected.
IPv4 and IPv6 have both this behaviour but for different reasons.

For IPv4 the bits are lost in the routing table lookup.
The patch copies the full tos out before the lookup and uses the copy.

For IPv6 the RT_TOS macro cuts off the 3 bits.
I'm not really familiar with the IPv6 code, but to me it seems as
if this part of the code only uses the TOS for the flowlabel.
Is there any reason why the flowlabel should be restricted to these
3 bits? The patch simply removes the usage of this macro, but i don't
know if there was a specific intention behind that. I can't find any
immediate breakage, but then again my IPv6 testing is fairly limited.

Matthias May (2):
geneve: fix TOS inheriting for ipv4
geneve: fix TOS inheriting for ipv6

drivers/net/geneve.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

--
2.35.1


2022-07-25 17:45:11

by Guillaume Nault

[permalink] [raw]
Subject: Re: [PATCH 0/2 net-next] geneve: fix TOS inheriting

On Sun, Jul 24, 2022 at 02:37:39AM +0200, Matthias May wrote:
> Is there any reason why the flowlabel should be restricted to these
> 3 bits?

It's a bug, likely a copy/paste mistake.

> The patch simply removes the usage of this macro, but i don't
> know if there was a specific intention behind that.

RT_TOS() is an old macro used to interprete IPv4 TOS as described in
the obsolete RFC 1349. It's conceptually wrong to use it even in IPv4
code, although, given the current state of the code, most of the
existing calls have no consequence.

But using RT_TOS() in IPv6 code is always a bug: IPv6 never had a "TOS"
field to be interpreted the RFC 1349 way. There's no historical
compatibility to worry about.