Improve readbility of NVME "wwid" attribute
Subsystems (3)
v2 cover Improve readbility of NVME "wwid" attribute
With the current implementation, the default "fallback" WWID generation code (if no nguid, euid etc. are defined) for Linux NVME host and target results in the following WWID format:
nvme.0000-3163653363666438366239656630386200-4c696e75780000000000000000000000000000000000000000000000000000000000000000000000-00000002
This is not only hard to read, it poses real problems e.g. for multipath (dm WWIDs are limited to 128 characters).
With this patch series, the WWID on a Linux host connected to a Linux target looks like this:
nvme.0000-65613435333665653738613464363961-4c696e7578-00000001
Changes wrt v1:
* 1/3: new, moved helper to include/linux/string.h (Christoph Hellwig) (you suggested kernel.h, but I think this matches string.h better)
* Dropped the last patch from the v1 series that would have changed valid WWIDs for HW NVME controllers.
Martin Wilck (3):
string.h: add memcpy_and_pad()
nvmet: identify controller: improve standard compliance
nvme: wwid_show: strip trailing 0-bytes
drivers/nvme/host/core.c | 6 ++++-- drivers/nvme/target/admin-cmd.c | 13 ++++++------- include/linux/string.h | 30 ++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 9 deletions(-)
Signature · 1 line
2.13.2
PATCH v2 1/3 string.h: add memcpy_and_pad()
This helper function is useful for the nvme subsystem, and maybe others.
--- include/linux/string.h | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
include/linux/string.h +30 -0
@@ -200,6 +200,7 @@ static inline const char *kbasename(const char *path)void fortify_panic (const char *name)__noreturn __cold ; void __read_overflow(void) __compiletime_error("detected read beyond size of object passed as 1st parameter"); void __read_overflow2(void) __compiletime_error("detected read beyond size of object passed as 2nd parameter"); +void __read_overflow3(void) __compiletime_error("detected read beyond size of object passed as 3rd parameter"); void __write_overflow(void) __compiletime_error("detected write beyond size of object passed as 1st parameter");#if !defined(__NO_FORTIFY) && defined(__OPTIMIZE__) && defined(CONFIG_FORTIFY_SOURCE )
@@ -395,4 +396,33 @@ __FORTIFY_INLINE char *strcpy(char *p, const char *q)#endif +/** +* memcpy_and_pad - Copy one buffer to another with padding +* @dest: Where to copy to +* @dest_len: The destination buffer size +* @src: Where to copy from +* @count: The number of bytes to copy +* @pad: Character to use for padding if space is left in destination. +*/ +__FORTIFY_INLINE void memcpy_and_pad(void *dest, size_t dest_len, +const void *src,size_t count,int pad) +{ +size_t dest_size =__builtin_object_size (dest,0 ); +size_t src_size =__builtin_object_size (src,0 ); + +if (__builtin_constant_p (dest_len) &&__builtin_constant_p (count)) { +if (dest_size < dest_len && dest_size < count) +__write_overflow (); +else if (src_size < dest_len && src_size < count) +__read_overflow3 (); + } +if (dest_size < dest_len) +fortify_panic (__func__); +if (dest_len > count) { +memcpy (dest, src, count); +memset (dest + count, pad, dest_len - count); + }else +memcpy (dest, src, dest_len); +} +#endif /* _LINUX_STRING_H_ */
Signature · 1 line
2.13.2
PATCH v2 2/3 nvmet: identify controller: improve standard compliance
In reply to Martin WilckThe NVME standard mandates that the SN, MN, and FR fields of the Indentify Controller Data Structure be "ASCII strings". That means that they may not contain 0-bytes, not even string terminators.
--- drivers/nvme/target/admin-cmd.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
drivers/nvme/target/admin-cmd.c +6 -7
@@ -173,6 +173,7 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)struct nvmet_ctrl *ctrl = req->sq->ctrl;struct nvme_id_ctrl *id;u16 status =0 ; +const char MODEL [] ="Linux" ; id =kzalloc (sizeof (*id),GFP_KERNEL );if (!id) {
@@ -184,14 +185,12 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req) id->vid =0 ; id->ssvid =0 ; -memset (id->sn,' ' ,sizeof (id->sn)); -snprintf (id->sn,sizeof (id->sn),"%llx" , ctrl->serial); +bin2hex (id->sn, &ctrl->serial,min (sizeof (ctrl->serial), +sizeof (id->sn) /2 )); -memset (id->mn,' ' ,sizeof (id->mn)); -strncpy ((char *)id->mn,"Linux" ,sizeof (id->mn)); - -memset (id->fr,' ' ,sizeof (id->fr)); -strncpy ((char *)id->fr,UTS_RELEASE ,sizeof (id->fr)); +memcpy_and_pad (id->mn,sizeof (id->mn),MODEL ,sizeof (MODEL ) -1 ,' ' ); +memcpy_and_pad (id->fr,sizeof (id->fr), +UTS_RELEASE ,strlen (UTS_RELEASE ),' ' ); id->rab =6 ;
Signature · 1 line
2.13.2
PATCH v2 3/3 nvme: wwid_show: strip trailing 0-bytes
In reply to Martin WilckSome broken targets (such as the current Linux target) pad model or serial fields with 0-bytes rather than spaces. The NVME spec disallows 0 bytes in "ASCII" fields. Thus strip trailing 0-bytes, too.
--- drivers/nvme/host/core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/nvme/host/core.c +4 -2
@@ -2001,9 +2001,11 @@ static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,if (memchr_inv (ns->eui,0 ,sizeof (ns->eui)))return sprintf (buf,"eui.%8phN\n" , ns->eui); -while (ctrl->serial[serial_len -1 ] ==' ' ) +while (ctrl->serial[serial_len -1 ] ==' ' || + ctrl->serial[serial_len -1 ] =='\0' ) serial_len--; -while (ctrl->model[model_len -1 ] ==' ' ) +while (ctrl->model[model_len -1 ] ==' ' || + ctrl->model[model_len -1 ] =='\0' ) model_len--; return sprintf(buf, "nvme.%04x-%*phN-%*phN-%08x\n", ctrl->vid,
Signature · 1 line
2.13.2
Re: [PATCH v2 3/3] nvme: wwid_show: strip trailing 0-bytes
drivers/nvme/host/core.c
Quoted 14 lines · Martin Wilck
Some broken targets (such as the current Linux target) pad model or serial fields with 0-bytes rather than spaces. The NVME spec disallows 0 bytes in "ASCII" fields. Thus strip trailing 0-bytes, too.
Signed-off-by Martin Wilck <mwilck at suse.com>Reviewed-by Hannes Reinecke <hare at suse.de>Acked-by Christoph Hellwig <hch at lst.de>--- drivers/nvme/host/core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/nvme/host/core.c +0
-0
[]
drivers/nvme/host/core.c static ssize_t wwid_show(struct device *dev, struct device_attribute *attr, · line 2009
Quoted 12 lines
drivers/nvme/host/core.c +4
-2@@ -2001,9 +2001,11 @@ static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,if (memchr_inv (ns->eui,0 ,sizeof (ns->eui)))return sprintf (buf,"eui.%8phN\n" , ns->eui); -while (ctrl->serial[serial_len -1 ] ==' ' ) +while (ctrl->serial[serial_len -1 ] ==' ' || + ctrl->serial[serial_len -1 ] =='\0' ) serial_len--; -while (ctrl->model[model_len -1 ] ==' ' ) +while (ctrl->model[model_len -1 ] ==' ' || + ctrl->model[model_len -1 ] =='\0' ) model_len--;
Please add a <foo>_len > 0 to the while loops too