[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [lwip-devel] ppp-new
Re: [lwip-devel] ppp-new
Thu, 5 Jul 2012 21:24:23 +0200
On Wed, Jul 04, 2012 at 12:06:51PM -0600, Ivan Delamer wrote:
> I tried to run the ppp-new branch, but unfortunately I got some hard
> faults. Probably something wrong in my configuration, but I need to dig
> deeper and will find time to do this in a couple weeks.
Well, this is a bad news :-) How are you starting the PPP session ?
> Some suggestions based on what I saw:
> - change uint_32_t type declarations (already done, I see!)
Yeah, I saw the issue about uint32_t when building for my AVR32 target.
> - in magic.c, lets try to avoid library srand() and rand() functions.
> We have a LWIP_RAND() definition which is used in IGMP. Could we reuse
> this? The reason is that the library introduces all sorts of
> additional code to support rand functions -> code bloating. Otherwise,
> I can provide a lightweight rand() implementation that I use.
srand() and rand() are only used if PPP_MD5_RANDM is false, otherwise
the magic module use MD5 as random pool + entropy from pseudo randoms
events like packet input, the combination of both provide an acceptable
Maybe we should move to the core code the PPP random module and let the
user the choice of using the provided random functions or not.
PPP is using random for cryptography, it requires the random pool to be
"churned" so that we get not-so predictable random challenges.
I am not a entropy-random-crypto-... expert, but I guess that using
LWIP_RAND() in PPP require at least a new LWIP_CHURNRAND() definition.
Doing changes without enough cryptography knowledge in a random
generator used for cryptography might end up in a terrible disaster,
this is why I didn't change much the random code used in the previous
> - are there any thread-safety issue with the public API? I am using
> tcpip callbacks for sigHUP, but calling other functions directly. Any
This is a good question, quick look results:
ppp_new() is thread safe, it only use memp_malloc() and change its own
ppp_set_auth() is of course thread safe too
regarding I/O, for PPPoS:
ppp_write() might be called from ppp_over_serial_open() (first LCP MRU
sent to the peer) and is called from the thread calling pppos_input()
regarding PPP negotiation. Operating in full duplex mode, it might
happens that ppp_write() suffer a serialisation issue if we receive a
LCP requests while sending the first LCP MRU request to the peer, at
least, in theory it could.
regarding I/O, for PPPoE:
pppoe_create() is thread safe
pppoe_output() called by pppoe_send_padi() called by pppoe_connect()
called by ppp_over_ethernet_open() called from a thread other than the
lwIP thread is calling ethif->linkoutput(), I guess it suffers from a
serialisation issue, and not only in theory. I don't think the
linkoutput() to be thread safe, and if so, it depends on the port, I
don't think we require the port to provide a serialized linkoutput()
I think we can stop the quick overview here, we need to provide
a netconn()-like API for PPP.
> Thanks for the great work! I noticed RAM use decreased,
Yeah, a PPP session without much features compiled-in (PAP+CHAP) use
about 1 KiB of RAM for PPPoE, and about 1 KiB + PPPOS_RX_BUF for PPPoS.
> but flash increased by about 6K. Will have to review new PPP options
> to see if I can decrease this.
This is not so much, less than I expected first :-)
There is however probably some other useless features that can be set at
Description: Digital signature