GRE tunnel iflags/oflags have the wrong endianness

[ThomasFaivre]

Hello!

I am currently using the IPRoute class to create some GRE interfaces, and I noticed that IFLAGS and OFLAGS have the wrong endianness.

It was correctly set until commit 2c207c5. I believe that issue #214 was misleading because commit c78babb was using the correct flag values, and 32 is not one of them.

According to the kernel iflags and oflags are supposed to be 'be16' (it is in 2c207c5 commitlog):

•  nla_put_be16(skb, IFLA_GRE_IFLAGS, tnl_flags_to_gre_flags(p->i_flags)) ||
  

• #define GRE_KEY __cpu_to_be16(0x2000)

GRE_KEY in little-endianness is the flag used in #214 test, and therefore the test is wrong.

I suggest reverting commit 2c207c5, and informing users that GRE flags should now use their correct value in big endian.

Thank you!

[ThomasFaivre]

Actually, it seems that a revert is not enough as ip6gre_iflags seems to have same issue. If you agree with the modification, I can make a patch to fix the issue.

[svinota]

I'll look into the issue tonight. Looks like I «fixed» it wrong, yep.

[svinota]

So far I see that the kernel uses this policy [IFLA_GRE_IFLAGS] = { .type = NLA_U16 }, to encode/decode the flags, and if I switch from uint16 to be16 iflags/oflags` simply stop working.

Could you please provide some meaningful iflags/oflags values to set and check?

[ThomasFaivre]

Hello,

i/okey uses NLA_U32 too and yet we encode/decode it as be32

('IFLA_GRE_IKEY', 'be32'), versus [IFLA_GRE_IKEY] = { .type = NLA_U32 },

Imho, I don't think we should use this array to determine the endianness.

Flags are defined in be16, values are available here:
https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/if_tunnel.h#L25

The netlink library for go seems to agree:
https://github.com/vishvananda/netlink/blob/master/nl/link_linux.go#L192

Value used in #214 is 32 (0x20) which is not in the list, though 0x2000 is.
I understand that it will break current implementation, but shouldn't we use the correct values?

[svinota]

We definitely should. Yep, I checked with correct flags values (e.g. 0x8000) via be16, everything works.

Ok then, let's proceed with that — could you prepare PR? And if there is a possibility to detect somehow the wrong usage of the flags, it would be cool to issue log.warning()

[svinota]

Found a kind of solution, will commit soon, after the tests.

[svinota]

Pls check if it's better now.

[ThomasFaivre]

It works for the flags! Thanks!

But now, it is the key that get the wrong value:

ip.link('add', kind='gre', ifname='gre2', gre_local='10.100.8.23', gre_remote='10.200.10.24', gre_iflags=0x2000, gre_ikey=2)

ip tunnel

gre0: gre/ip remote any local any ttl inherit nopmtudisc
gre2: gre/ip remote 10.200.10.24 local 10.100.8.23 ttl inherit ikey 33554432

You changed it in uint32 when it should have stayed in be32.

[svinota]

Oops, too many errors, too much in a hurry.

Pls excuse this :) Fixed i/okey, touched by mistake.

[svinota]

Already noticed, but didn't manage to push in time