aboutgitcodebugslistschat
Commit message (Collapse)AuthorAgeFilesLines
* tcp: Don't reset ACK_TO_TAP_DUE on any ACK, reschedule timer as needed2023_03_21.1ee2f7cStefano Brivio2023-03-211-4/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is mostly symmetric with commit cc6d8286d104 ("tcp: Reset ACK_FROM_TAP_DUE flag only as needed, update timer"): we shouldn't reset the ACK_TO_TAP_DUE flag on any inbound ACK segment, but only once we acknowledge everything we received from the guest or the container. If we don't, a client might unnecessarily hold off further data, especially during slow start, and in general we won't converge to the usable bandwidth. This is very visible especially with traffic tests on links with non-negligible latency, such as in the reported issue. There, a public iperf3 server sometimes aborts the test due do what appears to be a low iperf3's --rcv-timeout (probably less than a second). Even if this doesn't happen, the throughput will converge to a fraction of the usable bandwidth. Clear ACK_TO_TAP_DUE if we acknowledged everything, set it if we didn't, and reschedule the timer in case the flag is still set as the timer expires. While at it, decrease the ACK timer interval to 10ms. A 50ms interval is short enough for any bandwidth-delay product I had in mind (local connections, or non-local connections with limited bandwidth), but here I am, testing 1gbps transfers to a peer with 100ms RTT. Indeed, we could eventually make the timer interval dependent on the current window and estimated bandwidth-delay product, but at least for the moment being, 10ms should be long enough to avoid any measurable syscall overhead, yet usable for any real-world application. Reported-by: Lukas Mrtvy <lukas.mrtvy@gmail.com> Link: https://bugs.passt.top/show_bug.cgi?id=44 Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: When a connection flag it set, don't negate it for debug printStefano Brivio2023-03-211-1/+1
| | | | | | | | | Fix a copy and paste typo I added in commit 5474bc5485d8 ("tcp, tcp_splice: Get rid of false positive CWE-394 Coverity warning from fls()") and --debug altogether. Fixes: 5474bc5485d8 ("tcp, tcp_splice: Get rid of false positive CWE-394 Coverity warning from fls()") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Fix false positive if cppcheck doesn't give a false positiveDavid Gibson2023-03-211-1/+1
| | | | | | | | | | | | | | | da46fdac "tcp: Suppress knownConditionTrueFalse cppcheck false positive" introduced a suppression to work around a cppcheck bug causing a false positive warning. However, the suppression will itself cause a spurious unmatchedSuppression warning if used with a version of cppcheck from before the bug was introduced. That includes the packaged version of cppcheck in Fedora. Suppress the unmatchedSuppression as well. Fixes: da46fdac3605 ("tcp: Suppress knownConditionTrueFalse cppcheck false positive") Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Work around weird false positives with cppcheck-2.9.1David Gibson2023-03-216-6/+6
| | | | | | | | | | | | | | | | | | Commit 89e38f55 "treewide: Fix header includes to build with musl" added extra #includes to work with musl. Unfortunately with the cppcheck version I'm using (cppcheck-2.9-1.fc37.x86_64 in Fedora 37) this causes weird false positives: specifically cppcheck seems to hit a #error in <bits/unistd.h> complaining about including it directly instead of via <unistd.h> (which is not something we're doing). I have no idea why that would be happening; but I'm guessing it has to be a bug in the cpp implementation in that cppcheck version. In any case, it's possible to work around this by moving the include of <unistd.h> before the include of <signal.h>. So, do that. Fixes: 89e38f55405d ("treewide: Fix header includes to build with musl") Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Actually bind detected namespace ports in init namespaceStefano Brivio2023-03-211-1/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When I reworked udp_init() to move most of the port binding logic to conf_ports, I accidentally dropped this bit of automatic port detection (and binding) at start-up. On -U auto, in pasta mode, udp_sock_init_ns() binds ports in the namespace that correspond to ports bound in the init namespace, but on -u auto, nothing actually happens after port detection. Add udp_sock_init_init() to deal with this, and while at it fix the comment to udp_sock_init_ns(): the latter takes care of outbound "connections". This is currently not covered by tests, and the UDP port needs to be already bound in the namespace when pasta starts (periodic detection for UDP is a missing feature at the moment). It can be checked like this: $ unshare -rUn # echo $$ 590092 # socat -u UDP-LISTEN:5555 STDOUT $ pasta -q -u auto 590092 $ echo "test" | socat -u STDIN UDP:localhost:5555 Reported-by: Paul Holzinger <pholzing@redhat.com> Fixes: 3c6ae625101a ("conf, tcp, udp: Allow address specification for forwarded ports") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* pasta: fix tcp port forwarding in auto modePaul Holzinger2023-03-211-5/+5
The logic in tcp_timer() was inverted. fwd_out should expose the host ports in the ns. Therfore it must read the ports on the host and then bind them in the netns. The same for fwd_in which checks ports in the ns and then exposes them on the host. Note that this only fixes tcp ports, udp does not seems to work at all right now with the auto mode. Signed-off-by: Paul Holzinger <pholzing@redhat.com> Fixes: 1128fa03fe73 ("Improve types and names for port forwarding configuration") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>