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

Fix/enable UDP packet reassembly #7036

Merged
merged 8 commits into from Feb 22, 2020
Merged

Conversation

szekelyisz
Copy link
Contributor

UdpContext didn't care about pbuf chaining when receiving datagrams, leading
to fragments delivered to the application as individual packets.

UdpContext didn't care about pbuf chaining when receiving datagrams, leading
to fragments delivered to the application as individual packets.
@d-a-v d-a-v added component: network good first issue If you want to help, this is is a good place to start labels Jan 26, 2020
@d-a-v d-a-v added this to the 2.7.0 milestone Jan 26, 2020
Copy link
Collaborator

@d-a-v d-a-v left a comment

Choose a reason for hiding this comment

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

Thanks for fixing that (probably pending from the introduction of fragment/reassemble lwIP-v2 option).
You took the safe pbuf-logic way for lwip-1.4 (even if I think it does not have this option).

Out of curiosity, what was the UDP protocol which highlighted this bug ?

@szekelyisz
Copy link
Contributor Author

Thanks @d-a-v! Yeah I scratched my head a little because I found out in the meantime that reassembly is completely disabled in the LwIP 1.4 binary (although the feature is present in the source). Still I chose this approach for two reasons. First, having #ifs around every reference of pbuf->tot_len or pbuf->len is ugly. Second, someone could choose to recompile LwIP 1.4 with reassembly support, and it would be impolite to leave them with the original bug intentionally.

I came across this bug when I wanted to send data for WS2812 LED strips encoded in OSC packets as blobs. The usual MTU of 1500 bytes would leave me with around maximum 470 LEDs per strip, so I was wondering if I could use IP-level fragmentation to squeeze more data into a single datagram and avoid application-level fragmentation/reassembly. After making sure that LwIP had this feature and worked correctly I realized the bug was in the Arduino code. Now I can receive data for thousands of LEDs.

libraries/ESP8266WiFi/src/include/UdpContext.h Outdated Show resolved Hide resolved
libraries/ESP8266WiFi/src/include/UdpContext.h Outdated Show resolved Hide resolved
libraries/ESP8266WiFi/src/include/UdpContext.h Outdated Show resolved Hide resolved
libraries/ESP8266WiFi/src/include/UdpContext.h Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: network good first issue If you want to help, this is is a good place to start
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants