linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/6] rxrpc: Leak fixes
@ 2022-05-20 16:33 David Howells
  2022-05-20 16:33 ` [PATCH net 1/6] rxrpc: Enable IPv6 checksums on transport socket David Howells
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: David Howells @ 2022-05-20 16:33 UTC (permalink / raw)
  To: netdev
  Cc: Jeffrey Altman, linux-afs, Vadim Fedorenko, David S. Miller,
	Marc Dionne, Xin Long, dhowells, linux-afs, linux-kernel


Here are some fixes for AF_RXRPC:

 (1) Reenable IPv6 checksums on the UDP transport socket after the
     conversion to the UDP tunnel API disabled it.

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

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

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

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

 (6) 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).

The patches are tagged here:

	git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
	rxrpc-fixes-20220520

and can also be found on the following branch:

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

Tested-by: kafs-testing+fedora34_64checkkafs-build-495@auristor.com

David
---
David Howells (6):
      rxrpc: Enable IPv6 checksums on transport socket
      rxrpc: Fix listen() setting the bar too high for the prealloc rings
      rxrpc: Don't try to resend the request if we're receiving the reply
      rxrpc: Fix overlapping ACK accounting
      rxrpc: Don't let ack.previousPacket regress
      rxrpc: Fix decision on when to generate an IDLE ACK


 include/trace/events/rxrpc.h |  2 +-
 net/rxrpc/ar-internal.h      | 13 +++++++------
 net/rxrpc/call_event.c       |  3 ++-
 net/rxrpc/input.c            | 31 ++++++++++++++++++++-----------
 net/rxrpc/local_object.c     |  3 +++
 net/rxrpc/output.c           | 20 ++++++++++++--------
 net/rxrpc/recvmsg.c          |  8 +++-----
 net/rxrpc/sysctl.c           |  4 ++--
 8 files changed, 50 insertions(+), 34 deletions(-)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net 1/6] rxrpc: Enable IPv6 checksums on transport socket
  2022-05-20 16:33 [PATCH net 0/6] rxrpc: Leak fixes David Howells
@ 2022-05-20 16:33 ` David Howells
  2022-05-21  1:15   ` Jakub Kicinski
  2022-05-21  7:27   ` David Howells
  2022-05-20 16:33 ` [PATCH net 2/6] rxrpc: Fix listen() setting the bar too high for the prealloc rings David Howells
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 9+ messages in thread
From: David Howells @ 2022-05-20 16:33 UTC (permalink / raw)
  To: netdev
  Cc: Marc Dionne, Xin Long, Marc Dionne, Vadim Fedorenko,
	David S. Miller, linux-afs, dhowells, linux-afs, linux-kernel

AF_RXRPC doesn't currently enable IPv6 UDP Tx checksums on the transport
socket it opens and the checksums in the packets it generates end up 0.

It probably should also enable IPv6 UDP Rx checksums and IPv4 UDP
checksums.  The latter only seem to be applied if the socket family is
AF_INET and don't seem to apply if it's AF_INET6.  IPv4 packets from an
IPv6 socket seem to have checksums anyway.

What seems to have happened is that the inet_inv_convert_csum() call didn't
get converted to the appropriate udp_port_cfg parameters - and
udp_sock_create() disables checksums unless explicitly told not too.

Fix this by enabling the three udp_port_cfg checksum options.

Fixes: 1a9b86c9fd95 ("rxrpc: use udp tunnel APIs instead of open code in rxrpc_open_socket")
Reported-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Xin Long <lucien.xin@gmail.com>
Reviewed-by: Marc Dionne <marc.dionne@auristor.com>
cc: Vadim Fedorenko <vfedorenko@novek.ru>
cc: David S. Miller <davem@davemloft.net>
cc: linux-afs@lists.infradead.org
---

 net/rxrpc/local_object.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/rxrpc/local_object.c b/net/rxrpc/local_object.c
index a4111408ffd0..6a1611b0e303 100644
--- a/net/rxrpc/local_object.c
+++ b/net/rxrpc/local_object.c
@@ -117,6 +117,7 @@ static int rxrpc_open_socket(struct rxrpc_local *local, struct net *net)
 	       local, srx->transport_type, srx->transport.family);
 
 	udp_conf.family = srx->transport.family;
+	udp_conf.use_udp_checksums = true;
 	if (udp_conf.family == AF_INET) {
 		udp_conf.local_ip = srx->transport.sin.sin_addr;
 		udp_conf.local_udp_port = srx->transport.sin.sin_port;
@@ -124,6 +125,8 @@ static int rxrpc_open_socket(struct rxrpc_local *local, struct net *net)
 	} else {
 		udp_conf.local_ip6 = srx->transport.sin6.sin6_addr;
 		udp_conf.local_udp_port = srx->transport.sin6.sin6_port;
+		udp_conf.use_udp6_tx_checksums = true;
+		udp_conf.use_udp6_rx_checksums = true;
 #endif
 	}
 	ret = udp_sock_create(net, &udp_conf, &local->socket);



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net 2/6] rxrpc: Fix listen() setting the bar too high for the prealloc rings
  2022-05-20 16:33 [PATCH net 0/6] rxrpc: Leak fixes David Howells
  2022-05-20 16:33 ` [PATCH net 1/6] rxrpc: Enable IPv6 checksums on transport socket David Howells
@ 2022-05-20 16:33 ` David Howells
  2022-05-20 16:34 ` [PATCH net 3/6] rxrpc: Don't try to resend the request if we're receiving the reply David Howells
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2022-05-20 16:33 UTC (permalink / raw)
  To: netdev; +Cc: Marc Dionne, linux-afs, dhowells, linux-afs, linux-kernel

AF_RXRPC's listen() handler lets you set the backlog up to 32 (if you bump
up the sysctl), but whilst the preallocation circular buffers have 32 slots
in them, one of them has to be a dead slot because we're using CIRC_CNT().

This means that listen(rxrpc_sock, 32) will cause an oops when the socket
is closed because rxrpc_service_prealloc_one() allocated one too many calls
and rxrpc_discard_prealloc() won't then be able to get rid of them because
it'll think the ring is empty.  rxrpc_release_calls_on_socket() then tries
to abort them, but oopses because call->peer isn't yet set.

Fix this by setting the maximum backlog to RXRPC_BACKLOG_MAX - 1 to match
the ring capacity.

 BUG: kernel NULL pointer dereference, address: 0000000000000086
 ...
 RIP: 0010:rxrpc_send_abort_packet+0x73/0x240 [rxrpc]
 Call Trace:
  <TASK>
  ? __wake_up_common_lock+0x7a/0x90
  ? rxrpc_notify_socket+0x8e/0x140 [rxrpc]
  ? rxrpc_abort_call+0x4c/0x60 [rxrpc]
  rxrpc_release_calls_on_socket+0x107/0x1a0 [rxrpc]
  rxrpc_release+0xc9/0x1c0 [rxrpc]
  __sock_release+0x37/0xa0
  sock_close+0x11/0x20
  __fput+0x89/0x240
  task_work_run+0x59/0x90
  do_exit+0x319/0xaa0

Fixes: 00e907127e6f ("rxrpc: Preallocate peers, conns and calls for incoming service requests")
Reported-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-afs@lists.infradead.org
Link: https://lists.infradead.org/pipermail/linux-afs/2022-March/005079.html
---

 net/rxrpc/sysctl.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/rxrpc/sysctl.c b/net/rxrpc/sysctl.c
index 540351d6a5f4..555e0910786b 100644
--- a/net/rxrpc/sysctl.c
+++ b/net/rxrpc/sysctl.c
@@ -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",



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net 3/6] rxrpc: Don't try to resend the request if we're receiving the reply
  2022-05-20 16:33 [PATCH net 0/6] rxrpc: Leak fixes David Howells
  2022-05-20 16:33 ` [PATCH net 1/6] rxrpc: Enable IPv6 checksums on transport socket David Howells
  2022-05-20 16:33 ` [PATCH net 2/6] rxrpc: Fix listen() setting the bar too high for the prealloc rings David Howells
@ 2022-05-20 16:34 ` David Howells
  2022-05-20 16:34 ` [PATCH net 4/6] rxrpc: Fix overlapping ACK accounting David Howells
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2022-05-20 16:34 UTC (permalink / raw)
  To: netdev; +Cc: linux-afs, dhowells, linux-afs, linux-kernel

rxrpc has a timer to trigger resending of unacked data packets in a call.
This is not cancelled when a client call switches to the receive phase on
the basis that most calls don't last long enough for it to ever expire.
However, if it *does* expire after we've started to receive the reply, we
shouldn't then go into trying to retransmit or pinging the server to find
out if an ack got lost.

Fix this by skipping the resend code if we're into receiving the reply to a
client call.

Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-afs@lists.infradead.org
---

 net/rxrpc/call_event.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index 22e05de5d1ca..31761084a76f 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -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;
 	}



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net 4/6] rxrpc: Fix overlapping ACK accounting
  2022-05-20 16:33 [PATCH net 0/6] rxrpc: Leak fixes David Howells
                   ` (2 preceding siblings ...)
  2022-05-20 16:34 ` [PATCH net 3/6] rxrpc: Don't try to resend the request if we're receiving the reply David Howells
@ 2022-05-20 16:34 ` David Howells
  2022-05-20 16:34 ` [PATCH net 5/6] rxrpc: Don't let ack.previousPacket regress David Howells
  2022-05-20 16:34 ` [PATCH net 6/6] rxrpc: Fix decision on when to generate an IDLE ACK David Howells
  5 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2022-05-20 16:34 UTC (permalink / raw)
  To: netdev
  Cc: Jeffrey Altman, Marc Dionne, linux-afs, dhowells, linux-afs,
	linux-kernel

Fix accidental overlapping of Rx-phase ACK accounting with Tx-phase ACK
accounting through variables shared between the two.  call->acks_* members
refer to ACKs received in the Tx phase and call->ackr_* members to ACKs
sent/to be sent during the Rx phase.

Fixes: 1a2391c30c0b ("rxrpc: Fix detection of out of order acks")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---

 net/rxrpc/ar-internal.h |    7 ++++---
 net/rxrpc/input.c       |   16 ++++++++--------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 969e532f77a9..9a9688c41d4d 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -676,10 +676,9 @@ 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 */
@@ -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;	/* previous sequence number 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 */
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index dc201363f2c4..f11673cda217 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -812,7 +812,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 +820,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 +906,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 +922,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)



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net 5/6] rxrpc: Don't let ack.previousPacket regress
  2022-05-20 16:33 [PATCH net 0/6] rxrpc: Leak fixes David Howells
                   ` (3 preceding siblings ...)
  2022-05-20 16:34 ` [PATCH net 4/6] rxrpc: Fix overlapping ACK accounting David Howells
@ 2022-05-20 16:34 ` David Howells
  2022-05-20 16:34 ` [PATCH net 6/6] rxrpc: Fix decision on when to generate an IDLE ACK David Howells
  5 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2022-05-20 16:34 UTC (permalink / raw)
  To: netdev; +Cc: Marc Dionne, linux-afs, dhowells, linux-afs, linux-kernel

The previousPacket field in the rx ACK packet should never go backwards -
it's now the highest DATA sequence number received, not the last on
received (it used to be used for out of sequence detection).

Fixes: 248f219cb8bc ("rxrpc: Rewrite the data and ack handling code")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---

 net/rxrpc/ar-internal.h |    4 ++--
 net/rxrpc/input.c       |    4 +++-
 net/rxrpc/output.c      |    2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 9a9688c41d4d..8465985a4cb6 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -679,7 +679,7 @@ struct rxrpc_call {
 	/* 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_seq_t		ackr_prev_seq;	/* previous sequence number received */
+	rxrpc_seq_t		ackr_highest_seq; /* Higest sequence number received */
 	rxrpc_seq_t		ackr_consumed;	/* Highest packet shown consumed */
 	rxrpc_seq_t		ackr_seen;	/* Highest packet shown seen */
 
@@ -694,7 +694,7 @@ struct rxrpc_call {
 	/* 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;	/* previous 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 */
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index f11673cda217..2e61545ad8ca 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -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
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index a45c83f22236..46aae9b7006f 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -89,7 +89,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;



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net 6/6] rxrpc: Fix decision on when to generate an IDLE ACK
  2022-05-20 16:33 [PATCH net 0/6] rxrpc: Leak fixes David Howells
                   ` (4 preceding siblings ...)
  2022-05-20 16:34 ` [PATCH net 5/6] rxrpc: Don't let ack.previousPacket regress David Howells
@ 2022-05-20 16:34 ` David Howells
  5 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2022-05-20 16:34 UTC (permalink / raw)
  To: netdev; +Cc: Marc Dionne, linux-afs, dhowells, linux-afs, linux-kernel

Fix the decision on when to generate an IDLE ACK by keeping a count of the
number of packets we've received, but not yet soft-ACK'd, and the number of
packets we've processed, but not yet hard-ACK'd, rather than trying to keep
track of which DATA sequence numbers correspond to those points.

We then generate an ACK when either counter exceeds 2.  The counters are
both cleared when we transcribe the information into any sort of ACK packet
for transmission.  IDLE and DELAY ACKs are skipped if both counters are 0
(ie. no change).

Fixes: 805b21b929e2 ("rxrpc: Send an ACK after every few DATA packets we receive")
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 |    2 +-
 net/rxrpc/ar-internal.h      |    4 ++--
 net/rxrpc/input.c            |   11 +++++++++--
 net/rxrpc/output.c           |   18 +++++++++++-------
 net/rxrpc/recvmsg.c          |    8 +++-----
 5 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 4a3ab0ed6e06..1c714336b863 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -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",
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 8465985a4cb6..dce056adb78c 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -680,8 +680,8 @@ struct rxrpc_call {
 	u8			ackr_reason;	/* reason to ACK */
 	rxrpc_serial_t		ackr_serial;	/* serial of packet being ACK'd */
 	rxrpc_seq_t		ackr_highest_seq; /* Higest sequence number received */
-	rxrpc_seq_t		ackr_consumed;	/* Highest packet shown consumed */
-	rxrpc_seq_t		ackr_seen;	/* Highest packet shown seen */
+	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 */
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 2e61545ad8ca..1145cb14d86f 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -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;
@@ -569,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) {
@@ -588,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,
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index 46aae9b7006f..9683617db704 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -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);
@@ -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);
diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
index eca6dda26c77..250f23bc1c07 100644
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -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);



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net 1/6] rxrpc: Enable IPv6 checksums on transport socket
  2022-05-20 16:33 ` [PATCH net 1/6] rxrpc: Enable IPv6 checksums on transport socket David Howells
@ 2022-05-21  1:15   ` Jakub Kicinski
  2022-05-21  7:27   ` David Howells
  1 sibling, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2022-05-21  1:15 UTC (permalink / raw)
  To: David Howells
  Cc: netdev, Marc Dionne, Xin Long, Vadim Fedorenko, David S. Miller,
	linux-afs, linux-kernel

On Fri, 20 May 2022 17:33:48 +0100 David Howells wrote:
> AF_RXRPC doesn't currently enable IPv6 UDP Tx checksums on the transport
> socket it opens and the checksums in the packets it generates end up 0.
> 
> It probably should also enable IPv6 UDP Rx checksums and IPv4 UDP
> checksums.  The latter only seem to be applied if the socket family is
> AF_INET and don't seem to apply if it's AF_INET6.  IPv4 packets from an
> IPv6 socket seem to have checksums anyway.
> 
> What seems to have happened is that the inet_inv_convert_csum() call didn't
> get converted to the appropriate udp_port_cfg parameters - and
> udp_sock_create() disables checksums unless explicitly told not too.
> 
> Fix this by enabling the three udp_port_cfg checksum options.
> 
> Fixes: 1a9b86c9fd95 ("rxrpc: use udp tunnel APIs instead of open code in rxrpc_open_socket")
> Reported-by: Marc Dionne <marc.dionne@auristor.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Reviewed-by: Xin Long <lucien.xin@gmail.com>
> Reviewed-by: Marc Dionne <marc.dionne@auristor.com>

This is already in net..
pw build got gave up on this series.
Could you resend just the other 5 patches?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 1/6] rxrpc: Enable IPv6 checksums on transport socket
  2022-05-20 16:33 ` [PATCH net 1/6] rxrpc: Enable IPv6 checksums on transport socket David Howells
  2022-05-21  1:15   ` Jakub Kicinski
@ 2022-05-21  7:27   ` David Howells
  1 sibling, 0 replies; 9+ messages in thread
From: David Howells @ 2022-05-21  7:27 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: dhowells, netdev, Marc Dionne, Xin Long, Vadim Fedorenko,
	David S. Miller, linux-afs, linux-kernel

Jakub Kicinski <kuba@kernel.org> wrote:

> This is already in net..
> pw build got gave up on this series.
> Could you resend just the other 5 patches?

Will do.

David


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-05-21  7:27 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-20 16:33 [PATCH net 0/6] rxrpc: Leak fixes David Howells
2022-05-20 16:33 ` [PATCH net 1/6] rxrpc: Enable IPv6 checksums on transport socket David Howells
2022-05-21  1:15   ` Jakub Kicinski
2022-05-21  7:27   ` David Howells
2022-05-20 16:33 ` [PATCH net 2/6] rxrpc: Fix listen() setting the bar too high for the prealloc rings David Howells
2022-05-20 16:34 ` [PATCH net 3/6] rxrpc: Don't try to resend the request if we're receiving the reply David Howells
2022-05-20 16:34 ` [PATCH net 4/6] rxrpc: Fix overlapping ACK accounting David Howells
2022-05-20 16:34 ` [PATCH net 5/6] rxrpc: Don't let ack.previousPacket regress David Howells
2022-05-20 16:34 ` [PATCH net 6/6] rxrpc: Fix decision on when to generate an IDLE ACK David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).