Date:	Thu, 21 Mar 2013 14:00:15 -0400
From:	Willem de Bruijn <willemb@...gle.com>
To:	Daniel Borkmann <dborkman@...hat.com>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: fix psock_fanout selftest hash collision

On Thu, Mar 21, 2013 at 1:47 PM, Daniel Borkmann <dborkman@...hat.com> wrote:
> On 03/21/2013 06:27 PM, Willem de Bruijn wrote:
>>
>> On Thu, Mar 21, 2013 at 2:31 AM, Daniel Borkmann <dborkman@...hat.com>
>> wrote:
>>>
>>> On 03/21/2013 01:07 AM, Willem de Bruijn wrote:
>>>>
>>>> On Wed, Mar 20, 2013 at 1:59 PM, David Miller <davem@...emloft.net>
>>>> wrote:
>>>>>
>>>>>
>>>>> From: David Miller <davem@...emloft.net>
>>>>> Date: Wed, 20 Mar 2013 12:33:44 -0400 (EDT)
>>>>>
>>>>>> From: Willem de Bruijn <willemb@...gle.com>
>>>>>> Date: Wed, 20 Mar 2013 02:42:44 -0400
>>>>>>
>>>>>>> Fix flaky results with PACKET_FANOUT_HASH depending on whether the
>>>>>>> two flows hash into the same packet socket or not.
>>>>>>>
>>>>>>> Also adds tests for PACKET_FANOUT_LB and PACKET_FANOUT_CPU and
>>>>>>> replaces the counting method with a packet ring.
>>>>>>>
>>>>>>> Signed-off-by: Willem de Bruijn <willemb@...gle.com>
>>>>>>
>>>>>>
>>>>>> Applied, thanks.  I'll retest on my sparc64 box later today.
>>>>>
>>>>>
>>>>> Unfortunately, it's still broken there:
>>>>
>>>>
>>>> This looks like a new problem. Now the counters all stay zero.
>>>>
>>>> I am looking into it. I have not been able to reproduce this on my
>>>> x86_64 so far, so just brought a sparc32 up in qemu. Had less luck
>>>> with sparc64, but impressive that it works at all. Come to think of
>>>> it, is this a 64-bit kernel with 32-bit userland? Perhaps that
>>>> affects packet ring memory layout.
>>>
>>>
>>>
>>> That can affect the ring buffer in case of TPACKET_V1, which is default
>>> if not specified otherwise. See Documentation/networking/packet_mmap.txt
>>> +514
>>
>>
>> Thanks, Daniel. In that case, the following should fix it.
>> Unfortunately, I don't have the hardware to verify, but it still
>> passes on my platforms. Let me know if you prefer it as a regular
>> patch instead of inline.
>
>
> I can only tell you about x86_64: [PASS],

Thanks for testing.

>  although two ERRORs:

That is due to a workaround for probing sockets whose rxhashes map
onto different packet sockets in the test. The current approach tries
up to N (is 5) times until we find a pair of connections that map onto
the different queues. These ERRORS are from a previous try that
failed. Warning would be a better name.

But the whole workaround is not very good, as it only reduces flaky
failures, not fix them. Worst case, if the hash is uniformly random,
then for two packet sockets there's a 1/2 chance on every try that the
two connections hit the same socket, so a 1/(2^N) chance that the test
gives up and fails. I haven't found a better solution yet to generate
a pair of sockets that always hash onto the different sockets.

>
> running psock_fanout test
> --------------------
> test: control single socket
> test: control multiple sockets
> test: datapath 0x0
> info: count=0,0, expect=0,0
> info: count=20,0, expect=15,5
> ERROR: incorrect queue lengths
> info: count=20,0, expect=20,5
>
> ERROR: incorrect queue lengths
> info: trying alternate ports (4)
> test: datapath 0x0
> [...]
> OK. All tests passed
> [PASS]
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists