We currently don't attempt to get the full asked for length even if MSG_WAITALL is set, if we get a partial receive. If we do see a partial receive, then just note how many bytes we did and return -EAGAIN to get it retried.
The iov is advanced appropriately for the vector based case, and we manually bump the buffer and remainder for the non-vector case.
Cc: stable@vger.kernel.org Reported-by: Constantine Gavrilov constantine.gavrilov@gmail.com Signed-off-by: Jens Axboe axboe@kernel.dk --- fs/io_uring.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/fs/io_uring.c b/fs/io_uring.c index f41d91ce1fd0..2cd67b4ff924 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -612,6 +612,7 @@ struct io_sr_msg { int msg_flags; int bgid; size_t len; + size_t done_io; };
struct io_open { @@ -5417,12 +5418,14 @@ static int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) if (req->ctx->compat) sr->msg_flags |= MSG_CMSG_COMPAT; #endif + sr->done_io = 0; return 0; }
static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags) { struct io_async_msghdr iomsg, *kmsg; + struct io_sr_msg *sr = &req->sr_msg; struct socket *sock; struct io_buffer *kbuf; unsigned flags; @@ -5465,6 +5468,10 @@ static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags) return io_setup_async_msg(req, kmsg); if (ret == -ERESTARTSYS) ret = -EINTR; + if (ret > 0 && flags & MSG_WAITALL) { + sr->done_io += ret; + return io_setup_async_msg(req, kmsg); + } req_set_fail(req); } else if ((flags & MSG_WAITALL) && (kmsg->msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) { req_set_fail(req); @@ -5474,6 +5481,10 @@ static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags) if (kmsg->free_iov) kfree(kmsg->free_iov); req->flags &= ~REQ_F_NEED_CLEANUP; + if (ret >= 0) + ret += sr->done_io; + else if (sr->done_io) + ret = sr->done_io; __io_req_complete(req, issue_flags, ret, io_put_kbuf(req, issue_flags)); return 0; } @@ -5524,12 +5535,22 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags) return -EAGAIN; if (ret == -ERESTARTSYS) ret = -EINTR; + if (ret > 0 && flags & MSG_WAITALL) { + sr->len -= ret; + sr->buf += ret; + sr->done_io += ret; + return -EAGAIN; + } req_set_fail(req); } else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) { out_free: req_set_fail(req); }
+ if (ret >= 0) + ret += sr->done_io; + else if (sr->done_io) + ret = sr->done_io; __io_req_complete(req, issue_flags, ret, io_put_kbuf(req, issue_flags)); return 0; }
On 3/23/22 15:39, Jens Axboe wrote:
We currently don't attempt to get the full asked for length even if MSG_WAITALL is set, if we get a partial receive. If we do see a partial receive, then just note how many bytes we did and return -EAGAIN to get it retried.
The iov is advanced appropriately for the vector based case, and we manually bump the buffer and remainder for the non-vector case.
How datagrams work with MSG_WAITALL? I highly doubt it coalesces 2+ packets to satisfy the length requirement (e.g. because it may move the address back into the userspace). I'm mainly afraid about breaking io_uring users who are using the flag just to fail links when there is not enough data in a packet.
On 3/23/22 2:13 PM, Pavel Begunkov wrote:
On 3/23/22 15:39, Jens Axboe wrote:
We currently don't attempt to get the full asked for length even if MSG_WAITALL is set, if we get a partial receive. If we do see a partial receive, then just note how many bytes we did and return -EAGAIN to get it retried.
The iov is advanced appropriately for the vector based case, and we manually bump the buffer and remainder for the non-vector case.
How datagrams work with MSG_WAITALL? I highly doubt it coalesces 2+ packets to satisfy the length requirement (e.g. because it may move the address back into the userspace). I'm mainly afraid about breaking io_uring users who are using the flag just to fail links when there is not enough data in a packet.
Yes was thinking that too, nothing is final yet.. Need to write a SOCK_STREAM test case.
On Wed, Mar 23, 2022 at 10:14 PM Pavel Begunkov asml.silence@gmail.com wrote:
On 3/23/22 15:39, Jens Axboe wrote:
We currently don't attempt to get the full asked for length even if MSG_WAITALL is set, if we get a partial receive. If we do see a partial receive, then just note how many bytes we did and return -EAGAIN to get it retried.
The iov is advanced appropriately for the vector based case, and we manually bump the buffer and remainder for the non-vector case.
How datagrams work with MSG_WAITALL? I highly doubt it coalesces 2+ packets to satisfy the length requirement (e.g. because it may move the address back into the userspace). I'm mainly afraid about breaking io_uring users who are using the flag just to fail links when there is not enough data in a packet.
-- Pavel Begunkov
Pavel:
Datagrams have message boundaries and the MSG_WAITALL flag does not make sense there. I believe it is ignored by receive code on daragram sockets. MSG_WAITALL makes sends only on stream sockets, like TCP. The manual page says "This flag has no effect for datagram sockets.".
On 3/23/22 20:45, Constantine Gavrilov wrote:
On Wed, Mar 23, 2022 at 10:14 PM Pavel Begunkov asml.silence@gmail.com wrote:
On 3/23/22 15:39, Jens Axboe wrote:
We currently don't attempt to get the full asked for length even if MSG_WAITALL is set, if we get a partial receive. If we do see a partial receive, then just note how many bytes we did and return -EAGAIN to get it retried.
The iov is advanced appropriately for the vector based case, and we manually bump the buffer and remainder for the non-vector case.
How datagrams work with MSG_WAITALL? I highly doubt it coalesces 2+ packets to satisfy the length requirement (e.g. because it may move the address back into the userspace). I'm mainly afraid about breaking io_uring users who are using the flag just to fail links when there is not enough data in a packet.
-- Pavel Begunkov
Pavel:
Datagrams have message boundaries and the MSG_WAITALL flag does not make sense there. I believe it is ignored by receive code on daragram sockets. MSG_WAITALL makes sends only on stream sockets, like TCP. The manual page says "This flag has no effect for datagram sockets.".
Missed the line this in mans, thanks, and it's exactly as expected. The problem is on the io_uring side where with the patch it might blindly do a second call into the network stack consuming 2+ packets.
On 3/23/22 2:52 PM, Pavel Begunkov wrote:
On 3/23/22 20:45, Constantine Gavrilov wrote:
On Wed, Mar 23, 2022 at 10:14 PM Pavel Begunkov asml.silence@gmail.com wrote:
On 3/23/22 15:39, Jens Axboe wrote:
We currently don't attempt to get the full asked for length even if MSG_WAITALL is set, if we get a partial receive. If we do see a partial receive, then just note how many bytes we did and return -EAGAIN to get it retried.
The iov is advanced appropriately for the vector based case, and we manually bump the buffer and remainder for the non-vector case.
How datagrams work with MSG_WAITALL? I highly doubt it coalesces 2+ packets to satisfy the length requirement (e.g. because it may move the address back into the userspace). I'm mainly afraid about breaking io_uring users who are using the flag just to fail links when there is not enough data in a packet.
-- Pavel Begunkov
Pavel:
Datagrams have message boundaries and the MSG_WAITALL flag does not make sense there. I believe it is ignored by receive code on daragram sockets. MSG_WAITALL makes sends only on stream sockets, like TCP. The manual page says "This flag has no effect for datagram sockets.".
Missed the line this in mans, thanks, and it's exactly as expected. The problem is on the io_uring side where with the patch it might blindly do a second call into the network stack consuming 2+ packets.
Right, it should not be applied for datagrams.
linux-stable-mirror@lists.linaro.org
-
Constantine Gavrilov -
Jens Axboe -
Pavel Begunkov