Help with testing needed [Was: Re: [PATCH 1/3] ramips: ethernet: ralink: mt7620 jumbo frame support]

[Andrey Melnikov]

Once upon a time, Luiz Angelo Daros de Luca <luizluca at gmail.com> wrote:
>
> Hi Andrey,
>
> > It simple a) don't apply to master tree; b) not working - MAX_RX_LENGTH constant not
> > touched, GSW_REG_GMACCR not tweaked for accepting jumbo frames.
>
> a) Sorry, I missed your answer and thanks for the review. Yes, it was
> not applying to master. It was based on another patch deeper in my
> tree. I fixed that a long time ago but I postponed the fix as I didn't
> see any interest from ML (I was wrong). I was focusing on upstream
> patches (realtek dsa switches merged into 5.18) and postponed the
> ramips fixes for when they would be needed.
>
> b) MAX_RX_LENGTH seems to be used only for keeping the buffer at least
> over (MAX_RX_LENGTH - FE_RX_ETH_HLEN) and it does not care when mtu is
> actually even bigger than (MAX_RX_LENGTH - FE_RX_ETH_HLEN). See:

MAX_RX_LENGTH affects buffers for DMA transfers.

> target/linux/ramips/files/drivers/net/ethernet/ralink/mtk_eth_soc.c:
> static inline int fe_max_frag_size(int mtu)
> {
>        /* make sure buf_size will be at least MAX_RX_LENGTH */
>        if (mtu + FE_RX_ETH_HLEN < MAX_RX_LENGTH)
>                mtu = MAX_RX_LENGTH - FE_RX_ETH_HLEN;
>
>        return SKB_DATA_ALIGN(FE_RX_HLEN + mtu) +
>                SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> }
>
> static inline int fe_max_buf_size(int frag_size)
> {
>        int buf_size = frag_size - NET_SKB_PAD - NET_IP_ALIGN -
>                       SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>
>        BUG_ON(buf_size < MAX_RX_LENGTH);
>        return buf_size;
> }
>
> Does this logic need to be updated somehow for jumbo or even non jumbo frames?

No. Only maybe remove BUG_ON().

> c) MAX_RX_JUMBO and MAX_RX_PKT_LEN in GSW_REG_GMACCR are switch regs
> while I touched only ethernet frame engine regs.

There are two parts - FE and switch. And both need to be properly programmed.

> For me, the switch was happily forwarding frames with 8 extra bytes (DSA tag). In my
> device, the internal switch was being used as a transparent switch (no
> vlan), connecting to an external switch (RTL8367S) through the RGMII
> interface. Maybe the small increase was still falling into an accepted
> frame size range (for the switch), the RGMII interface might have a
> less restricted access or disabling vlans also makes the switch more
> tolerant to larger frames. The FastEthernet ports are not used in my
> device so extra bytes might need to come from the RTL8367S DSA switch,
> which currently does not support increasing the MTU of slave ports. I
> cannot easily generate incoming frames larger than 1508.

I have hardware with an internal switch. And your patch is not working for it.

> I wonder if it is the ethernet driver's responsibility to increase the
> switch frame size or if it is the switch driver (swconfig or DSA). It
> might not only affect the CPU port but any traffic between ports. I'm
> not very comfortable touching a piece of code I cannot really test. If
> a dev does have access to a mt7620 device that actually uses its
> ports, it might be easier for that dev to go ahead and fix it himself.
> The patch is as simple as:
>
> #ifdef CONFIG_SOC_MT7620
>    if (<frame size calculated from mtu> > 1536) {
>       // set MAX_RX_JUMBO as 2k
>       GSW_REG_GMACCR |= 0x2 << 2
>       // set MAX_RX_PKT_LEN as MAX_RX_JUMBO
>       GSW_REG_GMACCR |= 0x3
>    }
> #endif

> It might be inserted into the same place fe_max_frag_size is called in
> fe_change_mtu() but I'm not sure how it would interact with other non
> MT7621 SoCs.

> I wonder why MT7621 does not seem to bother the extra bytes. Anyway,
> the patch is a step further and it is still required and enough for an
> external DSA switch.

> The extra GSW_REG_GMACCR tweaks, if needed, might be added in the
> future. I'll wait a couple of days to settle this thread and resubmit
> a master-compatible v2 (just a minor fix you pointed out) with an
> extra patch for disabling checksum offload when the DSA tag is not
> Mediatek.


Attached patch tested on real MT7620 with internal switch and it is
able to operate with a 2048-bytes frame (included single VLAN tag).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 903-001-ramips-ethernet-ralink-mt7620-jumbo-frame-support.patch
Type: text/x-patch
Size: 4441 bytes
Desc: not available
URL: <http://lists.openwrt.org/pipermail/openwrt-devel/attachments/20220331/b6bb075f/attachment.bin>