[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a6f2700b-8b90-4dd7-b8cf-0a061b790b36@bp.renesas.com>
Date: Mon, 17 Jun 2024 15:20:41 +0100
From: Paul Barker <paul.barker.ct@...renesas.com>
To: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
Cc: Sergey Shtylyov <s.shtylyov@....ru>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Biju Das <biju.das.jz@...renesas.com>,
Claudiu Beznea <claudiu.beznea.uj@...renesas.com>,
Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
Mitsuhiro Kimura <mitsuhiro.kimura.kc@...esas.com>, netdev@...r.kernel.org,
linux-renesas-soc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [net-next PATCH 2/2] net: ravb: Fix R-Car RX frame size limit
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 Barker
Download attachment "OpenPGP_0x27F4B3459F002257.asc" of type "application/pgp-keys" (3521 bytes)
Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)
Powered by blists - more mailing lists