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_ipv6_ext_frag: remove fragment header when n-th fragment is first [backport 2020.01] #13169

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jan 20, 2020

Backport of #13156

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)

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.

(cherry picked from commit 28ef3b6)
@miri64 miri64 added 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: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Jan 20, 2020
@miri64 miri64 requested a review from fjmolinas January 20, 2020 09:03
@miri64 miri64 added this to the Release 2020.01 milestone Jan 20, 2020
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
Copy link
Contributor

I'll re-run the test for good measure.

@fjmolinas
Copy link
Contributor

All good

04-single-hop-6lowpan-icmp/test.py ....................................................................................................................................................................................................... fe80::54ad:fc65:106b:1115: icmp_seq=199 ttl=64 rssi=-46 dBm time=283.157 ms

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

round-trip min/avg/max = 257.905/278.354/293.439 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::50bd:fc65:106b:1115: icmp_seq=199 ttl=64 rssi=-45 dBm time=283.497 ms

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

round-trip min/avg/max = 258.559/278.569/293.421 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=289.037 ms

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

round-trip min/avg/max = 258.344/278.644/297.219 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=296.253 ms

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

round-trip min/avg/max = 256.061/278.558/296.253 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.158 ms

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

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

  position of last byte used: 7944
~ 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::50bd:fc65:106b:1115: icmp_seq=199 ttl=64 rssi=-46 dBm time=280.302 ms

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

round-trip min/avg/max = 260.547/278.550/295.276 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: 7944
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
....................................................................................................................................................................................................... fe80::24bc:fb65:106b:1115: icmp_seq=199 ttl=64 rssi=-43 dBm time=287.040 ms

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

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

  position of last byte used: 7944
~ 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=-42 dBm time=287.608 ms

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

round-trip min/avg/max = 253.546/278.528/296.911 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: 7944
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
..................................................................................................................................................................................................... fe80::54ad:fc65:106b:1115: icmp_seq=199 ttl=64 rssi=-46 dBm time=286.690 ms

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

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

  position of last byte used: 7944
~ 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::50bd:fc65:106b:1115: icmp_seq=199 ttl=64 rssi=-45 dBm time=273.252 ms

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

round-trip min/avg/max = 260.609/278.654/294.828 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: 7944
~ unused: 0x20001a10 (next: (nil), size: 8192) ~
.

================================================================================================================== 5 passed, 45 deselected in 1601.92s (0:26:41) ==================================================================================================================

@fjmolinas fjmolinas merged commit da47b00 into RIOT-OS:2020.01-branch Jan 20, 2020
@miri64 miri64 deleted the backport/2020.01/gnrc_ipv6_frag_ext/fix/rm-nth-fh-on-1st-loss branch January 20, 2020 10:18
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 Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants