Re: [PATCH v2 1/2] bpf-next: Introduced to support the ULP to get or set sockets

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

On 2/14/25 1:20 PM, Jakub Kicinski wrote:
> On Thu, 13 Feb 2025 22:23:39 -0800 Martin KaFai Lau wrote:
>> On 2/13/25 6:13 PM, kernel test robot wrote:
>>> [ 71.196846][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993)
>>> [ 71.196856][ T3759] ? __schedule (kernel/sched/core.c:5380)
>>> [ 71.196866][ T3759] __mutex_lock (kernel/locking/mutex.c:587 kernel/locking/mutex.c:730)
>>> [ 71.196872][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993)
>>> [ 71.196878][ T3759] ? rcu_read_unlock (include/linux/rcupdate.h:335)
>>> [ 71.196885][ T3759] ? mark_held_locks (kernel/locking/lockdep.c:4323)
>>> [ 71.196889][ T3759] ? lock_sock_nested (net/core/sock.c:3653)
>>> [ 71.196898][ T3759] mutex_lock_nested (kernel/locking/mutex.c:783)
>>
>> This is probably because __tcp_set_ulp is now under the rcu_read_lock() in patch 1.
>>
>> Even fixing patch 1 will not be enough. The bpf cgrp prog (e.g. sockops) cannot
>> sleep now, so it still cannot call bpf_setsockopt(TCP_ULP, "tls") which will
>> take a mutex. This is a blocker :(
> 
> Oh, kbuild bot was nice enough to CC netdev, it wasn't CCed on
> the submission.

Ah. I also didn't notice netdev was not cc-ed. will pay attention in the future.

> 
> I'd really rather we didn't allow setting ULP from BPF unless there
> is a strong and clear use case. The ULP configuration and stacking
> is a source of many bugs. And the use case here AFAIU is to allow
> attaching some ULP from an OOT module to a socket, which I think
> won't make core BPF folks happy either, right?

If the in-tree ulp does not work, there is little reason to do it for the 
out-of-tree module only.

My question on the ulp use case went to silence in v1, so we can assume it is 
out-of-tree ulp only. I also asked to replace the "smc" ulp testing with a more 
real "tls" ulp testing to see how it goes first. It does not work as the bot 
reported it.