From: Aurelien Aptel <aaptel@nvidia•com>
To: Sagi Grimberg <sagi@grimberg•me>,
linux-nvme@lists•infradead.org, netdev@vger•kernel.org,
hch@lst•de, kbusch@kernel•org, axboe@fb•com,
chaitanyak@nvidia•com, davem@davemloft•net, kuba@kernel•org
Cc: Boris Pismenny <borisp@nvidia•com>,
aurelien.aptel@gmail•com, smalin@nvidia•com, malin1024@gmail•com,
ogerlitz@nvidia•com, yorayz@nvidia•com, galshalom@nvidia•com,
mgurtovoy@nvidia•com
Subject: Re: [PATCH v12 08/26] nvme-tcp: Add DDP data-path
Date: Mon, 14 Aug 2023 19:12:47 +0300 [thread overview]
Message-ID: <2535y5hwqkg.fsf@nvidia.com> (raw)
In-Reply-To: <1d5adbe9-dcab-5eae-fff3-631b91c2da94@grimberg.me>
Sagi Grimberg <sagi@grimberg•me> writes:
> On 7/12/23 19:14, Aurelien Aptel wrote:
>> +static int nvme_tcp_req_map_ddp_sg(struct nvme_tcp_request *req, struct request *rq)
>
> Why do you pass both req and rq? You can derive each from the other.
Thanks, we will remove the redundant parameter.
>> +{
>> + int ret;
>> +
>> + req->ddp.sg_table.sgl = req->ddp.first_sgl;
>> + ret = sg_alloc_table_chained(&req->ddp.sg_table,
>> + blk_rq_nr_phys_segments(rq),
>> + req->ddp.sg_table.sgl, SG_CHUNK_SIZE);
>> + if (ret)
>> + return -ENOMEM;
>> + req->ddp.nents = blk_rq_map_sg(rq->q, rq, req->ddp.sg_table.sgl);
>
> General question, I'm assuming that the hca knows how to deal with
> a controller that sends c2hdata in parts?
Yes, the hardware supports the offloading of multiple c2hdata PDUs per IO.
>> +static int nvme_tcp_setup_ddp(struct nvme_tcp_queue *queue, u16 command_id,
>> + struct request *rq)
>
> I think you can use nvme_cid(rq) instead of passing the command_id.
Thanks, we will use it.
>> +{
>> + struct net_device *netdev = queue->ctrl->offloading_netdev;
>> + struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);
>> + int ret;
>> +
>> + if (rq_data_dir(rq) != READ ||
>> + queue->ctrl->offload_io_threshold > blk_rq_payload_bytes(rq))
>> + return 0;
>> +
>> + req->ddp.command_id = command_id;
>> + ret = nvme_tcp_req_map_ddp_sg(req, rq);
>
> Don't see why map_ddp_sg is not open-coded here, its the only call-site,
> and its pretty much does exactly what its called.
Sure, we will open code it.
>> @@ -1308,6 +1407,15 @@ static int nvme_tcp_try_send_cmd_pdu(struct nvme_tcp_request *req)
>> else
>> msg.msg_flags |= MSG_EOR;
>>
>> + if (test_bit(NVME_TCP_Q_OFF_DDP, &queue->flags)) {
>> + ret = nvme_tcp_setup_ddp(queue, pdu->cmd.common.command_id,
>> + blk_mq_rq_from_pdu(req));
>> + WARN_ONCE(ret, "ddp setup failed (queue 0x%x, cid 0x%x, ret=%d)",
>> + nvme_tcp_queue_id(queue),
>> + pdu->cmd.common.command_id,
>> + ret);
>> + }
>
> Any reason why this is done here when sending the command pdu and not
> in setup time?
We wish to interact with the HW from the same CPU per queue, hence we
are calling setup_ddp() after queue->io_cpu == raw_smp_processor_id()
was checked in nvme_tcp_queue_request().
next prev parent reply other threads:[~2023-08-14 16:12 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-12 16:14 [PATCH v12 00/26] nvme-tcp receive offloads Aurelien Aptel
2023-07-12 16:14 ` [PATCH v12 01/26] net: Introduce direct data placement tcp offload Aurelien Aptel
2023-08-09 7:15 ` Sagi Grimberg
2023-08-10 14:46 ` Aurelien Aptel
2023-07-12 16:14 ` [PATCH v12 02/26] net/ethtool: add new stringset ETH_SS_ULP_DDP_{CAPS,STATS} Aurelien Aptel
2023-07-12 16:14 ` [PATCH v12 03/26] net/ethtool: add ULP_DDP_{GET,SET} operations for caps and stats Aurelien Aptel
2023-07-15 10:14 ` Simon Horman
2023-07-17 9:45 ` Aurelien Aptel
2023-07-12 16:14 ` [PATCH v12 04/26] Documentation: document netlink ULP_DDP_GET/SET messages Aurelien Aptel
2023-07-15 10:17 ` Simon Horman
2023-07-17 9:47 ` Aurelien Aptel
2023-07-12 16:14 ` [PATCH v12 05/26] iov_iter: skip copy if src == dst for direct data placement Aurelien Aptel
2023-08-16 0:24 ` Max Gurtovoy
2023-07-12 16:14 ` [PATCH v12 06/26] net/tls,core: export get_netdev_for_sock Aurelien Aptel
2023-07-12 16:14 ` [PATCH v12 07/26] nvme-tcp: Add DDP offload control path Aurelien Aptel
2023-08-01 2:25 ` Chaitanya Kulkarni
2023-08-09 7:39 ` Sagi Grimberg
2023-08-11 5:28 ` Chaitanya Kulkarni
2023-08-16 0:50 ` Max Gurtovoy
2023-08-09 7:13 ` Sagi Grimberg
2023-08-14 16:11 ` Aurelien Aptel
2023-08-14 18:54 ` Sagi Grimberg
2023-08-16 12:30 ` Aurelien Aptel
2023-07-12 16:14 ` [PATCH v12 08/26] nvme-tcp: Add DDP data-path Aurelien Aptel
2023-08-09 7:35 ` Sagi Grimberg
2023-08-14 16:12 ` Aurelien Aptel [this message]
2023-08-14 19:01 ` Sagi Grimberg
2023-08-17 13:28 ` Aurelien Aptel
2023-07-12 16:14 ` [PATCH v12 09/26] nvme-tcp: RX DDGST offload Aurelien Aptel
2023-08-09 7:59 ` Sagi Grimberg
2023-08-10 14:48 ` Aurelien Aptel
2023-08-13 13:49 ` Sagi Grimberg
2023-07-12 16:14 ` [PATCH v12 10/26] nvme-tcp: Deal with netdevice DOWN events Aurelien Aptel
2023-08-09 8:00 ` Sagi Grimberg
2023-08-16 13:03 ` Aurelien Aptel
2023-08-16 14:10 ` Sagi Grimberg
2023-08-17 14:09 ` Aurelien Aptel
2023-08-20 10:50 ` Sagi Grimberg
2023-08-21 12:33 ` Aurelien Aptel
2023-07-12 16:14 ` [PATCH v12 11/26] nvme-tcp: Add modparam to control the ULP offload enablement Aurelien Aptel
2023-08-09 8:03 ` Sagi Grimberg
2023-08-10 14:50 ` Aurelien Aptel
2023-08-16 1:05 ` Max Gurtovoy
2023-07-12 16:14 ` [PATCH v12 12/26] nvme-tcp: Only enable offload with TLS if the driver supports it Aurelien Aptel
2023-08-09 8:05 ` Sagi Grimberg
2023-08-10 14:52 ` Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 13/26] Documentation: add ULP DDP offload documentation Aurelien Aptel
2023-07-15 10:32 ` Simon Horman
2023-07-17 9:48 ` Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 14/26] net/mlx5e: Rename from tls to transport static params Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 15/26] net/mlx5e: Refactor ico sq polling to get budget Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 16/26] net/mlx5e: Have mdev pointer directly on the icosq structure Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 17/26] net/mlx5e: Refactor doorbell function to allow avoiding a completion Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 18/26] net/mlx5: Add NVMEoTCP caps, HW bits, 128B CQE and enumerations Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 19/26] net/mlx5e: NVMEoTCP, offload initialization Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 20/26] net/mlx5e: TCP flow steering for nvme-tcp acceleration Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 21/26] net/mlx5e: NVMEoTCP, use KLM UMRs for buffer registration Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 22/26] net/mlx5e: NVMEoTCP, queue init/teardown Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 23/26] net/mlx5e: NVMEoTCP, ddp setup and resync Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 24/26] net/mlx5e: NVMEoTCP, async ddp invalidation Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 25/26] net/mlx5e: NVMEoTCP, data-path for DDP+DDGST offload Aurelien Aptel
2023-07-12 16:15 ` [PATCH v12 26/26] net/mlx5e: NVMEoTCP, statistics Aurelien Aptel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2535y5hwqkg.fsf@nvidia.com \
--to=aaptel@nvidia$(echo .)com \
--cc=aurelien.aptel@gmail$(echo .)com \
--cc=axboe@fb$(echo .)com \
--cc=borisp@nvidia$(echo .)com \
--cc=chaitanyak@nvidia$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=galshalom@nvidia$(echo .)com \
--cc=hch@lst$(echo .)de \
--cc=kbusch@kernel$(echo .)org \
--cc=kuba@kernel$(echo .)org \
--cc=linux-nvme@lists$(echo .)infradead.org \
--cc=malin1024@gmail$(echo .)com \
--cc=mgurtovoy@nvidia$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=ogerlitz@nvidia$(echo .)com \
--cc=sagi@grimberg$(echo .)me \
--cc=smalin@nvidia$(echo .)com \
--cc=yorayz@nvidia$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox