Re: [PATCH v3] netpoll: Remove 4s sleep during carrier detection

From: Andrew Lunn <andrew@lunn.ch>
To: David Laight <David.Laight@aculab.com>
Cc: 'Breno Leitao' <leitao@debian.org>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"leit@fb.com" <leit@fb.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"edumazet@google.com" <edumazet@google.com>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Michael van der Westhuizen <rmikey@meta.com>
Subject: Re: [PATCH v3] netpoll: Remove 4s sleep during carrier detection
Date: Thu, 26 Jan 2023 14:22:54 +0100	[thread overview]
Message-ID: <Y9J+rhRFxbLIWgQv@lunn.ch> (raw)
In-Reply-To: <6d13242627e84bde8129e75b6324d905@AcuMS.aculab.com>

On Thu, Jan 26, 2023 at 09:04:42AM +0000, David Laight wrote:
> From: Breno Leitao
> > Sent: 25 January 2023 18:53
> > This patch removes the msleep(4s) during netpoll_setup() if the carrier
> > appears instantly.
> > 
> > Here are some scenarios where this workaround is counter-productive in
> > modern ages:
> > 
> > Servers which have BMC communicating over NC-SI via the same NIC as gets
> > used for netconsole. BMC will keep the PHY up, hence the carrier
> > appearing instantly.
> > 
> > The link is fibre, SERDES getting sync could happen within 0.1Hz, and
> > the carrier also appears instantly.
> > 
> > Other than that, if a driver is reporting instant carrier and then
> > losing it, this is probably a driver bug.
> 
> I can't help feeling that this will break something.
> The 4 second delay does look counter productive though.
> Obvious alternatives are 'wait a bit before the first check'
> and 'require carrier to be present for a few checks'.

I'm guessing, but i think the issue is that the MAC reports the
carrier is up, even though autoneg has not completed, and so packets
are getting dropped. Autoneg takes around 1.5 seconds, so you need to
wait this long before starting to send to prevent packets landing in
the bit bucket. And i guess polling as you suggests does not help,
since it never returns the true status.

But this is pure guesswork. Maybe some mailing list archaeology can
help explain this code.

I guess the likely breaking scenario is that simply the first 1.5
seconds of the kernel log goes to the bit bucket for broken
MACs. Which is not fatal, just annoying for somebody trying to debug a
crash in the first few seconds. I suppose dhcp might also take longer
for broken MACs, since its first requests also get lost, and it might
get into exponential back off.

I guess the risks are small here. But i use the word guess a lot...

  Andrew