buglwIP - A Lightweight TCP/IP stack - Bugs: bug #36300, PPP CHAP stack requirements...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #36300: PPP CHAP stack requirements reduction

Submitter:  Sylvain Rochet <gradator>
Submitted:  Wed 25 Apr 2012 01:02:35 PM UTC
   
 
Category:  PPP Severity:  3 - Normal
Item Group:  Change Request Status:  Wont Fix
Privacy:  Public Assigned to:  None
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Jump to the original submission

Thu 22 May 2014 07:38:52 PM UTC, comment #7: 

Not really a bug, and the recently merged ppp-new branch already have numerous memory improvement.

Sylvain Rochet <gradator>
Group Member
Mon 07 May 2012 08:58:54 PM UTC, comment #6: 

Isn't the MEM_USE_POOLS define the way to use pbuf instead of the heap ?

Of course I will rework the patch to make it optional and I don't care at all if it is accepted or not, I just want to understand better the current LwIP strategy to provide better patchs :-)

Sylvain Rochet <gradator>
Group Member
Mon 07 May 2012 08:28:10 PM UTC, comment #5: 

No, there's not a strategy to remove the heap, but personally, I'd like the stack to be usable without a heap to prevent heap fragmentation of being a potential problem in long running systems.

BTW: DHCP and AutoIP can be prevented to use the heap, PBUF_RAM pbufs can be diverted to memp pools of different size (but in this mode, the heap replacement should only be used for pbufs).

I'm not saying your idea is bad, I only want the heap to be "optional". :-)

Simon Goldschmidt <goldsimon>
Group administrator
Mon 07 May 2012 12:25:15 PM UTC, comment #4: 

Humm, maybe I misunderstood something.

Am I wrong saying that mem_malloc() and mem_free() use memory from ram_heap[] (if LWIP_RAM_HEAP_POINTER is undefined as well as  MEM_USE_POOLS, which are the default) ?

Some other files are using mem_malloc/mem_free: lwip/src/core/dhcp.c, lwip/src/core/ipv4/autoip.c and lwip/src/core/pbuf.c use mem_malloc if type is PBUF_RAM, is the current lwip strategy to remove all calls to mem_malloc/mem_free ?

Sylvain Rochet <gradator>
Group Member
Thu 03 May 2012 07:04:20 PM UTC, comment #3: 

Hmm, making the heap mandatory is not my favourite. I'd make this at least optional since I want lwIP to support devices without a heap.

Can't we reuse/share another PPP-internal buffer?

Oh, and added to that: although the PPP code is not in best shape everywhere, we try to keep changes to the original code (samba pppd) as small as possible, while applying your patch probably brings us more differences. I'll have to think about that...

Simon Goldschmidt <goldsimon>
Group administrator
Mon 30 Apr 2012 08:35:38 AM UTC, comment #2: 

As a side note:

Calling ethernet_input() from a thread other than the LwIP thread is a bad idea, this is not a thread-safe way, this was discussed on the lwip-users mailing list and was a bug in the port I used.

This doesn't change anything about the huge stack requirement, except that it now requires stack memory on the LwIP thread.

Sylvain Rochet <gradator>
Group Member
Thu 26 Apr 2012 02:55:24 PM UTC, comment #1: 

Hum, there is a short glitch in the patch, all LOG_ERROR must be replaced by LOG_ERR, I should have read the debug.h header before :-)

Sylvain Rochet <gradator>
Group Member
Wed 25 Apr 2012 01:02:35 PM UTC, original submission:  

The current PPP CHAP implementation requires a lot of stack memory, let's say, about 1 KiB stack memory is required to complete the challenge:

static void ChapReceiveResponse(...) {
  ...
  char secret[MAXSECRETLEN]; // 256
  char rhostname[256];
  MD5_CTX mdContext; // 64+16+4*4+2*4 = 104

static void ChapSendResponse(...) {
  ...
  char msg[256]; /* @todo: this can be a char*, no strcpy needed */

This can be an issue with system without much memory, if Ethernet input is done through a thread that call ethernet_input() from etharp.c, then this thread is going to require about 1 KiB of static stack size only necessary for CHAP, which is only used from time to time (ideally, only one time).

Here is a patch that use mem_malloc() and mem_free() for heavy CHAP authentication data, so that it use memory from the mem/packet pool and doesn't waste memory.

It also uses the MAXNAMELEN define instead of the literal 256 there and there.

Sylvain

Sylvain Rochet <gradator>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by gradator (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-05-22 gradator StatusNone Wont Fix
        Open/ClosedOpen Closed
    2013-01-14 goldsimon CategoryNone PPP
    2012-04-25 gradator Attached File- Added chap-less-stack-memory.patch, #25731

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code