Re: [Qemu-devel] [PATCH] net/hub: remove can_receive handler

[Fedorov Sergey <s.fedorov@samsung.com>]

On 10/21/2013 03:44 PM, Fedorov Sergey wrote:
> On 04/23/2013 04:00 PM, Stefan Hajnoczi wrote:
>> On Tue, Apr 23, 2013 at 11:41:42AM +0400, Fedorov Sergey wrote:
>>>> Beyond that, we also want to avoid growing net queues 
>>>> indefinitely.  If
>>>> the hub does not implement .can_receive() then it relies on growing
>>>> queues (keeping packets buffered in memory).
>>> No, net_hub_receive() calls qemu_send_packet(). If the destination
>>> queue cannot receive the packet qemu_net_queue_append() will take
>>> care of queue->nq_maxlen.
>> You are right, sorry.  We do discard packets at nq_maxlen.
>>
>> The problem with ignoring .can_receive() on the hub is that it breaks
>> flow control.  For example, net/tap.c is designed to avoid reading more
>> packets if its peer cannot receive (see tap_can_send()).
>>
>> If the hub claims it can always receive we waste cycles reading packets
>> from the tap device only to discard them.
>>
>> Since qemu.git already has a fix which preserves flow control, I am not
>> going to merge your patch.
>>
>> Stefan
>>
>>
>
> Dear, Stefan Hajnoczi,
>
> After our discussion about this patch I decided to keep my patch in 
> our branch until rebase onto a new release. Recently I have rebased 
> our branch onto v1.5.3 and reverted my patch. Then I face an issue 
> when using user-mode networking with USB network device for mounting 
> root file system through NFS. Fragmented UDP packets from host to 
> guest does not handled properly. Seems that some fragments is lost or 
> somehow stalled. See guest tcpdump log below.
>
> 03:16:52.259690 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto 
> UDP (17), length 164)
>     10.0.2.15.3369105030 > 10.0.2.2.nfs: 136 readdirplus fh 
> Unknown/0100070004001200000000002873593C9B3C43388E23748B0BAD870C00000000 
> 512 bytes @ 0 max 4096 verf 0000000000000000
> 03:16:52.262323 IP (tos 0x0, ttl 64, id 16, offset 0, flags [+], proto 
> UDP (17), length 1500)
>     10.0.2.2.nfs > 10.0.2.15.3369105030: reply ok 1472 readdirplus 
> POST: DIR 40777 ids 0/0 sz 4096 verf 0000000000000000
> 03:16:52.264592 IP (tos 0x0, ttl 64, id 16, offset 1480, flags [+], 
> proto UDP (17), length 1500)
>     10.0.2.2 > 10.0.2.15: udp
> 03:16:54.462961 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto 
> UDP (17), length 164)
>     10.0.2.15.3369105030 > 10.0.2.2.nfs: 136 readdirplus fh 
> Unknown/0100070004001200000000002873593C9B3C43388E23748B0BAD870C00000000 
> 512 bytes @ 0 max 4096 verf 0000000000000000
> 03:16:54.466300 IP (tos 0x0, ttl 64, id 17, offset 0, flags [+], proto 
> UDP (17), length 1500)
>     10.0.2.2.nfs > 10.0.2.15.3369105030: reply ok 1472 readdirplus 
> POST: DIR 40777 ids 0/0 sz 4096 verf 0000000000000000
> 03:16:54.467084 IP (tos 0x0, ttl 64, id 17, offset 1480, flags [+], 
> proto UDP (17), length 1500)
>     10.0.2.2 > 10.0.2.15: udp
> ...
>
> I didn't investigate the cause of the problem in detail. I just reverted
>
> commit 199ee608f0d08510b5c6c37f31a7fbff211d63c4
> Author: Luigi Rizzo <rizzo@iet.unipi.it>
> Date:   Tue Feb 5 17:53:31 2013 +0100
>
>     net: fix qemu_flush_queued_packets() in presence of a hub
>
> And then applied my patch. After that everything works fine for me. 
> See guest tcpdump log below.
>
> 04:45:15.897245 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto 
> UDP (17), length 164)
>     10.0.2.15.3642011847 > 10.0.2.2.nfs: 136 readdirplus fh 
> Unknown/0100070004001200000000002873593C9B3C43388E23748B0BAD870C00000000 
> 512 bytes @ 0 max 4096 verf 0000000000000000
> 04:45:15.899686 IP (tos 0x0, ttl 64, id 15, offset 0, flags [+], proto 
> UDP (17), length 1500)
>     10.0.2.2.nfs > 10.0.2.15.3642011847: reply ok 1472 readdirplus 
> POST: DIR 40777 ids 0/0 sz 4096 verf 0000000000000000
> 04:45:15.906253 IP (tos 0x0, ttl 64, id 15, offset 1480, flags [+], 
> proto UDP (17), length 1500)
>     10.0.2.2 > 10.0.2.15: udp
> 04:45:15.906687 IP (tos 0x0, ttl 64, id 15, offset 2960, flags [none], 
> proto UDP (17), length 240)
>     10.0.2.2 > 10.0.2.15: udp
>
> So there must be something wrong with already applied patch. What 
> could you suggest?
>

Sorry, I missed that Anthony Liguori email address has been changed. So 
I resend the email.

-- 
Best regards,
Sergey Fedorov, Junior Software Engineer,
Samsung R&D Institute Rus.
E-mail: s.fedorov@samsung.com