Skip to content

Centralize net_mgmt layer code allocation #89083

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

Merged
merged 3 commits into from
Apr 29, 2025

Conversation

jukkar
Copy link
Member

@jukkar jukkar commented Apr 25, 2025

Place net_mgmt layer codes in one central place in zephyr/include/net/net_mgmt.h so that we can keep track of their allocation and usage in the code. Note that I specified the layer code as 7 bit value, although currently the existing code uses 4 more bits. We can steal those bits in the future for some other purposes.
This PR does not change functionality of the code in any way.

rlubos
rlubos previously approved these changes Apr 25, 2025
pdgendt
pdgendt previously approved these changes Apr 25, 2025
Copy link
Collaborator

@pdgendt pdgendt left a comment

Choose a reason for hiding this comment

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

Nice!

@jukkar jukkar dismissed stale reviews from pdgendt and rlubos via 0bed9ac April 25, 2025 10:52
@jukkar jukkar force-pushed the devel/net-mgmt-layer-codes branch from c1d9f22 to 0bed9ac Compare April 25, 2025 10:52
@jukkar
Copy link
Member Author

jukkar commented Apr 25, 2025

  • CI fixes

pdgendt
pdgendt previously approved these changes Apr 25, 2025
rlubos
rlubos previously approved these changes Apr 25, 2025
@@ -76,6 +76,29 @@ struct net_if;

/** @endcond */

/** Central place the definition of the layer codes (7 bit value) */
Copy link
Collaborator

@JordanYates JordanYates Apr 25, 2025

Choose a reason for hiding this comment

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

Should the codes be an enum so they are grouped nicely in the docs and the whole group has this header?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can certainly experiment with enum, the current doc does not look that great atm https://builds.zephyrproject.io/zephyr/pr/89083/docs/doxygen/html/group__net__mgmt.html
I will mark this as DNM and investigate this more.

@jukkar jukkar added the DNM This PR should not be merged (Do Not Merge) label Apr 27, 2025
jukkar added 3 commits April 28, 2025 10:40
Instead of littering the source tree with hard to track
layer code definitions, have a central place where to register
the layer codes.

Signed-off-by: Jukka Rissanen <[email protected]>
The _ is a reserved character in front of the symbols so remove
it from network management event macros. The remaining string
without the _ will identify the network API anyway so having
underscore there is not needed.

Signed-off-by: Jukka Rissanen <[email protected]>
Instead of hard coding some random value to layer code, use the
code that is registered in net_mgmt.h
This way it is easier to keep track of used layer codes in the
future, and we also allow out-of-tree components a way to avoid
allocating same layer code to in-tree-source code.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar jukkar dismissed stale reviews from rlubos and pdgendt via 2e47301 April 28, 2025 07:44
@jukkar jukkar force-pushed the devel/net-mgmt-layer-codes branch from 0bed9ac to 2e47301 Compare April 28, 2025 07:44
@jukkar
Copy link
Member Author

jukkar commented Apr 28, 2025

Replaced layer code definitions by enum values to make documentation look better.

@jukkar jukkar removed the DNM This PR should not be merged (Do Not Merge) label Apr 28, 2025
@kartben kartben merged commit 259d41f into zephyrproject-rtos:main Apr 29, 2025
28 of 29 checks passed
@jukkar jukkar deleted the devel/net-mgmt-layer-codes branch April 29, 2025 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants