Skip to content

congure_abe: initial import of TCP Alternative Backoff with ECN for CongURE #15968

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
Merged
merged 2 commits into from
Oct 18, 2022
Merged

congure_abe: initial import of TCP Alternative Backoff with ECN for CongURE #15968

merged 2 commits into from
Oct 18, 2022

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Feb 9, 2021 •
edited
Loading

Contribution description

This provides an implementation for CongURE of the congestion control mechanism of TCP Alternative Backoff with ECN (TCP ABE) as specified in RFC 8511.

Testing procedure

Tests are provided for which riotctrl (at least v0.2.2) is required for them to run. If installed,

make -j -C tests/congure_abe flash test

should succeed.

Issues/PRs references

Requires #15951 (merged) and #15953 (merged). When merged, I will adapt the test framework to use the turo framework introduced in #15950 (merged), which was the original idea to begin with (which is why it is already printing JSON strings).

PR dependency graph

@miri64 miri64 added Area: sys Area: System Type: new feature The issue requests / The PR implemements a new feature for RIOT State: waiting for other PR State: The PR requires another PR to be merged first labels Feb 9, 2021
@miri64 miri64 changed the title Congure/feat/congure abe congure_abe: initial import of TCP Alternative Backoff with ECN for CongURE Feb 9, 2021
@miri64 miri64 force-pushed the congure/feat/congure_abe branch 2 times, most recently from ae08061 to 3cc1464 Compare February 12, 2021 08:40
@miri64 miri64 force-pushed the congure/feat/congure_abe branch from 3cc1464 to 12596cf Compare March 3, 2021 09:10
@miri64
Copy link
Member Author

miri64 commented Mar 3, 2021

Rebased and adapted to master and dependencies.

@miri64 miri64 mentioned this pull request Mar 9, 2021
Merged
@miri64 miri64 force-pushed the congure/feat/congure_abe branch from d2fde2d to acdf835 Compare May 6, 2021 15:10
@miri64
Copy link
Member Author

miri64 commented May 6, 2021

Rebased and squashed to current master.

@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 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.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Mar 2, 2022
@miri64 miri64 removed the State: stale State: The issue / PR has no activity for >185 days label Mar 2, 2022
@miri64 miri64 mentioned this pull request Oct 12, 2022
Merged
@miri64 miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Oct 17, 2022
@miri64 miri64 force-pushed the congure/feat/congure_abe branch from acdf835 to a5af3b6 Compare October 17, 2022 13:12
@miri64 miri64 requested a review from MrKevinWeiss as a code owner October 17, 2022 13:12
@miri64
Copy link
Member Author

miri64 commented Oct 17, 2022

Rebased to current master.

@miri64
Copy link
Member Author

miri64 commented Oct 17, 2022

(no longer has any dependencies)

@github-actions github-actions bot added Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework labels Oct 17, 2022
@miri64 miri64 added CI: run tests If set, CI server will run tests on hardware for the labeled PR CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 17, 2022
@riot-ci
Copy link

riot-ci commented Oct 17, 2022 •
edited
Loading

Murdock results

✔️ PASSED

a4e7c93 tests: Initial import of congure_abe tests

Success Failures Total Runtime
1993 0 1993 07m:26s

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 congure/feat/congure_abe branch from a5af3b6 to 0bb595d Compare October 17, 2022 13:22
benpicco
benpicco reviewed Oct 17, 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.

Looks good to me, just to clarify: This is not used by GNRC TCP but by 6Lo SFR, right?

@miri64
Copy link
Member Author

miri64 commented Oct 17, 2022

Looks good to me, just to clarify: This is not used by GNRC TCP but by 6Lo SFR, right?

At the moment, yes. But nothing speaks against adding congure to GNRC TCP in the future (maybe we need a separate congure implementation though that utilizes the lessons learned from TCPlp).

@miri64 miri64 force-pushed the congure/feat/congure_abe branch from 8556a3b to 28d55cb Compare October 17, 2022 14:42
@miri64 miri64 force-pushed the congure/feat/congure_abe branch from bd1430f to 4bc4d9c Compare October 17, 2022 14:44
@miri64 miri64 enabled auto-merge October 17, 2022 15:01
@miri64 miri64 force-pushed the congure/feat/congure_abe branch from 4bc4d9c to a4e7c93 Compare October 18, 2022 07:23
@miri64 miri64 merged commit 07c04bc into RIOT-OS:master Oct 18, 2022
@aabadie
Copy link
Contributor

aabadie commented Oct 19, 2022 •
edited
Loading

For some reason the test of this PR failed in the latest nightly job.

@aabadie
Copy link
Contributor

aabadie commented Oct 19, 2022

And the test was not run for this PR even if "run tests" was set...

@miri64 miri64 deleted the congure/feat/congure_abe branch October 19, 2022 06:40
@miri64
Copy link
Member Author

miri64 commented Oct 19, 2022

Will have a look. but looks like the synchronization issue we often saw in the past.

@miri64 miri64 mentioned this pull request Oct 19, 2022
Merged
@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

@maribu maribu Awaiting requested review from maribu

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri 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

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

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

@smlng smlng Awaiting requested review from smlng

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

Assignees
No one assigned
Labels
Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration 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 CI: run tests If set, CI server will run tests on hardware 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 @riot-ci @aabadie @benpicco @kaspar030 @MrKevinWeiss