Here are a couple of patches to fix ACK handling in AF_RXRPC:
(1) Allow RTT determination to use an ACK of any type as the response from
which to calculate RTT, provided ack.serial matches the serial number
of the outgoing packet.
(2) Defer the response to a PING ACK packet (or any ACK with the
REQUEST_ACK flag set) until after we've parsed the packet so that we
carry up to date information if the Tx or Rx rings are advanced.
David
---
The patches can be found here also:
http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-fixes
David Howells (2):
rxrpc: Fix RTT determination to use any ACK as a source
rxrpc: Defer the response to a PING ACK until we've parsed it
include/trace/events/rxrpc.h | 2 +-
net/rxrpc/input.c | 61 +++++++++++++++++-------------------
2 files changed, 30 insertions(+), 33 deletions(-)
Fix RTT determination to be able to use any type of ACK as the response
from which RTT can be calculated provided its ack.serial is non-zero and
matches the serial number of an outgoing DATA or ACK packet. This
shouldn't be limited to REQUESTED-type ACKs as these can have other types
substituted for them for things like duplicate or out-of-order packets.
Fixes: 4700c4d80b7b ("rxrpc: Fix loss of RTT samples due to interposed ACK")
Signed-off-by: David Howells <[email protected]>
cc: Marc Dionne <[email protected]>
cc: "David S. Miller" <[email protected]>
cc: Eric Dumazet <[email protected]>
cc: Jakub Kicinski <[email protected]>
cc: Paolo Abeni <[email protected]>
cc: [email protected]
cc: [email protected]
---
include/trace/events/rxrpc.h | 2 +-
net/rxrpc/input.c | 35 ++++++++++++++++-------------------
2 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 4c53a5ef6257..f7e537f64db4 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -328,7 +328,7 @@
E_(rxrpc_rtt_tx_ping, "PING")
#define rxrpc_rtt_rx_traces \
- EM(rxrpc_rtt_rx_cancel, "CNCL") \
+ EM(rxrpc_rtt_rx_other_ack, "OACK") \
EM(rxrpc_rtt_rx_obsolete, "OBSL") \
EM(rxrpc_rtt_rx_lost, "LOST") \
EM(rxrpc_rtt_rx_ping_response, "PONG") \
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 030d64f282f3..3f9594d12519 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -643,12 +643,8 @@ static void rxrpc_complete_rtt_probe(struct rxrpc_call *call,
clear_bit(i + RXRPC_CALL_RTT_PEND_SHIFT, &call->rtt_avail);
smp_mb(); /* Read data before setting avail bit */
set_bit(i, &call->rtt_avail);
- if (type != rxrpc_rtt_rx_cancel)
- rxrpc_peer_add_rtt(call, type, i, acked_serial, ack_serial,
- sent_at, resp_time);
- else
- trace_rxrpc_rtt_rx(call, rxrpc_rtt_rx_cancel, i,
- orig_serial, acked_serial, 0, 0);
+ rxrpc_peer_add_rtt(call, type, i, acked_serial, ack_serial,
+ sent_at, resp_time);
matched = true;
}
@@ -801,20 +797,21 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb)
summary.ack_reason, nr_acks);
rxrpc_inc_stat(call->rxnet, stat_rx_acks[ack.reason]);
- switch (ack.reason) {
- case RXRPC_ACK_PING_RESPONSE:
- rxrpc_complete_rtt_probe(call, skb->tstamp, acked_serial, ack_serial,
- rxrpc_rtt_rx_ping_response);
- break;
- case RXRPC_ACK_REQUESTED:
- rxrpc_complete_rtt_probe(call, skb->tstamp, acked_serial, ack_serial,
- rxrpc_rtt_rx_requested_ack);
- break;
- default:
- if (acked_serial != 0)
+ if (acked_serial != 0) {
+ switch (ack.reason) {
+ case RXRPC_ACK_PING_RESPONSE:
rxrpc_complete_rtt_probe(call, skb->tstamp, acked_serial, ack_serial,
- rxrpc_rtt_rx_cancel);
- break;
+ rxrpc_rtt_rx_ping_response);
+ break;
+ case RXRPC_ACK_REQUESTED:
+ rxrpc_complete_rtt_probe(call, skb->tstamp, acked_serial, ack_serial,
+ rxrpc_rtt_rx_requested_ack);
+ break;
+ default:
+ rxrpc_complete_rtt_probe(call, skb->tstamp, acked_serial, ack_serial,
+ rxrpc_rtt_rx_other_ack);
+ break;
+ }
}
if (ack.reason == RXRPC_ACK_PING) {
Hello:
This series was applied to netdev/net.git (main)
by David S. Miller <[email protected]>:
On Thu, 16 Nov 2023 13:12:57 +0000 you wrote:
> Here are a couple of patches to fix ACK handling in AF_RXRPC:
>
> (1) Allow RTT determination to use an ACK of any type as the response from
> which to calculate RTT, provided ack.serial matches the serial number
> of the outgoing packet.
>
> (2) Defer the response to a PING ACK packet (or any ACK with the
> REQUEST_ACK flag set) until after we've parsed the packet so that we
> carry up to date information if the Tx or Rx rings are advanced.
>
> [...]
Here is the summary with links:
- [net,1/2] rxrpc: Fix RTT determination to use any ACK as a source
https://git.kernel.org/netdev/net/c/3798680f2fbb
- [net,2/2] rxrpc: Defer the response to a PING ACK until we've parsed it
https://git.kernel.org/netdev/net/c/1a01319feef7
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
On 11/16/2023 8:12 AM, David Howells wrote:
> Fix RTT determination to be able to use any type of ACK as the response
> from which RTT can be calculated provided its ack.serial is non-zero and
> matches the serial number of an outgoing DATA or ACK packet. This
> shouldn't be limited to REQUESTED-type ACKs as these can have other types
> substituted for them for things like duplicate or out-of-order packets.
>
> Fixes: 4700c4d80b7b ("rxrpc: Fix loss of RTT samples due to interposed ACK")
> Signed-off-by: David Howells <[email protected]>
> cc: Marc Dionne <[email protected]>
> cc: "David S. Miller" <[email protected]>
> cc: Eric Dumazet <[email protected]>
> cc: Jakub Kicinski <[email protected]>
> cc: Paolo Abeni <[email protected]>
> cc: [email protected]
> cc: [email protected]
Reviewed-by: Jeffrey Altman <[email protected]>