Messages in this thread | | | From | Eyal Birger <> | Date | Mon, 27 Feb 2023 12:05:51 +0200 | Subject | Re: [PATCH v2 0/1] net: geneve: accept every ethertype |
| |
On Mon, Feb 27, 2023 at 11:57 AM Josef Miegl <josef@miegl.cz> wrote: > > February 27, 2023 10:30 AM, "Eyal Birger" <eyal.birger@gmail.com> wrote: > > > Hi, > > > > On Mon, Feb 27, 2023 at 10:19 AM Josef Miegl <josef@miegl.cz> 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, for a reason not known > >> to me, it imposed a restriction that prohibits receiving payloads other > >> than IPv4, IPv6 and Ethernet. > > > > FWIW I added support for IPv4/IPv6 because these are the use cases I had > > and could validate. I don't know what problems could arise from supporting > > all possible ethertypes and can't test that. > > Yeah, I am hoping someone knowledgeable will tell whether this is a good > or bad idea. However I think that if any problem could arise, this is not > the place to artificially restrict payload types and potentional safeguarding > should be done somewhere down the packet chain. > > I can't imagine adding a payload Ethertype every time someone needs a > specific use-case would be a good idea.
I guess it's just a matter of practicality - which decision imposes more burden on future maintenance.
> > >> 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. > > > > This seems like an addition not a bugfix so personally seems like it should > > be targeting net-next (which is currently closed afaik). > > One could say the receive function should have behaved like that, the > transmit function already encapsulates every possible Ethertype and > IFLA_GENEVE_INNER_PROTO_INHERIT doesn't sound like it should be limited to > IPv4 and IPv6.
Indeed the flag is intentionally generic to allow for future extensions without having to rename. But both in the commit message, and in the iproute2 man page I noted support for IPv4/IPv6.
> > If no further modifications down the packet chain are required, I'd say it's > 50/50. However I haven't contributed to the Linux kernel ever before, so I > really have no clue as to how things go. > > > Eyal. > > > >> This is especially useful if one wants to encapsulate MPLS, because with > >> this patch the control-plane traffic (IP, LLC) and the data-plane > >> traffic (MPLS) can be encapsulated without an Ethernet frame, making > >> lightweight overlay networks a possibility. > >> > >> Changes in v2: > >> - added a cover letter > >> - lines no longer exceed 80 columns > >> > >> Josef Miegl (1): > >> net: geneve: accept every ethertype > >> > >> drivers/net/geneve.c | 15 ++++----------- > >> 1 file changed, 4 insertions(+), 11 deletions(-) > >> > >> -- > >> 2.37.1
| |