Question on inline support

Question on inline support

[Steve Wise]

Hey guys,

While testing the nvme rdma target with inline_data_size=0 for my patch
series [1], I found that the nvme host side rejected the queue
connections with this error:

"nvme nvme0: Mandatory keyed sgls are not support"

Due to this code in nvme/host/rdma.c:

?????? /* sanity check keyed sgls */
?????? if (!(ctrl->ctrl.sgls & (1 << 20))) {
?????????????? dev_err(ctrl->ctrl.device, "Mandatory keyed sgls are not
support\n");
?????????????? ret = -EINVAL;
?????????????? goto out_remove_admin_queue;
?????? }

Because the target side only sets bit 20 if inline_data_size is > 0, in
nvme/target/admin_cmd.c:

??????? if (ctrl->ops->sqe_inline_size)
??????????????? id->sgls |= cpu_to_le32(1 << 20);


I think the host-side code can be removed, yes?? I don't see why it
shouldn't be supported.? In fact, I removed it and things appear to work
just fine when I disable inline on the target.

Thanks,

Steve.

[1] http://lists.infradead.org/pipermail/linux-nvme/2018-May/017554.html

Question on inline support

[hch]

On Tue, May 29, 2018@12:33:56PM -0500, Steve Wise wrote:
> Hey guys,
> 
> While testing the nvme rdma target with inline_data_size=0 for my patch
> series [1], I found that the nvme host side rejected the queue
> connections with this error:
> 
> "nvme nvme0: Mandatory keyed sgls are not support"
> 
> Due to this code in nvme/host/rdma.c:
> 
> ?????? /* sanity check keyed sgls */
> ?????? if (!(ctrl->ctrl.sgls & (1 << 20))) {
> ?????????????? dev_err(ctrl->ctrl.device, "Mandatory keyed sgls are not
> support\n");
> ?????????????? ret = -EINVAL;
> ?????????????? goto out_remove_admin_queue;
> ?????? }

Keyed SGLs are bit 2, so the above should check for bit 2 as well.

We should still check for bit 20 if inline data is supported, though.

Question on inline support

[Steve Wise]

On 5/29/2018 12:44 PM, hch@lst.de wrote:
> On Tue, May 29, 2018@12:33:56PM -0500, Steve Wise wrote:
>> Hey guys,
>>
>> While testing the nvme rdma target with inline_data_size=0 for my patch
>> series [1], I found that the nvme host side rejected the queue
>> connections with this error:
>>
>> "nvme nvme0: Mandatory keyed sgls are not support"
>>
>> Due to this code in nvme/host/rdma.c:
>>
>> ?????? /* sanity check keyed sgls */
>> ?????? if (!(ctrl->ctrl.sgls & (1 << 20))) {
>> ?????????????? dev_err(ctrl->ctrl.device, "Mandatory keyed sgls are not
>> support\n");
>> ?????????????? ret = -EINVAL;
>> ?????????????? goto out_remove_admin_queue;
>> ?????? }
> Keyed SGLs are bit 2, so the above should check for bit 2 as well.

Ah.? I'll fix that.

> We should still check for bit 20 if inline data is supported, though.
>

Why, exactly?? Use of inline data is determined by the target's
advertised ioccsz, and handled in nvme/host/rdma.c:

static inline size_t nvme_rdma_inline_data_size(struct nvme_rdma_queue
*queue)
{
??????? return queue->cmnd_capsule_len - sizeof(struct nvme_command);
}

Are you saying that the host should support a target that doesn't set
bit 20, yet advertises an ioccsz that could support inline??

Thanks,

Steve.

Question on inline support

[Steve Wise]

On 5/29/2018 12:48 PM, Steve Wise wrote:
>
> On 5/29/2018 12:44 PM, hch@lst.de wrote:
>> On Tue, May 29, 2018@12:33:56PM -0500, Steve Wise wrote:
>>> Hey guys,
>>>
>>> While testing the nvme rdma target with inline_data_size=0 for my patch
>>> series [1], I found that the nvme host side rejected the queue
>>> connections with this error:
>>>
>>> "nvme nvme0: Mandatory keyed sgls are not support"
>>>
>>> Due to this code in nvme/host/rdma.c:
>>>
>>> ?????? /* sanity check keyed sgls */
>>> ?????? if (!(ctrl->ctrl.sgls & (1 << 20))) {
>>> ?????????????? dev_err(ctrl->ctrl.device, "Mandatory keyed sgls are not
>>> support\n");
>>> ?????????????? ret = -EINVAL;
>>> ?????????????? goto out_remove_admin_queue;
>>> ?????? }
>> Keyed SGLs are bit 2, so the above should check for bit 2 as well.
> Ah.? I'll fix that.
>
>> We should still check for bit 20 if inline data is supported, though.
>>
> Why, exactly?? Use of inline data is determined by the target's
> advertised ioccsz, and handled in nvme/host/rdma.c:
>
> static inline size_t nvme_rdma_inline_data_size(struct nvme_rdma_queue
> *queue)
> {
> ??????? return queue->cmnd_capsule_len - sizeof(struct nvme_command);
> }
>
> Are you saying that the host should support a target that doesn't set
> bit 20, yet advertises an ioccsz that could support inline??
>
> Thanks,
>
> Steve.


I added this:? only use inline if the target advertised it via bit 20
-and- there is space available based on ioccsz.

I'm resending a new version of my series soon.

Thanks,

Steve.