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

Submitted by:  Zhenwei Chu <blackfin>
Submitted on:  Thu 09 Apr 2009 02:03:28 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Item Group: Change RequestStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: 1.3.0

(Jump to the original submission 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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
Thu 29 Oct 2009 07:11:24 PM UTC, comment #6:

https://savannah.nongnu.org/patch/?6822

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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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>

 

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
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by gollakota (Posted a comment)
  • -unavailable- added by billauerbach (Posted a comment)
  • -unavailable- added by davidcl (Updated the item)
  • -unavailable- added by kieranm (Posted a comment)
  • -unavailable- added by blackfin (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 13 Jan 2010 01:12:09 PM UTCgoldsimonStatusNeed Info=>Fixed
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
    Sun 10 Jan 2010 01:20:51 PM UTCgoldsimonStatusNone=>Need Info
    Thu 29 Oct 2009 06:03:01 PM UTCgoldsimonSummaryProcessor 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
    Fri 01 May 2009 12:37:11 PM UTCkieranmPlanned Release=>1.4.0
    Thu 09 Apr 2009 03:35:13 PM UTCdavidclAttached File-=>Added temp.diff, #17897

    Back to the top


    Powered by Savane 3.1-cleanup1