On 2/22/25 00:08, Jens Axboe wrote:
Just a few minor drive-by nits.
@@ -1250,6 +1251,12 @@ int io_recvzc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
zc->ifq = req->ctx->ifq;
if (!zc->ifq)
return -EINVAL;
+ zc->len = READ_ONCE(sqe->len);
+ if (zc->len == UINT_MAX)
+ return -EINVAL;
+ /* UINT_MAX means no limit on readlen */
+ if (!zc->len)
+ zc->len = UINT_MAX;
Why not just make UINT_MAX allowed, meaning no limit? Would avoid two
branches here, and as far as I can tell not really change anything in
terms of API niceness.
I think 0 goes better as a special uapi value. It doesn't alter the
uapi, and commonly understood as "no limits", which is the opposite
to the other option, especially since UINT_MAX is not a max value for
an unlimited request, I'd easily expect it to drive more than 4GB.
@@ -1269,6 +1276,7 @@ int io_recvzc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
int io_recvzc(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_recvzc *zc = io_kiocb_to_cmd(req, struct io_recvzc);
+ bool limit = zc->len != UINT_MAX;
struct socket *sock;
int ret;
Doesn't seem to be used?
@@ -1296,6 +1304,13 @@ int io_recvzc(struct io_kiocb *req, unsigned int issue_flags)
return IOU_OK;
}
+ if (zc->len == 0) {
+ io_req_set_res(req, 0, 0);
+
+ if (issue_flags & IO_URING_F_MULTISHOT)
+ return IOU_STOP_MULTISHOT;
+ return IOU_OK;
+ }
if (issue_flags & IO_URING_F_MULTISHOT)
return IOU_ISSUE_SKIP_COMPLETE;
return -EAGAIN;
Might be cleaner as:
ret = -EAGAIN;
if (!zc->len) {
io_req_set_res(req, 0, 0);
ret = -IOU_OK;
}
if (issue_flags & IO_URING_F_MULTISHOT)
return IOU_ISSUE_SKIP_COMPLETE;
return ret;
rather than duplicate the flag checking.
You missed IOU_STOP_MULTISHOT, but even without it separate error
codes for polling is already an utter mess to try be smart about it.
I'll try to clean it up, but that's orthogonal.
...
@@ -930,7 +937,7 @@ static int io_zcrx_tcp_recvmsg(struct io_kiocb *req, struct io_zcrx_ifq *ifq,
ret = IOU_REQUEUE;
} else if (sock_flag(sk, SOCK_DONE)) {
/* Make it to retry until it finally gets 0. */
- if (issue_flags & IO_URING_F_MULTISHOT)
+ if (!limit && (issue_flags & IO_URING_F_MULTISHOT))
ret = IOU_REQUEUE;
else
ret = -EAGAIN;
In the two above hunks, the limit checking feels a bit hackish. For
example, why is it related to multishot or not? I think the patch would
benefit from being split into separate patches for singleshot and length
I agree with the statement, but fwiw there are no single shots and
can't be that, the message is misleading.
support. Eg one that adds singleshot support, and then one that adds
length capping on top. That would make it much easier to reason about
hunks like the above one.
@@ -942,7 +949,7 @@ static int io_zcrx_tcp_recvmsg(struct io_kiocb *req, struct io_zcrx_ifq *ifq,
int io_zcrx_recv(struct io_kiocb *req, struct io_zcrx_ifq *ifq,
struct socket *sock, unsigned int flags,
- unsigned issue_flags)
+ unsigned issue_flags, unsigned int *len)
{
struct sock *sk = sock->sk;
const struct proto *prot = READ_ONCE(sk->sk_prot);
Shouldn't recv support it too?
There is no "msg" variant of the request, if that's what you mean.
--
Pavel Begunkov
