nekromant / esp8266-frankenstein

Alternative firmware for ESP8266 modules
318 stars 71 forks source link

Lwip update #76

Closed d-a-v closed 9 years ago

d-a-v commented 9 years ago

two main changes:

LWIP: Old src/lwip is reworked and moved to src/lwip-esp/, new one is in src/lwip-git/. Both are working and improved, lwip-git version is bigger in rom. make menuconfig selects the version you wish. For both of them, hard work has been done to reduce the espressif diff against original versions to its minimum. Scripts are provided in src/contrib/lwipupdate/ to watch espressif patches against original version, and to bump lwip-git if needed (lwip guys should tag their repo, the git version is named 1.4.1 although the official stable 1.4.1 does not come from the git repo). espressif version is based on something between 1.4.0RC1 and RC2. Timers management (sys_now()) has been reworked. espressif odd coarse timer removed. Original lwip timers are working (in both versions).

tcpservice: it is aimed to be a very small and efficient infrastructure for raw tcp. the optional echo service is able to run for hours (with the tester src/contrib/tcpechotester.c) and transfer megabits with no freeze and no leaks. The very much goal of this is for a direct and above all stable serial<->wlan service which hopefully come soon.

Note that I have not tested AP and APSTA modes at all.

enjoy :)

d-a-v commented 9 years ago

Forgot to tell: TCP_MSS and TCP_WND have been reduced to their minimum commonly allowed (536 bytes and 2*MSS respectively). This reduces the ram footprint (works on both versions). Special care has to be done when using raw lwip and espressif blobs, I tried to be clear in include/tcpservice.h.

nekromant commented 9 years ago

That looks like one hell of awesomeness. Just a few questions though:

  1. Why don't we just ditch all the ICACHE_FLASH_ATTR for good? Antares already does all the relocating, so why bother? When we do realocation ICACHE_FLASH_ATTR is not only unneeded - it can be even harmful (There's a corner case, if ICACHE_FLASH_ATTR function is present and there's also a function explicitly put into iram0 - the linker magic will break and the binary won't work).
  2. May be it's better to move the lwip to antares itself and escape all the espressif's hacks with #ifdef CONFIG_ARCH_ESP8266 ?
d-a-v commented 9 years ago

On mar., avril 21, 2015 at 10:06:33 -0700, Andrew wrote:

That looks like one hell of awesomeness.

head banging for this to happen. really. https://www.youtube.com/watch?v=q8SWMAQYQf0 s/unreal tournament/lwip/g

Just a few questions though:

  1. Why don't we just ditch all the ICACHE_FLASH_ATTR for good? Antares already does all the relocating, so why bother?

They are. There's one left in cmd_listen.c which I did not touch.

If I understand well (according to what you told me before - and correct me if I'm wrong), ICACHE_FLASH_ATTR makes the code copied and executed in ram, so OTA is possible.

Currently antares, thanks to a kcnf option, artificially tags all functions to move them in ram (we don't need ICACHE_FLASH_ATTR anymore).

What would be interesting is to tag only the minimum subset (including lwip and some others) for OTA, and some few others for speed, than leave the rest in rom. We would gain ram space. All we have to do is to put open-source objects into two differents library, and tag only one.

Right ?

  1. May be it's better to move the lwip to antares itself and escape all the espressif's hacks with #ifdef CONFIG_ARCH_ESP8266 ?

We could do that. But I guess the #if and #ifdef names need to be rock-clear. And it would be better for antares a taggued or released version of lwip.

david

nekromant commented 9 years ago

david gauchard писал 21.04.2015 20:23:

On mar., avril 21, 2015 at 10:06:33 -0700, Andrew wrote:

That looks like one hell of awesomeness.

head banging for this to happen. really. https://www.youtube.com/watch?v=q8SWMAQYQf0 s/unreal tournament/lwip/g

Just a few questions though:

  1. Why don't we just ditch all the ICACHE_FLASH_ATTR for good? Antares already does all the relocating, so why bother?

They are. There's one left in cmd_listen.c which I did not touch.

If I understand well (according to what you told me before - and correct me if I'm wrong), ICACHE_FLASH_ATTR makes the code copied and executed in ram, so OTA is possible.

Nope, the opposite. Epressif's stuff works like this:

.text gets copied and executed from RAM .irom0.text gets executed from flash as is.

ICACHE_FLASH_ATTR instructs compiler to put function into irom0.text

To avoid that ICACHE_FLASH_ATTR everywhere antares does the following:

  1. Adds a new section iram0.text that always goes to RAM, just like .text
  2. Renames .text to irom0.text in all our code AFTER compilation before linking.
  3. Leaves all the blobs that have .text in ram as is, not to break anything.

Therefore, ICACHE_FLASH_ATTR is no longer needed, so that we only specify section name for stuff we really want in RAM (like we do in tftp firmware sync code). Since there's not much RAM available, we normally don't want that. Yes, it's an ugly hack and black magic, but the sole purpose of that was so that we can avoid hacking the code of every little bit to port it to esp8266.

Currently antares, thanks to a kcnf option, artificially tags all functions to move them in ram (we don't need ICACHE_FLASH_ATTR anymore).

What would be interesting is to tag only the minimum subset (including lwip and some others) for OTA, and some few others for speed, than leave the rest in rom. We would gain ram space. All we have to do is to put open-source objects into two differents library, and tag only one.

Right ?

  1. May be it's better to move the lwip to antares itself and escape all the espressif's hacks with #ifdef CONFIG_ARCH_ESP8266 ?

We could do that. But I guess the #if and #ifdef names need to be rock-clear. And it would be better for antares a taggued or released version of lwip.

david

Reply to this email directly or view it on GitHub [1].

*

Links:

[1] https://github.com/nekromant/esp8266-frankenstein/pull/76#issuecomment-94878860

Regards, Andrew

nekromant commented 9 years ago

In other words, the rule of thumb for frankenstein is - screw the ICACHE_FLASH_ATTR and never use it.

d-a-v commented 9 years ago

got it. The only one left in cmd_listen.c is not mine. I'll modify cmd_listen to use tcpservice instead if needed.

d-a-v commented 9 years ago

For the record, I have now 5 echoservice running concurrently for 17.5 hours. They transfered 2.75GBytes each so far, at an average bitrate of 1800 kbits/s overall (on a busy wifi network). The bit rate could be better at the price of bigger buffer (and less free ram). But my thinking is that there is no need for a large bandwidth, if it is at least above the maximal serial bitrate. I think this stability is thanks to the timer management which was not working well from the beginning. Before this patch, I had some transfers which stopped on heavy load, I guess this was because of the bad timer infrastructure which caused the tcp RTO (retransmit time out) to malfunction.

nekromant commented 9 years ago

@d-a-v I'm talking about ICACHE_FLASH_ATTR as seen in for example src/lwip/include/ipv4/lwip/icmp.h and other lwip headers. Do we really need them? As for the rest of the codebase, I used a sed script to purge them all already, so whatever's in cmd_listen.c - it must be a leftover, I'll fix it right away.

d-a-v commented 9 years ago

src/lwip does not exist anymore. It is now src/lwip-esp/ and there is no ICACHE_FLASH_ATTR in the pull request.

nekromant commented 9 years ago

@d-a-v I gave your lwip branch a quick try, looks like most stuff works, so I've merged it into master.

d-a-v commented 9 years ago

On sam., avril 25, 2015 at 11:08:00 -0700, Andrew wrote:

@d-a-v I gave your lwip branch a quick try, looks like most stuff works, so I've merged it into master.

Thanks, I have a timer management fix. I'll push in a short while.

nekromant commented 9 years ago

@d-a-v Yes, and mind that we have something broken on the latest SDK. I've updated the README, but had no chance to troubleshoot it yet.