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

[PoC] gnrc_netif: revamp gnrc_netif_ops_t interface #18965

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jia200x
Copy link
Member

@jia200x jia200x commented Nov 24, 2022

Contribution description

This PoC revamps the GNRC Network Interface to make it independent of the source of events and adapt it better to slotted MAC layers.

The reason for doing this are:

  1. It allows any other mechanism besides IPC messages to interact with the network interface. For example, it is possible to implement an interface using event_thread, which may enable running multiple interfaces on a single thread.
  2. It decouples IRQ processing from the interface. Each link layer technology can decide what's the best way to process their events. Note that since we use the same thread to process IRQ and netif events, a relatively long get or set function will postpone IRQ offloading, which can be fatal for slotted MAC layers.
  3. It gives the freedom to each layer to implement the send mechanism. E.g some MACs may simple put a frame in the queue, while other may try to send immediately. E.g as it is now, gnrc_netif_pktq is redundant with the openDSME MAC queue.
  4. It simplifies migration of gnrc_netif to non-netdev devices. We can move the gnrc_netif thread to a separate file and use it as legacy code, while new link layers implement the new API.

Proposed changes

  1. Use the gnrc_netif_ops_t interface ( netif->ops->xxx functions) to interact with the network interface, instead of gnrc_netapi. I propose to use gnrc_netif_xxx functions as a wrapper, in case we need to structure common code.
  2. Rename netif->ops->send to netif->ops->schedule and adapt documentation to indicate the function schedules a transmission to be as soon as possible.
  3. Add a ctx parameter to the init function, which can be used to pass link layer specific information (e.g thread stacksize for current netdev implementations).
  4. Stop identifying an interface by the PID and simply use a fixed ID. In the PoC I kept kernel_pid_t for simplicity, but this could be any identifier.

PoC

To showcase the proposed changes, I wrote the new gnrc_netif_ops_t interface to use event loops instead of IPC messages. The implementation saves ~100 bytes of RAM and seems to perform slightly better than the current master:
This PR

2022-11-23 17:59:52,179 # 2000 packets transmitted, 2000 packets received, 0% packet loss
2022-11-23 17:59:52,183 # round-trip min/avg/max = 122.322/134.179/145.675 ms

Master

OLD:
2022-11-23 18:07:48,686 # 2000 packets transmitted, 2000 packets received, 0% packet loss
2022-11-23 18:07:48,691 # round-trip min/avg/max = 124.255/134.208/148.003 ms

Testing procedure

I only implemented the changes to gnrc_netif_ieee802154. Feel free to try e.g gnrc_networking with a IEEE 802.15.4 transceiver.

Issues/PRs references

There have been long discussions in the past with @miri64 in order to make gnrc_netif independent of the source of events. But now that #18156 and future MAC layers are very close, this makes IMO the transition smoother.

@github-actions github-actions bot added Area: examples Area: Example Applications Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework labels Nov 24, 2022
@jia200x jia200x requested review from benpicco and maribu November 24, 2022 13:50
@jia200x
Copy link
Member Author

jia200x commented Nov 24, 2022

If the change makes sense I could already implement a minimal version that migrates the gnrc_netif thread to the proposed mechanism. Then we don't need to migrate everything at once while new link layers may benefit from a slim gnrc_netif integration.

@jia200x jia200x marked this pull request as draft November 24, 2022 14:06
@jia200x
Copy link
Member Author

jia200x commented Dec 15, 2022

@miri64 seems to agree with this change (during an offline conversation).
What's your opinion @benpicco @maribu ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: examples Area: Example Applications Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant