From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B629AC43387 for ; Thu, 17 Jan 2019 11:37:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 588192054F for ; Thu, 17 Jan 2019 11:37:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=vincit.fi header.i=@vincit.fi header.b="LQ1gnwPa" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727806AbfAQLh2 (ORCPT ); Thu, 17 Jan 2019 06:37:28 -0500 Received: from mail-lj1-f195.google.com ([209.85.208.195]:41071 "EHLO mail-lj1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725951AbfAQLh1 (ORCPT ); Thu, 17 Jan 2019 06:37:27 -0500 Received: by mail-lj1-f195.google.com with SMTP id k15-v6so8277530ljc.8 for ; Thu, 17 Jan 2019 03:37:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vincit.fi; s=ticniv; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=ZjzOiCZO7JJvsbhGlfvWo4zWOEFcnEMjeZMn07oocME=; b=LQ1gnwPaArH1JsxsK3UgTQiEfrRTVjpErVup68zCApLNYHvLsUdUPj/W2gvE1ojYEP yyAHe52m+LyKXAhIO1EeiGfVcvW5xrFQdUU2SEzmeJTUg9bmqAccxANPHM84MiMSoKRw Ba6XdfQ5+wCled5WHGq6YEw7vHb2bjWiv77Gk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=ZjzOiCZO7JJvsbhGlfvWo4zWOEFcnEMjeZMn07oocME=; b=PhjCp1/zpCyW6pboeqMLYnJDgywYTqhbjTH1e0f+bmeKLup3xMvUHk6kPy8s6Emx7f RrV7zEmLXMhH/UNb4igTWfrhdLCdMByOPBI1bCXyEnld3+IEbzM/t0/g/Mz3Qrvvk4Tj Mi506Rhdpyj40bjNLgE3s+jpZ3A2C8reuQ5wyKRqvLtKfNWz7HoKjVdN75uiJr2h1fdF E4vhOXHX4LxEWPSdss/SOJDT8aPD9AKiPjE2BAZDtCrZyaUdRyL5UpglHirClL+sSUaZ iPlVcVDFx83JMQ21yBYpo/dQ9LDreZl1ft1i3dSHG0WJ4a5zLpHOtKMPIKsORLiH5EWe tqGA== X-Gm-Message-State: AJcUukdkfYt+8U8QgdXYoBkXAr75h/dl4JJXJDqLRF4VL5aYu8S2mIEY TJtiALAXyNGWge7V8xI/yU2Rq3QGYAs= X-Google-Smtp-Source: ALg8bN7E/A0ZyrtYR/RSKLmh4cWCTcNa70iZuIoUwOvq4bBUVdR28zG58Y8mHlmhA1Xw1p7jhgE9eQ== X-Received: by 2002:a2e:2d11:: with SMTP id t17-v6mr9421657ljt.159.1547725045448; Thu, 17 Jan 2019 03:37:25 -0800 (PST) Received: from samu-ThinkPad-T480s (gw.vincit.cloud. [195.60.252.249]) by smtp.gmail.com with ESMTPSA id s24sm241788lfc.30.2019.01.17.03.37.24 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 17 Jan 2019 03:37:24 -0800 (PST) Date: Thu, 17 Jan 2019 13:37:22 +0200 From: Samu Nuutamo To: Andrew Lunn Cc: netdev@vger.kernel.org, Vivien Didelot , Florian Fainelli , "David S. Miller" Subject: Re: Regression: mv88e6xxx packet loss after 4.18's PHYLINK merge Message-ID: <20190117113722.GA22476@samu-ThinkPad-T480s> References: <20190114113444.GB16632@samu-ThinkPad-T480s> <20190117022935.GJ24870@lunn.ch> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190117022935.GJ24870@lunn.ch> User-Agent: Mutt/1.9.4 (2018-02-28) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, Jan 17, 2019 at 03:29:35AM +0100, Andrew Lunn wrote: > On Mon, Jan 14, 2019 at 01:34:44PM +0200, Samu Nuutamo wrote: > > Hi, > > > > We have an imx6q-b450v3 board that has one of the switch ports configured as a > > fixed link. After upgrading the kernel to version 4.18 the link has experienced > > a small amount of packet loss, around 0.15%. > > > > The issue was bisected to a commit: aab9c4067d23 net: dsa: Plug in PHYLINK support > > > > After enabling debug we could see that the new phylink code causes the link to > > reset once every second: > > > > [ 309.992368] mv88e6085 gpio-0:00 enembc: phylink_mac_config: mode=fixed//100Mbps/Full adv=00000,00000208 pause=10 link=1 an=1 > > [ 309.998451] mv88e6085 gpio-0:00: p5: Force link down > > [ 309.998869] mv88e6085 gpio-0:00: p5: Speed set to 100 Mbps > > [ 309.999280] mv88e6085 gpio-0:00: p5: Force full duplex > > [ 309.999895] mv88e6085 gpio-0:00: p5: Force link up > > [ 311.032400] mv88e6085 gpio-0:00 enembc: phylink_mac_config: mode=fixed//100Mbps/Full adv=00000,00000208 pause=10 link=1 an=1 > > [ 311.038248] mv88e6085 gpio-0:00: p5: Force link down > > [ 311.038660] mv88e6085 gpio-0:00: p5: Speed set to 100 Mbps > > [ 311.039069] mv88e6085 gpio-0:00: p5: Force full duplex > > [ 311.039678] mv88e6085 gpio-0:00: p5: Force link up > > [ 312.072328] mv88e6085 gpio-0:00 enembc: phylink_mac_config: mode=fixed//100Mbps/Full adv=00000,00000208 pause=10 link=1 an=1 > > [ 312.077603] mv88e6085 gpio-0:00: p5: Force link down > > [ 312.078010] mv88e6085 gpio-0:00: p5: Speed set to 100 Mbps > > [ 312.078417] mv88e6085 gpio-0:00: p5: Force full duplex > > [ 312.079026] mv88e6085 gpio-0:00: p5: Force link up > > Hi Samu > > Please could you try this patch. I've not had chance to properly test > it, and i'm about to go away for a long weekend. > > Thanks > Andrew > > From 02438712824d3c7a9dd1aab8bd5dfb4383e55bfd Mon Sep 17 00:00:00 2001 > From: Andrew Lunn > Date: Wed, 16 Jan 2019 20:22:04 -0600 > Subject: [PATCH] net: phy: phylink: Only change mac when fixed link changes > state > > phylink polls the fixed-link once per second to see if the GPIO has > changed state, or if the callback indicates if there has been a change > in state. It then calls the MAC to reconfigure itself to the current > state. > > For some MACs, reconfiguration can result in packets being dropped. > Hence keep track of the link state between polls and only reconfigure > the MAC if there has been a change of state. > > Reported-by: Samu Nuutamo > Fixes: 9cd00a8aa42e ("net: phy: phylink: Poll link GPIOs") > Signed-off-by: Andrew Lunn > --- > drivers/net/phy/phylink.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > index e7becc7379d7..6473af228842 100644 > --- a/drivers/net/phy/phylink.c > +++ b/drivers/net/phy/phylink.c > @@ -407,11 +407,14 @@ static void phylink_resolve(struct work_struct *w) > phylink_mac_config(pl, &link_state); > break; > > - case MLO_AN_FIXED: > - phylink_get_fixed_state(pl, &link_state); > - phylink_mac_config(pl, &link_state); > - break; > + case MLO_AN_FIXED: { > + bool old_link = pl->phy_state.link; > > + phylink_get_fixed_state(pl, &pl->phy_state); > + if (pl->phy_state.link != old_link) > + phylink_mac_config(pl, &pl->phy_state); > + break; > + } > case MLO_AN_INBAND: > phylink_get_mac_state(pl, &link_state); > if (pl->phydev) { > -- > 2.20.1 > Hi Andrew, I tested the patch and it has an issue that prevents the carrier from turning on. I think it's caused by the change from using link_state to phy->phy_state, as later in the same function the link_state attributes are read when deciding if the carrier needs to be turned on. - Samu