mirrors/passt
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

tcp: Clear ACK_FROM_TAP_DUE also on unchanged ACK sequence from peer

Browse source 
Since commit cc6d8286d1 ("tcp: Reset ACK_FROM_TAP_DUE flag only as
needed, update timer"), we don't clear ACK_FROM_TAP_DUE whenever we
process an ACK segment, but, more correctly, only if we're really not
waiting for a further ACK segment, that is, only if the acknowledged
sequence matches what we sent.

In the new function implementing this, tcp_update_seqack_from_tap(),
we also reset the retransmission counter and store the updated ACK
sequence. Both should be done iff forward progress is acknowledged,
implied by the fact that the new ACK sequence is greater than the
one we previously stored.

At that point, it looked natural to also include the statements that
clear and set the ACK_FROM_TAP_DUE flag inside the same conditional
block: if we're not making forward progress, the need for an ACK, or
lack thereof, should remain unchanged.

There might be cases where this isn't true, though: without the
previous commit 4e73e9bd65 ("tcp: Don't special case the handling
of the ack of a syn"), this would happen if a tap-side client
initiated a connection, and the server didn't send any data.

At that point we would never, in the established state of the
connection, call tcp_update_seqack_from_tap() with reported forward
progress.

That issue itself is fixed by the previous commit, now, but clearing
ACK_FROM_TAP_DUE only on ACK sequence progress doesn't really follow
any logic.

Clear the ACK_FROM_TAP_DUE flag regardless of reported forward
progress. If we clear it when it's already unset, conn_flag() will do
nothing with it.

This doesn't fix any known functional issue, rather a conceptual one.

Fixes: cc6d8286d1 ("tcp: Reset ACK_FROM_TAP_DUE flag only as needed, update timer")
Reported-by: David Gibson <david@gibson.dropbear.id.au>
Analysed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
This commit is contained in:
Stefano Brivio 2023-03-23 16:07:57 +01:00
parent 4e73e9bd65
commit 33d88f79d9
1 changed files with 5 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
tcp.c
View file

@ -1610,10 +1610,12 @@ out:
static void tcp_update_seqack_from_tap(const struct ctx *c, static void tcp_update_seqack_from_tap(const struct ctx *c,
struct tcp_tap_conn *conn, uint32_t seq) struct tcp_tap_conn *conn, uint32_t seq)
{ {
if (seq == conn->seq_to_tap)
conn_flag(c, conn, ~ACK_FROM_TAP_DUE);
if (SEQ_GT(seq, conn->seq_ack_from_tap)) { if (SEQ_GT(seq, conn->seq_ack_from_tap)) {
if (seq == conn->seq_to_tap) /* Forward progress, but more data to acknowledge: reschedule */
conn_flag(c, conn, ~ACK_FROM_TAP_DUE); if (SEQ_LT(seq, conn->seq_to_tap))
else
conn_flag(c, conn, ACK_FROM_TAP_DUE); conn_flag(c, conn, ACK_FROM_TAP_DUE);
conn->retrans = 0; conn->retrans = 0;