concat connections fail during connect-all

[martin-gpy]

Seems secure channel concat connections fail during nvme connect-all while using a JSON config file, as shown below:

# cat /usr/local/etc/nvme/config.json
[
  {
    "hostnqn":"nqn.2014-08.org.nvmexpress:uuid:e6550026-173e-4959-ba74-be367844bd8a",
    "hostid":"e6550026-173e-4959-ba74-be367844bd8a",
    "subsystems":[
      {
        "nqn":"nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:subsystem.subsys_CLIENT116_0",
        "ports":[
          {
            "transport":"tcp",
            "traddr":"192.168.1.116",
            "host_traddr":"192.168.1.6",
            "trsvcid":"4420",
            "dhchap_key":"DHHC-1:01:gTtZdM+d+SjkxHcCcJKuI0OQKEnIJiXFASAlNDQP94FD/lKe:",
            "concat":"true"
          }
        ]
      }
    ]
  }
]

Now attempt a connect-all on the above JSON config file:

# nvme connect-all -J /usr/local/etc/nvme/config.json -vv
kernel supports: instance cntlid transport traddr trsvcid nqn queue_size nr_io_queues reconnect_delay ctrl_loss_tmo keep_alive_tmo hostnqn host_traddr host_iface hostid duplicate_connect disable_sqflow hdr_digest data_digest nr_write_queues nr_poll_queues tos keyring tls_key fast_io_fail_tmo discovery dhchap_secret dhchap_ctrl_secret tls concat recovery_delay
connect ctrl, 'nqn=nqn.2014-08.org.nvmexpress.discovery,transport=tcp,traddr=192.168.1.116,host_traddr=192.168.1.6,trsvcid=4420,hostnqn=nqn.2014-08.org.nvmexpress:uuid:e6550026-173e-4959-ba74-be367844bd8a,hostid=e6550026-173e-4959-ba74-be367844bd8a,ctrl_loss_tmo=600'
connect ctrl, response 'instance=0,cntlid=192'
nvme0: nqn.2014-08.org.nvmexpress.discovery connected
lookup subsystem /sys/class/nvme-subsystem/nvme-subsys0/nvme0
nvme0: nqn.2014-08.org.nvmexpress.discovery disconnected
connect ctrl, 'nqn=nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:discovery,transport=tcp,traddr=192.168.1.116,host_traddr=192.168.1.6,trsvcid=4420,hostnqn=nqn.2014-08.org.nvmexpress:uuid:e6550026-173e-4959-ba74-be367844bd8a,hostid=e6550026-173e-4959-ba74-be367844bd8a,discovery,ctrl_loss_tmo=600'
connect ctrl, response 'instance=0,cntlid=256'
nvme0: nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:discovery connected
lookup subsystem /sys/class/nvme-subsystem/nvme-subsys0/nvme0
nvme0: get header (try 0/10)
nvme0: get 2 records (genctr 21)
nvme0: get header again
lookup ctrl (transport: tcp, traddr: 192.168.1.116, trsvcid 4420)
connect ctrl, 'nqn=nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:subsystem.subsys_CLIENT116_0,transport=tcp,traddr=192.168.1.116,trsvcid=4420,hostnqn=nqn.2014-08.org.nvmexpress:uuid:e6550026-173e-4959-ba74-be367844bd8a,hostid=e6550026-173e-4959-ba74-be367844bd8a,dhchap_secret=DHHC-1:01:gTtZdM+d+SjkxHcCcJKuI0OQKEnIJiXFASAlNDQP94FD/lKe:,ctrl_loss_tmo=600,tls,concat'
Failed to write to /dev/nvme-fabrics: Invalid argument
nvme0: nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:discovery disconnected

Tailing the messages file for the above failure:

# tail -f /var/log/messages
kernel: [T34705] nvme nvme0: mapped 2/0/0 default/read/poll queues.
kernel: [T34705] nvme nvme0: new ctrl: NQN "nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:subsystem.subsys_CLIENT116_0", addr 192.168.1.116:4420
kernel: [T34723] nvme nvme0: Removing ctrl: NQN "nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:subsystem.subsys_CLIENT116_0"
kernel: [T34835] nvme nvme0: queue_size 128 > ctrl sqsize 32, clamping down
kernel: [T34835] nvme nvme0: new ctrl: NQN "nqn.2014-08.org.nvmexpress.discovery", addr 192.168.1.116:4420
kernel: [T34835] nvme nvme0: Removing ctrl: NQN "nqn.2014-08.org.nvmexpress.discovery"
kernel: [T34835] nvme nvme0: queue_size 128 > ctrl sqsize 32, clamping down
kernel: [T34835] nvme nvme0: new ctrl: NQN "nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:discovery", addr 192.168.1.116:4420
kernel: [T34835] nvme_fabrics: Secure concatenation over TLS is not supported
kernel: [T34835] nvme nvme0: Removing ctrl: NQN "nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:discovery"

[martin-gpy]

From the above verbose logs, turns out this failed here:

connect ctrl, 'nqn=nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:subsystem.subsys_CLIENT116_0,transport=tcp,traddr=192.168.1.116,trsvcid=4420,hostnqn=nqn.2014-08.org.nvmexpress:uuid:e6550026-173e-4959-ba74-be367844bd8a,hostid=e6550026-173e-4959-ba74-be367844bd8a,dhchap_secret=DHHC-1:01:gTtZdM+d+SjkxHcCcJKuI0OQKEnIJiXFASAlNDQP94FD/lKe:,ctrl_loss_tmo=600,**tls,concat**'
Failed to write to /dev/nvme-fabrics: Invalid argument

It inserted tls before concat above while attempting the secure channel concat connection. And that comes from the following code snippet in nvmf_connect_disc_entry() at src/nvme/fabrics.c:

        if (e->trtype == NVMF_TRTYPE_TCP &&
            e->tsas.tcp.sectype != NVMF_TCP_SECTYPE_NONE)
                c->cfg.tls = true;

So while attempting both configured PSK TLS and generated PSK TLS (i.e. secure channel concat) connections during a connect-all, the host would set tls to true. But for generated PSK TLS alone, that would lead to a Failed to write to /dev/nvme-fabrics: Invalid argument failure as both tls and concat are mutually exclusive flags.

That explains the above failure.

[igaw]

Just a quick question, this didn't happen with nvme-cli v2.16, correct? Is this a new error introduced by the code refactoring?

I haven't really looked into this part of the spec yet. If I got this right, you are referring to this here:

In this case, I find he kernel log message a bit misleading with not supported. It should be more like not allowed or so. I'll send out a patch for this later.

We might need to add a check/warning/error to the connect/connect-all case when --tls and --concat is set at the same time. As the whole connect/discovery code is in the library now, these types of sanity checks could be added to nvmf_connect and nvmf_discovery. The idea is that have all the logic and checks in the library, the frontend just does the command line parsing. This ensures that all users of the library have this sanity checks available.

[martin-gpy]

Just a quick question, this didn't happen with nvme-cli v2.16, correct? Is this a new error introduced by the code refactoring?

This is a day-1 issue itself, and nothing to do with the recent code refactoring.

I haven't really looked into this part of the spec yet. If I got this right, you are referring to this here:

No, this has nothing to do with the spec either. This is an implementation issue in the nvme-cli.

In this case, I find he kernel log message a bit misleading with not supported. It should be more like not allowed or so. I'll send out a patch for this later.

Actually the kernel error message Failed to write to /dev/nvme-fabrics: Invalid argument is right here, because both tls and concat were used together in this case while attempting the concat connection as seen in the above verbose logs.

We might need to add a check/warning/error to the connect/connect-all case when --tls and --concat is set at the same time. As the whole connect/discovery code is in the library now, these types of sanity checks could be added to nvmf_connect and nvmf_discovery. The idea is that have all the logic and checks in the library, the frontend just does the command line parsing. This ensures that all users of the library have this sanity checks available.

Understood and agree with that. But this is a different problem. Currently in nvme-cli, we use the tls flag for creating configured PSK TLS connections and concat flag for creating generated PSK TLS (i.e. secure channel concat) connections. And they are obviously mutually exclusive flags. But the below snippet of code in nvmf_connect_disc_entry() at src/nvme/fabrics.c enables tls for both configured & generated PSK (since sectype is != NVMF_TCP_SECTYPE_NONE for both):

        if (e->trtype == NVMF_TRTYPE_TCP &&
            e->tsas.tcp.sectype != NVMF_TCP_SECTYPE_NONE)
                c->cfg.tls = true;

Ideally it should have enabled tls for configured PSK alone above, but the above check gets satisfied for both configured & generated PSK, hence enables tls for both. So this would end up succeeding for configured PSK, but fail for generated PSK connections as seen above.