On 15/06/2024 14:04, Niklas Söderlund wrote:
> Hi Paul,
>
> Thanks for your work.
>
> On 2024-06-15 11:30:38 +0100, Paul Barker wrote:
>> The RX frame size limit should not be based on the current MTU setting.
>> Instead it should be based on the hardware capabilities.
>>
>> Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
>
> I agree with the change the RFLR.RFL setting should not be connected to
> the MTU setting. And this likely comes from the early days of the driver
> where neither Rx or Tx supported multiple descriptors for each packet.
> In those days the single descriptor used for each packet was tied to the
> MTU setting. So likely the fixes tag should point to a later commit?>
If my understanding of MTU & MRU are correct, even with a single
descriptor we can always accept the same number of bytes regardless of
the current MTU.
> This is a great find and shows a flaw in the driver. But limiting the
> size of each descriptor used for Tx is only half the solution right? As
> the driver now supports multiple descriptors for Tx (on GbEth) the
> driver allows setting an MTU larger then the maximum size for single
> descriptor on those devices. But the xmit function of the driver still
> hardcode the maximum of 2 descriptors for each Tx packet. And it only
> uses the two descriptors to align the data to hardware constrains.
>
> Is it not incorrect for the driver to accept an MTU larger then the
> maximum size of a single descriptor with the current Tx implementation?
> The driver can only support larger MTU settings for Rx, but not Tx ATM.
>
> I think the complete fix is to extend ravb_start_xmit() to fully support
> split descriptors for packets larger then the maximum single descriptor
> size. Not just to align the packet between a DT_FSTART and DT_FEND
> descriptor when needed.
For the RZ SoCs I have looked at, this isn't an issue. We support
transmitting frames of slightly over 1500 bytes on the SoCs with GbEth
IP, or 2047 bytes on the SoCs with R-Car AVB IP (e.g. RZ/G2H). Given
that a single descriptor can cover up to 4095 bytes of data (with its 12
bit DS field), we don't need split TX descriptors for either of these.
Thanks,
--
Paul BarkerAttachment:
OpenPGP_0x27F4B3459F002257.asc
Description: OpenPGP public key
Attachment:
OpenPGP_signature.asc
Description: OpenPGP digital signature
