diff mbox series

[net,1/2] geneve: Fix incorrect inner network header offset when innerprotoinherit is set

Message ID 20240606203249.1054066-2-tariqt@nvidia.com (mailing list archive)
State Accepted
Commit c6ae073f5903f6c6439d0ac855836a4da5c0a701
Delegated to: Netdev Maintainers
Headers show
Series geneve fixes | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1039 this patch: 1039
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers fail 2 blamed authors not CCed: sd@queasysnail.net phil@philpotter.co.uk; 3 maintainers not CCed: sd@queasysnail.net dsahern@kernel.org phil@philpotter.co.uk
netdev/build_clang success Errors and warnings before: 868 this patch: 868
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1067 this patch: 1067
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 58 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-06-07--09-00 (tests: 1041)

Commit Message

Tariq Toukan June 6, 2024, 8:32 p.m. UTC 
From: Gal Pressman <gal@nvidia.com>

When innerprotoinherit is set, the tunneled packets do not have an inner
Ethernet header.
Change 'maclen' to not always assume the header length is ETH_HLEN, as
there might not be a MAC header.

This resolves issues with drivers (e.g. mlx5, in
mlx5e_tx_tunnel_accel()) who rely on the skb inner network header offset
to be correct, and use it for TX offloads.

Fixes: d8a6213d70ac ("geneve: fix header validation in geneve[6]_xmit_skb")
Signed-off-by: Gal Pressman <gal@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/geneve.c     | 10 ++++++----
 include/net/ip_tunnels.h |  5 +++--
 2 files changed, 9 insertions(+), 6 deletions(-)

Comments

Wojciech Drewek June 7, 2024, 11:10 a.m. UTC | #1 
On 06.06.2024 22:32, Tariq Toukan wrote:
> From: Gal Pressman <gal@nvidia.com>
> 
> When innerprotoinherit is set, the tunneled packets do not have an inner
> Ethernet header.
> Change 'maclen' to not always assume the header length is ETH_HLEN, as
> there might not be a MAC header.
> 
> This resolves issues with drivers (e.g. mlx5, in
> mlx5e_tx_tunnel_accel()) who rely on the skb inner network header offset
> to be correct, and use it for TX offloads.
> 
> Fixes: d8a6213d70ac ("geneve: fix header validation in geneve[6]_xmit_skb")
> Signed-off-by: Gal Pressman <gal@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---

Thanks!
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/geneve.c     | 10 ++++++----
>  include/net/ip_tunnels.h |  5 +++--
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 51495cb4b9be..838e85ddec67 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -815,6 +815,7 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  			   struct geneve_dev *geneve,
>  			   const struct ip_tunnel_info *info)
>  {
> +	bool inner_proto_inherit = geneve->cfg.inner_proto_inherit;
>  	bool xnet = !net_eq(geneve->net, dev_net(geneve->dev));
>  	struct geneve_sock *gs4 = rcu_dereference(geneve->sock4);
>  	const struct ip_tunnel_key *key = &info->key;
> @@ -826,7 +827,7 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  	__be16 sport;
>  	int err;
>  
> -	if (!skb_vlan_inet_prepare(skb))
> +	if (!skb_vlan_inet_prepare(skb, inner_proto_inherit))
>  		return -EINVAL;
>  
>  	if (!gs4)
> @@ -908,7 +909,7 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  	}
>  
>  	err = geneve_build_skb(&rt->dst, skb, info, xnet, sizeof(struct iphdr),
> -			       geneve->cfg.inner_proto_inherit);
> +			       inner_proto_inherit);
>  	if (unlikely(err))
>  		return err;
>  
> @@ -925,6 +926,7 @@ static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  			    struct geneve_dev *geneve,
>  			    const struct ip_tunnel_info *info)
>  {
> +	bool inner_proto_inherit = geneve->cfg.inner_proto_inherit;
>  	bool xnet = !net_eq(geneve->net, dev_net(geneve->dev));
>  	struct geneve_sock *gs6 = rcu_dereference(geneve->sock6);
>  	const struct ip_tunnel_key *key = &info->key;
> @@ -935,7 +937,7 @@ static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  	__be16 sport;
>  	int err;
>  
> -	if (!skb_vlan_inet_prepare(skb))
> +	if (!skb_vlan_inet_prepare(skb, inner_proto_inherit))
>  		return -EINVAL;
>  
>  	if (!gs6)
> @@ -997,7 +999,7 @@ static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  		ttl = ttl ? : ip6_dst_hoplimit(dst);
>  	}
>  	err = geneve_build_skb(dst, skb, info, xnet, sizeof(struct ipv6hdr),
> -			       geneve->cfg.inner_proto_inherit);
> +			       inner_proto_inherit);
>  	if (unlikely(err))
>  		return err;
>  
> diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
> index 9a6a08ec7713..1db2417b8ff5 100644
> --- a/include/net/ip_tunnels.h
> +++ b/include/net/ip_tunnels.h
> @@ -461,9 +461,10 @@ static inline bool pskb_inet_may_pull(struct sk_buff *skb)
>  
>  /* Variant of pskb_inet_may_pull().
>   */
> -static inline bool skb_vlan_inet_prepare(struct sk_buff *skb)
> +static inline bool skb_vlan_inet_prepare(struct sk_buff *skb,
> +					 bool inner_proto_inherit)
>  {
> -	int nhlen = 0, maclen = ETH_HLEN;
> +	int nhlen = 0, maclen = inner_proto_inherit ? 0 : ETH_HLEN;
>  	__be16 type = skb->protocol;
>  
>  	/* Essentially this is skb_protocol(skb, true)
diff mbox series

Patch

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 51495cb4b9be..838e85ddec67 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -815,6 +815,7 @@  static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
 			   struct geneve_dev *geneve,
 			   const struct ip_tunnel_info *info)
 {
+	bool inner_proto_inherit = geneve->cfg.inner_proto_inherit;
 	bool xnet = !net_eq(geneve->net, dev_net(geneve->dev));
 	struct geneve_sock *gs4 = rcu_dereference(geneve->sock4);
 	const struct ip_tunnel_key *key = &info->key;
@@ -826,7 +827,7 @@  static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
 	__be16 sport;
 	int err;
 
-	if (!skb_vlan_inet_prepare(skb))
+	if (!skb_vlan_inet_prepare(skb, inner_proto_inherit))
 		return -EINVAL;
 
 	if (!gs4)
@@ -908,7 +909,7 @@  static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
 	}
 
 	err = geneve_build_skb(&rt->dst, skb, info, xnet, sizeof(struct iphdr),
-			       geneve->cfg.inner_proto_inherit);
+			       inner_proto_inherit);
 	if (unlikely(err))
 		return err;
 
@@ -925,6 +926,7 @@  static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
 			    struct geneve_dev *geneve,
 			    const struct ip_tunnel_info *info)
 {
+	bool inner_proto_inherit = geneve->cfg.inner_proto_inherit;
 	bool xnet = !net_eq(geneve->net, dev_net(geneve->dev));
 	struct geneve_sock *gs6 = rcu_dereference(geneve->sock6);
 	const struct ip_tunnel_key *key = &info->key;
@@ -935,7 +937,7 @@  static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
 	__be16 sport;
 	int err;
 
-	if (!skb_vlan_inet_prepare(skb))
+	if (!skb_vlan_inet_prepare(skb, inner_proto_inherit))
 		return -EINVAL;
 
 	if (!gs6)
@@ -997,7 +999,7 @@  static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
 		ttl = ttl ? : ip6_dst_hoplimit(dst);
 	}
 	err = geneve_build_skb(dst, skb, info, xnet, sizeof(struct ipv6hdr),
-			       geneve->cfg.inner_proto_inherit);
+			       inner_proto_inherit);
 	if (unlikely(err))
 		return err;
 
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 9a6a08ec7713..1db2417b8ff5 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -461,9 +461,10 @@  static inline bool pskb_inet_may_pull(struct sk_buff *skb)
 
 /* Variant of pskb_inet_may_pull().
  */
-static inline bool skb_vlan_inet_prepare(struct sk_buff *skb)
+static inline bool skb_vlan_inet_prepare(struct sk_buff *skb,
+					 bool inner_proto_inherit)
 {
-	int nhlen = 0, maclen = ETH_HLEN;
+	int nhlen = 0, maclen = inner_proto_inherit ? 0 : ETH_HLEN;
 	__be16 type = skb->protocol;
 
 	/* Essentially this is skb_protocol(skb, true)