Re: [Qemu-devel] [PATCH v3 2/2] rtl8139: correctly track full receive buffer in standard mode

[Vlad Yasevich <vyasevic@redhat.com>]

On 08/31/2015 05:59 AM, Jason Wang wrote:
> 
> 
> On 08/28/2015 10:06 PM, Vladislav Yasevich wrote:
>> In standard operation mode, when the receive ring buffer
>> is full, the buffer actually appears empty to the driver since
>> the RxBufAddr (the location we wirte new data to) and RxBufPtr
>> (the location guest would stat reading from) are the same.
>> As a result, the call to rtl8139_RxBufferEmpty ends up
>> returning true indicating that the receive buffer is empty.
>> This would result in the next packet overwriting the recevie buffer
>> again and stalling receive operations.
>>
>> This patch tracks the number of unread bytes in the rxbuffer
>> using an unused C+ register.  On every read and write, the
>> number is adjsted and the special case of a full buffer is also
>> trapped.
>>
>> The C+ register trick is used to simplify migration and not require
>> a new machine type.  This register is not used in regular mode
>> and C+ mode doesn't have the same issue.
>>
>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>> ---
>>  hw/net/rtl8139.c | 45 +++++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 41 insertions(+), 4 deletions(-)
> 
> I'm not sure this can happen. For example, looks like the following
> check in rtl8139_do_receive():
> 
>         if (avail != 0 && size + 8 >= avail)
>         {
> 
> can guarantee there's no overwriting?
> 

The problem is the calculation of avail.  When the buffer is full,
avail will be the the size of the receive buffer.  So the test
above will be false because the driver thinks there is actually
enough room.

With his patch, 'avail' will be calculated to 0.

-vlad