On Fri, Oct 6, 2017 at 7:00 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 30 September 2017 at 01:17, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Hi Andrey,
>>
>> On 09/18/2017 04:50 PM, Andrey Smirnov wrote:
>>>
>>> Frame truncation length, TRUNC_FL, is determined by the contents of
>>> ENET_FTRL register, so convert the code to use it instead of a
>>> hardcoded constant.
>>>
>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>> Cc: Jason Wang <jasowang@redhat.com>
>>> Cc: qemu-devel@nongnu.org
>>> Cc: qemu-arm@nongnu.org
>>> Cc: yurovsky@gmail.com
>>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>>> ---
>>> hw/net/imx_fec.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
>>> index 767402909d..989c11be5f 100644
>>> --- a/hw/net/imx_fec.c
>>> +++ b/hw/net/imx_fec.c
>>> @@ -1050,8 +1050,8 @@ static ssize_t imx_enet_receive(NetClientState *nc,
>>> const uint8_t *buf,
>>> size += 4;
>>> /* Huge frames are truncted. */
>>> - if (size > ENET_MAX_FRAME_SIZE) {
>>> - size = ENET_MAX_FRAME_SIZE;
>>> + if (size > s->regs[ENET_FTRL]) {
>>> + size = s->regs[ENET_FTRL]; > flags |= ENET_BD_TR |
>>> ENET_BD_LG;
>>> }
>>
>>
>> for this to be ok you need to update imx_enet_write(), such:
>>
>> case ENET_FTRL:
>> - s->regs[index] = value & 0x00003fff;
>> + value &= 0x00003fff;
>> + if (value > ENET_MAX_FRAME_SIZE) {
>> + warn_report("%s: guest requested bigger "
>> + "frame size than QEMU supports "
>> + "(%u > %u)", value,
>> + ENET_MAX_FRAME_SIZE);
>> + value = ENET_MAX_FRAME_SIZE;
>> + }
>> + s->regs[index] = value;
>> break;
>
> Yes, and also an incoming-migration post_load callback that
> fails migration if the value is too large.
>
> It might be simpler to truncate to the smaller of the ENET_FTRL
> register value and ENET_MAX_FRAME_SIZE.
>
> (PS: what is the hardware behaviour if ENET_FTRL is set to
> a larger value than ENET_MAX_FRAME_SIZE ?)
>
I haven't tested it in practice, but I assume that hardware should be
capable of receiving packets of up to ENET_RCR[MAX_FL] bytes big (both
RCR[MAX_FL] and FTRL are 14-bits), since that would allow supporting
networks with jumbo frames.
What if ENET_MAX_FRAME_SIZE is increased to 16K instead? Would that
make all of that additional error checking code unnecessary?
Thanks,
Andrey Smirnov
P.S: And sure, I'll fix the typo in v2.