[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <253plzsis4h.fsf@nvidia.com>
Date: Wed, 29 Nov 2023 15:52:30 +0200
From: Aurelien Aptel <aaptel@...dia.com>
To: Sagi Grimberg <sagi@...mberg.me>, linux-nvme@...ts.infradead.org,
netdev@...r.kernel.org, hch@....de, kbusch@...nel.org, axboe@...com,
chaitanyak@...dia.com, davem@...emloft.net, kuba@...nel.org
Cc: Boris Pismenny <borisp@...dia.com>, aurelien.aptel@...il.com,
smalin@...dia.com, malin1024@...il.com, ogerlitz@...dia.com,
yorayz@...dia.com, galshalom@...dia.com, mgurtovoy@...dia.com,
brauner@...nel.org
Subject: Re: [PATCH v20 05/20] nvme-tcp: Add DDP offload control path
Hi Sagi,
Sagi Grimberg <sagi@...mberg.me> writes:
>> + ok = ulp_ddp_query_limits(netdev, &ctrl->ddp_limits,
>> + ULP_DDP_NVME, ULP_DDP_CAP_NVME_TCP,
>> + ctrl->ctrl.opts->tls);
>> + if (!ok) {
>
> please use a normal name (ret).
Ok, we will rename to ret and make ulp_ddp_query_limits() return int 0
on success to be consistent with the name.
> Plus, its strange that a query function receives a feature and returns
> true/false based on this. The query should return the limits, and the
> caller should look at the limits and see if it is appropriately
> supported.
We are not sure how to proceed as this seems to conflict with what you
suggested in v12 [1] about hiding the details of checking supports in
the API. Limits just dictate some constants the nvme-layer should use
once we know it is supported.
We can rename ulp_ddp_query_limits() to ulp_ddp_check_support(). This
function checks the support of the specified offload capability and also
returns the limitations of it.
Alternatively, we can split it in 2 API functions (check_support
and query_limits).
Let us know what you prefer.
Thanks
1: https://lkml.kernel.org/netdev/bc5cd2a7-efc4-e4df-cae5-5c527dd704a6@grimberg.me/
Powered by blists - more mailing lists