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_ipv6_ext_frag: remove fragment header when n-th fragment is first #13156

Merged
merged 1 commit into from
Jan 20, 2020
Merged

gnrc_ipv6_ext_frag: remove fragment header when n-th fragment is first #13156

merged 1 commit into from
Jan 20, 2020

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jan 17, 2020

Contribution description

The reassembly buffer only needs (and stores) the headers before the fragment header (called per-fragment headers in RFC 8200, section 4.5). Currently, when a subsequent IPv6 fragment is received before the first fragment the fragment header is however not removed. With this fix it does.

Testing procedure

See Release-Specs 4, Task 10. They should succeed for multiple runs, especially if packets get lost. To make sure loss is due to the first fragment missing I applied the following patch (which causes an E to appear in the output whenever a packet is lost under this circumstance in the given test run):

diff --git a/sys/net/gnrc/network_layer/ipv6/ext/frag/gnrc_ipv6_ext_frag.c b/sys/net/gnrc/network_layer/ipv6/ext/frag/gnrc_ipv6_ext_frag.c
index e7025a4eff..2ecbabae87 100644
--- a/sys/net/gnrc/network_layer/ipv6/ext/frag/gnrc_ipv6_ext_frag.c
+++ b/sys/net/gnrc/network_layer/ipv6/ext/frag/gnrc_ipv6_ext_frag.c
@@ -700,6 +700,7 @@ static gnrc_pktsnip_t *_completed(gnrc_ipv6_ext_frag_rbuf_t *rbuf)
         gnrc_ipv6_ext_frag_rbuf_free(rbuf);
         return res;
     }
+    puts("E");
     return NULL;
 }

Issues/PRs references

Cause of RIOT-OS/Release-Specs#145 (comment)

@miri64
gnrc_ipv6_ext_frag: remove fragment header when n-th fragment is first
28ef3b6 
The reassembly buffer only needs (and stores) the headers *before* the
fragment header (called per-fragment headers in RFC 8200, section 4.5).
Currently, when a subsequent IPv6 fragment is received before the first
fragment the fragment header is however not removed. With this fix it
does.
@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch labels Jan 17, 2020
@miri64 miri64 added this to the Release 2020.01 milestone Jan 17, 2020
@miri64 miri64 requested a review from fjmolinas January 17, 2020 17:59
@miri64 miri64 mentioned this pull request Jan 17, 2020
Closed
80 tasks
@fjmolinas
Copy link
Contributor

Running multiple runs of the test. But so far the issue seems fixed.

@fjmolinas
Copy link
Contributor

I performed 5 runs following Release-Specs 4, Task 10 pinging in both directions. I also applied the patch during that run, I had seen the bug testing Release-Specs 4, Task 10.

In the details below you can see a couple if times the E print.

04-single-hop-6lowpan-icmp/test.py ....................................................................................................................................................................................................... fe80::24bc:fb65:106b:1115: icmp_seq=199 ttl=64 rssi=-43 dBm time=284.692 ms

--- fe80::24bc:fb65:106b:1115 PING statistics ---
200 packets transmitted, 199 packets received, 0% packet loss

round-trip min/avg/max = 257.256/278.540/296.081 ms
> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 5872
~ unused: 0x20001a10 (next: (nil), size: 8192) ~

> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 4592
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
...................................................................................................................................................................................................... fe80::34bc:fd65:106b:1115: icmp_seq=199 ttl=64 rssi=-42 dBm time=293.680 ms

--- fe80::34bc:fd65:106b:1115 PING statistics ---
200 packets transmitted, 198 packets received, 1% packet loss

round-trip min/avg/max = 258.255/278.360/293.686 ms
> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 7928
~ unused: 0x20001a10 (next: (nil), size: 8192) ~

> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 5872
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
..................................................................................................................................................................................................... fe80::54ad:fc65:106b:1115: icmp_seq=199 ttl=64 rssi=-46 dBm time=282.793 ms

--- fe80::54ad:fc65:106b:1115 PING statistics ---
200 packets transmitted, 196 packets received, 2% packet loss

round-trip min/avg/max = 257.219/278.167/292.360 ms
> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 7928
~ unused: 0x20001a10 (next: (nil), size: 8192) ~

> E
E
pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 7928
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
........................................................................................................................................................................................................ fe80::50bd:fc65:106b:1115: icmp_seq=199 ttl=64 rssi=-44 dBm time=272.042 ms

--- fe80::50bd:fc65:106b:1115 PING statistics ---
200 packets transmitted, 200 packets received, 0% packet loss

round-trip min/avg/max = 258.217/278.383/294.328 ms
> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 7928
~ unused: 0x20001a10 (next: (nil), size: 8192) ~

> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 7928
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
........................................................................................................................................................................................................ fe80::24bc:fb65:106b:1115: icmp_seq=199 ttl=64 rssi=-43 dBm time=279.294 ms

--- fe80::24bc:fb65:106b:1115 PING statistics ---
200 packets transmitted, 199 packets received, 0% packet loss

round-trip min/avg/max = 258.265/278.466/291.768 ms
> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 4592
~ unused: 0x20001a10 (next: (nil), size: 8192) ~

> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 7928
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
........................................................................................................................................................................................................ fe80::34bc:fd65:106b:1115: icmp_seq=199 ttl=64 rssi=-41 dBm time=285.397 ms

--- fe80::34bc:fd65:106b:1115 PING statistics ---
200 packets transmitted, 200 packets received, 0% packet loss

round-trip min/avg/max = 260.488/278.455/292.718 ms
> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 7928
~ unused: 0x20001a10 (next: (nil), size: 8192) ~

> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 4592
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
........................................................................................................................................................................................................ fe80::54ad:fc65:106b:1115: icmp_seq=199 ttl=64 rssi=-46 dBm time=283.064 ms

--- fe80::54ad:fc65:106b:1115 PING statistics ---
200 packets transmitted, 199 packets received, 0% packet loss

round-trip min/avg/max = 257.865/278.308/293.337 ms
> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 4592
~ unused: 0x20001a10 (next: (nil), size: 8192) ~

> E
pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 7928
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
........................................................................................................................................................................................................ fe80::50bd:fc65:106b:1115: icmp_seq=199 ttl=64 rssi=-45 dBm time=276.359 ms

--- fe80::50bd:fc65:106b:1115 PING statistics ---
200 packets transmitted, 200 packets received, 0% packet loss

round-trip min/avg/max = 262.401/278.388/293.379 ms
> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 7928
~ unused: 0x20001a10 (next: (nil), size: 8192) ~

> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 4592
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
......................................................................................................................................................................................................... fe80::24bc:fb65:106b:1115: icmp_seq=199 ttl=64 rssi=-43 dBm time=283.085 ms

--- fe80::24bc:fb65:106b:1115 PING statistics ---
200 packets transmitted, 200 packets received, 0% packet loss

round-trip min/avg/max = 258.264/278.686/305.748 ms
> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 4592
~ unused: 0x20001a10 (next: (nil), size: 8192) ~

> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 4592
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
........................................................................................................................................................................................................ fe80::34bc:fd65:106b:1115: icmp_seq=199 ttl=64 rssi=-41 dBm time=291.183 ms

--- fe80::34bc:fd65:106b:1115 PING statistics ---
200 packets transmitted, 200 packets received, 0% packet loss

round-trip min/avg/max = 258.227/278.382/295.937 ms
> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 4592
~ unused: 0x20001a10 (next: (nil), size: 8192) ~

> pktbuf
packet buffer: first byte: 0x20001a10, last byte: 0x20003a10 (size: 8192)

  position of last byte used: 4592
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
.

============================================ 5 passed, 45 deselected in 1554.77s (0:25:54) =============================================

fjmolinas
fjmolinas approved these changes Jan 20, 2020
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK!

@fjmolinas fjmolinas merged commit 9a56892 into RIOT-OS:master Jan 20, 2020
@miri64 miri64 deleted the gnrc_ipv6_frag_ext/fix/rm-nth-fh-on-1st-loss branch January 20, 2020 08:57
@fjmolinas
Copy link
Contributor

Thanks a lot for investigating and providing the fix @miri64!

@miri64 miri64 mentioned this pull request Jan 20, 2020
Merged
@miri64
Copy link
Member Author

miri64 commented Jan 20, 2020

Backport provided in #13169

@fjmolinas fjmolinas mentioned this pull request Jan 22, 2020
Closed
80 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fjmolinas fjmolinas fjmolinas approved these changes

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 Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2020.01
Development

Successfully merging this pull request may close these issues.
2 participants
@miri64 @fjmolinas