netdev - Re: [PATCH bpf-next 0/5] fix test

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f9f46ed7-43ae-4421-4546-ffb09fabd1fe@gmail.com>
Date:   Mon, 21 May 2018 10:08:28 -0700
From:   John Fastabend <john.fastabend@...il.com>
To:     Prashant Bhole <bhole_prashant_q7@....ntt.co.jp>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Shuah Khan <shuah@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next 0/5] fix test_sockmap

On 05/20/2018 10:13 PM, Prashant Bhole wrote:
> 
> 
> On 5/19/2018 1:42 AM, John Fastabend wrote:
>> On 05/18/2018 12:17 AM, Prashant Bhole wrote:
>>> This series fixes bugs in test_sockmap code. They weren't caught
>>> previously because failure in RX/TX thread was not notified to the
>>> main thread.
>>>
>>> Also fixed data verification logic and slightly improved test output
>>> such that parameters values (cork, apply, start, end) of failed test
>>> can be easily seen.
>>>
>>
>> Great, this was on my list so thanks for taking care of it.
>>
>>> Note: Even after fixing above problems there are issues with tests
>>> which set cork parameter. Tests fail (RX thread timeout) when cork
>>> value is non-zero and overall data sent by TX thread isn't multiples
>>> of cork value.
>>
>>
>> This is expected. When 'cork' is set the sender should only xmit
>> the data when 'cork' bytes are available. If the user doesn't
>> provide the N bytes the data is cork'ed waiting for the bytes and
>> if the socket is closed the state is cleaned up. What these tests
>> are testing is the cleanup path when a user doesn't provide the
>> N bytes. In practice this is used to validate headers and prevent
>> users from sending partial headers. We want to keep these tests because
>> they verify a tear-down path in the code.
> 
> Ok.
> 
>>
>> After your changes do these get reported as failures? If so we
>> need to account for the above in the calculations.
> 
> Yes, cork related test are reported as failures because of RX thread
> timeout.
> 
> So with your above description, I think we need to differentiate cork
> tests with partial data and full data. In partial data test we can have
> something like "timeout_expected" flag. Any other way to fix it?
> 

Adding a flag seems reasonable to me. Lets do this for now. Also I
plan to add more negative tests so we can either use the same
flag or a new one for those cases as well.

Thanks,
John