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 into
RIOT-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 into
RIOT-OS:masterfrom
miri64:lwip_sock/fix/re-recv-signal

Conversation

@miri64

@miri64 miri64 commented May 12, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

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

benpicco commented May 12, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

miri64 commented May 19, 2020

Copy link
Copy Markdown
Member Author

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

@MichelRottleuthner

MichelRottleuthner commented May 19, 2020

Copy link
Copy Markdown
Contributor

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

miri64 commented May 19, 2020

Copy link
Copy Markdown
Member Author

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

@miri64

miri64 commented May 19, 2020

Copy link
Copy Markdown
Member Author

#13641

MichelRottleuthner
MichelRottleuthner approved these changes May 22, 2020
View reviewed changes

@MichelRottleuthner MichelRottleuthner left a comment

Copy link
Copy Markdown
Contributor

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