Skip to content

gnrc/sock: recv avoid spinning xtimer#16831

Merged
benpicco merged 1 commit intoRIOT-OS:masterfrom
kfessel:p-gnrc-short-recv-short-timeout
Sep 9, 2021
Merged

gnrc/sock: recv avoid spinning xtimer#16831
benpicco merged 1 commit intoRIOT-OS:masterfrom
kfessel:p-gnrc-short-recv-short-timeout

Conversation

@kfessel
Copy link
Copy Markdown
Contributor

@kfessel kfessel commented Sep 9, 2021 •
edited
Loading

Contribution description

avoid spinning xtimer on socket receive with short timeout by increasing the timeout to XTIMER_BACKOFF

Testing procedure

run benchmark_udp with short intervalls

Issues/PRs references

possibly Fixes #16808

@kfessel kfessel requested a review from kaspar030 September 9, 2021 13:53
@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System labels Sep 9, 2021
@kfessel kfessel added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Sep 9, 2021
benpicco
benpicco reviewed Sep 9, 2021
View reviewed changes
@benpicco
Copy link
Copy Markdown
Contributor

benpicco commented Sep 9, 2021

Good catch!

@benpicco
Copy link
Copy Markdown
Contributor

benpicco commented Sep 9, 2021

Just squash directly

@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 Sep 9, 2021
@kfessel kfessel mentioned this pull request Sep 9, 2021
Open
@kfessel kfessel force-pushed the p-gnrc-short-recv-short-timeout branch from 2b560b7 to 6e5ddc3 Compare September 9, 2021 14:22
@kaspar030
Copy link
Copy Markdown
Contributor

kaspar030 commented Sep 9, 2021

my gcc says this:

/home/kaspar/src/riot/sys/net/gnrc/sock/gnrc_sock.c: In function 'gnrc_sock_recv':
/home/kaspar/src/riot/sys/net/gnrc/sock/gnrc_sock.c:121:5: error: "/*" within comment [-Werror=comment]
  121 |     /* the minimal not spinning timeout */
      |

kaspar030
kaspar030 reviewed Sep 9, 2021
View reviewed changes
kaspar030
kaspar030 reviewed Sep 9, 2021
View reviewed changes
@kfessel kfessel force-pushed the p-gnrc-short-recv-short-timeout branch from 6e5ddc3 to 9975e05 Compare September 9, 2021 15:15
benpicco
benpicco reviewed Sep 9, 2021
View reviewed changes
@kfessel kfessel force-pushed the p-gnrc-short-recv-short-timeout branch from 9975e05 to d192024 Compare September 9, 2021 15:16
@miri64
Copy link
Copy Markdown
Member

miri64 commented Sep 9, 2021

Might also need to be fixed in the other sock implementations.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Sep 9, 2021

(but that can be done as a follow-up)

@benpicco
Copy link
Copy Markdown
Contributor

benpicco commented Sep 9, 2021 •
edited
Loading

Hm, now tests/gnrc_sock_udp and tests/gnrc_sock_ip are failing

@kfessel kfessel force-pushed the p-gnrc-short-recv-short-timeout branch from d192024 to be28c53 Compare September 9, 2021 20:42
@kfessel
Copy link
Copy Markdown
Contributor Author

kfessel commented Sep 9, 2021

special case of 0 timout

@benpicco benpicco merged commit d04eb36 into RIOT-OS:master Sep 9, 2021
@miri64 miri64 mentioned this pull request Sep 16, 2021
Closed
@benpicco benpicco added this to the Release 2021.10 milestone Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benpicco benpicco benpicco approved these changes

@kaspar030 kaspar030 kaspar030 left review comments

@cgundogan cgundogan Awaiting requested review from cgundogan

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@miri64 miri64 Awaiting requested review from miri64 miri64 is a code owner

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

Assignees

No one assigned

Labels

Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2021.10

Development

Successfully merging this pull request may close these issues.

benchmark_udp: hammering with low interval causes issues

4 participants

@kfessel @benpicco @kaspar030 @miri64