-
-
Notifications
You must be signed in to change notification settings - Fork 125
Introduce a service to wait for network uplink #603
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
Conversation
Better add "network" somewhere in the service name. |
bf95730
to
8b1948f
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #603 +/- ##
==========================================
- Coverage 71.10% 70.68% -0.42%
==========================================
Files 3 3
Lines 481 481
==========================================
- Hits 342 340 -2
- Misses 139 141 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025102219-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025081011-4.3&flavor=update
Failed tests12 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/149225#dependencies 84 fixed
Unstable testsPerformance TestsPerformance degradation:17 performance degradations
Remaining performance tests:163 tests
|
8b1948f
to
1bc691a
Compare
This fails reliably on Debian 13. I don't see in logs on which side the problem is, might be related to calling as non-root, but might be also something else. |
Ah, you found that in the other PR already. |
No failure happens if I use
So it means it is not waiting for something that would signal that network has been established. |
1bc691a
to
07c8f9f
Compare
Waiting for ping gateway to return seems enough https://github.com/QubesOS/qubes-core-agent-linux/compare/1bc691a9323151aa43793e0c5dc24de7c0a0a60c..07c8f9fe5eafcbe45587abd16656b932f4fcc45e
|
Except it doesn't always works:
|
When it fails, it only fails on ipv4, which is tested before ipv6. Ipv6 may not be failing because it takes more time as it tries to ping the gateway of both ipv4 and ipv6 if both are available. |
07c8f9f
to
b4219a1
Compare
Waiting for |
Ping is a bad idea, the backend may not necessarily respond to them (I wouldn't be surprised if Whonix blocks them at firewall, or Mirage firewall wouldn't respond either). Maybe add some debug info collection (see what network tests do on failure) before/after wait and see what is actually changing? Could be also some delay on the backend side maybe? If all fails, IMO better to accept that network may start working with some delay (and deal with that in tests), than having forced delay in disposable usage. After all, internet protocols need to deal with unreliable packet transfers anyway. |
Right...
The ping errors are:
Could be some delay on the backend/netvm side, yes. It tends to happen user@dom0:~/qubes-core-admin$ qvm-prefs untrusted netvm ''
user@dom0:~/qubes-core-admin$ qvm-prefs untrusted netvm -D; time qvm-run -p untrusted ping -c1 -W1 -n qubes-os.org
PING qubes-os.org (104.21.48.1) 56(84) bytes of data.
64 bytes from 104.21.48.1: icmp_seq=1 ttl=54 time=13.4 ms
--- qubes-os.org ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 13.401/13.401/13.401/0.000 ms
real 0m0.314s
user 0m0.110s
sys 0m0.068s
Sure. I will probably move the qvm-prefs untrusted netvm sys-firewall-alt
qvm-pause untrusted
qvm-prefs untrusted netvm -D # sys-firewall
qvm-shutdown --wait sys-firewall-alt
qvm-run -p ping -c1 -W1 -n qubes-os.org Ping works, no packet loss, but a bit worrying:
I also get this from journal in tests, see the journal of the client:
|
b4219a1
to
78e8b6c
Compare
Oh, sorry, you've got experimental kernel. Switch to stable one at least in VMs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this might not be necessary after fixing the kernel issue mentioned in the other PR, but since it isn't fixed yet, lets have it in
Fixes: QubesOS/qubes-issues#10173
For: QubesOS/qubes-issues#1512