ioccsz and iorcsz check failing

[Sagi Grimberg]

On 12/20/23 11:02, Daniel Wagner wrote:
> On Wed, Dec 20, 2023 at 02:10:42AM +0200, Max Gurtovoy wrote:
>> I just saw that Caleb also noticed that it shouldn't be relevant for
>> discovery controllers. The mail got lost in the mailbox.
> 
> I found it also in the spam folder.
> 
>> Anyway, I believe the initiator/host code should be updated for 6.7 and must
>> ignore these fields as the spec explicitly said it is reserved.
>> For the target side, I prefer to keep it 0 for reserved fields since this is
>> the convention in all the specifications that I'm aware of.
> 
> I've tried to figure out what our testing storage servers report (NetApp
> AF-700 and a Dell Powerstore). Though I can't access our equipment right
> now due to recabling activities in the datacenter.
> 
>> I have a small bug for above code so please use:
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index ee2e4c49892d..b218ac88fcf8 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -3068,14 +3068,14 @@ static int nvme_check_ctrl_fabric_info(struct
>> nvme_ctrl *ctrl, struct nvme_id_ct
>>                  return -EINVAL;
>>          }
>>
>> -       if (ctrl->ioccsz < 4) {
>> +       if (!nvme_discovery_ctrl(ctrl) && ctrl->ioccsz < 4) {
>>                  dev_err(ctrl->device,
>>                          "I/O queue command capsule supported size %d < 4\n",
>>                          ctrl->ioccsz);
>>                  return -EINVAL;
>>          }
>>
>> -       if (ctrl->iorcsz < 1) {
>> +       if (!nvme_discovery_ctrl(ctrl) && ctrl->iorcsz < 1) {
>>                  dev_err(ctrl->device,
>>                          "I/O queue response capsule supported size %d <
>> 1\n",
>>                          ctrl->iorcsz);
>>
> 
> With this blktests is happy and connecting to a remote nvme-tcp target
> (Linux 6.1) works fine. So if you spin a proper patch feel free to add
> 
> Tested-by: Daniel Wagner <dwagner at suse.de>

I think that nvmet should set these values as you suggested Daniel,
at the same time we can relax nvme host to issue a warning if it sees a
discovery controller that clears these values.