Skip to content

netdev_default: migrate to netif_dev_default#19052

Open
jia200x wants to merge 3 commits intoRIOT-OS:masterfrom
jia200x:pr/netdev_default_selector
Open

netdev_default: migrate to netif_dev_default#19052
jia200x wants to merge 3 commits intoRIOT-OS:masterfrom
jia200x:pr/netdev_default_selector

Conversation

@jia200x
Copy link
Copy Markdown
Member

@jia200x jia200x commented Dec 14, 2022

Contribution description

This PR renames netdev_default to netif_dev_default due to the following reasons:

  1. With the on-going migration to non-netdev devices (e.g Radio HAL), netdev_default is misleading.
  2. From an IoT OS perspective, IMO it makes sense to bring "default network interfaces" instead of network devices, as the network devices are not necessarily complete (e.g a MAC layer may be missing).

At the same time, this PR allows to define granularity for the kind of network interfaces (e.g select only IEEE 802.15.4 devices/interfaces). This is specially useful for test applications such as ieee802154_hal, which still require any IEEE 802.15.4 device but does not require other embedded network devices (e.g WiFi, Ethernet).

So far, this PR only defines the MODULE_NETIF_DEV_IEEE802154_DEFAULT granularity, but we should add more on demand.

Testing procedure

Compile tests should be enough. Make sure TEST_KCONFIG=1 and TEST_KCONFIG=0 builds converge.

Issues/PRs references

It will simplify merging #18472.

@jia200x jia200x requested a review from benpicco December 14, 2022 14:36
@github-actions github-actions bot added Area: boards Area: Board ports Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Dec 14, 2022
@jia200x jia200x marked this pull request as ready for review December 15, 2022 10:32
@jia200x
Copy link
Copy Markdown
Member Author

jia200x commented Dec 15, 2022

no idea why it was marked as a draft. This is ready for review

@jia200x jia200x requested a review from maribu December 15, 2022 10:32
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 15, 2022
@riot-ci
Copy link
Copy Markdown

riot-ci commented Dec 15, 2022 •
edited
Loading

Murdock results

❌ FAILED

3bdab84 fixup! netdev_default: migrate to netif_dev_default

Success Failures Total Runtime
1526 0 6882 03m:05s

Artifacts

benpicco
benpicco reviewed Dec 15, 2022
View reviewed changes
@jia200x jia200x force-pushed the pr/netdev_default_selector branch from 06170ac to 66515b9 Compare December 15, 2022 16:23
@github-actions github-actions bot added the Area: examples Area: Example Applications label Dec 15, 2022
@jia200x jia200x 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 Jan 16, 2023
@jia200x jia200x mentioned this pull request Jan 18, 2023
Merged
@jia200x jia200x force-pushed the pr/netdev_default_selector branch from 66515b9 to c52c8e6 Compare January 20, 2023 14:27
@github-actions github-actions bot added Area: pkg Area: External package ports Platform: AVR Platform: This PR/issue effects AVR-based platforms Platform: ESP Platform: This PR/issue effects ESP-based platforms Platform: native Platform: This PR/issue effects the native platform labels Jan 20, 2023
benpicco
benpicco reviewed Jan 20, 2023
View reviewed changes
sys/net/gnrc/Makefile.dep Outdated
endif

ifneq (,$(filter gnrc_netdev_default,$(USEMODULE)))
ifneq (,$(filter gnrc_netif_dev_default,$(USEMODULE)))
Copy link
Copy Markdown
Contributor

@benpicco benpicco Jan 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This should probably remain gnrc_netdev_default

Copy link
Copy Markdown
Member Author

@jia200x jia200x Jan 24, 2023

Choose a reason for hiding this comment

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

uh, that's a result of sed. I will revert it.

@benpicco
Copy link
Copy Markdown
Contributor

benpicco commented Jan 20, 2023

Is there no fallback to include netif_dev_default if netdev_default is selected by an external application?

Do I understand it correctly that netif_dev_default would now select netif_dev_ieee802154_default, so that applications that don't care about the type of network interface will get whatever the board provides, while 6lo specific apps would select netif_dev_ieee802154_default?

(Although those would also work on esp_now, nrfmin, cc110x or nrf24l01p_ng - so should we have another netif_dev_6lo_default that then can select netif_dev_ieee802154_default?

@jia200x
Copy link
Copy Markdown
Member Author

jia200x commented Jan 24, 2023

Is there no fallback to include netif_dev_default if netdev_default is selected by an external application?

Yes, it was there, but it got replaced by sed. I will fix that immediately.

Do I understand it correctly that netif_dev_default would now select netif_dev_ieee802154_default, so that applications that don't care about the type of network interface will get whatever the board provides, while 6lo specific apps would select netif_dev_ieee802154_default?

Exactly.

(Although those would also work on esp_now, nrfmin, cc110x or nrf24l01p_ng - so should we have another netif_dev_6lo_default that then can select netif_dev_ieee802154_default?

That would make sense. But since there are too many link layers, I just wanted to start with the baseline + IEEE 802.15.4. We could then migrate apps and tests as needed.

@jia200x
Copy link
Copy Markdown
Member Author

jia200x commented Jan 24, 2023

done!

@jia200x jia200x mentioned this pull request Apr 13, 2023
Open
@jia200x
Copy link
Copy Markdown
Member Author

jia200x commented Apr 13, 2023

ping. It seems this will be required for #19015

@jia200x jia200x force-pushed the pr/netdev_default_selector branch from 26a679b to 3bdab84 Compare April 13, 2023 11:47
@benpicco
Copy link
Copy Markdown
Contributor

benpicco commented Apr 13, 2023

Kconfig is not happy still

@kYc0o kYc0o mentioned this pull request Apr 18, 2023
Open
@benpicco
Copy link
Copy Markdown
Contributor

benpicco commented May 31, 2023

This needs a rebase and Kconfig needs to be fixed.
What do you think of introducing netif_dev_6lo_default? I'm thinking of the border router in particular where we support more than just IEEE 802.15.4 for the 6lo part.

@jia200x
Copy link
Copy Markdown
Member Author

jia200x commented May 31, 2023

What do you think of introducing netif_dev_6lo_default? I'm thinking of the border router in particular where we support more than just IEEE 802.15.4 for the 6lo part.

Good idea. I can add that while rebase

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

Reviewers

@benpicco benpicco benpicco left review comments

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

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

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

@smlng smlng Awaiting requested review from smlng

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

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

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

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

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

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

@cgundogan cgundogan Awaiting requested review from cgundogan

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

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

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Area: boards Area: Board ports Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: examples Area: Example Applications Area: Kconfig Area: Kconfig integration Area: network Area: Networking Area: pkg Area: External package ports 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 Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: AVR Platform: This PR/issue effects AVR-based platforms Platform: ESP Platform: This PR/issue effects ESP-based platforms Platform: native Platform: This PR/issue effects the native platform

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jia200x @riot-ci @benpicco