Re: [PATCH net v12 06/15] rxrpc: Fix aborting in rxperf test server

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message does not describe what this patch actually
  changes.

--

rxrpc: Fix aborting in rxperf test server

This patch changes rxperf_process_call() so that any send error from
rxrpc_kernel_send_data() aborts the call. Previously it aborted only on
-ENOMEM. The new abort uses RXGEN_SS_MARSHAL and the new
rxperf_abort_send_error trace reason, and the rxperf_abort_oom trace symbol
is removed.

Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com

> rxrpc: Fix aborting in rxperf test server
>
> Fix rxperf_process_call() to always abort if it gets a send error rather
> than only aborting on ENOMEM.

[Severity: Low]
Does this description match what the patch changes by itself?

The subject and the Fixes: tag suggest that some send errors used to leave
the call un-aborted. That was not the case before this patch.

Both send sites in rxperf_process_call() returned the error to
rxperf_deliver_to_call(). That function already aborted the call for
nearly every error:

net/rxrpc/rxperf.c:rxperf_deliver_to_call() {
    ...
		ret = call->deliver(call);
		if (ret == 0)
			ret = rxperf_process_call(call);

		switch (ret) {
    ...
		case -ENODATA:
		case -EBADMSG:
		case -EMSGSIZE:
		case -ENOMEM:
		case -EFAULT:
			rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
						RXGEN_SS_UNMARSHAL, ret,
						rxperf_abort_unmarshal_error);
			goto call_complete;
		default:
			rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
						RX_CALL_DEAD, ret,
						rxperf_abort_general_error);
			goto call_complete;
		}
    ...
}

Only three errors skipped the abort:

  - -ECONNABORTED, because the call has already ended
  - -EINPROGRESS
  - -EAGAIN, which needs MSG_DONTWAIT or an expired send timeout, and
    rxperf uses neither

So at this commit, the visible changes seem to be:

  - The peer now sees the abort code RXGEN_SS_MARSHAL instead of
    RXGEN_SS_UNMARSHAL or RX_CALL_DEAD. This is because
    rxrpc_propose_abort() only acts on the first abort proposal.
  - The trace reason becomes "rxperf-send-error", and "rxperf-oom" is
    removed.
  - rxperf_deliver_to_call() then makes a second abort proposal, which has
    no effect.

The underlying goal is to make rxperf_process_call() the only place that
aborts on a send failure. That only seems to take effect with the later
commit "rxrpc: Fix error handling in rxrpc_send_data()", which makes
rxperf_process_call() return void.

Could the commit message mention the abort code change and the dependency
on that later commit? Otherwise, a stable backporter going by the Fixes:
tag may assume this patch alone fixes calls that were left un-aborted.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006133011.531806-1-dhowells%40redhat.com