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.
