[PATCH 2/2] nvmet-tcp: fix connect error when setting param_inline_data_size to zero.

[Hou Pu]

On Sat, May 22, 2021 at 2:04 AM Sagi Grimberg <sagi at grimberg.me> wrote:
>
>
> >>> +static inline int nvmet_tcp_inline_data_size(struct nvmet_tcp_cmd *cmd)
> >>> +{
> >>> +     struct nvmet_tcp_queue *queue = cmd->queue;
> >>> +     struct nvme_command *nvme_cmd = cmd->req.cmd;
> >>> +     int inline_data_size = NVME_TCP_ADMIN_CCSZ;
> >>> +     u16 qid = 0;
> >>> +
> >>> +     if (likely(queue->nvme_sq.ctrl)) {
> >>> +             /* The connect admin/io queue has been executed. */
> >>> +             qid = queue->nvme_sq.qid;
> >>> +             if (qid)
> >>> +                     inline_data_size = cmd->req.port->inline_data_size;
> >>> +     } else if (nvme_cmd->connect.qid)
> >>> +             inline_data_size = cmd->req.port->inline_data_size;
> >>
> >> How can a connection to an I/O queue arrive without having the ctrl
> >> reference installed? Is this for the failure case?
> >
> > Hi Sagi,
> > AFAIK after the host finishes setting up the admin queue,
> > it connects to the io queue and sends the io-connect command. At this
> > point the nvmet_tcp_queue is firstly allocated and does not have a valid
> > queue->nvme_sq.ctrl. It is assigned after io-connect in nvmet_install_queue().
> > So this function tries to find the correct queue number before or after a
> > fabrics connect command.
>
> Why do you need the inline_data_size before the connect? its only
> relevant for nvme I/O..

>From my observation, the host always sends an admin-connect command with
nvmf_connect_data as inline data (by setting sg type to NVME_SGL_FMT_DATA_DESC).
This is fixed from the host side.

For io-connect command, the host sends nvmf_connect_data according
to ioccsz. If the size of nvmf_connect_data is smaller than ioccsz*16
it is sent as inline data. ioccsz is obtained by identifying the controller and
is controlled by param_inline_data_size from the target side.

If param_inline_data_size is set to zero, admin-connect will not
be received by target correctly. As target side thought there is
no inline data (by looking param_inline_data_size), but in fact
admin-connect has inline data.

This patch is trying to make param_inline_data_size only affect io-queue.
and admin-queue always allow 8k inline data. This is what the host
side do currently and also what the specification says AFAIK.

Thanks,
Hou