From mboxrd@z Thu Jan 1 00:00:00 1970 From: Prashant Bhole Subject: Re: [PATCH bpf-next 0/5] fix test_sockmap Date: Wed, 23 May 2018 18:44:15 +0900 Message-ID: <67df711c-75c0-b674-e394-148645353a5a@lab.ntt.co.jp> References: <20180518071753.4768-1-bhole_prashant_q7@lab.ntt.co.jp> <9c41f494-fc95-e4d5-067f-e000eedc7d7f@gmail.com> <5e08aa3f-acfb-f6a9-e918-bca2393ea95f@lab.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "David S . Miller" , Shuah Khan , netdev@vger.kernel.org To: John Fastabend , Alexei Starovoitov , Daniel Borkmann Return-path: Received: from tama500.ecl.ntt.co.jp ([129.60.39.148]:33985 "EHLO tama500.ecl.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932251AbeEWJpZ (ORCPT ); Wed, 23 May 2018 05:45:25 -0400 In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 5/22/2018 2:08 AM, John Fastabend wrote: > 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. > John, I worked on this for some time and noticed that the RX-timeout of tests with cork parameter is dependent on various parameters. So we can not set a flag like the way 'drop_expected' flag is set before executing the test. So I decided to write a function which judges all parameters before each test and decides whether a test with cork parameter will timeout or not. Then the conditions in the function became complicated. For example some tests fail if opt->rate < 17 (with some other conditions). Here is 17 is related to FRAGS_PER_SKB. Consider following two examples. ./test_sockmap --cgroup /mnt/cgroup2 -r 16 -i 1 -l 30 -t sendpage --txmsg --txmsg_cork 1024 # RX timeout occurs ./test_sockmap --cgroup /mnt/cgroup2 -r 17 -i 1 -l 30 -t sendpage --txmsg --txmsg_cork 1024 # Success! Do we need to keep such tests? if yes, then I will continue with adding such conditions in the function. -Prashant