diff mbox series

[net,8/8] net/mlx5e: kTLS, Add resiliency to zero-size record frags in TX resync flow

Message ID 20200429225449.60664-9-saeedm@mellanox.com
State Superseded
Delegated to: David Miller
Headers show
Series [net,1/8] net/mlx5: E-switch, Fix error unwinding flow for steering init failure | expand

Commit Message

Saeed Mahameed April 29, 2020, 10:54 p.m. UTC 
From: Tariq Toukan <tariqt@mellanox.com>

SKBs of TLS records might have empty zero-sized frags.
Posting a DUMP WQE for such frag would result an error completion.
Add in-driver resiliency and skip such frags.

Fixes: d2ead1f360e8 ("net/mlx5e: Add kTLS TX HW offload support")
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Reviewed-by: Boris Pismenny <borisp@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Jakub Kicinski April 30, 2020, 12:12 a.m. UTC | #1 
On Wed, 29 Apr 2020 15:54:49 -0700 Saeed Mahameed wrote:
> From: Tariq Toukan <tariqt@mellanox.com>
> 
> SKBs of TLS records might have empty zero-sized frags.

Why? Let's fix that instead of adding checks to drivers.

> Posting a DUMP WQE for such frag would result an error completion.
> Add in-driver resiliency and skip such frags.
> 
> Fixes: d2ead1f360e8 ("net/mlx5e: Add kTLS TX HW offload support")
Tariq Toukan April 30, 2020, 8:22 a.m. UTC | #2 
On 4/30/2020 3:12 AM, Jakub Kicinski wrote:
> On Wed, 29 Apr 2020 15:54:49 -0700 Saeed Mahameed wrote:
>> From: Tariq Toukan <tariqt@mellanox.com>
>>
>> SKBs of TLS records might have empty zero-sized frags.
> 
> Why? Let's fix that instead of adding checks to drivers.
> 

Hi Jakub,

The HW spec requires the DUMP size to be non-zero, this patch comes to 
guarantee this in driver.
In kernel stack, having zero-side fragments is for sure non-optimal 
practice, but still could be considered valid and tolerated.
I agree that we should find the source of this practice in stack and 
enhance it.

Thanks,
Tariq

>> Posting a DUMP WQE for such frag would result an error completion.
>> Add in-driver resiliency and skip such frags.
>>
>> Fixes: d2ead1f360e8 ("net/mlx5e: Add kTLS TX HW offload support")
Saeed Mahameed April 30, 2020, 3:37 p.m. UTC | #3 
On Thu, 2020-04-30 at 11:22 +0300, Tariq Toukan wrote:
> 
> On 4/30/2020 3:12 AM, Jakub Kicinski wrote:
> > On Wed, 29 Apr 2020 15:54:49 -0700 Saeed Mahameed wrote:
> > > From: Tariq Toukan <tariqt@mellanox.com>
> > > 
> > > SKBs of TLS records might have empty zero-sized frags.
> > 
> > Why? Let's fix that instead of adding checks to drivers.
> > 
> 
> Hi Jakub,
> 
> The HW spec requires the DUMP size to be non-zero, this patch comes
> to 
> guarantee this in driver.
> In kernel stack, having zero-side fragments is for sure non-optimal 
> practice, but still could be considered valid and tolerated.
> I agree that we should find the source of this practice in stack and 
> enhance it.
> 

Ok then, let's find the source, i will drop this patch for now..
We should aim for less driver code duplication, if this is something
that can be guaranteed by the ktls stack.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
index 52a56622034a..a04fa23e1a53 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
@@ -232,9 +232,14 @@  tx_sync_info_get(struct mlx5e_ktls_offload_context_tx *priv_tx,
 	remaining = info->sync_len;
 	while (remaining > 0) {
 		skb_frag_t *frag = &record->frags[i];
+		unsigned int fsz;
+
+		fsz = skb_frag_size(frag);
+		if (unlikely(!fsz))
+			continue;
 
 		get_page(skb_frag_page(frag));
-		remaining -= skb_frag_size(frag);
+		remaining -= fsz;
 		info->frags[i++] = *frag;
 	}
 	/* reduce the part which will be sent with the original SKB */