Re: [PATCH net 2/3] net: tls: Cancel RX async resync request on rdc_delta overflow

[Shahar Shitrit <shshitrit@nvidia.com>]

On 22/09/2025 18:54, Sabrina Dubroca wrote:
> 2025-09-22, 10:18:52 +0300, Shahar Shitrit wrote:
>>
>>
>> On 14/09/2025 21:53, Jakub Kicinski wrote:
>>> On Wed, 10 Sep 2025 09:47:40 +0300 Tariq Toukan wrote:
>>>> When a netdev issues an RX async resync request, the TLS module
>>>> increments rcd_delta for each new record that arrives. This tracks
>>>> how far the current record is from the point where synchronization
>>>> was lost.
>>>>
>>>> When rcd_delta reaches its threshold, it indicates that the device
>>>> response is either excessively delayed or unlikely to arrive at all
>>>> (at that point, tcp_sn may have wrapped around, so a match would no
>>>> longer be valid anyway).
>>>>
>>>> Previous patch introduced tls_offload_rx_resync_async_request_cancel()
>>>> to explicitly cancel resync requests when a device response failure
>>>> is detected.
>>>>
>>>> This patch adds a final safeguard: cancel the async resync request when
>>>> rcd_delta crosses its threshold, as reaching this point implies that
>>>> earlier cancellation did not occur.
>>>
>>> Missing a Fixes tag
>> Will add
>>>
>>>> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
>>>> index f672a62a9a52..56c14f1647a4 100644
>>>> --- a/net/tls/tls_device.c
>>>> +++ b/net/tls/tls_device.c
>>>> @@ -721,8 +721,11 @@ tls_device_rx_resync_async(struct tls_offload_resync_async *resync_async,
>>>>  		/* shouldn't get to wraparound:
>>>>  		 * too long in async stage, something bad happened
>>>>  		 */
>>>> -		if (WARN_ON_ONCE(resync_async->rcd_delta == USHRT_MAX))
>>>> +		if (WARN_ON_ONCE(resync_async->rcd_delta == USHRT_MAX)) {
>>>> +			/* cancel resync request */
>>>> +			atomic64_set(&resync_async->req, 0);
>>>
>>> we should probably use the helper added by the previous patch (I'd
>>> probably squash them TBH)
>>
>> It's not trivial to use the helper here, since we don't have the socket.
> 
> tls_device_rx_resync_async doesn't currently get the socket, but it
> has only one caller, tls_device_rx_resync_new_rec, which does. So
> tls_device_rx_resync_async could easily get the socket. Or just pass
> resync_async to tls_offload_rx_resync_async_request_cancel, since
> that's what it really needs?
> 
yes these are options, but we don't like too much passing the socket to
tls_device_rx_resync_new_rec() merely for this matter. Also we wanted to
keep tls_offload_rx_resync_async_request_cancel in the same format of
tls_offload_rx_resync_async_request_start/end meaning to have the socket
as a parameter.