sys/net/gnrc/netif: allow checking if a netdev is legacy or new API#18426
sys/net/gnrc/netif: allow checking if a netdev is legacy or new API#18426benpicco merged 1 commit intoRIOT-OS:masterfrom
Conversation
github-actions
bot
added
Area: build system
maribu
requested review from
MichelRottleuthner and
leandrolanzieri
as code owners
maribu
force-pushed
the
sys/net/gnrc/netif/api_check
branch
from
5a0145c to
6bba244
Compare
maribu
force-pushed
the
sys/net/gnrc/netif/api_check
branch
2 times, most recently
from
c212344 to
48a0841
Compare
|
It seems to not be easy to enforce that either |
maribu
force-pushed
the
sys/net/gnrc/netif/api_check
branch
2 times, most recently
from
5c7ff85 to
2104e4c
Compare
maribu
force-pushed
the
sys/net/gnrc/netif/api_check
branch
from
2104e4c to
cc4c34e
Compare
|
All green now :) |
benpicco
left a comment
benpicco
left a comment
There was a problem hiding this comment.
We can skip the compile test if you fix he doc error
A if `netdev_driver_t::confirm_send()` is provided, it provides the new netdev API. However, detecting the API at runtime and handling both API styles comes at a cost. This can be optimized in case only new or only old style netdevs are in use. To do so, this adds the pseudo modules `netdev_legacy_api` and `netdev_new_api`. As right now no netdev actually implements the new API, all netdevs pull in `netdev_legacy_api`. If `netdev_legacy_api` is in used but `netdev_new_api` is not, we can safely assume at compile time that only legacy netdevs are in use. Similar, if only `netdev_new_api` is used, only support for the new API is needed. Only when both are in use, run time checks are needed. This provides two helper function to check for a netif if the corresponding netdev implements the old or the new API. (With one being the inverse of the other.) They are suitable for constant folding when only new or only legacy devices are in use. Consequently, dead branches should be eliminated by the optimizer.
maribu
added
the
CI: skip compile test
maribu
force-pushed
the
sys/net/gnrc/netif/api_check
branch
from
376c643 to
276ad57
Compare
|
Once again, all green :) |
|
It looks like many new changes were introduced since it was last compiled, are you sure we should skip the compile check? |
|
The last run was 3 hours ago and I see no merges within the last three hours to |
|
A full CI run is 3h now and will randomly fail, currently it's best to avoid them if possible. |
|
Urgh looks like @MrKevinWeiss was right, sorry about that 😨 |
|
There were 5 hours between the last full CI run and the merge. The second CI run with build tests skipped came 4 hours after the full CI run and different in a typo fix of documentation. Unless we allow only fast forward merges like in release branches, or use GitLab style merge trains, or use bors, things like this will happen. The issue is not that the build tests were skipped. If there would be no typo and no rebuild with skipped build tests, a 5 hours old Murdock result would have been considered good enough and this would have been merged just the same. |
|
I'm a bit surprised this wasn't caught by CI in the last build, nothing about that code has changed. |
Nimble still implements the legacy netdev API. The `netdev_legacy_api` must be explicitly used, otherwise other network drivers that use the new API will overwrite it also for nimble. See RIOT-OS#18426.
Nimble still implements the legacy netdev API. The `netdev_legacy_api` must be explicitly used, otherwise other network drivers that use the new API will overwrite it also for nimble. See RIOT-OS#18426.
4 participants
Contribution description
A if
netdev_driver_t::confirm_send()is provided, it provides the new netdev API. However, detecting the API at runtime and handling both API styles comes at a cost. This can be optimized in case only new or only old style netdevs are in use.To do so, this adds the pseudo modules
netdev_legacy_apiandnetdev_new_api. As right now no netdev actually implements the new API, all netdevs pull innetdev_legacy_api. Ifnetdev_legacy_apiis in used butnetdev_new_apiis not, we can safely assume at compile time that only legacy netdevs are in use. Similar, if onlynetdev_new_apiis used, only support for the new API is needed. Only when both are in use, run time checks are needed.This provides two helper function to check for a netif if the corresponding netdev implements the old or the new API. (With one being the inverse of the other.) They are suitable for constant folding when only new or only legacy devices are in use. Consequently, dead branches should be eliminated by the optimizer.
Testing procedure
The provided functions are not actually used, so there is not really anything to test. It was only split out as the lwIP and the GNRC adaption to make use of
netdev_driver_t::confirm_send()both need this and this should be trivial to review.Issues/PRs references
None