Skip to content

Commit a5af7e1

Browse files
dhowellsdhowells
committed
rxrpc: Fix loss of PING RESPONSE ACK production due to PING ACKs
Separate the output of PING ACKs from the output of other sorts of ACK so that if we receive a PING ACK and schedule transmission of a PING RESPONSE ACK, the response doesn't get cancelled by a PING ACK we happen to be scheduling transmission of at the same time. If a PING RESPONSE gets lost, the other side might just sit there waiting for it and refuse to proceed otherwise. Signed-off-by: David Howells <dhowells@redhat.com>
1 parent 26cb02a commit a5af7e1

8 files changed

Lines changed: 82 additions & 29 deletions

File tree

‎net/rxrpc/ar-internal.h‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,7 @@ enum rxrpc_call_flag {
398398
RXRPC_CALL_EXPOSED, /* The call was exposed to the world */
399399
RXRPC_CALL_RX_LAST, /* Received the last packet (at rxtx_top) */
400400
RXRPC_CALL_TX_LAST, /* Last packet in Tx buffer (at rxtx_top) */
401+
RXRPC_CALL_SEND_PING, /* A ping will need to be sent */
401402
RXRPC_CALL_PINGING, /* Ping in process */
402403
RXRPC_CALL_RETRANS_TIMEOUT, /* Retransmission due to timeout occurred */
403404
};
@@ -410,6 +411,7 @@ enum rxrpc_call_event {
410411
RXRPC_CALL_EV_ABORT, /* need to generate abort */
411412
RXRPC_CALL_EV_TIMER, /* Timer expired */
412413
RXRPC_CALL_EV_RESEND, /* Tx resend required */
414+
RXRPC_CALL_EV_PING, /* Ping send required */
413415
};
414416

415417
/*
@@ -466,6 +468,7 @@ struct rxrpc_call {
466468
struct rxrpc_sock __rcu *socket; /* socket responsible */
467469
ktime_t ack_at; /* When deferred ACK needs to happen */
468470
ktime_t resend_at; /* When next resend needs to happen */
471+
ktime_t ping_at; /* When next to send a ping */
469472
ktime_t expire_at; /* When the call times out */
470473
struct timer_list timer; /* Combined event timer */
471474
struct work_struct processor; /* Event processor */
@@ -558,8 +561,10 @@ struct rxrpc_call {
558561
rxrpc_seq_t ackr_prev_seq; /* previous sequence number received */
559562
rxrpc_seq_t ackr_consumed; /* Highest packet shown consumed */
560563
rxrpc_seq_t ackr_seen; /* Highest packet shown seen */
561-
rxrpc_serial_t ackr_ping; /* Last ping sent */
562-
ktime_t ackr_ping_time; /* Time last ping sent */
564+
565+
/* ping management */
566+
rxrpc_serial_t ping_serial; /* Last ping sent */
567+
ktime_t ping_time; /* Time last ping sent */
563568

564569
/* transmission-phase ACK management */
565570
ktime_t acks_latest_ts; /* Timestamp of latest ACK received */
@@ -730,6 +735,7 @@ enum rxrpc_timer_trace {
730735
rxrpc_timer_init_for_reply,
731736
rxrpc_timer_expired,
732737
rxrpc_timer_set_for_ack,
738+
rxrpc_timer_set_for_ping,
733739
rxrpc_timer_set_for_resend,
734740
rxrpc_timer_set_for_send,
735741
rxrpc_timer__nr_trace
@@ -1068,7 +1074,7 @@ extern const s8 rxrpc_ack_priority[];
10681074
/*
10691075
* output.c
10701076
*/
1071-
int rxrpc_send_ack_packet(struct rxrpc_call *);
1077+
int rxrpc_send_ack_packet(struct rxrpc_call *, bool);
10721078
int rxrpc_send_abort_packet(struct rxrpc_call *);
10731079
int rxrpc_send_data_packet(struct rxrpc_call *, struct sk_buff *, bool);
10741080
void rxrpc_reject_packets(struct rxrpc_local *);

‎net/rxrpc/call_event.c‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,14 @@ void rxrpc_set_timer(struct rxrpc_call *call, enum rxrpc_timer_trace why,
5454
t = call->ack_at;
5555
}
5656

57+
if (!ktime_after(call->ping_at, now)) {
58+
call->ping_at = call->expire_at;
59+
if (!test_and_set_bit(RXRPC_CALL_EV_PING, &call->events))
60+
queue = true;
61+
} else if (ktime_before(call->ping_at, t)) {
62+
t = call->ping_at;
63+
}
64+
5765
t_j = nsecs_to_jiffies(ktime_to_ns(ktime_sub(t, now)));
5866
t_j += jiffies;
5967

@@ -77,6 +85,27 @@ void rxrpc_set_timer(struct rxrpc_call *call, enum rxrpc_timer_trace why,
7785
read_unlock_bh(&call->state_lock);
7886
}
7987

88+
/*
89+
* Propose a PING ACK be sent.
90+
*/
91+
static void rxrpc_propose_ping(struct rxrpc_call *call,
92+
bool immediate, bool background)
93+
{
94+
if (immediate) {
95+
if (background &&
96+
!test_and_set_bit(RXRPC_CALL_EV_PING, &call->events))
97+
rxrpc_queue_call(call);
98+
} else {
99+
ktime_t now = ktime_get_real();
100+
ktime_t ping_at = ktime_add_ms(now, rxrpc_idle_ack_delay);
101+
102+
if (ktime_before(ping_at, call->ping_at)) {
103+
call->ping_at = ping_at;
104+
rxrpc_set_timer(call, rxrpc_timer_set_for_ping, now);
105+
}
106+
}
107+
}
108+
80109
/*
81110
* propose an ACK be sent
82111
*/
@@ -90,6 +119,14 @@ static void __rxrpc_propose_ACK(struct rxrpc_call *call, u8 ack_reason,
90119
ktime_t now, ack_at;
91120
s8 prior = rxrpc_ack_priority[ack_reason];
92121

122+
/* Pings are handled specially because we don't want to accidentally
123+
* lose a ping response by subsuming it into a ping.
124+
*/
125+
if (ack_reason == RXRPC_ACK_PING) {
126+
rxrpc_propose_ping(call, immediate, background);
127+
goto trace;
128+
}
129+
93130
/* Update DELAY, IDLE, REQUESTED and PING_RESPONSE ACK serial
94131
* numbers, but we don't alter the timeout.
95132
*/
@@ -125,7 +162,6 @@ static void __rxrpc_propose_ACK(struct rxrpc_call *call, u8 ack_reason,
125162
expiry = rxrpc_soft_ack_delay;
126163
break;
127164

128-
case RXRPC_ACK_PING:
129165
case RXRPC_ACK_IDLE:
130166
if (rxrpc_idle_ack_delay < expiry)
131167
expiry = rxrpc_idle_ack_delay;
@@ -253,7 +289,7 @@ static void rxrpc_resend(struct rxrpc_call *call, ktime_t now)
253289
goto out;
254290
rxrpc_propose_ACK(call, RXRPC_ACK_PING, 0, 0, true, false,
255291
rxrpc_propose_ack_ping_for_lost_ack);
256-
rxrpc_send_ack_packet(call);
292+
rxrpc_send_ack_packet(call, true);
257293
goto out;
258294
}
259295

@@ -345,13 +381,17 @@ void rxrpc_process_call(struct work_struct *work)
345381
}
346382

347383
if (test_and_clear_bit(RXRPC_CALL_EV_ACK, &call->events)) {
348-
call->ack_at = call->expire_at;
349384
if (call->ackr_reason) {
350-
rxrpc_send_ack_packet(call);
385+
rxrpc_send_ack_packet(call, false);
351386
goto recheck_state;
352387
}
353388
}
354389

390+
if (test_and_clear_bit(RXRPC_CALL_EV_PING, &call->events)) {
391+
rxrpc_send_ack_packet(call, true);
392+
goto recheck_state;
393+
}
394+
355395
if (test_and_clear_bit(RXRPC_CALL_EV_RESEND, &call->events)) {
356396
rxrpc_resend(call, now);
357397
goto recheck_state;

‎net/rxrpc/call_object.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ static void rxrpc_start_call_timer(struct rxrpc_call *call)
205205
expire_at = ktime_add_ms(now, rxrpc_max_call_lifetime);
206206
call->expire_at = expire_at;
207207
call->ack_at = expire_at;
208+
call->ping_at = expire_at;
208209
call->resend_at = expire_at;
209210
call->timer.expires = jiffies + LONG_MAX / 2;
210211
rxrpc_set_timer(call, rxrpc_timer_begin, now);

‎net/rxrpc/input.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -625,9 +625,9 @@ static void rxrpc_input_ping_response(struct rxrpc_call *call,
625625
rxrpc_serial_t ping_serial;
626626
ktime_t ping_time;
627627

628-
ping_time = call->ackr_ping_time;
628+
ping_time = call->ping_time;
629629
smp_rmb();
630-
ping_serial = call->ackr_ping;
630+
ping_serial = call->ping_serial;
631631

632632
if (!test_bit(RXRPC_CALL_PINGING, &call->flags) ||
633633
before(orig_serial, ping_serial))

‎net/rxrpc/misc.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ const s8 rxrpc_ack_priority[] = {
9393
[RXRPC_ACK_EXCEEDS_WINDOW] = 6,
9494
[RXRPC_ACK_NOSPACE] = 7,
9595
[RXRPC_ACK_PING_RESPONSE] = 8,
96-
[RXRPC_ACK_PING] = 9,
9796
};
9897

9998
const char rxrpc_ack_names[RXRPC_ACK__INVALID + 1][4] = {
@@ -197,6 +196,7 @@ const char rxrpc_timer_traces[rxrpc_timer__nr_trace][8] = {
197196
[rxrpc_timer_expired] = "*EXPR*",
198197
[rxrpc_timer_init_for_reply] = "IniRpl",
199198
[rxrpc_timer_set_for_ack] = "SetAck",
199+
[rxrpc_timer_set_for_ping] = "SetPng",
200200
[rxrpc_timer_set_for_send] = "SetTx ",
201201
[rxrpc_timer_set_for_resend] = "SetRTx",
202202
};

‎net/rxrpc/output.c‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ struct rxrpc_abort_buffer {
3838
static size_t rxrpc_fill_out_ack(struct rxrpc_call *call,
3939
struct rxrpc_ack_buffer *pkt,
4040
rxrpc_seq_t *_hard_ack,
41-
rxrpc_seq_t *_top)
41+
rxrpc_seq_t *_top,
42+
u8 reason)
4243
{
4344
rxrpc_serial_t serial;
4445
rxrpc_seq_t hard_ack, top, seq;
@@ -58,10 +59,10 @@ static size_t rxrpc_fill_out_ack(struct rxrpc_call *call,
5859
pkt->ack.firstPacket = htonl(hard_ack + 1);
5960
pkt->ack.previousPacket = htonl(call->ackr_prev_seq);
6061
pkt->ack.serial = htonl(serial);
61-
pkt->ack.reason = call->ackr_reason;
62+
pkt->ack.reason = reason;
6263
pkt->ack.nAcks = top - hard_ack;
6364

64-
if (pkt->ack.reason == RXRPC_ACK_PING)
65+
if (reason == RXRPC_ACK_PING)
6566
pkt->whdr.flags |= RXRPC_REQUEST_ACK;
6667

6768
if (after(top, hard_ack)) {
@@ -93,7 +94,7 @@ static size_t rxrpc_fill_out_ack(struct rxrpc_call *call,
9394
/*
9495
* Send an ACK call packet.
9596
*/
96-
int rxrpc_send_ack_packet(struct rxrpc_call *call)
97+
int rxrpc_send_ack_packet(struct rxrpc_call *call, bool ping)
9798
{
9899
struct rxrpc_connection *conn = NULL;
99100
struct rxrpc_ack_buffer *pkt;
@@ -102,8 +103,8 @@ int rxrpc_send_ack_packet(struct rxrpc_call *call)
102103
rxrpc_serial_t serial;
103104
rxrpc_seq_t hard_ack, top;
104105
size_t len, n;
105-
bool ping = false;
106106
int ret;
107+
u8 reason;
107108

108109
spin_lock_bh(&call->lock);
109110
if (call->conn)
@@ -136,14 +137,18 @@ int rxrpc_send_ack_packet(struct rxrpc_call *call)
136137
pkt->whdr.serviceId = htons(call->service_id);
137138

138139
spin_lock_bh(&call->lock);
139-
if (!call->ackr_reason) {
140-
spin_unlock_bh(&call->lock);
141-
ret = 0;
142-
goto out;
140+
if (ping) {
141+
reason = RXRPC_ACK_PING;
142+
} else {
143+
reason = call->ackr_reason;
144+
if (!call->ackr_reason) {
145+
spin_unlock_bh(&call->lock);
146+
ret = 0;
147+
goto out;
148+
}
149+
call->ackr_reason = 0;
143150
}
144-
ping = (call->ackr_reason == RXRPC_ACK_PING);
145-
n = rxrpc_fill_out_ack(call, pkt, &hard_ack, &top);
146-
call->ackr_reason = 0;
151+
n = rxrpc_fill_out_ack(call, pkt, &hard_ack, &top, reason);
147152

148153
spin_unlock_bh(&call->lock);
149154

@@ -161,7 +166,7 @@ int rxrpc_send_ack_packet(struct rxrpc_call *call)
161166
pkt->ack.reason, pkt->ack.nAcks);
162167

163168
if (ping) {
164-
call->ackr_ping = serial;
169+
call->ping_serial = serial;
165170
smp_wmb();
166171
/* We need to stick a time in before we send the packet in case
167172
* the reply gets back before kernel_sendmsg() completes - but
@@ -170,18 +175,19 @@ int rxrpc_send_ack_packet(struct rxrpc_call *call)
170175
* the packet transmission is more likely to happen towards the
171176
* end of the kernel_sendmsg() call.
172177
*/
173-
call->ackr_ping_time = ktime_get_real();
178+
call->ping_time = ktime_get_real();
174179
set_bit(RXRPC_CALL_PINGING, &call->flags);
175180
trace_rxrpc_rtt_tx(call, rxrpc_rtt_tx_ping, serial);
176181
}
177182

178183
ret = kernel_sendmsg(conn->params.local->socket, &msg, iov, 2, len);
179184
if (ping)
180-
call->ackr_ping_time = ktime_get_real();
185+
call->ping_time = ktime_get_real();
181186

182187
if (call->state < RXRPC_CALL_COMPLETE) {
183188
if (ret < 0) {
184-
clear_bit(RXRPC_CALL_PINGING, &call->flags);
189+
if (ping)
190+
clear_bit(RXRPC_CALL_PINGING, &call->flags);
185191
rxrpc_propose_ACK(call, pkt->ack.reason,
186192
ntohs(pkt->ack.maxSkew),
187193
ntohl(pkt->ack.serial),

‎net/rxrpc/recvmsg.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ static void rxrpc_end_rx_phase(struct rxrpc_call *call, rxrpc_serial_t serial)
143143
if (call->state == RXRPC_CALL_CLIENT_RECV_REPLY) {
144144
rxrpc_propose_ACK(call, RXRPC_ACK_IDLE, 0, serial, true, false,
145145
rxrpc_propose_ack_terminal_ack);
146-
rxrpc_send_ack_packet(call);
146+
rxrpc_send_ack_packet(call, false);
147147
}
148148

149149
write_lock_bh(&call->state_lock);
@@ -212,7 +212,7 @@ static void rxrpc_rotate_rx_window(struct rxrpc_call *call)
212212
true, false,
213213
rxrpc_propose_ack_rotate_rx);
214214
if (call->ackr_reason)
215-
rxrpc_send_ack_packet(call);
215+
rxrpc_send_ack_packet(call, false);
216216
}
217217
}
218218

‎net/rxrpc/sendmsg.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
197197
do {
198198
/* Check to see if there's a ping ACK to reply to. */
199199
if (call->ackr_reason == RXRPC_ACK_PING_RESPONSE)
200-
rxrpc_send_ack_packet(call);
200+
rxrpc_send_ack_packet(call, false);
201201

202202
if (!skb) {
203203
size_t size, chunk, max, space;

0 commit comments

Comments
 (0)