Re: [PATCH v2 net] tcp/dccp: Don't use timer_pending() in reqsk_queue_unlink().

[Martin KaFai Lau <martin.lau@linux.dev>]

On 10/10/24 10:36 AM, Kuniyuki Iwashima wrote:
> From: Martin KaFai Lau <martin.lau@linux.dev>
> Date: Wed, 9 Oct 2024 22:46:57 -0700
>> On 10/9/24 10:42 AM, Kuniyuki Iwashima wrote:
>>> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
>>> index 2c5632d4fddb..23cff5278a64 100644
>>> --- a/net/ipv4/inet_connection_sock.c
>>> +++ b/net/ipv4/inet_connection_sock.c
>>> @@ -1045,12 +1045,13 @@ static bool reqsk_queue_unlink(struct request_sock *req)
>>>    		found = __sk_nulls_del_node_init_rcu(sk);
>>>    		spin_unlock(lock);
>>>    	}
>>> -	if (timer_pending(&req->rsk_timer) && del_timer_sync(&req->rsk_timer))
>>> -		reqsk_put(req);
>>> +
>>>    	return found;
>>>    }
>>>    
>>> -bool inet_csk_reqsk_queue_drop(struct sock *sk, struct request_sock *req)
>>> +static bool __inet_csk_reqsk_queue_drop(struct sock *sk,
>>> +					struct request_sock *req,
>>> +					bool from_timer)
>>>    {
>>>    	bool unlinked = reqsk_queue_unlink(req);
>>>    
>>> @@ -1058,8 +1059,17 @@ bool inet_csk_reqsk_queue_drop(struct sock *sk, struct request_sock *req)
>>>    		reqsk_queue_removed(&inet_csk(sk)->icsk_accept_queue, req);
>>>    		reqsk_put(req);
>>>    	}
>>> +
>>> +	if (!from_timer && timer_delete_sync(&req->rsk_timer))
>>
>> timer_delete_sync() is now done after the above reqsk_queue_removed().
>> The reqsk_timer_handler() may do the "req->num_timeout++" while the above
>> reqsk_queue_removed() needs to check for req->num_timeout. Would it race?
> 
> Ah thanks!
> I moved it for better @unlinked access, but will move above.
> 
> Btw, do you have any hint why the connection was processed on a different
> cpu, not one where reqsk timer was pinned ?

Just saw this after replying on v1. I don't know what exactly caused this. I am 
only aware we have a recent steering test to test different packet steering setup.

[ I had some email client issues, so the reply ordering has been wrong :( ]