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.

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

drivers/ethos: use luid_get_eui48() #13944

Merged
merged 1 commit into from
May 12, 2020

Conversation

benpicco
Copy link
Contributor

Contribution description

Previously the MAC address of the border router was entirely random.
That meant that as a DHCPv6 client it would get a new prefix with every reboot.

Due to #12210 the nodes will never use the new address.

Fix this by using luid_get_eui48() which will always return the same address across reboots.

It also makes the code simpler.

Testing procedure

  • Flash examples/gnrc_border_router with the default options (UPLINK=ethos).
  • Run ifconfig to view the address of the upstream interface.
  • reboot the border router
  • Inspect the address of the upstream interface again with ifconfig.

On master, the upstream interface will have a different address with each reboot.
This fixes that.

Issues/PRs references

Previously the MAC address of the border router was entirely random.
That meant that as a DHCPv6 client it would get a new prefix with every
reboot.

Due to RIOT-OS#12210 the nodes will never use the new address.

Fix this by using luid_get_eui48() which will always return the same
address across reboots.

It also makes the code simpler.
@benpicco benpicco added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 24, 2020
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

LGTM. @kaspar030 AFAIR the only reason it uses a random address, is because the luid mechanism did not exist back than, right?

@miri64 miri64 added Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Apr 24, 2020
@cgundogan cgundogan 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 Apr 24, 2020
@kaspar030
Copy link
Contributor

AFAIR the only reason it uses a random address, is because the luid mechanism did not exist back than, right?

yes!

Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

ACK.

@kaspar030 kaspar030 merged commit 4042fa2 into RIOT-OS:master May 12, 2020
@benpicco benpicco deleted the drivers/ethos-luid branch May 12, 2020 08:48
@miri64 miri64 added this to the Release 2020.07 milestone Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants