Messages in this thread | | | Date | Mon, 20 Mar 2023 17:03:09 +0100 | From | Simon Horman <> | Subject | 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/ > > <quote> > >> + 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 <josef@miegl.cz>
Above nit not withstanding, and in keeping with the discussion of v1, I am happy with this patch.
Reviewed-by: Simon Horman <simon.horman@corigine.com>
...
| |