qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] fix the return value of rtl8139_can_receive()


From: Wen Congyang
Subject: Re: [Qemu-devel] [PATCH] fix the return value of rtl8139_can_receive()
Date: Fri, 17 Jun 2011 09:49:43 +0800
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100413 Fedora/3.0.4-2.fc13 Thunderbird/3.0.4

At 06/16/2011 06:39 PM, Kevin Wolf Write:
> Am 16.06.2011 10:23, schrieb Wen Congyang:
>> If rtl8139_can_receive() returns 1, it means that the nic can receive packet,
>> otherwise, it means the nic can not receive packet.
>>
>> If !s->clock_enabled or !rtl8139_receiver_enabled(s), it means that the nic
>> can not receive packet. So the return value should be 0, not 1.
>>
>> Signed-off-by: Wen Congyang <address@hidden>
>>
>> ---
>>  hw/rtl8139.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
>> index 2f8db58..9084678 100644
>> --- a/hw/rtl8139.c
>> +++ b/hw/rtl8139.c
>> @@ -810,9 +810,9 @@ static int rtl8139_can_receive(VLANClientState *nc)
>>  
>>      /* Receive (drop) packets if card is disabled.  */
>>      if (!s->clock_enabled)
>> -      return 1;
>> +      return 0;
>>      if (!rtl8139_receiver_enabled(s))
>> -      return 1;
>> +      return 0;
>>  
>>      if (rtl8139_cp_receiver_enabled(s)) {
>>          /* ??? Flow control not implemented in c+ mode.
> 
> NACK.
> 
> The old behaviour is clearly intentional. IIRC, can_receive() returning
> 0 means that the packet is kept in a queue and qemu tries to deliver it
> later. For a disabled receiver, what I would expect is that it should
> just drop the packets. This is what this code does by returning 1 in
> can_receive() and then return -1 without processing the packet in receive().

Thanks for your detailed explanation.
I know why can_receive() returns 1 now.

> 
> That said, e1000 has a check for (s->mac_reg[RCTL] & E1000_RCTL_EN) in
> can_receive. Should it be changed or is there a reason behind it? If

This check is introduced in commit 4105de67 by Anthony Liguori.
He may know the reason.

> there is, we may as well change rtl8139, but it definitely needs a
> better justification.
> 
> Kevin
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]