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

WiFi stack can freeze if CONFIG_ESP32_WIFI_RX_BA_WIN > CONFIG_ESP32_WIFI_STATIC_RX_BUFFER_NUM (IDFGH-1383) #3663

Closed
ssilverman opened this issue Jun 20, 2019 · 14 comments

Comments

@ssilverman
Copy link

Suggest you change the issue to this:

WiFi stack can freeze if CONFIG_ESP32_WIFI_RX_BA_WIN > CONFIG_ESP32_WIFI_STATIC_RX_BUFFER_NUM.

Tested on esp-idf v3.2

This configuration should be prevented or checked and an error thrown.

Originally posted by @negativekelvin in #3646 (comment)

@github-actions github-actions bot changed the title WiFi stack can freeze if CONFIG_ESP32_WIFI_RX_BA_WIN > CONFIG_ESP32_WIFI_STATIC_RX_BUFFER_NUM WiFi stack can freeze if CONFIG_ESP32_WIFI_RX_BA_WIN > CONFIG_ESP32_WIFI_STATIC_RX_BUFFER_NUM (IDFGH-1383) Jun 20, 2019
@ssilverman
Copy link
Author

See also: espressif/arduino-esp32#2899

@r1dd1ck
Copy link

r1dd1ck commented Jun 20, 2019

Well, are you sure it's this?

Because the iperf example uses:
WIFI_STATIC_RX_BUFFER_NUM=16
WIFI_RX_BA_WIN=32

and it does not "freeze" 😐

@negativekelvin
Copy link
Contributor

negativekelvin commented Jun 20, 2019

Well it solves the problem reported in espressif/arduino-esp32#2899 but maybe only happens "under certain conditions" or certain values so needs more constraints from wifi stack developers

@r1dd1ck
Copy link

r1dd1ck commented Jun 20, 2019

Yes, I certainly agree that changing the values "fixes" the reported issue 👍

But what I was pointing at is, whether the supplied conditional really is the underlying cause of the freezing stack, because it is in direct conflict with the iperf example defaults (which doesn't seem to show the symptoms).

Other than that, all is fine 🙂

@ssilverman
Copy link
Author

ssilverman commented Sep 26, 2019

This issue still happens in the v1.0.3 Arduino core: espressif/arduino-esp32#3287
There, CONFIG_ESP32_WIFI_RX_BA_WIN = CONFIG_ESP32_WIFI_STATIC_RX_BUFFER_NUM = 16.

@ssilverman
Copy link
Author

ssilverman commented Sep 26, 2019

For those following along, the stack now fails to receive more packets than defined by CONFIG_ESP32_WIFI_DYNAMIC_RX_BUFFER_NUM. Doing some spelunking for the word dynamic_rx_buf_num (the consumer of this define, as found with a simple grep) led me to tools/sdk/lib/libesp32.a, that's why I think the issue is in esp-idf as opposed to just in the Arduino core. (Forgive me if that cursory assumption is incorrect.)

@espxiehang
Copy link
Contributor

@ssilverman hi,I use your script to send upd packets, but I didn't reproduce the issue.
Can you give me an easy to reproduce method and your log?

@ssilverman
Copy link
Author

ssilverman commented Nov 14, 2019

When I filed this and related issues, this was the behaviour. However, the later Arduino releases seem to fix the problem. I haven’t dived in, but if you use settings from 1.0.2 (I think? See the issues in that repo that link to this one.), then you’ll see this behaviour. I realize this repo isn’t Arduino, and I haven’t explored much in the non-Arduino space of the esp-idf repo, but I think you’ll find the behaviour by using the settings mentioned in those Arduino versions.

There’s a bunch of related issues, but see this one (including the links there): espressif/arduino-esp32#2899
Also see here, where the comment author suggested I file this issue: #3646 (comment)

@negativekelvin
Copy link
Contributor

I think this can be closed with dd26caf#diff-fdc67a5904b65c3885aad20459aa1cef

@ssilverman
Copy link
Author

@negativekelvin was the source of this problem found?

@negativekelvin
Copy link
Contributor

negativekelvin commented Nov 15, 2019

Yes, enforce restrictions on the value of CONFIG_ESP32_WIFI_RX_BA_WIN per the commit. The other issues are all outside the scope of esp-idf code so I think you can close this. Unless you want to request backport to another version.

@ssilverman
Copy link
Author

ssilverman commented Nov 15, 2019

Right, but I meant: why does not meeting those conditions cause the problem? Is there a deeper problem somewhere? Should a comment in the code that uses these values make note of a fault potential? I’m just suggesting another commit that addresses that problem or at least makes a note of it somewhere. Other than that, I can certainly close this if you’re saying it’s fixed.

@r1dd1ck what do you think?

Edit: I re-read what you wrote and you stated that the real problem is outside esp-idf. I’ll close. But what do you think of @r1dd1ck’s comment that this disagrees with iperf?

@negativekelvin
Copy link
Contributor

The actual conditions in the commit are

(CONFIG_ESP32_WIFI_RX_BA_WIN ≤ CONFIG_ESP32_WIFI_DYNAMIC_RX_BUFFER_NUM) && (CONFIG_ESP32_WIFI_RX_BA_WIN ≤ 2 * CONFIG_ESP32_WIFI_STATIC_RX_BUFFER_NUM)

so iperf is not in violation

@espxiehang
Copy link
Contributor

Yes, its rules are defined in “components/esp_wifi/src/wifi_init.c”

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants