* [PATCH net-next v2] net: geneve: accept every ethertype
@ 2023-03-19 22:09 Josef Miegl
2023-03-20 16:03 ` Simon Horman
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Josef Miegl @ 2023-03-19 22:09 UTC (permalink / raw)
Cc: Eyal Birger, Pravin B Shelar, Josef Miegl, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
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.
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>
---
drivers/net/geneve.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 89ff7f8e8c7e..32684e94eb4f 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -365,13 +365,6 @@ static int geneve_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
if (unlikely(geneveh->ver != GENEVE_VER))
goto drop;
- inner_proto = geneveh->proto_type;
-
- if (unlikely((inner_proto != htons(ETH_P_TEB) &&
- inner_proto != htons(ETH_P_IP) &&
- inner_proto != htons(ETH_P_IPV6))))
- goto drop;
-
gs = rcu_dereference_sk_user_data(sk);
if (!gs)
goto drop;
@@ -380,6 +373,8 @@ static int geneve_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
if (!geneve)
goto drop;
+ inner_proto = geneveh->proto_type;
+
if (unlikely((!geneve->cfg.inner_proto_inherit &&
inner_proto != htons(ETH_P_TEB)))) {
geneve->dev->stats.rx_dropped++;
--
2.37.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2] net: geneve: accept every ethertype
2023-03-19 22:09 [PATCH net-next v2] net: geneve: accept every ethertype Josef Miegl
@ 2023-03-20 16:03 ` Simon Horman
2023-03-21 9:59 ` Eyal Birger
2023-03-21 12:50 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2023-03-20 16:03 UTC (permalink / raw)
To: Josef Miegl
Cc: Eyal Birger, Pravin B Shelar, Josef Miegl, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
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>
...
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2] net: geneve: accept every ethertype
2023-03-19 22:09 [PATCH net-next v2] net: geneve: accept every ethertype Josef Miegl
2023-03-20 16:03 ` Simon Horman
@ 2023-03-21 9:59 ` Eyal Birger
2023-03-21 12:50 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Eyal Birger @ 2023-03-21 9:59 UTC (permalink / raw)
To: Josef Miegl
Cc: Pravin B Shelar, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, linux-kernel
On Mon, Mar 20, 2023 at 12:10 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, 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.
>
> 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>
> ---
> drivers/net/geneve.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 89ff7f8e8c7e..32684e94eb4f 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -365,13 +365,6 @@ static int geneve_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
> if (unlikely(geneveh->ver != GENEVE_VER))
> goto drop;
>
> - inner_proto = geneveh->proto_type;
> -
> - if (unlikely((inner_proto != htons(ETH_P_TEB) &&
> - inner_proto != htons(ETH_P_IP) &&
> - inner_proto != htons(ETH_P_IPV6))))
> - goto drop;
> -
> gs = rcu_dereference_sk_user_data(sk);
> if (!gs)
> goto drop;
> @@ -380,6 +373,8 @@ static int geneve_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
> if (!geneve)
> goto drop;
>
> + inner_proto = geneveh->proto_type;
> +
> if (unlikely((!geneve->cfg.inner_proto_inherit &&
> inner_proto != htons(ETH_P_TEB)))) {
> geneve->dev->stats.rx_dropped++;
> --
> 2.37.1
>
Reviewed-by: Eyal Birger <eyal.birger@gmail.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2] net: geneve: accept every ethertype
2023-03-19 22:09 [PATCH net-next v2] net: geneve: accept every ethertype Josef Miegl
2023-03-20 16:03 ` Simon Horman
2023-03-21 9:59 ` Eyal Birger
@ 2023-03-21 12:50 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-03-21 12:50 UTC (permalink / raw)
To: Josef Miegl
Cc: eyal.birger, pshelar, davem, edumazet, kuba, pabeni, netdev,
linux-kernel
Hello:
This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Sun, 19 Mar 2023 23:09:54 +0100 you 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.
>
> [...]
Here is the summary with links:
- [net-next,v2] net: geneve: accept every ethertype
https://git.kernel.org/netdev/net-next/c/251d5a2813f9
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-21 12:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-19 22:09 [PATCH net-next v2] net: geneve: accept every ethertype Josef Miegl
2023-03-20 16:03 ` Simon Horman
2023-03-21 9:59 ` Eyal Birger
2023-03-21 12:50 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).