[PATCH net 0/3] Fix ipsec offlaods with vxlan tunnel

[PATCH net 0/3] Fix ipsec offlaods with vxlan tunnel

[Saeed Mahameed]

From: Saeed Mahameed <saeedm@nvidia.com>

This small series fixes ipsec TX offloads with vxlan overlay on top of
the offloaded ipsec packet, the driver (mlx5) was lacking such information
and the skb->encapsulation bit wasn't enough as indication to reach the
vxlan inner headers, as a solution we mark the tunnel in the offloaded
context of ipsec.

For more information please see commit messages.

Thanks,
Saeed.

---

Huy Nguyen (3):
  net/mlx5: Optimize mlx5e_feature_checks for non IPsec packet
  net/xfrm: Add inner_ipproto into sec_path
  net/mlx5: Fix checksum issue of VXLAN and IPsec crypto offload

 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c  | 65 ++++++++++++++-----
 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h  | 34 ++++++++--
 .../net/ethernet/mellanox/mlx5/core/en_main.c |  8 ++-
 include/net/xfrm.h                            |  1 +
 net/xfrm/xfrm_output.c                        | 36 +++++++++-
 5 files changed, 118 insertions(+), 26 deletions(-)

-- 
2.30.2

[PATCH net 1/3] net/mlx5: Optimize mlx5e_feature_checks for non IPsec packet

[Saeed Mahameed]

From: Huy Nguyen <huyn@nvidia.com>

mlx5e_ipsec_feature_check belongs to mlx5e_tunnel_features_check.
Also, IPsec is not the default configuration so it should be
checked at the end instead of the beginning of mlx5e_features_check.

Fixes: 2ac9cfe78223 ("net/mlx5e: IPSec, Add Innova IPSec offload TX data path")
Signed-off-by: Raed Salem <raeds@nvidia.com>
Signed-off-by: Huy Nguyen <huyn@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h      | 15 +++++++++------
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c |  8 +++++---
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
index 3e80742a3caf..cfa98272e4a9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
@@ -93,8 +93,8 @@ static inline bool mlx5e_ipsec_eseg_meta(struct mlx5_wqe_eth_seg *eseg)
 void mlx5e_ipsec_tx_build_eseg(struct mlx5e_priv *priv, struct sk_buff *skb,
 			       struct mlx5_wqe_eth_seg *eseg);
 
-static inline bool mlx5e_ipsec_feature_check(struct sk_buff *skb, struct net_device *netdev,
-					     netdev_features_t features)
+static inline netdev_features_t
+mlx5e_ipsec_feature_check(struct sk_buff *skb, netdev_features_t features)
 {
 	struct sec_path *sp = skb_sec_path(skb);
 
@@ -102,9 +102,11 @@ static inline bool mlx5e_ipsec_feature_check(struct sk_buff *skb, struct net_dev
 		struct xfrm_state *x = sp->xvec[0];
 
 		if (x && x->xso.offload_handle)
-			return true;
+			return features;
 	}
-	return false;
+
+	/* Disable CSUM and GSO for software IPsec */
+	return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
 }
 
 #else
@@ -120,8 +122,9 @@ static inline bool mlx5e_ipsec_eseg_meta(struct mlx5_wqe_eth_seg *eseg)
 }
 
 static inline bool mlx5_ipsec_is_rx_flow(struct mlx5_cqe64 *cqe) { return false; }
-static inline bool mlx5e_ipsec_feature_check(struct sk_buff *skb, struct net_device *netdev,
-					     netdev_features_t features) { return false; }
+static inline netdev_features_t
+mlx5e_ipsec_feature_check(struct sk_buff *skb, netdev_features_t features)
+{ return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK); }
 #endif /* CONFIG_MLX5_EN_IPSEC */
 
 #endif /* __MLX5E_IPSEC_RXTX_H__ */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 5db63b9f3b70..1b7fa0264652 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -4552,6 +4552,11 @@ static netdev_features_t mlx5e_tunnel_features_check(struct mlx5e_priv *priv,
 		/* Support Geneve offload for default UDP port */
 		if (port == GENEVE_UDP_PORT && mlx5_geneve_tx_allowed(priv->mdev))
 			return features;
+#endif
+		break;
+#ifdef CONFIG_MLX5_EN_IPSEC
+	case IPPROTO_ESP:
+		return mlx5e_ipsec_feature_check(skb, features);
 #endif
 	}
 
@@ -4569,9 +4574,6 @@ netdev_features_t mlx5e_features_check(struct sk_buff *skb,
 	features = vlan_features_check(skb, features);
 	features = vxlan_features_check(skb, features);
 
-	if (mlx5e_ipsec_feature_check(skb, netdev, features))
-		return features;
-
 	/* Validate if the tunneled packet is being offloaded by HW */
 	if (skb->encapsulation &&
 	    (features & NETIF_F_CSUM_MASK || features & NETIF_F_GSO_MASK))
-- 
2.30.2

[PATCH net 2/3] net/xfrm: Add inner_ipproto into sec_path

[Saeed Mahameed]

From: Huy Nguyen <huyn@nvidia.com>

The inner_ipproto saves the inner IP protocol of the plain
text packet. This allows vendor's IPsec feature making offload
decision at skb's features_check and configuring hardware at
ndo_start_xmit.

For example, ConnectX6-DX IPsec device needs the plaintext's
IP protocol to support partial checksum offload on
VXLAN/GENEVE packet over IPsec transport mode tunnel.

Signed-off-by: Raed Salem <raeds@nvidia.com>
Signed-off-by: Huy Nguyen <huyn@nvidia.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/net/xfrm.h     |  1 +
 net/xfrm/xfrm_output.c | 36 +++++++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index c58a6d4eb610..e535700431fb 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1032,6 +1032,7 @@ struct sec_path {
 
 	struct xfrm_state	*xvec[XFRM_MAX_DEPTH];
 	struct xfrm_offload	ovec[XFRM_MAX_OFFLOAD_DEPTH];
+	u8			inner_ipproto;
 };
 
 struct sec_path *secpath_set(struct sk_buff *skb);
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index e4cb0ff4dcf4..da412928093b 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -565,6 +565,36 @@ static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb
 	return 0;
 }
 
+/* Save inner ip protocol for vendor offload usage */
+static void get_inner_ipproto(struct sk_buff *skb, struct sec_path *sp)
+{
+	const struct ethhdr *eth;
+
+	if (!skb->inner_protocol)
+		return;
+
+	if (skb->inner_protocol_type == ENCAP_TYPE_IPPROTO) {
+		sp->inner_ipproto = skb->inner_protocol;
+		return;
+	}
+
+	if (skb->inner_protocol_type != ENCAP_TYPE_ETHER)
+		return;
+
+	eth = (struct ethhdr *)skb_inner_mac_header(skb);
+
+	switch (eth->h_proto) {
+	case ntohs(ETH_P_IPV6):
+		sp->inner_ipproto = inner_ipv6_hdr(skb)->nexthdr;
+		break;
+	case ntohs(ETH_P_IP):
+		sp->inner_ipproto = inner_ip_hdr(skb)->protocol;
+		break;
+	default:
+		return;
+	}
+}
+
 int xfrm_output(struct sock *sk, struct sk_buff *skb)
 {
 	struct net *net = dev_net(skb_dst(skb)->dev);
@@ -594,8 +624,12 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
 			kfree_skb(skb);
 			return -ENOMEM;
 		}
-		skb->encapsulation = 1;
 
+		sp->inner_ipproto = 0;
+		if (skb->encapsulation)
+			get_inner_ipproto(skb, sp);
+
+		skb->encapsulation = 1;
 		sp->olen++;
 		sp->xvec[sp->len++] = x;
 		xfrm_state_hold(x);
-- 
2.30.2

[PATCH net 3/3] net/mlx5: Fix checksum issue of VXLAN and IPsec crypto offload

[Saeed Mahameed]

From: Huy Nguyen <huyn@nvidia.com>

The packet is VXLAN packet over IPsec transport mode tunnel
which has the following format: [IP1 | ESP | UDP | VXLAN | IP2 | TCP]
NVIDIA ConnectX card cannot do checksum offload for two L4 headers.
The solution is using the checksum partial offload similar to
VXLAN | TCP packet. Hardware calculates IP1, IP2 and TCP checksums and
software calculates UDP checksum. However, unlike VXLAN | TCP case,
IPsec's mlx5 driver cannot access the inner plaintext IP protocol type.
Therefore, inner_ipproto is added in the sec_path structure
to provide this information. Also, utilize the skb's csum_start to
program L4 inner checksum offset.

While at it, remove the call to mlx5e_set_eseg_swp and setup software parser
fields directly in mlx5e_ipsec_set_swp. mlx5e_set_eseg_swp is not
needed as the two features (GENEVE and IPsec) are different and adding
this sharing layer creates unnecessary complexity and affect
performance.

For the case VXLAN packet over IPsec tunnel mode tunnel, checksum offload
is disabled because the hardware does not support checksum offload for
three L3 (IP) headers.

Fixes: 2ac9cfe78223 ("net/mlx5e: IPSec, Add Innova IPSec offload TX data path")
Signed-off-by: Raed Salem <raeds@nvidia.com>
Signed-off-by: Huy Nguyen <huyn@nvidia.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c  | 65 ++++++++++++++-----
 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h  | 21 +++++-
 2 files changed, 69 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
index a97e8d205094..6d2b809988b1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
@@ -136,7 +136,7 @@ static void mlx5e_ipsec_set_swp(struct sk_buff *skb,
 				struct mlx5_wqe_eth_seg *eseg, u8 mode,
 				struct xfrm_offload *xo)
 {
-	struct mlx5e_swp_spec swp_spec = {};
+	struct sec_path *sp = skb_sec_path(skb);
 
 	/* Tunnel Mode:
 	 * SWP:      OutL3       InL3  InL4
@@ -146,23 +146,58 @@ static void mlx5e_ipsec_set_swp(struct sk_buff *skb,
 	 * SWP:      OutL3       InL4
 	 *           InL3
 	 * Pkt: MAC  IP     ESP  L4
+	 *
+	 * Tunnel(VXLAN TCP/UDP) over Transport Mode
+	 * SWP:      OutL3                   InL3  InL4
+	 * Pkt: MAC  IP     ESP  UDP  VXLAN  IP    L4
 	 */
-	swp_spec.l3_proto = skb->protocol;
-	swp_spec.is_tun = mode == XFRM_MODE_TUNNEL;
-	if (swp_spec.is_tun) {
-		if (xo->proto == IPPROTO_IPV6) {
-			swp_spec.tun_l3_proto = htons(ETH_P_IPV6);
-			swp_spec.tun_l4_proto = inner_ipv6_hdr(skb)->nexthdr;
-		} else {
-			swp_spec.tun_l3_proto = htons(ETH_P_IP);
-			swp_spec.tun_l4_proto = inner_ip_hdr(skb)->protocol;
-		}
-	} else {
-		swp_spec.tun_l3_proto = skb->protocol;
-		swp_spec.tun_l4_proto = xo->proto;
+
+	/* Shared settings */
+	eseg->swp_outer_l3_offset = skb_network_offset(skb) / 2;
+	if (skb->protocol == htons(ETH_P_IPV6))
+		eseg->swp_flags |= MLX5_ETH_WQE_SWP_OUTER_L3_IPV6;
+
+	/* Tunnel mode */
+	if (mode == XFRM_MODE_TUNNEL) {
+		eseg->swp_inner_l3_offset = skb_inner_network_offset(skb) / 2;
+		eseg->swp_inner_l4_offset = skb_inner_transport_offset(skb) / 2;
+		if (xo->proto == IPPROTO_IPV6)
+			eseg->swp_flags |= MLX5_ETH_WQE_SWP_INNER_L3_IPV6;
+		if (inner_ip_hdr(skb)->protocol == IPPROTO_UDP)
+			eseg->swp_flags |= MLX5_ETH_WQE_SWP_INNER_L4_UDP;
+		return;
+	}
+
+	/* Transport mode */
+	if (mode != XFRM_MODE_TRANSPORT)
+		return;
+
+	if (!sp->inner_ipproto) {
+		eseg->swp_inner_l3_offset = skb_network_offset(skb) / 2;
+		eseg->swp_inner_l4_offset = skb_inner_transport_offset(skb) / 2;
+		if (skb->protocol == htons(ETH_P_IPV6))
+			eseg->swp_flags |= MLX5_ETH_WQE_SWP_INNER_L3_IPV6;
+		if (xo->proto == IPPROTO_UDP)
+			eseg->swp_flags |= MLX5_ETH_WQE_SWP_INNER_L4_UDP;
+		return;
+	}
+
+	/* Tunnel(VXLAN TCP/UDP) over Transport Mode */
+	switch (sp->inner_ipproto) {
+	case IPPROTO_UDP:
+		eseg->swp_flags |= MLX5_ETH_WQE_SWP_INNER_L4_UDP;
+		fallthrough;
+	case IPPROTO_TCP:
+		eseg->swp_inner_l3_offset = skb_inner_network_offset(skb) / 2;
+		eseg->swp_inner_l4_offset = (skb->csum_start + skb->head - skb->data) / 2;
+		if (skb->protocol == htons(ETH_P_IPV6))
+			eseg->swp_flags |= MLX5_ETH_WQE_SWP_INNER_L3_IPV6;
+		break;
+	default:
+		break;
 	}
 
-	mlx5e_set_eseg_swp(skb, eseg, &swp_spec);
+	return;
 }
 
 void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state *x,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
index cfa98272e4a9..de242cf1510a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
@@ -101,11 +101,28 @@ mlx5e_ipsec_feature_check(struct sk_buff *skb, netdev_features_t features)
 	if (sp && sp->len) {
 		struct xfrm_state *x = sp->xvec[0];
 
-		if (x && x->xso.offload_handle)
-			return features;
+		if (!x || !x->xso.offload_handle)
+			goto out_disable;
+
+		if (sp->inner_ipproto) {
+			/* Cannot support tunnel packet over IPsec tunnel mode
+			 * because we cannot offload three IP header csum
+			 */
+			if (x->props.mode == XFRM_MODE_TUNNEL)
+				goto out_disable;
+
+			/* Only support UDP or TCP L4 checksum */
+			if (sp->inner_ipproto != IPPROTO_UDP &&
+			    sp->inner_ipproto != IPPROTO_TCP)
+				goto out_disable;
+		}
+
+		return features;
+
 	}
 
 	/* Disable CSUM and GSO for software IPsec */
+out_disable:
 	return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
 }
 
-- 
2.30.2

Re: [PATCH net 2/3] net/xfrm: Add inner_ipproto into sec_path

[Steffen Klassert]

On Wed, Apr 14, 2021 at 04:25:39PM -0700, Saeed Mahameed wrote:
> From: Huy Nguyen <huyn@nvidia.com>
> 
> The inner_ipproto saves the inner IP protocol of the plain
> text packet. This allows vendor's IPsec feature making offload
> decision at skb's features_check and configuring hardware at
> ndo_start_xmit.
> 
> For example, ConnectX6-DX IPsec device needs the plaintext's
> IP protocol to support partial checksum offload on
> VXLAN/GENEVE packet over IPsec transport mode tunnel.
> 
> Signed-off-by: Raed Salem <raeds@nvidia.com>
> Signed-off-by: Huy Nguyen <huyn@nvidia.com>
> Cc: Steffen Klassert <steffen.klassert@secunet.com>
> ---
>  include/net/xfrm.h     |  1 +
>  net/xfrm/xfrm_output.c | 36 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index c58a6d4eb610..e535700431fb 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -1032,6 +1032,7 @@ struct sec_path {
>  
>  	struct xfrm_state	*xvec[XFRM_MAX_DEPTH];
>  	struct xfrm_offload	ovec[XFRM_MAX_OFFLOAD_DEPTH];
> +	u8			inner_ipproto;

This is for offload, so it should go to struct xfrm_offload.
We have already 'proto' there, so it is just easy add
'inner_proto'.

Re: [PATCH net 2/3] net/xfrm: Add inner_ipproto into sec_path

[Jakub Kicinski]

On Wed, 14 Apr 2021 16:25:39 -0700 Saeed Mahameed wrote:
> +static void get_inner_ipproto(struct sk_buff *skb, struct sec_path *sp)
> +{
> +	const struct ethhdr *eth;
> +
> +	if (!skb->inner_protocol)
> +		return;
> +
> +	if (skb->inner_protocol_type == ENCAP_TYPE_IPPROTO) {
> +		sp->inner_ipproto = skb->inner_protocol;
> +		return;
> +	}
> +
> +	if (skb->inner_protocol_type != ENCAP_TYPE_ETHER)
> +		return;
> +
> +	eth = (struct ethhdr *)skb_inner_mac_header(skb);
> +
> +	switch (eth->h_proto) {
> +	case ntohs(ETH_P_IPV6):
> +		sp->inner_ipproto = inner_ipv6_hdr(skb)->nexthdr;
> +		break;
> +	case ntohs(ETH_P_IP):
> +		sp->inner_ipproto = inner_ip_hdr(skb)->protocol;
> +		break;
> +	default:
> +		return;
> +	}
> +}

Bunch of sparse warnings here, please check.

Re: [PATCH net 2/3] net/xfrm: Add inner_ipproto into sec_path

[Huy Nguyen]

I fixed. Thank you. Saeed will resubmit.

On 4/15/2021 12:00 PM, Jakub Kicinski wrote:
> On Wed, 14 Apr 2021 16:25:39 -0700 Saeed Mahameed wrote:
>> +static void get_inner_ipproto(struct sk_buff *skb, struct sec_path *sp)
>> +{
>> +	const struct ethhdr *eth;
>> +
>> +	if (!skb->inner_protocol)
>> +		return;
>> +
>> +	if (skb->inner_protocol_type == ENCAP_TYPE_IPPROTO) {
>> +		sp->inner_ipproto = skb->inner_protocol;
>> +		return;
>> +	}
>> +
>> +	if (skb->inner_protocol_type != ENCAP_TYPE_ETHER)
>> +		return;
>> +
>> +	eth = (struct ethhdr *)skb_inner_mac_header(skb);
>> +
>> +	switch (eth->h_proto) {
>> +	case ntohs(ETH_P_IPV6):
>> +		sp->inner_ipproto = inner_ipv6_hdr(skb)->nexthdr;
>> +		break;
>> +	case ntohs(ETH_P_IP):
>> +		sp->inner_ipproto = inner_ip_hdr(skb)->protocol;
>> +		break;
>> +	default:
>> +		return;
>> +	}
>> +}
> Bunch of sparse warnings here, please check.