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

gnrc_sixlowpan_frag: initial import of minimal forwarding #11068

Merged
merged 10 commits into from
Dec 1, 2020

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Feb 26, 2019

Contribution description

This implements minimal fragment forwarding for GNRC.

Testing procedure

There is a test application tests/gnrc_sixlowpan_frag_minfwd. Having a set-up with >2 nodes and forward packets between them should also work, but for some reason (I suspect the radio) it doesn't work that well at the moment. Once I find out why, I will update the PR and the testing procedures accordingly (see #11256 for what I found out).

examples/gnrc_networking should work as normal on a 6LoWPAN compatible board when not using gnrc_sixlowpan_frag_minfwd and when using the module both with fragmentation (packet size >100) or without.

Issues/PRs references

Depends on #11000, #11061, #11063, #11164, #11182, #11263, #11264, and #12629 (all merged):

Route to 6LoWPAN fragment forwarding

@miri64 miri64 added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT State: waiting for other PR State: The PR requires another PR to be merged first labels Feb 26, 2019
@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/new/minfwd branch from 93d3b6c to 6542506 Compare March 8, 2019 18:40
@miri64
Copy link
Member Author

miri64 commented Mar 8, 2019

Rebased to current master and #11063 to resolve conflict caused by merging #11135.

@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/new/minfwd branch from 2e4c0ad to e64b92b Compare March 12, 2019 16:00
@miri64
Copy link
Member Author

miri64 commented Mar 12, 2019

With the latest addition of #11164 as a dependency and the other fixes I am able to forward, at least when debug output is activated ... I'm not able to forward without it yet though :-(. I will for now first add some test-cases for the bugs I found in the real-world application, and then try to find the bug.

@miri64
Copy link
Member Author

miri64 commented Mar 12, 2019

OK, I got one successful send (out of 15 so far) :(

@miri64
Copy link
Member Author

miri64 commented Mar 23, 2019

In #11068 (comment) I wrote:

With the latest addition of #11164 as a dependency and the other fixes I am able to forward, at least when debug output is activated ... I'm not able to forward without it yet though :-(. I will for now first add some test-cases for the bugs I found in the real-world application, and then try to find the bug.

Some intensive days of hardware debugging later (thank you @PeterKietzmann for all the help and patience) we figured out what happens: appearently most of the time the framebuffer of the driver is overridden with the send data of the forwarded fragment, after the next fragment is received. I'll open a separate issue to explain this in a bit more detail.

@miri64
Copy link
Member Author

miri64 commented Mar 28, 2019

Rebased and adapted for current master and dependencies (including new dependencies #11263 and #11264)

@miri64
Copy link
Member Author

miri64 commented Apr 9, 2019

Rebased to include current RC (so I can refer to the 2019.04 release for my experiments)

@miri64
Copy link
Member Author

miri64 commented Apr 9, 2019

And rebased once again to include fix in gnrc_netif

@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/new/minfwd branch 2 times, most recently from 496d05c to 9d835f2 Compare April 9, 2019 11:18
@miri64 miri64 added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Apr 18, 2019
@miri64
Copy link
Member Author

miri64 commented Apr 18, 2019

Set to WIP because I added some stuff that does not belong with this PR and need to be factored out.

@miri64
Copy link
Member Author

miri64 commented Dec 1, 2020

  • how many reassembly buffers can there?

Until the RAM is full, I'd say. The number is capped of by CONFIG_GNRC_SIXLOWPAN_FRAG_RBUF_SIZE.

  • Will we wait a bit before sending the next fragment, so not to immediately cause a collision with the retransmission from the next hop?

That's basically what is done in SFR (#12648). One could add this as a feature generally to 6LoWPAN, but I rather would do it in a separate PR and when we go into congestion control.

I didn't check yet, but I guess there is no effect on code size / behavior if the module is not used.

Should be, yes.

@miri64
Copy link
Member Author

miri64 commented Dec 1, 2020

Pushed everything. Please check, if everything was addressed.

@miri64
Copy link
Member Author

miri64 commented Dec 1, 2020

Should have at least compile tested before pushing ^^"

@benpicco
Copy link
Contributor

benpicco commented Dec 1, 2020

Please squash!

@miri64
Copy link
Member Author

miri64 commented Dec 1, 2020

Squashed!

@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/new/minfwd branch from 229459b to c8ef052 Compare December 1, 2020 14:10
@benpicco
Copy link
Contributor

benpicco commented Dec 1, 2020

Squashed!

almost 😉

@miri64
Copy link
Member Author

miri64 commented Dec 1, 2020

Oh, there I seemed to have referenced a commit foreign to this PR (used git blame to identify it) Will see where I put it :D

@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/new/minfwd branch from c8ef052 to 2c35465 Compare December 1, 2020 14:18
@miri64
Copy link
Member Author

miri64 commented Dec 1, 2020

Fixed!

Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

I didn't test multi-hop fragmenting since I don't have a test setup for that (already thought of extending the zep_dispatcher), but I rely on your extensive testing documented in your paper.

I can confirm that there are no regressions when the module is not used, so ACK.

@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/new/minfwd branch from 2c35465 to 3a90dc2 Compare December 1, 2020 14:42
@miri64
Copy link
Member Author

miri64 commented Dec 1, 2020

Fixed and squashed the one doxygen error caused by a legacy name.

@benpicco
Copy link
Contributor

benpicco commented Dec 1, 2020

ah looks like the test will need a Makefile.ci.
Do we have a script yet to generate that automatically, like add_insufficient_memory_board.sh?

@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/new/minfwd branch from 3a90dc2 to e70a42c Compare December 1, 2020 15:25
@miri64
Copy link
Member Author

miri64 commented Dec 1, 2020

And added boards that are to small to Makefile.ci to make Murdock happy.

@miri64
Copy link
Member Author

miri64 commented Dec 1, 2020

Do we have a script yet to generate that automatically, like add_insufficient_memory_board.sh?

Murdock reported 35 boards. Most GNRC applications list 35 boards in their Makefile.ci, so I just copied it from tests/gnrc_dhcpv6_client. Chances are, those are the same boards ;-).

@benpicco benpicco merged commit b18cbd8 into RIOT-OS:master Dec 1, 2020
@miri64 miri64 deleted the gnrc_sixlowpan_frag/new/minfwd branch December 1, 2020 17:56
@miri64
Copy link
Member Author

miri64 commented Dec 1, 2020

Finally 🎉

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 State: don't stale State: Tell state-bot to ignore this issue Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants