Re: Aw: Re: Re: [PATCH v3 0/5] arm: dts: mt7623: relocate gmacs, mt7530 switch, and add port@5

[Arınç ÜNAL]

On 18.02.2023 15:18, Frank Wunderlich wrote:
> Hi,
>
> > Gesendet: Samstag, 18. Februar 2023 um 11:49 Uhr
> > Von: "Arınç ÜNAL" <arinc.unal@xxxxxxxxxx>
> > An: "Frank Wunderlich" <frank-w@xxxxxxxxxxxxxxx>
>
> > On 18.02.2023 13:24, Frank Wunderlich wrote:
> > > Hi,
> > >
> > > finally get some time to bootup with this series on my r2.
> > >
> > > I see that inserting the port@5 as cpu-port maps this as default-cpu because dsa-core uses first found cpu-port as
> > > default (dsa_tree_setup_cpu_ports in dsa.c) and because of lower bandwidth (rgmii instead of trgmii) not the best choice.
> > >
> > > But it look worse...network is currently broken (set both gmacs up).
> > > I see arp-packets reaching remote side, but reponse is not received by r2
> > >
> > > I have tested it on 6.2-rc8 (wan-port), maybe additional patches are needed?...userspace setup should be right.
> > >
> > > so i added series on top of net-next (no additional patches except some basic like build-script,defconfig and such)...same result...
> > >
> > > i'm not sure if i change the mapping from userspace back to eth0, so disabled port@5 in switch, now port6 is
> > > cpu-port again and it works...so something is wrong with port5 of switch or gmac1.
> >
> > That's a driver issue and will be fixed once an accepted version of
> > these patches [0] [1] [2] are applied to net-next. You should have them
> > on your inbox, I specifically told Richard to CC you.
>
> yes, i've got them, but not applied when starting these tests. Not thought, that this change is necessary
> as we use both gmac on mt7531/bpi-r3 with out problems.
>
> with these 3 Patches network-connection works, but only at ~624 Mbit/s in TX-Mode (started iperf3-client on r2 without -R) and massive retransmitts on first run, other direction is clean.

Ok, so according to your tests, traffic through gmac1 is not very good. 
gmac0 should be the default DSA master.

By the way, did you make a bug report of this, by sending a mail to 
netdev mailing list or some other way?

> and these Patches need to be applied first (when fixed up) else network is broken.

That's true.

> > This is devicetree bindings. We're here to describe the hardware. The
> > way a driver works should not affect describing the hardware.
>
> thats right, but it should not break/change behaviour like now all ports have only ~600Mbit because of
> moving them all to the other gmac which has obvious issues.

Makes sense.

> > To address the lower bandwidth situation you mentioned, a devicetree
> > property to designate a CPU port as the default CPU port could be
> > introduced. I'm not aware of a similar conversation so I'll send a mail
> > to netdev to discuss this. Will CC you.
>
> isn't there a way to leave ports by default on the the better gmac (gmac0=trgmii)?
> maybe moving port5 below port6...not nice, but then port6 is the first cpu found.

This could be done but it comes off as an improper way to me.

> or maybe defined the default cpu in driver which gets picked up by core (define port6 if available as default).
> Imho additional dts-propperty is wrong approch...it should be handled by driver. But cpu-port-selection is
> currently done in dsa-core which makes it a bit difficult.
>
> I'm not sure how the multi-cpu support in dsa-core ended and how you use the other gmac not used by dsa-core
>
> set master in userspace-config? i remember you've sent a patch adding callback for it.

You can change the DSA master using iproute2-6.1.0 and above with this 
patch which should be in your inbox as well.

https://lore.kernel.org/netdev/20230211184101.651462-1-richard@xxxxxxxxxxxxxxx/

> imho dts change should be applied if these points are cleared...

The only issue I see here is that, with this patch series, port5 becomes 
the default CPU port which is not preferred for the reasons you explained.

So once we can have port6 to be the default CPU port of the DSA slaves, 
there's no issues left.

Arınç