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

net: conn_mgr: Add IPv4 and IPv6 tracking #72393

Merged
merged 5 commits into from
Jun 10, 2024

Conversation

glarsennordic
Copy link
Contributor

No description provided.

@glarsennordic
Copy link
Contributor Author

I haven't added unit tests for the new feature, nor have I updated old unit tests. I am going to put that off until after I transition my work laptop to linux, since it will be much easier to do local testing in that environment

@glarsennordic glarsennordic force-pushed the zcmv46 branch 2 times, most recently from 45c0833 to fa2e3b9 Compare May 22, 2024 20:04
@glarsennordic glarsennordic marked this pull request as ready for review May 22, 2024 20:07
@glarsennordic
Copy link
Contributor Author

@simensrostad I cannot add you as reviewer, but please feel free to review

Copy link
Contributor

@simensrostad simensrostad left a comment

Choose a reason for hiding this comment

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

Looked over new events. looks good

Copy link
Contributor

@rlubos rlubos left a comment

Choose a reason for hiding this comment

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

Looks good overall, just one concern to discuss.

include/zephyr/net/conn_mgr_connectivity.h Outdated Show resolved Hide resolved
rlubos
rlubos previously approved these changes Jun 6, 2024
Copy link
Member

@jukkar jukkar left a comment

Choose a reason for hiding this comment

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

LGTM, small nit about the commit message which has old information.

doc/connectivity/networking/conn_mgr/main.rst Show resolved Hide resolved
jukkar
jukkar previously approved these changes Jun 7, 2024
rlubos
rlubos previously approved these changes Jun 7, 2024
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

some minor doc issues and an actual comment typo

Comment on lines +80 to +83
- :c:macro:`NET_EVENT_L4_IPV4_CONNECTED`
- :c:macro:`NET_EVENT_L4_IPV4_DISCONNECTED`
- :c:macro:`NET_EVENT_L4_IPV6_CONNECTED`
- :c:macro:`NET_EVENT_L4_IPV6_DISCONNECTED`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- :c:macro:`NET_EVENT_L4_IPV4_CONNECTED`
- :c:macro:`NET_EVENT_L4_IPV4_DISCONNECTED`
- :c:macro:`NET_EVENT_L4_IPV6_CONNECTED`
- :c:macro:`NET_EVENT_L4_IPV6_DISCONNECTED`
- :c:enumerator:`NET_EVENT_L4_IPV4_CONNECTED`
- :c:enumerator:`NET_EVENT_L4_IPV4_DISCONNECTED`
- :c:enumerator:`NET_EVENT_L4_IPV6_CONNECTED`
- :c:enumerator:`NET_EVENT_L4_IPV6_DISCONNECTED`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you certain these should be changed to :c:enumerator:? They are defined as macros whose values derive from the actual enumerator values

Copy link
Collaborator

Choose a reason for hiding this comment

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

Of course! Read too fast and missed the difference between the "CMD" enumerators ans the macros

doc/connectivity/networking/conn_mgr/main.rst Show resolved Hide resolved
include/zephyr/net/net_event.h Outdated Show resolved Hide resolved
include/zephyr/net/net_event.h Outdated Show resolved Hide resolved
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

some minor doc issues and an actual comment typo

Instead of incrementing and decrementing global counter,
just recompute the ready-count from scratch every time
conn_mgr_mon_handle_update is called.

This will simplify the introduction of additional ready count types.

This should have no externally observable impact on the behavior of
conn_mgr.

Signed-off-by: Georges Oates_Larsen <[email protected]>
Don't track up/down blame separately.
Instead, track a single last-blame iface.

Don't track blame iface inside set_ready.
Instead, track directly inside handle_update.

These two changes will simplify the addition
of blame for IPv4- and IPv6-specific events.

Signed-off-by: Georges Oates_Larsen <[email protected]>
conn_mgr now fires:

- NET_EVENT_L4_IPV4_CONNECTED
- NET_EVENT_L4_IPV4_DISCONNECTED
- NET_EVENT_L4_IPV6_CONNECTED
- NET_EVENT_L4_IPV6_DISCONNECTED

These events track whether there are any ready ifaces offering
specifically IPv4 or specifically IPv6 connectivity.

Signed-off-by: Georges Oates_Larsen <[email protected]>
Modify the test cycle routines to verify recently added IPv4- and
IPv6-specific events.

Signed-off-by: Georges Oates_Larsen <[email protected]>
Add descriptions for recently introduced IPv4 and IPv6
connectivity events to the net event monitor.

Signed-off-by: Georges Oates_Larsen <[email protected]>
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

Doc ok, thx!

@carlescufi carlescufi merged commit 7fa6b53 into zephyrproject-rtos:main Jun 10, 2024
23 checks passed
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.

7 participants