Skip to content

Kconfig: Expose gnrc/ipv6/ext/frag configurations#12958

Merged
cgundogan merged 5 commits intoRIOT-OS:masterfrom
leandrolanzieri:pr/kconfig_migrate/gnrc/ipv6_ext_hdr
Jan 30, 2020
Merged

Kconfig: Expose gnrc/ipv6/ext/frag configurations#12958
cgundogan merged 5 commits intoRIOT-OS:masterfrom
leandrolanzieri:pr/kconfig_migrate/gnrc/ipv6_ext_hdr

Conversation

@leandrolanzieri
Copy link
Copy Markdown
Contributor

@leandrolanzieri leandrolanzieri commented Dec 16, 2019

Contribution description

This PR moves the configuration macros of gnrc_ipv6_ext_frag to the CONFIG_ namespace, and exposes them to Kconfig.

In the test application a check was added, so the parameters are not changed via CFLAGS when Kconfig is being used, that's why this depends on #12913.

Testing procedure

  • tests/gnrc_ipv6_ext_frag should still work as usual.
  • You should be able to configure the module's parameters via make menuconfig in the test application.

Issues/PRs references

Part of #12888
Depends on #12913

@leandrolanzieri leandrolanzieri added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT State: waiting for other PR State: The PR requires another PR to be merged first TF: Config Marks issues and PRs related to the work of the Configuration Task Force labels Dec 16, 2019
@leandrolanzieri leandrolanzieri added this to the Release 2020.01 milestone Dec 16, 2019
@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/gnrc/ipv6_ext_hdr branch from e8da247 to f22d1d2 Compare December 20, 2019 10:11
@leandrolanzieri
Copy link
Copy Markdown
Contributor Author

Rebased to master, this has no dependencies now.

@leandrolanzieri leandrolanzieri removed the State: waiting for other PR State: The PR requires another PR to be merged first label Dec 20, 2019
@fjmolinas
Copy link
Copy Markdown
Contributor

@leandrolanzieri this one needs rebase.

@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/gnrc/ipv6_ext_hdr branch from f22d1d2 to aeffa05 Compare January 9, 2020 07:50
@leandrolanzieri
Copy link
Copy Markdown
Contributor Author

@leandrolanzieri this one needs rebase.

@fjmolinas Thanks, rebased

@tcschmidt
Copy link
Copy Markdown
Member

@miri64 @leandrolanzieri do we get last issues resolved?

Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

When setting CONFIG_GNRC_IPV6_EXT_FRAG_LIMIT_POOL_SIZE to 2 I can't compile anymore tests/gnrc_ipv6_ext_frag:

$ make menuconfig
Using default symbol values (no '/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_ipv6_ext_frag/bin/native/generated/merged.config')
No changes to save (for '/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_ipv6_ext_frag/bin/native/generated/merged.config')
$ make
Building application "tests_gnrc_ipv6_ext_frag" for "native" with MCU "native".

In file included from <command-line>:
/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_ipv6_ext_frag/bin/native/generated/autoconf.h:72: error: "CONFIG_GNRC_IPV6_EXT_FRAG_LIMITS_POOL_SIZE" redefined [-Werror]
   72 | #define CONFIG_GNRC_IPV6_EXT_FRAG_LIMITS_POOL_SIZE 2
      | 
<command-line>: note: this is the location of the previous definition
In file included from <command-line>:
/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_ipv6_ext_frag/bin/native/riotbuild/riotbuild.h:4: error: "CONFIG_GNRC_IPV6_EXT_FRAG_LIMITS_POOL_SIZE" redefined [-Werror]
    4 | #define CONFIG_GNRC_IPV6_EXT_FRAG_LIMITS_POOL_SIZE 3
      | 
In file included from <command-line>:
/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_ipv6_ext_frag/bin/native/generated/autoconf.h:72: note: this is the location of the previous definition
   72 | #define CONFIG_GNRC_IPV6_EXT_FRAG_LIMITS_POOL_SIZE 2
      | 
cc1: all warnings being treated as errors
make[1]: *** [/home/mlenders/Repositories/RIOT-OS/RIOT/Makefile.base:103: /home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_ipv6_ext_frag/bin/native/application_tests_gnrc_ipv6_ext_frag/main.o] Error 1
make: *** [/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_ipv6_ext_frag/../../Makefile.include:540: /home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_ipv6_ext_frag/bin/native/application_tests_gnrc_ipv6_ext_frag.a] Error 2

Regarding the wording, I know that you mostly just copied what I wrote, but I have the feeling in isolation this has to be more precise.

@leandrolanzieri
Copy link
Copy Markdown
Contributor Author

When setting CONFIG_GNRC_IPV6_EXT_FRAG_LIMIT_POOL_SIZE to 2 I can't compile anymore tests/gnrc_ipv6_ext_frag:

I placed the Kconfig symbol check wrongly in the Makefile, now this should be fixed.

@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/gnrc/ipv6_ext_hdr branch from b868cc7 to 2a7b9a9 Compare January 29, 2020 11:07
@leandrolanzieri
Copy link
Copy Markdown
Contributor Author

Rebased to current master.

Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

My comments were addressed. I tested this PR using tests/gnrc_ipv6_ext_frag on native and samr21-xpro, the tests still succeed.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 30, 2020

Please squash!

Macros that changed:
GNRC_IPV6_EXT_FRAG_SEND_SIZE -> CONFIG_GNRC_IPV6_EXT_FRAG_SEND_SIZE
GNRC_IPV6_EXT_FRAG_RBUF_SIZE -> CONFIG_GNRC_IPV6_EXT_FRAG_RBUF_SIZE
GNRC_IPV6_EXT_FRAG_LIMITS_POOL_SIZE -> CONFIG_GNRC_IPV6_EXT_FRAG_LIMITS_POOL_SIZE
GNRC_IPV6_EXT_FRAG_RBUF_TIMEOUT_US -> CONFIG_GNRC_IPV6_EXT_FRAG_RBUF_TIMEOUT_US
This test needs the pool size for limit objects set to 3 by default so
it does not fail. As this is done with the 'app.config' file, we
explicitly disable Kconfig by default.
@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/gnrc/ipv6_ext_hdr branch from 2a7b9a9 to 6481076 Compare January 30, 2020 16:43
@leandrolanzieri leandrolanzieri added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 30, 2020
@cgundogan
Copy link
Copy Markdown
Member

Murdock shows green light! GO

@cgundogan cgundogan merged commit 0cf8bf1 into RIOT-OS:master Jan 30, 2020
@leandrolanzieri leandrolanzieri deleted the pr/kconfig_migrate/gnrc/ipv6_ext_hdr branch January 31, 2020 08:02
@leandrolanzieri leandrolanzieri added the Area: Kconfig Area: Kconfig integration label Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Kconfig Area: Kconfig integration 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

Development

Successfully merging this pull request may close these issues.

5 participants