netdev: add netdev_register() to keep track of netdev IDs by benpicco · Pull Request #13746 · RIOT-OS/RIOT

benpicco · 2020-03-28T19:07:46Z

Contribution description

This introduces the new type and index field to the netdev struct.
The type field is unique for each driver type and thus driver configuration struct. The index is the index of the device in the driver configuration struct.

For this the driver needs to call netdev_register() on setup.
Only a few drivers have been converted yet as an example.

This creates a unique and stable identifier for network devices that only depends on the board configuration.

With this we can continue (can be split off to a separate PR) to implement an EUI -> device mapping for boards that provide MAC addresses.

As a third step we can also extend luid_get_euiXX to use luid_custom for the fall-back case.
This will allow for stable addresses across driver resets and can be easily implemented as a follow-up to this.

Testing procedure

This is just adding IDs without using them, nothing to test.

Issues/PRs references

required for #14634

drivers/netdev_common/netdev_common.c

jia200x · 2020-03-30T07:51:25Z

nice one, we could use this for a "netdev_get_by_id" function.

drivers/include/net/netdev.h

benpicco · 2020-07-09T00:07:30Z

Ok I think I finally found a solution that I'm happy with.

uses 2 extra bytes of RAM / netdev
no need to update all drivers at once
boards can define which EUI providers are connected to which netdev

I can split this PR up, but with this comming all together the samr21-xpro and avr-rss2 will finally use the MAC address printed on their labels in RIOT.

drivers/include/net/netdev.h

Makefile.dep

miri64 · 2020-07-28T16:40:26Z

Please rebase #14632 was merged.

benpicco · 2020-08-02T19:47:20Z

Rebased 😉

benpicco · 2020-08-10T11:39:36Z

Ping?

fjmolinas · 2020-08-11T08:06:44Z

ping @miri64!

Currently don't have time to look into this in-depth :-(

maribu

ACK. Looks good to me and Murdock will do the testing for me :-)

It is desireable to have a way to identify network devices. This should be independent from the type of netdev, so a common identifier is needed. Base this on the driver ID and the index in the configuration struct. This way we achive unique IDs that stay consistent for any firmware flashed on a board.

benpicco · 2020-08-17T20:54:37Z

rebased to include #14113

leandrolanzieri · 2020-09-09T15:47:30Z

Makefile.dep

+# Don't register netdevs if there is only a single one  of them
+ifeq (,$(filter gnrc_netif_single,$(USEMODULE)))
+  USEMODULE += netdev_register
+endif


This is pulling netdev_register almost always now. Although it is a pseudomodule I think the dependency should be changed. AFIK there is no equivalent to gnrc_netif_single for other stacks, right? (@miri64 ?) Should this be added when some netdev_% module is used instead?

This is affecting #13565 now, as the unneeded dependency netdev_register is pulled.

netdev_register is a pseudomodule, so it won't do anything on it's own.
If not netdev_ code is used, this will not pull in any additional code.

But it changes the dependency resolution. This can indeed change the binary,

~~Isn't netdev_register a sub-module to netdev? If not, IMHO there is nothing speaking against it being one.~~ should have read the original question...

I guess we just missed a step in this discussion: #13746 (comment)

how to proceed with that?

but you just provided the solution yourself #13565 (comment)

I thought it was not intended to have a reverse dependency to gnrc_netif_single

I am fine with a reverse dependency how you implemented it. My original concern was about GNRC code in the network device code.

benpicco added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT State: demonstrator State: This PR (loosely) demonstrates a concept and is not necessarily meant to be merged. labels Mar 28, 2020

benpicco requested review from jia200x and miri64 March 29, 2020 21:24

miri64 reviewed Mar 30, 2020

View reviewed changes

drivers/netdev_common/netdev_common.c Outdated Show resolved Hide resolved

benpicco mentioned this pull request Jun 25, 2020

net/l2util: add eui_short/48/64_get() #12641

Closed

benpicco force-pushed the netdev-id branch from a058f49 to a7a79e7 Compare July 7, 2020 18:37

benpicco changed the title ~~drivers/netdev_common: add common netdev module to keep track of netdev IDs~~ netdev: add netdev_register() to keep track if netdev IDs Jul 7, 2020

benpicco force-pushed the netdev-id branch from a7a79e7 to 8b37d6f Compare July 7, 2020 18:47

jeandudey reviewed Jul 7, 2020

View reviewed changes

drivers/include/net/netdev.h Outdated Show resolved Hide resolved

benpicco force-pushed the netdev-id branch from f03e5dd to 18a8a4d Compare July 9, 2020 00:04

benpicco marked this pull request as ready for review July 9, 2020 00:04

benpicco requested review from MichelRottleuthner, PeterKietzmann, aabadie, cgundogan, daniel-k, dylad, fjmolinas, haukepetersen, leandrolanzieri and smlng as code owners July 9, 2020 00:04

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 9, 2020

benpicco mentioned this pull request Jul 9, 2020

drivers/kw41zrf: don't re-get UIDs during reset #14363

Closed

benpicco force-pushed the netdev-id branch 3 times, most recently from e351b47 to 230b541 Compare July 9, 2020 00:35

miri64 reviewed Jul 28, 2020

View reviewed changes

drivers/include/net/netdev.h Show resolved Hide resolved

benpicco force-pushed the netdev-id branch 2 times, most recently from e41d12d to 911e1d3 Compare July 28, 2020 09:45

miri64 previously requested changes Jul 28, 2020

View reviewed changes

drivers/include/net/netdev.h Outdated Show resolved Hide resolved

Makefile.dep Outdated Show resolved Hide resolved

This was referenced Jul 28, 2020

gnrc_netif: replace GNRC_NETIF_SINGLE with gnrc_netif_single pseudo-module #14632

Merged

net/link_layer: implement EUI provider #14634

Merged

benpicco force-pushed the netdev-id branch from 66c6f8a to 82bc4a6 Compare July 28, 2020 12:18

benpicco removed the State: demonstrator State: This PR (loosely) demonstrates a concept and is not necessarily meant to be merged. label Jul 28, 2020

benpicco force-pushed the netdev-id branch from 82bc4a6 to 4b73adc Compare July 28, 2020 16:56

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 7, 2020

benpicco requested a review from maribu August 17, 2020 16:08

benpicco changed the title ~~netdev: add netdev_register() to keep track if netdev IDs~~ netdev: add netdev_register() to keep track of netdev IDs Aug 17, 2020

maribu approved these changes Aug 17, 2020

View reviewed changes

maribu 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 Aug 17, 2020

benpicco and others added 5 commits August 17, 2020 22:50

drivers/at86rf2xx: register driver with netdev

53eedc2

drivers/at86rf215: register driver with netdev

4f19b54

drivers/dose: register driver with netdev

decadc4

drivers/at86rf2xx: fix typo

1582b10

benpicco force-pushed the netdev-id branch from 4b73adc to 1582b10 Compare August 17, 2020 20:54

benpicco merged commit e80407e into RIOT-OS:master Aug 17, 2020

benpicco deleted the netdev-id branch August 17, 2020 21:23

leandrolanzieri reviewed Sep 9, 2020

View reviewed changes

Conversation

benpicco commented Mar 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

Uh oh!

jia200x commented Mar 30, 2020

Uh oh!

Uh oh!

benpicco commented Jul 9, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miri64 commented Jul 28, 2020

Uh oh!

benpicco commented Aug 2, 2020

Uh oh!

benpicco commented Aug 10, 2020

Uh oh!

fjmolinas commented Aug 11, 2020

Uh oh!

maribu left a comment

Choose a reason for hiding this comment

Uh oh!

benpicco commented Aug 17, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miri64 Sep 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

benpicco commented Mar 28, 2020 •

edited

Loading

miri64 Sep 30, 2020 •

edited

Loading