-
Notifications
You must be signed in to change notification settings - Fork 2.1k
sys/net/gnrc/netif: allow checking if a netdev is legacy or new API #18426
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
Conversation
CI is not happy and a rebase is in order |
5a0145c
to
6bba244
Compare
c212344
to
48a0841
Compare
It seems to not be easy to enforce that either |
5c7ff85
to
2104e4c
Compare
2104e4c
to
cc4c34e
Compare
All green now :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
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_api
andnetdev_new_api
. As right now no netdev actually implements the new API, all netdevs pull innetdev_legacy_api
. Ifnetdev_legacy_api
is in used butnetdev_new_api
is not, we can safely assume at compile time that only legacy netdevs are in use. Similar, if onlynetdev_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.
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