Re: [PATCH v5 bpf-next 7/7] selftests/bpf: Test bpf_sock_destroy

Aditi Ghag
2023-04-04T00:15:41Z

> On Apr 3, 2023, at 10:35 AM, Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 4/3/23 8:55 AM, Aditi Ghag wrote:
>>> On Mar 31, 2023, at 3:32 PM, Martin KaFai Lau <martin.lau@linux.dev> wrote:
>>>
>>> On 3/30/23 11:46 AM, Stanislav Fomichev wrote:
>>>>> +void test_sock_destroy(void)
>>>>> +{
>>>>> + struct sock_destroy_prog *skel;
>>>>> + int cgroup_fd = 0;
>>>>> +
>>>>> + skel = sock_destroy_prog__open_and_load();
>>>>> + if (!ASSERT_OK_PTR(skel, "skel_open"))
>>>>> + return;
>>>>> +
>>>>> + cgroup_fd = test__join_cgroup("/sock_destroy");
>>>
>>> Please run this test in its own netns also to avoid affecting other tests as much as possible.
>> Is it okay if I defer this nit to a follow-up patch? It's not conflicting with other tests at the moment.
>
> Is it sure it won't affect other tests when running in parallel (test -j)?
> This test is iterating all in the current netns and only checks for port before aborting.
>
> It is easy to add. eg. ~10 lines of codes can be borrowed from prog_tests/mptcp.c which has recently done the same in commit 02d6a057c7be to run the test in its own netns to set a sysctl.

I haven't run the tests in parallel, but the tests are not using hard-coded ports anymore. Anyway I'm willing to run it in a separate netns as a follow-up for additional guards, but do you still think it's a blocker for this patch?