Date: Wed, 29 Sep 1999 21:25:08 +0200 (SAT) From: John Hay <jhay@mikom.csir.co.za> To: danhil@cwnt.com (Daniel Hilevich) Cc: freebsd-questions@FreeBSD.ORG, freebsd-hackers@FreeBSD.ORG Subject: Re: A bug in the sppp driver? Message-ID: <199909291925.VAA65012@zibbi.mikom.csir.co.za> In-Reply-To: <033a01bf0a92$8fe1b4e0$2e00a8c0@nt46daniel> from Daniel Hilevich at "Sep 29, 1999 04:51:58 pm"
next in thread | previous in thread | raw e-mail | index | archive | help
Are you busy with a leased line driver or a dialup/isdn kind of driver?
I have been busy fixing sppp to work properly with leased line drivers
again, but am not finished with it yet. :-/ Hopefully I won't break
the isdn handling at the same time.
> While trying to use the sppp, I came across this situation and I think it's
> a bug:
> When you trying to establish connection from one peer (local) to another
> (remote), you sent a CONF_REQ message to the remote peer. The remote peer
> should answer with a CONF_ACK message. In the code of the sppp driver
> (net/if_spppsubr.c, lines 1321 - 1357) you can see that the remote peer
> send's a CONF_ACK message to the local peer
> (in the line: rv = (cp->RCR)(sp, h, len);) but doesn't change it state to
> STATE_ACK_SENT (as I think it should do) . Further more, you can see that
> after a few lines, there are these strange lines:
> case STATE_ACK_SENT:
> case STATE_REQ_SENT:
> sppp_cp_change_state(cp, sp, rv?
> STATE_ACK_SENT: STATE_REQ_SENT);
> break;
My patch for this part looks like this, carefull I have just cut and
paste it, so the tabs got lost:
---------
@@ -1298,6 +1299,16 @@
/* fall through... */
case STATE_ACK_SENT:
case STATE_REQ_SENT:
+ /*
+ * sppp_cp_change_state() have the side effect of
+ * restarting the timeouts. We want to avoid that
+ * if the state don't change, otherwise we won't
+ * ever timeout and resend a configuration request
+ * that got lost.
+ */
+ if (sp->state[cp->protoidx] == (rv ? STATE_ACK_SENT:
+ STATE_REQ_SENT))
+ break;
sppp_cp_change_state(cp, sp, rv?
STATE_ACK_SENT: STATE_REQ_SENT);
break;
--------
>
> Question: if you are in the STATE_ACK_SENT why change the state to the same
> one according to the value of rv?
Because the state transition table on page 13 of rfc1661 says that if you
are in "Ack-Sent" and receive a +RCR, you send a sca and stay in the same
state?
>
> As I understand, the state should be changed according to the value of rv,
> but it should be done right after the call to cp->RCR. The it is implemented
> now, the state won't be changed.
Like it is, it does work under ideal (no packet loss) conditions. My patch
is just for the case where a scr packet got lost.
>
> (I have a lot of problems with this driver and any help will be
> appreciative)
There are two other "big" problems that I know of.
If the line is down long enough, sppp will give up and go in a illegal state.
It should just keep on trying to establish the link again. I have a fix that
I'm testing for this.
The loopback handling are broken. It just go in a transmit frenzy. I have
tried the solution in gnats 11238, but I'm not happy with it, because it
stops sppp when loopback is detected, which is also not what I want.
But appart from these and a few minor problems sppp is working just fine
here.
John
--
John Hay -- John.Hay@mikom.csir.co.za
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199909291925.VAA65012>