Re: [PATCH nvme-cli] nvme: warn about attaching a namespace to unknown controller

[Nilay Shroff <nilay@linux.ibm.com>]

On 6/25/24 01:02, Keith Busch wrote:
> On Sat, Jun 22, 2024 at 04:05:01PM +0530, Nilay Shroff wrote:
>>
>>
>> On 6/21/24 20:38, Keith Busch wrote:
>>> On Thu, Jun 20, 2024 at 06:25:45PM +0530, Nilay Shroff wrote:
>>>> Sometime it's possible for a multi-controller NVMe disk to have only
>>>> one of its controller discovered by the kernel. And if this happens
>>>> then it's also possible for a user to create and then attach a namespace
>>>> to a controller which has not been discovered by the kernel. In such a
>>>> case the attached namespace can't be used for IO because there's no path
>>>> to reach such namespace from the kernel.
>>>
>>> Isn't that a pretty normal thing to do, though? Like for an sriov
>>> situation, a primary controller attaches namespaces to secondaries, but
>>> not to itself.
>> Yes correct, but in case of sriov, AFAIK, the "nvme list-secondary ...." shall 
>> not list any secondary controller unless the secondary controller's corresponding 
>> VF is enabled (i.e. /sys/class/nvme/nvmeX/device/sriov_numvfs set to at least the 
>> secondary controller's VF number). 
>>
>> So it means that before attaching namespace to secondaries, user would first set 
>> sriov_numvfs. Setting numvfs would then enable the kernel to discover all secondary
>> controllers. 
> 
> You'd usually bind those to vfio, not nvme. So while the kernel knows
> those pci functions exist, the nvme driver doesn't necessarily know
> about these.
> 
> And that's not even an sriov exclusive thing. You can bind other PFs
> just the same, though they just don't have the resource provisioning
> features that VFs can do. But you can still attach and detach namespaces
> to/from them all the same.
>  
>> However, in the proposed patch we're trying to address a case where "nvme list-ctrl ..."
>> shows multiple controller entries however only one of those controllers is discovered
>> by kernel. All controllers shown in the output of "nvme list-ctrl ..." are physical
>> controllers on the nvme disk. In this case attaching a namespace to any of the 
>> undiscovered controller has a side effect due which the namespace rendered unusable.  
> 
> Unusable from the driver the admin ran the command from. Some other
> machine (real or virtual) may be able to use it, or maybe even the same
> machine that you handed some function to a user space driver, and that's
> not really an error.
> 
Ok got it... I didn't know this topology is also supported!

>> This patch tries to address this issue by showing a WARNING to the user and optionally
>> allowing user to cancel the attach operation. However if user still prefers to go 
>> ahead without cancelling the attach operation then nvme-cli would execute this command
>> as usual. 
> 
> Have you received bug reports of people mistakenly attaching namespaces
> to the wrong controller or something?
Not from customer but it was an internally found defect where someone (mistakenly!) 
attached a namespace to a controller which was not discovered by the kernel and that
caused the namespace remains hidden behind undiscovered controller... 

The "nvme list" command would not show up that hidden namespace, and also 
"nvme list-ns <dev>" wouldn't list that namespace. However later I found that 
the only way to list that hidden namespace was to use "nvme list <dev> -a" 
command.

I think, it might be difficult to gauge what's the intent here when user/admin runs 
a command to attach a namespace. If the intent is to attach namespace behind 
controller/VF which might be usable from other (real/virtual) machine then it's OK. 
However if the intent is to use the namespace on the same host machine and using the 
same kernel driver where the command is being executed then there could be an issue. 
Any comment...?

Thanks,
--Nilay