Skip to content
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.

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

update to lwIP-2.1.0: partial SACK support by default (de-selectable in menu) #5126

Merged
merged 33 commits into from
Oct 9, 2018

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Sep 12, 2018

fix #4176 and hopefully others

lwIP-2.0.3 was new reno only, SACK out support increases TCP reliability with high latency / long distance networks, also with large transfers (may help OTA, no guarantee though)

lwIP configuration changes are visible with scripts (unix, uses meld) in builder/glue-lwip/arduino/history/

…y was incorrectly enabled)

(ahah I scared you)
@d-a-v
Copy link
Collaborator Author

d-a-v commented Sep 12, 2018

Here is a capture of what meld gives using the conf editing/comparing scripts.

  • left: our 2.1.0 conf file
  • middle: original 2.1.0 conf file
  • right: our 2.0.3 conf file

sack

@devyte
Copy link
Collaborator

devyte commented Sep 22, 2018

@d-a-v is there anything pending here, besides testing?

@d-a-v
Copy link
Collaborator Author

d-a-v commented Sep 24, 2018

is there anything pending here, besides testing?

I don't think so
(a merge is needed though in lwip2, before merging here, because this PR does not point to current lwip2 master yet).

And the question of flash size increase remains. I'll try reduce it if I can.

@d-a-v d-a-v changed the title update to lwIP-2.1.0rc1: partial SACK support update to lwIP-2.1.0: partial SACK support by default (unselectable in menu) Sep 27, 2018
@d-a-v
Copy link
Collaborator Author

d-a-v commented Sep 27, 2018

Actually, lwip-2.1.0 is smaller than 2.0.3 with same options.
Larger by 2600B with SACK, that would be the default with this PR.
40 more ram bytes available in both cases.

lwIP-2.0.3 (before PR)

Sketch uses 273108 bytes (54%) of program storage space. Maximum is 499696 bytes.
Global variables use 40512 bytes (49%) of dynamic memory, leaving 41408 bytes for local variables. 

lwIP-2.1.0 w/o SACK (after PR, optional)

Sketch uses 272940 bytes (54%) of program storage space. Maximum is 499696 bytes.
Global variables use 40472 bytes (49%) of dynamic memory, leaving 41448 bytes for local variables. 

lwip-2.1.0 w/ SACK (after PR, default, fixes long transfers freezes - possibly OTA too)

Sketch uses 275708 bytes (55%) of program storage space. Maximum is 499696 bytes.
Global variables use 40472 bytes (49%) of dynamic memory, leaving 41448 bytes for local variables.

@d-a-v
Copy link
Collaborator Author

d-a-v commented Sep 27, 2018

| is there anything pending here, besides testing?

It is now ready

lwIP-2.1 changelog

@d-a-v d-a-v added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Sep 28, 2018
@d-a-v d-a-v changed the title update to lwIP-2.1.0: partial SACK support by default (unselectable in menu) update to lwIP-2.1.0: partial SACK support by default (de-selectable in menu) Oct 1, 2018
@CRCinAU
Copy link

CRCinAU commented Oct 2, 2018

I got pinged here from Issue #5023 - but I'm not exactly sure what to do or how to test this.

Anyone got instructions to verify?

@d-a-v
Copy link
Collaborator Author

d-a-v commented Oct 2, 2018

@CRCinAU

How to try:

git fetch origin pull/5126/head:lwip210
git checkout lwip210

Revert to master:

git checkout master

in case of updates in the PR, do this before restarting the above.

git checkout master
git branch -D lwip210

@CRCinAU
Copy link

CRCinAU commented Oct 2, 2018

@d-a-v Thanks. Which directory does this need to be in to ensure that it gets picked up within PlatformIO?

I assume there's something magic with paths.

For reference, I use the following right now in my platformio.ini:

[env:d1_mini]
platform = espressif8266
board = d1_mini
framework = arduino
board_build.flash_mode = qio
build_flags =
  -Wl,-Teagle.flash.1m0.ld
  -D PIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH

@d-a-v
Copy link
Collaborator Author

d-a-v commented Oct 2, 2018

About the PATH I can't tell.
Find your arduino core directory, rename it, replace it by the one from this PR (or clone this one)
I hope that's enough.

@CRCinAU
Copy link

CRCinAU commented Oct 2, 2018

Ok, so I started building with the stock framework for comparison:

DATA:    [====      ]  38.5% (used 31580 bytes from 81920 bytes)
PROGRAM: [===       ]  30.3% (used 310364 bytes from 1023984 bytes)

I replaced the package in ~/.platformio/packages/framework-arduinoespressif8266 with the lwip210 branch from your git:

DATA:    [====      ]  38.5% (used 31580 bytes from 81920 bytes)
PROGRAM: [===       ]  30.3% (used 310340 bytes from 1023984 bytes)

I flashed this second binary via USB, then tried to upload the same file via $ip/update. The transfer still hangs at 10%. I haven't yet tried with the ESP doing a pull via http.

@d-a-v
Copy link
Collaborator Author

d-a-v commented Oct 3, 2018

(edited)
I'm not sure, about PIO, if this PR is working or if this is the right way to replace the core.
Can you print Esp.getFullVersion() ?
(sizes should have changed)

@d-a-v
Copy link
Collaborator Author

d-a-v commented Oct 3, 2018

@ivankravets
Is this change sufficient ?

@ivankravets
Copy link
Collaborator

@d-a-v it seems that your additions are valid. In any case, you can run verbose build via $ pio run -v and see all build flags.

@CRCinAU you need to use this PR including modifications to https://github.com/esp8266/Arduino/blob/master/tools/platformio-build.py

@d-a-v
Copy link
Collaborator Author

d-a-v commented Oct 5, 2018

@Jeroen88 confirmed SACK efficiency.

@CRCinAU
Copy link

CRCinAU commented Oct 6, 2018

Ok, doesn't look like my method was working....

SDK:2.2.1(cfd48f3)/Core:2.4.2/lwIP:2.0.3(STABLE-2_0_3_RELEASE/glue:arduino-2.4.1-13-g163bb82)/BearSSL:6d1cefc

I'm not really familiar with PIO or the Arduino build environments as yet - so I'm still having that guesswork as to what I need to do...

Further info. I did this via:

cd ~/.platformio/packages/
mv framework-arduinoespressif8266 framework-arduinoespressif8266-stock
git clone https://github.com/esp8266/Arduino.git
mv Arduino framework-arduinoespressif8266-github
ln -s  framework-arduinoespressif8266-github  framework-arduinoespressif8266
cd  framework-arduinoespressif8266-github
git fetch origin pull/5126/head:lwip210
git checkout lwip210

I then rebuilt / reflashed the code - but only got the above versionings.

Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

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

Nothing jumps out at me, looks good.

@devyte devyte merged commit a1e59e9 into esp8266:master Oct 9, 2018
@d-a-v d-a-v deleted the lwip210 branch October 9, 2018 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ESP8266HttpClient never seems to finish large files when using lwip2
4 participants