Skip to content

gnrc_netif: reapply event flags on device reset#9606

Merged
PeterKietzmann merged 1 commit intoRIOT-OS:masterfrom
bergzand:pr/netif/conf_events_on_reset
Jul 24, 2018
Merged

gnrc_netif: reapply event flags on device reset#9606
PeterKietzmann merged 1 commit intoRIOT-OS:masterfrom
bergzand:pr/netif/conf_events_on_reset

Conversation

@bergzand
Copy link
Copy Markdown
Member

On a NETOPT_STATE set call with NETOPT_STATE_RESET the netdev device
resets the callback event flags. This requires that after the netdev
device resets, the network stack also reapplies these callback event
flags

Issues/PRs references

Introduced with #9577 (This PR should be backported if #9577 is backported)

Testing

Due to #8118, this can't be tested on the at86rf2xx (this also does not fix the issue from #8118)

@bergzand bergzand added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking GNRC CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 19, 2018
@bergzand bergzand changed the title gnrc_netif: reaplly event flags on device reset gnrc_netif: reapply event flags on device reset Jul 19, 2018
On a NETOPT_STATE set call with NETOPT_STATE_RESET the netdev device
resets the callback event flags. This requires that after the netdev
device resets, the network stack also reapplies these callback event
flags
@bergzand bergzand force-pushed the pr/netif/conf_events_on_reset branch from 91f4ddb to 30e683c Compare July 19, 2018 12:51
@miri64
Copy link
Copy Markdown
Member

miri64 commented Jul 19, 2018

I only have boards with at86rf2xx on my desk right now. @PeterKietzmann can you test, please?

@PeterKietzmann
Copy link
Copy Markdown
Member

Why isn't that change needed for the other stacks as well?

@PeterKietzmann
Copy link
Copy Markdown
Member

@miri64 how would you test this?

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jul 23, 2018

@PeterKietzmann Take a board with a radio that doesn't have a at86rf2xx on board, change some values using ifconfig, check if the value is actually changed, use ifconfig to reset the interface, check if values are the initial values again.

@PeterKietzmann
Copy link
Copy Markdown
Member

Now that I looked at it I think changing "any value" for the interface makes no sense. _configure_netdev() (re-)sets the interrupt configuration, so this is the only setting that might get lost during reset of the interface (without this PR). If this is the case I would assume that ping6 shouldn't work anymore, after the reset. However, even without this PR I was always able to ping between a phytec node and a samr21-xpro, resetting the interface on the phytec board via ifconfig 7 set state reset in between.

Code-wise I agree with this change. @bergzand how did you determine this problem, which boards/which commands did you use?

@bergzand
Copy link
Copy Markdown
Member Author

@PeterKietzmann I used a nucleo with the mrf24j40.

I think the problem is not visible with the kw2xrf driver because it doesn't use extra settings for indicating which events should be propagated. Instead it always reports all events.

Copy link
Copy Markdown
Member

@PeterKietzmann PeterKietzmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. Tested as explained before but with a nucleo+mrf24j40 <-> samr21-xpro

@PeterKietzmann PeterKietzmann merged commit 1b7e164 into RIOT-OS:master Jul 24, 2018
PeterKietzmann added a commit that referenced this pull request Aug 2, 2018
gnrc_netif & radios: event reporting flags  [backport 2018.07] for #9601 #9606
@cladmi cladmi added this to the Release 2018.10 milestone Aug 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants