virtio-win / kvm-guest-drivers-windows

Windows paravirtualized drivers for QEMU\KVM
https://www.linux-kvm.org/page/WindowsGuestDrivers
BSD 3-Clause "New" or "Revised" License
2.01k stars 385 forks source link

viosock: fix the recving stuck issue #953

Closed sochinpub closed 1 year ago

sochinpub commented 1 year ago

The RxPktList must be processed in time, or it is very easy to stuck both the Server sending side and the Windows Client recving side. Think about the following scenario:

1) the host server(with CID 2) side continuously send data; 2) the VM windows client side continuously receive data; 3) the server side sent 128(ring num) rw packets, and the client side receives 128(ring num) rw packets, then each packet enqueues one Cb and returns it's buffer; 4) then, the client side postpone all 128 packet to RxPktList, since it can not find free buffers(VIOSockRxCbPop return NULL), meanwhile the host send side can not send any data any more; 5) one client side read thread reads all the 128 packet data, without process the RxPktList;

till now, the client side will not be able to receive data any more, the host server side will not be able to send data any more, and the client side new recv call will block forever.

It is easy to reproduce this stuck by continuously sending data in the server side.

sochinpub commented 1 year ago

Sorry, looks like a workaround that just pops one element and kicks the ring. Can you please elaborate more on the regular exiting flow? If the kick is missing in some place, let's add it appropriately.

Well, till now, the unique two functions that can pop this RxPktList are VIOSockRxVqCleanup() and VIOSockRxPktListProcess(), let's just skip the cleanup func. The unique one function that calls VIOSockRxPktListProcess() is VIOSockRxPktInsertOrPostpone(), then the unique function that calls VIOSockRxPktInsertOrPostpone() is VIOSockRxVqProcess(). Calling VIOSockRxPktInsertOrPostpone() in the end of VIOSockRxVqProcess() is not enough.

This is because we may already do not have free buffers before calling VIOSockRxPktInsertOrPostpone() in VIOSockRxVqProcess(), that can happen easily, for no recv is called from upper layer, thus VIOSockReadDequeueCb() do nothing at all(do not free any buffers), the subsequent VIOSockRxPktInsertOrPostpone() will not call VIOSockRxPktListProcess() either.

Till now, the only way to cope with this situation is calling VIOSockReadProcessDequeueCb() from VIOSockReadSocketState(), which is triggered by a request that enqueues to the socket read queue(pSocket->ReadQueue), the WDF framework will call our registered read callback function(that is VIOSockReadSocketState). Unfortunately, we did not call VIOSockRxPktListProcess()to pop the 'RxPktList ' in VIOSockReadDequeueCb(), which seems to be a must.