nvme-cli: add minimal ana-log page support
Subsystems (2)
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.
--- 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
PATCH 2/2 nvme-cli add ana-log documentation
--- 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
@@ -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
@@ -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
Christoph Hellwig
maintainerPATCH 1/2 nvme-cli: add minimal ana-log page support
In reply to Chaitanya KulkarniQuoted 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.hheader 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.
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.hheader 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.
PATCH 1/2 nvme-cli: add minimal ana-log page support
In reply to Christoph HellwigQuoted 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.hheader 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.
Christoph Hellwig
maintainerPATCH 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.
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.