gnrc_ipv6: assume no preparsed packets by miri64 · Pull Request #10233 · RIOT-OS/RIOT

miri64 · 2018-10-23T20:02:54Z

Contribution description

As stated in #10229, with #9484 and #10229 we don't have to assume anymore that an IPv6 packet was preparsed before it is handed to IPv6's receive routine. This allows us to remove a whole chunk of code.
Further simplifications not related to finding the IPv6 header in a preparsed packet will follow later.

Testing procedure

Compile gnrc_networking for a IEEE802.15.4-equipped board of your choice and flash. Try to send IPHC-compressed UDP packets and fragmented packets. All should be received and handled correctly. Here is what I did to test (on iotlab-m3:

m3-9;ping6 fe80::1711:6b10:65f8:a83a
1540324618.692019;m3-9;> ping6 fe80::1711:6b10:65f8:a83a
1540324618.700864;m3-9;12 bytes from fe80::1711:6b10:65f8:a83a: id=83 seq=1 hop limit=64 time = 6.087 ms
1540324619.710639;m3-9;12 bytes from fe80::1711:6b10:65f8:a83a: id=83 seq=2 hop limit=64 time = 7.367 ms
1540324620.716881;m3-9;12 bytes from fe80::1711:6b10:65f8:a83a: id=83 seq=3 hop limit=64 time = 4.820 ms
1540324620.717727;m3-9;--- fe80::1711:6b10:65f8:a83a ping statistics ---
1540324620.718661;m3-9;3 packets transmitted, 3 received, 0% packet loss, time 2.0623902 s
1540324620.719706;m3-9;rtt min/avg/max = 4.820/6.091/7.367 ms
m3-16;udp server start 61616
1540324626.987787;m3-16;> udp server start 61616
1540324626.988009;m3-16;Success: started UDP server on port 61616
m3-9;udp send fe80::1711:6b10:65f8:a83a 61616 "Hello World"                                                                                                                          
1540324643.437245;m3-9;> udp send fe80::1711:6b10:65f8:a83a 61616 "Hello World"
1540324643.439125;m3-9;Success: sent 11 byte(s) to [fe80::1711:6b10:65f8:a83a]:61616
1540324643.443109;m3-16;> PKTDUMP: data received:
1540324643.443296;m3-16;~~ SNIP  0 - size:  11 byte, type: NETTYPE_UNDEF (0)
1540324643.443430;m3-16;00000000  48  65  6C  6C  6F  20  57  6F  72  6C  64
1540324643.444055;m3-16;~~ SNIP  1 - size:   8 byte, type: NETTYPE_UDP (4)
1540324643.444646;m3-16;   src-port: 61616  dst-port: 61616
1540324643.445563;m3-16;   length: 19  cksum: 0xcd02
1540324643.446555;m3-16;~~ SNIP  2 - size:  40 byte, type: NETTYPE_IPV6 (2)
1540324643.447535;m3-16;traffic class: 0x00 (ECN: 0x0, DSCP: 0x00)
1540324643.447722;m3-16;flow label: 0x00000
1540324643.448541;m3-16;length: 19  next header: 17  hop limit: 64
1540324643.449528;m3-16;source address: fe80::1711:6b10:65fb:8a22
1540324643.450545;m3-16;destination address: fe80::1711:6b10:65f8:a83a
1540324643.451547;m3-16;~~ SNIP  3 - size:  24 byte, type: NETTYPE_NETIF (-1)
1540324643.452530;m3-16;if_pid: 7  rssi: -61  lqi: 255
1540324643.453067;m3-16;flags: 0x0
1540324643.453629;m3-16;src_l2addr: 15:11:6B:10:65:FB:8A:22
1540324643.454545;m3-16;dst_l2addr: 15:11:6B:10:65:F8:A8:3A
1540324643.455523;m3-16;~~ PKT    -  4 snips, total size:  83 byte
m3-9;ping6 fe80::1711:6b10:65f8:a83a 512
1540324664.883888;m3-9;> ping6 fe80::1711:6b10:65f8:a83a 512
1540324664.950310;m3-9;520 bytes from fe80::1711:6b10:65f8:a83a: id=84 seq=1 hop limit=64 time = 62.718 ms
1540324666.022960;m3-9;520 bytes from fe80::1711:6b10:65f8:a83a: id=84 seq=2 hop limit=64 time = 71.836 ms
1540324667.093857;m3-9;520 bytes from fe80::1711:6b10:65f8:a83a: id=84 seq=3 hop limit=64 time = 68.880 ms
1540324667.094768;m3-9;--- fe80::1711:6b10:65f8:a83a ping statistics ---
1540324667.095403;m3-9;3 packets transmitted, 3 received, 0% packet loss, time 2.06209628 s
1540324667.096432;m3-9;rtt min/avg/max = 62.718/67.811/71.836 ms

Also repeat testing proceedures from #10229.

Issues/PRs references

Depends on ~~#10229 (and its dependencies)~~ (merged).

…into HEAD

miri64 · 2018-10-23T20:21:07Z

sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c

 static void _receive(gnrc_pktsnip_t *pkt)
 {
    gnrc_netif_t *netif = NULL;
-    gnrc_pktsnip_t *ipv6, *netif_hdr, *first_ext;


Due to pkt now only consisting of the IPv6 header (and possibly the netif header), with the IPv6 header later being marked (i.e. the packet becomes payload -> ipv6 -> netif), first_ext became only an alias for pkt and this unnecessary.

… into HEAD

Since with RIOT-OS#10233 we now assume IPv6 packets always to not be pre-parsed, we can iterate over the extension headers by gradually "eating" them away. This allows us to move the iteration over them out of `gnrc_ipv6_ext_demux()` and into `gnrc_ipv6_demux()`. By moving the iteration over all extension headers out of `gnrc_ipv6_ext_demux()` we also can 1. simplify the extension header handling a lot, as it now just a loop inside `gnrc_ipv6_demux()`, 2. remove the recursion to `gnrc_ipv6_demux()` within `gnrc_ipv6_ext_demux()`.

miri64 · 2018-10-25T17:27:41Z

Rebased to current #10229.

Since with RIOT-OS#10233 we now assume IPv6 packets always to not be pre-parsed, we can iterate over the extension headers by gradually "eating" them away. This allows us to move the iteration over them out of `gnrc_ipv6_ext_demux()` and into `gnrc_ipv6_demux()`. By moving the iteration over all extension headers out of `gnrc_ipv6_ext_demux()` we also can 1. simplify the extension header handling a lot, as it now just a loop inside `gnrc_ipv6_demux()`, 2. remove the recursion to `gnrc_ipv6_demux()` within `gnrc_ipv6_ext_demux()`.

miri64 · 2018-10-25T22:02:48Z

Rebased to current #10229.

Since with RIOT-OS#10233 we now assume IPv6 packets always to not be pre-parsed, we can iterate over the extension headers by gradually "eating" them away. This allows us to move the iteration over them out of `gnrc_ipv6_ext_demux()` and into `gnrc_ipv6_demux()`. By moving the iteration over all extension headers out of `gnrc_ipv6_ext_demux()` we also can 1. simplify the extension header handling a lot, as it now just a loop inside `gnrc_ipv6_demux()`, 2. remove the recursion to `gnrc_ipv6_demux()` within `gnrc_ipv6_ext_demux()`.

miri64 · 2018-11-08T08:34:25Z

Reminder to self: adapt for ugly bugfix in #10246.

Since with RIOT-OS#10233 we now assume IPv6 packets always to not be pre-parsed, we can iterate over the extension headers by gradually "eating" them away. This allows us to move the iteration over them out of `gnrc_ipv6_ext_demux()` and into `gnrc_ipv6_demux()`. By moving the iteration over all extension headers out of `gnrc_ipv6_ext_demux()` we also can 1. simplify the extension header handling a lot, as it now just a loop inside `gnrc_ipv6_demux()`, 2. remove the recursion to `gnrc_ipv6_demux()` within `gnrc_ipv6_ext_demux()`.

miri64 · 2018-11-17T15:19:01Z

Rebased to current #10229.

Since with RIOT-OS#10233 we now assume IPv6 packets always to not be pre-parsed, we can iterate over the extension headers by gradually "eating" them away. This allows us to move the iteration over them out of `gnrc_ipv6_ext_demux()` and into `gnrc_ipv6_demux()`. By moving the iteration over all extension headers out of `gnrc_ipv6_ext_demux()` we also can 1. simplify the extension header handling a lot, as it now just a loop inside `gnrc_ipv6_demux()`, 2. remove the recursion to `gnrc_ipv6_demux()` within `gnrc_ipv6_ext_demux()`.

Since the packet is now guaranteed to be preparsed, the currently handled IPv6 header will always be in the first snip. Because of this the packet parser can't get confused anymore which IPv6 header is the one to be handled so we don't need to remove the more outer ones. Because of this we can just use the normal packet dispatching (which is already used by other `GNRC_NETTYPE_*`-known protocol numbers such as UDP). This also reverts d54ac38.

miri64 · 2018-12-06T15:24:53Z

Rebased to current master. No longer waiting on other PR.

jia200x · 2018-12-10T10:10:36Z

Tested with the described procedure (and also with the one in #10229 )

jia200x · 2018-12-10T10:15:24Z

nothing interesting to report on coding style and documentation doesn't apply here

jia200x · 2018-12-10T10:22:25Z

btw it reduces around 800 bytes on samr21-xpro when rebased to master :)

jia200x

ACK

Since with RIOT-OS#10233 we now assume IPv6 packets always to not be pre-parsed, we can iterate over the extension headers by gradually "eating" them away. This allows us to move the iteration over them out of `gnrc_ipv6_ext_demux()` and into `gnrc_ipv6_demux()`. By moving the iteration over all extension headers out of `gnrc_ipv6_ext_demux()` we also can 1. simplify the extension header handling a lot, as it now just a loop inside `gnrc_ipv6_demux()`, 2. remove the recursion to `gnrc_ipv6_demux()` within `gnrc_ipv6_ext_demux()`.

miri64 added Area: network Area: Networking Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation State: waiting for other PR State: The PR requires another PR to be merged first labels Oct 23, 2018

miri64 requested review from bergzand, cgundogan and jnohlgard October 23, 2018 20:02

miri64 added a commit to miri64/RIOT that referenced this pull request Oct 23, 2018

Merge branch 'gnrc_ipv6/enh/assume-no-preparsed-pkt' (RIOT-OS#10233) …

7838ae0

…into HEAD

miri64 mentioned this pull request Oct 23, 2018

gnrc_ipv6_ext: needs heavy refactoring #10133

Closed

5 tasks

miri64 force-pushed the gnrc_ipv6/enh/assume-no-preparsed-pkt branch from b5d3471 to eee483e Compare October 23, 2018 20:18

miri64 commented Oct 23, 2018

View reviewed changes

miri64 added a commit to miri64/RIOT that referenced this pull request Oct 23, 2018

Merge branch 'gnrc_ipv6/enh/assume-no-preparsed-pkt' (RIOT-OS#10233)…

5a4c512

… into HEAD

miri64 added a commit to miri64/RIOT that referenced this pull request Oct 23, 2018

Merge branch 'gnrc_ipv6/enh/assume-no-preparsed-pkt' (RIOT-OS#10233)…

e0beeed

… into HEAD

miri64 mentioned this pull request Oct 23, 2018

gnrc_ipv6: move ipv6_ext iteration out of ext_demux() #10234

Merged

miri64 force-pushed the gnrc_ipv6/enh/assume-no-preparsed-pkt branch from a943d97 to 3a564fb Compare October 24, 2018 19:32

miri64 force-pushed the gnrc_ipv6/enh/assume-no-preparsed-pkt branch 2 times, most recently from b0d3dca to 36f6265 Compare October 25, 2018 17:27

miri64 force-pushed the gnrc_ipv6/enh/assume-no-preparsed-pkt branch from 36f6265 to e9b1aef Compare October 25, 2018 22:02

RIOT-OS deleted a comment Oct 26, 2018

miri64 mentioned this pull request Nov 8, 2018

gnrc_ipv6: don't recurse into receive for encapsulated IPv6 #10246

Merged

miri64 force-pushed the gnrc_ipv6/enh/assume-no-preparsed-pkt branch 2 times, most recently from 3208262 to bea82a9 Compare November 8, 2018 11:56

miri64 force-pushed the gnrc_ipv6/enh/assume-no-preparsed-pkt branch from bea82a9 to 8ed76a8 Compare November 17, 2018 15:18

RIOT-OS deleted a comment Nov 17, 2018

miri64 and others added 3 commits December 6, 2018 16:22

gnrc_ipv6: assume no preparsed packets

4befe0b

tests/gnrc_ipv6_ext: remove test for parsed packet reception

2b280f8

miri64 force-pushed the gnrc_ipv6/enh/assume-no-preparsed-pkt branch from 8ed76a8 to 3fde0ef Compare December 6, 2018 15:23

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 Dec 6, 2018

jia200x added the Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines label Dec 7, 2018

RIOT-OS deleted a comment Dec 8, 2018

jia200x added 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 labels Dec 10, 2018

jia200x added the Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines label Dec 10, 2018

jia200x added the Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines label Dec 10, 2018

jia200x approved these changes Dec 10, 2018

View reviewed changes

jia200x merged commit 970bec1 into RIOT-OS:master Dec 10, 2018

miri64 deleted the gnrc_ipv6/enh/assume-no-preparsed-pkt branch December 10, 2018 13:28

miri64 mentioned this pull request Dec 10, 2018

gnrc_ipv6: make gnrc_netapi documentation more detailed #10583

Merged

aabadie added this to the Release 2019.01 milestone Dec 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gnrc_ipv6: assume no preparsed packets#10233

gnrc_ipv6: assume no preparsed packets#10233
jia200x merged 3 commits intoRIOT-OS:masterfrom
miri64:gnrc_ipv6/enh/assume-no-preparsed-pkt

miri64 commented Oct 23, 2018 •

edited

Loading

Uh oh!

miri64 Oct 23, 2018

Uh oh!

miri64 commented Oct 25, 2018

Uh oh!

miri64 commented Oct 25, 2018

Uh oh!

miri64 commented Nov 8, 2018

Uh oh!

miri64 commented Nov 17, 2018

Uh oh!

miri64 commented Dec 6, 2018

Uh oh!

jia200x commented Dec 10, 2018

Uh oh!

jia200x commented Dec 10, 2018

Uh oh!

jia200x commented Dec 10, 2018

Uh oh!

jia200x left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

miri64 commented Oct 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

miri64 Oct 23, 2018

Choose a reason for hiding this comment

Uh oh!

miri64 commented Oct 25, 2018

Uh oh!

miri64 commented Oct 25, 2018

Uh oh!

miri64 commented Nov 8, 2018

Uh oh!

miri64 commented Nov 17, 2018

Uh oh!

miri64 commented Dec 6, 2018

Uh oh!

jia200x commented Dec 10, 2018

Uh oh!

jia200x commented Dec 10, 2018

Uh oh!

jia200x commented Dec 10, 2018

Uh oh!

jia200x left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

miri64 commented Oct 23, 2018 •

edited

Loading