-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Network stack runtime configuration #97266
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
base: main
Are you sure you want to change the base?
Conversation
|
f5dd020 to
9d1bacb
Compare
|
@pdgendt compliance checker complains about python code The code looks like this The code looks correct or am I missing something here? |
There's a distinction between python built-in libraries vs third-party libraries. You can run the following to fix it: $ ruff check --fix --select I scripts/net/net-yaml-config.py |
64825cb to
56dd77e
Compare
include/zephyr/net/net_config.h
Outdated
| #if defined(CONFIG_NET_CONFIG_SETTINGS) | ||
| #include <net_init_config.inc> | ||
| #else | ||
| struct net_init_config; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this struct generated from the yaml input? If so, almost any change would result in a different sizeof and make it unable to read from the settings subsystem, correct?
I think that's an issue to be able to update your devices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The size would change only if one changes the schema. There is a hash embedded into the data to detect this and discard the saved data if that happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a similar concern here, like I had in #68127 (comment). Do we want to maintain our own IDL? This code hinges on many external details, and I'm afraid this can break easily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how I could make this code / use case more simpler. Introducing nanopb here feels complex, but perhaps I have been too concentrated to supporting the networking use case (testing multiple network interfaces). One big problem when saving user changed data, is the fact that the data is structured i.e., there can be multiple network interfaces, each with additional structured data like IP address etc. Saving this data to flat space like key=value is not very easy, and then we could just do the same with current Kconfig variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think we should pick up a discussion in the arch meeting first? CC @carlescufi @henrikbrixandersen
You're putting a lot of effort into this, I'd hate it if we can't come to a consensus and let it go to waste (again).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the plan is to discuss this on next arch meeting on Tue.
8fec811 to
4562de7
Compare
4562de7 to
3ae87c6
Compare
|
2159a72 to
438f419
Compare
|
Architecture WG meeting:
to
|
438f419 to
2ad8e9d
Compare
|
Updated the code according to comments
|
Add a way to convert a user supplied yaml file to a configuration file. For networking, a C header file is generated which can then be included to the application. Add a cmake function that calls the generate config macros with suitable values for networking needs. Signed-off-by: Jukka Rissanen <[email protected]>
This contains rules to validate network configuration in yaml format. Signed-off-by: Jukka Rissanen <[email protected]>
Add a python script that converts a user supplied network configuration yaml file to a valid network configuration C header file. Signed-off-by: Jukka Rissanen <[email protected]>
If CONFIG_NET_CONFIG_SETTINGS is enabled, then the network configuration library will check if a yaml file network-config.yaml is found in the application directory and use it to generate network initial configuration that is then applied when the device boots. If the yaml file is not there, then the relevant Kconfig options are used to create initial network configuration. Signed-off-by: Jukka Rissanen <[email protected]>
User can set NET_CONFIG_FILE to override the name of the default configuration file name which is network-config.yaml This way a substitute yaml config file can be used instead of the default one. Signed-off-by: Jukka Rissanen <[email protected]>
Example networking yaml configuration file. This contains more complex network setup that can be used to see what can be done. Signed-off-by: Jukka Rissanen <[email protected]>
Add documentation about the network stack configuration using a yaml file. Signed-off-by: Jukka Rissanen <[email protected]>
If user is not interested in the mask length, the value can be set to NULL so that it is not been returned to the caller. Signed-off-by: Jukka Rissanen <[email protected]>
If user has supplied a yaml file, it is used as configuration data that is applied to network configuration. In this case the CONFIG_NET_CONFIG_* Kconfig variables used for configuration are ignored. Signed-off-by: Jukka Rissanen <[email protected]>
Add simple unit tests that verify that options specified in yaml file can be used to configure network stack. Signed-off-by: Jukka Rissanen <[email protected]>
Add set and get functions that return/set either the default settings or if user has changed them, the user adjusted value. Make these two functions to use settings API. Signed-off-by: Jukka Rissanen <[email protected]>
The hash is used to detect whether the yaml schema file is changed. If the data format in the settings is different that what is found in the ROM, then the settings db must be discarded because the data is now incompatible. Signed-off-by: Jukka Rissanen <[email protected]>
Make sure we run the tests with settings db enabled. Signed-off-by: Jukka Rissanen <[email protected]>
Add "net config" command to view the current configuration and modify or remove the configuration data. Signed-off-by: Jukka Rissanen <[email protected]>
Define max length of the IPv6 address as a string without IPv4 mapping. Useful if you have to print IPv6 address string that will never contain IPv4 mapping address. Signed-off-by: Jukka Rissanen <[email protected]>
If ipv4_multicast_address[] is not set in the yaml config file,
then compiler may emit this warning which looks like false positive:
net/lib/config/init.c: In function 'ipv4_setup.constprop':
net/lib/config/init.c:599:67: warning: offset '4294967279' outside
bounds of constant string [-Warray-bounds]
599 | ret = parse_mask(ipv4->ipv4_multicast_addresses[j].value,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
In file included from zephyr/subsys/net/lib/config/init.c:2028:
build/test/zephyr/include/generated/net_init_config.inc:137:37:
note: 'net_init_config_data' declared here
137 | static const struct net_init_config net_init_config_data = {
| ^~~~~~~~~~~~~~~~~~~~
If user has specified ipv4_multicast_addresses in the yaml file,
there is no warning printed.
Disable optimization to avoid these warnings.
Signed-off-by: Jukka Rissanen <[email protected]>
2ad8e9d to
65df8f4
Compare
|
|
@nordicjm I noticed the same, not sure what is going on there. Most likely something related to this PR, I will look into it after the 4.3 has been released. |
| # from a user supplied yaml file. | ||
| # | ||
| function(generate_config_file | ||
| source_file # The yaml source file to be converted to config data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmake indent is 2 spaces, also the functions are not documented like this in CMake and it goes in the bit above, see https://github.com/zephyrproject-rtos/zephyr/blob/main/cmake/modules/extensions.cmake#L1615 for an example
cmake/modules/kernel.cmake
Outdated
| # If network configuration library is enabled, check if the yaml file | ||
| # is present and use it to generate an initial configuration. Otherwise | ||
| # use the .config file to generate the initial configuration. | ||
| if(EXISTS ${APPLICATION_SOURCE_DIR}/network-config.yaml) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like this should be using the zephyr_file() functions so they can pick up e.g. file suffix, also makes me think there should be an option to specify the exact file to use
| zephyr_get(EXTRA_CONF_FILE SYSBUILD LOCAL VAR EXTRA_CONF_FILE OVERLAY_CONFIG MERGE REVERSE) | ||
| zephyr_get(EXTRA_DTC_OVERLAY_FILE SYSBUILD LOCAL MERGE REVERSE) | ||
| zephyr_get(DTS_EXTRA_CPPFLAGS SYSBUILD LOCAL MERGE REVERSE) | ||
| zephyr_get(NET_INIT_CONFIG_FILE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be using SYSBUILD LOCAL flags
| one network interface that needs to be setup at boot. | ||
| See available Kconfig options in the network API documentation. | ||
|
|
||
| * A yaml configuration file ``network-config.yaml``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:file:`name`
| The net-init-config.yaml Syntax | ||
| ******************************* | ||
|
|
||
| The ``network-config.yaml`` can be placed to the application directory. When the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as above
| source "subsys/net/Kconfig.template.log_config.net" | ||
|
|
||
| menuconfig NET_CONFIG_SETTINGS | ||
| bool "Set network settings for applications" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| bool "Set network settings for applications" | |
| bool "Set network settings for applications [EXPERIMENTAL]" |
| #if defined(CONFIG_NET_CONFIG_SETTINGS) | ||
| #include <network_config.inc> | ||
| #else | ||
| struct networking; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be a more unique name for this, networking is very plain and non-descriptive
| BUILD_ASSERT(sizeof(((struct networking *)0)->interfaces[0].field) >= (size), \ | ||
| "Field " STRINGIFY(field) " is too small, expecting " STRINGIFY(size) " bytes") | ||
|
|
||
| CHECK_FIELD(ipv4.ipv4_addresses[0].value, INET_ADDRSTRLEN + sizeof("/32") - 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this check if IPV4, IPV6 Kconfigs are enabled?
|
|
||
| config NET_CONFIG_SETTINGS_SHELL_ACCESS | ||
| bool "Configure options via shell" | ||
| select SETTINGS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
depends on SHELL and depends on SETTINGS (instead)?
| # Disable optimization for only init.c in order to avoid false | ||
| # positives from bounds checking in optimized builds. | ||
| set_source_files_properties( | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/init.c | ||
| PROPERTIES COMPILE_OPTIONS "-O0" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not think this should be here, should not be adjusting build flags for files



Note that this PR is continuing the work started on #68127. Most of the commits come directly from that PR, couple of latest ones are new as they add settings db support.
Executive summary of how the PR works:
More detailed description follows:
This PR adds support to autoconfigure the network sub-system when the device boots. It is mainly meant to be used when there are multiple network interfaces in the system. In that case Kconfig based network configure parameters cannot be used easily as the system will only apply the configuration to one of the network interfaces.
This is related to old enhancement #29750 request. There it was pondered to use devicetree to configuring network stack. As DT is only meant to describe hardware configuration some other way of configuration is needed.
This PR uses yaml file to describe network stack configuration. The configuration data in yaml file is not restricted to network configuration but any subsystem configuration could be done via it. I concentrated to network configuration as we need this multi network interface autoconfiguration in networking.
The PR provides minimal infrastructure to validate a user supplied yaml file and then generate a C configuration data file from it. So we do not import the yaml file directly to the zephyr image, but we use a generator script to generate C header file that is then included by the network config library. When the device boots, the network configuration library (
subsys/net/lib/config/init.c) reads the configuration stored in flash, and applies the configuration to relevant network interface. When the user creates the yaml file in host system, they should know the network configuration in order to map the yaml file data to the device network configuration. The configuration data from flash is used as a default read-only configuration.The user supplied yaml file can be stored outside of zephyr. When building the network application, the location of the yaml file is given so the build system can then utilize it.
At runtime, user is able to adjust the configuration values stored in flash. The changes will be stored in settings database, the backend can be selected by the user. The changes to the network configuration can be done via
net_config_get/set()API or via anet configshell which provides commands (view/set/remove) to adjust the values in the network configuration. When the device boots, the default network configuration in flash is used as a base and any user changed network configuration values from settings db are applied on top of the default network configuration.Relates to #77638