Re: [PATCH net v3] Revert ncsi: Propagate carrier gain/loss events to the NCSI controller

Johnathan Mantey
2023-11-14T20:33:11Z

Simon Horman <horms@kernel.org> writes:

> On Mon, Nov 13, 2023 at 08:30:29AM -0800, Johnathan Mantey
> wrote:
>> This reverts commit 3780bb29311eccb7a1c9641032a112eed237f7e3.
>>
>> The cited commit introduced unwanted behavior.
>>
>> The intent for the commit was to be able to detect carrier
>> loss/gain
>> for just the NIC connected to the BMC. The unwanted effect is a
>> carrier loss for auxiliary paths also causes the BMC to lose
>> carrier. The BMC never regains carrier despite the secondary
>> NIC
>> regaining a link.
>>
>> This change, when merged, needs to be backported to stable
>> kernels.
>> 5.4-stable, 5.10-stable, 5.15-stable, 6.1-stable, 6.5-stable
>>
>> Fixes: 3780bb29311e ("ncsi: Propagate carrier gain/loss events
>> to the NCSI controller")
>> CC: stable@vger.kernel.org
>> Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
>
> Hi Jonathan,
>
> thanks for addressing my feedback on v2.
>
> So far as addressing a regression goes, this looks good to me.
> But I do wonder what can be done about the issue that
> the cited commit was intended to address: will this patch
> regress things
> on that front?

Unfortunately the original issue will reoccur. I'm not sure which
behavior is worse. What's been present for the lifespan of the
ncsi driver, or this new issue I've introduced. In both instances
a cable unplug causes undesirable behavior. I'm going to
investigate solving this for Intel's specific use case ATM. NCSI
has numerous modes in which it can be configured. I don't have a
good feel for how to generalize the code given the side effect
introduced by my change.

>
> ...

--
Johnathan Mantey