Date:   Tue, 26 Apr 2022 12:35:59 +0300
From:   Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
To:     Mickaël Salaün <mic@...ikod.net>
CC:     <willemdebruijn.kernel@...il.com>,
        <linux-security-module@...r.kernel.org>, <netdev@...r.kernel.org>,
        <netfilter-devel@...r.kernel.org>, <yusongping@...wei.com>,
        <artem.kuzin@...wei.com>, <anton.sirazetdinov@...wei.com>
Subject: Re: [RFC PATCH v4 10/15] seltest/landlock: add tests for bind() hooks



4/8/2022 7:41 PM, Mickaël Salaün пишет:
> 
> On 06/04/2022 16:12, Konstantin Meskhidze wrote:
>>
>>
>> 4/4/2022 12:44 PM, Mickaël Salaün пишет:
>>>
>>> On 04/04/2022 10:28, Konstantin Meskhidze wrote:
>>>>
>>>>
>>>> 4/1/2022 7:52 PM, Mickaël Salaün пишет:
>>>
>>> [...]
>>>
>>>>>> +static int create_socket(struct __test_metadata *const _metadata)
>>>>>> +{
>>>>>> +
>>>>>> +        int sockfd;
>>>>>> +
>>>>>> +        sockfd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
>>>>>> +        ASSERT_LE(0, sockfd);
>>>>>> +        /* Allows to reuse of local address */
>>>>>> +        ASSERT_EQ(0, setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, 
>>>>>> &one, sizeof(one)));
>>>>>
>>>>> Why is it required?
>>>>
>>>>    Without SO_REUSEADDR there is an error that a socket's port is in 
>>>> use.
>>>
>>> I'm sure there is, but why is this port reused? I think this means 
>>> that there is an issue in the tests and that could hide potential 
>>> issue with the tests (and then with the kernel code). Could you 
>>> investigate and find the problem? This would make these tests reliable.
>>    The next scenario is possible here:
>>    "In order for a network connection to close, both ends have to send 
>> FIN (final) packets, which indicate they will not send any additional 
>> data, and both ends must ACK (acknowledge) each other's FIN packets. 
>> The FIN packets are initiated by the application performing a close(), 
>> a shutdown(), or an exit(). The ACKs are handled by the kernel after 
>> the close() has completed. Because of this, it is possible for the 
>> process to complete before the kernel has released the associated 
>> network resource, and this port cannot be bound to another process 
>> until the kernel has decided that it is done."
>> https://hea-www.harvard.edu/~fine/Tech/addrinuse.html.
>>
>> So in this case we have busy port in network selfttest and one of the 
>> solution is to set SO_REUSEADDR socket option, "which explicitly 
>> allows a process to bind to a port which remains in TIME_WAIT (it 
>> still only allows a single process to be bound to that port). This is 
>> the both the simplest and the most effective option for reducing the 
>> "address already in use" error".
> 
> In know what this option does, but I'm wondering what do you need it for 
> these tests: which specific line requires it and why? Isn't it a side 
> effect of running partial tests? I'm worried that this hides some issues 
> in the tests that may make them flaky.
> 
   I need it cause we have a possibility here that process (launching 
tests) has to wait the kernel's releasing the associated network socket 
after closing it.
> 
>>>
>>> Without removing the need to find this issue, the next series should 
>>> use a network namespace per test, which will confine such issue from 
>>> other tests and the host.
>>
>>    So there are 2 options here:
>>      1. Using SO_REUSEADDR option
>>      2. Using network namespace.
>>
>> I prefer the first option - "the simplest and the most effective one"
> 
> If SO_REUSEADDR is really required (and justified), then it should be 
> used. Either it is required or not, we should use a dedicated network 
> namespace for each test anyway. This enables to not mess with the host 
> and not be impacted by it neither (e.g. if some process already use such 
> ports).
> 
   Ok. I update the code.
> 
>>
>>>
>>> [...]
> .

Powered by blists - more mailing lists