Bug #436
opentp_meter: Potential inverted logic check for fast recovery
0%
Description
According to Sashiko, Fast Retransmit is bypassed if the following evaluates to true:
if (!batadv_seq_before(recv_ack, tp_vars->cc.recover))
return BATADV_TP_ACK_REACTION_IGNORE;
Or previously:
if (recv_ack >= tp_vars->recover)
goto out;
Since recover is initialized to BATADV_TP_FIRST_SEQ, recv_ack >= recover is always true for any new loss. Does this break Fast Recovery for all packet drops and force a fallback to RTO timeouts?
(I personally haven't checked the logic for FastRetransmit. It was just a new remark from Sashiko for which I don't have the time. But I can at least see that it will only be changed when recv_ack >= tp_vars->recover would be false - which never [without an overflow] seems to be the case)
See https://sashiko.dev/#/patchset/20260601-tp-reason-missing-v5-0-78a5b8fe6e67%40narfation.org?part=2
The code in question is following part in the RFC6582:
When the third duplicate ACK is received, the TCP sender first
checks the value of recover to see if the Cumulative
Acknowledgment field covers more than recover.
Updated by Sven Eckelmann 17 days ago
@Antonio Quartulli the question would now be: Shouldn't actually be:
if (recv_ack <= tp_vars->recover)
goto out;
See also in the same RFC:
Step 2 above specifies a check that the Cumulative Acknowledgment
field covers more than recover. Because the acknowledgment field
contains the sequence number that the sender next expects to receive,
the acknowledgment "ack_number" covers more than recover whenack_number - 1 > recover;
i.e., at least one byte more of data is acknowledged beyond the
highest byte that was outstanding when fast retransmit was last
entered.
Updated by Sven Eckelmann 14 days ago
- Status changed from New to Resolved
- Assignee changed from Antonio Quartulli to Sven Eckelmann
- Target version set to 2026.2