gnrc_netif: Add support for internal event loop #9326

jnohlgard · 2018-06-11T14:15:01Z

Contribution description

Enabled by the gnrc_netif_events pseudo module. Using an internal event loop eliminates the risk of lost interrupts and lets ISR events always be handled before any send/receive requests from other threads are processed.
The events in the event loop is also a potential hook for MAC layers and other link layer modules which may need a way to inject and process events before any external IPC messages are handled.

Issues/PRs references

miri64

The main conceptual question I have, is why you are under the impression that messages are ordered and synchronous and events are none of those.

sys/include/net/gnrc/netif.h

sys/net/gnrc/netif/gnrc_netif.c

sys/include/net/gnrc/netif.h

sys/net/gnrc/netif/gnrc_netif.c

jnohlgard · 2018-06-13T14:44:15Z

@miri64 Perhaps a bad choice of words. The idea is to make sure that ISRs will always be executed before any queued external requests (send/recv/get/set). In the future, it will also be useful to be able to insert other implementation specific events which can be handled first just like the ISR event, for example turning on the radio when it is time for a wake period in a duty cycling MAC, or handling timeouts and other exceptional events.

jnohlgard · 2018-06-13T14:49:54Z

There's also the concept that there should only ever be a need for exactly one queued ISR event in the event loop, since the radio driver should probably take care of everything that has to be done in one go and the duplicated ISR events will just be no-ops, whereas there may be several queued send events with different data.

sys/include/net/gnrc/netif.h

jnohlgard

@miri64 addressed comments, rebased

miri64

@gebart do you have something prepared to test this?

sys/net/gnrc/netif/gnrc_netif.c

jnohlgard · 2018-08-24T09:44:15Z

@miri64 rebased, refactored to address comment.
Testing:
Add USEMODULE += gnrc_netif_events to the makefile. I have added a test commit for this change in the gnrc_networking example.

jnohlgard · 2018-08-24T09:51:10Z

@miri64 The ContikiMAC implementation requires this event loop to be able to schedule waking periods and retransmissions properly, using the message queue would let applications interfere with the timing of wake periods and retransmissions, or we would have to perform tasks from within interrupt context to preempt the normal flow of the thread.

sys/net/gnrc/netif/gnrc_netif.c

miri64 · 2018-08-24T13:15:11Z

I think apart from the ifdef issue pointed out above, I think I'm alright with this, but I need to see when the diff is better trackable.

sys/net/gnrc/netif/gnrc_netif.c

jnohlgard · 2018-08-24T20:46:46Z

@miri64 new version pushed. The block you mentioned was rewritten into a separate function, also the behaviour was slightly altered to let the thread check for events between each successive IPC message, even when more than one msg have been queued. This should make the behaviour more predictable and robust. Events are always processed as soon as possible, and any events which occur as a side effect of one of the processed messages will be handled before the next message is processed.

miri64 · 2018-08-24T20:52:37Z

Ok, finally before I give my ACK, some doc on the new behavior would be nice (also mention that events take precedence over messages and ISR events are handled as a event)... I don't know exactly this should go though :-/. Maybe in a newly introduced details section in net/gnrc/netif.h?

Enabled by the gnrc_netif_events pseudo module. Using an internal event loop within the gnrc_netif thread eliminates the risk of lost interrupts and lets ISR events always be handled before any send/receive requests from other threads are processed. The events in the event loop is also a potential hook for MAC layers and other link layer modules which may need to inject and process events before any external IPC messages are handled.

…t loop

…working example

miri64 · 2019-01-28T15:44:25Z

If this PR is rebased (and squashed) and #9326 (comment) is addressed optionally, I think we can finally merge this (sorry dropped off my radar again >.<)

miri64 · 2019-02-16T13:02:48Z

@gebart ping?

miri64 · 2019-03-21T13:43:23Z

Ping @gebart?

miri64 · 2019-03-21T14:08:39Z

sys/net/gnrc/netif/gnrc_netif.c

        /* dispatch netdev, MAC and gnrc_netapi messages */
+        DEBUG("gnrc_netif: message %u\n", (unsigned)msg.type);
        switch (msg.type) {
            case NETDEV_MSG_TYPE_EVENT:


I believe this case can be #ifndef'd

jia200x · 2019-09-30T12:24:17Z

@gebart this is indeed an interesting feature! Thanks

Since there's no activity for some time, mind if I take it?

stale · 2020-04-10T12:25:24Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

miri64 · 2020-04-14T08:27:33Z

Adopted by @bergzand in #13669.

jnohlgard added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT GNRC CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 11, 2018

jnohlgard added this to the Release 2018.07 milestone Jun 11, 2018

jnohlgard requested a review from miri64 June 11, 2018 14:15

miri64 requested changes Jun 13, 2018

View reviewed changes

sys/include/net/gnrc/netif.h Outdated Show resolved Hide resolved

sys/net/gnrc/netif/gnrc_netif.c Outdated Show resolved Hide resolved

sys/include/net/gnrc/netif.h Outdated Show resolved Hide resolved

sys/net/gnrc/netif/gnrc_netif.c Outdated Show resolved Hide resolved

jnohlgard force-pushed the pr/gnrc_netif_async_events branch from 1dfdad0 to db4c8ce Compare June 30, 2018 14:09

jnohlgard changed the title ~~gnrc_netif: Add support for asynchronous events~~ gnrc_netif: Add support for internal event loop Jun 30, 2018

jnohlgard commented Jun 30, 2018

View reviewed changes

sys/include/net/gnrc/netif.h Show resolved Hide resolved

jnohlgard commented Jun 30, 2018

View reviewed changes

miri64 reviewed Jul 2, 2018

View reviewed changes

sys/net/gnrc/netif/gnrc_netif.c Outdated Show resolved Hide resolved

jnohlgard mentioned this pull request Jul 2, 2018

contikimac: a ContikiMAC compatible netdev layer #9478

Closed

cladmi modified the milestones: Release 2018.07, Release 2018.10 Aug 1, 2018

jnohlgard force-pushed the pr/gnrc_netif_async_events branch from d5f76b3 to 9980882 Compare August 24, 2018 09:21

cladmi 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 Aug 24, 2018

miri64 requested changes Aug 24, 2018

View reviewed changes

sys/net/gnrc/netif/gnrc_netif.c Outdated Show resolved Hide resolved

jnohlgard commented Aug 24, 2018

View reviewed changes

sys/net/gnrc/netif/gnrc_netif.c Show resolved Hide resolved

miri64 reviewed Aug 24, 2018

View reviewed changes

sys/net/gnrc/netif/gnrc_netif.c Show resolved Hide resolved

jnohlgard force-pushed the pr/gnrc_netif_async_events branch from ef37898 to 7e873e3 Compare August 25, 2018 13:50

jnohlgard force-pushed the pr/gnrc_netif_async_events branch from 2338476 to 4462fd5 Compare September 12, 2018 14:37

miri64 mentioned this pull request Nov 29, 2018

[RFC] gnrc_netif: Group multiple devices/connections into single thread? #10496

Open

Joakim Nohlgård added 6 commits December 1, 2018 14:08

fixup! gnrc_netif: Add support for internal event loop

05cfc2a

fixup! fixup! gnrc_netif: Add support for internal event loop

652a928

fixup! fixup! fixup! gnrc_netif: Add support for internal event loop

7973668

fixup! fixup! fixup! fixup! gnrc_netif: Add support for internal even…

8fb3dc0

…t loop

squash REMOVE ME: test commit to enable gnrc_netif_events in gnrc_net…

39a29bb

…working example

jnohlgard force-pushed the pr/gnrc_netif_async_events branch from 44c1fc2 to 39a29bb Compare December 1, 2018 13:08

aabadie modified the milestones: Release 2019.01, Release 2019.04 Jan 25, 2019

miri64 reviewed Mar 21, 2019

View reviewed changes

miri64 modified the milestones: Release 2019.04, Release 2019.07 May 7, 2019

miri64 mentioned this pull request Jun 7, 2019

[RFC] netdev: change receive mechanism #11652

Closed

MrKevinWeiss modified the milestones: Release 2019.07, Release 2019.10 Jul 16, 2019

kb2ma removed this from the Release 2019.10 milestone Oct 8, 2019

This was referenced Oct 10, 2019

core/tasklet: add initial support #12420

Closed

gnrc_netif: use irq_handler instead of IPC #12462

Closed

jia200x mentioned this pull request Nov 13, 2019

RFC: lower network stack rework #12688

Closed

14 tasks

miri64 mentioned this pull request Mar 12, 2020

netdev_event_thread: add support for event_thread irq handler #13573

Closed

bergzand mentioned this pull request Mar 20, 2020

gnrc_netif: Add support for internal event loop #13669

Merged

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Apr 10, 2020

miri64 closed this Apr 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gnrc_netif: Add support for internal event loop #9326

gnrc_netif: Add support for internal event loop #9326

jnohlgard commented Jun 11, 2018 •

edited

Loading

miri64 left a comment

jnohlgard commented Jun 13, 2018

jnohlgard commented Jun 13, 2018

jnohlgard left a comment

miri64 left a comment

jnohlgard commented Aug 24, 2018

jnohlgard commented Aug 24, 2018

miri64 commented Aug 24, 2018

jnohlgard commented Aug 24, 2018

miri64 commented Aug 24, 2018

miri64 commented Jan 28, 2019

miri64 commented Feb 16, 2019

miri64 commented Mar 21, 2019

miri64 Mar 21, 2019

jia200x commented Sep 30, 2019 •

edited

Loading

stale bot commented Apr 10, 2020

miri64 commented Apr 14, 2020

gnrc_netif: Add support for internal event loop #9326

gnrc_netif: Add support for internal event loop #9326

Conversation

jnohlgard commented Jun 11, 2018 • edited Loading

Contribution description

Issues/PRs references

miri64 left a comment

Choose a reason for hiding this comment

jnohlgard commented Jun 13, 2018

jnohlgard commented Jun 13, 2018

jnohlgard left a comment

Choose a reason for hiding this comment

miri64 left a comment

Choose a reason for hiding this comment

jnohlgard commented Aug 24, 2018

jnohlgard commented Aug 24, 2018

miri64 commented Aug 24, 2018

jnohlgard commented Aug 24, 2018

miri64 commented Aug 24, 2018

miri64 commented Jan 28, 2019

miri64 commented Feb 16, 2019

miri64 commented Mar 21, 2019

miri64 Mar 21, 2019

Choose a reason for hiding this comment

jia200x commented Sep 30, 2019 • edited Loading

stale bot commented Apr 10, 2020

miri64 commented Apr 14, 2020

jnohlgard commented Jun 11, 2018 •

edited

Loading

jia200x commented Sep 30, 2019 •

edited

Loading