netdev - Re: [PATCH net V2 3/3] net/mlx5e: kTLS, Cancel RX async resync request in error flows

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8186f892-4753-4059-b396-6895c38cc07a@nvidia.com>
Date: Tue, 21 Oct 2025 18:25:38 +0300
From: Shahar Shitrit <shshitrit@...dia.com>
To: Sabrina Dubroca <sd@...asysnail.net>, Tariq Toukan <tariqt@...dia.com>
Cc: Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Andrew Lunn <andrew+netdev@...n.ch>,
 "David S. Miller" <davem@...emloft.net>, Saeed Mahameed <saeedm@...dia.com>,
 Leon Romanovsky <leon@...nel.org>, Mark Bloch <mbloch@...dia.com>,
 John Fastabend <john.fastabend@...il.com>, netdev@...r.kernel.org,
 linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org,
 Gal Pressman <gal@...dia.com>
Subject: Re: [PATCH net V2 3/3] net/mlx5e: kTLS, Cancel RX async resync
 request in error flows



On 21/10/2025 17:54, Sabrina Dubroca wrote:
> 2025-10-20, 10:05:54 +0300, Tariq Toukan wrote:
>> From: Shahar Shitrit <shshitrit@...dia.com>
>>
>> When device loses track of TLS records, it attempts to resync by
>> monitoring records and requests an asynchronous resynchronization
>> from software for this TLS connection.
>>
>> The TLS module handles such device RX resync requests by logging record
>> headers and comparing them with the record tcp_sn when provided by the
>> device. It also increments rcd_delta to track how far the current
>> record tcp_sn is from the tcp_sn of the original resync request.
>> If the device later responds with a matching tcp_sn, the TLS module
>> approves the tcp_sn for resync.
>>
>> However, the device response may be delayed or never arrive,
>> particularly due to traffic-related issues such as packet drops or
>> reordering. In such cases, the TLS module remains unaware that resync
>> will not complete, and continues performing unnecessary work by logging
>> headers and incrementing rcd_delta, which can eventually exceed the
>> threshold and trigger a WARN(). For example, this was observed when the
>> device got out of tracking, causing
>> mlx5e_ktls_handle_get_psv_completion() to fail and ultimately leading
>> to the rcd_delta warning.
>>
>> To address this, call tls_offload_rx_resync_async_request_cancel()
>> to cancel the resync request and stop resync tracking in such error
>> cases. Also, increment the tls_resync_req_skip counter to track these
>> cancellations.
>>
>> Fixes: 0419d8c9d8f8 ("net/mlx5e: kTLS, Add kTLS RX resync support")
>> Signed-off-by: Shahar Shitrit <shshitrit@...dia.com>
>> Signed-off-by: Tariq Toukan <tariqt@...dia.com>
>> ---
>>  .../mellanox/mlx5/core/en_accel/ktls_rx.c     | 33 ++++++++++++++++---
>>  .../mellanox/mlx5/core/en_accel/ktls_txrx.h   |  4 +++
>>  .../net/ethernet/mellanox/mlx5/core/en_rx.c   |  4 +++
>>  3 files changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c
>> index 5fbc92269585..ae325c471e7f 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_rx.c
>> @@ -339,14 +339,19 @@ static void resync_handle_work(struct work_struct *work)
>>  
>>  	if (unlikely(test_bit(MLX5E_PRIV_RX_FLAG_DELETING, priv_rx->flags))) {
>>  		mlx5e_ktls_priv_rx_put(priv_rx);
>> +		priv_rx->rq_stats->tls_resync_req_skip++;
>> +		tls_offload_rx_resync_async_request_cancel(&resync->core);
>>  		return;
>>  	}
>>  
>>  	c = resync->priv->channels.c[priv_rx->rxq];
>>  	sq = &c->async_icosq;
>>  
>> -	if (resync_post_get_progress_params(sq, priv_rx))
>> +	if (resync_post_get_progress_params(sq, priv_rx)) {
>> +		priv_rx->rq_stats->tls_resync_req_skip++;
> 
> There's already a tls_resync_req_skip++ at the end of
> resync_post_get_progress_params() just before returning an error, so I
> don't think this one is needed? (or keep this one and remove the one
> in resync_post_get_progress_params, so that tls_resync_req_skip++ and
> _cancel() are together like in the rest of the patch)
> 
> Other than that, I don't understand much about the resync handling in
> the driver and how the various bits fit together, but the patch looks
> consistent.

Right, thank you. Will fix>
>> +		tls_offload_rx_resync_async_request_cancel(&resync->core);
>>  		mlx5e_ktls_priv_rx_put(priv_rx);
>> +	}
>>  }
>