[PATCH 0/4] Corrections to seeting of ACK flag in TCP packets
A recently reported podman bug shows transfer failures in podman custom rootless networks connected with pasta. Analysis suggests this is triggered by pasta generating a TCP packet without the ACK flag when it should have one. The exact symptoms seem to arise because of some odd kernel behaviour - rather than simply ignoring the packet, an RST is observed killing the connection. However, there are also packets seen after the RST which don't seem to make sense. While there are some mysteries which we still hope to track down here, in the meantime it definitely seems like pasta's ACK behaviour isn't correct, and appears to trigger the other problems. So, fix it. Link: https://github.com/containers/podman/issues/22146 Link: https://bugs.passt.top/show_bug.cgi?id=84 David Gibson (4): tcp: Split handling of DUP_ACK from ACK tcp: Rearrange logic for setting ACK flag in tcp_send_flag() tcp: Never automatically add the ACK flag to RST packets tcp: Unconditionally force ACK for all !SYN, !RST packets tcp.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) -- 2.44.0
The DUP_ACK flag to tcp_send_flag() has two effects: first it forces the
setting of the ACK flag in the packet, even if we otherwise wouldn't.
Secondly, it causes a duplicate of the flags packet to be sent immediately
after the first.
Setting the ACK flag to tcp_send_flag() also has the first effect, so
instead of having DUP_ACK also do that, pass both flags when we need both
operations. This slightly simplifies the logic of tcp_send_flag() in a way
that makes some future changes easier.
Signed-off-by: David Gibson
We have different paths for controlling the ACK flag for the SYN and !SYN
paths. This amounts to sometimes forcing on the ACK flag in the !SYN path
regardless of options. We can rearrange things to explicitly be that which
will make things neater for some future changes.
Signed-off-by: David Gibson
tcp_send_flag() will sometimes force on the ACK flag for all !SYN packets.
This doesn't make sense for RST packets, where plain RST and RST+ACK have
somewhat different meanings. AIUI, RST+ACK indicates an abrupt end to
a connection, but acknowledges data already sent. Plain RST indicates an
abort, when one end receives a packet that doesn't seem to make sense in
the context of what it knows about the connection. All of the cases where
we send RSTs are the second, so we don't want an ACK flag, but we currently
could add one anyway.
Change that, so we won't add an ACK to an RST unless the caller explicitly
requests it.
Signed-off-by: David Gibson
Currently we set ACK on flags packets only when the acknowledged byte
pointer has advanced, or we hadn't previously set a window. This means
in particular that we can send a window update with no ACK flag, which
doesn't appear to be correct. RFC 9293 requires a receiver to ignore such
a packet [0], and indeed it appears that every non-SYN, non-RST packet
should have the ACK flag.
The reason for the existing logic, rather than always forcing an ACK seems
to be to avoid having the packet mistaken as a duplicate ACK which might
trigger a fast retransmit. However, earlier tests in the function mean we
won't reach here if we don't have either an advance in the ack pointer -
which will already set the ACK flag, or a window update - which shouldn't
trigger a fast retransmit.
[0] https://www.ietf.org/rfc/rfc9293.html#section-3.10.7.4-2.5.2.1
Link: https://github.com/containers/podman/issues/22146
Link: https://bugs.passt.top/show_bug.cgi?id=84
Signed-off-by: David Gibson
On Tue, 26 Mar 2024 16:42:20 +1100
David Gibson
A recently reported podman bug shows transfer failures in podman custom rootless networks connected with pasta. Analysis suggests this is triggered by pasta generating a TCP packet without the ACK flag when it should have one.
The exact symptoms seem to arise because of some odd kernel behaviour - rather than simply ignoring the packet, an RST is observed killing the connection. However, there are also packets seen after the RST which don't seem to make sense.
While there are some mysteries which we still hope to track down here, in the meantime it definitely seems like pasta's ACK behaviour isn't correct, and appears to trigger the other problems. So, fix it.
Link: https://github.com/containers/podman/issues/22146 Link: https://bugs.passt.top/show_bug.cgi?id=84
David Gibson (4): tcp: Split handling of DUP_ACK from ACK tcp: Rearrange logic for setting ACK flag in tcp_send_flag() tcp: Never automatically add the ACK flag to RST packets tcp: Unconditionally force ACK for all !SYN, !RST packets
Applied. -- Stefano
participants (2)
-
David Gibson
-
Stefano Brivio