Re: [PATCH] Carrier detect ok, don't turn off negotiation

[Denis Du]

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


ÂÂÂÂÂÂÂÂif (hdlc->carrier == on)
ÂÂÂÂÂÂÂÂgoto carrier_exit; /* no change in DCD line level */

ÂÂÂÂhdlc->carrier = on;

ÂÂÂÂif (!hdlc->open)
ÂÂÂÂÂÂÂÂgoto carrier_exit;

ÂÂÂÂif (hdlc->carrier) {
ÂÂÂÂÂÂÂÂnetdev_info(dev, "Carrier detected\n");
ÂÂÂÂÂÂÂÂhdlc_proto_start(dev);
ÂÂÂÂ} else {
ÂÂÂÂÂÂÂÂnetdev_info(dev, "Carrier lost\n");
ÂÂÂÂÂÂÂÂhdlc_proto_stop(dev);
ÂÂÂÂ}

carrier_exit:
ÂÂÂÂspin_unlock_irqrestore(&hdlc->state_lock, flags);
ÂÂÂÂreturn 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 previous timer expired due to protocol fail.

If carrier keep no change by if (hdlc->carrier == on)
ÂÂÂÂÂÂÂÂgoto carrier_exit; /* no change 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 noise, and this issue was found and complained by our customers. So it is not my theory guessing, it is a real problem.






On Monday, January 22, 2018, 3:25:16 PM EST, David Miller <davem@xxxxxxxxxxxxx> wrote: 





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

> From b5902a4dfc709b62b704997ab64f31c9ef69a6db Mon Sep 17 00:00:00 2001
> From: Denis Du <dudenis2000@xxxxxxxx>
> Date: Mon, 15 Jan 2018 17:26:06 -0500
> Subject: [PATCH] netdev: carrier detect ok, don't turn off negotiation
> 
> 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.
> 
> Signed-off-by: Denis Du <dudenis2000@xxxxxxxx>

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.