On 11/9/2023 5:06 PM, David Howells wrote:
I do not believe the ack_reason matters within rxrpc_input_ack(). As long asthe acked_serial is non-zero, rxrpc_complete_rtt_probe() can be called to attempt to compute an RTT. If there is an exact match for the acked_serial then an RTT can be computed and if acked_serial is later than the pending rtt probe, the probe can be abandoned with the following caveats. 1. Receiving an acked_serial that is later than the serial of the transmitted probe indicates that a packet transmitted after the probe was received first. Or that reordering of the transmitted packets occurred. Or that the probe was never received by the peer; or that the peer's response to the probe was lost in transit. 2. The serial number namespace is unsigned 32-bit shared across all of the call channels of the associated rx connection. As the serial numbers will wrap the use of after() within rxrpc_complete_rtt_probe to compare their values is questionable. If serial numbers will be compared in this manner then they need to be locally tracked and compared as unsigned 64-bit values where only the low 32-bits are transmitted on the wire and any wire serial number equal to zero is ignored.I do ignore ack.serial == 0 for this purpose.
Zero has the special meaning - this ACK is not explicitly in response to a received packet.
However, as mentioned, the serial number counter wraps frequently and most RxRPC implementations
do not transition from serial 0xffffffff -> 0x00000001 when wrapping.
rxrpc_complete_rtt_probe contains the following logic that relies on after() being able to detectI'm not sure how expanding it internally to 64-bits actually helps since the upper 32 bits is not visible to the peer.
a serial number wrap.
/* If a later serial is being acked, then mark this slot as
* being available.
*/
if (after(acked_serial, orig_serial)) {
trace_rxrpc_rtt_rx(call, rxrpc_rtt_rx_obsolete, i,
orig_serial, acked_serial, 0, 0);
clear_bit(i + RXRPC_CALL_RTT_PEND_SHIFT, &call->rtt_avail);
smp_wmb();
set_bit(i, &call->rtt_avail);
}
Otherwise, acked_serial = 0x01 will be considered smaller than
orig_serial = 0xfffffffe and the slot will not be marked available.
I will note that there is a similar problem with rxrpc_seq_t values which are u32 on the wire but which will wrap for calls that transmit more than approximately 5.5TB of data. Calls of this size are unlikely for a cache manager but are common for any service transmitting volume dumps.
Jeffrey Altman
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
