netdev - Re: gmac1 issues with mtk_eth_soc & port 5 issues with MT7530 DSA driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6249fc14-b38a-c770-36b4-5af6d41c21d3@arinc9.com>
Date:   Wed, 25 Jan 2023 16:21:53 +0300
From:   Arınç ÜNAL <arinc.unal@...nc9.com>
To:     netdev <netdev@...r.kernel.org>, Felix Fietkau <nbd@....name>
Cc:     Lorenzo Bianconi <lorenzo@...nel.org>,
        Sergio Paracuellos <sergio.paracuellos@...il.com>,
        Frank Wunderlich <frank-w@...lic-files.de>,
        erkin.bozoglu@...ont.com
Subject: Re: gmac1 issues with mtk_eth_soc & port 5 issues with MT7530 DSA
 driver

On 23.01.2023 19:19, Arınç ÜNAL wrote:
> After testing stuff out, I've got things to share on these issues.
> 
> All of this is tested on the current net-next/main on GB-PC2 and Unielec 
> U7621-06.
> 
> On 13.09.2022 15:54, Arınç ÜNAL wrote:
>> I'd like to post a few more issues I stumbled upon on mtk_eth_soc and 
>> MT7530 DSA drivers. All of this is tested on vanilla 6.0-rc5 on GB-PC2.
>>
>> ## MT7621 Ethernet gmac1 won’t work when gmac1 is used as DSA master 
>> for MT7530 switch
>>
>> There’s recently been changes on the MT7530 DSA driver by Frank to 
>> support using port 5 as a CPU port.
>>
>> The MT7530 switch’s port 5 is wired to the MT7621 SoC’s gmac1.
>>
>> Master eth1 and slave interfaces initialise fine. Packets are sent out 
>> from eth1 fine but won't be received on eth1.
>>
>> This issue existed before Lorenzo’s changes on 6.0-rc1.
>>
>> I’m not sure if this is an issue with mtk_eth_soc or the MT7530 DSA 
>> driver.
> 
> Traffic from CPU goes out through DSA slave fine. Traffic from DSA slave 
> to CPU reaches, RX bytes go up on eth1, but nothing on tcpdump.
> 
> Recently, I tried this on a Bananapi BPI-R2 (MT7623NI SoC). Everything 
> works fine after setting up eth0, `ip l set up eth0`. It still works 
> after setting down eth0. This makes me believe that, on mtk_eth_soc.c, 
> there is code for opening the first MAC, which is actually needed to 
> make communication work on all MACs. It should be moved to a more 
> generic location where the code will run even when the first MAC is not 
> being used.
> 
> After fiddling with the MediaTek ethernet driver, I found out that gmac1 
> works only when hardware special tag parsing is disabled. This is the 
> case for the MT7621A and MT7623NI SoCs.
> 
> With Felix's commit 2d7605a729062bb554f03c5983d8cfb8c0b42e9c ("net: 
> ethernet: mtk_eth_soc: enable hardware DSA untagging"), hardware special 
> tag parsing is disabled only when at least one MAC does not use DSA.
> 
> If someone could give me code to test where this function is disabled 
> for these two SoCs, I'd appreciate it.
> 
> Currently this works:
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 801deac58bf7..3c462179dcf6 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -3241,6 +3241,7 @@ static int mtk_open(struct net_device *dev)
>   {
>       struct mtk_mac *mac = netdev_priv(dev);
>       struct mtk_eth *eth = mac->hw;
> +    u32 val;
>       int i, err;
> 
>       if (mtk_uses_dsa(dev) && !eth->prog) {
> @@ -3258,15 +3259,15 @@ static int mtk_open(struct net_device *dev)
>               md_dst->u.port_info.port_id = i;
>               eth->dsa_meta[i] = md_dst;
>           }
> -    } else {
> -        /* Hardware special tag parsing needs to be disabled if at least
> -         * one MAC does not use DSA.
> -         */
> -        u32 val = mtk_r32(eth, MTK_CDMP_IG_CTRL);
> -        val &= ~MTK_CDMP_STAG_EN;
> -        mtk_w32(eth, val, MTK_CDMP_IG_CTRL);
>       }
> 
> +    /* Hardware special tag parsing needs to be disabled if at least
> +     * one MAC does not use DSA.
> +     */
> +    val = mtk_r32(eth, MTK_CDMP_IG_CTRL);
> +    val &= ~MTK_CDMP_STAG_EN;
> +    mtk_w32(eth, val, MTK_CDMP_IG_CTRL);
> +
>       err = phylink_of_phy_connect(mac->phylink, mac->of_node, 0);
>       if (err) {
>           netdev_err(dev, "%s: could not attach PHY: %d\n", __func__,

I've done some more tests and I'm confident this is caused by Felix's 
commit which enables hardware DSA untagging.

According to my tests on MT7621AT and MT7623NI SoCs, hardware DSA 
untagging is not supported on the second GMAC.

So I've got to disable this feature when the second gmac of MT7621 or 
MT7623 SoCs is enabled.

The MTK_GMAC1_TRGMII capability is only on the MT7621 and MT7623 SoCs 
which I see this problem on. I'm new to coding so I took an educated 
guess from the use of MTK_NETSYS_V2 to disable this feature altogether 
for MT7986 SoC.

I believe this check does it perfectly well. I tested it on both SoCs 
along with some debug info on different DTs.
- Only gmac0 as dsa master
- gmac0 & gmac1 as dsa master
- Only gmac1 as dsa master
- gmac0 as dsa master & gmac1 as non-dsa

if ((mtk_uses_dsa(dev) && !eth->prog) && !(mac->id == 1 && 
MTK_HAS_CAPS(eth->soc->caps, MTK_GMAC1_TRGMII)))

Initially, I wanted to disable this feature on MT7621 and MT7623 SoCs 
altogether but there's no reason to do this with the check above.

As a last note, I would appreciate it if new features are only enabled 
on SoCs which were thoroughly tested.

Arınç