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

[Luiz Angelo Daros de Luca]

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:

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?

c) MAX_RX_JUMBO and MAX_RX_PKT_LEN in GSW_REG_GMACCR are switch regs
while I touched only ethernet frame engine regs. 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 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.

Regards,

Luiz