Re: Help with testing needed [Was: Re: [PATCH 1/3] ramips: ethernet: ralink: mt7620 jumbo frame support]
> > > > 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. Is it because when you unconditionally enable jumbo frames in switch, you need to prepare the ethernet driver to receive a larger frame, even when mtu is 1500 or smaller? > > > 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. > > Attached patch tested on real MT7620 with internal switch and it is > able to operate with a 2048-bytes frame (included single VLAN tag). Are you ignoring CONFIG_SOC_MT7621 support for this driver? (it is indeed upstream for recent kernels). The patch worked as expected! Thanks a lot! Do you plan to submit it? Tested-by: Luiz Angelo Daros de Luca ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: Help with testing needed [Was: Re: [PATCH 1/3] ramips: ethernet: ralink: mt7620 jumbo frame support]
Once upon a time, Luiz Angelo Daros de Luca 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 ( > 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).
--- a/target/linux/ramips/files/drivers/net/ethernet/ralink/mtk_eth_soc.c
+++ b/target/linux/ramips/files/drivers/net/ethernet/ralink/mtk_eth_soc.c
@@ -42,7 +42,14 @@
#include "mdio.h"
#include "ethtool.h"
+#if defined(CONFIG_SOC_MT7620)
+#define DMA_FWD_REG MT7620A_GDMA1_FWD_CFG
+#define MAX_RX_LENGTH 2048
+#else
+#define DMA_FWD_REG FE_GDMA1_FWD_CFG
#define MAX_RX_LENGTH 1536
+#endif
+
#define FE_RX_ETH_HLEN (VLAN_ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN)
#define FE_RX_HLEN (NET_SKB_PAD + FE_RX_ETH_HLEN + NET_IP_ALIGN)
#define DMA_DUMMY_DESC 0x
@@ -1182,7 +1165,7 @@ void fe_fwd_config(struct fe_priv *priv)
{
u32 fwd_cfg;
- fwd_cfg = fe_r32(FE_GDMA1_FWD_CFG);
+ fwd_cfg = fe_r32(DMA_FWD_REG);
/* disable jumbo frame */
if (priv->flags & FE_FLAG_JUMBO_FRAME)
@@ -1191,19 +1174,19 @@ void fe_fwd_config(struct fe_priv *priv)
/* set
Re: Help with testing needed [Was: Re: [PATCH 1/3] ramips: ethernet: ralink: mt7620 jumbo frame support]
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 ( > 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
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [PATCH] build: target: improve UX of CONFIG_TARGET handling
Hi Petr,
On 29.03.2022 12:55, Petr Štetiar wrote:
Piotr Dymacz [2022-03-29 10:10:00]:
Hi,
It seems I've been misusing 'target' for a long time and now I'm trying to
understand how this leads to the same behavior as 'platform' :)
comments right above that code states following:
# defaults to subtarget if subtarget exists and target does not
# defaults to target otherwise
Should we maybe make 'target' as a valid value?
It's consumed by developers, so I wouldn't waste time and complicate the code
with such band aids for wrong documentation, I would simply fix the
documentation instead.
My suggestion was only about keeping things with reality. Even the
comments above, in the code you mentioned, refer to 'target' and
'subtarget', not 'platform'.
I might be simply wrong here but I believe most of us refer to 'target',
not 'platform' these days. That was the reason for my idea about
'{subtarget_}target', while keeping backward compatibility with the
'platform' but I'm really fine keeping it this way and fixing Wiki.
btw, CONFIG_TARGET=platform|subtarget|env was introduced in2011, in
caf4747f0c.
--
Cheers,
Piotr
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
