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.

Sign up for GitHub

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

Jump to bottom

gnrc_sixlowpan_frag: initial import of the VRB #11000

Merged
merged 3 commits into from
Aug 7, 2019
Merged

gnrc_sixlowpan_frag: initial import of the VRB #11000

merged 3 commits into from
Aug 7, 2019

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Feb 12, 2019 •
edited
Loading

Contribution description

This is my initial import of the virtual reassembly buffer (VRB) including some minimal forwarding by using it (separated out). It is currently still WIP. The following is still missing:

  • IPHC integration (separated out)
  • Tests

Testing procedure

Compile and run unittests for a board of your choice.

Issues/PRs references

Requires #10988.

Route to 6Lo minimal fragment forwarding

@miri64 miri64 added Area: network Area: Networking State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Feb 12, 2019
@miri64 miri64 added this to the Release 2019.04 milestone Feb 12, 2019
@miri64
Copy link
Member Author

miri64 commented Feb 12, 2019

#11000 🎉

@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/new/vrb branch 2 times, most recently from 5b41025 to 86abdfa Compare February 13, 2019 15:09
@miri64
Copy link
Member Author

miri64 commented Feb 13, 2019

I moved the minimal forwarding implementation to a separate branch, so I can just focus on the VRB here.

@miri64 miri64 added State: waiting for other PR State: The PR requires another PR to be merged first and removed State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Feb 13, 2019
@miri64
Copy link
Member Author

miri64 commented Feb 13, 2019

No longer WIP, as such I squashed and removed the label. Still depending on #10988 though.

@miri64 miri64 mentioned this pull request Feb 26, 2019
Merged
@miri64
Copy link
Member Author

miri64 commented Mar 29, 2019 •
edited
Loading

#11068 is too experimental so it doesn't make any sense to try to get this into the release => post-poned.

@miri64 miri64 removed this from the Release 2019.04 milestone Mar 29, 2019
@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/new/vrb branch from 6610e1e to 8ae9648 Compare April 18, 2019 14:18
@miri64
Copy link
Member Author

miri64 commented Apr 18, 2019

Rebased to current master and dependencies

@miri64
Copy link
Member Author

miri64 commented Apr 18, 2019

And added the VRB compile time configurations to net/gnrc/sixlowpan/config.h (see #11415).

@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/new/vrb branch from 7398f3f to 79263e9 Compare May 28, 2019 17:22
@miri64
Copy link
Member Author

miri64 commented May 28, 2019

Rebased and squashed to current master

@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels May 28, 2019
@miri64
Copy link
Member Author

miri64 commented Jul 12, 2019

Ping? I'd like to have this merged (or at least some initial review done) before going on with further 6Lo-forwarding techniques!

@waehlisch
Copy link
Member

@miri64 don't count on me on this. I'm swamped with other stuff until mid of next week, then is IETF. maybe Semjon can give it a review?

@miri64
Copy link
Member Author

miri64 commented Jul 12, 2019

maybe Semjon can give it a review?

@SemjonKerner do you have any spare time?

@miri64
Copy link
Member Author

miri64 commented Jul 12, 2019

Fixed up for #11577.

@miri64 miri64 mentioned this pull request Jul 12, 2019
Merged
@SemjonWilke SemjonWilke self-requested a review July 12, 2019 17:08
miri64
miri64 commented Aug 6, 2019
View reviewed changes
sys/include/net/gnrc/sixlowpan/config.h
#else /* defined(MODULE_GNRC_SIXLOWPAN_FRAG_VRB) || defined(DOXYGEN) */
#define GNRC_SIXLOWPAN_FRAG_VRB_SIZE (0U)
#endif /* defined(MODULE_GNRC_SIXLOWPAN_FRAG_VRB) || defined(DOXYGEN) */
#endif /* GNRC_SIXLOWPAN_FRAG_VRB_SIZE */
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm doing this so complicated, to allow for easy fragment interval multiplicity calculation in #11068 (and coming fragment forwarding solutions):

RIOT/sys/net/gnrc/network_layer/sixlowpan/frag/rbuf.c

Lines 50 to 51 in 7d54251

#define RBUF_INT_SIZE (DIV_CEIL(IPV6_MIN_MTU, GNRC_SIXLOWPAN_FRAG_SIZE) * \
(RBUF_SIZE + GNRC_SIXLOWPAN_FRAG_VRB_SIZE))

If if gnrc_sixlowpan_frag_vrb is not used GNRC_SIXLOWPAN_FRAG_VRB_SIZE is 0 so I just use the fragment intervals required for the real reassembly buffer.

haukepetersen
haukepetersen requested changes Aug 7, 2019
View reviewed changes
Copy link
Contributor

@haukepetersen haukepetersen left a comment

Choose a reason for hiding this comment

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

Two minor findings.

Else everything looks good to me, and the tests work as expected on native and on the nrf52dk

sys/include/net/gnrc/sixlowpan/frag/vrb.h Outdated Show resolved Hide resolved
sys/net/gnrc/network_layer/sixlowpan/frag/vrb/gnrc_sixlowpan_frag_vrb.c Show resolved Hide resolved
@haukepetersen haukepetersen added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Aug 7, 2019
haukepetersen
haukepetersen approved these changes Aug 7, 2019
View reviewed changes
Copy link
Contributor

@haukepetersen haukepetersen left a comment

Choose a reason for hiding this comment

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

ACK, please squash

@miri64
Copy link
Member Author

miri64 commented Aug 7, 2019

Squashed

@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/new/vrb branch from 21bae77 to c617ee1 Compare August 7, 2019 14:13
@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/new/vrb branch from c617ee1 to 1f1ff16 Compare August 7, 2019 14:20
@miri64
Copy link
Member Author

miri64 commented Aug 7, 2019 •
edited
Loading

Addressed findings by Travis and squashed immediately.

@haukepetersen
Copy link
Contributor

all green, lets go!

@haukepetersen haukepetersen merged commit 02df6ab into RIOT-OS:master Aug 7, 2019
@miri64 miri64 deleted the gnrc_sixlowpan_frag/new/vrb branch August 7, 2019 15:33
@miri64
Copy link
Member Author

miri64 commented Aug 7, 2019

Thanks for the review :-)

@miri64 miri64 mentioned this pull request Aug 10, 2019
Closed
@kb2ma kb2ma added this to the Release 2019.10 milestone Sep 16, 2019
miri64 added a commit to 5G-I3/RIOT-public that referenced this pull request Sep 25, 2019
@miri64
gnrc_sixlowpan_frag: initial import of the VRB
a5a4ef3 
VRB = virtual reassembly buffer

(cherry picked from commit 26a526e,
see RIOT-OS#11000)
miri64 added a commit to 5G-I3/RIOT-public that referenced this pull request Sep 25, 2019
@miri64
tests: add unittests for gnrc_sixlowpan_frag_vrb
07a810d 
(cherry picked from commit 2cd46e5,
see RIOT-OS#11000)
miri64 added a commit to 5G-I3/RIOT-public that referenced this pull request Sep 25, 2019
@miri64
gnrc_sixlowpan_frag: include VRB in normal GC
4c8443a 
(cherry picked from commit 1f1ff16,
see RIOT-OS#11000)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haukepetersen haukepetersen haukepetersen approved these changes

@cgundogan cgundogan Awaiting requested review from cgundogan

@waehlisch waehlisch Awaiting requested review from waehlisch

@SemjonWilke SemjonWilke Awaiting requested review from SemjonWilke

Assignees
No one assigned
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 Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
Release 2019.10
Development

Successfully merging this pull request may close these issues.
4 participants
@miri64 @waehlisch @haukepetersen @kb2ma