rxrpc: Don't transmit DELAY ACKs immediately on proposal

Don't transmit a DELAY ACK immediately on proposal when the Rx window is
rotated, but rather defer it to the work function. This means that

rxrpc: Don't transmit DELAY ACKs immediately on proposal

Don't transmit a DELAY ACK immediately on proposal when the Rx window is
rotated, but rather defer it to the work function. This means that we have
a chance to queue/consume more received packets before we actually send the
DELAY ACK, or even cancel it entirely, thereby reducing the number of
packets transmitted.

We do, however, want to continue sending other types of packet immediately,
particularly REQUESTED ACKs, as they may be used for RTT calculation by the
other side.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Fix call timeouts

Fix the rxrpc call expiration timeouts and make them settable from
userspace. By analogy with other rx implementations, there should be three
timeouts:

(1) "Normal timeou

rxrpc: Fix call timeouts

Fix the rxrpc call expiration timeouts and make them settable from
userspace. By analogy with other rx implementations, there should be three
timeouts:

(1) "Normal timeout"

This is set for all calls and is triggered if we haven't received any
packets from the peer in a while. It is measured from the last time
we received any packet on that call. This is not reset by any
connection packets (such as CHALLENGE/RESPONSE packets).

If a service operation takes a long time, the server should generate
PING ACKs at a duration that's substantially less than the normal
timeout so is to keep both sides alive. This is set at 1/6 of normal
timeout.

(2) "Idle timeout"

This is set only for a service call and is triggered if we stop
receiving the DATA packets that comprise the request data. It is
measured from the last time we received a DATA packet.

(3) "Hard timeout"

This can be set for a call and specified the maximum lifetime of that
call. It should not be specified by default. Some operations (such
as volume transfer) take a long time.

Allow userspace to set/change the timeouts on a call with sendmsg, using a
control message:

RXRPC_SET_CALL_TIMEOUTS

The data to the message is a number of 32-bit words, not all of which need
be given:

u32 hard_timeout; /* sec from first packet */
u32 idle_timeout; /* msec from packet Rx */
u32 normal_timeout; /* msec from data Rx */

This can be set in combination with any other sendmsg() that affects a
call.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Delay terminal ACK transmission on a client call

Delay terminal ACK transmission on a client call by deferring it to the
connection processor. This allows it to be skipped if we can send the

rxrpc: Delay terminal ACK transmission on a client call

Delay terminal ACK transmission on a client call by deferring it to the
connection processor. This allows it to be skipped if we can send the next
call instead, the first DATA packet of which will implicitly ack this call.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Lock around calling a kernel service Rx notification

Place a spinlock around the invocation of call->notify_rx() for a kernel
service call and lock again when ending the call and replace the

rxrpc: Lock around calling a kernel service Rx notification

Place a spinlock around the invocation of call->notify_rx() for a kernel
service call and lock again when ending the call and replace the
notification pointer with a pointer to a dummy function.

This is required because it's possible for rxrpc_notify_socket() to be
called after the call has been ended by the kernel service if called from
the asynchronous work function rxrpc_process_call().

However, rxrpc_notify_socket() currently only holds the RCU read lock when
invoking ->notify_rx(), which means that the afs_call struct would need to
be disposed of by call_rcu() rather than by kfree().

But we shouldn't see any notifications from a call after calling
rxrpc_kernel_end_call(), so a lock is required in rxrpc code.

Without this, we may see the call wait queue as having a corrupt spinlock:

BUG: spinlock bad magic on CPU#0, kworker/0:2/1612
general protection fault: 0000 [#1] SMP
...
Workqueue: krxrpcd rxrpc_process_call
task: ffff88040b83c400 task.stack: ffff88040adfc000
RIP: 0010:spin_bug+0x161/0x18f
RSP: 0018:ffff88040adffcc0 EFLAGS: 00010002
RAX: 0000000000000032 RBX: 6b6b6b6b6b6b6b6b RCX: ffffffff81ab16cf
RDX: ffff88041fa14c01 RSI: ffff88041fa0ccb8 RDI: ffff88041fa0ccb8
RBP: ffff88040adffcd8 R08: 00000000ffffffff R09: 00000000ffffffff
R10: ffff88040adffc60 R11: 000000000000022c R12: ffff88040aca2208
R13: ffffffff81a58114 R14: 0000000000000000 R15: 0000000000000000
....
Call Trace:
do_raw_spin_lock+0x1d/0x89
_raw_spin_lock_irqsave+0x3d/0x49
? __wake_up_common_lock+0x4c/0xa7
__wake_up_common_lock+0x4c/0xa7
? __lock_is_held+0x47/0x7a
__wake_up+0xe/0x10
afs_wake_up_call_waiter+0x11b/0x122 [kafs]
rxrpc_notify_socket+0x12b/0x258
rxrpc_process_call+0x18e/0x7d0
process_one_work+0x298/0x4de
? rescuer_thread+0x280/0x280
worker_thread+0x1d1/0x2ae
? rescuer_thread+0x280/0x280
kthread+0x12c/0x134
? kthread_create_on_node+0x3a/0x3a
ret_from_fork+0x27/0x40

In this case, note the corrupt data in EBX. The address of the offending
afs_call is in R12, plus the offset to the spinlock.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Support service upgrade from a kernel service

Provide support for a kernel service to make use of the service upgrade
facility. This involves:

(1) Pass an upgrade request flag to rxrpc_ker

rxrpc: Support service upgrade from a kernel service

Provide support for a kernel service to make use of the service upgrade
facility. This involves:

(1) Pass an upgrade request flag to rxrpc_kernel_begin_call().

(2) Make rxrpc_kernel_recv_data() return the call's current service ID so
that the caller can detect service upgrade and see what the service
was upgraded to.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Separate the connection's protocol service ID from the lookup ID

Keep the rxrpc_connection struct's idea of the service ID that is exposed
in the protocol separate from the service ID that's

rxrpc: Separate the connection's protocol service ID from the lookup ID

Keep the rxrpc_connection struct's idea of the service ID that is exposed
in the protocol separate from the service ID that's used as a lookup key.

This allows the protocol service ID on a client connection to get upgraded
without making the connection unfindable for other client calls that also
would like to use the upgraded connection.

The connection's actual service ID is then returned through recvmsg() by
way of msg_name.

Whilst we're at it, we get rid of the last_service_id field from each
channel. The service ID is per-connection, not per-call and an entire
connection is upgraded in one go.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Trace protocol errors in received packets

Add a tracepoint (rxrpc_rx_proto) to record protocol errors in received
packets. The following changes are made:

(1) Add a function, __rxrpc_abort

rxrpc: Trace protocol errors in received packets

Add a tracepoint (rxrpc_rx_proto) to record protocol errors in received
packets. The following changes are made:

(1) Add a function, __rxrpc_abort_eproto(), to note a protocol error on a
call and mark the call aborted. This is wrapped by
rxrpc_abort_eproto() that makes the why string usable in trace.

(2) Add trace_rxrpc_rx_proto() or rxrpc_abort_eproto() to protocol error
generation points, replacing rxrpc_abort_call() with the latter.

(3) Only send an abort packet in rxkad_verify_packet*() if we actually
managed to abort the call.

Note that a trace event is also emitted if a kernel user (e.g. afs) tries
to send data through a call when it's not in the transmission phase, though
it's not technically a receive event.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Use negative error codes in rxrpc_call struct

Use negative error codes in struct rxrpc_call::error because that's what
the kernel normally deals with and to make the code consistent. We only

rxrpc: Use negative error codes in rxrpc_call struct

Use negative error codes in struct rxrpc_call::error because that's what
the kernel normally deals with and to make the code consistent. We only
turn them positive when transcribing into a cmsg for userspace recvmsg.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Call state should be read with READ_ONCE() under some circumstances

The call state may be changed at any time by the data-ready routine in
response to received packets, so if the call state i

rxrpc: Call state should be read with READ_ONCE() under some circumstances

The call state may be changed at any time by the data-ready routine in
response to received packets, so if the call state is to be read and acted
upon several times in a function, READ_ONCE() must be used unless the call
state lock is held.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

show more ...

sched/headers: Prepare to move signal wakeup & sigpending methods from <linux/sched.h> into <linux/sched/signal.h>

Fix up affected files that include this signal functionality via sched.h.

Acked-by

sched/headers: Prepare to move signal wakeup & sigpending methods from <linux/sched.h> into <linux/sched/signal.h>

Fix up affected files that include this signal functionality via sched.h.

Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>

show more ...

rxrpc: Fix deadlock between call creation and sendmsg/recvmsg

All the routines by which rxrpc is accessed from the outside are serialised
by means of the socket lock (sendmsg, recvmsg, bind,
rxrpc_k

rxrpc: Fix deadlock between call creation and sendmsg/recvmsg

All the routines by which rxrpc is accessed from the outside are serialised
by means of the socket lock (sendmsg, recvmsg, bind,
rxrpc_kernel_begin_call(), ...) and this presents a problem:

(1) If a number of calls on the same socket are in the process of
connection to the same peer, a maximum of four concurrent live calls
are permitted before further calls need to wait for a slot.

(2) If a call is waiting for a slot, it is deep inside sendmsg() or
rxrpc_kernel_begin_call() and the entry function is holding the socket
lock.

(3) sendmsg() and recvmsg() or the in-kernel equivalents are prevented
from servicing the other calls as they need to take the socket lock to
do so.

(4) The socket is stuck until a call is aborted and makes its slot
available to the waiter.

Fix this by:

(1) Provide each call with a mutex ('user_mutex') that arbitrates access
by the users of rxrpc separately for each specific call.

(2) Make rxrpc_sendmsg() and rxrpc_recvmsg() unlock the socket as soon as
they've got a call and taken its mutex.

Note that I'm returning EWOULDBLOCK from recvmsg() if MSG_DONTWAIT is
set but someone else has the lock. Should I instead only return
EWOULDBLOCK if there's nothing currently to be done on a socket, and
sleep in this particular instance because there is something to be
done, but we appear to be blocked by the interrupt handler doing its
ping?

(3) Make rxrpc_new_client_call() unlock the socket after allocating a new
call, locking its user mutex and adding it to the socket's call tree.
The call is returned locked so that sendmsg() can add data to it
immediately.

From the moment the call is in the socket tree, it is subject to
access by sendmsg() and recvmsg() - even if it isn't connected yet.

(4) Lock new service calls in the UDP data_ready handler (in
rxrpc_new_incoming_call()) because they may already be in the socket's
tree and the data_ready handler makes them live immediately if a user
ID has already been preassigned.

Note that the new call is locked before any notifications are sent
that it is live, so doing mutex_trylock() *ought* to always succeed.
Userspace is prevented from doing sendmsg() on calls that are in a
too-early state in rxrpc_do_sendmsg().

(5) Make rxrpc_new_incoming_call() return the call with the user mutex
held so that a ping can be scheduled immediately under it.

Note that it might be worth moving the ping call into
rxrpc_new_incoming_call() and then we can drop the mutex there.

(6) Make rxrpc_accept_call() take the lock on the call it is accepting and
release the socket after adding the call to the socket's tree. This
is slightly tricky as we've dequeued the call by that point and have
to requeue it.

Note that requeuing emits a trace event.

(7) Make rxrpc_kernel_send_data() and rxrpc_kernel_recv_data() take the
new mutex immediately and don't bother with the socket mutex at all.

This patch has the nice bonus that calls on the same socket are now to some
extent parallelisable.

Note that we might want to move rxrpc_service_prealloc() calls out from the
socket lock and give it its own lock, so that we don't hang progress in
other calls because we're waiting for the allocator.

We probably also want to avoid calling rxrpc_notify_socket() from within
the socket lock (rxrpc_accept_call()).

Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Marc Dionne <marc.c.dionne@auristor.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

show more ...

rxrpc: Kernel calls get stuck in recvmsg

Calls made through the in-kernel interface can end up getting stuck because
of a missed variable update in a loop in rxrpc_recvmsg_data(). The problem
is li

rxrpc: Kernel calls get stuck in recvmsg

Calls made through the in-kernel interface can end up getting stuck because
of a missed variable update in a loop in rxrpc_recvmsg_data(). The problem
is like this:

(1) A new packet comes in and doesn't cause a notification to be given to
the client as there's still another packet in the ring - the
assumption being that if the client will keep drawing off data until
the ring is empty.

(2) The client is in rxrpc_recvmsg_data(), inside the big while loop that
iterates through the packets. This copies the window pointers into
variables rather than using the information in the call struct
because:

(a) MSG_PEEK might be in effect;

(b) we need a barrier after reading call->rx_top to pair with the
barrier in the softirq routine that loads the buffer.

(3) The reading of call->rx_top is done outside of the loop, and top is
never updated whilst we're in the loop. This means that even through
there's a new packet available, we don't see it and may return -EFAULT
to the caller - who will happily return to the scheduler and await the
next notification.

(4) No further notifications are forthcoming until there's an abort as the
ring isn't empty.

The fix is to move the read of call->rx_top inside the loop - but it needs
to be done before the condition is checked.

Reported-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

show more ...

rxrpc: Need to produce an ACK for service op if op takes a long time

We need to generate a DELAY ACK from the service end of an operation if we
start doing the actual operation work and it takes lon

rxrpc: Need to produce an ACK for service op if op takes a long time

We need to generate a DELAY ACK from the service end of an operation if we
start doing the actual operation work and it takes longer than expected.
This will hard-ACK the request data and allow the client to release its
resources.

To make this work:

(1) We have to set the ack timer and propose an ACK when the call moves to
the RXRPC_CALL_SERVER_ACK_REQUEST and clear the pending ACK and cancel
the timer when we start transmitting the reply (the first DATA packet
of the reply implicitly ACKs the request phase).

(2) It must be possible to set the timer when the caller is holding
call->state_lock, so split the lock-getting part of the timer function
out.

(3) Add trace notes for the ACK we're requesting and the timer we clear.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Return negative error code to kernel service

In rxrpc_kernel_recv_data(), when we return the error number incurred by a
failed call, we must negate it before returning it as it's stored as
po

rxrpc: Return negative error code to kernel service

In rxrpc_kernel_recv_data(), when we return the error number incurred by a
failed call, we must negate it before returning it as it's stored as
positive (that's what we have to pass back to userspace).

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Fix loss of PING RESPONSE ACK production due to PING ACKs

Separate the output of PING ACKs from the output of other sorts of ACK so
that if we receive a PING ACK and schedule transmission of

rxrpc: Fix loss of PING RESPONSE ACK production due to PING ACKs

Separate the output of PING ACKs from the output of other sorts of ACK so
that if we receive a PING ACK and schedule transmission of a PING RESPONSE
ACK, the response doesn't get cancelled by a PING ACK we happen to be
scheduling transmission of at the same time.

If a PING RESPONSE gets lost, the other side might just sit there waiting
for it and refuse to proceed otherwise.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Fix warning by splitting rxrpc_send_call_packet()

Split rxrpc_send_data_packet() to separate ACK generation (which is more
complicated) from ABORT generation. This simplifies the code a bit

rxrpc: Fix warning by splitting rxrpc_send_call_packet()

Split rxrpc_send_data_packet() to separate ACK generation (which is more
complicated) from ABORT generation. This simplifies the code a bit and
fixes the following warning:

In file included from ../net/rxrpc/output.c:20:0:
net/rxrpc/output.c: In function 'rxrpc_send_call_packet':
net/rxrpc/ar-internal.h:1187:27: error: 'top' may be used uninitialized in this function [-Werror=maybe-uninitialized]
net/rxrpc/output.c:103:24: note: 'top' was declared here
net/rxrpc/output.c:225:25: error: 'hard_ack' may be used uninitialized in this function [-Werror=maybe-uninitialized]

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: The offset field in struct rxrpc_skb_priv is unnecessary

The offset field in struct rxrpc_skb_priv is unnecessary as the value can
always be calculated.

Signed-off-by: David Howells <dhowell

rxrpc: The offset field in struct rxrpc_skb_priv is unnecessary

The offset field in struct rxrpc_skb_priv is unnecessary as the value can
always be calculated.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Include the last reply DATA serial number in the final ACK

In a client call, include the serial number of the last DATA packet of the
reply in the final ACK.

Signed-off-by: David Howells <dh

rxrpc: Include the last reply DATA serial number in the final ACK

In a client call, include the serial number of the last DATA packet of the
reply in the final ACK.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Send an ACK after every few DATA packets we receive

Send an ACK if we haven't sent one for the last two packets we've received.
This keeps the other end apprised of where we've got to - which

rxrpc: Send an ACK after every few DATA packets we receive

Send an ACK if we haven't sent one for the last two packets we've received.
This keeps the other end apprised of where we've got to - which is
important if they're doing slow-start.

We do this in recvmsg so that we can dispatch a packet directly without the
need to wake up the background thread.

This should possibly be made configurable in future.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Add tracepoint for ACK proposal

Add a tracepoint to log proposed ACKs, including whether the proposal is
used to update a pending ACK or is discarded in favour of an easlier,
higher priority

rxrpc: Add tracepoint for ACK proposal

Add a tracepoint to log proposed ACKs, including whether the proposal is
used to update a pending ACK or is discarded in favour of an easlier,
higher priority ACK.

Whilst we're at it, get rid of the rxrpc_acks() function and access the
name array directly. We do, however, need to validate the ACK reason
number given to trace_rxrpc_rx_ack() to make sure we don't overrun the
array.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Don't send an ACK at the end of service call response transmission

Don't send an IDLE ACK at the end of the transmission of the response to a
service call. The service end resends DATA packe

rxrpc: Don't send an ACK at the end of service call response transmission

Don't send an IDLE ACK at the end of the transmission of the response to a
service call. The service end resends DATA packets until the client sends an
ACK that hard-acks all the send data. At that point, the call is complete.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Improve skb tracing

Improve sk_buff tracing within AF_RXRPC by the following means:

(1) Use an enum to note the event type rather than plain integers and use
an array of event names ra

rxrpc: Improve skb tracing

Improve sk_buff tracing within AF_RXRPC by the following means:

(1) Use an enum to note the event type rather than plain integers and use
an array of event names rather than a big multi ?: list.

(2) Distinguish Rx from Tx packets and account them separately. This
requires the call phase to be tracked so that we know what we might
find in rxtx_buffer[].

(3) Add a parameter to rxrpc_{new,see,get,free}_skb() to indicate the
event type.

(4) A pair of 'rotate' events are added to indicate packets that are about
to be rotated out of the Rx and Tx windows.

(5) A pair of 'lost' events are added, along with rxrpc_lose_skb() for
packet loss injection recording.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Remove printks from rxrpc_recvmsg_data() to fix uninit var

Remove _enter/_debug/_leave calls from rxrpc_recvmsg_data() of which one
uses an uninitialised variable.

Signed-off-by: David Howel

rxrpc: Remove printks from rxrpc_recvmsg_data() to fix uninit var

Remove _enter/_debug/_leave calls from rxrpc_recvmsg_data() of which one
uses an uninitialised variable.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...

rxrpc: Add a tracepoint to follow what recvmsg does

Add a tracepoint to follow what recvmsg does within AF_RXRPC.

Signed-off-by: David Howells <dhowells@redhat.com>

rxrpc: Add a tracepoint to follow packets in the Rx buffer

Add a tracepoint to follow the life of packets that get added to a call's
receive buffer.

Signed-off-by: David Howells <dhowells@redhat.co

rxrpc: Add a tracepoint to follow packets in the Rx buffer

Add a tracepoint to follow the life of packets that get added to a call's
receive buffer.

Signed-off-by: David Howells <dhowells@redhat.com>

show more ...