The reference tag is parsed incorrectly, causing the PI test case to fail when the format is 512+8, with metadata transferred as a separate buffer

[yalanliufio]

We recently upgraded our nvme cli version from 2.2.1 to 2.9.1, then encountered this problem.

Release 2.2.1 works correctly, but release 2.9.1 and the latest release 2.12, both have this issue.

When SSD drive is format as 512+8, and metadata is transferred as separate buffer, this is the id-ns output:

[root@localhost test-scripts]# nvme id-ns /dev/nvme1n1 
NVME Identify Namespace 1:
nsze    : 0x1bf1f72b0
ncap    : 0x1bf1f72b0
nuse    : 0x10
nsfeat  : 0x10
nlbaf   : 4
flbas   : 0x1
mc      : 0x3
dpc     : 0x13
dps     : 0x1
nmic    : 0
lbaf  0 : ms:0   lbads:9  rp:0 
lbaf  1 : ms:8   lbads:9  rp:0 (in use)
lbaf  2 : ms:0   lbads:12 rp:0 
lbaf  3 : ms:8   lbads:12 rp:0 
lbaf  4 : ms:64  lbads:12 rp:0 

When using nvme version 2.2.1,

[root@localhost test-scripts]# nvme --version
nvme version 2.2.1 (git 2.2.1)
libnvme version 1.2 (git 1.2)

issue a write command and also enable the kernel trace:, this is the output

nvme write /dev/nvme1n1 -s 0x02000000 -c 2 -z 1536 -y 24 -r 0x02000000 -d write_D1D0027T.data -M write_meta_D1D0027T_slba02000000_nlb2.meta -p 0 -a 0
            nvme-29400   [004] ....... 235741.995845: nvme_setup_cmd: nvme1: disk=nvme1n1, qid=5, cmdid=768, nsid=1, flags=0x0, meta=0x1, cmd=(nvme_cmd_write slba=33554432, len=2, ctrl=0x0, dsmgmt=0, reftag=33554432)
          <idle>-0       [004] d..h1.. 235741.995853: nvme_sq: nvme1: disk=nvme1n1, qid=5, head=1, tail=1
          <idle>-0       [004] d..h1.. 235741.995854: nvme_complete_rq: nvme1: disk=nvme1n1, qid=5, cmdid=768, res=0x0, retries=0, flags=0x2, status=0x0

In this case, reference tag is parsed correctly, command finished successfully

However, in both nvme release 2.9.1 and 2.12, repeat about command, get this output:

[root@localhost test-scripts]# nvme --version
nvme version 2.9.1 (git 2.9.1)
libnvme version 1.9 (git 1.9)

nvme write /dev/nvme1n1 -s 0x02000000 -c 2 -z 1536 -y 24 -r 0x02000000 -d write_D1D0027T.data -M write_meta_D1D0027T_slba02000000_nlb2.meta -p 0 -a 0
            nvme-21954   [016] ....... 234476.695797: nvme_setup_cmd: nvme1: disk=nvme1n1, qid=17, cmdid=64, nsid=1, flags=0x0, meta=0x1, cmd=(nvme_cmd_write slba=33554432, len=2, ctrl=0x0, dsmgmt=0, reftag=0)
          <idle>-0       [016] d..h1.. 234476.695844: nvme_sq: nvme1: disk=nvme1n1, qid=17, head=1, tail=1
          <idle>-0       [016] d..h1.. 234476.695844: nvme_complete_rq: nvme1: disk=nvme1n1, qid=17, cmdid=64, res=0x0, retries=0, flags=0x2, status=0x181

The command failed, this is due to the reference tag is parsed as reftag=0 but the value given by command is 0x2000000, decimal=33554432

This is the command dry-run output, but not very helpful as it is not 16 command dwords, reference tag is the command dword14, is there a way to show the 16 dwords of write command?

[root@localhost test-scripts]# nvme write /dev/nvme1n1 -s 0x02000000 -c 2 -z 1536 -y 24 -r 0x02000000 -d write_D1D0027T.data -M write_meta_D1D0027T_slba02000000_nlb2.meta -p 0 -a 0 -w
opcode       : 01
nsid         : 01
flags        : 00
control      : 0000
nblocks      : 0002
metadata     : 1177800
addr         : 1178000
slba         : 2000000
dsmgmt       : 00000000
reftag       : 2000000
apptag       : 0000
appmask      : 0000
storagetagcheck : 0000
storagetag      : 0
pif             : 00
sts             : 00

write_D1D0027T.data and write_meta_D1D0027T_slba02000000_nlb2.meta is attached, I changed it to txt file as data file can't be attached.

Other format with meta data probably has same problem, but I only tested this 512+8 format so far

write_D1D0027T.data.txt

write_meta_D1D0027T_slba02000000_nlb2.meta.txt

[yalanliufio]

this is from NVM Command Set Specification Revision 1.0c

[yalanliufio]

io-passthru command passed with the same parameters

[root@localhost test-scripts]# nvme io-passthru /dev/nvme1 -n 1 -O 1 -l 1536 -m 24 -4 0x02000000 -6 1 -8 0x02000000 -9 0 -i write_D1D0027T.data -M write_meta_D1D0027T_slba02000000_nlb2.meta
IO Command Write is Success and result: 0x00000000

[root@localhost test-scripts]# nvme write /dev/nvme1n1 -s 0x02000000 -c 2 -z 1536 -y 24 -r 0x02000000 -d write_D1D0027T.data -M write_meta_D1D0027T_slba02000000_nlb2.meta -p 0 -a 0
NVMe status: Invalid Protection Information: The command's Protection Information Field settings are invalid for the namespace's Protection Information format(0x181)
[root@localhost test-scripts]#

[igaw]

Are you sure about the -p 0 argument? This is for a 16 bit tag as far I understand, but the tag provided is clearly more than 16bits.

Yes, extending --dry-run output would be useful. Someone has to do it, that's the problem.

[yalanliufio]

Definition of -p (prinfo), it is 4 bits, bits 0:2 is PRCHK, bit 3 is PRACT, we disabled PRCHK and PRACT=0, so -p 0 is correct.

For the reference tag, it is 32 bits (dword14) because our drive doesn't support Extended LBA Format, I attached CTRATT value here
nvme id-ctrl /dev/nvme7 | grep ctratt
ctratt : 0x290
the bit15 is 0, so we don't support Storage tag, don't support 32-bit guard field, and don't support 64-bit guard field.

For the SSD drive I am testing, PI have 8 bytes, 4 bytes or 32 bits are reference tag, 16 bits are application tag, 16 bits are guard field

In the commands I used for tesing, -r 0x02000000 is used to set reference tag to 0x02000000, it is less than 32-bits. Application tag is set to 0, the -a 0

[ikegami-t]

You can use '-vv' option to dump dword values.

[igaw]

For some reason I though the pif value is mapped to -p command line argument.

There were many changes in the handling of the PI code between 2.2.1 to 2.9.1. @francispravin5 do you have any ideas?

[ikegami-t]

The function submit_io() only sets reftag_u64 but nvme_io() uses both reftag and reftag_u64.

1. submit_io()

static int submit_io(int opcode, char *command, const char *desc, int argc, char **argv)
{
...
	struct nvme_io_args args = {
...
		.reftag_u64	= cfg.ref_tag,

2. nvme_io()

int nvme_io(struct nvme_io_args *args, __u8 opcode)
{
...
	if (args->args_size == size_v1) {
...
		cdw14 = args->reftag;
	} else {
		if (nvme_set_var_size_tags(&cdw2, &cdw3, &cdw14,
...
				args->reftag_u64,

[ikegami-t]

Changed by the commit 113c80f and linux-nvme/libnvme@9350e73e.
@brandon-paupore-sndk Currenty the nvme write comand uses only nvme_io_args reftag_u64 but the libnvme function uses both reftag and reftag_u64. Do you know which is correct handling? (For me it seems better to delete the duplicated reftag value from the structure.)

[ikegami-t]

@yalanliufio Could you please try to test the PR #2765 changes?

[brandon-paupore-sndk]

Changed by the commit 113c80f and linux-nvme/libnvme@9350e73e. @brandon-paupore-sndk Currenty the nvme write comand uses only nvme_io_args reftag_u64 but the libnvme function uses both reftag and reftag_u64. Do you know which is correct handling? (For me it seems better to delete the duplicated reftag value from the structure.)

This should be correct as only intended to be run with the updated v2 version of nvme_io_args for nvme-cli, but if the args->args_size checking isn't working then that'd explain the issue here. The idea was to avoid requiring updates if some function was using the older version of the structure but calling an updated LibNVMe.

In any case, those changes you put up seems like a good test to verify.

[ikegami-t]

Thanks for your confirmation and comments.

[yalanliufio]

@ikegami-t the '--vv' option doesn't work, it says: unrecognized option 'vv' like this:
nvme write /dev/nvme7n1 -s 0 -c 2 -z 1536 -y 24 -r 2 -d write_D1D0027T.data -M write_meta_D1D0027T_slba02000000_nlb2.meta -p 0 -a 0 --vv
write: unrecognized option '--vv'

[yalanliufio]

@ikegami-t
I would like to try this PR 2765 change, could you please tell me how to compile and build this code?