gnrc_sixlowpan_frag: initial import of Selective Fragment Recovery by miri64 · Pull Request #12648 · RIOT-OS/RIOT

miri64 · 2019-11-05T19:39:24Z

Contribution description

This provides an implementation for 6LoWPAN Selective Fragment Recovery (SFR), an alternative fragmentation protocol for 6LoWPAN, which allows for end-to-end recovery and congestion control (currently not implemented) for fragmented IPv6 datagrams.

Testing procedure

There is still a lot of more in-depth testing required including a dedicated test application (this is why is it still WIP), but you can compile examples/gnrc_networking with SFR already for a 6LoWPAN-capable board:

USEMODULE="gnrc_sixlowpan_frag_sfr" BOARD=samr21-xpro make -C examples/gnrc_networking flash term

and then ping another node also shipping with gnrc_sixlowpan_frag_sfr:

ping6 -s 1232 fe80::7b65:822:8693:9d5a

~~[edit]: after posting this PR, I noticed, that there is a leak in the packet buffer after this... All the more reason why this is still WIP[/edit]~~

There are also tests that should pass on a board of your choice:

BOARD=... make -C tests/gnrc_sixlowpan_frag_sfr -j flash term

Issues/PRs references

Requires ~~#12220~~ (merged), ~~#12559~~ (merged), ~~#12629~~, ~~#12845~~, and ~~#12848~~ (merged):

miri64

Some thoughts from my subway ride home ;-)

sys/net/gnrc/network_layer/sixlowpan/frag/sfr/gnrc_sixlowpan_frag_sfr.c

tests/unittests/tests-gnrc_sixlowpan_frag_vrb/tests-gnrc_sixlowpan_frag_vrb.c

miri64 · 2019-11-06T15:11:45Z

Rebased to current to include #12653 for make debug ;-)

miri64 · 2019-11-07T13:24:28Z

Rebased to current master as #12220 and #12559 got merged.

miri64 · 2019-11-07T18:33:50Z

Ok, I now can ping with huge datagrams in "rapid" (200ms as this is the estimate RTT) intervals without any crashes or leaks between two nodes. I'm not 100% sure resending is working as it should be. Further testing will show this hopefully. Next: actual forwarding.

miri64 · 2019-11-08T17:55:09Z

Forwarding is now also working. There is still some weirdness happening with the ACKs and retries (I'm not sure if there is >1 retry made and the ACK timeout is restarted after it was fired even though the datagram is complete)... Will investigate this next week.

miri64 · 2019-11-09T13:22:31Z

the ACK timeout is restarted after it was fired even though the datagram is complete

At least this I found now.

miri64 · 2019-11-12T18:43:42Z

Rebased to current master

miri64 · 2019-11-13T19:10:33Z

Rebased to current master and dependencies.

miri64 · 2019-11-15T17:21:30Z

Added first round of tests.

miri64 · 2019-11-21T20:06:56Z

I have full test coverage now (at least as far as I can judge without a coverage tester right now 😅). On native they succeed, bur are still a bit flaky, on iotlab-m3 and samr21-xpro they reliably fail. I will investigate what's up next. If that is found, I think I can remove the WIP label. @cgundogan may I squash before that?

miri64 · 2019-11-22T14:20:40Z

On native they succeed, bur are still a bit flaky, on iotlab-m3 and samr21-xpro they reliably fail. I will investigate what's up next. If that is found, I think I can remove the WIP labe

And done. It was in the end as I suspected a timing issue with the ARQ timeout.

miri64 · 2020-12-14T08:01:35Z

Seems only to happen with netdev tests though, so might be caused by some dependency re-ordering.

IIRC there is no netdev-related dependency stuff happening in this PR. So this must be some weird side effect of unexpected netdev dependencies then ;-).

miri64 · 2020-12-14T08:12:27Z

Mhhh the Murdock errors seem to be something related to gnrc_netif pulling in SFR headers, pulling in evtimer, pulling in xtimer and then xtimer having some messed up definitions. Not really sure exactly what is happening here... This used to work. Maybe @maribu or @kaspar030 can help?

miri64 · 2020-12-14T08:31:17Z

Fixed static tests and squashed fixes in the meantime.

miri64 · 2020-12-14T09:18:59Z

The problem with examples/nanocoap_server was also legitimate. Fixed and squashed.

miri64 · 2020-12-14T09:31:01Z

If that was @benpicco's intention: the problem is not fixed when merging #15627 :-/

miri64 · 2020-12-14T09:54:54Z

There was a rebase error, that un-guarded the xtimer.h include in gnrc_netif.c
If I #if IS_USED(...) guard the sfr.h include, tests/driver_w5100 compiles for waspmote-pro now. It is only used to call the interface-specific init function of SFR, so I think the pre-processor magic is small enough. Still would be interesting why without 6LoWPAN and xtimer compiled in, ztimer-only boards have problems compiling such a combination...

maribu · 2020-12-14T10:02:49Z

Still would be interesting why without 6LoWPAN and xtimer compiled in, ztimer-only boards have problems compiling such a combination...

The issue is that the ztimer-only boards cannot function with plain xtimer, due to incompatible clock frequency. To fix that, there is an

ifneq (,$(filter xtimer,$(USEMODULE)))
  USEMODULE += xtimer_on_ztimer
endif

There is a compile time check via preprocessor that gets run when including xtimer.h that verifies that the XTIMER_HZ value is correctly set and compatible. Since boards incompatible with xtimer cannot provide a compliant value for them, this check will fail. However, if xtimer_on_ztimer is used, that check is skipped - ztimer can manage any timer frequency.

So the include to xtimer.h only works for those boards, if xtimer_on_ztimer is used - which is for those boards automatically done if xtimer is used.

miri64 · 2020-12-14T10:08:41Z

So the include to xtimer.h only works for those boards, if xtimer_on_ztimer is used - which is for those boards automatically done if xtimer is used.

So the conclusion is: if xtimer.h is included without the xtimer module compiled in, for those boards that are incompatible to xtimer, the compilation fails :-) (as xtimer_on_ztimer is not automatically included to fix the situation).

miri64 · 2020-12-14T11:24:04Z

And some more bugs found by the compile tests fixed and squashed.

benpicco · 2020-12-14T11:53:18Z

Did some testing (only single hop) and fragmentation works between two 6lo nodes with SFR enabled.
As expected fragmentation does not work between a SFR node and a 'classic' node, not sure if we should add that to the documentation.

Two nodes with 'classic' fragmentation can still communicate as before.

miri64 · 2020-12-14T11:56:58Z

As expected fragmentation does not work between a SFR node and a 'classic' node, not sure if we should add that to the documentation.

Will add a note.

miri64 · 2020-12-14T12:00:46Z

Done.

miri64 · 2020-12-14T13:35:50Z

🎉 Thanks for the review and your input @benpicco

miri64 added this to the Release 2020.01 milestone Nov 5, 2019

miri64 requested review from cgundogan and waehlisch November 5, 2019 19:39

miri64 commented Nov 5, 2019

View reviewed changes

sys/net/gnrc/network_layer/sixlowpan/frag/sfr/gnrc_sixlowpan_frag_sfr.c Outdated Show resolved Hide resolved

sys/net/gnrc/network_layer/sixlowpan/frag/sfr/gnrc_sixlowpan_frag_sfr.c Outdated Show resolved Hide resolved

miri64 mentioned this pull request Nov 6, 2019

core/kernel_defines: Introduce 'IS_ACTIVE' macro #12626

Merged

cgundogan reviewed Nov 6, 2019

View reviewed changes

tests/unittests/tests-gnrc_sixlowpan_frag_vrb/tests-gnrc_sixlowpan_frag_vrb.c Outdated Show resolved Hide resolved

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from 2a2c932 to 7ad40d0 Compare November 6, 2019 15:11

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from b3a9f6c to f51cb1b Compare November 7, 2019 13:23

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from f51cb1b to 28d050e Compare November 7, 2019 18:31

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from 28d050e to 09a69ef Compare November 8, 2019 17:53

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from 09a69ef to 6ed6b68 Compare November 9, 2019 13:22

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from 6ed6b68 to 33c432f Compare November 12, 2019 18:43

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from 9d231d8 to 9c7ecd3 Compare November 13, 2019 19:10

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from ef62df9 to b5644b9 Compare November 15, 2019 17:20

miri64 mentioned this pull request Nov 20, 2019

bitfield: unify order #12759

Merged

miri64 removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Nov 22, 2019

miri64 added 4 commits December 14, 2020 09:30

gnrc_sixlowpan_config: rename SRF parameters for Kconfig

82c4d26

gnrc_netif: 6lo.h: add flags to indicate SRF capability

80ff517

gnrc_sixlowpan_frag_rb: add check function for empty interval pool

bd300a3

gnrc_sixlowpan_frag_rb: externalize get_by_tag function

d393008

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from a051c28 to 6c5fbde Compare December 14, 2020 08:30

gnrc_sixlowpan_frag_rb: add handling for RFRAG packets

ec436e9

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from 6c5fbde to 542f3a1 Compare December 14, 2020 09:18

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from 542f3a1 to 60de138 Compare December 14, 2020 09:52

miri64 mentioned this pull request Dec 14, 2020

vera++: reports "warning: keyword 'new' not followed by a single space" #15626

Closed

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from 60de138 to 15716a5 Compare December 14, 2020 11:23

miri64 added 3 commits December 14, 2020 13:00

gnrc_sixlowpan_frag: initial import of Selective Fragment Recovery

1cd1716

tests: provide tests for gnrc_sixlowpan_frag_sfr

ed4ac70

gnrc_sixlowpan_frag_sfr: provide statistics sub-module

e980405

miri64 force-pushed the gnrc_sixlowpan_frag_sfr/feat/initial branch from 15716a5 to e980405 Compare December 14, 2020 12:00

benpicco approved these changes Dec 14, 2020

View reviewed changes

miri64 merged commit 41a844f into RIOT-OS:master Dec 14, 2020

miri64 deleted the gnrc_sixlowpan_frag_sfr/feat/initial branch December 14, 2020 13:32

Conversation

miri64 commented Nov 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miri64 commented Nov 6, 2019

Uh oh!

miri64 commented Nov 7, 2019

Uh oh!

miri64 commented Nov 7, 2019

Uh oh!

miri64 commented Nov 8, 2019

Uh oh!

miri64 commented Nov 9, 2019

Uh oh!

miri64 commented Nov 12, 2019

Uh oh!

miri64 commented Nov 13, 2019

Uh oh!

miri64 commented Nov 15, 2019

Uh oh!

miri64 commented Nov 21, 2019

Uh oh!

miri64 commented Nov 22, 2019

Uh oh!

miri64 commented Dec 14, 2020

Uh oh!

miri64 commented Dec 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miri64 commented Dec 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miri64 commented Dec 14, 2020

Uh oh!

miri64 commented Dec 14, 2020

Uh oh!

miri64 commented Dec 14, 2020

Uh oh!

maribu commented Dec 14, 2020

Uh oh!

miri64 commented Dec 14, 2020

Uh oh!

miri64 commented Dec 14, 2020

Uh oh!

benpicco commented Dec 14, 2020

Uh oh!

miri64 commented Dec 14, 2020

Uh oh!

miri64 commented Dec 14, 2020

Uh oh!

miri64 commented Dec 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

miri64 commented Nov 5, 2019 •

edited

Loading

miri64 commented Dec 14, 2020 •

edited

Loading

miri64 commented Dec 14, 2020 •

edited

Loading