[PATCH net 0/5] rxrpc: Miscellaneous fixes

[PATCH net 0/5] rxrpc: Miscellaneous fixes

[David Howells]

Here some miscellaneous fixes for AF_RXRPC:

 (1) Fix the congestion control algorithm to start cwnd at 4 and to not cut
     ssthresh when the peer cuts its rwind size.

 (2) Only transmit a single ACK for all the DATA packets glued together
     into a jumbo packet to reduce the number of ACKs being generated.

 (3) Clean up the generation of flags in the protocol header when creating
     a packet for transmission.  This means we don't carry the old
     REQUEST-ACK bit around from previous transmissions, will make it
     easier to fix the MORE-PACKETS flag and make it easier to do jumbo
     packet assembly in future.

 (4) Fix how the MORE-PACKETS flag is driven.  We shouldn't be setting it
     in sendmsg() as the packet is then queued and the bit is left in that
     state, no matter how long it takes us to transmit the packet - and
     will still be in that state if the packet is retransmitted.

 (5) Request an ACK on an impending transmission stall due to the app layer
     not feeding us new data fast enough.  If we don't request an ACK, we
     may have to hold on to the packet buffers for a significant amount of
     time until the receiver gets bored and sends us an ACK anyway.

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 (5):
  rxrpc: Fix congestion control algorithm
  rxrpc: Only transmit one ACK per jumbo packet received
  rxrpc: Clean up Tx header flags generation handling
  rxrpc: Change how the MORE-PACKETS rxrpc wire header flag is driven
  rxrpc: Request an ACK on impending Tx stall

 include/trace/events/rxrpc.h |  2 +-
 net/rxrpc/ar-internal.h      |  2 +-
 net/rxrpc/call_object.c      |  7 +-----
 net/rxrpc/input.c            | 49 +++++++++++++++++++++++++-----------
 net/rxrpc/output.c           | 26 ++++++++++++++-----
 net/rxrpc/proc.c             |  6 ++---
 net/rxrpc/sendmsg.c          |  3 ---
 7 files changed, 61 insertions(+), 34 deletions(-)

[PATCH net 1/5] rxrpc: Fix congestion control algorithm

[David Howells]

Make the following fixes to the congestion control algorithm:

 (1) Don't vary the cwnd starting value by the size of RXRPC_TX_SMSS since
     that's currently held constant - set to the size of a jumbo subpacket
     payload so that we can create jumbo packets on the fly.  The current
     code invariably picks 3 as the starting value.

     Further, the starting cwnd needs to be an even number because we ack
     every other packet, so set it to 4.

 (2) Don't cut ssthresh when we see an ACK come from the peer with a
     receive window (rwind) less than ssthresh.  ssthresh keeps track of
     characteristics of the connection whereas rwind may be reduced by the
     peer for any reason - and may be reduced to 0.

Fixes: 1fc4fa2ac93d ("rxrpc: Fix congestion management")
Fixes: 0851115090a3 ("rxrpc: Reduce ssthresh to peer's receive window")
Signed-off-by: David Howells <dhowells@redhat.com>
Suggested-by: Simon Wilkinson <sxw@auristor.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
---
 net/rxrpc/ar-internal.h | 2 +-
 net/rxrpc/call_object.c | 7 +------
 net/rxrpc/input.c       | 3 ---
 3 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 08c0a32db8c7..08de24658f4f 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -697,7 +697,7 @@ struct rxrpc_call {
 	 * packets) rather than bytes.
 	 */
 #define RXRPC_TX_SMSS		RXRPC_JUMBO_DATALEN
-#define RXRPC_MIN_CWND		(RXRPC_TX_SMSS > 2190 ? 2 : RXRPC_TX_SMSS > 1095 ? 3 : 4)
+#define RXRPC_MIN_CWND		4
 	u8			cong_cwnd;	/* Congestion window size */
 	u8			cong_extra;	/* Extra to send for congestion management */
 	u8			cong_ssthresh;	/* Slow-start threshold */
diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 01fa71e8b1f7..f9e983a12c14 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -174,12 +174,7 @@ struct rxrpc_call *rxrpc_alloc_call(struct rxrpc_sock *rx, gfp_t gfp,
 	call->rx_winsize = rxrpc_rx_window_size;
 	call->tx_winsize = 16;
 
-	if (RXRPC_TX_SMSS > 2190)
-		call->cong_cwnd = 2;
-	else if (RXRPC_TX_SMSS > 1095)
-		call->cong_cwnd = 3;
-	else
-		call->cong_cwnd = 4;
+	call->cong_cwnd = RXRPC_MIN_CWND;
 	call->cong_ssthresh = RXRPC_TX_MAX_WINDOW;
 
 	call->rxnet = rxnet;
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 3dedb8c0618c..860075f1290b 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -685,9 +685,6 @@ static void rxrpc_input_ack_trailer(struct rxrpc_call *call, struct sk_buff *skb
 		call->tx_winsize = rwind;
 	}
 
-	if (call->cong_ssthresh > rwind)
-		call->cong_ssthresh = rwind;
-
 	mtu = min(ntohl(trailer->maxMTU), ntohl(trailer->ifMTU));
 
 	peer = call->peer;

[PATCH net 2/5] rxrpc: Only transmit one ACK per jumbo packet received

[David Howells]

Only generate one ACK packet for all the subpackets in a jumbo packet.  If
we would like to generate more than one ACK, we prioritise them base on
their reason code, in the order, highest first:

   OutOfSeq > NoSpace > ExceedsWin > Duplicate > Requested > Delay > Idle

For the first four, we reference the lowest offending subpacket; for the
last three, the highest.

This reduces the number of ACKs we end up transmitting to one per UDP
packet transmitted to reduce network loading and packet parsing.

Fixes: 5d7edbc9231e ("rxrpc: Get rid of the Rx ring")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
---
 net/rxrpc/input.c | 46 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 860075f1290b..16d49a861dbb 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -9,6 +9,17 @@
 
 #include "ar-internal.h"
 
+/* Override priority when generating ACKs for received DATA */
+static const u8 rxrpc_ack_priority[RXRPC_ACK__INVALID] = {
+	[RXRPC_ACK_IDLE]		= 1,
+	[RXRPC_ACK_DELAY]		= 2,
+	[RXRPC_ACK_REQUESTED]		= 3,
+	[RXRPC_ACK_DUPLICATE]		= 4,
+	[RXRPC_ACK_EXCEEDS_WINDOW]	= 5,
+	[RXRPC_ACK_NOSPACE]		= 6,
+	[RXRPC_ACK_OUT_OF_SEQUENCE]	= 7,
+};
+
 static void rxrpc_proto_abort(struct rxrpc_call *call, rxrpc_seq_t seq,
 			      enum rxrpc_abort_reason why)
 {
@@ -365,7 +376,7 @@ static void rxrpc_input_queue_data(struct rxrpc_call *call, struct sk_buff *skb,
  * Process a DATA packet.
  */
 static void rxrpc_input_data_one(struct rxrpc_call *call, struct sk_buff *skb,
-				 bool *_notify)
+				 bool *_notify, rxrpc_serial_t *_ack_serial, int *_ack_reason)
 {
 	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
 	struct sk_buff *oos;
@@ -418,8 +429,6 @@ static void rxrpc_input_data_one(struct rxrpc_call *call, struct sk_buff *skb,
 		/* Send an immediate ACK if we fill in a hole */
 		else if (!skb_queue_empty(&call->rx_oos_queue))
 			ack_reason = RXRPC_ACK_DELAY;
-		else
-			call->ackr_nr_unacked++;
 
 		window++;
 		if (after(window, wtop)) {
@@ -497,12 +506,16 @@ static void rxrpc_input_data_one(struct rxrpc_call *call, struct sk_buff *skb,
 	}
 
 send_ack:
-	if (ack_reason >= 0)
-		rxrpc_send_ACK(call, ack_reason, serial,
-			       rxrpc_propose_ack_input_data);
-	else
-		rxrpc_propose_delay_ACK(call, serial,
-					rxrpc_propose_ack_input_data);
+	if (ack_reason >= 0) {
+		if (rxrpc_ack_priority[ack_reason] > rxrpc_ack_priority[*_ack_reason]) {
+			*_ack_serial = serial;
+			*_ack_reason = ack_reason;
+		} else if (rxrpc_ack_priority[ack_reason] == rxrpc_ack_priority[*_ack_reason] &&
+			   ack_reason == RXRPC_ACK_REQUESTED) {
+			*_ack_serial = serial;
+			*_ack_reason = ack_reason;
+		}
+	}
 }
 
 /*
@@ -513,9 +526,11 @@ static bool rxrpc_input_split_jumbo(struct rxrpc_call *call, struct sk_buff *skb
 	struct rxrpc_jumbo_header jhdr;
 	struct rxrpc_skb_priv *sp = rxrpc_skb(skb), *jsp;
 	struct sk_buff *jskb;
+	rxrpc_serial_t ack_serial = 0;
 	unsigned int offset = sizeof(struct rxrpc_wire_header);
 	unsigned int len = skb->len - offset;
 	bool notify = false;
+	int ack_reason = 0;
 
 	while (sp->hdr.flags & RXRPC_JUMBO_PACKET) {
 		if (len < RXRPC_JUMBO_SUBPKTLEN)
@@ -535,7 +550,7 @@ static bool rxrpc_input_split_jumbo(struct rxrpc_call *call, struct sk_buff *skb
 		jsp = rxrpc_skb(jskb);
 		jsp->offset = offset;
 		jsp->len = RXRPC_JUMBO_DATALEN;
-		rxrpc_input_data_one(call, jskb, &notify);
+		rxrpc_input_data_one(call, jskb, &notify, &ack_serial, &ack_reason);
 		rxrpc_free_skb(jskb, rxrpc_skb_put_jumbo_subpacket);
 
 		sp->hdr.flags = jhdr.flags;
@@ -548,7 +563,16 @@ static bool rxrpc_input_split_jumbo(struct rxrpc_call *call, struct sk_buff *skb
 
 	sp->offset = offset;
 	sp->len    = len;
-	rxrpc_input_data_one(call, skb, &notify);
+	rxrpc_input_data_one(call, skb, &notify, &ack_serial, &ack_reason);
+
+	if (ack_reason > 0) {
+		rxrpc_send_ACK(call, ack_reason, ack_serial,
+			       rxrpc_propose_ack_input_data);
+	} else {
+		call->ackr_nr_unacked++;
+		rxrpc_propose_delay_ACK(call, sp->hdr.serial,
+					rxrpc_propose_ack_input_data);
+	}
 	if (notify) {
 		trace_rxrpc_notify_socket(call->debug_id, sp->hdr.serial);
 		rxrpc_notify_socket(call);

[PATCH net 3/5] rxrpc: Clean up Tx header flags generation handling

[David Howells]

Clean up the generation of the header flags when building packet headers
for transmission:

 (1) Assemble the flags in a local variable rather than in the txb->flags.

 (2) Do the flags masking and JUMBO-PACKET setting in one bit of code for
     both the main header and the jumbo headers.

 (3) Generate the REQUEST-ACK flag afresh each time.  There's a possibility
     we might want to do jumbo retransmission packets in future.

 (4) Pass the local flags variable to the rxrpc_tx_data tracepoint rather
     than the combination of the txb flags and the wire header flags (the
     latter belong only to the first subpacket).

This makes it easier to clean up setting the MORE-PACKETS flag

Fixes: 44125d5aadda ("rxrpc: Split up the DATA packet transmission function")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
---
 include/trace/events/rxrpc.h |  1 -
 net/rxrpc/ar-internal.h      |  2 +-
 net/rxrpc/output.c           | 18 ++++++++++++------
 net/rxrpc/proc.c             |  3 +--
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index a1b126a6b0d7..7b6c1db53401 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -449,7 +449,6 @@
 
 #define rxrpc_req_ack_traces \
 	EM(rxrpc_reqack_ack_lost,		"ACK-LOST  ")	\
-	EM(rxrpc_reqack_already_on,		"ALREADY-ON")	\
 	EM(rxrpc_reqack_more_rtt,		"MORE-RTT  ")	\
 	EM(rxrpc_reqack_no_srv_last,		"NO-SRVLAST")	\
 	EM(rxrpc_reqack_old_rtt,		"OLD-RTT   ")	\
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 08de24658f4f..c11a6043c8f2 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -110,7 +110,7 @@ struct rxrpc_net {
 	atomic_t		stat_tx_acks[256];
 	atomic_t		stat_rx_acks[256];
 
-	atomic_t		stat_why_req_ack[8];
+	atomic_t		stat_why_req_ack[7];
 
 	atomic_t		stat_io_loop;
 };
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index 5ea9601efd05..bf2d0f847cdb 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -330,6 +330,8 @@ static void rxrpc_prepare_data_subpacket(struct rxrpc_call *call, struct rxrpc_t
 	struct rxrpc_wire_header *whdr = txb->kvec[0].iov_base;
 	enum rxrpc_req_ack_trace why;
 	struct rxrpc_connection *conn = call->conn;
+	bool last;
+	u8 flags;
 
 	_enter("%x,{%d}", txb->seq, txb->len);
 
@@ -339,6 +341,10 @@ static void rxrpc_prepare_data_subpacket(struct rxrpc_call *call, struct rxrpc_t
 	    txb->seq == 1)
 		whdr->userStatus = RXRPC_USERSTATUS_SERVICE_UPGRADE;
 
+	txb->flags &= ~RXRPC_REQUEST_ACK;
+	flags = txb->flags & RXRPC_TXBUF_WIRE_FLAGS;
+	last = txb->flags & RXRPC_LAST_PACKET;
+
 	/* If our RTT cache needs working on, request an ACK.  Also request
 	 * ACKs if a DATA packet appears to have been lost.
 	 *
@@ -346,9 +352,7 @@ static void rxrpc_prepare_data_subpacket(struct rxrpc_call *call, struct rxrpc_t
 	 * service call, lest OpenAFS incorrectly send us an ACK with some
 	 * soft-ACKs in it and then never follow up with a proper hard ACK.
 	 */
-	if (txb->flags & RXRPC_REQUEST_ACK)
-		why = rxrpc_reqack_already_on;
-	else if ((txb->flags & RXRPC_LAST_PACKET) && rxrpc_sending_to_client(txb))
+	if (last && rxrpc_sending_to_client(txb))
 		why = rxrpc_reqack_no_srv_last;
 	else if (test_and_clear_bit(RXRPC_CALL_EV_ACK_LOST, &call->events))
 		why = rxrpc_reqack_ack_lost;
@@ -367,15 +371,17 @@ static void rxrpc_prepare_data_subpacket(struct rxrpc_call *call, struct rxrpc_t
 
 	rxrpc_inc_stat(call->rxnet, stat_why_req_ack[why]);
 	trace_rxrpc_req_ack(call->debug_id, txb->seq, why);
-	if (why != rxrpc_reqack_no_srv_last)
+	if (why != rxrpc_reqack_no_srv_last) {
 		txb->flags |= RXRPC_REQUEST_ACK;
+		flags |= RXRPC_REQUEST_ACK;
+	}
 dont_set_request_ack:
 
-	whdr->flags = txb->flags & RXRPC_TXBUF_WIRE_FLAGS;
+	whdr->flags	= flags;
 	whdr->serial	= htonl(txb->serial);
 	whdr->cksum	= txb->cksum;
 
-	trace_rxrpc_tx_data(call, txb->seq, txb->serial, txb->flags, false);
+	trace_rxrpc_tx_data(call, txb->seq, txb->serial, flags, false);
 }
 
 /*
diff --git a/net/rxrpc/proc.c b/net/rxrpc/proc.c
index 263a2251e3d2..3b7e34dd4385 100644
--- a/net/rxrpc/proc.c
+++ b/net/rxrpc/proc.c
@@ -519,9 +519,8 @@ int rxrpc_stats_show(struct seq_file *seq, void *v)
 		   atomic_read(&rxnet->stat_rx_acks[RXRPC_ACK_DELAY]),
 		   atomic_read(&rxnet->stat_rx_acks[RXRPC_ACK_IDLE]));
 	seq_printf(seq,
-		   "Why-Req-A: acklost=%u already=%u mrtt=%u ortt=%u\n",
+		   "Why-Req-A: acklost=%u mrtt=%u ortt=%u\n",
 		   atomic_read(&rxnet->stat_why_req_ack[rxrpc_reqack_ack_lost]),
-		   atomic_read(&rxnet->stat_why_req_ack[rxrpc_reqack_already_on]),
 		   atomic_read(&rxnet->stat_why_req_ack[rxrpc_reqack_more_rtt]),
 		   atomic_read(&rxnet->stat_why_req_ack[rxrpc_reqack_old_rtt]));
 	seq_printf(seq,

[PATCH net 4/5] rxrpc: Change how the MORE-PACKETS rxrpc wire header flag is driven

[David Howells]

Currently, the MORE-PACKETS rxrpc header flag is set by sendmsg trying to
guess how it should be set by looking to see if there's space in the Tx
window and setting it if there is - long before the packet gets
transmitted (and it gets left in this state).  As a consequence, it's not
very meaningful.

Change this such that it is turned on at the point of transmission if we
have more packets after it in the send buffers and it is left clear if we
don't yet.

Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
cc: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
---
 net/rxrpc/output.c  | 8 +++++++-
 net/rxrpc/sendmsg.c | 3 ---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index bf2d0f847cdb..4ebd0bd40a02 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -330,7 +330,7 @@ static void rxrpc_prepare_data_subpacket(struct rxrpc_call *call, struct rxrpc_t
 	struct rxrpc_wire_header *whdr = txb->kvec[0].iov_base;
 	enum rxrpc_req_ack_trace why;
 	struct rxrpc_connection *conn = call->conn;
-	bool last;
+	bool more, last;
 	u8 flags;
 
 	_enter("%x,{%d}", txb->seq, txb->len);
@@ -345,6 +345,12 @@ static void rxrpc_prepare_data_subpacket(struct rxrpc_call *call, struct rxrpc_t
 	flags = txb->flags & RXRPC_TXBUF_WIRE_FLAGS;
 	last = txb->flags & RXRPC_LAST_PACKET;
 
+	more = (!last &&
+		(!list_is_last(&txb->call_link, &call->tx_buffer) ||
+		 !list_empty(&call->tx_sendmsg)));
+	if (more)
+		flags |= RXRPC_MORE_PACKETS;
+
 	/* If our RTT cache needs working on, request an ACK.  Also request
 	 * ACKs if a DATA packet appears to have been lost.
 	 *
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index 894b8fa68e5e..eaf4441a340b 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -384,9 +384,6 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 		    (msg_data_left(msg) == 0 && !more)) {
 			if (msg_data_left(msg) == 0 && !more)
 				txb->flags |= RXRPC_LAST_PACKET;
-			else if (call->tx_top - call->acks_hard_ack <
-				 call->tx_winsize)
-				txb->flags |= RXRPC_MORE_PACKETS;
 
 			ret = call->security->secure_packet(call, txb);
 			if (ret < 0)

[PATCH net 5/5] rxrpc: Request an ACK on impending Tx stall

[David Howells]

Set the REQUEST-ACK flag on the DATA packet we're about to send if we're
about to stall transmission because the app layer isn't keeping up
supplying us with data to transmit.

Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
---
 include/trace/events/rxrpc.h | 1 +
 net/rxrpc/ar-internal.h      | 2 +-
 net/rxrpc/output.c           | 2 ++
 net/rxrpc/proc.c             | 5 +++--
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 7b6c1db53401..dca9f4759dcb 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -449,6 +449,7 @@
 
 #define rxrpc_req_ack_traces \
 	EM(rxrpc_reqack_ack_lost,		"ACK-LOST  ")	\
+	EM(rxrpc_reqack_app_stall,		"APP-STALL ")	\
 	EM(rxrpc_reqack_more_rtt,		"MORE-RTT  ")	\
 	EM(rxrpc_reqack_no_srv_last,		"NO-SRVLAST")	\
 	EM(rxrpc_reqack_old_rtt,		"OLD-RTT   ")	\
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index c11a6043c8f2..08de24658f4f 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -110,7 +110,7 @@ struct rxrpc_net {
 	atomic_t		stat_tx_acks[256];
 	atomic_t		stat_rx_acks[256];
 
-	atomic_t		stat_why_req_ack[7];
+	atomic_t		stat_why_req_ack[8];
 
 	atomic_t		stat_io_loop;
 };
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index 4ebd0bd40a02..32626ff377e1 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -372,6 +372,8 @@ static void rxrpc_prepare_data_subpacket(struct rxrpc_call *call, struct rxrpc_t
 		why = rxrpc_reqack_more_rtt;
 	else if (ktime_before(ktime_add_ms(call->peer->rtt_last_req, 1000), ktime_get_real()))
 		why = rxrpc_reqack_old_rtt;
+	else if (!more && !last)
+		why = rxrpc_reqack_app_stall;
 	else
 		goto dont_set_request_ack;
 
diff --git a/net/rxrpc/proc.c b/net/rxrpc/proc.c
index 3b7e34dd4385..1bab7f5a7d0f 100644
--- a/net/rxrpc/proc.c
+++ b/net/rxrpc/proc.c
@@ -519,10 +519,11 @@ int rxrpc_stats_show(struct seq_file *seq, void *v)
 		   atomic_read(&rxnet->stat_rx_acks[RXRPC_ACK_DELAY]),
 		   atomic_read(&rxnet->stat_rx_acks[RXRPC_ACK_IDLE]));
 	seq_printf(seq,
-		   "Why-Req-A: acklost=%u mrtt=%u ortt=%u\n",
+		   "Why-Req-A: acklost=%u mrtt=%u ortt=%u stall=%u\n",
 		   atomic_read(&rxnet->stat_why_req_ack[rxrpc_reqack_ack_lost]),
 		   atomic_read(&rxnet->stat_why_req_ack[rxrpc_reqack_more_rtt]),
-		   atomic_read(&rxnet->stat_why_req_ack[rxrpc_reqack_old_rtt]));
+		   atomic_read(&rxnet->stat_why_req_ack[rxrpc_reqack_old_rtt]),
+		   atomic_read(&rxnet->stat_why_req_ack[rxrpc_reqack_app_stall]));
 	seq_printf(seq,
 		   "Why-Req-A: nolast=%u retx=%u slows=%u smtxw=%u\n",
 		   atomic_read(&rxnet->stat_why_req_ack[rxrpc_reqack_no_srv_last]),

Re: [PATCH net 0/5] rxrpc: Miscellaneous fixes

[Jakub Kicinski]

On Fri,  3 May 2024 16:07:38 +0100 David Howells wrote:
> Here some miscellaneous fixes for AF_RXRPC:
> 
>  (1) Fix the congestion control algorithm to start cwnd at 4 and to not cut
>      ssthresh when the peer cuts its rwind size.
> 
>  (2) Only transmit a single ACK for all the DATA packets glued together
>      into a jumbo packet to reduce the number of ACKs being generated.
> 
>  (3) Clean up the generation of flags in the protocol header when creating
>      a packet for transmission.  This means we don't carry the old
>      REQUEST-ACK bit around from previous transmissions, will make it
>      easier to fix the MORE-PACKETS flag and make it easier to do jumbo
>      packet assembly in future.
> 
>  (4) Fix how the MORE-PACKETS flag is driven.  We shouldn't be setting it
>      in sendmsg() as the packet is then queued and the bit is left in that
>      state, no matter how long it takes us to transmit the packet - and
>      will still be in that state if the packet is retransmitted.
> 
>  (5) Request an ACK on an impending transmission stall due to the app layer
>      not feeding us new data fast enough.  If we don't request an ACK, we
>      may have to hold on to the packet buffers for a significant amount of
>      time until the receiver gets bored and sends us an ACK anyway.

Looks like these got marked as Rejected in patchwork.
I think either because lore is confused and attaches an exchange with
DaveM from 2022 to them (?) or because I mentioned to DaveM that I'm
not sure these are fixes. So let me ask - on a scale of 1 to 10, how
convinced are you that these should go to Linus this week rather than
being categorized as general improvements and go during the merge
window (without the Fixes tags)?

Re: [PATCH net 0/5] rxrpc: Miscellaneous fixes

[Jeffrey Altman]

[-- Attachment #1: Type: text/plain, Size: 2071 bytes --]


> On May 7, 2024, at 8:44 PM, Jakub Kicinski <kuba@kernel.org> wrote:
> 
> On Fri,  3 May 2024 16:07:38 +0100 David Howells wrote:
>> Here some miscellaneous fixes for AF_RXRPC:
>> 
>> (1) Fix the congestion control algorithm to start cwnd at 4 and to not cut
>>  ssthresh when the peer cuts its rwind size.
>> 
>> (2) Only transmit a single ACK for all the DATA packets glued together
>>  into a jumbo packet to reduce the number of ACKs being generated.
>> 
>> (3) Clean up the generation of flags in the protocol header when creating
>>  a packet for transmission.  This means we don't carry the old
>>  REQUEST-ACK bit around from previous transmissions, will make it
>>  easier to fix the MORE-PACKETS flag and make it easier to do jumbo
>>  packet assembly in future.
>> 
>> (4) Fix how the MORE-PACKETS flag is driven.  We shouldn't be setting it
>>  in sendmsg() as the packet is then queued and the bit is left in that
>>  state, no matter how long it takes us to transmit the packet - and
>>  will still be in that state if the packet is retransmitted.
>> 
>> (5) Request an ACK on an impending transmission stall due to the app layer
>>  not feeding us new data fast enough.  If we don't request an ACK, we
>>  may have to hold on to the packet buffers for a significant amount of
>>  time until the receiver gets bored and sends us an ACK anyway.
> 
> Looks like these got marked as Rejected in patchwork.
> I think either because lore is confused and attaches an exchange with
> DaveM from 2022 to them (?) or because I mentioned to DaveM that I'm
> not sure these are fixes. So let me ask - on a scale of 1 to 10, how
> convinced are you that these should go to Linus this week rather than
> being categorized as general improvements and go during the merge
> window (without the Fixes tags)?

Jakub,

In my opinion, the first two patches in the series I believe are important to back port to the stable branches.

Reviewed-by: Jeffrey Altman <jaltman@auristor.com <mailto:jaltman@auristor.com>>

Jeffrey




[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3929 bytes --]

Re: [PATCH net 0/5] rxrpc: Miscellaneous fixes

[Jakub Kicinski]

On Wed, 8 May 2024 01:57:43 -0600 Jeffrey Altman wrote:
> > Looks like these got marked as Rejected in patchwork.
> > I think either because lore is confused and attaches an exchange with
> > DaveM from 2022 to them (?) or because I mentioned to DaveM that I'm
> > not sure these are fixes. So let me ask - on a scale of 1 to 10, how
> > convinced are you that these should go to Linus this week rather than
> > being categorized as general improvements and go during the merge
> > window (without the Fixes tags)?  
> 
> Jakub,
> 
> In my opinion, the first two patches in the series I believe are important to back port to the stable branches.
> 
> Reviewed-by: Jeffrey Altman <jaltman@auristor.com <mailto:jaltman@auristor.com>>

Are they regressions? Seems possible from the Fixes tag but unclear
from the text of the commit messages.

In any case, taking the first two may be a reasonable compromise.
Does it sounds good to you, David?

Re: [PATCH net 0/5] rxrpc: Miscellaneous fixes

[David Howells]

Jakub Kicinski <kuba@kernel.org> wrote:

> Looks like these got marked as Rejected in patchwork.
> I think either because lore is confused and attaches an exchange with
> DaveM from 2022 to them (?) or because I mentioned to DaveM that I'm
> not sure these are fixes. So let me ask - on a scale of 1 to 10, how
> convinced are you that these should go to Linus this week rather than
> being categorized as general improvements and go during the merge
> window (without the Fixes tags)?

Ah, sorry.  I marked them rejected as I put myself as cc: not S-o-b on one of
them, but then got distracted and didn't get around to reposting them.  And
Jeff mentioned that the use of the MORE-PACKETS flag is not exactly
consistent between various implementations.

So if you could take just the first two for the moment?

Thanks,
David

Re: [PATCH net 0/5] rxrpc: Miscellaneous fixes

[Jakub Kicinski]

On Wed, 08 May 2024 15:00:28 +0100 David Howells wrote:
> Jakub Kicinski <kuba@kernel.org> wrote:
> 
> > Looks like these got marked as Rejected in patchwork.
> > I think either because lore is confused and attaches an exchange with
> > DaveM from 2022 to them (?) or because I mentioned to DaveM that I'm
> > not sure these are fixes. So let me ask - on a scale of 1 to 10, how
> > convinced are you that these should go to Linus this week rather than
> > being categorized as general improvements and go during the merge
> > window (without the Fixes tags)?  
> 
> Ah, sorry.  I marked them rejected as I put myself as cc: not S-o-b on one of
> them, but then got distracted and didn't get around to reposting them.  And
> Jeff mentioned that the use of the MORE-PACKETS flag is not exactly
> consistent between various implementations.

Ah, mystery solved :)

> So if you could take just the first two for the moment?

Done!

Re: [PATCH net 0/5] rxrpc: Miscellaneous fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri,  3 May 2024 16:07:38 +0100 you wrote:
> Here some miscellaneous fixes for AF_RXRPC:
> 
>  (1) Fix the congestion control algorithm to start cwnd at 4 and to not cut
>      ssthresh when the peer cuts its rwind size.
> 
>  (2) Only transmit a single ACK for all the DATA packets glued together
>      into a jumbo packet to reduce the number of ACKs being generated.
> 
> [...]

Here is the summary with links:
  - [net,1/5] rxrpc: Fix congestion control algorithm
    https://git.kernel.org/netdev/net/c/ba4e103848d3
  - [net,2/5] rxrpc: Only transmit one ACK per jumbo packet received
    https://git.kernel.org/netdev/net/c/012b7206918d
  - [net,3/5] rxrpc: Clean up Tx header flags generation handling
    (no matching commit)
  - [net,4/5] rxrpc: Change how the MORE-PACKETS rxrpc wire header flag is driven
    (no matching commit)
  - [net,5/5] rxrpc: Request an ACK on impending Tx stall
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html