Re: [PATCH bpf v2 1/2] bpf: Extract shared reqsk-to-listener upgrade

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 11bb0d236822..fede810ef37f 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -7079,6 +7079,28 @@ __bpf_skc_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
>  	return sk;
>  }
>
> +static struct sock *
> +bpf_sk_lookup_full_sk(struct sock *sk)
> +{

This isn't a bug, but since this helper only converts a reqsk to its
listener rather than performing a lookup, would a name outside the
bpf_sk_lookup_* family - say sk_lookup_full_sk() or
bpf_reqsk_to_listener() - read more clearly next to
bpf_sk_lookup_tcp()/bpf_sk_lookup_udp() and struct bpf_sk_lookup?

The bpf_sk_lookup_ prefix in this file already denotes the BPF helper family
(bpf_sk_lookup_tcp(), bpf_sk_lookup_udp(), bpf_sk_lookup_assign(),
bpf_sk_lookup_enabled) and the UAPI context type struct bpf_sk_lookup /
struct bpf_sk_lookup_kern. Sitting about 20 lines above bpf_sk_lookup()
itself, the name reads like another lookup entry point rather than a
reqsk-to-listener conversion.

> +	struct sock *sk2 = sk_to_full_sk(sk);
> +
> +	/*
> +	 * sk_to_full_sk() may return sk->rsk_listener, make sure the original
> +	 * sk sock refcnt is decremented to prevent a request_sock leak.
> +	 */
> +	if (sk2 != sk) {
> +		sock_gen_put(sk);
> +		/* Ensure there is no need to bump sk2 refcnt. */
> +		if (unlikely(sk2 && !sock_flag(sk2, SOCK_RCU_FREE))) {
> +			WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
> +			return NULL;
> +		}
> +		sk = sk2;
> +	}
> +
> +	return sk;
> +}

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30800053349