Re: [PATCH net-next v2] net: geneve: accept every ethertype

On Sun, Mar 19, 2023 at 11:09:54PM +0100, Josef Miegl wrote:

The Geneve encapsulation, as defined in RFC 8926, has a Protocol Type
field, which states the Ethertype of the payload appearing after the
Geneve header.

Commit 435fe1c0c1f7 ("net: geneve: support IPv4/IPv6 as inner protocol")
introduced a new IFLA_GENEVE_INNER_PROTO_INHERIT flag that allowed the
use of other Ethertypes than Ethernet. However, it did not get rid of a
restriction that prohibits receiving payloads other than Ethernet,
instead the commit white-listed additional Ethertypes, IPv4 and IPv6.

This patch removes this restriction, making it possible to receive any
Ethertype as a payload, if the IFLA_GENEVE_INNER_PROTO_INHERIT flag is
set.

The restriction was set in place back in commit 0b5e8b8eeae4
("net: Add Geneve tunneling protocol driver"), which implemented a
protocol layer driver for Geneve to be used with Open vSwitch. The
relevant discussion about introducing the Ethertype white-list can be
found here:
https://lore.kernel.org/netdev/CAEP_g=_1q3ACX5NTHxLDnysL+dTMUVzdLpgw1apLKEdDSWPztw@mail.gmail.com/ (local)

<quote>

quoted

quoted

+       if (unlikely(geneveh->proto_type != htons(ETH_P_TEB)))

Why? I thought the point of geneve carrying protocol field was to
allow protocols other than Ethernet... is this temporary maybe?

Yes, it is temporary. Currently OVS only handles Ethernet packets but
this restriction can be lifted once we have a consumer that is capable
of handling other protocols.
</quote>

This white-list was then ported to a generic Geneve netdevice in commit
371bd1061d29 ("geneve: Consolidate Geneve functionality in single
module."). Preserving the Ethertype white-list at this point made sense,
as the Geneve device could send out only Ethernet payloads anyways.

I'm not sure if it ought to be fixed, but checkpatch complains that:

:379: ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 371bd1061d29 ("geneve: Consolidate Geneve functionality in single module.")'
This white-list was then ported to a generic Geneve netdevice in commit

However, now that the Geneve netdevice supports encapsulating other
payloads with IFLA_GENEVE_INNER_PROTO_INHERIT and we have a consumer
capable of other protocols, it seems appropriate to lift the restriction
and allow any Geneve payload to be received.

Signed-off-by: Josef Miegl <redacted>

Above nit not withstanding, and in keeping with the discussion of v1,
I am happy with this patch.

Reviewed-by: Simon Horman <redacted>

...