* [PATCH] *** Discovery feature fix in nvme-cli *** @ 2016-07-28 20:03 Jay Freyensee 2016-07-28 20:03 ` [PATCH] nvme-cli: user-defined hostnqn option for discover Jay Freyensee 0 siblings, 1 reply; 6+ messages in thread From: Jay Freyensee @ 2016-07-28 20:03 UTC (permalink / raw) Since the only way to do the discovery feature of NVMe-over-Fabrics is through nvme-cli, I thought it will be appropriate to send a patch to this email list (if nothing more than to inform and educate). I'll push this to Keith Busch (the nvme-cli owner). Jay Freyensee (1): nvme-cli: user-defined hostnqn option for discover fabrics.c | 11 +++++++++++ 1 file changed, 11 insertions(+) -- 2.4.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] nvme-cli: user-defined hostnqn option for discover 2016-07-28 20:03 [PATCH] *** Discovery feature fix in nvme-cli *** Jay Freyensee @ 2016-07-28 20:03 ` Jay Freyensee 2016-07-28 20:20 ` Roy Shterman 0 siblings, 1 reply; 6+ messages in thread From: Jay Freyensee @ 2016-07-28 20:03 UTC (permalink / raw) The nvme-cli will always use the default hostnqn in /dev/nvme-fabrics for the discovery query, even though both the NVMe Target and NVMe Host rdma implementations allow user-defined hostnqn naming. For example, this is the current, somewhat broken behavior if you used your own hostnqn provision naming on the NVMe kernel target: nvme discover /dev/nvme-fabrics -t rdma --traddr=192.168.1.3 --trsvcid=4420 in dmesg: [591910.025779] nvme nvme0: Connect Invalid Data Parameter, hostnqn "nqn.2014-08.org.nvmexpress:NVMf:uuid:a2d7752c-a31b-477a-a003-31a5e1c424a9" New, fixed behavior introduced by this patch: [root at fedora23-fabrics-host1 nvme-cli]# nvme discover -t rdma --traddr=192.168.1.3 --trsvcid=4420 --hostnqn=host1-rogue-nqn Discovery Log Number of Records 1, Generation counter 10 =====Discovery Log Entry 0====== trtype: ipv4 adrfam: rdma nqntype: 2 treq: 0 portid: 1 trsvcid: 4420 subnqn: nullside-nqn traddr: 192.168.1.3 rdma_prtype: 0 rdma_qptype: 0 rdma_cms: 0 rdma_pkey: 0x0000 [root at fedora23-fabrics-host1 nvme-cli]# Signed-off-by: Jay Freyensee <james_p_freyensee at linux.intel.com> --- fabrics.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/fabrics.c b/fabrics.c index 3666a01..41a80df 100644 --- a/fabrics.c +++ b/fabrics.c @@ -49,6 +49,7 @@ struct config { char *transport; char *traddr; char *trsvcid; + char *hostnqn; char *raw; char *device; } cfg = { 0 }; @@ -395,6 +396,14 @@ static int build_options(char *argstr, int max_len) max_len -= len; } + if (cfg.hostnqn) { + len = snprintf(argstr, max_len, ",hostnqn=%s", cfg.hostnqn); + if (len < 0) + return -EINVAL; + argstr += len; + max_len -= len; + } + return 0; } @@ -525,6 +534,8 @@ int discover(const char *desc, int argc, char **argv, bool connect) "transport address" }, {"trsvcid", 's', "LIST", CFG_STRING, &cfg.trsvcid, required_argument, "transport service id (e.g. IP port)" }, + {"hostnqn", 'h', "LIST", CFG_STRING, &cfg.hostnqn, required_argument, + "user-defined hostnqn (if default not used)" }, {"raw", 'r', "LIST", CFG_STRING, &cfg.raw, required_argument, "raw output file" }, {0}, -- 2.4.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] nvme-cli: user-defined hostnqn option for discover 2016-07-28 20:03 ` [PATCH] nvme-cli: user-defined hostnqn option for discover Jay Freyensee @ 2016-07-28 20:20 ` Roy Shterman 2016-07-28 20:35 ` Keith Busch 0 siblings, 1 reply; 6+ messages in thread From: Roy Shterman @ 2016-07-28 20:20 UTC (permalink / raw) Hi Jay, Hi Jay, Actually I planned to send the same patch tomorrow, I think we should add user-defined hostnqn parameter into 'connect' command. On 7/28/2016 11:03 PM, Jay Freyensee wrote: > The nvme-cli will always use the default hostnqn > in /dev/nvme-fabrics for the discovery query, even though > both the NVMe Target and NVMe Host rdma implementations allow > user-defined hostnqn naming. > > For example, this is the current, somewhat broken behavior if you > used your own hostnqn provision naming on the NVMe kernel target: > > nvme discover /dev/nvme-fabrics -t rdma --traddr=192.168.1.3 --trsvcid=4420 > > in dmesg: > [591910.025779] nvme nvme0: Connect Invalid Data Parameter, hostnqn "nqn.2014-08.org.nvmexpress:NVMf:uuid:a2d7752c-a31b-477a-a003-31a5e1c424a9" > > New, fixed behavior introduced by this patch: > > [root at fedora23-fabrics-host1 nvme-cli]# nvme discover -t rdma --traddr=192.168.1.3 --trsvcid=4420 --hostnqn=host1-rogue-nqn > > Discovery Log Number of Records 1, Generation counter 10 > =====Discovery Log Entry 0====== > trtype: ipv4 > adrfam: rdma > nqntype: 2 > treq: 0 > portid: 1 > trsvcid: 4420 > subnqn: nullside-nqn > traddr: 192.168.1.3 > rdma_prtype: 0 > rdma_qptype: 0 > rdma_cms: 0 > rdma_pkey: 0x0000 > [root at fedora23-fabrics-host1 nvme-cli]# > > Signed-off-by: Jay Freyensee <james_p_freyensee at linux.intel.com> > --- > fabrics.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/fabrics.c b/fabrics.c > index 3666a01..41a80df 100644 > --- a/fabrics.c > +++ b/fabrics.c > @@ -49,6 +49,7 @@ struct config { > char *transport; > char *traddr; > char *trsvcid; > + char *hostnqn; > char *raw; > char *device; > } cfg = { 0 }; > @@ -395,6 +396,14 @@ static int build_options(char *argstr, int max_len) > max_len -= len; > } > > + if (cfg.hostnqn) { > + len = snprintf(argstr, max_len, ",hostnqn=%s", cfg.hostnqn); > + if (len < 0) > + return -EINVAL; > + argstr += len; > + max_len -= len; > + } > + > return 0; > } > > @@ -525,6 +534,8 @@ int discover(const char *desc, int argc, char **argv, bool connect) > "transport address" }, > {"trsvcid", 's', "LIST", CFG_STRING, &cfg.trsvcid, required_argument, > "transport service id (e.g. IP port)" }, > + {"hostnqn", 'h', "LIST", CFG_STRING, &cfg.hostnqn, required_argument, > + "user-defined hostnqn (if default not used)" }, I believe 'h' short option is already dedicated for help parameter. maybe we should change it to different character? > {"raw", 'r', "LIST", CFG_STRING, &cfg.raw, required_argument, > "raw output file" }, > {0}, ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] nvme-cli: user-defined hostnqn option for discover 2016-07-28 20:20 ` Roy Shterman @ 2016-07-28 20:35 ` Keith Busch 2016-07-28 21:21 ` J Freyensee 0 siblings, 1 reply; 6+ messages in thread From: Keith Busch @ 2016-07-28 20:35 UTC (permalink / raw) On Thu, Jul 28, 2016@11:20:15PM +0300, Roy Shterman wrote: > Hi Jay, > > Actually I planned to send the same patch tomorrow, > > I think we should add user-defined hostnqn parameter into 'connect' command. Is it okay to apply this as-is, or should I wait for the patch for connect? ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] nvme-cli: user-defined hostnqn option for discover 2016-07-28 20:35 ` Keith Busch @ 2016-07-28 21:21 ` J Freyensee [not found] ` <45CF57A8-206F-4353-9386-A0BB8158AB5A@mellanox.com> 0 siblings, 1 reply; 6+ messages in thread From: J Freyensee @ 2016-07-28 21:21 UTC (permalink / raw) On Thu, 2016-07-28@16:35 -0400, Keith Busch wrote: > On Thu, Jul 28, 2016@11:20:15PM +0300, Roy Shterman wrote: > > Hi Jay, > > > > Actually I planned to send the same patch tomorrow, > > > > I think we should add user-defined hostnqn parameter into 'connect' > > command. > > Is it okay to apply this as-is, or should I wait for the patch for > connect? I think we should break up the patches as one for discover, and one for connect. Each patch can show a usage for each of the commands (one for 'discover', one for 'connect'. How about apply my discover patch as-is, then Roy re-submit his patch for connect? And in the connect patch, supply an example usage (which I think this will be useful as there is currently no man pages for these new fabrics commands). > > _______________________________________________ > Linux-nvme mailing list > Linux-nvme at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <45CF57A8-206F-4353-9386-A0BB8158AB5A@mellanox.com>]
* [PATCH] [RESEND] nvme-cli: user-defined hostnqn option for discover [not found] ` <45CF57A8-206F-4353-9386-A0BB8158AB5A@mellanox.com> @ 2016-07-28 21:36 ` J Freyensee 0 siblings, 0 replies; 6+ messages in thread From: J Freyensee @ 2016-07-28 21:36 UTC (permalink / raw) On Thu, 2016-07-28@21:27 +0000, Roy Shterman wrote: > Sounds good to me, but what about the short option character? > > I think using 'h' as a short option will not work because it is > already dedicated for --help parameter. > That is a good point :-/. OK, I'll respin my discover patch to use 'q' short option you chose. I'll send the patch out shortly. > I will send my patch later only for connect after Keith will merge > discover patch. > > Thanks, > Roy > > ?-29 ????? 2016, ???? 00:22, ??J Freyensee ?< > james_p_freyensee at linux.intel.com> ???/?:? > > > On Thu, 2016-07-28@16:35 -0400, Keith Busch wrote: > > > On Thu, Jul 28, 2016@11:20:15PM +0300, Roy Shterman wrote: > > > > Hi Jay, > > > > Actually I planned to send the same patch tomorrow, > > > > I think we should add user-defined hostnqn parameter into > > > > 'connect' > > > > command. > > > Is it okay to apply this as-is, or should I wait for the patch > > > for > > > connect? > > I think we should break up the patches as one for discover, and one > > for > > connect. Each patch can show a usage for each of the commands (one > > for > > 'discover', one for 'connect'. > > > > How about apply my discover patch as-is, then Roy re-submit his > > patch > > for connect? And in the connect patch, supply an example usage > > (which > > I think this will be useful as there is currently no man pages for > > these new fabrics commands). > > > > > > > _______________________________________________ > > > Linux-nvme mailing list > > > Linux-nvme at lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-28 21:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-28 20:03 [PATCH] *** Discovery feature fix in nvme-cli *** Jay Freyensee
2016-07-28 20:03 ` [PATCH] nvme-cli: user-defined hostnqn option for discover Jay Freyensee
2016-07-28 20:20 ` Roy Shterman
2016-07-28 20:35 ` Keith Busch
2016-07-28 21:21 ` J Freyensee
[not found] ` <45CF57A8-206F-4353-9386-A0BB8158AB5A@mellanox.com>
2016-07-28 21:36 ` [PATCH] [RESEND] " J Freyensee
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).