Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gnrc_netif_conf: fix auto-6ctx switch #18370

Merged
merged 1 commit into from
Jul 26, 2022

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jul 26, 2022

Contribution description

When configuring a boolean via Kconfig and setting it to n, the result is that the symbol will not be defined as a preprocessor macro. As a result, setting CONFIG_GNRC_NETIF_IPV6_BR_AUTO_6CTX to n has no effect, as

#ifndef CONFIG_GNRC_NETIF_IPV6_BR_AUTO_6CTX
#define CONFIG_GNRC_NETIF_IPV6_BR_AUTO_6CTX   1
#endif

(as was pointed out in #17678 (comment)). Without changing the name to a negative, the only other valid option is to check if gnrc_netif was configured via Kconfig in addition to the normal #ifndef. This PR applies that change.

Testing procedure

Once a global address is configured with

BOARD=native RIOT_CONFIG_GNRC_NETIF_IPV6_BR_AUTO_6CTX=n RIOT_CONFIG_KCONFIG_USEMODULE_GNRC_NETIF=y make -C examples/gnrc_border_router flash -j term

the table returned by 6ctx should remain empty.

Issues/PRs references

Follow-up on #17678

@miri64 miri64 added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Jul 26, 2022
@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 26, 2022
@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System labels Jul 26, 2022
@miri64 miri64 added CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Jul 26, 2022
@leandrolanzieri
Copy link
Contributor

I think the test proceeding is missing a SHOULD_RUN_KCONFIG=1 or a previous Kconfig run to actually run Kconfig and apply the command line options.

BOARD=native RIOT_CONFIG_GNRC_NETIF_IPV6_BR_AUTO_6CTX=n RIOT_CONFIG_KCONFIG_USEMODULE_GNRC_NETIF=y SHOULD_RUN_KCONFIG=1 make -C examples/gnrc_border_router -j

I tested by adding a check in gnrc_netif.c:

#if IS_ACTIVE(CONFIG_GNRC_NETIF_IPV6_BR_AUTO_6CTX)
#error "Compression context"
#else
#error "No compression context"
#endif

With this PR the option is not overwritten by the header file when configuring with Kconfig.

@miri64
Copy link
Member Author

miri64 commented Jul 26, 2022

I think the test proceeding is missing a SHOULD_RUN_KCONFIG=1 or a previous Kconfig run to actually run Kconfig and apply the command line options.

IIRC with the RIOT_CONFIG environment variables this is implied.

@miri64 miri64 enabled auto-merge July 26, 2022 14:36
@leandrolanzieri
Copy link
Contributor

I think the test proceeding is missing a SHOULD_RUN_KCONFIG=1 or a previous Kconfig run to actually run Kconfig and apply the command line options.

IIRC with the RIOT_CONFIG environment variables this is implied.

The example is explicitly disabling it

# As there is an 'Kconfig' we want to explicitly disable Kconfig by setting
# the variable to empty
SHOULD_RUN_KCONFIG ?=

@miri64 miri64 merged commit 7ee1b58 into RIOT-OS:master Jul 26, 2022
@miri64 miri64 deleted the gnrc_netif_conf/fix/6ctx-switch branch July 26, 2022 19:31
@miri64
Copy link
Member Author

miri64 commented Jul 26, 2022

Backport provided in #18375

@maribu maribu added this to the Release 2022.10 milestone Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants