Skip to content

dhcpv6_client: fix scaling of get_rand_us_factor#16661

Closed
miri64 wants to merge 1 commit intoRIOT-OS:masterfrom
miri64:dhcpv6_client/bug/RAND
Closed

dhcpv6_client: fix scaling of get_rand_us_factor#16661
miri64 wants to merge 1 commit intoRIOT-OS:masterfrom
miri64:dhcpv6_client/bug/RAND

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Jul 20, 2021

Contribution description

From what I can tell, this is a bug. The RFC defines RAND "a random number chosen with a uniform distribution between -0.1 and +0.1.", so we want to first take a number between 0μs and 200000μs and then subtract 100000μs from it to get the desired range, not take a number between 0μs and 200000μs and then subtract 100000000μs like it is the case in the current code.

Testing procedure

Since I found this bug only through code review, I'm not sure how to test this. I think the retransmission times of the DHCPv6 should now be far less erratic.

Issues/PRs references

None

@miri64 miri64 added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Jul 20, 2021
@miri64 miri64 requested a review from benpicco July 20, 2021 17:54
@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System labels Jul 20, 2021
@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 20, 2021
Copy link
Copy Markdown
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.

Needs a rebase though

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jul 27, 2021

Since #16668 changed the scaling to milliseconds anyways, this PR is no longer necessary.

@miri64 miri64 closed this Jul 27, 2021
@miri64 miri64 deleted the dhcpv6_client/bug/RAND branch July 27, 2021 20:17
@miri64 miri64 added the State: invalid State: The issue / PR is not valid label Jul 27, 2021
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jul 27, 2021

(cmp. this change and following).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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 State: invalid State: The issue / PR is not valid Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants