Re: [PATCH bpf 1/3] bpf, sockmap: Use sock_hold() instead of refcount_inc_not_zero() in lookup

[Michal Luczaj]

On 7/25/26 02:00, John Fastabend wrote:
> On Thu, Jul 23, 2026 at 01:33:27PM +0200, Michal Luczaj wrote:
>> psock's hold on the looked up socket isn't dropped until sk_psock_drop() ->
>> queue_rcu_work() -> sk_psock_destroy() runs, which happens only after the
>> entry is unlinked and an RCU grace period elapses. Since the lookup runs
>> under RCU, a non-NULL result guarantees sk_refcnt >= 1:
>> refcount_inc_not_zero() can never fail here. Use sock_hold() instead.
>>
>> Signed-off-by: Michal Luczaj <mhal@xxxxxxx>
>> ---
> 
> Should bpf-next right? this is an optimization not a fix?

Right. Would you rather have it squashed with patch 3 or should I save it
for bpf-next?

>> diff --git a/net/core/sock_map.c b/net/core/sock_map.c
>> index 9efbd8ca7db8..ca49bc7f8687 100644
>> --- a/net/core/sock_map.c
>> +++ b/net/core/sock_map.c
>> @@ -392,8 +392,8 @@ static void *sock_map_lookup(struct bpf_map *map, void *key)
>> 	sk = __sock_map_lookup_elem(map, *(u32 *)key);
>> 	if (!sk)
>> 		return NULL;
>> -	if (sk_is_refcounted(sk) && !refcount_inc_not_zero(&sk->sk_refcnt))
>> -		return NULL;
>> +	if (sk_is_refcounted(sk))
>> +		sock_hold(sk);
>> 	return sk;
>> }
>>
>> @@ -1218,8 +1218,8 @@ static void *sock_hash_lookup(struct bpf_map *map, void *key)
>> 	sk = __sock_hash_lookup_elem(map, key);
>> 	if (!sk)
>> 		return NULL;
>> -	if (sk_is_refcounted(sk) && !refcount_inc_not_zero(&sk->sk_refcnt))
>> -		return NULL;
>> +	if (sk_is_refcounted(sk))
>> +		sock_hold(sk);
>> 	return sk;
>> }