net_ifaces: general fixes and improvements#454
Open
andreagilardoni wants to merge 3 commits into
Open
Conversation
removing event handler, since they provide only logging information that is always discarded
- it may happen that net_if_up is never called on iface inside of the begin function, forcing it if the netif is not up - it may happen that the dhcpv4 address is already bound before the wait for the event is started, avoid that as it may be reported as an error - removed wait forever in dhcp wait event listener in favor of 10s which seems enough in order to report the error to the user and let him start the retrial procedure
andreagilardoni
force-pushed
the
net_iface_improvements
branch
from
75c14d7 to
1d931e6
Compare
switch to dhcp_restart, when a dhcp lease is already in place, thus forcing zephyr to ask dhcp server for a renewal of the lease
andreagilardoni
force-pushed
the
net_iface_improvements
branch
from
1d931e6 to
b7b1491
Compare
Built
|
| Artifact | Board | Core | Tests | RAM | Sketches | Warnings | Errors |
|---|---|---|---|---|---|---|---|
✅ zephyr_contrib |
ek_ra8d1
| 📗 | ✅ |
11.9% |
2 | - | - |
frdm_mcxn947
| 6 🏷️ | ✅ |
58.0% |
2 | - | - | |
frdm_rw612
| 2 🏷️ | ✅ |
83.0% |
2 | - | - | |
✔️* zephyr_main |
giga
| 4 🏷️ | ✅* |
54.6% |
56 | 20 | - |
nano33ble
| 1 🏷️ | ✅* |
78.8% |
28 | 10 | - | |
nano_matter
| 📗 | ✔️* |
|
20 | 8 | (2*) | |
nicla_vision
| 4 🏷️ | ✔️* |
56.9% |
50 | 12 | (10*) | |
niclasense
| 2 🏷️ | ✅* |
|
26 | 10 | - | |
opta
| 4 🏷️ | ✅* |
56.5% |
60 | 34 | - | |
portentac33
| 3 🏷️ | ✔️* |
|
62 | 26 | (8*) | |
portentah7
| 3 🏷️ | ✅* |
57.2% |
70 | 34 | - | |
✅* zephyr_unoq |
unoq
| 📗 | ✅* |
26.9% |
74 | 8 | - |
Legend
Board Test Status description 🔥 🔥 Test run failed to complete. ❌ 🔴 Test completed with unexpected errors. ✔️* 🚫 Test completed with errors, but all are known/expected. ✅* 🟡 Test completed with some warnings; no errors detected. ✅ 🟢 Test passed successfully, with no warnings or errors. 🌑 🌑 Test was skipped.
andreagilardoni
force-pushed
the
net_iface_improvements
branch
2 times, most recently
from
4164d93 to
9e28eba
Compare
andreagilardoni
changed the title
Built
|
| Artifact | Board | Core | Tests | RAM | Sketches | Warnings | Errors |
|---|---|---|---|---|---|---|---|
✅ zephyr_contrib |
ek_ra8d1
| 📗 | ✅ |
11.9% |
2 | - | - |
frdm_mcxn947
| 7 🏷️ | ✅ |
58.0% |
2 | - | - | |
frdm_rw612
| 2 🏷️ | ✅ |
83.0% |
2 | - | - | |
✔️* zephyr_main |
giga
| 5 🏷️ | ✅* |
54.7% |
56 | 20 | - |
nano33ble
| 1 🏷️ | ✅* |
78.8% |
28 | 10 | - | |
nano_matter
| 1 🏷️ | ✔️* |
|
20 | 8 | (2*) | |
nicla_vision
| 5 🏷️ | ✔️* |
57.0% |
50 | 12 | (10*) | |
niclasense
| 2 🏷️ | ✅* |
|
26 | 10 | - | |
opta
| 5 🏷️ | ✅* |
56.5% |
60 | 34 | - | |
portentac33
| 3 🏷️ | ✔️* |
|
62 | 26 | (8*) | |
portentah7
| 4 🏷️ | ✅* |
57.2% |
70 | 34 | - | |
✅* zephyr_unoq |
unoq
| 📗 | ✅* |
26.9% |
74 | 8 | - |
Legend
Board Test Status description 🔥 🔥 Test run failed to complete. ❌ 🔴 Test completed with unexpected errors. ✔️* 🚫 Test completed with errors, but all are known/expected. ✅* 🟡 Test completed with some warnings; no errors detected. ✅ 🟢 Test passed successfully, with no warnings or errors. 🌑 🌑 Test was skipped.
andreagilardoni
force-pushed
the
net_iface_improvements
branch
from
54a8481 to
b7b1491
Compare
Built
|
| Artifact | Board | Core | Tests | RAM | Sketches | Warnings | Errors |
|---|---|---|---|---|---|---|---|
✅ zephyr_contrib |
ek_ra8d1
| 📗 | ✅ |
11.9% |
2 | - | - |
frdm_mcxn947
| 6 🏷️ | ✅ |
58.0% |
2 | - | - | |
frdm_rw612
| 2 🏷️ | ✅ |
83.0% |
2 | - | - | |
✔️* zephyr_main |
giga
| 4 🏷️ | ✅* |
54.6% |
56 | 20 | - |
nano33ble
| 1 🏷️ | ✅* |
78.8% |
28 | 10 | - | |
nano_matter
| 📗 | ✔️* |
|
20 | 8 | (2*) | |
nicla_vision
| 4 🏷️ | ✔️* |
56.9% |
50 | 12 | (10*) | |
niclasense
| 2 🏷️ | ✅* |
|
26 | 10 | - | |
opta
| 4 🏷️ | ✅* |
56.5% |
60 | 34 | - | |
portentac33
| 3 🏷️ | ✔️* |
|
62 | 26 | (8*) | |
portentah7
| 3 🏷️ | ✅* |
57.2% |
70 | 34 | - | |
✅* zephyr_unoq |
unoq
| 📗 | ✅* |
26.9% |
74 | 8 | - |
Legend
Board Test Status description 🔥 🔥 Test run failed to complete. ❌ 🔴 Test completed with unexpected errors. ✔️* 🚫 Test completed with errors, but all are known/expected. ✅* 🟡 Test completed with some warnings; no errors detected. ✅ 🟢 Test passed successfully, with no warnings or errors. 🌑 🌑 Test was skipped.
|
Memory usage change @ b7b1491
Click for full report table
Click for full report CSV |
8 tasks
dani-clo
reviewed
May 27, 2026
| net_dhcpv4_start(netif); | ||
| // If dhcp was never started on this interface, start it, otherwise restart it, so that we force | ||
| // to ask the dhcp server for a new lease | ||
| if (netif->config.dhcpv4.state == NET_DHCPV4_DISABLED) { |
There was a problem hiding this comment.
Imho this condition is too strong: every non-DISABLED state triggers net_dhcpv4_restart(), including transient states (INIT/SELECTING/REQUESTING).
If begin() is called repeatedly while DHCP is already negotiating in slow network, we may keep resetting the state machine.
We could restart only from NET_DHCPV4_BOUND (or other “stable” states), and just start from NET_DHCPV4_DISABLED, while for in-progress states, it seems safer to leave DHCP running and avoid unnecessary restarts.
Author
There was a problem hiding this comment.
I got your point, I am unsure on: leaving the user the responsibility to handle this situation, because it may be desired to work this way or handle transient states. In most cases this won't be a problem because the user must specifically call a non blocking begin.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
begin function, forcing it if the netif is not up
for the event is started, avoid that as it may be reported as an error
seems enough in order to report the error to the user and let him
start the retrial procedure