Skip to content

Allow LwIP TCP retransmissions to be configured and tune those smaller. #9183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 4, 2019
Merged

Allow LwIP TCP retransmissions to be configured and tune those smaller. #9183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50eb243
Don't consume 36 kB just for Ethernet buffers.
Dec 21, 2018
f3bbd2b
Allow LwIP TCP retransmissions to be configured and tune those smaller.
Dec 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions features/lwipstack/lwipopts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,14 @@
#define TCP_WND MBED_CONF_LWIP_TCP_WND
#endif

#ifdef MBED_CONF_LWIP_TCP_MAXRTX
#define TCP_MAXRTX MBED_CONF_LWIP_TCP_MAXRTX
#endif

#ifdef MBED_CONF_LWIP_TCP_SYNMAXRTX
#define TCP_SYNMAXRTX MBED_CONF_LWIP_TCP_SYNMAXRTX
#endif

// Number of pool pbufs.
// Each requires 684 bytes of RAM (if MSS=536 and PBUF_POOL_BUFSIZE defaulting to be based on MSS)
#ifdef MBED_CONF_LWIP_PBUF_POOL_SIZE
Expand Down
10 changes: 9 additions & 1 deletion features/lwipstack/mbed_lib.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@
"help": "TCP sender buffer space (bytes). Current default (used if null here) is set to (4 * TCP_MSS) in opt.h, unless overridden by target Ethernet drivers.",
"value": null
},
"tcp-maxrtx": {
"help": "Maximum number of retransmissions of data segments.",
"value": 6
},
"tcp-synmaxrtx": {
"help": "Maximum number of retransmissions of SYN segments. Current default (used if null here) is set to 6 in opt.h",
"value": null
},
"pbuf-pool-size": {
"help": "Number of pbufs in pool - usually used for received packets, so this determines how much data can be buffered between reception and the application reading. If a driver uses PBUF_RAM for reception, less pool may be needed. Current default (used if null here) is set to 5 in lwipopts.h, unless overridden by target Ethernet drivers.",
"value": null
Expand Down Expand Up @@ -125,7 +133,7 @@
"mem-size": 25600
},
"Freescale": {
"mem-size": 36560
"mem-size": 16384
},
"LPC1768": {
"mem-size": 16362
Expand Down
4 changes: 2 additions & 2 deletions features/netsocket/emac-drivers/TARGET_Freescale_EMAC/mbed_lib.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "kinetis-emac",
"config": {
"rx-ring-len": 16,
"tx-ring-len": 8
"rx-ring-len": 2,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this without correspondingly reducing the lwIP mem config is a little "sneaky" - effectively you've giving this specific platform a huge memory pool for lwIP, because it was set big in a target override apparently to make room for these buffers.

I'd like you to either

a) reduce the target-override memory pool for lwIP by the corresponding amount - if you can (but I assume you do need the extra space, so you can't).
b) reduce by less, and raise the default memory pool (and other target overrides?) by the difference - so that all platforms get the extra memory you've found you need. Maybe the other targets with big pools can also have their buffer count reduced, so they don't need to get bigger.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll lower the lwip.mem-size value for some amount..
This 16, was actually causing memory_manager->alloc_heap(ENET_ETH_MAX_FLEN, ENET_BUFF_ALIGNMENT); called 16 times.. Where ENET_ETH_MAX_FLEN is 1522 bytes and alignment is 16. I'm unsure of how much the allocator overhead is.

Total memory usage is something between 24352 bytes 24608 bytes.

As a result, I lowered the lwip.mem-size for Freescale boards from 36560 to 16384 (~20 kB less), which still seems to pass the Cloud Client FW update test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If 16K is required with 2 RX buffers, that does suggest cloud wants about 12K of lwIP mem workspace, so maybe that should be the default mem-size? (I believe we assume that target defaults should be good for cloud client). And then other targets should be doing 12K + their buffer size?

I guess the issue is that any upward increase like that might not be appropriate for a patch release, so should be done separately. Something to follow up.

"tx-ring-len": 1
}
}