linux-kernel - Re: [PATCH v2 1/3] nvme: add capability to connect to an admin controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <MN2PR04MB686294B136454530692AEC1ABC40A@MN2PR04MB6862.namprd04.prod.outlook.com>
Date: Wed, 2 Jul 2025 18:39:09 +0000
From: Kamaljit Singh <Kamaljit.Singh1@....com>
To: Damien Le Moal <dlemoal@...nel.org>, "kbusch@...nel.org"
	<kbusch@...nel.org>, "axboe@...nel.dk" <axboe@...nel.dk>, hch <hch@....de>,
	"sagi@...mberg.me" <sagi@...mberg.me>, "linux-nvme@...ts.infradead.org"
	<linux-nvme@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
CC: "cassel@...nel.org" <cassel@...nel.org>
Subject: Re: [PATCH v2 1/3] nvme: add capability to connect to an admin
 controller

Hi Damien,

On 7/1/25 19:11, Damien Le Moal wrote:
>> +static inline bool nvme_admin_ctrl(struct nvme_ctrl *ctrl)
>> +{
>> +     return (ctrl->cntrltype == NVME_CTRL_ADMIN);
>
>No need for the parenthesis.
Updated for v3.


>> +/*
>> + * An admin controller has one admin queue, but no I/O queues.
>> + * Override queue_count so it only creates an admin queue.
>> + */
>> +void nvme_override_prohibited_io_queues(struct nvme_ctrl *ctrl)
>> +{
>> +     if (nvme_admin_ctrl(ctrl))
>> +             ctrl->queue_count = 1;
>> +}
>> +EXPORT_SYMBOL_GPL(nvme_override_prohibited_io_queues);
>
>Why not have this done in nvme_init_subsystem() ? That would avoid needing to
>call this in all fabrics drivers.
Like I mentioned in response for 3/3, this can affect other drivers like apple/fc
and I don't have any way of testing them. If you're okay with that, I can move
nvme_override_prohibited_io_queues() into nvme_init_subsystem(). I did some
analysis and nvme_init_subsystem() seems to be covering reconnects as well.


>>  static bool nvme_validate_cntlid(struct nvme_subsystem *subsys,
>>               struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
>>  {
>> @@ -3215,6 +3231,11 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
>>               kfree(subsys);
>>               return -EINVAL;
>>       }
>> +     if (nvme_admin_ctrl(ctrl))
>> +             dev_info(ctrl->device,
>> +                     "Subsystem %s is an administrative controller",
>> +                     subsys->subnqn);
>
>We do not print an equivalent message for other subsystem controller types. So
>drop this.
I left that msg in there for debugging purposes. I can either change it to
dev_dbg() or if that's still not likeable/cluttering, I can remove it.
Please let me know.

Thanks,
Kamaljit