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.

Sign up for GitHub

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

Jump to bottom

gnrc_sixlowpan_frag_sfr: provide CongURE support #16156

Merged
merged 3 commits into from
Oct 27, 2022
Merged

gnrc_sixlowpan_frag_sfr: provide CongURE support #16156

merged 3 commits into from
Oct 27, 2022

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Mar 5, 2021

Contribution description

This provides CongURE support for GNRC's 6LoWPAN SFR implementation and thus a to hook any CongURE congestion control implementation into SFR. The latter will be done in several follow-up PRs, this PR only adds the calling of the functions and tests to if the CongURE state machine is sane.

Testing procedure

Tests are provided in tests/gnrc_sixlowpan_frag_sfr_congure. They should pass

BOARD="<your choice>" make -C tests/gnrc_sixlowpan_frag_sfr_congure -j flash test

They are based mostly on tests/gnrc_sixlowpan_frag_sfr with some cleanup (and tests that do not check sending behavior removed) and additional checks for CongURE. I tried to provide reasoning for the checks in the comments. Please read them carefully.

Issues/PRs references

Depends on #16119 and #16133 and their respective dependencies.

PR dependency graph

@miri64 miri64 added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT 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 labels Mar 5, 2021
@miri64 miri64 requested review from benpicco and MrKevinWeiss March 5, 2021 14:25
@miri64 miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 5, 2021
This was referenced Mar 5, 2021
Open
Merged
@miri64 miri64 mentioned this pull request May 4, 2021
Merged
@miri64 miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/congure branch from 208aeea to 669caaa Compare May 4, 2021 09:38
@miri64 miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label May 4, 2021
@miri64
Copy link
Member Author

miri64 commented May 4, 2021 •
edited
Loading

Rebased and squashed to current master. This PR does not have any dependencies anymore.

@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels May 4, 2021
@miri64 miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/congure branch from 669caaa to a12b238 Compare May 4, 2021 11:35
@miri64
Copy link
Member Author

miri64 commented May 6, 2021

Rebased again.

@miri64 miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/congure branch from a12b238 to e6d0f4d Compare May 6, 2021 15:12
@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 22, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@riot-ci
Copy link

riot-ci commented Oct 12, 2022 •
edited
Loading

Murdock results

✔️ PASSED

bc24f9a tests: add test for 6LoWPAN SFR using CongURE

Success Failures Total Runtime
1996 0 1996 06m:36s

Artifacts

This only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now.

@miri64 miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/congure branch from d4f03f0 to fe8d824 Compare October 18, 2022 12:38
@miri64
Copy link
Member Author

miri64 commented Oct 18, 2022

Rebased to current master.

benpicco
benpicco reviewed Oct 18, 2022
View reviewed changes
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Open comments still apply 😉

tests/gnrc_sixlowpan_frag_sfr_congure/mockup_netif.c Outdated Show resolved Hide resolved
sys/net/gnrc/network_layer/sixlowpan/frag/sfr/gnrc_sixlowpan_frag_sfr.c Show resolved Hide resolved
sys/net/gnrc/network_layer/sixlowpan/frag/sfr/gnrc_sixlowpan_frag_sfr.c Show resolved Hide resolved
sys/net/gnrc/network_layer/sixlowpan/frag/sfr/gnrc_sixlowpan_frag_sfr.c Show resolved Hide resolved
@miri64 miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/congure branch from bbef956 to cbf2b58 Compare October 25, 2022 10:32
@kaspar030 kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 25, 2022
benpicco
benpicco approved these changes Oct 25, 2022
View reviewed changes
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

ACK, please squash

@miri64 miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/congure branch from 3e616d3 to bb5e982 Compare October 25, 2022 16:38
@miri64
Copy link
Member Author

miri64 commented Oct 25, 2022

Squashed

@miri64 miri64 removed the State: waiting for maintainer State: Action by a maintainer is required label Oct 25, 2022
@miri64 miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/congure branch from bb5e982 to bc24f9a Compare October 26, 2022 07:12
@miri64 miri64 enabled auto-merge October 26, 2022 13:49
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed 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 labels Oct 26, 2022
@benpicco
Copy link
Contributor

Failure of tests/gcoap_fileserver should be unrelated.

@miri64
Copy link
Member Author

miri64 commented Oct 27, 2022

What was the error?

@miri64 miri64 merged commit 9ae66be into RIOT-OS:master Oct 27, 2022
@benpicco
Copy link
Contributor

The test uses randomness so probabilistic failures are possible.
We should just set a seed value there…

@miri64 miri64 deleted the gnrc_sixlowpan_frag_sfr/feat/congure branch October 27, 2022 13:40
@miri64
Copy link
Member Author

miri64 commented Oct 27, 2022

The test uses randomness so probabilistic failures are possible.
We should just set a seed value there…

Yeah, or remove the randomness. ;-).

@benpicco
Copy link
Contributor

Yea I mean that's what the seed value would do

@miri64
Copy link
Member Author

miri64 commented Oct 27, 2022

Yea I mean that's what the seed value would do

I know. But why is there randomness in a test (that does not test randomness) in the first place ;-).

@benpicco
Copy link
Contributor

Because the test uses ZEP dispatcher with a set packet loss probability.

@kaspar030 kaspar030 added this to the Release 2023.01 milestone Jan 19, 2023
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

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

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

@cgundogan cgundogan Awaiting requested review from cgundogan

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

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

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

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

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

@smlng smlng Awaiting requested review from smlng

Assignees
No one assigned
Labels
Area: build system Area: Build system Area: Kconfig Area: Kconfig integration Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
Release 2023.01
Development

Successfully merging this pull request may close these issues.
6 participants
@miri64 @OlegHahm @riot-ci @benpicco @kaspar030 @MrKevinWeiss