buglwIP - A Lightweight TCP/IP stack - Bugs: bug #26133, Processor specific porting needs...

 
 

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

bug #26133: Processor specific porting needs flexibility in terms of placement of various memory pools and heap

Submitter:  Zhenwei Chu <blackfin>
Submitted:  Thu 09 Apr 2009 02:03:28 PM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  Change Request Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  1.3.0

Jump to the original submission

Wed 13 Jan 2010 01:12:09 PM UTC, comment #15: 

Done:
- the heap can be relocated by defining LWIP_RAM_HEAP_POINTER as a pointer to the memory's address;
- both the heap and the memp pools can be relocated by including an 'extern' definition (including the corresponding relocation attributes or pragmas) in cc.h

Simon Goldschmidt <goldsimon>
Group administrator
Tue 12 Jan 2010 08:21:10 PM UTC, comment #14: 

Sorry that I didn't understand that from comment #10.  I don't really care how it's done as long as it is supported - I require it in one of my lwIP-based projects.

Bill Auerbach <billauerbach>
Tue 12 Jan 2010 08:15:11 PM UTC, comment #13: 

The per-compiler specific stuff would  be in cc.h, not lwipopt.h - that would just select where you wanted things to be by selecting one of the macros defined in cc.h - but I'm happy to go with the easy way for now as you suggest.


Kieran Mansley <kieranm>
Group Member
Tue 12 Jan 2010 07:45:09 PM UTC, comment #12: 

Comment 6822/#10 also means adding compiler and target specific items to lwipopts.h.  Do you really want to do that?  lwipopts now enables, disables, turns on or turns off different features or modes.  AFAIK there are no compiler or platform specific settings there.

Bill Auerbach <billauerbach>
Tue 12 Jan 2010 06:36:12 PM UTC, comment #11: 

Me too, the only problem is that would result in 15 (or how many pools do we have currently) 2 (_PRE and _POST to satisfy all compilers) defines in lwipopts.h. Plus compilers using pragmas would need special include handling (as for packed structs), only with 2 files (pre and post) *for each pool.

Of course we could still hide that away in memp_std.h, but it's a whole lot of code... Therefore, I'll solve it the easy way now and wait for people to complain if they still face problems.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 12 Jan 2010 04:07:48 PM UTC, comment #10: 

I still like the approach outlined in patch #6822 comment #10: https://savannah.nongnu.org/patch/?6822#comment10 best but happy to go with this for now and see how we get on.


Kieran Mansley <kieranm>
Group Member
Mon 11 Jan 2010 06:16:32 PM UTC, comment #9: 

Kieran and Zhenwei Chu, is that OK with you? It's not the cleanest approach, but it doesn't clutter our sources with that much ifdefs or defines.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 11 Jan 2010 01:52:54 PM UTC, comment #8: 


>Are there any architectures where this doesn't work?


If not, there's not much choice than for the user to change the source code where the buffers are allocated, right?  You could add to the definitions LWIP_MEM_XXX_ATTRIBUTE (XXX is the pool name as done with the array name) before the name and if a compiler comes up that needs it with the definition, the user can provide this.  It would mean adding LWIP_MEM_XXX_ATTRIBUTE #defines (set to blank) in cc.h.

At least now GCC ports can use this if needed which supports a significant lwip user base.  The number of users who will use this feature and the number of those not using GCC is even smaller.

Bill Auerbach <billauerbach>
Sun 10 Jan 2010 01:20:51 PM UTC, comment #7: 

As I already said in patch #6822: I think Bill's idea to include prototype of the heap- or pool-variable definition in cc.h (including the appropriate _attribute_ or pragme to relocate them) is good enough plus it keeps us from having _PRE and _POST defines everywhere.

Are there any architectures where this doesn't work?

Of course, there is still a limitation when pool elements shall come from totally different areas (e.g. pbuf pool in internal, non-contiguous buffers) - we would need a different mechanism for that. But then again, this is rather a thing to discuss in task #7896 (Support zero-copy drivers).

Simon Goldschmidt <goldsimon>
Group administrator
Thu 29 Oct 2009 07:11:24 PM UTC, comment #6: 
Bill Auerbach <billauerbach>
Thu 29 Oct 2009 06:03:02 PM UTC, comment #5: 

Idea from Ioardan Neshev on lwip-devel (29-10-2009): Add define for linker section to definition of 'ram_heap' (we need a _PRE and _POST define here for compiler compatibility reasons, see dns.c:local_hostlist_static).

This define would make sense on memp_memory, too.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 15 Apr 2009 09:06:27 PM UTC, comment #4: 


>Ideally this should be an optional feature, so the existing
>behaviour is the default (as that is what most will want) and >those that need it could specify something like #define >LWIP_MEMP_EXTERNALLY_ALLOCATED 1 (just a suggestion for the >name) in lwipopts.h


Sure,This would be an optional feature for those who want to have finer control on the placement rather than compiler chosen one.

One way is as below.

#ifdef LWIP_MEMP_EXTERNALLY_ALLOCATED
static u8_t* memp_memory;
u8_t** lwip_memp_ptr = &memp_memory;
u32_t  lwip_memp_len = xxx; // length of the memp memory
#else
 // default
#endif

and init routine will set the pointer to appropriate location
*lwip_memp_ptr = xxx;

Srinivas Gollakota <gollakota>
Thu 09 Apr 2009 04:39:39 PM UTC, comment #3: 

I can provide a patch that places each pool in it's own static array.  By preceding the additions with extern declaration one can move them to different linker segments.  One or more pools can be moved.  I got a big increase moving what I could to onchip memory.

Bill

Bill Auerbach <billauerbach>
Thu 09 Apr 2009 03:35:13 PM UTC, comment #2: 

I use this kind of trick to allocate the pool on an external SDRAM memory. This is prefered to keep internal RAM usage low.

See attached patch which is really trivial.



(file #17897)

Clément DAVID <davidcl>
Thu 09 Apr 2009 02:43:07 PM UTC, comment #1: 

Can you explain how this should be used?  I presume porters of lwIP will have to allocate an area of memory and poke it into ADI_TOOLS_memp_ptr or something.  It's not clear what advantage this brings, and would by the look of it break the existing ports.

I think this would be better done with a function call.  Use lwip/memp_std.h to calculate the length required (as you have done), then call a (port-defined) architecture-specific function to allocate the pool memory with that length as an argument.  This function would return a pointer to the memory that could be assigned to memp_memory.  There would also need to be the corresponding "free this memory" call I suppose.

Ideally this should be an optional feature, so the existing behaviour is the default (as that is what most will want) and those that need it could specify something like #define LWIP_MEMP_EXTERNALLY_ALLOCATED 1 (just a suggestion for the name) in lwipopts.h

Kieran Mansley <kieranm>
Group Member
Thu 09 Apr 2009 02:03:28 PM UTC, original submission:  

Protecting them with simple macro will give them a chance to declare/define at the upper level so lwip code changes as below

Files changed:
core\memp.c

Changes:
#if !defined(LWIP_RAM)
/* Adam original */
static u8_t ram[MEM_SIZE + sizeof(struct mem) + MEM_ALIGNMENT];
#else
/* Christiaan alignment fix */
static u8_t* ram;
static struct mem ram_heap[1 + ( (MEM_SIZE + sizeof(struct mem) - 1) / sizeof(struct mem))]; 
#endif

memp.c: line 136:
From:
/** This is the actual memory used by the pools. */
static u8_t memp_memory[MEM_ALIGNMENT - 1
#define LWIP_MEMPOOL(name,num,size,desc) + ( (num) * (MEMP_SIZE + MEMP_ALIGN_SIZE(size) ) )
#include "lwip/memp_std.h"
];


To:
/** This is the actual memory used by the pools. */
static u8_t* memp_memory;
u8_t** ADI_TOOLS_memp_ptr = &memp_memory;
u32_t  ADI_TOOLS_memp_len = MEM_ALIGNMENT - 1
#define LWIP_MEMPOOL(name,num,size,desc) + ( (num) * (MEMP_SIZE + MEMP_ALIGN_SIZE(size) ) )
#include "lwip/memp_std.h"
  ;

pbuf.c: line76

From:
/*static u8_t pbuf_pool_memory[MEM_ALIGNMENT - 1 + PBUF_POOL_SIZE MEM_ALIGN_SIZE(PBUF_POOL_BUFSIZE + sizeof(struct pbuf))]; /


To:
static u8_t* pbuf_pool_memory;
u8_t** ADI_TOOLS_pbuf_pool_ptr = &pbuf_pool_memory;
/*u32_t  ADI_TOOLS_pbuf_pool_len =
  (PBUF_POOL_SIZE MEM_ALIGN_SIZE(PBUF_POOL_BUFSIZE + sizeof(struct pbuf)));/

Zhenwei Chu <blackfin>

 

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

Attached Files
file #17897:  temp.diff added by davidcl (404B - application/octet-stream - Adding EVK1100 board (AVR32) sdram memp allocation support)

 

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 gollakota (Posted a comment)
  • -email is unavailable- added by billauerbach (Posted a comment)
  • -email is unavailable- added by davidcl (Updated the item)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by blackfin (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-01-13 goldsimon StatusNeed Info Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2010-01-10 goldsimon StatusNone Need Info
    2009-10-29 goldsimon SummaryProcessor specific porting needs flexibility in terms of placement of various memory pools. Processor specific porting needs flexibility in terms of placement of various memory pools and heap
    2009-05-01 kieranm Planned Release 1.4.0
    2009-04-09 davidcl Attached File- Added temp.diff, #17897

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code