Hi Hangbin Liu,
On 13/02/2023 05:10, Hangbin Liu wrote:
> On Fri, Feb 10, 2023 at 05:22:24PM +0100, Matthieu Baerts wrote:
>> Hi Hangbin Liu,
>>
>> On 10/02/2023 10:32, Hangbin Liu wrote:
>>> Some distros may not enable mptcp by default. Enable it before start the
>>> mptcp server. To use the {read/write}_int_sysctl() functions, I moved
>>> them to test_progs.c
>>>
>>> Fixes: 8039d353217c ("selftests/bpf: Add MPTCP test base")
>>> Signed-off-by: Hangbin Liu <liuhangbin@xxxxxxxxx>
>>> ---
>>> .../testing/selftests/bpf/prog_tests/mptcp.c | 15 ++++++-
>>
>> Thank you for the patch!
>>
>> The modifications linked to MPTCP look good to me.
>>
>> But I don't think it is needed here: I maybe didn't look properly at
>> 'bpf/test_progs.c' file but I think each program from 'prog_tests'
>> directory is executed in a dedicated netns, no?
>>
>> I don't have an environment ready to validate that but if yes, it means
>> that on a "vanilla" kernel, net.mptcp.enabled sysctl knob should be set
>> to 1. In this case, this modification would be specific to these distros
>> patching MPTCP code to disable it by default. It might then be better to
>> add this patch next to the one disabling MPTCP by default, no? (or
>> revert it to have MPTCP available by default for the applications asking
>> for it :) )
>
> I think this issue looks like the rp_filter setting. The default rp_filter is 0.
> But many distros set it to 1 for safety reason. Thus there are some fixes for
> the rp_filter setting like this one. e.g.
I think we should not compare net.mptcp.enabled with rp_filter because
the latter is a bit particular: the initial value of rp_filter in a
newly created netns is inherited from the "host" (the initial netns). In
this case, it is difficult to predict the value of rp_filter in a new
netns and it makes sense to force it to the expected value.
Here for net.mptcp.enabled, the value should be 1 in a newly created
netns. If not, it means the kernel has been modified. (It was making
sense to disable it when MPTCP was backported to older kernels and
marked as "Tech Preview". Now, I don't think we should recommend distros
to do that and support such modifications in the upstream kernel :) )
But again, I'm not totally against that, I'm just saying that if these
tests are executed in dedicated netns, this modification is not needed
when using a vanilla kernel ;-)
Except if I misunderstood and these tests are not executed in dedicated
netns?
(Note: to reduce the size of the patch, if these tests are correctly
executed in dedicated netns, then you can simply force net.mptcp.enabled
to be set to 1, no need to reset it to the previous value.)
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
