-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
gnrc_sixlowpan_iphc: add fragment forwarding stubs #12629
gnrc_sixlowpan_iphc: add fragment forwarding stubs #12629
Conversation
8f99811
to
4fd7dc9
Compare
I also added a subroutine to do the actual forwarding in now. This code will be common between #11068 and SFR, so it makes sense to have a common base PR. Currently the function doesn't do anything except for returning |
I started a test application. Up until now, only the success case is tested, I plan however to add some failure cases. When setting
|
eac9232
to
799261d
Compare
Finished tests and squashd. No longer WIP. |
Updated the testing procedures |
Not really sure why Murdock is failing on Arch LinuxOperating System Environment ----------------------------- Operating System: "Arch Linux" Kernel: Linux 5.3.10-arch1-1 x86_64 unknown Ubuntu 19.10Operating System Environment ----------------------------- Operating System: "Ubuntu" "19.10 (Eoan Ermine)" Kernel: Linux 5.3.0-23-generic x86_64 x86_64 |
Mhhhhh... they do work on my machine :-/ |
Ah, there might be something going wrong with current master rebased. Will check! |
b0aa053
to
d964e69
Compare
First of all: squashed! |
(and rebased, though this wasn't my intention ^^") |
d964e69
to
ddcc6ea
Compare
Fixed. The reason was that the arrival time of the VRB entries that filled the VRB was not set for the VRB-full test, so the garbage collection just through them out when the new entry was created. |
Not sure why this worked with the old version of this PR... But now its fixed so 🤷♀️ |
ddcc6ea
to
b31d0db
Compare
Another update to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, most of it is just adding stub code that doesn't do anything on it's own yet.
b1d4cb3 is a nice small cleanup.
Only found some small things - comments are always good to not get lost in the sea of acronyms.
sys/net/gnrc/network_layer/sixlowpan/iphc/gnrc_sixlowpan_iphc.c
Outdated
Show resolved
Hide resolved
b31d0db
to
3273602
Compare
Addressed comments and squashed directly as otherwise I wouldn't have been able to adapt the comment that was removed in a later commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good and tests are still passing - changes outside of MODULE_GNRC_SIXLOWPAN_FRAG_VRB
are minimal anyway.
Thanks for the review! |
Contribution description
... when VRB is available
This is once again one of these tiny PRs that belong to a bigger PR, @benpicco is always talking about. However, in this case it was already introduced in an earlier version to #11068 (edc9b42) but is now needed also for SFR. So instead of having this piece of code copy-pasted in both, I decided to split it out (even though it is not functional without it).
Testing procedure
There is a test application
tests/gnrc_sixlowpan_iphc_w_vrb
in this PR that should work on any board that it fits on:Confirm that a fragmented and unfragmented UDP packet can still be sent between two 6LoWPAN nodes.
Additionally, you can check-out #11068 and
gnrc_sixlowpan_iphc.c
make -C tests/gnrc_sixlowpan_frag_minfwd flash test
on a board they fit onIssues/PRs references
Taken out of #11068's edc9b42 (as it is also required for other 6LoWPAN fragment forwarding schemes like SFR). It is however completely independent of that.