Re: [RFC PATCH net-next 07/12] io_uring/opaque: add IORING_OP_OPAQUE_OBJ_SEND

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] io_uring/opaque: missing IO_URING_F_NONBLOCK check in
  io_opaque_send()
--

commit 1e659b6cd96d02ac2bb983ff5c26c204fe0a5ef3
Author: Alex Markuze <amarkuze@redhat.com>

io_uring/opaque: add IORING_OP_OPAQUE_OBJ_SEND

This commit adds the IORING_OP_OPAQUE_OBJ_SEND operation to send an optional
prefix, an opaque object range, and an optional suffix to a TCP socket with
per-socket FIFO ordering across the store.

> diff --git a/io_uring/opaque.c b/io_uring/opaque.c
> index 81fbf2011446d..b62c4be92fead 100644
> --- a/io_uring/opaque.c
> +++ b/io_uring/opaque.c

[ ... ]

> @@ -1591,9 +1669,858 @@ void io_opaque_cleanup(struct io_kiocb *req)

[ ... ]

> +sent:
> +	if (ret > 0) {
> +		if (body)
> +			op->iter = msg.msg_iter;
> +		op->progress += ret;
> +		if (op->progress < op->total_length && !iov_iter_count(&msg.msg_iter)) {
> +			/*
> +			 * TCP lets go of the socket while it waits for room, and a
> +			 * protocol may have taken it over by then: ask again.
> +			 */
> +			if (owned && !io_opaque_send_direct(sk)) {
> +				release_sock(sk);
> +				io_ring_submit_lock(req->ctx, issue_flags);
> +				owned = false;
> +			}
> +			goto again;
> +		}
> +		if (op->progress < op->total_length && !(req->flags & REQ_F_NOWAIT)) {
> +			ret = -EAGAIN;
> +			goto leave;
> +		}
> +	} else if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK) &&
> +		   !(req->flags & REQ_F_NOWAIT)) {
> +		goto leave;
> +	}

[Severity: Medium]
Should the partial-send check in io_opaque_send() also test
(issue_flags & IO_URING_F_NONBLOCK), matching the ret == -EAGAIN branch
right below it?

When io_opaque_send() runs in blocking mode from an io-wq worker
(!(issue_flags & IO_URING_F_NONBLOCK) and !(req->flags & REQ_F_NOWAIT)),
tcp_sendmsg_locked() or sock_sendmsg() can return a partial byte count if
sk_stream_wait_memory() is interrupted by a signal, cancellation, socket
error, or SO_SNDTIMEO after some bytes have already been queued.
Overwriting ret with -EAGAIN and jumping to leave skips the finish path's
io_opaque_tx_put() and io_opaque_send_unpin_cheap() before release_sock(),
and returns -EAGAIN to io_wq_submit_work() while needs_poll is false:

io_uring/io_uring.c:io_wq_submit_work() {
    ...
		if (!needs_poll) {
			if (!(req->flags & REQ_F_IOPOLL))
				break;
    ...
	if (ret)
		io_req_task_queue_fail(req, ret);
}

Because opaque stores require IORING_SETUP_DEFER_TASKRUN, the abandoned
head request stays at the head of tx->requests with tx->failed still false
and its object version pinned until the ring's submitter task runs
task_work cleanup.  Until then, queued followers are not canceled and new
sends to the same socket from other rings sharing the store can still pass
io_opaque_tx_join() and consume IORING_OPAQUE_SEND_LAST handles.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110132006.3280986-1-amarkuze@redhat.com?part=7