MIME-Version: 1.0
References: <20230227074104.42153-1-josef@miegl.cz> <CAHsH6GtArNCyA3UAJbSYYD86fb2QxskbSoNQo2RVHQzKC643zg@mail.gmail.com>
 <79dee14b9b96d5916a8652456b78c7a5@miegl.cz>
In-Reply-To: <79dee14b9b96d5916a8652456b78c7a5@miegl.cz>
From:   Eyal Birger <eyal.birger@gmail.com>
Date:   Mon, 27 Feb 2023 12:05:51 +0200
Message-ID: <CAHsH6GuHiRDgY+_Epu=ejTAWONuXgzHk326SUuAeRp6pGaTEpA@mail.gmail.com>
Subject: Re: [PATCH v2 0/1] net: geneve: accept every ethertype
To:     Josef Miegl <josef@miegl.cz>
Cc:     "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Feb 27, 2023 at 11:57=E2=80=AFAM 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 othe=
r
> >> than IPv4, IPv6 and Ethernet.
> >
> > FWIW I added support for IPv4/IPv6 because these are the use cases I ha=
d
> > and could validate. I don't know what problems could arise from support=
ing
> > 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 safeguar=
ding
> 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 sh=
ould
> > 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 t=
o
> 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 iprout=
e2
man page I noted support for IPv4/IPv6.

>
> If no further modifications down the packet chain are required, I'd say i=
t'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 wi=
th
> >> 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