Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC)

[Fabien Chouteau <chouteau@adacore.com>]

On 07/18/2013 10:37 PM, Scott Wood wrote:
> On 07/18/2013 04:27:50 AM, Fabien Chouteau wrote:
>> On 07/17/2013 11:02 PM, Scott Wood wrote:
>>> On 07/17/2013 05:17:06 AM, Fabien Chouteau wrote:
>>>> Can you explain a possible error scenario?
>>>
>>> 126 byte packet, no fcb.  rx_padding is 4 for CRC.  Suppose MRBLR is 128.  Wouldn't *size be 2 here?
>>>
>>
>> Yes, at the end of the function, but then rx_padding is 2 as well.
>
> How is rx_padding 2?  rx_init_frame() will set it to 4 (since the packet size is not less than 60).

Yes, rx_padding == 4 at the beginning of fill_rx_bd() but rx_padding
will be 2 at the end.

>> value of "to_write" will be 126:
>>
>> *size = etsec->rx_remaining_data + etsec->rx_padding;
>>       = 126 + 4;
>>       = 130;
>>
>> to_write = MIN(etsec->rx_fcb_size + *size - etsec->rx_padding, etsec->regs[MRBLR].value);
>>          = MIN(0 + 130 - 4, 128);
>>          = MIN(126, 128);
>>          = 126;
>>
>> So we write the packet in the first part of the BD, and there's 2 bytes
>> left in the BD.
>>
>> *size -= to_write;
>>        = 4;
>> bd->length = to_write;
>>            = 126;
>>
>> So *size == etsec->rx_padding (This is expected as the first write
>> operation can only write data and no padding, I will comment this fact)
>>
>> rem = MIN(etsec->regs[MRBLR].value - bd->length, etsec->rx_padding);
>>     = MIN(128 - 126, 4);
>>     = MIN(2, 4);
>>     = 2;
>>
>> We write 2 bytes of padding.
>>
>> etsec->rx_padding -= rem;
>>                    = 2;
>> *size             -= rem;
>>                    = 2;
>> bd->length        += rem;
>>                    = 128;
>>
>> The BD is full, we will have to put the rest of padding in the next one.
>
> What rest of padding?  I thought you said rx_padding was 2 somehow?  If that were true, then it would be zero at the end.
>

Read my description again.

Regards,

-- 
Fabien Chouteau