Message-ID: <4eaff461-47d1-cb9e-b24f-3699a77c3a3d@debian.org>
Date:   Thu, 26 Jan 2023 10:52:11 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.13.1
Cc:     leit@meta.com, "leit@fb.com" <leit@fb.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "andrew@lunn.ch" <andrew@lunn.ch>,
        "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
To:     David Laight <David.Laight@ACULAB.COM>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
References: <20230125185230.3574681-1-leitao@debian.org>
 <6d13242627e84bde8129e75b6324d905@AcuMS.aculab.com>
From:   Breno Leitao <leitao@debian.org>
In-Reply-To: <6d13242627e84bde8129e75b6324d905@AcuMS.aculab.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 26/01/2023 09:04, David Laight wrote:

>> 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.

If we see breakages after this patch, then we can identify broken 
drivers, and fix the driver itself.

On the other side, if we keep this workaround, we are penalizing the 
boot of every modern machine in 4s, just because we might have some 
broken driver somewhere.