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

udp: fix again pbuf management #7132

Merged
merged 2 commits into from
Mar 4, 2020
Merged

udp: fix again pbuf management #7132

merged 2 commits into from
Mar 4, 2020

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Mar 3, 2020

This follows #7036 where udp pbuf chain handling is made hairy because of interleaved information fragment introduced with IPv6.

@szekelyisz Could you please test this change ?
fixes #7124

@d-a-v d-a-v added this to the 2.7.0 milestone Mar 3, 2020
@MarkusAD
Copy link

MarkusAD commented Mar 4, 2020

Confirming this does indeed fix #7124

@szekelyisz
Copy link
Contributor

Reassembly on IPv4 still works, no problem. Something's up though.

I'm sending 4 UDP packets with 1024 bytes payload each in quick succession. The first one is delivered immediately, but the remaining 3 are buffered by LwIP. How those are delivered depends on the commit.

  • Vanilla 2.6.2: all packets are correctly delivered to the application.
  • Fix/enable UDP packet reassembly #7036: packets in the buffer are dropped.
  • This PR: three packets are delivered, the first one contains all three of them interleaved with some garbage data, the second one contains the last two, again interleaved with some garbage, and the last one contains the last packet. I'm suspecting the garbage is the address info pbuf.

I can take a look tomorrow. Not tested IPv6 yet.

@d-a-v
Copy link
Collaborator Author

d-a-v commented Mar 4, 2020

This PR: three packets are delivered, the first one contains all three of them interleaved with some garbage data, the second one contains the last two, again interleaved with some garbage, and the last one contains the last packet. I'm suspecting the garbage is the address info pbuf.

I think (and while I'm writing it becomes clear) it is because of

    size_t getSize() const
    {
        if (!_rx_buf)
            return 0;

        return _rx_buf->tot_len - _rx_buf_offset;
    }

The info pbuf (the garbage) should not be accessible but it is because of getSize() (which should only report size before next info pbuf) and your fix (use of pbuf_contiguous()).
So an update of this PR is needed in order to cache this value for the current packet.

I can take a look tomorrow. Not tested IPv6 yet.

This is not needed. The code needed for IPv6 is also used for IPv4 and is the info pbuf management.

@d-a-v
Copy link
Collaborator Author

d-a-v commented Mar 4, 2020

@szekelyisz Can you please retry with latest commit ?

@szekelyisz
Copy link
Contributor

@szekelyisz Can you please retry with latest commit ?

Reassembly still works and smaller packets are also delivered correctly.

@d-a-v
Copy link
Collaborator Author

d-a-v commented Mar 4, 2020

Thanks for testing !

@d-a-v d-a-v merged commit b8e4ca4 into esp8266:master Mar 4, 2020
@d-a-v d-a-v deleted the udpchain branch March 4, 2020 20:40
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

Successfully merging this pull request may close these issues.

OTA broken after commit #7036
4 participants