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

RFC: Add tests for IPv6 fragmentation #137

Closed
miri64 opened this issue Oct 4, 2019 · 6 comments
Closed

RFC: Add tests for IPv6 fragmentation #137

miri64 opened this issue Oct 4, 2019 · 6 comments

Comments

@miri64
Copy link
Member

miri64 commented Oct 4, 2019

With IPv6 fragmentation and reassembly merged (RIOT-OS/RIOT#11623 and RIOT-OS/RIOT#11596 respectively) during the 2019.10 release cycle, I think it makes sense to extend the release specifications for packets of sizes (MTU, 2*MTU]. Specifically I would extend 03, 04 and 07 with one task each to ping with packets with e.g. 2 KiB payload size.

All weird corner cases should already be covered by the tests/gnrc_ipv6_ext_frag application (which should be covered by 02, but who knows, since it requires sudo and people tend to skip tests that are not auto-run ;-P). However, I prefer to have at least some "in-the-wild" tests done during the release testing.

TBH, I personally never tested IPv6 + 6LoWPAN fragmentation (but AFAIK @benpicco did during the testing of RIOT-OS/RIOT#11623 [edit]after rereading his review comment I'm less sure[/edit]), so we have to see if the 04 extension works as expected and I don't believe multihop (07) was properly tested yet (tests/gnrc_ipv6_ext_frag tests forwarding of fragments though). I would thus suggest that the 04 and 07 extensions should be marked as experimental at least for the first RC they are tested in, in case some weird side effects arise.

miri64 added a commit to miri64/RIOT that referenced this issue Oct 4, 2019
miri64 added a commit to miri64/RIOT that referenced this issue Oct 4, 2019
While looking at tests/gnrc_ipv6_ext_frag again while writing
RIOT-OS/Release-Specs#137, I noticed that several of tests that I
definitely wrote myself from scretch are attributed wrong (and
sometimes even documented wrong). I guess this was caused by just
copy-pasting the files...
@leandrolanzieri
Copy link
Contributor

+1 for extending the tests and making those tasks experimental for now in 04 and 07, I think we should test this feature in multiple conditions.

On a separate matter, tasks 4.(5-8) still remain experimental? They seem to have been passing, @aabadie ?

@miri64
Copy link
Member Author

miri64 commented Oct 7, 2019

Ok, then I prepare a PR with the specifications.

@miri64
Copy link
Member Author

miri64 commented Oct 7, 2019

Good that we test this. Pinging with 2KiB already maxes out the packet buffer xD

miri64 added a commit to miri64/RIOT that referenced this issue Oct 7, 2019
When trying to ping 2 KiB, 6 KiB of packet buffer just aren't enough.
@fjmolinas
Copy link
Contributor

+1 for extending the tests and making those tasks experimental for now in 04 and 07, I think we should test this feature in multiple conditions.

I agree +1

@miri64
Copy link
Member Author

miri64 commented Oct 7, 2019

On a separate matter, tasks 4.(5-8) still remain experimental? They seem to have been passing, @aabadie ?

Let's discuss this in a separate issue, please.

@kb2ma
Copy link
Member

kb2ma commented Oct 10, 2019

I just merged #137. With @leandrolanzieri and @fjmolinas agreement here, let's close this issue.

@kb2ma kb2ma closed this as completed Oct 10, 2019
miri64 added a commit to miri64/RIOT that referenced this issue Oct 10, 2019
kb2ma added a commit to RIOT-OS/RIOT that referenced this issue Oct 10, 2019
gdoffe pushed a commit to gdoffe/RIOT that referenced this issue Dec 17, 2019
While looking at tests/gnrc_ipv6_ext_frag again while writing
RIOT-OS/Release-Specs#137, I noticed that several of tests that I
definitely wrote myself from scretch are attributed wrong (and
sometimes even documented wrong). I guess this was caused by just
copy-pasting the files...
gdoffe pushed a commit to gdoffe/RIOT that referenced this issue Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants