[PATCH net-next 0/4] rxrpc: Miscellaneous changes

[PATCH net-next 0/4] rxrpc: Miscellaneous changes

[David Howells]

Here are some miscellaneous changes for rxrpc:

 (1) Use consume_skb() rather than kfree_skb_reason().

 (2) Fix unnecessary waking when poking and already-poked call.

 (3) Add ack.rwind to the rxrpc_tx_ack tracepoint as this indicates how
     many incoming DATA packets we're telling the peer that we are
     currently willing to accept on this call.

 (4) Reduce duplicate ACK transmission.  We send ACKs to let the peer know
     that we're increasing the receive window (ack.rwind) as we consume
     packets locally.  Normal ACK transmission is triggered in three places
     and that leads to duplicates being sent.

The patches are tagged here:

	git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git tags/rxrpc-next-20230208

And can be found on this branch:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-next

David

David Howells (4):
  rxrpc: Use consume_skb() rather than kfree_skb_reason()
  rxrpc: Fix overwaking on call poking
  rxrpc: Trace ack.rwind
  rxrpc: Reduce unnecessary ack transmission

 include/trace/events/rxrpc.h | 11 +++++++----
 net/rxrpc/call_object.c      |  6 ++++--
 net/rxrpc/conn_event.c       |  2 +-
 net/rxrpc/output.c           | 10 +++++++---
 net/rxrpc/recvmsg.c          |  2 +-
 net/rxrpc/skbuff.c           |  4 ++--
 6 files changed, 22 insertions(+), 13 deletions(-)

[PATCH net-next 1/4] rxrpc: Use consume_skb() rather than kfree_skb_reason()

[David Howells]

Use consume_skb() rather than kfree_skb_reason().

Reported-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---
 net/rxrpc/skbuff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/rxrpc/skbuff.c b/net/rxrpc/skbuff.c
index 944320e65ea8..3bcd6ee80396 100644
--- a/net/rxrpc/skbuff.c
+++ b/net/rxrpc/skbuff.c
@@ -63,7 +63,7 @@ void rxrpc_free_skb(struct sk_buff *skb, enum rxrpc_skb_trace why)
 	if (skb) {
 		int n = atomic_dec_return(select_skb_count(skb));
 		trace_rxrpc_skb(skb, refcount_read(&skb->users), n, why);
-		kfree_skb_reason(skb, SKB_CONSUMED);
+		consume_skb(skb);
 	}
 }
 
@@ -78,6 +78,6 @@ void rxrpc_purge_queue(struct sk_buff_head *list)
 		int n = atomic_dec_return(select_skb_count(skb));
 		trace_rxrpc_skb(skb, refcount_read(&skb->users), n,
 				rxrpc_skb_put_purge);
-		kfree_skb_reason(skb, SKB_CONSUMED);
+		consume_skb(skb);
 	}
 }

[PATCH net-next 2/4] rxrpc: Fix overwaking on call poking

[David Howells]

If an rxrpc call is given a poke, it will get woken up unconditionally,
even if there's already a poke pending (for which there will have been a
wake) or if the call refcount has gone to 0.

Fix this by only waking the call if it is still referenced and if it
doesn't already have a poke pending.

Fixes: 15f661dc95da ("rxrpc: Implement a mechanism to send an event notification to a call")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---
 net/rxrpc/call_object.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 6eaffb0d8fdc..e9f1f49d18c2 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -54,12 +54,14 @@ void rxrpc_poke_call(struct rxrpc_call *call, enum rxrpc_call_poke_trace what)
 		spin_lock_bh(&local->lock);
 		busy = !list_empty(&call->attend_link);
 		trace_rxrpc_poke_call(call, busy, what);
+		if (!busy && !rxrpc_try_get_call(call, rxrpc_call_get_poke))
+			busy = true;
 		if (!busy) {
-			rxrpc_get_call(call, rxrpc_call_get_poke);
 			list_add_tail(&call->attend_link, &local->call_attend_q);
 		}
 		spin_unlock_bh(&local->lock);
-		rxrpc_wake_up_io_thread(local);
+		if (!busy)
+			rxrpc_wake_up_io_thread(local);
 	}
 }
 

[PATCH net-next 3/4] rxrpc: Trace ack.rwind

[David Howells]

Log ack.rwind in the rxrpc_tx_ack tracepoint.  This value is useful to see
as it represents flow-control information to the peer.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---
 include/trace/events/rxrpc.h | 11 +++++++----
 net/rxrpc/conn_event.c       |  2 +-
 net/rxrpc/output.c           | 10 +++++++---
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index d7bb4acf4580..c3c0b0aa8381 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -1118,9 +1118,9 @@ TRACE_EVENT(rxrpc_tx_data,
 TRACE_EVENT(rxrpc_tx_ack,
 	    TP_PROTO(unsigned int call, rxrpc_serial_t serial,
 		     rxrpc_seq_t ack_first, rxrpc_serial_t ack_serial,
-		     u8 reason, u8 n_acks),
+		     u8 reason, u8 n_acks, u16 rwind),
 
-	    TP_ARGS(call, serial, ack_first, ack_serial, reason, n_acks),
+	    TP_ARGS(call, serial, ack_first, ack_serial, reason, n_acks, rwind),
 
 	    TP_STRUCT__entry(
 		    __field(unsigned int,	call)
@@ -1129,6 +1129,7 @@ TRACE_EVENT(rxrpc_tx_ack,
 		    __field(rxrpc_serial_t,	ack_serial)
 		    __field(u8,			reason)
 		    __field(u8,			n_acks)
+		    __field(u16,		rwind)
 			     ),
 
 	    TP_fast_assign(
@@ -1138,15 +1139,17 @@ TRACE_EVENT(rxrpc_tx_ack,
 		    __entry->ack_serial = ack_serial;
 		    __entry->reason = reason;
 		    __entry->n_acks = n_acks;
+		    __entry->rwind = rwind;
 			   ),
 
-	    TP_printk(" c=%08x ACK  %08x %s f=%08x r=%08x n=%u",
+	    TP_printk(" c=%08x ACK  %08x %s f=%08x r=%08x n=%u rw=%u",
 		      __entry->call,
 		      __entry->serial,
 		      __print_symbolic(__entry->reason, rxrpc_ack_names),
 		      __entry->ack_first,
 		      __entry->ack_serial,
-		      __entry->n_acks)
+		      __entry->n_acks,
+		      __entry->rwind)
 	    );
 
 TRACE_EVENT(rxrpc_receive,
diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
index 44414e724415..95f4bc206b3d 100644
--- a/net/rxrpc/conn_event.c
+++ b/net/rxrpc/conn_event.c
@@ -163,7 +163,7 @@ void rxrpc_conn_retransmit_call(struct rxrpc_connection *conn,
 		trace_rxrpc_tx_ack(chan->call_debug_id, serial,
 				   ntohl(pkt.ack.firstPacket),
 				   ntohl(pkt.ack.serial),
-				   pkt.ack.reason, 0);
+				   pkt.ack.reason, 0, rxrpc_rx_window_size);
 		break;
 
 	default:
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index 6b2022240076..5e53429c6922 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -80,7 +80,8 @@ static void rxrpc_set_keepalive(struct rxrpc_call *call)
  */
 static size_t rxrpc_fill_out_ack(struct rxrpc_connection *conn,
 				 struct rxrpc_call *call,
-				 struct rxrpc_txbuf *txb)
+				 struct rxrpc_txbuf *txb,
+				 u16 *_rwind)
 {
 	struct rxrpc_ackinfo ackinfo;
 	unsigned int qsize, sack, wrap, to;
@@ -124,6 +125,7 @@ static size_t rxrpc_fill_out_ack(struct rxrpc_connection *conn,
 	jmax = rxrpc_rx_jumbo_max;
 	qsize = (window - 1) - call->rx_consumed;
 	rsize = max_t(int, call->rx_winsize - qsize, 0);
+	*_rwind = rsize;
 	ackinfo.rxMTU		= htonl(rxrpc_rx_mtu);
 	ackinfo.maxMTU		= htonl(mtu);
 	ackinfo.rwind		= htonl(rsize);
@@ -190,6 +192,7 @@ int rxrpc_send_ack_packet(struct rxrpc_call *call, struct rxrpc_txbuf *txb)
 	rxrpc_serial_t serial;
 	size_t len, n;
 	int ret, rtt_slot = -1;
+	u16 rwind;
 
 	if (test_bit(RXRPC_CALL_DISCONNECTED, &call->flags))
 		return -ECONNRESET;
@@ -205,7 +208,7 @@ int rxrpc_send_ack_packet(struct rxrpc_call *call, struct rxrpc_txbuf *txb)
 	if (txb->ack.reason == RXRPC_ACK_PING)
 		txb->wire.flags |= RXRPC_REQUEST_ACK;
 
-	n = rxrpc_fill_out_ack(conn, call, txb);
+	n = rxrpc_fill_out_ack(conn, call, txb, &rwind);
 	if (n == 0)
 		return 0;
 
@@ -217,7 +220,8 @@ int rxrpc_send_ack_packet(struct rxrpc_call *call, struct rxrpc_txbuf *txb)
 	txb->wire.serial = htonl(serial);
 	trace_rxrpc_tx_ack(call->debug_id, serial,
 			   ntohl(txb->ack.firstPacket),
-			   ntohl(txb->ack.serial), txb->ack.reason, txb->ack.nAcks);
+			   ntohl(txb->ack.serial), txb->ack.reason, txb->ack.nAcks,
+			   rwind);
 
 	if (txb->ack.reason == RXRPC_ACK_PING)
 		rtt_slot = rxrpc_begin_rtt_probe(call, serial, rxrpc_rtt_tx_ping);

[PATCH net-next 4/4] rxrpc: Reduce unnecessary ack transmission

[David Howells]

rxrpc_recvmsg_data() schedules an ACK to be transmitted every time at least
two packets have been consumed and any time it runs out of data and would
return -EAGAIN to the caller.  Both events may occur within a single loop,
however, and if the I/O thread is quick enough it may send duplicate ACKs.

The ACKs are sent to inform the peer that more space has been made in the
local Rx window, but the I/O thread is going to send an ACK every couple of
DATA packets anyway, so we end up sending a lot more ACKs than we really
need to.

So reduce the rate at which recvmsg() schedules ACKs, such that if the I/O
thread sends ACKs at its normal faster rate, recvmsg() won't actually
schedule ACKs until the Rx flow stops (call->rx_consumed is cleared any
time we transmit an ACK for that call, resetting the counter used by
recvmsg).

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---
 net/rxrpc/recvmsg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
index 50d263a6359d..76eb2b9cd936 100644
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -137,7 +137,7 @@ static void rxrpc_rotate_rx_window(struct rxrpc_call *call)
 	/* Check to see if there's an ACK that needs sending. */
 	acked = atomic_add_return(call->rx_consumed - old_consumed,
 				  &call->ackr_nr_consumed);
-	if (acked > 2 &&
+	if (acked > 8 &&
 	    !test_and_set_bit(RXRPC_CALL_RX_IS_IDLE, &call->flags))
 		rxrpc_poke_call(call, rxrpc_call_poke_idle);
 }

Re: [PATCH net-next 0/4] rxrpc: Miscellaneous changes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David Howells <dhowells@redhat.com>:

On Wed,  8 Feb 2023 10:27:46 +0000 you wrote:
> Here are some miscellaneous changes for rxrpc:
> 
>  (1) Use consume_skb() rather than kfree_skb_reason().
> 
>  (2) Fix unnecessary waking when poking and already-poked call.
> 
>  (3) Add ack.rwind to the rxrpc_tx_ack tracepoint as this indicates how
>      many incoming DATA packets we're telling the peer that we are
>      currently willing to accept on this call.
> 
> [...]

Here is the summary with links:
  - [net-next,1/4] rxrpc: Use consume_skb() rather than kfree_skb_reason()
    https://git.kernel.org/netdev/net-next/c/16d5677ef104
  - [net-next,2/4] rxrpc: Fix overwaking on call poking
    https://git.kernel.org/netdev/net-next/c/a33395ab85b9
  - [net-next,3/4] rxrpc: Trace ack.rwind
    https://git.kernel.org/netdev/net-next/c/f789bff2deb3
  - [net-next,4/4] rxrpc: Reduce unnecessary ack transmission
    https://git.kernel.org/netdev/net-next/c/5a2c5a5b0829

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