[pull request][net 0/2] Mellanox, mlx5 fixes 2018-05-24

[pull request][net 0/2] Mellanox, mlx5 fixes 2018-05-24

[Saeed Mahameed]

Hi Dave,

This series includes two mlx5 fixes.

1) add FCS data to checksum complete when required, from Eran Ben
Elisha.

2) Fix A race in IPSec sandbox QP commands, from Yossi Kuperman.

Please pull and let me know if there's any problem.

for -stable v4.15
("net/mlx5e: When RXFCS is set, add FCS data into checksum calculation")

Thanks,
Saeed.


---

git format-pullreq $NTAG "for-next" $BASE $TARGET $NTAG

The following changes since commit d546b67cda015fb92bfee93d5dc0ceadb91deaee:

  net/mlx4: Fix irq-unsafe spinlock usage (2018-05-23 15:48:58 -0400)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git tags/mlx5-fixes-2018-05-24

for you to fetch changes up to 1dcbc01f73f9abc4779f71eae5e6dc61bee37229:

  net/mlx5: IPSec, Fix a race between concurrent sandbox QP commands (2018-05-24 14:40:40 -0700)

----------------------------------------------------------------
mlx5-fixes-2018-05-24

----------------------------------------------------------------
Eran Ben Elisha (1):
      net/mlx5e: When RXFCS is set, add FCS data into checksum calculation

Yossi Kuperman (1):
      net/mlx5: IPSec, Fix a race between concurrent sandbox QP commands

 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c    | 42 ++++++++++++++++++++++
 .../net/ethernet/mellanox/mlx5/core/fpga/ipsec.c   | 12 +++----
 2 files changed, 47 insertions(+), 7 deletions(-)

[net 1/2] net/mlx5e: When RXFCS is set, add FCS data into checksum calculation

[Saeed Mahameed]

From: Eran Ben Elisha <eranbe@mellanox.com>

When RXFCS feature is enabled, the HW do not strip the FCS data,
however it is not present in the checksum calculated by the HW.

Fix that by manually calculating the FCS checksum and adding it to the SKB
checksum field.

Add helper function to find the FCS data for all SKB forms (linear,
one fragment or more).

Fixes: 102722fc6832 ("net/mlx5e: Add support for RXFCS feature flag")
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 176645762e49..1ff0b0e93804 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -615,6 +615,45 @@ static inline bool is_last_ethertype_ip(struct sk_buff *skb, int *network_depth)
 	return (ethertype == htons(ETH_P_IP) || ethertype == htons(ETH_P_IPV6));
 }
 
+static __be32 mlx5e_get_fcs(struct sk_buff *skb)
+{
+	int last_frag_sz, bytes_in_prev, nr_frags;
+	u8 *fcs_p1, *fcs_p2;
+	skb_frag_t *last_frag;
+	__be32 fcs_bytes;
+
+	if (!skb_is_nonlinear(skb))
+		return *(__be32 *)(skb->data + skb->len - ETH_FCS_LEN);
+
+	nr_frags = skb_shinfo(skb)->nr_frags;
+	last_frag = &skb_shinfo(skb)->frags[nr_frags - 1];
+	last_frag_sz = skb_frag_size(last_frag);
+
+	/* If all FCS data is in last frag */
+	if (last_frag_sz >= ETH_FCS_LEN)
+		return *(__be32 *)(skb_frag_address(last_frag) +
+				   last_frag_sz - ETH_FCS_LEN);
+
+	fcs_p2 = (u8 *)skb_frag_address(last_frag);
+	bytes_in_prev = ETH_FCS_LEN - last_frag_sz;
+
+	/* Find where the other part of the FCS is - Linear or another frag */
+	if (nr_frags == 1) {
+		fcs_p1 = skb_tail_pointer(skb);
+	} else {
+		skb_frag_t *prev_frag = &skb_shinfo(skb)->frags[nr_frags - 2];
+
+		fcs_p1 = skb_frag_address(prev_frag) +
+			    skb_frag_size(prev_frag);
+	}
+	fcs_p1 -= bytes_in_prev;
+
+	memcpy(&fcs_bytes, fcs_p1, bytes_in_prev);
+	memcpy(((u8 *)&fcs_bytes) + bytes_in_prev, fcs_p2, last_frag_sz);
+
+	return fcs_bytes;
+}
+
 static inline void mlx5e_handle_csum(struct net_device *netdev,
 				     struct mlx5_cqe64 *cqe,
 				     struct mlx5e_rq *rq,
@@ -643,6 +682,9 @@ static inline void mlx5e_handle_csum(struct net_device *netdev,
 			skb->csum = csum_partial(skb->data + ETH_HLEN,
 						 network_depth - ETH_HLEN,
 						 skb->csum);
+		if (unlikely(netdev->features & NETIF_F_RXFCS))
+			skb->csum = csum_add(skb->csum,
+					     (__force __wsum)mlx5e_get_fcs(skb));
 		rq->stats.csum_complete++;
 		return;
 	}
-- 
2.17.0

[net 2/2] net/mlx5: IPSec, Fix a race between concurrent sandbox QP commands

[Saeed Mahameed]

From: Yossi Kuperman <yossiku@mellanox.com>

Sandbox QP Commands are retired in the order they are sent. Outstanding
commands are stored in a linked-list in the order they appear. Once a
response is received and the callback gets called, we pull the first
element off the pending list, assuming they correspond.

Sending a message and adding it to the pending list is not done atomically,
hence there is an opportunity for a race between concurrent requests.

Bind both send and add under a critical section.

Fixes: bebb23e6cb02 ("net/mlx5: Accel, Add IPSec acceleration interface")
Signed-off-by: Yossi Kuperman <yossiku@mellanox.com>
Signed-off-by: Adi Nissim <adin@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c
index 0f5da499a223..fad8c2e3804e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c
@@ -237,19 +237,17 @@ static void *mlx5_fpga_ipsec_cmd_exec(struct mlx5_core_dev *mdev,
 	context->buf.sg[0].data = &context->command;
 
 	spin_lock_irqsave(&fdev->ipsec->pending_cmds_lock, flags);
-	list_add_tail(&context->list, &fdev->ipsec->pending_cmds);
+	res = mlx5_fpga_sbu_conn_sendmsg(fdev->ipsec->conn, &context->buf);
+	if (!res)
+		list_add_tail(&context->list, &fdev->ipsec->pending_cmds);
 	spin_unlock_irqrestore(&fdev->ipsec->pending_cmds_lock, flags);
 
-	res = mlx5_fpga_sbu_conn_sendmsg(fdev->ipsec->conn, &context->buf);
 	if (res) {
-		mlx5_fpga_warn(fdev, "Failure sending IPSec command: %d\n",
-			       res);
-		spin_lock_irqsave(&fdev->ipsec->pending_cmds_lock, flags);
-		list_del(&context->list);
-		spin_unlock_irqrestore(&fdev->ipsec->pending_cmds_lock, flags);
+		mlx5_fpga_warn(fdev, "Failed to send IPSec command: %d\n", res);
 		kfree(context);
 		return ERR_PTR(res);
 	}
+
 	/* Context will be freed by wait func after completion */
 	return context;
 }
-- 
2.17.0

Re: [pull request][net 0/2] Mellanox, mlx5 fixes 2018-05-24

[David Miller]

From: Saeed Mahameed <saeedm@mellanox.com>
Date: Thu, 24 May 2018 14:53:11 -0700

> This series includes two mlx5 fixes.
> 
> 1) add FCS data to checksum complete when required, from Eran Ben
> Elisha.
> 
> 2) Fix A race in IPSec sandbox QP commands, from Yossi Kuperman.
> 
> Please pull and let me know if there's any problem.

Pulled.

> for -stable v4.15
> ("net/mlx5e: When RXFCS is set, add FCS data into checksum calculation")

Queued up.

Re: [net 1/2] net/mlx5e: When RXFCS is set, add FCS data into checksum calculation

[Eric Dumazet]

On 05/24/2018 02:53 PM, Saeed Mahameed wrote:
> From: Eran Ben Elisha <eranbe@mellanox.com>
> 
> When RXFCS feature is enabled, the HW do not strip the FCS data,
> however it is not present in the checksum calculated by the HW.
> 
> Fix that by manually calculating the FCS checksum and adding it to the SKB
> checksum field.
> 
> Add helper function to find the FCS data for all SKB forms (linear,
> one fragment or more).
> 
> Fixes: 102722fc6832 ("net/mlx5e: Add support for RXFCS feature flag")
> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
>  .../net/ethernet/mellanox/mlx5/core/en_rx.c   | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> index 176645762e49..1ff0b0e93804 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -615,6 +615,45 @@ static inline bool is_last_ethertype_ip(struct sk_buff *skb, int *network_depth)
>  	return (ethertype == htons(ETH_P_IP) || ethertype == htons(ETH_P_IPV6));
>  }
>  
> +static __be32 mlx5e_get_fcs(struct sk_buff *skb)
> +{
> +	int last_frag_sz, bytes_in_prev, nr_frags;
> +	u8 *fcs_p1, *fcs_p2;
> +	skb_frag_t *last_frag;
> +	__be32 fcs_bytes;
> +
> +	if (!skb_is_nonlinear(skb))
> +		return *(__be32 *)(skb->data + skb->len - ETH_FCS_LEN);
> +
> +	nr_frags = skb_shinfo(skb)->nr_frags;
> +	last_frag = &skb_shinfo(skb)->frags[nr_frags - 1];
> +	last_frag_sz = skb_frag_size(last_frag);
> +
> +	/* If all FCS data is in last frag */
> +	if (last_frag_sz >= ETH_FCS_LEN)
> +		return *(__be32 *)(skb_frag_address(last_frag) +
> +				   last_frag_sz - ETH_FCS_LEN);
> +
> +	fcs_p2 = (u8 *)skb_frag_address(last_frag);
> +	bytes_in_prev = ETH_FCS_LEN - last_frag_sz;
> +
> +	/* Find where the other part of the FCS is - Linear or another frag */
> +	if (nr_frags == 1) {
> +		fcs_p1 = skb_tail_pointer(skb);
> +	} else {
> +		skb_frag_t *prev_frag = &skb_shinfo(skb)->frags[nr_frags - 2];
> +
> +		fcs_p1 = skb_frag_address(prev_frag) +
> +			    skb_frag_size(prev_frag);
> +	}
> +	fcs_p1 -= bytes_in_prev;
> +
> +	memcpy(&fcs_bytes, fcs_p1, bytes_in_prev);
> +	memcpy(((u8 *)&fcs_bytes) + bytes_in_prev, fcs_p2, last_frag_sz);
> +
> +	return fcs_bytes;
> +}
>

Oh well, this is so ugly, why isn't skb_header_pointer() used ?

Untested patch :

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 94224c22ecc310a87b6715051e335446f29bec03..11129e3a50d6f3b9a49861a99023541720bbcbe4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -713,43 +713,12 @@ static inline void mlx5e_enable_ecn(struct mlx5e_rq *rq, struct sk_buff *skb)
        rq->stats->ecn_mark += !!rc;
 }
 
-static __be32 mlx5e_get_fcs(struct sk_buff *skb)
+static __be32 mlx5e_get_fcs(const struct sk_buff *skb)
 {
-       int last_frag_sz, bytes_in_prev, nr_frags;
-       u8 *fcs_p1, *fcs_p2;
-       skb_frag_t *last_frag;
        __be32 fcs_bytes;
 
-       if (!skb_is_nonlinear(skb))
-               return *(__be32 *)(skb->data + skb->len - ETH_FCS_LEN);
-
-       nr_frags = skb_shinfo(skb)->nr_frags;
-       last_frag = &skb_shinfo(skb)->frags[nr_frags - 1];
-       last_frag_sz = skb_frag_size(last_frag);
-
-       /* If all FCS data is in last frag */
-       if (last_frag_sz >= ETH_FCS_LEN)
-               return *(__be32 *)(skb_frag_address(last_frag) +
-                                  last_frag_sz - ETH_FCS_LEN);
-
-       fcs_p2 = (u8 *)skb_frag_address(last_frag);
-       bytes_in_prev = ETH_FCS_LEN - last_frag_sz;
-
-       /* Find where the other part of the FCS is - Linear or another frag */
-       if (nr_frags == 1) {
-               fcs_p1 = skb_tail_pointer(skb);
-       } else {
-               skb_frag_t *prev_frag = &skb_shinfo(skb)->frags[nr_frags - 2];
-
-               fcs_p1 = skb_frag_address(prev_frag) +
-                           skb_frag_size(prev_frag);
-       }
-       fcs_p1 -= bytes_in_prev;
-
-       memcpy(&fcs_bytes, fcs_p1, bytes_in_prev);
-       memcpy(((u8 *)&fcs_bytes) + bytes_in_prev, fcs_p2, last_frag_sz);
-
-       return fcs_bytes;
+       return *(__be32 *)skb_header_pointer(skb, skb->len - ETH_FCS_LEN,
+                                            ETH_FCS_LEN, &fcs_bytes);
 }