| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87f547b0-7826-b232-cd01-c879b6829951@grimberg.me>
Date: Wed, 21 Jun 2023 13:15:15 +0300
From: Sagi Grimberg <sagi@...mberg.me>
To: David Howells <dhowells@...hat.com>, netdev@...r.kernel.org
Cc: Alexander Duyck <alexander.duyck@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Willem de Bruijn <willemdebruijn.kernel@...il.com>,
David Ahern <dsahern@...nel.org>,
Matthew Wilcox <willy@...radead.org>,
Jens Axboe <axboe@...nel.dk>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org,
Willem de Bruijn <willemb@...gle.com>,
Keith Busch <kbusch@...nel.org>, Jens Axboe <axboe@...com>,
Christoph Hellwig <hch@....de>,
Chaitanya Kulkarni <kch@...dia.com>,
linux-nvme@...ts.infradead.org
Subject: Re: [PATCH net-next v3 10/18] nvme/host: Use
sendmsg(MSG_SPLICE_PAGES) rather then sendpage
One comment:
format for title in nvme-tcp is:
"nvme-tcp: ..." for host patches, and
"nvmet-tcp: ..." for target patches.
But this can be fixed up when applying the patch set.
Other than that, for both nvme patches:
Reviewed-by: Sagi Grimberg <sagi@...mberg.me>
What tree will this be going from btw?
On 6/20/23 17:53, David Howells wrote:
> When transmitting data, call down into TCP using a single sendmsg with
> MSG_SPLICE_PAGES to indicate that content should be spliced rather than
> performing several sendmsg and sendpage calls to transmit header, data
> pages and trailer.
>
> Signed-off-by: David Howells <dhowells@...hat.com>
> Tested-by: Sagi Grimberg <sagi@...mberg.me>
> Acked-by: Willem de Bruijn <willemb@...gle.com>
> cc: Keith Busch <kbusch@...nel.org>
> cc: Jens Axboe <axboe@...com>
> cc: Christoph Hellwig <hch@....de>
> cc: Chaitanya Kulkarni <kch@...dia.com>
> cc: "David S. Miller" <davem@...emloft.net>
> cc: Eric Dumazet <edumazet@...gle.com>
> cc: Jakub Kicinski <kuba@...nel.org>
> cc: Paolo Abeni <pabeni@...hat.com>
> cc: Jens Axboe <axboe@...nel.dk>
> cc: Matthew Wilcox <willy@...radead.org>
> cc: linux-nvme@...ts.infradead.org
> cc: netdev@...r.kernel.org
> ---
>
> Notes:
> ver #2)
> - Wrap lines at 80.
>
> ver #3)
> - Split nvme/host from nvme/target changes.
>
> drivers/nvme/host/tcp.c | 46 +++++++++++++++++++++--------------------
> 1 file changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index bf0230442d57..6f31cdbb696a 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -997,25 +997,25 @@ static int nvme_tcp_try_send_data(struct nvme_tcp_request *req)
> u32 h2cdata_left = req->h2cdata_left;
>
> while (true) {
> + struct bio_vec bvec;
> + struct msghdr msg = {
> + .msg_flags = MSG_DONTWAIT | MSG_SPLICE_PAGES,
> + };
> struct page *page = nvme_tcp_req_cur_page(req);
> size_t offset = nvme_tcp_req_cur_offset(req);
> size_t len = nvme_tcp_req_cur_length(req);
> bool last = nvme_tcp_pdu_last_send(req, len);
> int req_data_sent = req->data_sent;
> - int ret, flags = MSG_DONTWAIT;
> + int ret;
>
> if (last && !queue->data_digest && !nvme_tcp_queue_more(queue))
> - flags |= MSG_EOR;
> + msg.msg_flags |= MSG_EOR;
> else
> - flags |= MSG_MORE | MSG_SENDPAGE_NOTLAST;
> + msg.msg_flags |= MSG_MORE;
>
> - if (sendpage_ok(page)) {
> - ret = kernel_sendpage(queue->sock, page, offset, len,
> - flags);
> - } else {
> - ret = sock_no_sendpage(queue->sock, page, offset, len,
> - flags);
> - }
> + bvec_set_page(&bvec, page, len, offset);
> + iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1, len);
> + ret = sock_sendmsg(queue->sock, &msg);
> if (ret <= 0)
> return ret;
>
> @@ -1054,22 +1054,24 @@ static int nvme_tcp_try_send_cmd_pdu(struct nvme_tcp_request *req)
> {
> struct nvme_tcp_queue *queue = req->queue;
> struct nvme_tcp_cmd_pdu *pdu = nvme_tcp_req_cmd_pdu(req);
> + struct bio_vec bvec;
> + struct msghdr msg = { .msg_flags = MSG_DONTWAIT | MSG_SPLICE_PAGES, };
> bool inline_data = nvme_tcp_has_inline_data(req);
> u8 hdgst = nvme_tcp_hdgst_len(queue);
> int len = sizeof(*pdu) + hdgst - req->offset;
> - int flags = MSG_DONTWAIT;
> int ret;
>
> if (inline_data || nvme_tcp_queue_more(queue))
> - flags |= MSG_MORE | MSG_SENDPAGE_NOTLAST;
> + msg.msg_flags |= MSG_MORE;
> else
> - flags |= MSG_EOR;
> + msg.msg_flags |= MSG_EOR;
>
> if (queue->hdr_digest && !req->offset)
> nvme_tcp_hdgst(queue->snd_hash, pdu, sizeof(*pdu));
>
> - ret = kernel_sendpage(queue->sock, virt_to_page(pdu),
> - offset_in_page(pdu) + req->offset, len, flags);
> + bvec_set_virt(&bvec, (void *)pdu + req->offset, len);
> + iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1, len);
> + ret = sock_sendmsg(queue->sock, &msg);
> if (unlikely(ret <= 0))
> return ret;
>
> @@ -1093,6 +1095,8 @@ static int nvme_tcp_try_send_data_pdu(struct nvme_tcp_request *req)
> {
> struct nvme_tcp_queue *queue = req->queue;
> struct nvme_tcp_data_pdu *pdu = nvme_tcp_req_data_pdu(req);
> + struct bio_vec bvec;
> + struct msghdr msg = { .msg_flags = MSG_DONTWAIT | MSG_MORE, };
> u8 hdgst = nvme_tcp_hdgst_len(queue);
> int len = sizeof(*pdu) - req->offset + hdgst;
> int ret;
> @@ -1101,13 +1105,11 @@ static int nvme_tcp_try_send_data_pdu(struct nvme_tcp_request *req)
> nvme_tcp_hdgst(queue->snd_hash, pdu, sizeof(*pdu));
>
> if (!req->h2cdata_left)
> - ret = kernel_sendpage(queue->sock, virt_to_page(pdu),
> - offset_in_page(pdu) + req->offset, len,
> - MSG_DONTWAIT | MSG_MORE | MSG_SENDPAGE_NOTLAST);
> - else
> - ret = sock_no_sendpage(queue->sock, virt_to_page(pdu),
> - offset_in_page(pdu) + req->offset, len,
> - MSG_DONTWAIT | MSG_MORE);
> + msg.msg_flags |= MSG_SPLICE_PAGES;
> +
> + bvec_set_virt(&bvec, (void *)pdu + req->offset, len);
> + iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1, len);
> + ret = sock_sendmsg(queue->sock, &msg);
> if (unlikely(ret <= 0))
> return ret;
>
>
Powered by blists - more mailing lists