ioccsz and iorcsz check failing

[Caleb Sander]

I'm not sure IOCCSZ and IORCSZ even make sense for discovery
controllers. These fields are "I/O Queue Command (Response) Capsule
Supported Size", but discovery controllers don't have I/O queues.
Presumably the host should ignore the values in these fields for
admin-only controllers. But the base spec doesn't seem to exempt admin
controllers from the requirement that "The minimum value that shall be
indicated is 4 (1) corresponding to 64 (16) bytes." Perhaps this is a
better question for the NVMe technical workgroups?


On Tue, Dec 19, 2023 at 9:31 AM Daniel Wagner <dwagner at suse.de> wrote:
>
> On Mon, Dec 18, 2023 at 06:04:46PM +0200, Sagi Grimberg wrote:
> >
> > > > Is anything else in the identify wrong, or is it just these
> > > > fabrics fields?
> > >
> > > So what I am seeing on the wire are a few fabrics commands (connect, get
> > > property) followed by the nvme id ctrl command (opcode 0x6).
> > >
> > >   nvmet: nvmet_req_init:962
> > >   nvmet: nvmet_parse_admin_cmd:1011
> > >   nvmet: nvmet_parse_discovery_cmd:359 opcode 6
> > >
> > > This calls then nvmet_execute_disc_identify, so adding
> > >
> > > --- a/drivers/nvme/target/discovery.c
> > > +++ b/drivers/nvme/target/discovery.c
> > > @@ -249,6 +249,7 @@ static void nvmet_execute_disc_identify(struct nvmet_req *req)
> > >   {
> > >          struct nvmet_ctrl *ctrl = req->sq->ctrl;
> > >          struct nvme_id_ctrl *id;
> > > +       u32 cmd_capsule_size;
> > >          u16 status = 0;
> > >
> > >          if (!nvmet_check_transfer_len(req, NVME_IDENTIFY_DATA_SIZE))
> > > @@ -289,6 +290,17 @@ static void nvmet_execute_disc_identify(struct nvmet_req *req)
> > >                  id->sgls |= cpu_to_le32(1 << 2);
> > >          if (req->port->inline_data_size)
> > >                  id->sgls |= cpu_to_le32(1 << 20);
> > > +       /*
> > > +        * Max command capsule size is sqe + in-capsule data size.
> > > +        * Disable in-capsule data for Metadata capable controllers.
> > > +        */
> > > +       cmd_capsule_size = sizeof(struct nvme_command);
> > > +       if (!ctrl->pi_support)
> > > +               cmd_capsule_size += req->port->inline_data_size;
> > > +       id->ioccsz = cpu_to_le32(cmd_capsule_size / 16);
> >
> > Yes, this is the culprit. Nice that it exposed a bug.
> >
> > There is no in-capsule data for discovery controllers afaict.
>
> Okay, I have no idea. I stole this from nvmet_execute_identify_ctrl().
>
> I tried to read up but couldn't really find a proper answer. And I am a
> bit confused about the TCP transport definition. If I got this right
> then a min size is not defined either for fabrics commands or responses
> depending if it is in-capsule or !in-capsule supported. But never are
> both values are defined at the same time.
>
> RDMA is a bit more clearer on this topic and says for admin commands
> these value are fixed is in line with your statement
>
> The PI check is kind of useless as we only deal with admin commands
> anyway.
>
> Anyway I am fine by just dropping the inline part as it doesn't seem to
> impact the functionality for the discovery controller. But I suppose it
> would be good to know why this is the correct thing to do.
>
>
>   3.3.2.1 Capsules and Data Transfers
>
>   The capsule size for the Admin Queue commands and responses is fixed
>   and defined in the NVMe Transport binding specification.
>
>
>   7 I/O Commands
>
>   The user data format and any end-to-end protection information is I/O
>   Command Set specific. Refer to each I/O Command Set specification for
>   applicability and additional details, if any. Refer to the referenced
>   I/O Command Set specification for all I/O Command Set specific
>   commands described in Figure 390.
>
>
>   7.2  Transport Capsule and Data Binding: Fibre Channel
>
>   [ no public documents ]
>
>
>   7.3.2 Capsules and SGLs (RDMA)
>
>   The capsule size for Fabrics commands are fixed in size regardless of
>   whether commands are submitted on an Admin Queue or an I/O Queue. The
>   command capsule size is 64 bytes and the response capsule size is 16
>   bytes.
>
>   The capsule sizes for the Admin Queue are fixed in size. The command
>   capsule size is 64 bytes and the response capsule size is 16 bytes.
>   In-capsule data is not supported for the Admin Queue.
>
>
>   7.4.3 Capsules (TCP)
>
>   The NVMe/TCP transport supports a message model. Data transfers are
>   supported via a transport specific data transfer mechanism, described
>   in section 7.4.5, and optionally via in-capsule data. NVMe/TCP capsule
>   sizes are summarized in Figure 62. The size of capsules is variable
>   when in-capsule data is supported and fixed when in-capsule data is
>   not supported
>
>   Figure 64 (size in bytes):
>                                !in-capsule    in-capsule
>   Fabrics and Admin Commands:  n/a            64 - 8256
>   Fabrics and Admin Responses: 16             n/a
>