Re: [PATCH mptcp-net v4 1/2] mptcp: use sockopt_lock(release)_sock in sockopt

[Paolo Abeni <pabeni@redhat.com>]

On 5/15/26 11:39 AM, gang.yan@linux.dev wrote:
> May 15, 2026 at 5:06 PM, "Paolo Abeni" <pabeni@redhat.com mailto:pabeni@redhat.com?to=%22Paolo%20Abeni%22%20%3Cpabeni%40redhat.com%3E > wrote:
>> On 5/15/26 3:27 AM, gang.yan@linux.dev wrote:
>>>  The subflow locking concern is valid for future BPF support.
>>>  
>>>  That said, the observation about the subflow locking gap is a valid design
>>>  concern. As sashiko pointed out, when the MPTCP setsockopt handler is
>>>  eventually called from BPF context, the MPTCP meta socket (msk) is already
>>>  locked externally, but the subflow sockets (ssk) are not. The internal
>>>  delegation via sk_setsockopt(ssk, ...) or tcp_setsockopt(ssk, ...) calls
>>>  sockopt_lock_sock(ssk) which skips the lock due to the per-thread
>>>  has_current_bpf_ctx() flag, leaving ssk unprotected.
>>>  
>>>  However, solving this is not straightforward. In BPF context we don't have a
>>>  good way to call lock_sock() on the subflow — as sashiko noted in first point,
>>>  lock_sock() unconditionally calls might_sleep(), and will cause 'sleeping in
>>>  atomic context' problems in BPF context. So properly locking the subflow in
>>>  BPF context remains an open question that needs to be addressed when MPTCP
>>>  BPF setsockopt support is actually introduced.
>>>
> Hi Paolo,
> 
> Thanks for your reply.
> 
>> I'm sorry for the late feedback. I think such concern is actually quite
>> relevant. Note that replacing
>>
>>  bool slow = lock_sock_fast(ssk);
>>
>> with:
>>
>>  lock_sock(ssk);
>>
>> does not change much WRT the sleeping issue.
> 
> I'm not questioning your point, but just to confirm — when you mentioned the
> "sleeping issue", are you referring to the BPF context?

Yes.

> In fact, this patch addresses an issue that occurs in non-BPF scenarios like
> sashiko said in [1]:
> 
> https://sashiko.dev/#/patchset/20260420093343.16443-1-gang.yan@linux.dev

I think that bit/chunk should in a separate patch targeting net.

>> I *think* the only viable path in the short term is making the sockopt
>> requiring subflow-level lock fail when called by BPF.
>>
>> In a possible long term alternative could be delegating the
>> subflow-level changes to a workqueue, but that really looks invasive and
>> I would really like to avoid such option.
> 
> I agree that the short-term approach sounds reasonable for now. We could
> either implement the sockopt failure for BPF calls that require subflow-level
> locking, or simply add a comment to mark this limitation clearly.

I suggest avoid the comment, because application/BPF programs will trip
on this. Returning an error will make it clear that is not working and
will prevent racy accesses.

> As far as I can see, MPTCP currently doesn’t have any real-world use cases
> for setting sockopt via BPF anyway. The workqueue approach also seems too complex
> and invasive for the current stage, so I’d prefer to avoid it for now.
> 
> WDYT？

I'm very fine with keeping this as simple as possible.

/P