Renesas : Improve LWIP speed #7683

TomoYamanaka · 2018-08-03T07:09:10Z

Description

For speeding up of LWIP on RZ/A1, I added four new config processes in mbed_lib.json and lwipopts.h, overriden the values.
Since RZ/A1 incorporates a large-capacity memory, can actualize speeding up by running with the overriden value.

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

For speeding up of LWIP in RZ/A1, I added four new config processes in mbed_lib.json and lwipopts.h, overriden those values. Since RZ/A1 incorporates a large memory, can actualize speeding up by running with the override value. Also those new config processes will be helpful for more customize.

SeppoTakalo · 2018-08-03T10:09:27Z

@mikaleppanen Please review

SeppoTakalo · 2018-08-03T10:11:47Z

features/lwipstack/lwipopts.h

@@ -139,6 +139,26 @@

 #define LWIP_RAM_HEAP_POINTER       lwip_ram_heap

+// Number of simultaneously queued TCP segments.
+#ifdef MBED_CONF_LWIP_MEMP_NUM_TCP_SEG


Why #ifdef

If we use Mbed compilation, these values always come from mbed_lib.json

I think that there is no setting process in mbed_lib.json. However I'd like to set the customized value when coming from the mbed_lib.json and set the default value when not coming. Thus, I added #ifdef process to this file as with other elements.

This is consistent with other settings - basically not setting it in the JSON lets lwIP do its own default, which sometimes is not a simple constant we could express in the JSON.

Or could we - I guess you could define something as "5 * TCP_MSS". Maybe we could put all lwIP's defaults in as explicit JSON? I think it was only really those derived values that put us off doing that.

@kjbracey-arm
Thank you for your comments, and may I ask you a bit?
What does "5 * TCP_MSS" indicate? is it related with the value of "tcp-snd-buf" and "tcp-wnd" ?
If so, I defined them as "8 * TCP_MSS"(is customize value for RZ/A1) in json file.

SeppoTakalo · 2018-08-03T10:12:50Z

features/lwipstack/mbed_lib.json

@@ -72,6 +72,22 @@
            "help": "Maximum number of open UDPSocket instances allowed, including one used internally for DNS.  Each requires 84 bytes of pre-allocated RAM",
            "value": 4
        },
+        "memp-num-tcp-seg": {
+            "help": "Number of simultaneously queued TCP segments. Current default (used if null here) is set to 16 in opt.h, unless overridden by target Ethernet drivers.",
+            "value": null


Maybe we could use default values here, instead of null to get rid of #ifdef in lwipopts.h

I'd like to use the customized value(32) not default value(16) against this element for speeding up of RZ/A1. Thus by setting this element as null and overriding the value for RZ/A1 at following process, the other boards excepting RZ/A1 use default value and RZ/A1 use customized value.

mikaleppanen · 2018-08-06T08:54:36Z

features/lwipstack/mbed_lib.json

@@ -132,6 +148,11 @@
            "tcpip-thread-stacksize": 1328,
            "default-thread-stacksize": 640,
            "ppp-thread-stacksize": 896,
+            "memp-num-tcp-seg": 32,
+            "tcp-mss": 1460,


Should this be at max 1440 to make space for ipv6 header which fixed size is 40 bytes.

…ize.

TomoYamanaka · 2018-08-06T09:18:11Z

@mikaleppanen Thank you for your suggestion. I updated the override value.

TomoYamanaka · 2018-08-10T14:32:58Z

@mikaleppanen Any additional step required to this PR?

TomoYamanaka · 2018-08-16T00:18:07Z

@SeppoTakalo @kjbracey-arm Any additional required step ? Could you approval if no problem ?

TomoYamanaka · 2018-08-20T01:36:09Z

@SeppoTakalo @mikaleppanen
Thank you for your approval.

@0xc0170
Could you trigger the CI test?

TomoYamanaka · 2018-08-22T00:25:14Z

@0xc0170 or Someone of @ARMmbed/mbed-os-ipcore
Anything else to go ahead this PR ? I will appreciate if you can trigger CI test.

TomoYamanaka · 2018-08-27T12:44:40Z

@SeppoTakalo @mikaleppanen @kjbracey-arm
Is there required anything step to go ahead this PR? If nothing, could you remove review label?
This PR wil make GR-PEACH's users more activie.

0xc0170 · 2018-08-27T12:56:36Z

/morph build

0xc0170 · 2018-08-27T12:57:59Z

@TomoYamanaka In CI now. One question while you mentioned more active, there's one bug I've checked today: #6975. Can you please review it

mbed-ci · 2018-08-27T16:35:39Z

Build : SUCCESS

Build number : 2925
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7683/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-08-27T18:31:24Z

Exporter Build : SUCCESS

Build number : 2545
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7683/

TomoYamanaka · 2018-08-28T01:17:25Z

@0xc0170
Thank you for triggering CI test. I will check issue #6975.

mbed-ci · 2018-08-28T05:33:35Z

Test : FAILURE

Build number : 2676
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7683/2676

0xc0170 · 2018-08-28T07:46:29Z

/morph test

mbed-ci · 2018-08-28T19:23:07Z

Test : SUCCESS

Build number : 2685
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7683/2685

cmonr · 2018-08-28T20:26:38Z

@AnotherButler Do these config additions also need docs to go with them?

AnotherButler · 2018-08-28T20:54:05Z

@cmonr Yes, the storage config page needs to be updated with the additions:

https://github.com/ARMmbed/mbed-os-5-docs/blob/development/docs/reference/configuration/Connectivity.md

If general, #ifdefs should be converted to use the config system, but it sounds like there are reasons I don't really understand why we're not doing that here.

0xc0170 · 2018-08-29T08:19:16Z

If general, #ifdefs should be converted to use the config system, but it sounds like there are reasons I don't really understand why we're not doing that here.

As I understand, we use config here but need to translate the values to the lwip macros thus ifdef in this PR.

@SeppoTakalo Please correct me if wrong. Can you send the PR for updating the connectivity page?

0xc0170 · 2018-09-02T14:05:51Z

@ARMmbed/mbed-os-ipcore Can you review please again?

0xc0170 added the needs: review label Aug 3, 2018

0xc0170 requested a review from a team August 3, 2018 07:48

SeppoTakalo reviewed Aug 3, 2018

View reviewed changes

mikaleppanen suggested changes Aug 6, 2018

View reviewed changes

Change override value of "tcp-mss" to max 1440 byte for IPV6 header s…

e080561

…ize.

mikaleppanen approved these changes Aug 13, 2018

View reviewed changes

SeppoTakalo approved these changes Aug 16, 2018

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Aug 27, 2018

cmonr added needs: review release-version: 5.9.7 release-version: 5.10.0-rc1 and removed needs: CI labels Aug 28, 2018

cmonr removed the release-version: 5.9.7 label Aug 28, 2018

cmonr added needs: work and removed needs: review labels Aug 29, 2018

SeppoTakalo approved these changes Sep 2, 2018

View reviewed changes

0xc0170 approved these changes Sep 2, 2018

View reviewed changes

0xc0170 added ready for merge and removed needs: work labels Sep 2, 2018

cmonr merged commit 0edce1d into ARMmbed:master Sep 2, 2018

0xc0170 removed the ready for merge label Sep 2, 2018

TomoYamanaka deleted the Improve_LWIP branch September 3, 2018 01:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Renesas : Improve LWIP speed #7683

Renesas : Improve LWIP speed #7683

TomoYamanaka commented Aug 3, 2018 •

edited

Loading

SeppoTakalo commented Aug 3, 2018

SeppoTakalo Aug 3, 2018

TomoYamanaka Aug 3, 2018 •

edited

Loading

kjbracey Aug 6, 2018

TomoYamanaka Aug 8, 2018

SeppoTakalo Aug 3, 2018

TomoYamanaka Aug 3, 2018 •

edited

Loading

mikaleppanen Aug 6, 2018

TomoYamanaka commented Aug 6, 2018

TomoYamanaka commented Aug 10, 2018

TomoYamanaka commented Aug 16, 2018

TomoYamanaka commented Aug 20, 2018

TomoYamanaka commented Aug 22, 2018 •

edited

Loading

TomoYamanaka commented Aug 27, 2018

0xc0170 commented Aug 27, 2018

0xc0170 commented Aug 27, 2018

mbed-ci commented Aug 27, 2018

mbed-ci commented Aug 27, 2018

TomoYamanaka commented Aug 28, 2018 •

edited

Loading

mbed-ci commented Aug 28, 2018

0xc0170 commented Aug 28, 2018

mbed-ci commented Aug 28, 2018

cmonr commented Aug 28, 2018

AnotherButler commented Aug 28, 2018

0xc0170 commented Aug 29, 2018

0xc0170 commented Sep 2, 2018

Renesas : Improve LWIP speed #7683

Renesas : Improve LWIP speed #7683

Conversation

TomoYamanaka commented Aug 3, 2018 • edited Loading

Description

Pull request type

SeppoTakalo commented Aug 3, 2018

Choose a reason for hiding this comment

TomoYamanaka Aug 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomoYamanaka Aug 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomoYamanaka commented Aug 6, 2018

TomoYamanaka commented Aug 10, 2018

TomoYamanaka commented Aug 16, 2018

TomoYamanaka commented Aug 20, 2018

TomoYamanaka commented Aug 22, 2018 • edited Loading

TomoYamanaka commented Aug 27, 2018

0xc0170 commented Aug 27, 2018

0xc0170 commented Aug 27, 2018

mbed-ci commented Aug 27, 2018

Triggering tests

mbed-ci commented Aug 27, 2018

TomoYamanaka commented Aug 28, 2018 • edited Loading

mbed-ci commented Aug 28, 2018

0xc0170 commented Aug 28, 2018

mbed-ci commented Aug 28, 2018

cmonr commented Aug 28, 2018

AnotherButler commented Aug 28, 2018

0xc0170 commented Aug 29, 2018

0xc0170 commented Sep 2, 2018

TomoYamanaka commented Aug 3, 2018 •

edited

Loading

TomoYamanaka Aug 3, 2018 •

edited

Loading

TomoYamanaka Aug 3, 2018 •

edited

Loading

TomoYamanaka commented Aug 22, 2018 •

edited

Loading

TomoYamanaka commented Aug 28, 2018 •

edited

Loading