Skip to content

Commit

Permalink
Merge branch 'rxrpc-fixes'
Browse files Browse the repository at this point in the history
David Howells says:

====================
rxrpc: Miscellaneous fixes

Here some miscellaneous fixes for AF_RXRPC:

 (1) The zero serial number has a special meaning in an ACK packet serial
     reference, so skip it when assigning serial numbers to transmitted
     packets.

 (2) Don't set the reference serial number in a delayed ACK as the ACK
     cannot be used for RTT calculation.

 (3) Don't emit a DUP ACK response to a PING RESPONSE ACK coming back to a
     call that completed in the meantime.

 (4) Fix the counting of acks and nacks in ACK packet to better drive
     congestion management.  We want to know if there have been new
     acks/nacks since the last ACK packet, not that there are still
     acks/nacks.  This is more complicated as we have to save the old SACK
     table and compare it.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Feb 5, 2024
2 parents fdeba0b + 41b7fa1 commit 645eb54
Show file tree
Hide file tree
Showing 9 changed files with 154 additions and 43 deletions.
8 changes: 5 additions & 3 deletions include/trace/events/rxrpc.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@
EM(rxrpc_skb_eaten_by_unshare_nomem, "ETN unshar-nm") \
EM(rxrpc_skb_get_conn_secured, "GET conn-secd") \
EM(rxrpc_skb_get_conn_work, "GET conn-work") \
EM(rxrpc_skb_get_last_nack, "GET last-nack") \
EM(rxrpc_skb_get_local_work, "GET locl-work") \
EM(rxrpc_skb_get_reject_work, "GET rej-work ") \
EM(rxrpc_skb_get_to_recvmsg, "GET to-recv ") \
Expand All @@ -141,6 +142,7 @@
EM(rxrpc_skb_put_error_report, "PUT error-rep") \
EM(rxrpc_skb_put_input, "PUT input ") \
EM(rxrpc_skb_put_jumbo_subpacket, "PUT jumbo-sub") \
EM(rxrpc_skb_put_last_nack, "PUT last-nack") \
EM(rxrpc_skb_put_purge, "PUT purge ") \
EM(rxrpc_skb_put_rotate, "PUT rotate ") \
EM(rxrpc_skb_put_unknown, "PUT unknown ") \
Expand Down Expand Up @@ -1552,17 +1554,17 @@ TRACE_EVENT(rxrpc_congest,
memcpy(&__entry->sum, summary, sizeof(__entry->sum));
),

TP_printk("c=%08x r=%08x %s q=%08x %s cw=%u ss=%u nA=%u,%u+%u r=%u b=%u u=%u d=%u l=%x%s%s%s",
TP_printk("c=%08x r=%08x %s q=%08x %s cw=%u ss=%u nA=%u,%u+%u,%u b=%u u=%u d=%u l=%x%s%s%s",
__entry->call,
__entry->ack_serial,
__print_symbolic(__entry->sum.ack_reason, rxrpc_ack_names),
__entry->hard_ack,
__print_symbolic(__entry->sum.mode, rxrpc_congest_modes),
__entry->sum.cwnd,
__entry->sum.ssthresh,
__entry->sum.nr_acks, __entry->sum.saw_nacks,
__entry->sum.nr_acks, __entry->sum.nr_retained_nacks,
__entry->sum.nr_new_acks,
__entry->sum.nr_rot_new_acks,
__entry->sum.nr_new_nacks,
__entry->top - __entry->hard_ack,
__entry->sum.cumulative_acks,
__entry->sum.dup_acks,
Expand Down
37 changes: 30 additions & 7 deletions net/rxrpc/ar-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,19 @@ struct rxrpc_host_header {
*/
struct rxrpc_skb_priv {
struct rxrpc_connection *conn; /* Connection referred to (poke packet) */
u16 offset; /* Offset of data */
u16 len; /* Length of data */
u8 flags;
union {
struct {
u16 offset; /* Offset of data */
u16 len; /* Length of data */
u8 flags;
#define RXRPC_RX_VERIFIED 0x01

};
struct {
rxrpc_seq_t first_ack; /* First packet in acks table */
u8 nr_acks; /* Number of acks+nacks */
u8 nr_nacks; /* Number of nacks */
};
};
struct rxrpc_host_header hdr; /* RxRPC packet header from this packet */
};

Expand Down Expand Up @@ -510,7 +518,7 @@ struct rxrpc_connection {
enum rxrpc_call_completion completion; /* Completion condition */
s32 abort_code; /* Abort code of connection abort */
int debug_id; /* debug ID for printks */
atomic_t serial; /* packet serial number counter */
rxrpc_serial_t tx_serial; /* Outgoing packet serial number counter */
unsigned int hi_serial; /* highest serial number received */
u32 service_id; /* Service ID, possibly upgraded */
u32 security_level; /* Security level selected */
Expand Down Expand Up @@ -692,11 +700,11 @@ struct rxrpc_call {
u8 cong_dup_acks; /* Count of ACKs showing missing packets */
u8 cong_cumul_acks; /* Cumulative ACK count */
ktime_t cong_tstamp; /* Last time cwnd was changed */
struct sk_buff *cong_last_nack; /* Last ACK with nacks received */

/* Receive-phase ACK management (ACKs we send). */
u8 ackr_reason; /* reason to ACK */
u16 ackr_sack_base; /* Starting slot in SACK table ring */
rxrpc_serial_t ackr_serial; /* serial of packet being ACK'd */
rxrpc_seq_t ackr_window; /* Base of SACK window */
rxrpc_seq_t ackr_wtop; /* Base of SACK window */
unsigned int ackr_nr_unacked; /* Number of unacked packets */
Expand Down Expand Up @@ -730,7 +738,8 @@ struct rxrpc_call {
struct rxrpc_ack_summary {
u16 nr_acks; /* Number of ACKs in packet */
u16 nr_new_acks; /* Number of new ACKs in packet */
u16 nr_rot_new_acks; /* Number of rotated new ACKs */
u16 nr_new_nacks; /* Number of new nacks in packet */
u16 nr_retained_nacks; /* Number of nacks retained between ACKs */
u8 ack_reason;
bool saw_nacks; /* Saw NACKs in packet */
bool new_low_nack; /* T if new low NACK found */
Expand Down Expand Up @@ -822,6 +831,20 @@ static inline bool rxrpc_sending_to_client(const struct rxrpc_txbuf *txb)

#include <trace/events/rxrpc.h>

/*
* Allocate the next serial number on a connection. 0 must be skipped.
*/
static inline rxrpc_serial_t rxrpc_get_next_serial(struct rxrpc_connection *conn)
{
rxrpc_serial_t serial;

serial = conn->tx_serial;
if (serial == 0)
serial = 1;
conn->tx_serial = serial + 1;
return serial;
}

/*
* af_rxrpc.c
*/
Expand Down
12 changes: 5 additions & 7 deletions net/rxrpc/call_event.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ void rxrpc_propose_delay_ACK(struct rxrpc_call *call, rxrpc_serial_t serial,
unsigned long expiry = rxrpc_soft_ack_delay;
unsigned long now = jiffies, ack_at;

call->ackr_serial = serial;

if (rxrpc_soft_ack_delay < expiry)
expiry = rxrpc_soft_ack_delay;
if (call->peer->srtt_us != 0)
Expand Down Expand Up @@ -114,6 +112,7 @@ static void rxrpc_congestion_timeout(struct rxrpc_call *call)
void rxrpc_resend(struct rxrpc_call *call, struct sk_buff *ack_skb)
{
struct rxrpc_ackpacket *ack = NULL;
struct rxrpc_skb_priv *sp;
struct rxrpc_txbuf *txb;
unsigned long resend_at;
rxrpc_seq_t transmitted = READ_ONCE(call->tx_transmitted);
Expand Down Expand Up @@ -141,14 +140,15 @@ void rxrpc_resend(struct rxrpc_call *call, struct sk_buff *ack_skb)
* explicitly NAK'd packets.
*/
if (ack_skb) {
sp = rxrpc_skb(ack_skb);
ack = (void *)ack_skb->data + sizeof(struct rxrpc_wire_header);

for (i = 0; i < ack->nAcks; i++) {
for (i = 0; i < sp->nr_acks; i++) {
rxrpc_seq_t seq;

if (ack->acks[i] & 1)
continue;
seq = ntohl(ack->firstPacket) + i;
seq = sp->first_ack + i;
if (after(txb->seq, transmitted))
break;
if (after(txb->seq, seq))
Expand Down Expand Up @@ -373,7 +373,6 @@ static void rxrpc_send_initial_ping(struct rxrpc_call *call)
bool rxrpc_input_call_event(struct rxrpc_call *call, struct sk_buff *skb)
{
unsigned long now, next, t;
rxrpc_serial_t ackr_serial;
bool resend = false, expired = false;
s32 abort_code;

Expand Down Expand Up @@ -423,8 +422,7 @@ bool rxrpc_input_call_event(struct rxrpc_call *call, struct sk_buff *skb)
if (time_after_eq(now, t)) {
trace_rxrpc_timer(call, rxrpc_timer_exp_ack, now);
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_send_ACK(call, RXRPC_ACK_DELAY, 0,
rxrpc_propose_ack_ping_for_lost_ack);
}

Expand Down
1 change: 1 addition & 0 deletions net/rxrpc/call_object.c
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@ static void rxrpc_destroy_call(struct work_struct *work)

del_timer_sync(&call->timer);

rxrpc_free_skb(call->cong_last_nack, rxrpc_skb_put_last_nack);
rxrpc_cleanup_ring(call);
while ((txb = list_first_entry_or_null(&call->tx_sendmsg,
struct rxrpc_txbuf, call_link))) {
Expand Down
10 changes: 9 additions & 1 deletion net/rxrpc/conn_event.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,14 @@ void rxrpc_conn_retransmit_call(struct rxrpc_connection *conn,

_enter("%d", conn->debug_id);

if (sp && sp->hdr.type == RXRPC_PACKET_TYPE_ACK) {
if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header),
&pkt.ack, sizeof(pkt.ack)) < 0)
return;
if (pkt.ack.reason == RXRPC_ACK_PING_RESPONSE)
return;
}

chan = &conn->channels[channel];

/* If the last call got moved on whilst we were waiting to run, just
Expand All @@ -117,7 +125,7 @@ void rxrpc_conn_retransmit_call(struct rxrpc_connection *conn,
iov[2].iov_base = &ack_info;
iov[2].iov_len = sizeof(ack_info);

serial = atomic_inc_return(&conn->serial);
serial = rxrpc_get_next_serial(conn);

pkt.whdr.epoch = htonl(conn->proto.epoch);
pkt.whdr.cid = htonl(conn->proto.cid | channel);
Expand Down
Loading

0 comments on commit 645eb54

Please sign in to comment.