Received-SPF: pass (google.com: domain of linux-kernel+bounces-193797-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 29 May 2024 09:48:41 +0100
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: xiaolei wang <xiaolei.wang@windriver.com>, alexandre.torgue@foss.st.com,
	joabreu@synopsys.com, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, mcoquelin.stm32@gmail.com,
	netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [net PATCH] net: stmmac: update priv->speed to SPEED_UNKNOWN
 when link down
Message-ID: <Zlbr6ZCI0GZR6FKn@shell.armlinux.org.uk>
References: <20240528092010.439089-1-xiaolei.wang@windriver.com>
 <775f3274-69b4-4beb-84f3-a796343fc095@lunn.ch>
 <b499cbcd-a3c9-4f38-a69a-ad465e7f8d5a@windriver.com>
 <98e6266f-805c-4da2-b2dc-b25297c53742@lunn.ch>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <98e6266f-805c-4da2-b2dc-b25297c53742@lunn.ch>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>

On Wed, May 29, 2024 at 02:57:27AM +0200, Andrew Lunn wrote:
> On Wed, May 29, 2024 at 08:22:01AM +0800, xiaolei wang wrote:
> > 
> > On 5/28/24 21:20, Andrew Lunn wrote:
> > > CAUTION: This email comes from a non Wind River email account!
> > > Do not click links or open attachments unless you recognize the sender and know the content is safe.
> > > 
> > > On Tue, May 28, 2024 at 05:20:10PM +0800, Xiaolei Wang wrote:
> > > > The CBS parameter can still be configured when the port is
> > > > currently disconnected and link down. This is unreasonable.
> > > This sounds like a generic problem. Can the core check the carrier
> > > status and error out there? Maybe return a useful extack message.
> > > 
> > > If you do need to return an error code, ENETDOWN seems more
> > 
> > Currently cbs does not check link status. If ops->ndo_setup_tc() returns
> > failure, there will only be an output of "Specified device failed to setup
> > cbs hardware offload".
> 
> So it sounds like we should catch this in the core then, not the
> driver. And cbs_enable_offload() takes an extack, so you can report a
> user friendly reason for failing, the at the carrier is off.

It's worse than that (see my other reply.) If the link speed changes,
there's nothing that deals with updating the CBS configuration for the
new speed. CBS here is basically buggy - unless one reconfigures CBS
each time the link comes up.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!