Re: [PATCH v20 07/20] nvme-tcp: RX DDGST offload

Aurelien Aptel
2023-11-29T13:57:15Z

Sagi Grimberg <sagi@grimberg.me> writes:
>> +static void nvme_tcp_ddp_ddgst_recalc(struct ahash_request *hash,
>> + struct request *rq,
>> + __le32 *ddgst)
>> +{
>> + struct nvme_tcp_request *req;
>> +
>> + if (!rq)
>> + return;
>
> How is this even possible? And what happens down the road if this is
> indeed a null rq?

You are correct, this isn't possible. We can see the req is fetched
earlier in nvme_tcp_recv_pdu() so it must exist.
We will remove it.

>> +
>> + req = blk_mq_rq_to_pdu(rq);
>> + ahash_request_set_crypt(hash, req->ddp.sg_table.sgl, (u8 *)ddgst,
>> + req->data_len);
>> + crypto_ahash_digest(hash);
>> +}
>> +
>> static bool nvme_tcp_resync_request(struct sock *sk, u32 seq, u32 flags);
>> static void nvme_tcp_ddp_teardown_done(void *ddp_ctx);
>> static const struct ulp_ddp_ulp_ops nvme_tcp_ddp_ulp_ops = {
>> @@ -430,6 +459,8 @@ static void nvme_tcp_setup_ddp(struct nvme_tcp_queue *queue,
>> static int nvme_tcp_offload_socket(struct nvme_tcp_queue *queue)
>> {
>> struct ulp_ddp_config config = {.type = ULP_DDP_NVME};
>> + bool offload_ddgst_rx = ulp_ddp_is_cap_active(queue->ctrl->ddp_netdev,
>> + ULP_DDP_CAP_NVME_TCP_DDGST_RX);
>
> Not sure a local variable is needed here.

Ok we will remove it.

Thanks