Skip to content

Commit

Permalink
rxrpc: Clean up ACK handling
Browse files Browse the repository at this point in the history 
Clean up the rxrpc_propose_ACK() function.  If deferred PING ACK proposal
is split out, it's only really needed for deferred DELAY ACKs.  All other
ACKs, bar terminal IDLE ACK are sent immediately.  The deferred IDLE ACK
submission can be handled by conversion of a DELAY ACK into an IDLE ACK if
there's nothing to be SACK'd.

Also, because there's a delay between an ACK being generated and being
transmitted, it's possible that other ACKs of the same type will be
generated during that interval.  Apart from the ACK time and the serial
number responded to, most of the ACK body, including window and SACK
parameters, are not filled out till the point of transmission - so we can
avoid generating a new ACK if there's one pending that will cover the SACK
data we need to convey.

Therefore, don't propose a new DELAY or IDLE ACK for a call if there's one
already pending.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
  • Loading branch information
David Howells committed Nov 8, 2022
1 parent 72f0c6f commit 530403d
Show file tree
Hide file tree
Showing 11 changed files with 86 additions and 138 deletions.
52 changes: 36 additions & 16 deletions include/trace/events/rxrpc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@
#define rxrpc_propose_ack_traces \
EM(rxrpc_propose_ack_client_tx_end, "ClTxEnd") \
EM(rxrpc_propose_ack_input_data, "DataIn ") \
EM(rxrpc_propose_ack_input_data_hole, "DataInH") \
EM(rxrpc_propose_ack_ping_for_check_life, "ChkLife") \
EM(rxrpc_propose_ack_ping_for_keepalive, "KeepAlv") \
EM(rxrpc_propose_ack_ping_for_lost_ack, "LostAck") \
Expand All @@ -170,11 +171,6 @@
EM(rxrpc_propose_ack_rotate_rx, "RxAck ") \
E_(rxrpc_propose_ack_terminal_ack, "ClTerm ")

#define rxrpc_propose_ack_outcomes \
EM(rxrpc_propose_ack_subsume, " Subsume") \
EM(rxrpc_propose_ack_update, " Update") \
E_(rxrpc_propose_ack_use, " New")

#define rxrpc_congest_modes \
EM(RXRPC_CALL_CONGEST_AVOIDANCE, "CongAvoid") \
EM(RXRPC_CALL_FAST_RETRANSMIT, "FastReTx ") \
Expand Down Expand Up @@ -313,7 +309,6 @@ rxrpc_congest_changes;
rxrpc_congest_modes;
rxrpc_conn_traces;
rxrpc_local_traces;
rxrpc_propose_ack_outcomes;
rxrpc_propose_ack_traces;
rxrpc_receive_traces;
rxrpc_recvmsg_traces;
Expand Down Expand Up @@ -1012,7 +1007,7 @@ TRACE_EVENT(rxrpc_timer,
__entry->call = call->debug_id;
__entry->why = why;
__entry->now = now;
__entry->ack_at = call->ack_at;
__entry->ack_at = call->delay_ack_at;
__entry->ack_lost_at = call->ack_lost_at;
__entry->resend_at = call->resend_at;
__entry->expect_rx_by = call->expect_rx_by;
Expand Down Expand Up @@ -1054,7 +1049,7 @@ TRACE_EVENT(rxrpc_timer_expired,
TP_fast_assign(
__entry->call = call->debug_id;
__entry->now = now;
__entry->ack_at = call->ack_at;
__entry->ack_at = call->delay_ack_at;
__entry->ack_lost_at = call->ack_lost_at;
__entry->resend_at = call->resend_at;
__entry->expect_rx_by = call->expect_rx_by;
Expand Down Expand Up @@ -1098,33 +1093,29 @@ TRACE_EVENT(rxrpc_rx_lose,

TRACE_EVENT(rxrpc_propose_ack,
TP_PROTO(struct rxrpc_call *call, enum rxrpc_propose_ack_trace why,
u8 ack_reason, rxrpc_serial_t serial,
enum rxrpc_propose_ack_outcome outcome),
u8 ack_reason, rxrpc_serial_t serial),

TP_ARGS(call, why, ack_reason, serial, outcome),
TP_ARGS(call, why, ack_reason, serial),

TP_STRUCT__entry(
__field(unsigned int, call )
__field(enum rxrpc_propose_ack_trace, why )
__field(rxrpc_serial_t, serial )
__field(u8, ack_reason )
__field(enum rxrpc_propose_ack_outcome, outcome )
),

TP_fast_assign(
__entry->call = call->debug_id;
__entry->why = why;
__entry->serial = serial;
__entry->ack_reason = ack_reason;
__entry->outcome = outcome;
),

TP_printk("c=%08x %s %s r=%08x%s",
TP_printk("c=%08x %s %s r=%08x",
__entry->call,
__print_symbolic(__entry->why, rxrpc_propose_ack_traces),
__print_symbolic(__entry->ack_reason, rxrpc_ack_names),
__entry->serial,
__print_symbolic(__entry->outcome, rxrpc_propose_ack_outcomes))
__entry->serial)
);

TRACE_EVENT(rxrpc_send_ack,
Expand Down Expand Up @@ -1154,6 +1145,35 @@ TRACE_EVENT(rxrpc_send_ack,
__entry->serial)
);

TRACE_EVENT(rxrpc_drop_ack,
TP_PROTO(struct rxrpc_call *call, enum rxrpc_propose_ack_trace why,
u8 ack_reason, rxrpc_serial_t serial, bool nobuf),

TP_ARGS(call, why, ack_reason, serial, nobuf),

TP_STRUCT__entry(
__field(unsigned int, call )
__field(enum rxrpc_propose_ack_trace, why )
__field(rxrpc_serial_t, serial )
__field(u8, ack_reason )
__field(bool, nobuf )
),

TP_fast_assign(
__entry->call = call->debug_id;
__entry->why = why;
__entry->serial = serial;
__entry->ack_reason = ack_reason;
__entry->nobuf = nobuf;
),

TP_printk("c=%08x %s %s r=%08x nbf=%u",
__entry->call,
__print_symbolic(__entry->why, rxrpc_propose_ack_traces),
__print_symbolic(__entry->ack_reason, rxrpc_ack_names),
__entry->serial, __entry->nobuf)
);

TRACE_EVENT(rxrpc_retransmit,
TP_PROTO(struct rxrpc_call *call, rxrpc_seq_t seq, u8 annotation,
s64 expiry),
Expand Down
10 changes: 5 additions & 5 deletions net/rxrpc/ar-internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,8 @@ enum rxrpc_call_flag {
RXRPC_CALL_DISCONNECTED, /* The call has been disconnected */
RXRPC_CALL_KERNEL, /* The call was made by the kernel */
RXRPC_CALL_UPGRADE, /* Service upgrade was requested for the call */
RXRPC_CALL_DELAY_ACK_PENDING, /* DELAY ACK generation is pending */
RXRPC_CALL_IDLE_ACK_PENDING, /* IDLE ACK generation is pending */
};

/*
Expand Down Expand Up @@ -582,7 +584,7 @@ struct rxrpc_call {
struct rxrpc_net *rxnet; /* Network namespace to which call belongs */
const struct rxrpc_security *security; /* applied security module */
struct mutex user_mutex; /* User access mutex */
unsigned long ack_at; /* When deferred ACK needs to happen */
unsigned long delay_ack_at; /* When DELAY ACK needs to happen */
unsigned long ack_lost_at; /* When ACK is figured as lost */
unsigned long resend_at; /* When next resend needs to happen */
unsigned long ping_at; /* When next to send a ping */
Expand Down Expand Up @@ -834,7 +836,8 @@ int rxrpc_user_charge_accept(struct rxrpc_sock *, unsigned long);
void rxrpc_propose_ping(struct rxrpc_call *call, u32 serial,
enum rxrpc_propose_ack_trace why);
void rxrpc_send_ACK(struct rxrpc_call *, u8, rxrpc_serial_t, enum rxrpc_propose_ack_trace);
void rxrpc_propose_ACK(struct rxrpc_call *, u8, u32, enum rxrpc_propose_ack_trace);
void rxrpc_propose_delay_ACK(struct rxrpc_call *, rxrpc_serial_t,
enum rxrpc_propose_ack_trace);
void rxrpc_process_call(struct work_struct *);

void rxrpc_reduce_call_timer(struct rxrpc_call *call,
Expand Down Expand Up @@ -1016,15 +1019,12 @@ static inline bool __rxrpc_use_local(struct rxrpc_local *local)
* misc.c
*/
extern unsigned int rxrpc_max_backlog __read_mostly;
extern unsigned long rxrpc_requested_ack_delay;
extern unsigned long rxrpc_soft_ack_delay;
extern unsigned long rxrpc_idle_ack_delay;
extern unsigned int rxrpc_rx_window_size;
extern unsigned int rxrpc_rx_mtu;
extern unsigned int rxrpc_rx_jumbo_max;

extern const s8 rxrpc_ack_priority[];

/*
* net_ns.c
*/
Expand Down
95 changes: 26 additions & 69 deletions net/rxrpc/call_event.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,94 +29,53 @@ void rxrpc_propose_ping(struct rxrpc_call *call, u32 serial,
spin_lock_bh(&call->lock);

if (time_before(ping_at, call->ping_at)) {
rxrpc_inc_stat(call->rxnet, stat_tx_acks[RXRPC_ACK_PING]);
WRITE_ONCE(call->ping_at, ping_at);
rxrpc_reduce_call_timer(call, ping_at, now,
rxrpc_timer_set_for_ping);
trace_rxrpc_propose_ack(call, why, RXRPC_ACK_PING, serial,
rxrpc_propose_ack_use);
trace_rxrpc_propose_ack(call, why, RXRPC_ACK_PING, serial);
}

spin_unlock_bh(&call->lock);
}

/*
* propose an ACK be sent
* Propose a DELAY ACK be sent in the future.
*/
static void __rxrpc_propose_ACK(struct rxrpc_call *call, u8 ack_reason,
u32 serial, enum rxrpc_propose_ack_trace why)
static void __rxrpc_propose_delay_ACK(struct rxrpc_call *call,
rxrpc_serial_t serial,
enum rxrpc_propose_ack_trace why)
{
enum rxrpc_propose_ack_outcome outcome = rxrpc_propose_ack_use;
unsigned long expiry = rxrpc_soft_ack_delay;
unsigned long now = jiffies, ack_at;
s8 prior = rxrpc_ack_priority[ack_reason];

rxrpc_inc_stat(call->rxnet, stat_tx_acks[ack_reason]);

/* Update DELAY, IDLE, REQUESTED and PING_RESPONSE ACK serial
* numbers, but we don't alter the timeout.
*/
_debug("prior %u %u vs %u %u",
ack_reason, prior,
call->ackr_reason, rxrpc_ack_priority[call->ackr_reason]);
if (ack_reason == call->ackr_reason) {
if (RXRPC_ACK_UPDATEABLE & (1 << ack_reason)) {
outcome = rxrpc_propose_ack_update;
call->ackr_serial = serial;
}
} else if (prior > rxrpc_ack_priority[call->ackr_reason]) {
call->ackr_reason = ack_reason;
call->ackr_serial = serial;
} else {
outcome = rxrpc_propose_ack_subsume;
}

switch (ack_reason) {
case RXRPC_ACK_REQUESTED:
if (rxrpc_requested_ack_delay < expiry)
expiry = rxrpc_requested_ack_delay;
break;

case RXRPC_ACK_DELAY:
if (rxrpc_soft_ack_delay < expiry)
expiry = rxrpc_soft_ack_delay;
break;

case RXRPC_ACK_IDLE:
if (rxrpc_idle_ack_delay < expiry)
expiry = rxrpc_idle_ack_delay;
break;

default:
WARN_ON(1);
return;
}

call->ackr_serial = serial;

if (rxrpc_soft_ack_delay < expiry)
expiry = rxrpc_soft_ack_delay;
if (call->peer->srtt_us != 0)
ack_at = usecs_to_jiffies(call->peer->srtt_us >> 3);
else
ack_at = expiry;

ack_at += READ_ONCE(call->tx_backoff);
ack_at += now;
if (time_before(ack_at, call->ack_at)) {
WRITE_ONCE(call->ack_at, ack_at);
if (time_before(ack_at, call->delay_ack_at)) {
WRITE_ONCE(call->delay_ack_at, ack_at);
rxrpc_reduce_call_timer(call, ack_at, now,
rxrpc_timer_set_for_ack);
}

trace_rxrpc_propose_ack(call, why, ack_reason, serial, outcome);
trace_rxrpc_propose_ack(call, why, RXRPC_ACK_DELAY, serial);
}

/*
* propose an ACK be sent, locking the call structure
* Propose a DELAY ACK be sent, locking the call structure
*/
void rxrpc_propose_ACK(struct rxrpc_call *call, u8 ack_reason, u32 serial,
enum rxrpc_propose_ack_trace why)
void rxrpc_propose_delay_ACK(struct rxrpc_call *call, rxrpc_serial_t serial,
enum rxrpc_propose_ack_trace why)
{
spin_lock_bh(&call->lock);
__rxrpc_propose_ACK(call, ack_reason, serial, why);
__rxrpc_propose_delay_ACK(call, serial, why);
spin_unlock_bh(&call->lock);
}

Expand All @@ -131,6 +90,11 @@ void rxrpc_send_ACK(struct rxrpc_call *call, u8 ack_reason,

if (test_bit(RXRPC_CALL_DISCONNECTED, &call->flags))
return;
if (ack_reason == RXRPC_ACK_DELAY &&
test_and_set_bit(RXRPC_CALL_DELAY_ACK_PENDING, &call->flags)) {
trace_rxrpc_drop_ack(call, why, ack_reason, serial, false);
return;
}

rxrpc_inc_stat(call->rxnet, stat_tx_acks[ack_reason]);

Expand Down Expand Up @@ -319,7 +283,6 @@ void rxrpc_process_call(struct work_struct *work)
unsigned long now, next, t;
unsigned int iterations = 0;
rxrpc_serial_t ackr_serial;
u8 ackr_reason;

rxrpc_see_call(call);

Expand Down Expand Up @@ -364,19 +327,13 @@ void rxrpc_process_call(struct work_struct *work)
set_bit(RXRPC_CALL_EV_EXPIRED, &call->events);
}

t = READ_ONCE(call->ack_at);
t = READ_ONCE(call->delay_ack_at);
if (time_after_eq(now, t)) {
trace_rxrpc_timer(call, rxrpc_timer_exp_ack, now);
cmpxchg(&call->ack_at, t, now + MAX_JIFFY_OFFSET);
spin_lock_bh(&call->lock);
ackr_reason = call->ackr_reason;
ackr_serial = call->ackr_serial;
call->ackr_reason = 0;
call->ackr_serial = 0;
spin_unlock_bh(&call->lock);
if (ackr_reason)
rxrpc_send_ACK(call, ackr_reason, ackr_serial,
rxrpc_propose_ack_ping_for_lost_ack);
cmpxchg(&call->delay_ack_at, t, now + MAX_JIFFY_OFFSET);
ackr_serial = xchg(&call->ackr_serial, 0);
rxrpc_send_ACK(call, RXRPC_ACK_DELAY, ackr_serial,
rxrpc_propose_ack_ping_for_lost_ack);
}

t = READ_ONCE(call->ack_lost_at);
Expand Down Expand Up @@ -441,7 +398,7 @@ void rxrpc_process_call(struct work_struct *work)

set(call->expect_req_by);
set(call->expect_term_by);
set(call->ack_at);
set(call->delay_ack_at);
set(call->ack_lost_at);
set(call->resend_at);
set(call->keepalive_at);
Expand Down
2 changes: 1 addition & 1 deletion net/rxrpc/call_object.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ static void rxrpc_start_call_timer(struct rxrpc_call *call)
unsigned long now = jiffies;
unsigned long j = now + MAX_JIFFY_OFFSET;

call->ack_at = j;
call->delay_ack_at = j;
call->ack_lost_at = j;
call->resend_at = j;
call->ping_at = j;
Expand Down
8 changes: 4 additions & 4 deletions net/rxrpc/input.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ static bool rxrpc_receiving_reply(struct rxrpc_call *call)
now = jiffies;
timo = now + MAX_JIFFY_OFFSET;
WRITE_ONCE(call->resend_at, timo);
WRITE_ONCE(call->ack_at, timo);
WRITE_ONCE(call->delay_ack_at, timo);
trace_rxrpc_timer(call, rxrpc_timer_init_for_reply, now);
}

Expand Down Expand Up @@ -542,7 +542,7 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
/* Send an immediate ACK if we fill in a hole */
if (!acked) {
rxrpc_send_ACK(call, RXRPC_ACK_DELAY, serial,
rxrpc_propose_ack_input_data);
rxrpc_propose_ack_input_data_hole);
acked = true;
}
}
Expand Down Expand Up @@ -584,8 +584,8 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
rxrpc_send_ACK(call, RXRPC_ACK_IDLE, ack_serial,
rxrpc_propose_ack_input_data);
else
rxrpc_propose_ACK(call, RXRPC_ACK_DELAY, ack_serial,
rxrpc_propose_ack_input_data);
rxrpc_propose_delay_ACK(call, ack_serial,
rxrpc_propose_ack_input_data);

trace_rxrpc_notify_socket(call->debug_id, serial);
rxrpc_notify_socket(call);
Expand Down
18 changes: 0 additions & 18 deletions net/rxrpc/misc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@
*/
unsigned int rxrpc_max_backlog __read_mostly = 10;

/*
* How long to wait before scheduling ACK generation after seeing a
* packet with RXRPC_REQUEST_ACK set (in jiffies).
*/
unsigned long rxrpc_requested_ack_delay = 1;

/*
* How long to wait before scheduling an ACK with subtype DELAY (in jiffies).
*
Expand Down Expand Up @@ -62,15 +56,3 @@ unsigned int rxrpc_rx_mtu = 5692;
* sender that we're willing to handle.
*/
unsigned int rxrpc_rx_jumbo_max = 4;

const s8 rxrpc_ack_priority[] = {
[0] = 0,
[RXRPC_ACK_DELAY] = 1,
[RXRPC_ACK_REQUESTED] = 2,
[RXRPC_ACK_IDLE] = 3,
[RXRPC_ACK_DUPLICATE] = 4,
[RXRPC_ACK_OUT_OF_SEQUENCE] = 5,
[RXRPC_ACK_EXCEEDS_WINDOW] = 6,
[RXRPC_ACK_NOSPACE] = 7,
[RXRPC_ACK_PING_RESPONSE] = 8,
};
Loading

0 comments on commit 530403d

Please sign in to comment.