Commit c12b9588 authored by David S. Miller's avatar David S. Miller

Merge branch 'rxrpc-fixes'

David Howells says:

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

Here are some fixes for AF_RXRPC:

 (1) Fix listen() allowing preallocation to overrun the prealloc buffer.

 (2) Prevent resending the request if we've seen the reply starting to
     arrive.

 (3) Fix accidental sharing of ACK state between transmission and
     reception.

 (4) Ignore ACKs in which ack.previousPacket regresses.  This indicates the
     highest DATA number so far seen, so should not be seen to go
     backwards.

 (5) Fix the determination of when to generate an IDLE-type ACK,
     simplifying it so that we generate one if we have more than two DATA
     packets that aren't hard-acked (consumed) or soft-acked (in the rx
     buffer, but could be discarded and re-requested).
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents eb4c0788 9a3dedcf
......@@ -1509,7 +1509,7 @@ TRACE_EVENT(rxrpc_call_reset,
__entry->call_serial = call->rx_serial;
__entry->conn_serial = call->conn->hi_serial;
__entry->tx_seq = call->tx_hard_ack;
__entry->rx_seq = call->ackr_seen;
__entry->rx_seq = call->rx_hard_ack;
),
TP_printk("c=%08x %08x:%08x r=%08x/%08x tx=%08x rx=%08x",
......
......@@ -676,13 +676,12 @@ struct rxrpc_call {
spinlock_t input_lock; /* Lock for packet input to this call */
/* receive-phase ACK management */
/* Receive-phase ACK management (ACKs we send). */
u8 ackr_reason; /* reason to ACK */
rxrpc_serial_t ackr_serial; /* serial of packet being ACK'd */
rxrpc_serial_t ackr_first_seq; /* first sequence number received */
rxrpc_seq_t ackr_prev_seq; /* previous sequence number received */
rxrpc_seq_t ackr_consumed; /* Highest packet shown consumed */
rxrpc_seq_t ackr_seen; /* Highest packet shown seen */
rxrpc_seq_t ackr_highest_seq; /* Higest sequence number received */
atomic_t ackr_nr_unacked; /* Number of unacked packets */
atomic_t ackr_nr_consumed; /* Number of packets needing hard ACK */
/* RTT management */
rxrpc_serial_t rtt_serial[4]; /* Serial number of DATA or PING sent */
......@@ -692,8 +691,10 @@ struct rxrpc_call {
#define RXRPC_CALL_RTT_AVAIL_MASK 0xf
#define RXRPC_CALL_RTT_PEND_SHIFT 8
/* transmission-phase ACK management */
/* Transmission-phase ACK management (ACKs we've received). */
ktime_t acks_latest_ts; /* Timestamp of latest ACK received */
rxrpc_seq_t acks_first_seq; /* first sequence number received */
rxrpc_seq_t acks_prev_seq; /* Highest previousPacket received */
rxrpc_seq_t acks_lowest_nak; /* Lowest NACK in the buffer (or ==tx_hard_ack) */
rxrpc_seq_t acks_lost_top; /* tx_top at the time lost-ack ping sent */
rxrpc_serial_t acks_lost_ping; /* Serial number of probe ACK */
......
......@@ -406,7 +406,8 @@ void rxrpc_process_call(struct work_struct *work)
goto recheck_state;
}
if (test_and_clear_bit(RXRPC_CALL_EV_RESEND, &call->events)) {
if (test_and_clear_bit(RXRPC_CALL_EV_RESEND, &call->events) &&
call->state != RXRPC_CALL_CLIENT_RECV_REPLY) {
rxrpc_resend(call, now);
goto recheck_state;
}
......
......@@ -412,8 +412,8 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
{
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
enum rxrpc_call_state state;
unsigned int j, nr_subpackets;
rxrpc_serial_t serial = sp->hdr.serial, ack_serial = 0;
unsigned int j, nr_subpackets, nr_unacked = 0;
rxrpc_serial_t serial = sp->hdr.serial, ack_serial = serial;
rxrpc_seq_t seq0 = sp->hdr.seq, hard_ack;
bool immediate_ack = false, jumbo_bad = false;
u8 ack = 0;
......@@ -453,7 +453,6 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
!rxrpc_receiving_reply(call))
goto unlock;
call->ackr_prev_seq = seq0;
hard_ack = READ_ONCE(call->rx_hard_ack);
nr_subpackets = sp->nr_subpackets;
......@@ -534,6 +533,9 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
ack_serial = serial;
}
if (after(seq0, call->ackr_highest_seq))
call->ackr_highest_seq = seq0;
/* Queue the packet. We use a couple of memory barriers here as need
* to make sure that rx_top is perceived to be set after the buffer
* pointer and that the buffer pointer is set after the annotation and
......@@ -567,6 +569,8 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
sp = NULL;
}
nr_unacked++;
if (last) {
set_bit(RXRPC_CALL_RX_LAST, &call->flags);
if (!ack) {
......@@ -586,9 +590,14 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
}
call->rx_expect_next = seq + 1;
}
if (!ack)
ack_serial = serial;
}
ack:
if (atomic_add_return(nr_unacked, &call->ackr_nr_unacked) > 2 && !ack)
ack = RXRPC_ACK_IDLE;
if (ack)
rxrpc_propose_ACK(call, ack, ack_serial,
immediate_ack, true,
......@@ -812,7 +821,7 @@ static void rxrpc_input_soft_acks(struct rxrpc_call *call, u8 *acks,
static bool rxrpc_is_ack_valid(struct rxrpc_call *call,
rxrpc_seq_t first_pkt, rxrpc_seq_t prev_pkt)
{
rxrpc_seq_t base = READ_ONCE(call->ackr_first_seq);
rxrpc_seq_t base = READ_ONCE(call->acks_first_seq);
if (after(first_pkt, base))
return true; /* The window advanced */
......@@ -820,7 +829,7 @@ static bool rxrpc_is_ack_valid(struct rxrpc_call *call,
if (before(first_pkt, base))
return false; /* firstPacket regressed */
if (after_eq(prev_pkt, call->ackr_prev_seq))
if (after_eq(prev_pkt, call->acks_prev_seq))
return true; /* previousPacket hasn't regressed. */
/* Some rx implementations put a serial number in previousPacket. */
......@@ -906,8 +915,8 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb)
/* Discard any out-of-order or duplicate ACKs (outside lock). */
if (!rxrpc_is_ack_valid(call, first_soft_ack, prev_pkt)) {
trace_rxrpc_rx_discard_ack(call->debug_id, ack_serial,
first_soft_ack, call->ackr_first_seq,
prev_pkt, call->ackr_prev_seq);
first_soft_ack, call->acks_first_seq,
prev_pkt, call->acks_prev_seq);
return;
}
......@@ -922,14 +931,14 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb)
/* Discard any out-of-order or duplicate ACKs (inside lock). */
if (!rxrpc_is_ack_valid(call, first_soft_ack, prev_pkt)) {
trace_rxrpc_rx_discard_ack(call->debug_id, ack_serial,
first_soft_ack, call->ackr_first_seq,
prev_pkt, call->ackr_prev_seq);
first_soft_ack, call->acks_first_seq,
prev_pkt, call->acks_prev_seq);
goto out;
}
call->acks_latest_ts = skb->tstamp;
call->ackr_first_seq = first_soft_ack;
call->ackr_prev_seq = prev_pkt;
call->acks_first_seq = first_soft_ack;
call->acks_prev_seq = prev_pkt;
/* Parse rwind and mtu sizes if provided. */
if (buf.info.rxMTU)
......
......@@ -74,11 +74,18 @@ static size_t rxrpc_fill_out_ack(struct rxrpc_connection *conn,
u8 reason)
{
rxrpc_serial_t serial;
unsigned int tmp;
rxrpc_seq_t hard_ack, top, seq;
int ix;
u32 mtu, jmax;
u8 *ackp = pkt->acks;
tmp = atomic_xchg(&call->ackr_nr_unacked, 0);
tmp |= atomic_xchg(&call->ackr_nr_consumed, 0);
if (!tmp && (reason == RXRPC_ACK_DELAY ||
reason == RXRPC_ACK_IDLE))
return 0;
/* Barrier against rxrpc_input_data(). */
serial = call->ackr_serial;
hard_ack = READ_ONCE(call->rx_hard_ack);
......@@ -89,7 +96,7 @@ static size_t rxrpc_fill_out_ack(struct rxrpc_connection *conn,
pkt->ack.bufferSpace = htons(8);
pkt->ack.maxSkew = htons(0);
pkt->ack.firstPacket = htonl(hard_ack + 1);
pkt->ack.previousPacket = htonl(call->ackr_prev_seq);
pkt->ack.previousPacket = htonl(call->ackr_highest_seq);
pkt->ack.serial = htonl(serial);
pkt->ack.reason = reason;
pkt->ack.nAcks = top - hard_ack;
......@@ -223,6 +230,10 @@ int rxrpc_send_ack_packet(struct rxrpc_call *call, bool ping,
n = rxrpc_fill_out_ack(conn, call, pkt, &hard_ack, &top, reason);
spin_unlock_bh(&call->lock);
if (n == 0) {
kfree(pkt);
return 0;
}
iov[0].iov_base = pkt;
iov[0].iov_len = sizeof(pkt->whdr) + sizeof(pkt->ack) + n;
......@@ -259,13 +270,6 @@ int rxrpc_send_ack_packet(struct rxrpc_call *call, bool ping,
ntohl(pkt->ack.serial),
false, true,
rxrpc_propose_ack_retry_tx);
} else {
spin_lock_bh(&call->lock);
if (after(hard_ack, call->ackr_consumed))
call->ackr_consumed = hard_ack;
if (after(top, call->ackr_seen))
call->ackr_seen = top;
spin_unlock_bh(&call->lock);
}
rxrpc_set_keepalive(call);
......
......@@ -260,11 +260,9 @@ static void rxrpc_rotate_rx_window(struct rxrpc_call *call)
rxrpc_end_rx_phase(call, serial);
} else {
/* Check to see if there's an ACK that needs sending. */
if (after_eq(hard_ack, call->ackr_consumed + 2) ||
after_eq(top, call->ackr_seen + 2) ||
(hard_ack == top && after(hard_ack, call->ackr_consumed)))
rxrpc_propose_ACK(call, RXRPC_ACK_DELAY, serial,
true, true,
if (atomic_inc_return(&call->ackr_nr_consumed) > 2)
rxrpc_propose_ACK(call, RXRPC_ACK_IDLE, serial,
true, false,
rxrpc_propose_ack_rotate_rx);
if (call->ackr_reason && call->ackr_reason != RXRPC_ACK_DELAY)
rxrpc_send_ack_packet(call, false, NULL);
......
......@@ -12,7 +12,7 @@
static struct ctl_table_header *rxrpc_sysctl_reg_table;
static const unsigned int four = 4;
static const unsigned int thirtytwo = 32;
static const unsigned int max_backlog = RXRPC_BACKLOG_MAX - 1;
static const unsigned int n_65535 = 65535;
static const unsigned int n_max_acks = RXRPC_RXTX_BUFF_SIZE - 1;
static const unsigned long one_jiffy = 1;
......@@ -89,7 +89,7 @@ static struct ctl_table rxrpc_sysctl_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec_minmax,
.extra1 = (void *)&four,
.extra2 = (void *)&thirtytwo,
.extra2 = (void *)&max_backlog,
},
{
.procname = "rx_window_size",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment