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

From: Aurelien Aptel <[email-masked]>
Date: Wed, 29 Nov 2023 15:56:54 +0200

Sagi Grimberg <[email-masked]> 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