All of lore.kernel.org
 help / color / mirror / Atom feed
* fabrics connect cmd with a non-zero reserved value
@ 2021-01-06 10:37 Engel, Amit
  2021-01-14  0:34 ` Sagi Grimberg
  0 siblings, 1 reply; 6+ messages in thread
From: Engel, Amit @ 2021-01-06 10:37 UTC (permalink / raw)
  To: linux-nvme, Sagi Grimberg; +Cc: Anner, Ran, Grupi, Elad, Engel, Amit

Hello, 
We see that when executing nvme connect cmd (Opcode: 0x7f Fabric Cmd)
The host is submitting the cmd with reserved filed set to non-zero value - which is out of spec.
Seems like this issue is not only for connect but also for other fabrics cmds.

Here is a packet capture for nvme connect :

    NVM Express Fabrics (Cmd)
        Opcode: 0x7f Fabric Cmd
        [Fabric Cqe in: 302]
        Reserved: 0x40
        Command ID: 0x0000
        Fabric Cmd Type: Connect (0x01)
        Reserved: 00000000000000000000000000000000000000
        SGL1
        Record Format: 0x0000
        Queue ID: 0x0000
        SQ Size: 0x001f
        Connect Attributes: 0x00
        Reserved: 00
        Keep Alive Timeout: 15000ms
        Reserved: 000000000000000000000000
    Data
        Host Identifier: f1aa9784-195b-4814-893d-44f90a6a92d9
        Controller ID: 0xffff
        Reserved: 000000000000000000000000000000000000000000000000000000000000000000000000...
        Subsystem NQN: nqn.2015-10.com.dell:dellemc-powerstore-23b5145591975EDFF2FE
        Host NQN: nqn.2014-08.org.nvmexpress:uuid:nc5199007
        Reserved: 000000000000000000000000000000000000000000000000000000000000000000000000...

0x40 value is corresponding to PSDT, in a normal (non-fabric) nvme cmd, PSDT field exists.
Do we expect for the same for fabric cmd?
If so, the spec can't mark the second byte of connect cmd as reserved.
Based on nvme spec:
"A reserved bit, byte, word, field, or register shall be cleared to 0h, or in accordance with a future extension to this specification"

Is this a real bug or a spec bug?

Thanks
Amit


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: fabrics connect cmd with a non-zero reserved value
  2021-01-06 10:37 fabrics connect cmd with a non-zero reserved value Engel, Amit
@ 2021-01-14  0:34 ` Sagi Grimberg
  2021-01-14  1:10   ` Sagi Grimberg
  0 siblings, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2021-01-14  0:34 UTC (permalink / raw)
  To: Engel, Amit, linux-nvme; +Cc: Anner, Ran, Grupi, Elad


> Hello,
> We see that when executing nvme connect cmd (Opcode: 0x7f Fabric Cmd)
> The host is submitting the cmd with reserved filed set to non-zero value - which is out of spec.
> Seems like this issue is not only for connect but also for other fabrics cmds.
> 
> Here is a packet capture for nvme connect :
> 
>      NVM Express Fabrics (Cmd)
>          Opcode: 0x7f Fabric Cmd
>          [Fabric Cqe in: 302]
>          Reserved: 0x40
>          Command ID: 0x0000
>          Fabric Cmd Type: Connect (0x01)
>          Reserved: 00000000000000000000000000000000000000
>          SGL1
>          Record Format: 0x0000
>          Queue ID: 0x0000
>          SQ Size: 0x001f
>          Connect Attributes: 0x00
>          Reserved: 00
>          Keep Alive Timeout: 15000ms
>          Reserved: 000000000000000000000000
>      Data
>          Host Identifier: f1aa9784-195b-4814-893d-44f90a6a92d9
>          Controller ID: 0xffff
>          Reserved: 000000000000000000000000000000000000000000000000000000000000000000000000...
>          Subsystem NQN: nqn.2015-10.com.dell:dellemc-powerstore-23b5145591975EDFF2FE
>          Host NQN: nqn.2014-08.org.nvmexpress:uuid:nc5199007
>          Reserved: 000000000000000000000000000000000000000000000000000000000000000000000000...
> 
> 0x40 value is corresponding to PSDT, in a normal (non-fabric) nvme cmd, PSDT field exists.
> Do we expect for the same for fabric cmd?
> If so, the spec can't mark the second byte of connect cmd as reserved.
> Based on nvme spec:
> "A reserved bit, byte, word, field, or register shall be cleared to 0h, or in accordance with a future extension to this specification"
> 
> Is this a real bug or a spec bug?

We shouldn't be setting reserved bits. But from what I see, we zero the 
nvme commands before setting it so this is strange to me..

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: fabrics connect cmd with a non-zero reserved value
  2021-01-14  0:34 ` Sagi Grimberg
@ 2021-01-14  1:10   ` Sagi Grimberg
  2021-01-14 17:35     ` Christoph Hellwig
  0 siblings, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2021-01-14  1:10 UTC (permalink / raw)
  To: Engel, Amit, linux-nvme; +Cc: Anner, Ran, Frederick.Knight, Grupi, Elad



On 1/13/21 4:34 PM, Sagi Grimberg wrote:
> 
>> Hello,
>> We see that when executing nvme connect cmd (Opcode: 0x7f Fabric Cmd)
>> The host is submitting the cmd with reserved filed set to non-zero 
>> value - which is out of spec.
>> Seems like this issue is not only for connect but also for other 
>> fabrics cmds.
>>
>> Here is a packet capture for nvme connect :
>>
>>      NVM Express Fabrics (Cmd)
>>          Opcode: 0x7f Fabric Cmd
>>          [Fabric Cqe in: 302]
>>          Reserved: 0x40
>>          Command ID: 0x0000
>>          Fabric Cmd Type: Connect (0x01)
>>          Reserved: 00000000000000000000000000000000000000
>>          SGL1
>>          Record Format: 0x0000
>>          Queue ID: 0x0000
>>          SQ Size: 0x001f
>>          Connect Attributes: 0x00
>>          Reserved: 00
>>          Keep Alive Timeout: 15000ms
>>          Reserved: 000000000000000000000000
>>      Data
>>          Host Identifier: f1aa9784-195b-4814-893d-44f90a6a92d9
>>          Controller ID: 0xffff
>>          Reserved: 
>> 000000000000000000000000000000000000000000000000000000000000000000000000... 
>>
>>          Subsystem NQN: 
>> nqn.2015-10.com.dell:dellemc-powerstore-23b5145591975EDFF2FE
>>          Host NQN: nqn.2014-08.org.nvmexpress:uuid:nc5199007
>>          Reserved: 
>> 000000000000000000000000000000000000000000000000000000000000000000000000... 
>>
>>
>> 0x40 value is corresponding to PSDT, in a normal (non-fabric) nvme 
>> cmd, PSDT field exists.
>> Do we expect for the same for fabric cmd?
>> If so, the spec can't mark the second byte of connect cmd as reserved.
>> Based on nvme spec:
>> "A reserved bit, byte, word, field, or register shall be cleared to 
>> 0h, or in accordance with a future extension to this specification"
>>
>> Is this a real bug or a spec bug?
> 
> We shouldn't be setting reserved bits. But from what I see, we zero the 
> nvme commands before setting it so this is strange to me..

OK, now I see the issue.

This appears to me this would be an issue with the spec. As the connect
data (like any command payload) is coming in the form of an sgl.

Adding Fred for this question.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: fabrics connect cmd with a non-zero reserved value
  2021-01-14  1:10   ` Sagi Grimberg
@ 2021-01-14 17:35     ` Christoph Hellwig
  2021-01-14 17:45       ` Engel, Amit
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2021-01-14 17:35 UTC (permalink / raw)
  To: Sagi Grimberg
  Cc: Anner, Ran, Grupi, Elad, Frederick.Knight, linux-nvme, Engel, Amit

On Wed, Jan 13, 2021 at 05:10:05PM -0800, Sagi Grimberg wrote:
> OK, now I see the issue.
> 
> This appears to me this would be an issue with the spec. As the connect
> data (like any command payload) is coming in the form of an sgl.

Reserved fields in host to controller data structure may be checked,
but may also not.  So we can verify anyting we think is to bogus to
pass, but there is not requirement for the controller to check every
reserved field or bit.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: fabrics connect cmd with a non-zero reserved value
  2021-01-14 17:35     ` Christoph Hellwig
@ 2021-01-14 17:45       ` Engel, Amit
  2021-01-14 21:12         ` Sagi Grimberg
  0 siblings, 1 reply; 6+ messages in thread
From: Engel, Amit @ 2021-01-14 17:45 UTC (permalink / raw)
  To: Christoph Hellwig, Sagi Grimberg; +Cc: Anner, Ran, Frederick.Knight, linux-nvme

Since the data is coming in the form of an sgl, the reserved bits that the spec defines are not really reserved.
I would expect that the spec will not mark the second byte of connect cmd as reserved
(These reserved bits are not cleared to 0h)

-----Original Message-----
From: Christoph Hellwig <hch@infradead.org> 
Sent: Thursday, January 14, 2021 7:36 PM
To: Sagi Grimberg
Cc: Engel, Amit; linux-nvme@lists.infradead.org; Anner, Ran; Frederick.Knight@netapp.com; Grupi, Elad
Subject: Re: fabrics connect cmd with a non-zero reserved value


[EXTERNAL EMAIL] 

On Wed, Jan 13, 2021 at 05:10:05PM -0800, Sagi Grimberg wrote:
> OK, now I see the issue.
> 
> This appears to me this would be an issue with the spec. As the 
> connect data (like any command payload) is coming in the form of an sgl.

Reserved fields in host to controller data structure may be checked, but may also not.  So we can verify anyting we think is to bogus to pass, but there is not requirement for the controller to check every reserved field or bit.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: fabrics connect cmd with a non-zero reserved value
  2021-01-14 17:45       ` Engel, Amit
@ 2021-01-14 21:12         ` Sagi Grimberg
  0 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2021-01-14 21:12 UTC (permalink / raw)
  To: Engel, Amit, Christoph Hellwig; +Cc: Anner, Ran, Frederick.Knight, linux-nvme


> Since the data is coming in the form of an sgl, the reserved bits that the spec defines are not really reserved.
> I would expect that the spec will not mark the second byte of connect cmd as reserved
> (These reserved bits are not cleared to 0h)

I tend to agree, connect payload can and does come in a form of SGL, so
it appears that PSDT is required also for connect...

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-14 21:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-06 10:37 fabrics connect cmd with a non-zero reserved value Engel, Amit
2021-01-14  0:34 ` Sagi Grimberg
2021-01-14  1:10   ` Sagi Grimberg
2021-01-14 17:35     ` Christoph Hellwig
2021-01-14 17:45       ` Engel, Amit
2021-01-14 21:12         ` Sagi Grimberg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.