Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 23 Jan 2018 17:08:10 +0000 (UTC)
From:   Denis Du <dudenis2000@yahoo.ca>
To:     David Miller <davem@davemloft.net>
Cc:     khc@pm.waw.pl, netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Message-ID: <998451043.3408644.1516727290310@mail.yahoo.com>
In-Reply-To: <20180122.152513.1108868799788445512.davem@davemloft.net>
References: <751079597.1884905.1516121905374.ref@mail.yahoo.com> <751079597.1884905.1516121905374@mail.yahoo.com> <20180122.152513.1108868799788445512.davem@davemloft.net>
Subject: Re: [PATCH] Carrier detect ok, don't turn off negotiation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Content-Length: 2785
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


Ok, I check the source code again. It have nothing to do with the interrupt=
s, it is related how the hdlc.c is implemented.
In drivers/net/wan/hdlc.c#L108


=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (hdlc->carrier =3D=3D on=
)
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0goto carrier_exit; /* no ch=
ange in DCD line level */

=C2=A0=C2=A0=C2=A0=C2=A0hdlc->carrier =3D on;

=C2=A0=C2=A0=C2=A0=C2=A0if (!hdlc->open)
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0goto carrier_exit;

=C2=A0=C2=A0=C2=A0=C2=A0if (hdlc->carrier) {
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0netdev_info(dev, "Carrier d=
etected\n");
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0hdlc_proto_start(dev);
=C2=A0=C2=A0=C2=A0=C2=A0} else {
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0netdev_info(dev, "Carrier l=
ost\n");
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0hdlc_proto_stop(dev);
=C2=A0=C2=A0=C2=A0=C2=A0}

carrier_exit:
=C2=A0=C2=A0=C2=A0=C2=A0spin_unlock_irqrestore(&hdlc->state_lock, flags);
=C2=A0=C2=A0=C2=A0=C2=A0return NOTIFY_DONE;

From the above code, I can get that only Carrier have some change, it will =
restart the protocol by hdlc_proto_start(dev);and thus the timer, the previ=
ous timer expired due to protocol fail.

If carrier keep no change by if (hdlc->carrier =3D=3D on)
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0goto carrier_exit; /* no ch=
ange in DCD line level */It will do nothing, not start any new protocol and=
 thus the timer.


My case is the carrier always good, but protocol will fail due to perfect n=
oise, and this issue was found and complained by our customers. So it is no=
t my theory guessing, it is a real problem.


On Monday, January 22, 2018, 3:25:16 PM EST, David Miller <davem@davemloft.=
net> wrote:=20


From: Denis Du <dudenis2000@yahoo.ca>
Date: Tue, 16 Jan 2018 16:58:25 +0000 (UTC)

> From b5902a4dfc709b62b704997ab64f31c9ef69a6db Mon Sep 17 00:00:00 2001
> From: Denis Du <dudenis2000@yahoo.ca>
> Date: Mon, 15 Jan 2018 17:26:06 -0500
> Subject: [PATCH] netdev: carrier detect ok, don't turn off negotiation
>=20
> Sometimes when physical lines have a just good noise to make the protocol
> handshaking fail, but the carrier detect still good. Then after remove of
> the noise, nobody will trigger this protocol to be start again to cause
> the link to never come back. The fix is when the carrier is still on, not
> terminate the protocol handshaking.
>=20
> Signed-off-by: Denis Du <dudenis2000@yahoo.ca>

The timer is supposed to restart the protocol again, that's how this
whole thing is designed to work.

I think you are making changes to the symptom rather than the true
cause of the problems you are seeing.

Sorry, I will not apply this until the exact issue is better
understood.

Thank you.