Skip to content

gnrc_rpl_srh: cleanup and optimize SRH processing function#10227

Merged
miri64 merged 7 commits intoRIOT-OS:masterfrom
miri64:gnrc_rpl_srh/enh/cleanup-handler
Nov 27, 2018
Merged

gnrc_rpl_srh: cleanup and optimize SRH processing function#10227
miri64 merged 7 commits intoRIOT-OS:masterfrom
miri64:gnrc_rpl_srh/enh/cleanup-handler

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Oct 23, 2018 •
edited
Loading

Contribution description

While starting preparations I noticed that the processing function for RPL source routing headers in general needs some polishing. This PR provides that some code cleanup and optimizations.

Testing procedure

It can be ensured that the function is still doing the same behavior by using the tests-rpl_srh unittests. I also ensured with those on samr21-xpro with ps that the binary size and stack usage indeed goes down:

diff --git a/tests/unittests/tests-rpl_srh/Makefile.include b/tests/unittests/tests-rpl_srh/Makefile.include
index 4aa358a..7425e14 100644
--- a/tests/unittests/tests-rpl_srh/Makefile.include
+++ b/tests/unittests/tests-rpl_srh/Makefile.include
@@ -1,3 +1,6 @@
 USEMODULE += gnrc_ipv6
 USEMODULE += ipv6_addr
 USEMODULE += gnrc_rpl_srh
+USEMODULE += ps
+
+CFLAGS += -DDEVELHELP
diff --git a/tests/unittests/tests-rpl_srh/tests-rpl_srh.c b/tests/unittests/tests-rpl_srh/tests-rpl_srh.c
index 5bc75a2..f6390c7 100644
--- a/tests/unittests/tests-rpl_srh/tests-rpl_srh.c
+++ b/tests/unittests/tests-rpl_srh/tests-rpl_srh.c
@@ -19,6 +19,7 @@
 #include "net/ipv6/ext.h"
 #include "net/ipv6/hdr.h"
 #include "net/gnrc/rpl/srh.h"
+#include "ps.h"
 
 #include "unittests-constants.h"
 #include "tests-rpl_srh.h"
@@ -120,5 +121,6 @@ Test *tests_rpl_srh_tests(void)
 void tests_rpl_srh(void)
 {
     TESTS_RUN(tests_rpl_srh_tests());
+    ps();
 }
 /** @} */

Issues/PRs references

PR dependencies

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 23, 2018
@miri64 miri64 requested a review from cgundogan October 23, 2018 13:13
miri64 added a commit to miri64/RIOT that referenced this pull request Oct 23, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into …
dfa9ad7 
…HEAD
miri64 added a commit to miri64/RIOT that referenced this pull request Oct 23, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into…
34f4f89 
… HEAD
miri64 added a commit to miri64/RIOT that referenced this pull request Oct 23, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into…
a00ad34 
… HEAD
miri64 added a commit to miri64/RIOT that referenced this pull request Oct 24, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into…
85babc9 
… HEAD
miri64 added a commit to miri64/RIOT that referenced this pull request Oct 24, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into …
7d7b29e 
…HEAD
@miri64 miri64 mentioned this pull request Oct 24, 2018
Merged
@miri64 miri64 force-pushed the gnrc_rpl_srh/enh/cleanup-handler branch 2 times, most recently from 630f057 to 0aca680 Compare October 24, 2018 16:57
miri64 added a commit to miri64/RIOT that referenced this pull request Oct 24, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into …
7c7a57b 
…HEAD
@miri64 miri64 force-pushed the gnrc_rpl_srh/enh/cleanup-handler branch from 0aca680 to 80e85e3 Compare October 25, 2018 18:21
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Oct 25, 2018

Rebased to current master

miri64 added a commit to miri64/RIOT that referenced this pull request Oct 25, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into…
8436ef6 
… HEAD
miri64 added a commit to miri64/RIOT that referenced this pull request Oct 29, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into …
8fbcc50 
…HEAD
@miri64 miri64 mentioned this pull request Oct 30, 2018
Merged
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Oct 30, 2018

#10300 can help with testing this.

miri64 added a commit to miri64/RIOT that referenced this pull request Nov 6, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into…
d73419d 
… HEAD
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 7, 2018

@cgundogan can you have a look?

miri64 added a commit to miri64/RIOT that referenced this pull request Nov 8, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into…
6d3491e 
… HEAD
miri64 added a commit to miri64/RIOT that referenced this pull request Nov 14, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into…
5f9df66 
… HEAD
miri64 added a commit to miri64/RIOT that referenced this pull request Nov 17, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into…
76a79e5 
… HEAD
@miri64 miri64 force-pushed the gnrc_rpl_srh/enh/cleanup-handler branch from 535b7ed to d831758 Compare November 23, 2018 12:57
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 23, 2018

Rebased to current master to include #10300

miri64 added a commit to miri64/RIOT that referenced this pull request Nov 23, 2018
@miri64
Merge branch 'gnrc_rpl_srh/enh/cleanup-handler' (RIOT-OS#10227) into…
e1d3878 
… HEAD
@jia200x jia200x self-requested a review November 23, 2018 13:36
@jia200x jia200x added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines labels Nov 27, 2018
@jia200x
Copy link
Copy Markdown
Member

jia200x commented Nov 27, 2018

Tested on b-l072z-lrwan1. The size indeed goes down and all tests pass.

@jia200x jia200x added Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Nov 27, 2018
@jia200x
Copy link
Copy Markdown
Member

jia200x commented Nov 27, 2018

Ran vera++ and the code style is OK. Documentation also seems to be OK.

jia200x
jia200x approved these changes Nov 27, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@jia200x jia200x left a comment

Choose a reason for hiding this comment

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

ACK

@miri64 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 Nov 27, 2018
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 27, 2018

Restarted murdock build, since the last build was quite stale.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 27, 2018

Thanks for the review @jia200x! :-)

@miri64 miri64 merged commit 310ef3c into RIOT-OS:master Nov 27, 2018
@jia200x
Copy link
Copy Markdown
Member

jia200x commented Nov 27, 2018

@miri64 np :)

@miri64 miri64 deleted the gnrc_rpl_srh/enh/cleanup-handler branch November 27, 2018 17:27
@aabadie aabadie added this to the Release 2019.01 milestone Dec 2, 2018
@miri64 miri64 mentioned this pull request Dec 3, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jia200x jia200x jia200x approved these changes

@cgundogan cgundogan Awaiting requested review from cgundogan

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 Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines 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 Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

Release 2019.01

Development

Successfully merging this pull request may close these issues.

3 participants

@miri64 @jia200x @aabadie