Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] nvme: Treat discovery subsystems as unique subsystems
@ 2019-09-03 21:20 James Smart
  2019-09-04 12:46 ` Christoph Hellwig
  2019-09-04 14:08 ` Max Gurtovoy
  0 siblings, 2 replies; 3+ messages in thread
From: James Smart @ 2019-09-03 21:20 UTC (permalink / raw)
  To: linux-nvme; +Cc: James Smart

Current code matches subnqn and collapses all controllers to the
same subnqn to a single subsystem structure. This is good for
recognizing multiple controllers for the same subsystem. But with
the well-known discovery subnqn, the subsystems aren't truly the
same subsystem. As such, subsystem specific rules, such as no
overlap of controller id, do not apply. With today's behavior, the
check for overlap of controller id can fail, preventing the new
discovery controller from being created.

When searching for like subsystem nqn, exclude the discovery nqn
from matching. This will result in each discovery controller being
attached to a unique subsystem structure.

Signed-off-by: James Smart <jsmart2021@gmail.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Christoph Hellwig <hch@lst.de>

---
v2: add comment
---
 drivers/nvme/host/core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 4660505eded9..6960781a514e 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2358,6 +2358,17 @@ static struct nvme_subsystem *__nvme_find_get_subsystem(const char *subsysnqn)
 
 	lockdep_assert_held(&nvme_subsystems_lock);
 
+	/*
+	 * Fail matches for discovery subsystems. This results
+	 * in each discovery controller bound to a unique subsystem.
+	 * This avoids issues with validating controller values
+	 * that can only be true when there is a single unique subsystem.
+	 * There may be multiple and completely independent entities
+	 * that provide discovery controllers.
+	 */
+	if (!strcmp(subsysnqn, NVME_DISC_SUBSYS_NAME))
+		return NULL;
+
 	list_for_each_entry(subsys, &nvme_subsystems, entry) {
 		if (strcmp(subsys->subnqn, subsysnqn))
 			continue;
-- 
2.13.7


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] nvme: Treat discovery subsystems as unique subsystems
  2019-09-03 21:20 [PATCH v2] nvme: Treat discovery subsystems as unique subsystems James Smart
@ 2019-09-04 12:46 ` Christoph Hellwig
  2019-09-04 14:08 ` Max Gurtovoy
  1 sibling, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2019-09-04 12:46 UTC (permalink / raw)
  To: James Smart; +Cc: linux-nvme

On Tue, Sep 03, 2019 at 02:20:37PM -0700, James Smart wrote:
> Current code matches subnqn and collapses all controllers to the
> same subnqn to a single subsystem structure. This is good for
> recognizing multiple controllers for the same subsystem. But with
> the well-known discovery subnqn, the subsystems aren't truly the
> same subsystem. As such, subsystem specific rules, such as no
> overlap of controller id, do not apply. With today's behavior, the
> check for overlap of controller id can fail, preventing the new
> discovery controller from being created.
> 
> When searching for like subsystem nqn, exclude the discovery nqn
> from matching. This will result in each discovery controller being
> attached to a unique subsystem structure.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] nvme: Treat discovery subsystems as unique subsystems
  2019-09-03 21:20 [PATCH v2] nvme: Treat discovery subsystems as unique subsystems James Smart
  2019-09-04 12:46 ` Christoph Hellwig
@ 2019-09-04 14:08 ` Max Gurtovoy
  1 sibling, 0 replies; 3+ messages in thread
From: Max Gurtovoy @ 2019-09-04 14:08 UTC (permalink / raw)
  To: James Smart, linux-nvme


On 9/4/2019 12:20 AM, James Smart wrote:
> Current code matches subnqn and collapses all controllers to the
> same subnqn to a single subsystem structure. This is good for
> recognizing multiple controllers for the same subsystem. But with
> the well-known discovery subnqn, the subsystems aren't truly the
> same subsystem. As such, subsystem specific rules, such as no
> overlap of controller id, do not apply. With today's behavior, the
> check for overlap of controller id can fail, preventing the new
> discovery controller from being created.
>
> When searching for like subsystem nqn, exclude the discovery nqn
> from matching. This will result in each discovery controller being
> attached to a unique subsystem structure.
>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> ---
> v2: add comment
> ---
>   drivers/nvme/host/core.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 4660505eded9..6960781a514e 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2358,6 +2358,17 @@ static struct nvme_subsystem *__nvme_find_get_subsystem(const char *subsysnqn)
>   
>   	lockdep_assert_held(&nvme_subsystems_lock);
>   
> +	/*
> +	 * Fail matches for discovery subsystems. This results
> +	 * in each discovery controller bound to a unique subsystem.
> +	 * This avoids issues with validating controller values
> +	 * that can only be true when there is a single unique subsystem.
> +	 * There may be multiple and completely independent entities
> +	 * that provide discovery controllers.
> +	 */
> +	if (!strcmp(subsysnqn, NVME_DISC_SUBSYS_NAME))
> +		return NULL;
> +
>   	list_for_each_entry(subsys, &nvme_subsystems, entry) {
>   		if (strcmp(subsys->subnqn, subsysnqn))
>   			continue;


Looks good,

Reviewed-by: Max Gurtovoy <maxg@mellanox.com>



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-03 21:20 [PATCH v2] nvme: Treat discovery subsystems as unique subsystems James Smart
2019-09-04 12:46 ` Christoph Hellwig
2019-09-04 14:08 ` Max Gurtovoy

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org linux-nvme@archiver.kernel.org
	public-inbox-index linux-nvme


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/ public-inbox