buglwIP - A Lightweight TCP/IP stack - Bugs: bug #18782, the default PBUF_POOL_BUFSIZE...

 
 

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

bug #18782: the default PBUF_POOL_BUFSIZE value should be changed

Submitter:  dupengbo <dupengbo>
Submitted:  Mon 15 Jan 2007 03:37:41 AM UTC
   
 
Category:  IPv4 Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  kieranm
Open/Closed:  Closed Planned Release:  None
lwIP version:  None

Jump to the original submission

Wed 28 Mar 2007 12:27:59 PM UTC, comment #9: 

Now done, closing.

Kieran Mansley <kieranm>
Group Member
Mon 26 Mar 2007 10:46:23 AM UTC, comment #8: 

OK.  Reopening to include the PBUF_LINK_HLEN.  That had occured to me as a good solution, but wasn't sure if that value would have been specified by this point and didn't have time to investigate further.  Will do so now.

Kieran Mansley <kieranm>
Group Member
Fri 23 Mar 2007 07:58:24 AM UTC, comment #7: 

It is better if PBUF_LINK_HLEN is accounted.

User can change PBUF_LINK_HLEN value according to user's usage.

If user do not need ethernet head, he can define PBUF_LINK_HLEN  = 0, ....


If replace all these const value with macro define, it maybe better.

For example:
In "opt.h"
#define TCP_MSS 128
#define TRANSPORT_HLEN 20
#define IP_HLEN        20
#define LINK_HLEN      0 // or something else

#define PBUF_POOL_BUFSIZE                (TCP_MSS + TRANSPORT_HLEN + IP_HLEN + LINK_HLEN)


in "pbuf.h"
#define PBUF_TRANSPORT_HLEN TRANSPORT_HLEN
#define PBUF_IP_HLEN        IP_HLEN



dupengbo <dupengbo>
Fri 23 Mar 2007 04:25:35 AM UTC, comment #6: 

We sort of should be putting ethernet specific settings here, or rather we should account for it by using PBUF_LINK_HLEN. That's a user-settable option and reflects the intention of what should be reserved, so we should use that.

Jonathan Larmour <jifl>
Group Member
Thu 22 Mar 2007 01:41:54 PM UTC, comment #5: 

You're probably right, but (i) should we be putting ethernet specific settings here? and (ii) the default is so small that most people will want to change it.

I think adding comments is perhaps the best change so that people can choose a good value for themselves.

Kieran Mansley <kieranm>
Group Member
Thu 22 Mar 2007 01:32:27 PM UTC, comment #4: 

Shouldn't the PBUF_POOL_BUFSIZE include ethernet header, also? I did not look into the code, just thinking: If I send TCP_MSS data, I have to put a TCP, IP and ETH header in front of the data or else I will have to allocate a new pbuf for that.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 21 Mar 2007 01:07:04 PM UTC, comment #3: 

Done, closing fixed.  The real issue with this is that we need PBUF_POOL_BUFSIZE to be TCP_MSS+40 for most efficient operation.  This change fixes it for the default case.

Kieran Mansley <kieranm>
Group Member
Mon 26 Feb 2007 01:41:27 PM UTC, comment #2: 

Makes sense to me FWIW.

Jonathan Larmour <jifl>
Group Member
Mon 26 Feb 2007 12:59:51 PM UTC, comment #1: 

I think this is a good suggestion.  Any objections to this change going in?

Kieran Mansley <kieranm>
Group Member
Mon 15 Jan 2007 03:37:41 AM UTC, original submission:  

for example:
A is LWIP system , B is other TCPIP system.

A: the default TCP_WND is 2048, PBUF_POOL_SIZE is 16, PBUF_POOL_BUFSIZE  is 128. TCP_MSS is 128.
static u8_t pbuf_pool_memory[MEM_ALIGNMENT - 1 + PBUF_POOL_SIZE * MEM_ALIGN_SIZE(PBUF_POOL_BUFSIZE + sizeof(struct pbuf))];
the size of pbuf_pool_memory is 2048.

B: has known A has 2048 bytes available buffer.

So, B will send 2048 bytes to A (do not wait the ACK from A).

But B will send 128 + TCP header + IP header = 168 bytes to A.
LWIP will need two POOLs to save the data, because the PBUF_POOL_BUFSIZE  is 128, it could not save the 168 bytes data.

the result: B know the A has 2048 bytes available buffer, but actually A only has 1024 bytes buffer available, because every segment need two POOLs.

suggestion: PBUF_POOL_BUFSIZE is 128 + TCP header + IP header


dupengbo <dupengbo>

 

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

No files currently attached

 

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 jifl (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by dupengbo (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-03-28 kieranm StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2007-03-26 kieranm StatusFixed In Progress
        Open/ClosedClosed Open
    2007-03-21 kieranm StatusNone Fixed
        Open/ClosedOpen Closed
    2007-02-26 kieranm Assigned toNone kieranm
    2007-01-15 dupengbo Carbon-CopyRemoved -email is unavailable- -
    2007-01-15 dupengbo Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code