From nobody Tue Apr 14 12:35:52 2026 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4fw3fd1WMMz6YV6g for ; Tue, 14 Apr 2026 12:36:09 +0000 (UTC) (envelope-from asomers@gmail.com) Received: from mail-ed1-f41.google.com (mail-ed1-f41.google.com [209.85.208.41]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "WR4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4fw3fc4hzmz3nyS for ; Tue, 14 Apr 2026 12:36:08 +0000 (UTC) (envelope-from asomers@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-ed1-f41.google.com with SMTP id 4fb4d7f45d1cf-6708cc2d6f6so6160915a12.0 for ; Tue, 14 Apr 2026 05:36:08 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1776170167; cv=none; d=google.com; s=arc-20240605; b=Bb1u+QZi+UrCsUa0jaIDYykoDDLH52zCqb6KbWQTLQ7PTKxHxKfhGt1Rcwt5Wxu9Fv dSSQT6NKTc79PUSg9ZgjdxnQH23oFYEvL0vqQYUJgrZN/+3l8VyTI5Osd6EdDQwQofAW Ej4C28eESBGIR3a8Zt2hywf9VqO9kRUN3mOt2oPdoz5nsOrxO+a8ptNmz/P4hlG1nsOm E/SiHNAHQWCxKE0hvd0gCsJGIgCCIUlPMU62EHEq8AU3yHyI2Wr87tdhjP/k+9qLfbqr rAwWX8vg7YBWX8YZV/YgIrtuU/kkBCh09rmLFQ4QHgNnLAjk/wbRK398PRMwzvw9DfEv I0tw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version; bh=S97K0PQrWD5HNxxAVVzqsWN4puhz/Doi4O1coJ94MyA=; fh=ixXqPNmnjA6fwwX6wSuwjL1jQS6HC0yNpNZQY9GVoVg=; b=AGzZHoEjbSJnEIKhRCs9/FrYrrKqORim/M/1H/aCobC4nYR/0fK6mPqSAWYMZ/f98a WOfFSyVbZxE5RYWipEKjnsnlSbJjgX3x6YNt6A+FIqz605P0i9RVQcegLJtQ00D2zblr JQ33BQwrT1sP8rMrXKvEzlnBO83ND9e4pG1lHHtopqLq4o+ZHsIs94Nj5ZLposPSOfW1 pHliYTJ50Kp/QkTgxIzM8M1AB0DgHT3Iz7o6bDtEUKnx0zdLYz5G8b0CKyVQrPmQQvJ6 G2oEemj7c3MQzDMkWsake/jhQRdZAkFxHFUP7OJ5SyBS7kMfUO62heW1l/hX8Jn0c4Fo H51Q==; darn=freebsd.org ARC-Authentication-Results: i=1; mx.google.com; arc=none X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776170167; x=1776774967; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=S97K0PQrWD5HNxxAVVzqsWN4puhz/Doi4O1coJ94MyA=; b=HEeCVAphok8iuDDmWLEq1FUcDOz7OjlGJHZ1/I5fe0WxHiBoEwmAdeQaIVDxnddZDO HCF4TD3fEwQHta00beVcF1xVcyk7gx3KLKbi2PfOXDt0PZDsQ8awaHXD7xu5+c8rebBb 5Zl03tnZyupoa1taWwrE7HngUpeAKsKAlDwZBwAMYnujttTLRBVD5Muod68hgS1tEFgj dtqEGAyxC9fQ77EFbM2YvINM51A8K+5rareVXXRGNZirX+JTI+MT5wQggmW6EFgzeSe5 JifWCueSY8xRmE18Q8pfomeDBoOyN2HtMdxjvmfwUbFPuubORtQysJcZzXFnpb0NFhVs ULHA== X-Forwarded-Encrypted: i=1; AFNElJ+l3YHMql0ApE50XdEPzboc55a5HbirVW4A+xsxt9zoTg1qHTse5iNzr7RGqUnqK47pQOndQtO0UnTzNzO8rSAALGuK@freebsd.org X-Gm-Message-State: AOJu0YzUSO6bhekTucBpOcpKLFhmyKq4k6pyoYv5zMy7DQsRkks3b6Gb 35vfFcY4r8MjBnyVQB8g8n3yDHfquFfqbktiE6Ji6Tp8CN6zkD6PuG/ONmfwO9BW7Pu8DpEnfBx X96NJu6r0ACH/SoSdO74vsbimNvtl0J5ucw== X-Gm-Gg: AeBDiesSjxeArTrOzdfg9yQFMfqYW/kGgb75bj9oaAwlcSyLxAYNAMh3a0XgFALqGp8 j5ujOj57zfn0lAeI7kpu3wVlkCMc3gvvbVGIAYeAf3291M9lDEriK8E+ea3T4LezEBoAVYfqza+ wDKPFeTK4bgqG9vxU3ulL0Xuc3gb2JDDeiT9o0EDV6X0PcPguoSmOIZrKVuGl+au6ximZnA9k/K 2KyTvBlvGSSwXJlHJt9FvxXLQMDUuUy0cHVAhpbRAaS8UGFuOjMLS1fataY95BUwcko0pb0gvja sGKnPGNWDjwpkdupWRmCPMWYCfB4BxpCrntVoI7W7i3ZksLn1Z/gN60aOmJb/3s2MrgPPLWjtsL uah0= X-Received: by 2002:a05:6402:21ce:b0:66e:bff3:8608 with SMTP id 4fb4d7f45d1cf-6707af15efemr6906663a12.25.1776170165739; Tue, 14 Apr 2026 05:36:05 -0700 (PDT) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 References: <202508062010.576KA2Mk062184@gitrepo.freebsd.org> <680f1cb7-f8f6-4d40-af31-83e2ea0205d6@FreeBSD.org> In-Reply-To: <680f1cb7-f8f6-4d40-af31-83e2ea0205d6@FreeBSD.org> From: Alan Somers Date: Tue, 14 Apr 2026 06:35:52 -0600 X-Gm-Features: AQROBzAV1j70Yc8aUtsuj9z-RB6SHKiORzvbW8bfGT8fleHQvpNoUu89MiXC6SI Message-ID: Subject: Re: git: 66b5296f1b29 - main - ctld: Add support for NVMe over Fabrics To: John Baldwin Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US] X-Rspamd-Queue-Id: 4fw3fc4hzmz3nyS X-Spamd-Bar: ---- On Tue, Apr 14, 2026 at 4:31=E2=80=AFAM John Baldwin wrot= e: > > On 4/13/26 13:36, Alan Somers wrote: > > On Mon, Apr 13, 2026 at 10:56=E2=80=AFAM John Baldwin = wrote: > >> > >> On 4/13/26 11:51, Alan Somers wrote: > >>> On Wed, Aug 6, 2025 at 2:10=E2=80=AFPM John Baldwin = wrote: > >>>> > >>>> The branch main has been updated by jhb: > >>>> > >>>> URL: https://cgit.FreeBSD.org/src/commit/?id=3D66b5296f1b29083634e28= 75ff08c32e7b6b866a8 > >>>> > >>>> commit 66b5296f1b29083634e2875ff08c32e7b6b866a8 > >>>> Author: John Baldwin > >>>> AuthorDate: 2025-08-06 19:57:50 +0000 > >>>> Commit: John Baldwin > >>>> 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 supp= ort uses > >>>> an alternate configuration syntax. > >>>> > >>>> - In authentication groups, permitted NVMeoF hosts can be allo= wed by > >>>> names (NQNs) via "host-nqn" values (similar to "initiator-na= me" for > >>>> iSCSI). Similarly, "host-address" accepts permitted host ad= dresses > >>>> similar to "initiator-portal" for iSCSI. > >>>> > >>>> - A new "transport-group" context enumerates transports that c= an be > >>>> used by a group of NVMeoF controllers similar to the "portal= -group" > >>>> context for iSCSI. In this section, the "listen" keyword ac= cepts a > >>>> transport as well as an address to permit other types of tra= nsports > >>>> besides TCP in the future. The "foreign", "offload", and "r= edirect" > >>>> keywords are also not meaningful and thus not supported. > >>>> > >>>> - A new "controller" context describes an NVMeoF I/O controlle= r > >>>> similar to the "target" context for iSCSI. One key differen= ce here > >>>> is that "lun" objects are replaced by "namespace" objects. = However, > >>>> a "namespace" can reference a named global lun permitting LU= Ns 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 NVM= e", name); > >>>> + return nullptr; > >>>> + } > >>>> + > >>>> + /* > >>>> + * Normalize the name to lowercase to match iSCSI. > >>>> + */ > >>>> + std::string t_name(name); > >>>> + for (char &c : t_name) > >>>> + c =3D 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=3D257 -O > >>> cfiscsi_target=3Diqn.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 > >> > >> Note that this function is for NVMe, not iSCSI. iSCSI targets are cre= ated in > >> conf::add_target which has similar code: > >> > >> struct target * > >> conf::add_target(const char *name) > >> { > >> if (!valid_iscsi_name(name, log_warnx)) > >> return (nullptr); > >> > >> /* > >> * RFC 3722 requires us to normalize the name to lowercase. > >> */ > >> std::string t_name(name); > >> for (char &c : t_name) > >> c =3D tolower(c); > >> > >> Prior to the C++ commit, this change was already in place: > >> > >> struct target * > >> target_new(struct conf *conf, const char *name) > >> { > >> struct target *targ; > >> int i, len; > >> > >> targ =3D target_find(conf, name); > >> if (targ !=3D NULL) { > >> log_warnx("duplicated target \"%s\"", name); > >> return (NULL); > >> } > >> if (valid_iscsi_name(name, log_warnx) =3D=3D false) { > >> return (NULL); > >> } > >> targ =3D new target(); > >> targ->t_name =3D checked_strdup(name); > >> > >> /* > >> * RFC 3722 requires us to normalize the name to lowercase. > >> */ > >> len =3D strlen(name); > >> for (i =3D 0; i < len; i++) > >> targ->t_name[i] =3D tolower(targ->t_name[i]); > >> > >> targ->t_conf =3D conf; > >> TAILQ_INSERT_TAIL(&conf->conf_targets, targ, t_next); > >> > >> return (targ); > >> } > >> > >> This was present in commit 009ea47eb2d21856af4529aaaca32cd67748daea > >> which brought in the iSCSI target, so it has always been present > >> in ctld. > >> > >> Also, AFAICT, the names are still accepted, they are just normalized. > >> > >> I guess one difference is that before, target_new() called target_find= () > >> with the non-normalized name to check for duplicates, and now we check > >> for duplicates after normalizing the name. I'm not sure how that work= ed > >> in the past in practice as you would have had two targets with the sam= e > >> name (e.g. I wonder what the ctladm portlist output looked like for th= is > >> case and if it would have listed two ports with the same name)? I sus= pect > >> that was more by accident and probably didn't work properly in practic= e > >> (e.g. the kernel handoff ioctl used the normalized name when invoking > >> CTL_ISCSI, so connections to both "names" probably were always mapped = to > >> only one of the connections, and finding a port during login processin= g > >> probably only found the first target, and only if the initiator gave t= he > >> all-lowercase name). > >> > >> That is to say, you didn't get an error before, but it didn't work, an= d > >> now it tells you that it doesn't work AFAICT. > > > > Excuse me, I spoke a little too soon. You are correct that ctld has > > been converting target names to lower case before registering them in > > the kernel for a long time. The change is that previously, if an > > initiator attempted to connect to an uppercase target name, ctld would > > accept it. That's because port_find_in_pg used strcasecmp in > > stable/14. But change 4b1aac931465f39c5c26bfa1d5539a428d340f20 > > removed strcasecmp, replacing it by the C++ STL's find method on > > std::unordered_map. > > > > So we used to accept connections case-insensitively, and now we accept > > them case-sensitively. To restore the previous behavior, should we > > add tolower() on the target_name in iscsi_connection::login() ? > > Yes, we should normalize there, and that indeed is my fault and warrants > a Fixes tag. > > -- > John Baldwin Ok. I'll take care of it.