gnrc_netif: make use of counting event
Contribution description
Can't we have a counting event type?
This adds a counting event type. If multiple events of the same type occur, the callback function is called for each of them instead of being called only once.
This solves the issue where if a network driver has a queue and generates an event for each queued packet, those packets will be stuck in the queue if gnrc_netif is not fast enough to process them in time before the next event occurs.
Testing procedure
Issues/PRs references
alternative to #18204, #18201
I changed my mind. Considering that we spend quite some effort on having the event handler thread(s) being merged to safe stack space, it is counter intuitive to now double the number of worker threads needed to handle both counting and non-counting events.
Could we instead just enqueue both counting and non-counting events into the same queue and use pointer tagging to mark those which are counting? In case counting events are not used (the module is not included), no pointer tagging is needed and no overhead is introduced. By using pointer tagger this would not blow up the size of event_t.
I definitely like the idea of having a "counter" event. However, after the discussed use case in gnrc_netif, I think we cannot guarantee a deterministic behavior for all devices when there are multiple RX Done events without some cooperation from the device drivers.
As it is now, the NETDEV_EVENT_ISR event is used to offload the _isr function to thread context. Therefore, this follows a semantic of "please offload asap" more than "process interrupts in the order they arrive". as the information for the latter case is discarded when calling _isr. Therefore, if a device is able to queue several RX frames before ISR offloading, the device will need to keep state of this anyway.
Therefore, I would be in favour of:
- Merging this PR anyway, as it enables some use cool cases
- Simply take https://github.com/RIOT-OS/RIOT/pull/18201 for the multiple RX frames before ISR offloading.
Opinions?
#18201 would certainly be the least invasive change. My concern for creating this PR was that I imagined many other places, where event users might be surprised by the singleton nature of posting events and having a solution ready for those subtle bugs.
For SLIP (and ethos) it would certainly be an argument to move forward with #18066 then as those would then also benefit from the common infrastructure in the chunked ringbuffer for queued frames.
My concern for creating this PR was that I imagined many other places, where event users might be surprised by the singleton nature of posting events and having a solution ready for those subtle bugs
Yup. As mentioned early, I still think this is a good idea as a module. We should definitely indicate the singleton behavior of normal events though.
Merging this PR anyway, as it enables some use cool cases
Sorry, I confused this PR. I meant, merging this mechanism (#18204 ), not the GNRC netif mechanism
Now I'm confused. #18204 makes all events counting and thus changes the API which may have undesired consequences.
Instead this PR introduces a separate counting event type (and tries to share most code with normal event) and makes use of that in gnrc_netif.
Ah sorry, you are right. What I meant is merging the counting mechanism, but not the usage on GNRC, as it's mainly used for IRQ offloading.
Now that it's the responsibility of the driver to handle this, we can close this PR.