Skip to content

lwip_sock: re-issue receive events if there are still received messages after *_recv() or *_accept() was called.#14060

Merged
MichelRottleuthner merged 1 commit intoRIOT-OS:masterfrom
miri64:lwip_sock/fix/re-recv-signal
May 22, 2020
Merged

lwip_sock: re-issue receive events if there are still received messages after *_recv() or *_accept() was called.#14060
MichelRottleuthner merged 1 commit intoRIOT-OS:masterfrom
miri64:lwip_sock/fix/re-recv-signal

Conversation

@miri64
Copy link
Member

@miri64 miri64 commented May 12, 2020 •
edited
Loading

Contribution description

This prevents a client from needing to call *_recv() or *_accept() in a loop.

Testing procedure

See #14034, but with USEPKG = lwip instead of using GNRC.

Issues/PRs references

Fixes #14034 when used with lwIP.
See also #14059 for the GNRC variant of this fix.

@miri64
lwip_sock: re-issue receive events if there are still received messages
ef1d458 
... after `*_recv()` or `*_accept()` was called.

This prevents a client from needing to call `*_recv()` or `*_accept()`
in a loop.
@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch CI: run tests If set, CI server will run tests on hardware for the labeled PR labels May 12, 2020
@miri64 miri64 mentioned this pull request May 12, 2020
Closed
@benpicco benpicco removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 12, 2020
@benpicco
Copy link
Contributor

benpicco commented May 12, 2020 •
edited
Loading

CI is broken right now, all tests run into timeouts leading to a failed job after ~2h

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 12, 2020
@miri64 miri64 mentioned this pull request May 15, 2020
Merged
@miri64
Copy link
Member Author

miri64 commented May 19, 2020

Ping? We should merge this soonish so the APIs with GNRC and lwIP don't get too much out of sync.

@MichelRottleuthner
Copy link
Contributor

MichelRottleuthner commented May 19, 2020

I'd be happy to test but I'm not sure how to properly build e.g. examples/gcoap with lwip instead of gnrc (never really used lwip before). The USEPKG = lwip doesn't seem to be everything that is needed and It's probably missing some module(s). Can you provide more details on that?

@miri64
Copy link
Member Author

miri64 commented May 19, 2020

Mh, I know @kb2ma had gcoap already running. Let me have a look if I can find it.

@miri64
Copy link
Member Author

miri64 commented May 19, 2020

#13641

MichelRottleuthner
MichelRottleuthner approved these changes May 22, 2020
View reviewed changes
Copy link
Contributor

@MichelRottleuthner MichelRottleuthner left a comment

Choose a reason for hiding this comment

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

Even with #13641 it doesn't seem possible to use the same procedure for testing as I described in #14034 because CoAp simply doesn't work for me with that. But that is probably related to #13641 itself.

Using tests/lwip I can confirm that using bare UDP and TCP with sock_async still works and this PR gives less dropped packets on fast incoming bursts. ACK.

@MichelRottleuthner MichelRottleuthner merged commit b13a4d6 into RIOT-OS:master May 22, 2020
@miri64 miri64 deleted the lwip_sock/fix/re-recv-signal branch May 22, 2020 14:28
@miri64 miri64 added this to the Release 2020.07 milestone Jun 24, 2020
@miri64 miri64 modified the milestone: Release 2020.07 Jul 23, 2020
@miri64 miri64 removed the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichelRottleuthner MichelRottleuthner MichelRottleuthner approved these changes

@kb2ma kb2ma Awaiting requested review from kb2ma

Assignees

No one assigned

Labels

Area: network Area: Networking Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2020.07

Development

Successfully merging this pull request may close these issues.

gcoap server breaks or becomes very unresponsive (sock_async related?)

3 participants

@miri64 @benpicco @MichelRottleuthner