Re: Re: [PATCH bpf-next] bpf: Allow error injection for update_socket_protocol

Re: Re: [PATCH bpf-next] bpf: Allow error injection for update_socket_protocol

[Gang Yan]

Hi Alexei:
It's my honor to recieve your reply. The response to your concerns is attached below 
for your review.
On Mon, Aug 26, 2024 at 10:57:12AM +0800, Gang Yan wrote:
> On Thu, Aug 22, 2024 at 8:33 AM Jakub Kicinski wrote:
> >
> > On Thu, 22 Aug 2024 14:08:57 +0800 Gang Yan wrote:
> > > diff --git a/net/socket.c b/net/socket.c
> > > index fcbdd5bc47ac..63ce1caf75eb 100644
> > > --- a/net/socket.c
> > > +++ b/net/socket.c
> > > @@ -1695,6 +1695,7 @@ __weak noinline int update_socket_protocol(int family, int type, int protocol)
> > > {
> > > return protocol;
> > > }
> > > +ALLOW_ERROR_INJECTION(update_socket_protocol, ERRNO);
> >
> > IDK if this falls under BPF or directly net, but could you explain
> > what test will use this? I'd prefer not to add test hooks into the
> > kernel unless they are exercised by in-tree tests.

> This looks unnecessary.
> update_socket_protocol is already registered as fmodret.
> There is even selftest that excises this feature:
> tools/testing/selftests/bpf/progs/mptcpify.c
> 
> It doesn't need to be part of the error-inject.

The 'update_socket_protocol' is a BPF interface designed primarily to
fix the socket protocol from TCP protocol to MPTCP protocol without
requiring modifications to user-space application codes. However,
when attempting to achieve this using the BCC tool in user-space,
the BCC tool doesn't support 'fmod_ret'. Therefore, this patch aims to 
further expand capabilities, enabling the 'kprobe' method can overriding 
the update_socket_protocol interface.

As a Python developer, the BCC tool is a commonly utilized instrument for 
interacting with the kernel. If the kernel could permit the use of an 
error-inject method to modify the `update_socket_protocol`, it would significantly 
benefit the subsequent promotion and development of MPTCP applications. 
Thank you for considering this enhancement.

Best wishes!
Gang Yan

Re: [PATCH bpf-next] bpf: Allow error injection for update_socket_protocol

[Yonghong Song]

On 8/25/24 8:29 PM, Gang Yan wrote:
> Hi Alexei:
> It's my honor to recieve your reply. The response to your concerns is attached below
> for your review.
> On Mon, Aug 26, 2024 at 10:57:12AM +0800, Gang Yan wrote:
>> On Thu, Aug 22, 2024 at 8:33 AM Jakub Kicinski wrote:
>>> On Thu, 22 Aug 2024 14:08:57 +0800 Gang Yan wrote:
>>>> diff --git a/net/socket.c b/net/socket.c
>>>> index fcbdd5bc47ac..63ce1caf75eb 100644
>>>> --- a/net/socket.c
>>>> +++ b/net/socket.c
>>>> @@ -1695,6 +1695,7 @@ __weak noinline int update_socket_protocol(int family, int type, int protocol)
>>>> {
>>>> return protocol;
>>>> }
>>>> +ALLOW_ERROR_INJECTION(update_socket_protocol, ERRNO);
>>> IDK if this falls under BPF or directly net, but could you explain
>>> what test will use this? I'd prefer not to add test hooks into the
>>> kernel unless they are exercised by in-tree tests.
>> This looks unnecessary.
>> update_socket_protocol is already registered as fmodret.
>> There is even selftest that excises this feature:
>> tools/testing/selftests/bpf/progs/mptcpify.c
>>
>> It doesn't need to be part of the error-inject.
> The 'update_socket_protocol' is a BPF interface designed primarily to
> fix the socket protocol from TCP protocol to MPTCP protocol without
> requiring modifications to user-space application codes. However,
> when attempting to achieve this using the BCC tool in user-space,
> the BCC tool doesn't support 'fmod_ret'. Therefore, this patch aims to
> further expand capabilities, enabling the 'kprobe' method can overriding
> the update_socket_protocol interface.

Gang Yan, could you explore to add fmod_ret support in bcc? It should be
similar to kfunc/kretfunc support. I am happy to review your patches.

Thanks,
Yonghong

>
> As a Python developer, the BCC tool is a commonly utilized instrument for
> interacting with the kernel. If the kernel could permit the use of an
> error-inject method to modify the `update_socket_protocol`, it would significantly
> benefit the subsequent promotion and development of MPTCP applications.
> Thank you for considering this enhancement.
>
> Best wishes!
> Gang Yan
>
>

Re: [PATCH bpf-next] bpf: Allow error injection for update_socket_protocol

[Geliang Tang]

Hi Yonghong,

On Sun, 2024-08-25 at 21:05 -0700, Yonghong Song wrote:
> 
> On 8/25/24 8:29 PM, Gang Yan wrote:
> > Hi Alexei:
> > It's my honor to recieve your reply. The response to your concerns
> > is attached below
> > for your review.
> > On Mon, Aug 26, 2024 at 10:57:12AM +0800, Gang Yan wrote:
> > > On Thu, Aug 22, 2024 at 8:33 AM Jakub Kicinski wrote:
> > > > On Thu, 22 Aug 2024 14:08:57 +0800 Gang Yan wrote:
> > > > > diff --git a/net/socket.c b/net/socket.c
> > > > > index fcbdd5bc47ac..63ce1caf75eb 100644
> > > > > --- a/net/socket.c
> > > > > +++ b/net/socket.c
> > > > > @@ -1695,6 +1695,7 @@ __weak noinline int
> > > > > update_socket_protocol(int family, int type, int protocol)
> > > > > {
> > > > > return protocol;
> > > > > }
> > > > > +ALLOW_ERROR_INJECTION(update_socket_protocol, ERRNO);
> > > > IDK if this falls under BPF or directly net, but could you
> > > > explain
> > > > what test will use this? I'd prefer not to add test hooks into
> > > > the
> > > > kernel unless they are exercised by in-tree tests.
> > > This looks unnecessary.
> > > update_socket_protocol is already registered as fmodret.
> > > There is even selftest that excises this feature:
> > > tools/testing/selftests/bpf/progs/mptcpify.c
> > > 
> > > It doesn't need to be part of the error-inject.
> > The 'update_socket_protocol' is a BPF interface designed primarily
> > to
> > fix the socket protocol from TCP protocol to MPTCP protocol without
> > requiring modifications to user-space application codes. However,
> > when attempting to achieve this using the BCC tool in user-space,
> > the BCC tool doesn't support 'fmod_ret'. Therefore, this patch aims
> > to
> > further expand capabilities, enabling the 'kprobe' method can
> > overriding
> > the update_socket_protocol interface.
> 
> Gang Yan, could you explore to add fmod_ret support in bcc? It should
> be
> similar to kfunc/kretfunc support. I am happy to review your patches.

It took us some time to add this support in bcc, and we have recently
completed it in [1]. We would be grateful if you could help review
these patches.

Thanks,
-Geliang

[1]
https://github.com/iovisor/bcc/pull/5274

> 
> Thanks,
> Yonghong
> 
> > 
> > As a Python developer, the BCC tool is a commonly utilized
> > instrument for
> > interacting with the kernel. If the kernel could permit the use of
> > an
> > error-inject method to modify the `update_socket_protocol`, it
> > would significantly
> > benefit the subsequent promotion and development of MPTCP
> > applications.
> > Thank you for considering this enhancement.
> > 
> > Best wishes!
> > Gang Yan
> > 
> > 
> 

Re: [PATCH bpf-next] bpf: Allow error injection for update_socket_protocol

[Yonghong Song]

On 4/9/25 2:17 AM, Geliang Tang wrote:
> Hi Yonghong,
>
> On Sun, 2024-08-25 at 21:05 -0700, Yonghong Song wrote:
>> On 8/25/24 8:29 PM, Gang Yan wrote:
>>> Hi Alexei:
>>> It's my honor to recieve your reply. The response to your concerns
>>> is attached below
>>> for your review.
>>> On Mon, Aug 26, 2024 at 10:57:12AM +0800, Gang Yan wrote:
>>>> On Thu, Aug 22, 2024 at 8:33 AM Jakub Kicinski wrote:
>>>>> On Thu, 22 Aug 2024 14:08:57 +0800 Gang Yan wrote:
>>>>>> diff --git a/net/socket.c b/net/socket.c
>>>>>> index fcbdd5bc47ac..63ce1caf75eb 100644
>>>>>> --- a/net/socket.c
>>>>>> +++ b/net/socket.c
>>>>>> @@ -1695,6 +1695,7 @@ __weak noinline int
>>>>>> update_socket_protocol(int family, int type, int protocol)
>>>>>> {
>>>>>> return protocol;
>>>>>> }
>>>>>> +ALLOW_ERROR_INJECTION(update_socket_protocol, ERRNO);
>>>>> IDK if this falls under BPF or directly net, but could you
>>>>> explain
>>>>> what test will use this? I'd prefer not to add test hooks into
>>>>> the
>>>>> kernel unless they are exercised by in-tree tests.
>>>> This looks unnecessary.
>>>> update_socket_protocol is already registered as fmodret.
>>>> There is even selftest that excises this feature:
>>>> tools/testing/selftests/bpf/progs/mptcpify.c
>>>>
>>>> It doesn't need to be part of the error-inject.
>>> The 'update_socket_protocol' is a BPF interface designed primarily
>>> to
>>> fix the socket protocol from TCP protocol to MPTCP protocol without
>>> requiring modifications to user-space application codes. However,
>>> when attempting to achieve this using the BCC tool in user-space,
>>> the BCC tool doesn't support 'fmod_ret'. Therefore, this patch aims
>>> to
>>> further expand capabilities, enabling the 'kprobe' method can
>>> overriding
>>> the update_socket_protocol interface.
>> Gang Yan, could you explore to add fmod_ret support in bcc? It should
>> be
>> similar to kfunc/kretfunc support. I am happy to review your patches.
> It took us some time to add this support in bcc, and we have recently
> completed it in [1]. We would be grateful if you could help review
> these patches.

Sounds good. I will take a look.

>
> Thanks,
> -Geliang
>
> [1]
> https://github.com/iovisor/bcc/pull/5274
>
>> Thanks,
>> Yonghong
>>
>>> As a Python developer, the BCC tool is a commonly utilized
>>> instrument for
>>> interacting with the kernel. If the kernel could permit the use of
>>> an
>>> error-inject method to modify the `update_socket_protocol`, it
>>> would significantly
>>> benefit the subsequent promotion and development of MPTCP
>>> applications.
>>> Thank you for considering this enhancement.
>>>
>>> Best wishes!
>>> Gang Yan
>>>
>>>
>