Skip to content

Conversation

@jukkar
Copy link
Member

@jukkar jukkar commented Jan 25, 2024

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 configurator reads the configuration stored in ROM, and applies the configuration to each network interface found in the system. When the user creates the yaml file, she/he should know the network configuration in order to map the yaml file to the device in use.

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.

I am planning to add documentation and tests, either in this PR or in subsequent PRs.

Please take a look and comment.

@hongshui3000
Copy link
Contributor

According to my understanding of the current method, is it necessary to rebuild the entire C code every time the network needs to be reconfigured, instead of just building the yaml generation settings file?

@jukkar
Copy link
Member Author

jukkar commented Jan 26, 2024

is it necessary to rebuild the entire C code every time the network needs to be reconfigured, instead of just building the yaml generation settings file?

Hmm, the yaml->c-file generation is done following the same design patter as when calling existing generate_inc_file_for_target() and related functions. When I do touch my-config-file.yaml and then running ninja manually, only the changed files are compiled.

build/echo-server$ touch configuration.yaml
build/echo-server$ ninja
[4/6] Linking C executable zephyr/zephyr.elf
Generating files from build/echo-server/zephyr/zephyr.elf for board: native_sim
[6/6] cd build/echo-server/zephyr && /u...ile=build/echo-server/zephyr/zephyr.exe

@hongshui3000
Copy link
Contributor

is it necessary to rebuild the entire C code every time the network needs to be reconfigured, instead of just building the yaml generation settings file?

Hmm, the yaml->c-file generation is done following the same design patter as when calling existing generate_inc_file_for_target() and related functions. When I do touch my-config-file.yaml and then running ninja manually, only the changed files are compiled.

build/echo-server$ touch configuration.yaml
build/echo-server$ ninja
[4/6] Linking C executable zephyr/zephyr.elf
Generating files from build/echo-server/zephyr/zephyr.elf for board: native_sim
[6/6] cd build/echo-server/zephyr && /u...ile=build/echo-server/zephyr/zephyr.exe

image
It may be troublesome to update some configurations dynamically, and you need to re-update the entire zephyr code.

@jukkar
Copy link
Member Author

jukkar commented Jan 26, 2024

It may be troublesome to update some configurations dynamically, and you need to re-update the entire zephyr code.

Note that all of this is just enhancing the existing way of auto configuration in networking stack. Currently only Kconfig way is possible, but this PR would allow multiple network interfaces to be configured according to the setup defined in yaml file.
In all cases this configuration data is created at built time and is compiled in. User does not need to use the network config library at all, and everything can be done via some other mean (like settings db etc) during runtime. But that is out of scope of this PR.

@jukkar jukkar force-pushed the devel/yaml-for-net branch from 837d820 to ab53c73 Compare January 26, 2024 08:22
@hongshui3000
Copy link
Contributor

It seems that I can't delete some configuration items that I don't need, or add some custom configuration items, or overwrite some default configuration items like dts configuration???

Comment on lines 28 to 39
Copy link
Contributor

Choose a reason for hiding this comment

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

This is really not good, who sets up a yaml file for a unique device and makes the configuration unchangable? Imagine buying a printer whereby it has one IP address that you can't change, and you can't change the mode from static IP to DHCP or anything like that. Now also imagine you're someone manufacturing devices for production with zephyr-based firmware, how would you even use this? Seems overly complex and not very useful to myself as either a developer, a systems integrator, or an end user

Copy link
Member Author

Choose a reason for hiding this comment

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

Please note that this is just an example file, no one in their sane mind would use static IP address in the production device. But we need to set these for testing purposes, that is why you see various static IP addresses in our prj.conf files in the tests/net and samples/net. This PR is nothing more than just allowing us to configure the device with more than one network interface, all of this is already in use for one network interface (using Kconfig variables).

Copy link
Member Author

Choose a reason for hiding this comment

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

Also note that this PR is not about provisioning the device by the vendor. It allows that kind of use, but user needs to be very careful about that as you noticed.

Copy link
Contributor

Choose a reason for hiding this comment

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

A test can just set it up manually, I see no need for this. What would be useful is if there was a system on the device itself that it could set/use settings for interfaces, e.g. dhcpcd config from linux but using settings system, that would be really useful

Copy link
Contributor

Choose a reason for hiding this comment

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

Shameless plug to my comment here, where a concise binary representation would help here.

Scenario Runtime update: encode and write it to a settings backend, or littlefs and read it back after cold boot/wakeup.

Even supports DFU as the data can be decoded in a backwards compatible way.

Copy link
Contributor

Choose a reason for hiding this comment

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

That would be brilliant because basically anyone doing this now in zephyr has to do it from scratch (as I have done previously) so to have such a feature would be useful for anyone using networking in zephyr - applications, tests and samples

Copy link
Member

Choose a reason for hiding this comment

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

Please note that this is just an example file, no one in their sane mind would use static IP address in the production device.

Hehehe... I know some people who would do that.. oh sane, nm 😅

@jukkar jukkar force-pushed the devel/yaml-for-net branch from ab53c73 to 2d7cadc Compare January 26, 2024 09:25
@pdgendt
Copy link
Contributor

pdgendt commented Jan 26, 2024

@jukkar thanks for all the work put into this, I'll try to do an in-depth review later

On the flip side, I'm a bit concerned of how much effort it costs to write these custom parsers.
Looking at the amount of code needed for the network configuration, does this scale if we were to introduce similar concepts in other subsystems? It feels a bit like re-inventing the wheel.

I'll first add this disclaimer before the rest of the comment; as the maintainer of nanopb I'm a fan of using the protobuf IDL. Don't shoot me :)

Just an idea I had for solving this problem (we use this in our company for other application data already), what if we describe the network configuration as follows:

syntax = "proto3";

// some draft message types

message IPv4Config {
    bool dhcp = 1;
    uint32 address = 2; // or string
    uint32 gateway = 3;
    // ...
}

message NetworkInterface {
    string name = 1;
    repeated IPv4Config ipv4_config = 2;
    // ...
}

message NetworkConfig {
    repeated NetworkInterface interfaces = 1;
    // ...
}

The data can be "scripted" using existing python tooling, for example using json input, as converting between json and binary data is supported by most languages.
We could put this data into linker sections and decode whenever needed.

Pros:

  • Uses existing tooling
  • Language independent (python, c, rust, ...)
  • Concise binary data representation (imagine a scenario where a settings backend can be used to override defaults set at compile time)
  • Forward/backward compatible data structures

Cons:

  • Runtime decoding overhead
  • Users/developers need to "learn" protobuf (can probably be abstracted)
  • Probably more that I can't think of now :)

@jukkar
Copy link
Member Author

jukkar commented Jan 26, 2024

Pros:

* Uses existing tooling

This uses mostly existing tooling. Note that this PR does not really care to format the data is converted from yaml. For networking purposes it felt natural to convert it directly to c-structs, but it does not need to be that way.

* Language independent (python, c, rust, ...)

* Concise binary data representation (imagine a scenario where a settings backend can be used to override defaults set at compile time)

I pondered if we should create a provision db to zephyr in the original issue, but then decided that at least for time being it is best to leave to vendor to decide how the provisioning should be done. So did not go that route.

* [Forward/backward compatible](https://protobuf.dev/overview/#updating-defs) data structures

Cons:

* Decoding overhead

I was considering this, but it felt overkill to add a decoder to the build when not strictly necessary.

* Users/developers need to "learn" protobuf (can probably be abstracted)

* Probably more that I can't think of now :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for having those IPv4 configs given it's disabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a reason for having those IPv4 configs given it's disabled?

If you want to disable the IPv4/6 support without removing the configuration. Then it is easy to re-enable it later if needed without need to add the (now missing) options again to the file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it needed to generate config file in the sample's CMakeLists.txt, or could it be done in a more generic way in config's CMakeLists.txt (it's needed to enable the feature with Kconfig anyway).

Copy link
Member Author

Choose a reason for hiding this comment

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

IMHO the configuration yaml file should not be placed to subsys/net/lib/config directory, but near where it is needed. It could be in one directory too if we want to share these with tests/samples.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to place these settings to config lib CMakefilẹ file. Unfortunately the system cannot find the generated file in that case.

ninja: error: '..../zephyr/subsys/net/lib/config/configuration.yaml', needed by 'zephyr/include/generated/net_init_config.inc', missing and no known rule to make it

Copy link

Choose a reason for hiding this comment

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

I agree with @rlubos that we should investigate further how to apply the "convention over configuration" approach here.

Copy link
Contributor

Choose a reason for hiding this comment

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

A very nit, but for now we only support assigning addresses within own subnet only.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just an example file, the values in here are just "invented" somewhat semi randomly.

@jukkar jukkar force-pushed the devel/yaml-for-net branch 4 times, most recently from 683fded to 6a968ee Compare September 1, 2024 10:22
@jukkar jukkar requested review from a user, pdgendt and rlubos September 1, 2024 10:22
@jukkar
Copy link
Member Author

jukkar commented Sep 1, 2024

This is ready for a re-review, please take a look.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Hi Jukka,

first of all thank you so much that you have gone to such lengths to make your "YAML-to-C-struct" converter more generic. I think this is also proving that (as expected) something in this direction could be the first building block of a more generic configuration system.

I already reviewed your code changes in detail and built a personal opinion. But I think it would not be right do a public review, until we have a commonly agreed upon list of requirements from the arch WG against which all of us can review w/o fighting over basics in this PR.

It is obvious that some of the requirements that have already been formulated by me and others in #77638 are partially met, others are not met but could be achieved in later iterations, but there are also a few more fundamental architectural drawbacks still that IMO need to be (and can be fixed) so that we do not create a precedence that will be very hard to evolve into a more broadly usable config system later on.

Under these circumstances I'll leave my review in the "request changes" state until we have an arch WG decision on requirements and ideally also in the solution space (which is most probably not going to be very far from where you stand now AFAICS).

@jukkar jukkar force-pushed the devel/yaml-for-net branch from 6a968ee to ec270ee Compare September 5, 2024 09:42
@jukkar
Copy link
Member Author

jukkar commented Sep 5, 2024

  • CI fixes

@github-actions
Copy link

github-actions bot commented Nov 5, 2024

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Nov 5, 2024
@jukkar jukkar removed the Stale label Nov 5, 2024
jukkar added 10 commits November 6, 2024 15:31
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]>
Add a function that will parse a IP string that can contain
optional IPv4 netmask or IPv6 prefix length.

Examples of possible acceptable IP address strings:

192.0.2.1/24
192.0.2.98
2001:db8::1/64
2001:db8::2

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 net-init-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_INIT_CONFIG_FILE to override the name of the
default configuration file name which is net-init-config.yaml
This way a substitute yaml config file can be used instead of
the default one.

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]>
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 initialization using
a yaml file.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar jukkar force-pushed the devel/yaml-for-net branch from ec270ee to 1b0845b Compare November 6, 2024 13:37
@jukkar
Copy link
Member Author

jukkar commented Nov 6, 2024

  • Rebased to latest main and resolved merge conflicts.

@github-actions
Copy link

github-actions bot commented Jan 6, 2025

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.