[PATCH 0/3] nvme: discovery controller discovery support

[PATCH 0/3] nvme: discovery controller discovery support

[Hannes Reinecke]

Hi all,

TPAR 8014 (discovery controller discovery) adds a new discovery
log page entry type '3' to describe the current discovery controller.
This patchset adds support for this for the nvme target.

As usual, comments and reviews are welcome.

Hannes Reinecke (3):
  nvmet: switch check for subsystem type
  nvme: add new discovery log page entry definitions
  nvmet: register discovery subsystem as 'current'

 drivers/nvme/target/admin-cmd.c   |  4 ++--
 drivers/nvme/target/core.c        |  8 +++++---
 drivers/nvme/target/discovery.c   | 13 ++++++++-----
 drivers/nvme/target/fabrics-cmd.c |  2 +-
 include/linux/nvme.h              | 13 +++++++++++--
 5 files changed, 27 insertions(+), 13 deletions(-)

-- 
2.29.2


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

[PATCH 1/3] nvmet: switch check for subsystem type

[Hannes Reinecke]

Invert the check for discovery subsystem type to allow for additional
discovery subsystem types.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/target/admin-cmd.c   | 4 ++--
 drivers/nvme/target/core.c        | 6 +++---
 drivers/nvme/target/fabrics-cmd.c | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 8546bae5244d..e98f3e277571 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -374,7 +374,7 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 
 	id->rab = 6;
 
-	id->cntrltype = ctrl->subsys->type == NVME_NQN_DISC ?
+	id->cntrltype = ctrl->subsys->type != NVME_NQN_NVME ?
 		NVME_CTRL_DISC : NVME_CTRL_IO;
 
 	/*
@@ -1011,7 +1011,7 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
 
 	if (nvme_is_fabrics(cmd))
 		return nvmet_parse_fabrics_cmd(req);
-	if (nvmet_req_subsys(req)->type == NVME_NQN_DISC)
+	if (nvmet_req_subsys(req)->type != NVME_NQN_NVME)
 		return nvmet_parse_discovery_cmd(req);
 
 	ret = nvmet_check_ctrl_status(req);
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 88ed746c675f..873b02d8c31c 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1140,7 +1140,7 @@ static void nvmet_start_ctrl(struct nvmet_ctrl *ctrl)
 	 * should verify iosqes,iocqes are zeroed, however that
 	 * would break backwards compatibility, so don't enforce it.
 	 */
-	if (ctrl->subsys->type != NVME_NQN_DISC &&
+	if (ctrl->subsys->type == NVME_NQN_NVME &&
 	    (nvmet_cc_iosqes(ctrl->cc) != NVME_NVM_IOSQES ||
 	     nvmet_cc_iocqes(ctrl->cc) != NVME_NVM_IOCQES)) {
 		ctrl->csts = NVME_CSTS_CFS;
@@ -1278,7 +1278,7 @@ bool nvmet_host_allowed(struct nvmet_subsys *subsys, const char *hostnqn)
 	if (subsys->allow_any_host)
 		return true;
 
-	if (subsys->type == NVME_NQN_DISC) /* allow all access to disc subsys */
+	if (subsys->type != NVME_NQN_NVME) /* allow all access to disc subsys */
 		return true;
 
 	list_for_each_entry(p, &subsys->hosts, entry) {
@@ -1411,7 +1411,7 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 	 * Discovery controllers may use some arbitrary high value
 	 * in order to cleanup stale discovery sessions
 	 */
-	if ((ctrl->subsys->type == NVME_NQN_DISC) && !kato)
+	if ((ctrl->subsys->type != NVME_NQN_NVME) && !kato)
 		kato = NVMET_DISC_KATO_MS;
 
 	/* keep-alive timeout in seconds */
diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c
index 822601103f29..d1e179959768 100644
--- a/drivers/nvme/target/fabrics-cmd.c
+++ b/drivers/nvme/target/fabrics-cmd.c
@@ -222,7 +222,7 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
 	}
 
 	pr_info("creating %s controller %d for subsystem %s for NQN %s%s.\n",
-		ctrl->subsys->type == NVME_NQN_DISC ? "discovery" : "nvm",
+		ctrl->subsys->type != NVME_NQN_NVME ? "discovery" : "nvm",
 		ctrl->cntlid, ctrl->subsys->subsysnqn, ctrl->hostnqn,
 		ctrl->pi_support ? " T10-PI is enabled" : "");
 	req->cqe->result.u16 = cpu_to_le16(ctrl->cntlid);
-- 
2.29.2


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

[PATCH 2/3] nvme: add new discovery log page entry definitions

[Hannes Reinecke]

TP8014 adds a new SUBTYPE value and a new field EFLAGS for the
discovery log page entry.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 include/linux/nvme.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index ed2428918bca..9e816fab1e58 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -27,8 +27,9 @@
 #define NVME_NSID_ALL		0xffffffff
 
 enum nvme_subsys_type {
-	NVME_NQN_DISC	= 1,		/* Discovery type target subsystem */
+	NVME_NQN_DISC	= 1,		/* Referral to another discovery type target subsystem */
 	NVME_NQN_NVME	= 2,		/* NVME type target subsystem */
+	NVME_NQN_CURR	= 3,		/* Current discovery type target subsystem */
 };
 
 enum nvme_ctrl_type {
@@ -1311,6 +1312,13 @@ struct nvmf_common_command {
 
 #define MAX_DISC_LOGS	255
 
+/* Discovery log page entry flags (EFLAGS): */
+enum {
+	NVME_DISC_EFLAGS_EPCSD		= (1 << 1),
+	NVME_DISC_EFLAGS_DUPRETINFO	= (1 << 0),
+};
+
+
 /* Discovery log page entry */
 struct nvmf_disc_rsp_page_entry {
 	__u8		trtype;
@@ -1320,7 +1328,8 @@ struct nvmf_disc_rsp_page_entry {
 	__le16		portid;
 	__le16		cntlid;
 	__le16		asqsz;
-	__u8		resv8[22];
+	__le16		eflags;
+	__u8		resv10[20];
 	char		trsvcid[NVMF_TRSVCID_SIZE];
 	__u8		resv64[192];
 	char		subnqn[NVMF_NQN_FIELD_LEN];
-- 
2.29.2


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

[PATCH 3/3] nvmet: register discovery subsystem as 'current'

[Hannes Reinecke]

Register the discovery subsystem as the 'current' discovery subsystem,
and add a new discovery log page entry for this.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/target/core.c      |  2 ++
 drivers/nvme/target/discovery.c | 13 ++++++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 873b02d8c31c..0e3de8670a27 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1539,6 +1539,8 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
 		subsys->max_qid = NVMET_NR_QUEUES;
 		break;
 	case NVME_NQN_DISC:
+		fallthrough;
+	case NVME_NQN_CURR:
 		subsys->max_qid = 0;
 		break;
 	default:
diff --git a/drivers/nvme/target/discovery.c b/drivers/nvme/target/discovery.c
index 7b360f8d07e9..a2f08c49f07c 100644
--- a/drivers/nvme/target/discovery.c
+++ b/drivers/nvme/target/discovery.c
@@ -146,7 +146,7 @@ static size_t discovery_log_entries(struct nvmet_req *req)
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	struct nvmet_subsys_link *p;
 	struct nvmet_port *r;
-	size_t entries = 0;
+	size_t entries = 1;
 
 	list_for_each_entry(p, &req->port->subsystems, entry) {
 		if (!nvmet_host_allowed(p->subsys, ctrl->hostnqn))
@@ -171,6 +171,7 @@ static void nvmet_execute_disc_get_log_page(struct nvmet_req *req)
 	u32 numrec = 0;
 	u16 status = 0;
 	void *buffer;
+	char traddr[NVMF_TRADDR_SIZE];
 
 	if (!nvmet_check_transfer_len(req, data_len))
 		return;
@@ -204,14 +205,16 @@ static void nvmet_execute_disc_get_log_page(struct nvmet_req *req)
 		goto out;
 	}
 
+	nvmet_set_disc_traddr(req, req->port, traddr);
 	hdr = buffer;
+	nvmet_format_discovery_entry(hdr, req->port,
+				     nvmet_disc_subsys->subsysnqn,
+				     traddr, NVME_NQN_CURR, numrec);
+	numrec++;
 	list_for_each_entry(p, &req->port->subsystems, entry) {
-		char traddr[NVMF_TRADDR_SIZE];
-
 		if (!nvmet_host_allowed(p->subsys, ctrl->hostnqn))
 			continue;
 
-		nvmet_set_disc_traddr(req, req->port, traddr);
 		nvmet_format_discovery_entry(hdr, req->port,
 				p->subsys->subsysnqn, traddr,
 				NVME_NQN_NVME, numrec);
@@ -389,7 +392,7 @@ u16 nvmet_parse_discovery_cmd(struct nvmet_req *req)
 int __init nvmet_init_discovery(void)
 {
 	nvmet_disc_subsys =
-		nvmet_subsys_alloc(NVME_DISC_SUBSYS_NAME, NVME_NQN_DISC);
+		nvmet_subsys_alloc(NVME_DISC_SUBSYS_NAME, NVME_NQN_CURR);
 	return PTR_ERR_OR_ZERO(nvmet_disc_subsys);
 }
 
-- 
2.29.2


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

Re: [PATCH 0/3] nvme: discovery controller discovery support

[Christoph Hellwig]

Can you resend this against nvme-5.16?  It won't apply as-is.

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

[PATCHv2 0/3] nvmet: discovery controller discovery support

[Hannes Reinecke]

TPAR 8014 (discovery controller discovery) adds a new discovery
log page entry type '3' to describe the current discovery controller.
This patchset adds support for this to the nvme target.

As usual, comments and reviews are welcome.

Changes to v1:
- Rebased to nvme-5.16

Hannes Reinecke (3):
  nvmet: switch check for subsystem type
  nvme: add new discovery log page entry definitions
  nvmet: register discovery subsystem as 'current'

 drivers/nvme/target/core.c      |  2 ++
 drivers/nvme/target/discovery.c | 13 ++++++++-----
 drivers/nvme/target/nvmet.h     |  2 +-
 include/linux/nvme.h            | 12 ++++++++++--
 4 files changed, 21 insertions(+), 8 deletions(-)

-- 
2.29.2

[PATCH 1/3] nvmet: switch check for subsystem type

[Hannes Reinecke]

Invert the check for discovery subsystem type to allow for additional
discovery subsystem types.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/target/nvmet.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index f31dcc4fb1a2..af193423c10b 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -579,7 +579,7 @@ static inline struct nvmet_subsys *nvmet_req_subsys(struct nvmet_req *req)
 
 static inline bool nvmet_is_disc_subsys(struct nvmet_subsys *subsys)
 {
-    return subsys->type == NVME_NQN_DISC;
+    return subsys->type != NVME_NQN_NVME;
 }
 
 #ifdef CONFIG_NVME_TARGET_PASSTHRU
-- 
2.29.2

Re: [PATCH 1/3] nvmet: switch check for subsystem type

[Chaitanya Kulkarni]

On 10/18/2021 8:21 AM, Hannes Reinecke wrote:
> Invert the check for discovery subsystem type to allow for additional
> discovery subsystem types.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   drivers/nvme/target/nvmet.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index f31dcc4fb1a2..af193423c10b 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -579,7 +579,7 @@ static inline struct nvmet_subsys *nvmet_req_subsys(struct nvmet_req *req)
>   
>   static inline bool nvmet_is_disc_subsys(struct nvmet_subsys *subsys)
>   {
> -    return subsys->type == NVME_NQN_DISC;
> +    return subsys->type != NVME_NQN_NVME;

shouldn't we use switch and make code more readable for allowed 
subsys->type values ? unless there is a reason ..

>   }
>   
>   #ifdef CONFIG_NVME_TARGET_PASSTHRU
> 

Re: [PATCH 1/3] nvmet: switch check for subsystem type

[Hannes Reinecke]

On 10/19/21 8:37 AM, Chaitanya Kulkarni wrote:
> On 10/18/2021 8:21 AM, Hannes Reinecke wrote:
>> Invert the check for discovery subsystem type to allow for additional
>> discovery subsystem types.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>    drivers/nvme/target/nvmet.h | 2 +-
>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
>> index f31dcc4fb1a2..af193423c10b 100644
>> --- a/drivers/nvme/target/nvmet.h
>> +++ b/drivers/nvme/target/nvmet.h
>> @@ -579,7 +579,7 @@ static inline struct nvmet_subsys *nvmet_req_subsys(struct nvmet_req *req)
>>    
>>    static inline bool nvmet_is_disc_subsys(struct nvmet_subsys *subsys)
>>    {
>> -    return subsys->type == NVME_NQN_DISC;
>> +    return subsys->type != NVME_NQN_NVME;
> 
> shouldn't we use switch and make code more readable for allowed
> subsys->type values ? unless there is a reason ..
> 

Well, with this patch we still have only two values; introducing a 
switch for this feels a bit silly.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer