Overhaul net_mgmt to use bit fields #89048
Conversation
|
What changed from your #88495 (comment) that 64 bits will eventually be insufficient? |
include/zephyr/net/net_mgmt.h
Outdated
|
|
||
| /** @endcond */ | ||
|
|
||
| /** Central place the definition of the layer codes (7 bit value) */ |
There was a problem hiding this comment.
Defining these layer codes as integers rather than individual bits essentially makes the global_event_mask optimisation in net_mgmt.c pointless. This is no different from the current behaviour, but worth looking at if we are redoing the definitions of events in order to get it right the first time?
There was a problem hiding this comment.
You mentioned earlier the problems with net_mgmt_event_wait() but it seems that I missed your point there as after looking the API it looks ok after all. Or were you just wanting to avoid checking enum values in the first place and just use bit values?
There was a problem hiding this comment.
I think the problems with net_mgmt_event_wait are solved by this PR.
This comment was about
zephyr/subsys/net/ip/net_mgmt.c
Lines 152 to 155 in 77c2c45
zephyr/subsys/net/ip/net_mgmt.c
Lines 172 to 183 in 77c2c45
This code is supposed to be doing some early filtering of events that no-one is subscribed to, but because the layer codes mostly all share bits with each other, basically no events are actually going to be filtered, its just extra computation for no benefit.
Nothing really, just wanted to experiment with this idea of having one bit / event thingy. I looked also using bitarray but that would be much bigger change. So just experimenting and path finding a solution for having one bit / event with minimal API changes. |
|
From the current set of changes here, the first three are more or less generic and we could probably apply them regardless of what is being done with the actual event values. |
rlubos
left a comment
rlubos
left a comment
There was a problem hiding this comment.
I just wonder if we should differentiate the actual event codes (integers defined in for example enum net_event_if_cmd) from the even representation in the 64-bit event, it seems it could cause some confustion.
Yeah, I thought about the same. It is just a bigger API change so did not go that route yet. |
|
I submitted the first three commits to #89083 as they are pretty generic and could be used in main already. |
jukkar
force-pushed
the
fix/88534/net-mgmt-event-handling
branch
from
aaa7421 to
d52258e
Compare
github-actions
bot
added
area: Networking
area: IEEE 802.15.4
area: Wi-Fi
github-actions
bot
requested review from
MaochenWang1,
danieldegrasse,
dkalowsk,
glarsennordic,
kartben,
krish2718 and
ssharks
|
I marked this as a normal PR so that we can have a verdict whether this is a desired change. Please take a look and comment. |
jukkar
force-pushed
the
fix/88534/net-mgmt-event-handling
branch
from
d52258e to
837f31b
Compare
github-actions
bot
added
area: Samples
|
update manifest and remove DNM once #91494 merges due to NXP HAL synchronizing order |
Instead of using 32 bit enum values for event numbers, convert the code to use 64 bit long bit fields. This means that the user API is changed to use 64 bit event values instead of 32 bit event values. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
We cannot use the network management event number directly as a socket option value because the management value is uint64_t and that cannot be mapped directly to 32 bit integer. So have an intermediate socket option that is mapped to actual network management request number in getsockopt() and setsockopt(). Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Add information how the net_mgmt request handler is changed. The event number type is changed from uint32_t to uint64_t to allow the event command to be a bit mask instead of enum value. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
As per API overview documentation, a braking API change must increment major version number. https://docs.zephyrproject.org/latest/develop/api/overview.html#api-overview Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
jukkar
force-pushed
the
fix/88534/net-mgmt-event-handling
branch
from
a562ab5 to
b961cd9
Compare
github-actions
bot
removed
manifest
manifest-hal_nxp
DNM (manifest)
jukkar
removed
the
Architecture Review
|
8 participants
The current net_mgmt API uses 32 bit enum values for network event codes. This means that user needs to check what event is being triggered because the individual bits do not tell that. This has led to great confusion over the years and is being discussed more in #88534
This PR proposes that each network event is mapped to one specific bit in the layer. To make sure that there are enough bits available, the net_mgmt API is changed to use 64 bit event number values.