gnrc_ipv6_ext: merge _handle_rh and gnrc_ipv6_ext_rh_process by miri64 · Pull Request #10238 · RIOT-OS/RIOT

miri64 · 2018-10-24T12:42:13Z

Contribution description

It makes much more sense (and saves us a huge ifdef), when _handle_rh and gnrc_ipv6_ext_rh are merged into one function.

Testing procedure

I used scapy again for testing (see testing procedure in #10234).

Issues/PRs references

Depends on ~~#10231~~ (merged) and ~~#10234 (and its dependencies)~~ (merged).

…into HEAD

miri64 · 2018-10-25T18:12:20Z

Rebased to current #10234

… into HEAD

miri64 · 2018-10-29T19:47:22Z

Rebased to current #10234.

…into HEAD

miri64 · 2018-11-06T19:46:27Z

Rebased to current master and #10234.

miri64 · 2018-11-06T19:52:42Z

Oops, forgot to fetch current master ^^

… into HEAD

miri64 · 2018-12-14T10:38:54Z

Rebased to current master. No longer dependent on other PRs.

… into HEAD

miri64 · 2018-12-17T09:29:33Z

Rebased to current master.

… into HEAD

jia200x · 2018-12-18T12:44:27Z

AFAIS the packet is being freed when it's expected. I'm OK with the code design.

jia200x · 2018-12-18T12:45:27Z

AFAIS the packet is being freed when it's expected. I'm OK with the code design.

... after the merge, I mean :)

jia200x · 2018-12-18T13:00:31Z

It works as described.

E.g

>>> sendp(Ether() /  
...:       IPv6(dst=DST_ADDR) / 
...:       UDP(sport=1339, dport=1337) / 
...:       "\x00\x01", iface="tapbr0")

> ipv6: GNRC_NETAPI_MSG_TYPE_RCV received
ipv6: Received (src = fe80::a9f1:fcc1:4389:4748, dst = fe80::4c3a:39ff:fe4f:45d2, next header = 17, length = 10)
ipv6: forward nh = 17 to other threads
ipv6: waiting for incoming message.
PKTDUMP: data rece>ived:
~~ SNIP  0 - size:   2 byte, type: NETTYPE_UNDEF (0)
000000 00  00  01
~~ SNIP  1 i- size:   8 byte, type: NETTYPE_UDpP (3)
   src-port:  1339  dst-porvt:  1337
   length: 10 6 cksum: 0xfc82
:~~ SNIP  2 - size:  40 byte, ty pe: NETTYPE_IPV6 (1)
Gtraffic class: 0x00 (ECN: N0x0, DSCP: 0x00)
flow lRabel: 0x00000
length: 10 C next header: 17  hop lim_it: 64
source address: Nfe80::a9f1:fcc1:4389:4748
Edestination address: fe80::4c3Ta:39ff:fe4f:45d2
~~ SNIAP  3 - size:  20 byte, type: NETTYPEP_NETIF (-1)
if_pid: 6  rssi: 0  Ilqi: 0
flags: 0x0
src__l2addr: 00:50:B6:8C:B5:1A
Mdst_l2addr: FF:FF:FF:FF:FF:FFS
~~ PKT    -  4 snips, Gtotal size:  70 byte
_TYPE_RCV received

or

>>> sendp(Ether() / 
...:       IPv6(dst=DST_ADDR) / 
...:       IPv6ExtHdrDestOpt() / 
...:       UDP(sport=1339, dport=1337) / 
...:       "\x00\x01", iface="tapbr0")

ipv6> ipv6: GNRC_NETAPI_MSG_TYPE_RCV received
ipv6: Received (src = fe80::a9f1:fcc1:4389:4748, dst = fe80::4c3a:39ff:fe4f:45d2, next header = 60, length = 18)
ipv6: handle extension header (protnum = 60)
ipv6_ext: next header = 60
ipv6_ext: skipping over unsupported extension header
ipv6: forward nh = 17 to other threads
ipv6: waiting for incoming message.
PKTDUMP: data received:
~~ SNIP  0 - size:   2 byte, type: NETTYPE_UNDEF (0)
00000000  00  01
~~ SNIP  1 - size:   8 byte, type: NETTYPE_UDP (4)
   src-port:  1339  dst-port:  1337
   length: 10  cksum: 0xfc82
~~ SNIP  2 - size:   8 byte, type: NETTYPE_UNKNOWN (2)
00000000  11  00  01  04  00  00  00  00
~~ SNIP  3 - size:  40 byte, type: NETTYPE_IPV6 (1)
traffic class: 0x00 (ECN: 0x0, DSCP: 0x00)
flow label: 0x00000
length: 18  next header: 60  hop limit: 64
source address: fe80::a9f1:fcc1:4389:4748
destination address: fe80::4c3a:39ff:fe4f:45d2
~~ SNIP  4 - size:  20 byte, type: NETTYPE_NETIF (-1)
if_pid: 6  rssi: 0  lqi: 0
flags: 0x0
src_l2addr: 00:50:B6:8C:B5:1A
dst_l2addr: FF:FF:FF:FF:FF:FF
~~ PKT    -  5 snips, total size:  78 byte
: waiting for incoming message.

jia200x · 2018-12-18T13:02:10Z

The documentation also looks OK.

sys/net/gnrc/network_layer/ipv6/ext/rh/gnrc_ipv6_ext_rh.c

It's a lot cleaner and makes more sense if we merge those two.

miri64 · 2018-12-18T18:04:34Z

Changed @cgundogan mailing addresses as requested by him.

miri64 · 2018-12-18T18:04:44Z

(and squashed directly).

cgundogan

code change looks sensible and comments by @jia200x were already addressed. ACK

comments was already addressed

miri64 added Area: network Area: Networking Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. labels Oct 24, 2018

miri64 requested review from aabadie, bergzand and cgundogan October 24, 2018 12:42

miri64 force-pushed the gnrc_ipv6_ext/cleanup/merge-handle-rh branch from 67b4249 to b6da875 Compare October 24, 2018 12:42

miri64 changed the title ~~Gnrc ipv6 ext/cleanup/merge handle rh~~ gnrc_ipv6_ext: merge _handle_rh and gnrc_ipv6_ext_rh_process Oct 24, 2018

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

Merge branch 'gnrc_ipv6_ext/cleanup/merge-handle-rh' (RIOT-OS#10238) …

cd7aeab

…into HEAD

miri64 force-pushed the gnrc_ipv6_ext/cleanup/merge-handle-rh branch from b6da875 to 4d8fae8 Compare October 24, 2018 14:01

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

Merge branch 'gnrc_ipv6_ext/cleanup/merge-handle-rh' (RIOT-OS#10238) …

46a4063

…into HEAD

This was referenced Oct 24, 2018

gnrc_ipv6_ext_rh: add ICMPv6 error message sending #10242

Merged

gnrc_ipv6_ext: move ipv6_ext_rh (partly) to GNRC #10231

Merged

miri64 force-pushed the gnrc_ipv6_ext/cleanup/merge-handle-rh branch from 013a4ce to 9e916ef Compare October 24, 2018 19:45

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

Merge branch 'gnrc_ipv6_ext/cleanup/merge-handle-rh' (RIOT-OS#10238) …

6aa21dc

…into HEAD

miri64 force-pushed the gnrc_ipv6_ext/cleanup/merge-handle-rh branch from 9e916ef to f9ed9d2 Compare October 25, 2018 18:11

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

Merge branch 'gnrc_ipv6_ext/cleanup/merge-handle-rh' (RIOT-OS#10238)…

3c3d6b7

… into HEAD

miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Oct 25, 2018

RIOT-OS deleted a comment Oct 26, 2018

miri64 force-pushed the gnrc_ipv6_ext/cleanup/merge-handle-rh branch 2 times, most recently from 4344794 to 49d5ce3 Compare October 29, 2018 19:47

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

Merge branch 'gnrc_ipv6_ext/cleanup/merge-handle-rh' (RIOT-OS#10238) …

c276cd6

…into HEAD

RIOT-OS deleted a comment Oct 30, 2018

miri64 force-pushed the gnrc_ipv6_ext/cleanup/merge-handle-rh branch from 49d5ce3 to 8e98c30 Compare November 6, 2018 19:46

miri64 force-pushed the gnrc_ipv6_ext/cleanup/merge-handle-rh branch from 8e98c30 to c522dc6 Compare November 6, 2018 19:53

miri64 added a commit to miri64/RIOT that referenced this pull request Nov 6, 2018

Merge branch 'gnrc_ipv6_ext/cleanup/merge-handle-rh' (RIOT-OS#10238)…

a666e06

… into HEAD

miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 14, 2018

miri64 force-pushed the gnrc_ipv6_ext/cleanup/merge-handle-rh branch from 8bd46b3 to b930af2 Compare December 14, 2018 10:38

miri64 added a commit to miri64/RIOT that referenced this pull request Dec 14, 2018

Merge branch 'gnrc_ipv6_ext/cleanup/merge-handle-rh' (RIOT-OS#10238)…

e7adc5e

… into HEAD

RIOT-OS deleted a comment Dec 15, 2018

miri64 force-pushed the gnrc_ipv6_ext/cleanup/merge-handle-rh branch from b930af2 to f01ab11 Compare December 17, 2018 09:29

miri64 added a commit to miri64/RIOT that referenced this pull request Dec 17, 2018

Merge branch 'gnrc_ipv6_ext/cleanup/merge-handle-rh' (RIOT-OS#10238)…

94dd857

… into HEAD

miri64 mentioned this pull request Dec 17, 2018

gnrc_ipv6_ext: needs heavy refactoring #10133

Closed

5 tasks

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

jia200x added the Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines label Dec 18, 2018

miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 18, 2018

jia200x added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Dec 18, 2018

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

jia200x previously requested changes Dec 18, 2018

View reviewed changes

sys/net/gnrc/network_layer/ipv6/ext/rh/gnrc_ipv6_ext_rh.c Outdated Show resolved Hide resolved

miri64 force-pushed the gnrc_ipv6_ext/cleanup/merge-handle-rh branch from f01ab11 to 5a70723 Compare December 18, 2018 17:00

gnrc_ipv6_ext: merge _handle_rh and gnrc_ipv6_ext_rh_process

8701c0f

It's a lot cleaner and makes more sense if we merge those two.

miri64 force-pushed the gnrc_ipv6_ext/cleanup/merge-handle-rh branch from 5a70723 to 8701c0f Compare December 18, 2018 18:04

cgundogan approved these changes Dec 18, 2018

View reviewed changes

miri64 merged commit 13e66ce into RIOT-OS:master Dec 18, 2018

miri64 deleted the gnrc_ipv6_ext/cleanup/merge-handle-rh branch December 18, 2018 18:48

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

Conversation

miri64 commented Oct 24, 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 commented Oct 25, 2018

Uh oh!

miri64 commented Oct 29, 2018

Uh oh!

miri64 commented Nov 6, 2018

Uh oh!

miri64 commented Nov 6, 2018

Uh oh!

miri64 commented Dec 14, 2018

Uh oh!

miri64 commented Dec 17, 2018

Uh oh!

jia200x commented Dec 18, 2018

Uh oh!

jia200x commented Dec 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jia200x commented Dec 18, 2018

Uh oh!

jia200x commented Dec 18, 2018

Uh oh!

Uh oh!

miri64 commented Dec 18, 2018

Uh oh!

miri64 commented Dec 18, 2018

Uh oh!

cgundogan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

miri64 commented Oct 24, 2018 •

edited

Loading

jia200x commented Dec 18, 2018 •

edited

Loading