Re: git: 66b5296f1b29 - main - ctld: Add support for NVMe over Fabrics

On Wed, Aug 6, 2025 at 2:10 PM John Baldwin <jhb@freebsd.org> wrote:
>
> The branch main has been updated by jhb:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=66b5296f1b29083634e2875ff08c32e7b6b866a8
>
> commit 66b5296f1b29083634e2875ff08c32e7b6b866a8
> Author:     John Baldwin <jhb@FreeBSD.org>
> AuthorDate: 2025-08-06 19:57:50 +0000
> Commit:     John Baldwin <jhb@FreeBSD.org>
> CommitDate: 2025-08-06 19:59:13 +0000
>
>     ctld: Add support for NVMe over Fabrics
>
>     While the overall structure is similar for NVMeoF controllers and
>     iSCSI targets, there are sufficient differences that NVMe support uses
>     an alternate configuration syntax.
>
>     - In authentication groups, permitted NVMeoF hosts can be allowed by
>       names (NQNs) via "host-nqn" values (similar to "initiator-name" for
>       iSCSI).  Similarly, "host-address" accepts permitted host addresses
>       similar to "initiator-portal" for iSCSI.
>
>     - A new "transport-group" context enumerates transports that can be
>       used by a group of NVMeoF controllers similar to the "portal-group"
>       context for iSCSI.  In this section, the "listen" keyword accepts a
>       transport as well as an address to permit other types of transports
>       besides TCP in the future.  The "foreign", "offload", and "redirect"
>       keywords are also not meaningful and thus not supported.
>
>     - A new "controller" context describes an NVMeoF I/O controller
>       similar to the "target" context for iSCSI.  One key difference here
>       is that "lun" objects are replaced by "namespace" objects.  However,
>       a "namespace" can reference a named global lun permitting LUNs to be
>       shared between iSCSI targets and NVMeoF controllers.
>
>     NB: Authentication via CHAP is not implemented for NVMeoF.
>
>     Reviewed by:    imp
>     Sponsored by:   Chelsio Communications
>     Differential Revision:  https://reviews.freebsd.org/D48773
...
> +struct target *
> +conf::add_controller(const char *name)
> +{
> +       if (!nvmf_nqn_valid_strict(name)) {
> +               log_warnx("controller name \"%s\" is invalid for NVMe", name);
> +               return nullptr;
> +       }
> +
> +       /*
> +        * Normalize the name to lowercase to match iSCSI.
> +        */
> +       std::string t_name(name);
> +       for (char &c : t_name)
> +               c = tolower(c);
...

This makes it impossible to define an uppercase or mixed case target
name in ctld.  I guess the intent was to comply with rfc3722[^1]?
Even so, it's surprising, because such target names used to work.
It's also inconsistent, because it's still possible to create an
uppercase target name using ctladm directly, like this:

ctladm port -c -d iscsi -O cfiscsi_portal_group_tag=257 -O
cfiscsi_target=iqn.2018-10.myhost:TESTVOL1

Should we warn the user if they specify an uppercase target name, or
even fail to create it?

[^1]: https://datatracker.ietf.org/doc/html/rfc3722