Re: PPP Issues

[Nick Garnett <nickg at ecoscentric dot com>]

"John Paul King" <jpking@advantexmail.net> writes:

> The following are various issues I have encountered with the PPP stack.
> Attached is a  patch for your consideration.
> 
> 
> 1.  The stack does not prepend initial frames with a starting Flag Sequence.
> Per RFC 1549:
> 
> "Transmitters SHOULD send an open Flag Sequence whenever 'appreciable time'
> has elapsed after the prior closing Flag Sequence. The maximum value for
> 'appreciable time' is likely to be no greater than the typing rate of a slow
> typist, say 1 second (S4)."
> 
> Although a timer could be implemented, this patch prepends every frame with
> a Flag sequence, regardless of the time between frames:
> 
> "Only one Flag Sequence is required between two frames. Two consecutive Flag
> Sequences constitute an empty frame, which is ignored, and not counted as a
> FCS error (S3.1)."
> 
> Affects: ppp_io.c

As far as I can see, your patch just adds an extra flag sequence at
the beginning of each packet. The driver emits a flag sequence at the
end of each packet anyway. So the extra flag sequence is redundant.

I am very wary of making hastily conceived changes to this code. It
has been working for about a decade so far and if there were genuinely
a problem here I would have thought that it would have been identified
before now.

What is the actual problem you are seeing, and why do you think that
this patch fixes it?

> 
> 
> 2.  If a session is terminated due to unanswered echo-requests, an
> echo-request still remains in the timeout queue; consequently, once the
> session is reestablished, every echo-request interval will send out two
> echo-requests. If the session is terminated again for the same reason, then
> three echo-requests will be sent out each interval (and so on--the process
> is cumulative, and will add an additional request for each termination due
> to unanswered echo-reuquests).  This patch prevents the above from
> happening.
> 
> Affects: lcp.c

This seems like a reasonable fix. The original code here was part of a
stand-alone program that got destroyed once the PPP link went down. So
there was no need to clean these things up.

> 
> 
> 3.  Warnings are generated when compiling pppd.c and magic.c:
> 
> arm-elf-gcc -c -I/home/jpking/src/ecos/ecos_install/include -I/opt/ecos/ecos
> _cvs/packages/net/ppp/current -I/opt/ecos/ecos_cvs/packages/net/ppp/current/
> src -I/opt/ecos/ecos_cvs/packages/net/ppp/current/tests -I. -I/opt/ecos/ecos
> _cvs/packages/net/ppp/current/src/ -finline-limit=7000 -mcpu=arm7tdmi -mno-s
> hort-load-words -Wall -Wpointer-arith -Wstrict-prototypes -Winline -Wundef -
> g -O2 -ffunction-sections -fdata-sections -fno-exceptions -D__ECOS -Wp,-MD,s
> rc/pppd.tmp -o src/net_ppp_pppd.o
> /opt/ecos/ecos_cvs/packages/net/ppp/current/src/pppd.c
> /opt/ecos/ecos_cvs/packages/net/ppp/current/src/pppd.c: In function
> `cyg_ppp_timeout':
> /opt/ecos/ecos_cvs/packages/net/ppp/current/src/pppd.c:598: warning:
> implicit declaration of function `cyg_ppp_gettimeofday'
> 
> "cyg_ppp_gettimeofday" is not defined anywhere in the ecos source, but a
> version of "gettimeofday" is defined in the ppp source. This patch uses that
> version.
> 
> Affects: pppd.h and sys-ecos.c

The header <names.h> #defines gettimeofday to
cyg_ppp_gettimeofday. Hence all references to gettimeofday are
translated to cyg_ppp_gettimeofday. This is done to stop the PPP code
polluting the namespace of any application code. What is missing is
a declaration of gettimeofday() in the standard <sys/time.h>
headers. However there are various issues with doing that easily.

The correct approach at present would be to just add a declaration of
gettimeofday() in pppd.h.


-- 
Nick Garnett                    eCos Kernel Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts