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

Submitted by:  dupengbo <dupengbo>
Submitted on:  Mon 15 Jan 2007 03:37:41 AM UTC  
 
Category: IPv4Severity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Kieran Mansley <kieranm>
Open/Closed: ClosedPlanned Release: None
lwIP version: None

(Jump to the original submission Jump to the original submission)

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

Now done, closing.

Kieran Mansley <kieranm>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project 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>
Project AdministratorIn charge of this item.
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>
Project 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>
Project AdministratorIn charge of this item.
Mon 26 Feb 2007 01:41:27 PM UTC, comment #2:

Makes sense to me FWIW.

Jonathan Larmour <jifl>
Project 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>
Project AdministratorIn charge of this item.
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>

 

No files currently attached

 

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

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 28 Mar 2007 12:27:59 PM UTCkieranmStatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
    Mon 26 Mar 2007 10:46:23 AM UTCkieranmStatusFixed=>In Progress
      Open/ClosedClosed=>Open
    Wed 21 Mar 2007 01:07:04 PM UTCkieranmStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Mon 26 Feb 2007 12:59:51 PM UTCkieranmAssigned toNone=>kieranm
    Mon 15 Jan 2007 04:39:22 AM UTCdupengboCarbon-CopyRemoved -unavailable-=>-
    Mon 15 Jan 2007 03:37:41 AM UTCdupengboCarbon-Copy-=>Added -unavailable-

    Back to the top


    Powered by Savane 3.1-cleanup1