Re: Marvell 88E6141 DSA degrades after 7fb5a711545d7d25fe9726a9ad277474dd83bd06

Re: Marvell 88E6141 DSA degrades after 7fb5a711545d7d25fe9726a9ad277474dd83bd06

[Andrew Lunn]

On Wed, Oct 02, 2019 at 11:57:30AM +0000, Denis Odintsov wrote:
> Hello,
> 
> Hope you are doing fine, I have a report regarding Marvell DSA after 7fb5a711545d7d25fe9726a9ad277474dd83bd06<https://github.com/torvalds/linux/commit/7fb5a711545d7d25fe9726a9ad277474dd83bd06> patch.
> 
> Thing is that after this commit:
> https://github.com/torvalds/linux/commit/7fb5a711545d7d25fe9726a9ad277474dd83bd06
> on linux 5.3 DSA stopped working properly for me.
> I'm using Clearfog GT 8k board, with 88E6141 switch and bridge config where all lanN interfaces are bridged together and ip is assigned to the bridge.
> 
> It stopped working properly in the matter that everything fires up from the board point of view, interfaces are there, all is good, but there are never any packet registered as RX on lanN interfaces in counters. Packets are always TX'ed and 0 as RX. But! This is where weird starts, the actual link is negotiated fine (I have 100Mb clients, and interfaces have correct speed and duplex, meaning they actually handshake with the other end). Even more, if I would set ip lanN interface itself with ip address, the networks somehow work, meaning a client, if set ip manually, can kind of ping the router, but with huge volatile times, like >300ms round trip. And still not a single RX packet on the interface shown in the counter.
> 
> So this is really weird behaviour, and the most sad part in that is that while on 5.3 with this patch reverted everything start to work fine, the trick doesn't work for 5.4 anymore.

Hi Denis

Could you give us the call stack when mv88e6xxx_adjust_link() is used
in 5.3. A WARN_ON(1) should do that.

We are probably missing a use case where it is used, but we did not
expect it to be used. The call stack should help us find that use
case.

Thanks
	Andrew

Re: Marvell 88E6141 DSA degrades after 7fb5a711545d7d25fe9726a9ad277474dd83bd06

[Denis Odintsov]

Hello,

Thank you for your reply, now that you've said that I actually put WARN_ON(1) into mv88e6xxx_adjust_link and found out that it is not actually being called. Not even on 5.3. What I saw was a warning produced by block like "if (ds->ops->adjust_link)" in net/dsa/ code, but not the actual call. My bad. So it seems the content of the function is irrelevant, and as I can see there are many block like this, so most probably it is something one of these blocks were doing on 5.3 which changed to 5.4, which is way harder to debug I guess. Any other things I could check in that matter?

Denis. 

> Am 02.10.2019 um 14:19 schrieb Andrew Lunn <andrew@lunn.ch>:
> 
> On Wed, Oct 02, 2019 at 11:57:30AM +0000, Denis Odintsov wrote:
>> Hello,
>> 
>> Hope you are doing fine, I have a report regarding Marvell DSA after 7fb5a711545d7d25fe9726a9ad277474dd83bd06<https://github.com/torvalds/linux/commit/7fb5a711545d7d25fe9726a9ad277474dd83bd06> patch.
>> 
>> Thing is that after this commit:
>> https://github.com/torvalds/linux/commit/7fb5a711545d7d25fe9726a9ad277474dd83bd06
>> on linux 5.3 DSA stopped working properly for me.
>> I'm using Clearfog GT 8k board, with 88E6141 switch and bridge config where all lanN interfaces are bridged together and ip is assigned to the bridge.
>> 
>> It stopped working properly in the matter that everything fires up from the board point of view, interfaces are there, all is good, but there are never any packet registered as RX on lanN interfaces in counters. Packets are always TX'ed and 0 as RX. But! This is where weird starts, the actual link is negotiated fine (I have 100Mb clients, and interfaces have correct speed and duplex, meaning they actually handshake with the other end). Even more, if I would set ip lanN interface itself with ip address, the networks somehow work, meaning a client, if set ip manually, can kind of ping the router, but with huge volatile times, like >300ms round trip. And still not a single RX packet on the interface shown in the counter.
>> 
>> So this is really weird behaviour, and the most sad part in that is that while on 5.3 with this patch reverted everything start to work fine, the trick doesn't work for 5.4 anymore.
> 
> Hi Denis
> 
> Could you give us the call stack when mv88e6xxx_adjust_link() is used
> in 5.3. A WARN_ON(1) should do that.
> 
> We are probably missing a use case where it is used, but we did not
> expect it to be used. The call stack should help us find that use
> case.
> 
> Thanks
> 	Andrew

Re: Marvell 88E6141 DSA degrades after 7fb5a711545d7d25fe9726a9ad277474dd83bd06

[Andrew Lunn]

On Thu, Oct 03, 2019 at 08:52:34AM +0000, Denis Odintsov wrote:
> Hello,
> 
> Thank you for your reply, now that you've said that I actually put WARN_ON(1) into mv88e6xxx_adjust_link and found out that it is not actually being called. Not even on 5.3. What I saw was a warning produced by block like "if (ds->ops->adjust_link)" in net/dsa/ code, but not the actual call. My bad. So it seems the content of the function is irrelevant, and as I can see there are many block like this, so most probably it is something one of these blocks were doing on 5.3 which changed to 5.4, which is way harder to debug I guess. Any other things I could check in that matter?
> 
> Denis. 

Hi Danis

Please don't top post. And wrap your emails to around 75 characters.

How did you decide on 7fb5a711545d7d25fe9726a9ad277474dd83bd06? Did
you do a git bisect?

    Andrew

Re: Marvell 88E6141 DSA degrades after 7fb5a711545d7d25fe9726a9ad277474dd83bd06

[Denis Odintsov]

> Am 03.10.2019 um 14:13 schrieb Andrew Lunn <andrew@lunn.ch>:
> 
> On Thu, Oct 03, 2019 at 08:52:34AM +0000, Denis Odintsov wrote:
>> Hello,
>> 
>> Thank you for your reply, now that you've said that I actually put WARN_ON(1) into mv88e6xxx_adjust_link and found out that it is not actually being called. Not even on 5.3. What I saw was a warning produced by block like "if (ds->ops->adjust_link)" in net/dsa/ code, but not the actual call. My bad. So it seems the content of the function is irrelevant, and as I can see there are many block like this, so most probably it is something one of these blocks were doing on 5.3 which changed to 5.4, which is way harder to debug I guess. Any other things I could check in that matter?
>> 
>> Denis. 
> 
> Hi Danis
> 
> Please don't top post. And wrap your emails to around 75 characters.
> 
> How did you decide on 7fb5a711545d7d25fe9726a9ad277474dd83bd06? Did
> you do a git bisect?
> 
>    Andrew

Hi,

My approach after I've found the DSA no longer work properly on 5.3.0-rc1 that time was to compare dmesg of 5.2 and 5.3, where I did found the difference to be this line coming after DSA init:
[    2.812111] mv88e6085 f412a200.mdio-mii:04: Using legacy PHYLIB callbacks. Please migrate to PHYLINK!
Present on 5.2 and info about phylink being there on 5.3. Looking into where it come from I've found it to be net/dsa/port.c in function dsa_port_link_register_of in case adjust_link defined. I've searched to what happened to adjust_link function in mv88e6xxx code between 5.2 and 5.3 and immediately hit 7fb5a711545d7d25fe9726a9ad277474dd83bd06. With that commit reverted everything worked fine again, so I assumed this is some part that is not yet fully done for mv88e6xxx or my 88E6141 specifically and is going to be done later, so I just used 5.3 with that patch reverted.

PS. Sorry if I write something wrong, this is first time I communicate with kernel development so close, thank you for your patience.

Denis