Skip to content

Kconfig: Expose gnrc/ipv6/whitelist configurations#12893

Merged
miri64 merged 5 commits intoRIOT-OS:masterfrom
leandrolanzieri:pr/kconfig_migrate/gnrc/ipv6_whitelisting
Dec 10, 2019
Merged

Kconfig: Expose gnrc/ipv6/whitelist configurations#12893
miri64 merged 5 commits intoRIOT-OS:masterfrom
leandrolanzieri:pr/kconfig_migrate/gnrc/ipv6_whitelisting

Conversation

@leandrolanzieri
Copy link
Copy Markdown
Contributor

@leandrolanzieri leandrolanzieri commented Dec 6, 2019

Contribution description

This moves the gnrc_ipv6_whitelist module configuration macros to the CONFIG_ namespace and exposes them to Kconfig.

Testing procedure

  • Compile gnrc_networking example application using this module (USEMODULE=gnrc_ipv6_whitelist make all term), the default configuration should still be the same.

  • Call menuconfig using this module (USEMODULE=gnrc_ipv6_whitelist make menuconfig), you should the list size as an option. The default list size should still be the same. If you change the value it should change in the app.

Issues/PRs references

Part of #12888

@leandrolanzieri leandrolanzieri added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT TF: Config Marks issues and PRs related to the work of the Configuration Task Force labels Dec 6, 2019
@leandrolanzieri leandrolanzieri added this to the Release 2020.01 milestone Dec 6, 2019
@leandrolanzieri leandrolanzieri mentioned this pull request Dec 6, 2019
Open
71 tasks
miri64
miri64 reviewed Dec 6, 2019
View reviewed changes
sys/include/net/gnrc/ipv6/whitelist.h
#ifndef GNRC_IPV6_WHITELIST_SIZE
#define GNRC_IPV6_WHITELIST_SIZE (8)
#ifndef CONFIG_GNRC_IPV6_WHITELIST_SIZE
#define CONFIG_GNRC_IPV6_WHITELIST_SIZE (8)
Copy link
Copy Markdown
Member

@miri64 miri64 Dec 6, 2019

Choose a reason for hiding this comment

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

Do we have to rename all config macros to be usable with kconfig?

Copy link
Copy Markdown
Contributor Author

@leandrolanzieri leandrolanzieri Dec 6, 2019

Choose a reason for hiding this comment

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

Yes, Kconfig generates the .config and .h files appending the CONFIG_ symbol. Regardless of that, I think it's good to have configuration macros under a single namespace.

Copy link
Copy Markdown
Member

@miri64 miri64 Dec 6, 2019

Choose a reason for hiding this comment

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

Ok, were just wondering for future integrations.

@jia200x
Copy link
Copy Markdown
Member

jia200x commented Dec 10, 2019

I set the whitelist size to 100 via menuconfig and applied the following patch:

diff --git a/examples/gnrc_networking/main.c b/examples/gnrc_networking/main.c
index 6301f4291..c9b4b9efe 100644
--- a/examples/gnrc_networking/main.c
+++ b/examples/gnrc_networking/main.c
@@ -39,6 +39,7 @@ int main(void)
      * receive potentially fast incoming networking packets */
     msg_init_queue(_main_msg_queue, MAIN_QUEUE_SIZE);
     puts("RIOT network stack example application");
+    printf("%i\n", CONFIG_GNRC_IPV6_WHITELIST_SIZE);
 
     /* start shell */
     puts("All up, running the shell now");

The output is

main(): This is RIOT! (Version: 2020.01-devel-1311-g9a0a51-copr/12893)
RIOT network stack example application
100
All up, running the shell now

So, it works as expected.

@jia200x
Copy link
Copy Markdown
Member

jia200x commented Dec 10, 2019

implementation of KConfig files looks also good. It should be easy to proceed.

jia200x
jia200x approved these changes Dec 10, 2019
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 on my side. @miri64 do you have something to add?

@jia200x jia200x added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 10, 2019
@miri64
Copy link
Copy Markdown
Member

miri64 commented Dec 10, 2019

@miri64 do you have something to add?

Nope all good from my side. Murdock seems to have failed on an unrelated error though.

@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 Dec 10, 2019
@miri64 miri64 merged commit d926810 into RIOT-OS:master Dec 10, 2019
@leandrolanzieri leandrolanzieri deleted the pr/kconfig_migrate/gnrc/ipv6_whitelisting branch December 10, 2019 17:34
@leandrolanzieri leandrolanzieri mentioned this pull request Dec 11, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 left review comments

@jia200x jia200x jia200x 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 TF: Config Marks issues and PRs related to the work of the Configuration Task Force Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Milestone

Release 2020.01

Development

Successfully merging this pull request may close these issues.

3 participants

@leandrolanzieri @jia200x @miri64