Skip to contentSkip to conversation

nvme-cli: add minimal ana-log page support

PATCH Superseded Chaitanya Kulkarni 3 participants 7 messages Jul 25, 2018 – Jul 26, 2018

Subsystems (2)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 nvme-cli: add minimal ana-log page support

This patch adds a new command to retrieve the ANA Log page. We update identify ctrl/ns data structure to support this command. We also add ana based error codes and different identifiers to the linux/nvme.h header file in order to support this command.

Signed-off-by Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 linux/nvme.h   | 52 ++++++++++++++++++++++++++++++++++++---
 nvme-builtin.h |  1 +
 nvme-ioctl.c   |  8 ++++++
 nvme-ioctl.h   |  1 +
 nvme-print.c   | 66 +++++++++++++++++++++++++++++++++++++++++++++++++-
 nvme-print.h   |  1 +
 nvme.c         | 47 +++++++++++++++++++++++++++++++++++
 7 files changed, 171 insertions(+), 5 deletions(-)
linux/nvme.h +48 -4
index dc8ea89..50faaf0 100644
@@ -241,7 +241,12 @@ struct nvme_id_ctrl {
 	__le32			hmminds;
 	__le16			hmmaxd;
 	__le16			nsetidmax;
-	__u8			rsvd340[172];
+	__u8			rsvd340[2];
+	__u8			anatt;
+	__u8			anacap;
+	__le32			anagrpmax;
+	__le32			nanagrpid;
+	__u8			rsvd352[160];
 	__u8			sqes;
 	__u8			cqes;
 	__le16			maxcmd;
@@ -257,7 +262,8 @@ struct nvme_id_ctrl {
 	__le16			acwu;
 	__u8			rsvd534[2];
 	__le32			sgls;
-	__u8			rsvd540[228];
+	__u8			mnan;
+	__u8			rsvd544[224];
 	char			subnqn[256];
 	__u8			rsvd1024[768];
 	__le32			ioccsz;
@@ -317,7 +323,9 @@ struct nvme_id_ns {
 	__le16			nabspf;
 	__le16			noiob;
 	__u8			nvmcap[16];
-	__u8			rsvd64[36];
+	__u8			rsvd64[28];
+	__le32			anagrpid;
+	__u8			rsvd95[4];
 	__le16			nvmsetid;
 	__le16			endgid;
 	__u8			nguid[16];
@@ -516,6 +524,32 @@ struct nvme_effects_log {
 	__u8   resv[2048];
 };
 
+enum nvme_ana_state {
+	NVME_ANA_OPTIMIZED		= 0x01,
+	NVME_ANA_NONOPTIMIZED		= 0x02,
+	NVME_ANA_INACCESSIBLE		= 0x03,
+	NVME_ANA_PERSISTENT_LOSS	= 0x04,
+	NVME_ANA_CHANGE			= 0x0f,
+};
+
+struct nvme_ana_group_desc {
+	__le32  grpid;
+	__le32  nnsids;
+	__le64  chgcnt;
+	__u8    state;
+	__u8    rsvd17[7];
+	__le32  nsids[];
+};
+
+/* flag for the log specific field of the ANA log */
+#define NVME_ANA_LOG_RGO   (1 << 0)
+
+struct nvme_ana_rsp_hdr {
+	__le64  chgcnt;
+	__le16  ngrps;
+	__le16  rsvd10[3];
+};
+
 enum {
 	NVME_SMART_CRIT_SPARE		= 1 << 0,
 	NVME_SMART_CRIT_TEMPERATURE	= 1 << 1,
@@ -531,6 +565,7 @@ enum {
 	NVME_AER_VS			= 7,
 	NVME_AER_NOTICE_NS_CHANGED	= 0x0002,
 	NVME_AER_NOTICE_FW_ACT_STARTING = 0x0102,
+	NVME_AER_NOTICE_ANA		= 0x03,
 };
 
 struct nvme_lba_range_type {
@@ -881,6 +916,7 @@ enum {
 	NVME_LOG_TELEMETRY_HOST = 0x07,
 	NVME_LOG_TELEMETRY_CTRL = 0x08,
 	NVME_LOG_ENDURANCE_GROUP = 0x09,
+	NVME_LOG_ANA		= 0x0c,
 	NVME_LOG_DISC		= 0x70,
 	NVME_LOG_RESERVATION	= 0x80,
 	NVME_LOG_SANITIZE	= 0x81,
@@ -892,6 +928,7 @@ enum {
 enum {
 	NVME_NO_LOG_LSP       = 0x0,
 	NVME_NO_LOG_LPO       = 0x0,
+	NVME_LOG_ANA_RGO      = 0x1,
 	NVME_TELEM_LSP_CREATE = 0x1,
 };
 
@@ -1040,7 +1077,7 @@ struct nvme_get_log_page_command {
 	__u64			rsvd2[2];
 	union nvme_data_ptr	dptr;
 	__u8			lid;
-	__u8			rsvd10;
+	__u8			lsp;
 	__le16			numdl;
 	__le16			numdu;
 	__u16			rsvd11;
@@ -1354,6 +1391,13 @@ enum {
 	NVME_SC_ACCESS_DENIED		= 0x286,
 	NVME_SC_UNWRITTEN_BLOCK		= 0x287,
 
+	/*
+	 * Path-related Errors:
+	 */
+	NVME_SC_ANA_PERSISTENT_LOSS	= 0x301,
+	NVME_SC_ANA_INACCESSIBLE	= 0x302,
+	NVME_SC_ANA_TRANSITION		= 0x303,
+
 	NVME_SC_DNR			= 0x4000,
 };
 
nvme-builtin.h +1 -0
index 2c26d57..f9e47b3 100644
@@ -25,6 +25,7 @@ COMMAND_LIST(
 	ENTRY("fw-log", "Retrieve FW Log, show it", get_fw_log)
 	ENTRY("changed-ns-list-log", "Retrieve Changed Namespace List, show it", get_changed_ns_list_log)
 	ENTRY("smart-log", "Retrieve SMART Log, show it", get_smart_log)
+	ENTRY("ana-log", "Retrieve ANA Log, show it", get_ana_log)
 	ENTRY("error-log", "Retrieve Error Log, show it", get_error_log)
 	ENTRY("effects-log", "Retrieve Command Effects Log, show it", get_effects_log)
 	ENTRY("endurance-log", "Retrieve Endurance Group Log, show it", get_endurance_log)
nvme-ioctl.c +8 -0
index 9cf2a33..0ebdbf4 100644
@@ -467,6 +467,14 @@ int nvme_smart_log(int fd, __u32 nsid, struct nvme_smart_log *smart_log)
 	return nvme_get_log(fd, nsid, NVME_LOG_SMART, sizeof(*smart_log), smart_log);
 }
 
+int nvme_ana_log(int fd, void *ana_log, size_t ana_log_len, int rgo)
+{
+	__u64 lpo = 0;
+
+	return nvme_get_log13(fd, NVME_NSID_ALL, NVME_LOG_ANA, rgo, lpo, 0, 0,
+			ana_log_len, ana_log);
+}
+
 int nvme_self_test_log(int fd, struct nvme_self_test_log *self_test_log)
 {
 	return nvme_get_log(fd, NVME_NSID_ALL, NVME_LOG_DEVICE_SELF_TEST,
nvme-ioctl.h +1 -0
index b34f5de..d3823a4 100644
@@ -92,6 +92,7 @@ int nvme_changed_ns_list_log(int fd,
 		struct nvme_changed_ns_list_log *changed_ns_list_log);
 int nvme_error_log(int fd, int entries, struct nvme_error_log_page *err_log);
 int nvme_smart_log(int fd, __u32 nsid, struct nvme_smart_log *smart_log);
+int nvme_ana_log(int fd, void *ana_log, size_t ana_log_len, int rgo);
 int nvme_effects_log(int fd, struct nvme_effects_log_page *effects_log);
 int nvme_discovery_log(int fd, struct nvmf_disc_rsp_page_hdr *log, __u32 size);
 int nvme_sanitize_log(int fd, struct nvme_sanitize_log_page *sanitize_log);
nvme-print.c +65 -1

114 lines not on this page · raw message

nvme-print.h +1 -0
index da287c2..5f3e62c 100644
@@ -24,6 +24,7 @@ void show_nvme_resv_report(struct nvme_reservation_status *status, int bytes, __
 void show_lba_range(struct nvme_lba_range_type *lbrt, int nr_ranges);
 void show_error_log(struct nvme_error_log_page *err_log, int entries, const char *devname);
 void show_smart_log(struct nvme_smart_log *smart, unsigned int nsid, const char *devname);
+void show_ana_log(struct nvme_ana_rsp_hdr *ana_log, const char *devname);
 void show_self_test_log(struct nvme_self_test_log *self_test, const char *devname);
 void show_fw_log(struct nvme_firmware_log_page *fw_log, const char *devname);
 void show_effects_log(struct nvme_effects_log_page *effects, unsigned int flags);
nvme.c +47 -0

54 lines not on this page · raw message

Signature · 1 line
2.17.0
#1 lore raw

PATCH 2/2 nvme-cli add ana-log documentation

Signed-off-by Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 Documentation/nvme-ana-log.1    |  71 +++
 Documentation/nvme-ana-log.html | 800 ++++++++++++++++++++++++++++++++
 Documentation/nvme-ana-log.txt  |  36 ++
 3 files changed, 907 insertions(+)
 create mode 100644 Documentation/nvme-ana-log.1
 create mode 100644 Documentation/nvme-ana-log.html
 create mode 100644 Documentation/nvme-ana-log.txt
Documentation/nvme-ana-log.1 new file +71 -0
new file mode 100644
index 0000000..21fa0ab
@@ -0,0 +1,71 @@
+'\" t
+.\"     Title: nvme-ana-log
+.\"    Author: [FIXME: author] [see http://docbook.sf.net/el/author]
+.\" Generator: DocBook XSL Stylesheets v1.78.1 <http://docbook.sf.net/>
+.\"      Date: 07/24/2018
+.\"    Manual: NVMe Manual
+.\"    Source: NVMe
+.\"  Language: English
+.\"
+.TH "NVME\-ANA\-LOG" "1" "07/24/2018" "NVMe" "NVMe Manual"
+.\" -----------------------------------------------------------------
+.\" * Define some portability stuff
+.\" -----------------------------------------------------------------
+.\" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+.\" http://bugs.debian.org/507673
+.\" http://lists.gnu.org/archive/html/groff/2009-02/msg00013.html
+.\" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+.ie \n(.g .ds Aq \(aq
+.el       .ds Aq '
+.\" -----------------------------------------------------------------
+.\" * set default formatting
+.\" -----------------------------------------------------------------
+.\" disable hyphenation
+.nh
+.\" disable justification (adjust text to left margin only)
+.ad l
+.\" -----------------------------------------------------------------
+.\" * MAIN CONTENT STARTS HERE *
+.\" -----------------------------------------------------------------
+.SH "NAME"
+nvme-ana-log \- Send NVMe ANA log page request, returns result and log
+.SH "SYNOPSIS"
+.sp
+.nf
+\fInvme ana\-log\fR <device>
+.fi
+.SH "DESCRIPTION"
+.sp
+Retrieves the NVMe Asymmetric Namespace Access log page from an NVMe device and provides the returned structure\&.
+.sp
+The <device> parameter is mandatory NVMe character device (ex: /dev/nvme0)\&.
+.sp
+On success, the returned ANA log structure may be returned and printed in a readable format\&.
+.SH "OPTIONS"
+.sp
+None\&.
+.SH "EXAMPLES"
+.sp
+.RS 4
+.ie n \{\
+\h'-04'\(bu\h'+03'\c
+.\}
+.el \{\
+.sp -1
+.IP \(bu 2.3
+.\}
+Print the ANA log page in a human readable format:
+.RE
+.sp
+.if n \{\
+.RS 4
+.\}
+.nf
+# nvme ana\-log /dev/nvme0
+.fi
+.if n \{\
+.RE
+.\}
+.SH "NVME"
+.sp
+Part of the nvme\-user suite
Documentation/nvme-ana-log.html new file +800 -0

800 lines not on this page · raw message

Documentation/nvme-ana-log.txt new file +36 -0
new file mode 100644
index 0000000..73abdf2
@@ -0,0 +1,36 @@
+nvme-ana-log(1)
+===============
+
+NAME
+----
+nvme-ana-log - Send NVMe ANA log page request, returns result and log
+
+SYNOPSIS
+--------
+[verse]
+'nvme ana-log' <device>
+
+DESCRIPTION
+-----------
+Retrieves the NVMe Asymmetric Namespace Access log page from an NVMe device
+and provides the returned structure.
+
+The <device> parameter is mandatory NVMe character device (ex: /dev/nvme0).
+
+On success, the returned ANA log structure may be returned and printed in
+a readable format.
+
+OPTIONS
+-------
+None.
+
+EXAMPLES
+--------
+* Print the ANA log page in a human readable format:
+------------
+# nvme ana-log /dev/nvme0
+------------
+
+NVME
+----
+Part of the nvme-user suite
Signature · 1 line
2.17.0
#2 lore raw

PATCH 1/2 nvme-cli: add minimal ana-log page support

In reply to Chaitanya Kulkarni
Quoted 4 lines · Chaitanya Kulkarni

This patch adds a new command to retrieve the ANA Log page. We update identify ctrl/ns data structure to support this command. We also add ana based error codes and different identifiers to the linux/nvme.h header file in order to support this command.

One thing I am a little concerned about here is that reading the ANA log page may clear the ANA AEN. This actually is the same for other log pages that clear AENs, but for ANA the effects are much worse.

Keith, wat do you think of always setting the RAE bit for log pages associated with AENs? The only real issue I can see is that the bit is only defined for NVMe 1.3+ so we'd need to figure out the version somehow. Maybe with a new ioctl so that we don't have to send an identify every time.

We also really need the RAE bit implemented in the Linux target. This should be very simple but I haven't got to it yet. Chaitanya, do you want to give it a quick spin independent of this discussion?

And now some comments on the patch itself:

Quoted 1 line

+const char * nvme_ana_state_to_string(int state)

No space after the '*', please. Also the type of state should be 'enum nvme_ana_state'.

Quoted 6 lines
nvme-print.c +6 -0
+{
+	switch (state) {
+	case NVME_ANA_OPTIMIZED:
+		return "OPTIMIZED";
+	case NVME_ANA_NONOPTIMIZED:
+		return "NON-OPTIMIZED";

nvme-print.c void show_smart_log(struct nvme_smart_log *smart, unsigned int nsid, const char · line 1350

Quoted 10 lines
nvme-print.c +10 -0
+	case NVME_ANA_INACCESSIBLE:
+		return "INACCESSIBLE";
+	case NVME_ANA_PERSISTENT_LOSS:
+		return "PERSISTENT-LOSS";
+	case NVME_ANA_CHANGE:
+		return "CHANGE";
+	default:
+		return "UNKNOWN";
+	}
+}

Is there precedene for SHOUTING elsewhere in nvme-clu? Otherwise I'd suggest to print these in lower case and match what is in the kernel sysfs files.

Quoted 1 line

+void show_ana_log(struct nvme_ana_rsp_hdr *ana_log, const char *devname)

The devname argument seems unused.

Quoted 15 lines
nvme-print.c +15 -0
+{
+	int offset = sizeof(struct nvme_ana_rsp_hdr);
+	void *base = ana_log;
+	struct nvme_ana_rsp_hdr *hdr = base;
+	struct nvme_ana_group_desc *desc;
+	size_t nsid_buf_size;
+	__u32 nr_nsids;
+	int i;
+	int j;
+
+	printf("ANA LOG HEADER :-\n");
+	printf("chgcnt	:	%"PRIu64"\n", (uint64_t)le64_to_cpu(hdr->chgcnt));
+	printf("ngrps	:	%u\n", le16_to_cpu(hdr->ngrps));
+	printf("ANA Log Desc :-\n");
+

nvme-print.c void show_smart_log(struct nvme_smart_log *smart, unsigned int nsid, const char · line 1377

Quoted 10 lines
nvme-print.c +10 -0
+	for (i = 0; i < le16_to_cpu(ana_log->ngrps); i++) {
+		desc = base + offset;
+		nr_nsids = le32_to_cpu(desc->nnsids);
+		nsid_buf_size = nr_nsids * sizeof(__le32);
+
+		offset += sizeof(*desc);
+		printf("grpid	:	%u\n", le32_to_cpu(desc->grpid));
+		printf("nnsids	:	%u\n", le32_to_cpu(desc->nnsids));
+		printf("chgcnt	:	%llu\n", le64_to_cpu(desc->chgcnt));
+		printf("state	:	%s\n", nvme_ana_state_to_string(desc->state));

I think we also need a json version of all this, don't we?

Quoted 1 line

+static int get_ana_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)

Overly long line.

#3 lore raw

PATCH 1/2 nvme-cli: add minimal ana-log page support

From: Christoph Hellwig <[email protected]>
Sent: Tuesday, July 24, 2018 9:53 PM
To: Chaitanya Kulkarni
Cc: linux-nvme at lists.infradead.org; hch at lst.de; keith.busch at intel.com
Subject: Re: [PATCH 1/2] nvme-cli: add minimal ana-log page support ?

Quoted 4 lines · Chaitanya Kulkarni

This patch adds a new command to retrieve the ANA Log page. We update identify ctrl/ns data structure to support this command. We also add ana based error codes and different identifiers to the linux/nvme.h header file in order to support this command.

One thing I am a little concerned about here is that reading the ANA log page may clear the ANA AEN.? This actually is the same for other log pages that clear AENs, but for ANA the effects are much worse.

Keith, wat do you think of always setting the RAE bit for log pages associated with AENs?? The only real issue I can see is that the bit is only defined for NVMe 1.3+ so we'd need to figure out the version somehow.? Maybe with a new ioctl so that we don't have to send an identify every time.

We also really need the RAE bit implemented in the Linux target. This should be very simple but I haven't got to it yet.? Chaitanya, do you want to give it a quick spin independent of this discussion?

[CK] Yes.

And now some comments on the patch itself:

Quoted 1 line

+const char * nvme_ana_state_to_string(int state)

No space after the '*', please.? Also the type of state should be 'enum nvme_ana_state'.

Quoted 6 lines
(quoted diff) +6 -0
+{
+???? switch (state) {
+???? case NVME_ANA_OPTIMIZED:
+???????????? return "OPTIMIZED";
+???? case NVME_ANA_NONOPTIMIZED:
+???????????? return "NON-OPTIMIZED";

Review

Quoted 10 lines
(quoted diff) +10 -0
+???? case NVME_ANA_INACCESSIBLE:
+???????????? return "INACCESSIBLE";
+???? case NVME_ANA_PERSISTENT_LOSS:
+???????????? return "PERSISTENT-LOSS";
+???? case NVME_ANA_CHANGE:
+???????????? return "CHANGE";
+???? default:
+???????????? return "UNKNOWN";
+???? }
+}

Is there precedene for SHOUTING elsewhere in nvme-clu?? Otherwise I'd suggest to print these in lower case and match what is in the kernel sysfs files.

Quoted 1 line

+void show_ana_log(struct nvme_ana_rsp_hdr *ana_log, const char *devname)

The devname argument seems unused.

Quoted 15 lines
(quoted diff) +15 -0
+{
+???? int offset = sizeof(struct nvme_ana_rsp_hdr);
+???? void *base = ana_log;
+???? struct nvme_ana_rsp_hdr *hdr = base;
+???? struct nvme_ana_group_desc *desc;
+???? size_t nsid_buf_size;
+???? __u32 nr_nsids;
+???? int i;
+???? int j;
+
+???? printf("ANA LOG HEADER :-\n");
+???? printf("chgcnt? :?????? %"PRIu64"\n", (uint64_t)le64_to_cpu(hdr->chgcnt));
+???? printf("ngrps?? :?????? %u\n", le16_to_cpu(hdr->ngrps));
+???? printf("ANA Log Desc :-\n");
+

Review

Quoted 10 lines
(quoted diff) +10 -0
+???? for (i = 0; i < le16_to_cpu(ana_log->ngrps); i++) {
+???????????? desc = base + offset;
+???????????? nr_nsids = le32_to_cpu(desc->nnsids);
+???????????? nsid_buf_size = nr_nsids * sizeof(__le32);
+
+???????????? offset += sizeof(*desc);
+???????????? printf("grpid?? :?????? %u\n", le32_to_cpu(desc->grpid));
+???????????? printf("nnsids? :?????? %u\n", le32_to_cpu(desc->nnsids));
+???????????? printf("chgcnt? :?????? %llu\n", le64_to_cpu(desc->chgcnt));
+???????????? printf("state?? :?????? %s\n", nvme_ana_state_to_string(desc->state));

I think we also need a json version of all this, don't we?

[CK] Yes, we also need a binary mode to be compatible with other log commands, I'll add both it in the next version.

Quoted 1 line

+static int get_ana_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)

Overly long line.

[CK] Thanks for the comments, I'll fix all these in the next version.

#4 lore raw

PATCH 1/2 nvme-cli: add minimal ana-log page support

In reply to Christoph Hellwig
Quoted 11 lines · Christoph Hellwig
Quoted 4 lines · Chaitanya Kulkarni

This patch adds a new command to retrieve the ANA Log page. We update identify ctrl/ns data structure to support this command. We also add ana based error codes and different identifiers to the linux/nvme.h header file in order to support this command.

One thing I am a little concerned about here is that reading the ANA log page may clear the ANA AEN. This actually is the same for other log pages that clear AENs, but for ANA the effects are much worse.

Quoted 5 lines

Keith, wat do you think of always setting the RAE bit for log pages associated with AENs? The only real issue I can see is that the bit is only defined for NVMe 1.3+ so we'd need to figure out the version somehow. Maybe with a new ioctl so that we don't have to send an identify every time.

Adding the RAE option for user space tooling is a good idea. Defaulting to on should be safe too (will confirm on older drives), but does re-arming the AEN really create a problem for ANA? I thought the log page problem was when reading a page changes the result of the next read, but that only applies to very few logs.

#5 lore raw

PATCH 1/2 nvme-cli: add minimal ana-log page support

Quoted 2 lines · Keith Busch

Adding the RAE option for user space tooling is a good idea. Defaulting to on should be safe too (will confirm on older drives),

Is it? Strictly speaking the bit is reserved for drivers < 1.3, so they could reject it, even if I doubt anyone does.

Quoted 4 lines

but does re-arming
the AEN really create a problem for ANA? I thought the log page problem was when reading a page changes the result of the next read, but that only applies to very few logs.

Yes, for ANA we should be fine. The only real problem was the changed namespace log, and we stopped relying on that.

#6 lore raw

PATCH 1/2 nvme-cli: add minimal ana-log page support

Quoted 6 lines · Christoph Hellwig
Quoted 2 lines · Keith Busch

Adding the RAE option for user space tooling is a good idea. Defaulting to on should be safe too (will confirm on older drives),

Is it? Strictly speaking the bit is reserved for drivers < 1.3, so they could reject it, even if I doubt anyone does.

Oh, duh ... the definition of "reserved":

Receipt of reserved coded values in defined fields in commands shall be reported as an error.

You're right.