* [PATCH rfc 0/2] support ip resolution with hostnames
@ 2020-03-24 9:03 Sagi Grimberg
2020-03-24 9:03 ` [PATCH rfc 1/2] fabrics: add fabrics_ prefix to fabrics operations Sagi Grimberg
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Sagi Grimberg @ 2020-03-24 9:03 UTC (permalink / raw)
To: linux-nvme, Keith Busch
People hate IP addresses, that's why DNS exist. This simple
patch allows the user to connect to fabric device using
a --hostname parameter instead of a --traddr to make peoples'
lives a little more convinient (consider this nvme-cli small
contribution to the covid19 battle).
Sagi Grimberg (2):
fabrics: add fabrics_ prefix to fabrics operations
fabrics: allow user to pass hostname instead of traddr
fabrics.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
fabrics.h | 8 +++---
nvme.c | 10 +++----
3 files changed, 88 insertions(+), 13 deletions(-)
--
2.20.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH rfc 1/2] fabrics: add fabrics_ prefix to fabrics operations 2020-03-24 9:03 [PATCH rfc 0/2] support ip resolution with hostnames Sagi Grimberg @ 2020-03-24 9:03 ` Sagi Grimberg 2020-03-24 9:03 ` [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr Sagi Grimberg ` (2 subsequent siblings) 3 siblings, 0 replies; 13+ messages in thread From: Sagi Grimberg @ 2020-03-24 9:03 UTC (permalink / raw) To: linux-nvme, Keith Busch discover/connect/disconnect are generic names which might clash with other external included libraries. Signed-off-by: Sagi Grimberg <sagi@grimberg.me> --- fabrics.c | 8 ++++---- fabrics.h | 8 ++++---- nvme.c | 10 +++++----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/fabrics.c b/fabrics.c index e2c9bfb37177..a7d628b1f0c9 100644 --- a/fabrics.c +++ b/fabrics.c @@ -1251,7 +1251,7 @@ out: return ret; } -int discover(const char *desc, int argc, char **argv, bool connect) +int fabrics_discover(const char *desc, int argc, char **argv, bool connect) { char argstr[BUF_SIZE]; int ret; @@ -1306,7 +1306,7 @@ out: return nvme_status_to_errno(ret, true); } -int connect(const char *desc, int argc, char **argv) +int fabrics_connect(const char *desc, int argc, char **argv) { char argstr[BUF_SIZE]; int instance, ret; @@ -1439,7 +1439,7 @@ static int disconnect_by_device(char *device) return remove_ctrl(instance); } -int disconnect(const char *desc, int argc, char **argv) +int fabrics_disconnect(const char *desc, int argc, char **argv) { const char *nqn = "nqn name"; const char *device = "nvme device"; @@ -1484,7 +1484,7 @@ out: return nvme_status_to_errno(ret, true); } -int disconnect_all(const char *desc, int argc, char **argv) +int fabrics_disconnect_all(const char *desc, int argc, char **argv) { struct nvme_topology t = { }; int i, j, err; diff --git a/fabrics.h b/fabrics.h index b8e53f492b53..f5b8eaf6bba0 100644 --- a/fabrics.h +++ b/fabrics.h @@ -5,9 +5,9 @@ extern char *hostnqn_read(void); -extern int discover(const char *desc, int argc, char **argv, bool connect); -extern int connect(const char *desc, int argc, char **argv); -extern int disconnect(const char *desc, int argc, char **argv); -extern int disconnect_all(const char *desc, int argc, char **argv); +extern int fabrics_discover(const char *desc, int argc, char **argv, bool connect); +extern int fabrics_connect(const char *desc, int argc, char **argv); +extern int fabrics_disconnect(const char *desc, int argc, char **argv); +extern int fabrics_disconnect_all(const char *desc, int argc, char **argv); #endif diff --git a/nvme.c b/nvme.c index dd3ab5870e62..7e6713a1c8c7 100644 --- a/nvme.c +++ b/nvme.c @@ -4779,31 +4779,31 @@ static int show_hostnqn_cmd(int argc, char **argv, struct command *command, stru static int discover_cmd(int argc, char **argv, struct command *command, struct plugin *plugin) { const char *desc = "Send Get Log Page request to Discovery Controller."; - return discover(desc, argc, argv, false); + return fabrics_discover(desc, argc, argv, false); } static int connect_all_cmd(int argc, char **argv, struct command *command, struct plugin *plugin) { const char *desc = "Discover NVMeoF subsystems and connect to them"; - return discover(desc, argc, argv, true); + return fabrics_discover(desc, argc, argv, true); } static int connect_cmd(int argc, char **argv, struct command *command, struct plugin *plugin) { const char *desc = "Connect to NVMeoF subsystem"; - return connect(desc, argc, argv); + return fabrics_connect(desc, argc, argv); } static int disconnect_cmd(int argc, char **argv, struct command *command, struct plugin *plugin) { const char *desc = "Disconnect from NVMeoF subsystem"; - return disconnect(desc, argc, argv); + return fabrics_disconnect(desc, argc, argv); } static int disconnect_all_cmd(int argc, char **argv, struct command *command, struct plugin *plugin) { const char *desc = "Disconnect from all connected NVMeoF subsystems"; - return disconnect_all(desc, argc, argv); + return fabrics_disconnect_all(desc, argc, argv); } void register_extension(struct plugin *plugin) -- 2.20.1 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr 2020-03-24 9:03 [PATCH rfc 0/2] support ip resolution with hostnames Sagi Grimberg 2020-03-24 9:03 ` [PATCH rfc 1/2] fabrics: add fabrics_ prefix to fabrics operations Sagi Grimberg @ 2020-03-24 9:03 ` Sagi Grimberg 2020-03-24 9:46 ` Max Gurtovoy 2020-04-01 19:39 ` Keith Busch 2020-03-24 9:06 ` [PATCH rfc 0/2] support ip resolution with hostnames Sagi Grimberg 2020-04-01 6:03 ` Sagi Grimberg 3 siblings, 2 replies; 13+ messages in thread From: Sagi Grimberg @ 2020-03-24 9:03 UTC (permalink / raw) To: linux-nvme, Keith Busch Some users would like to use well known hostnames instead of remembering ip addresses. So, allow users to pass --hostname and we will attempt to resolve against the DNS. Signed-off-by: Sagi Grimberg <sagi@grimberg.me> --- fabrics.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/fabrics.c b/fabrics.c index a7d628b1f0c9..7bd95c4b0d10 100644 --- a/fabrics.c +++ b/fabrics.c @@ -33,6 +33,10 @@ #include <sys/stat.h> #include <stddef.h> +#include <sys/types.h> +#include <arpa/inet.h> +#include <netdb.h> + #include "util/parser.h" #include "nvme-ioctl.h" #include "nvme-status.h" @@ -60,6 +64,7 @@ static struct config { char *nqn; char *transport; char *traddr; + char *hostname; char *trsvcid; char *host_traddr; char *hostnqn; @@ -857,6 +862,54 @@ static int build_options(char *argstr, int max_len, bool discover) return 0; } +static int hostname2traddr(struct config *cfg) +{ + struct addrinfo *host_info, hints = {.ai_family = AF_UNSPEC}; + char addrstr[NVMF_TRADDR_SIZE]; + const char *p; + int ret; + + if (cfg->traddr) { + fprintf(stderr, "hostname and traddr cannot be passed together\n"); + return -EINVAL; + } + + ret = getaddrinfo(cfg->hostname, NULL, &hints, &host_info); + if (ret) { + fprintf(stderr, "failed to get host info for %s\n", cfg->hostname); + return ret; + } + + switch (host_info->ai_family) { + case AF_INET: + p = inet_ntop(host_info->ai_family, + &(((struct sockaddr_in *)host_info->ai_addr)->sin_addr), + addrstr, NVMF_TRADDR_SIZE); + break; + case AF_INET6: + p = inet_ntop(host_info->ai_family, + &(((struct sockaddr_in6 *)host_info->ai_addr)->sin6_addr), + addrstr, NVMF_TRADDR_SIZE); + break; + default: + fprintf(stderr, "unrecognized address family (%d) %s\n", + host_info->ai_family, cfg->hostname); + ret = -EINVAL; + goto free_addrinfo; + } + + if (!p) { + fprintf(stderr, "failed to get traddr for %s\n", cfg->hostname); + ret = -errno; + goto free_addrinfo; + } + cfg->traddr = strdup(addrstr); + +free_addrinfo: + freeaddrinfo(host_info); + return ret; +} + static int connect_ctrl(struct nvmf_disc_rsp_page_entry *e) { char argstr[BUF_SIZE], *p; @@ -1230,6 +1283,13 @@ static int discover_from_conf_file(const char *desc, char *argstr, if (cfg.persistent && !cfg.keep_alive_tmo) cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO; + if (cfg.hostname) { + ret = hostname2traddr(&cfg); + if (ret) + goto out; + cfg.hostname = NULL; + } + err = build_options(argstr, BUF_SIZE, true); if (err) { ret = err; @@ -1259,6 +1319,7 @@ int fabrics_discover(const char *desc, int argc, char **argv, bool connect) OPT_ARGS(opts) = { OPT_LIST("transport", 't', &cfg.transport, "transport type"), OPT_LIST("traddr", 'a', &cfg.traddr, "transport address"), + OPT_LIST("hostname", 'A', &cfg.hostname, "transport address in a hostname form"), OPT_LIST("trsvcid", 's', &cfg.trsvcid, "transport service id (e.g. IP port)"), OPT_LIST("host-traddr", 'w', &cfg.host_traddr, "host traddr (e.g. FC WWN's)"), OPT_LIST("hostnqn", 'q', &cfg.hostnqn, "user-defined hostnqn (if default not used)"), @@ -1295,6 +1356,13 @@ int fabrics_discover(const char *desc, int argc, char **argv, bool connect) } else { if (cfg.persistent && !cfg.keep_alive_tmo) cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO; + + if (cfg.hostname) { + ret = hostname2traddr(&cfg); + if (ret) + goto out; + } + ret = build_options(argstr, BUF_SIZE, true); if (ret) goto out; @@ -1315,6 +1383,7 @@ int fabrics_connect(const char *desc, int argc, char **argv) OPT_LIST("transport", 't', &cfg.transport, "transport type"), OPT_LIST("nqn", 'n', &cfg.nqn, "nqn name"), OPT_LIST("traddr", 'a', &cfg.traddr, "transport address"), + OPT_LIST("hostname", 'A', &cfg.hostname, "transport address in a hostname form"), OPT_LIST("trsvcid", 's', &cfg.trsvcid, "transport service id (e.g. IP port)"), OPT_LIST("host-traddr", 'w', &cfg.host_traddr, "host traddr (e.g. FC WWN's)"), OPT_LIST("hostnqn", 'q', &cfg.hostnqn, "user-defined hostnqn"), @@ -1339,6 +1408,12 @@ int fabrics_connect(const char *desc, int argc, char **argv) if (ret) goto out; + if (cfg.hostname) { + ret = hostname2traddr(&cfg); + if (ret) + goto out; + } + ret = build_options(argstr, BUF_SIZE, false); if (ret) goto out; -- 2.20.1 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr 2020-03-24 9:03 ` [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr Sagi Grimberg @ 2020-03-24 9:46 ` Max Gurtovoy 2020-03-24 16:21 ` Sagi Grimberg 2020-04-01 19:39 ` Keith Busch 1 sibling, 1 reply; 13+ messages in thread From: Max Gurtovoy @ 2020-03-24 9:46 UTC (permalink / raw) To: Sagi Grimberg, linux-nvme, Keith Busch On 3/24/2020 11:03 AM, Sagi Grimberg wrote: > Some users would like to use well known hostnames instead > of remembering ip addresses. So, allow users to pass --hostname > and we will attempt to resolve against the DNS. > > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > --- > fabrics.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 75 insertions(+) > > diff --git a/fabrics.c b/fabrics.c > index a7d628b1f0c9..7bd95c4b0d10 100644 > --- a/fabrics.c > +++ b/fabrics.c > @@ -33,6 +33,10 @@ > #include <sys/stat.h> > #include <stddef.h> > > +#include <sys/types.h> > +#include <arpa/inet.h> > +#include <netdb.h> > + > #include "util/parser.h" > #include "nvme-ioctl.h" > #include "nvme-status.h" > @@ -60,6 +64,7 @@ static struct config { > char *nqn; > char *transport; > char *traddr; > + char *hostname; > char *trsvcid; > char *host_traddr; > char *hostnqn; > @@ -857,6 +862,54 @@ static int build_options(char *argstr, int max_len, bool discover) > return 0; > } > > +static int hostname2traddr(struct config *cfg) > +{ > + struct addrinfo *host_info, hints = {.ai_family = AF_UNSPEC}; > + char addrstr[NVMF_TRADDR_SIZE]; > + const char *p; > + int ret; > + > + if (cfg->traddr) { > + fprintf(stderr, "hostname and traddr cannot be passed together\n"); > + return -EINVAL; > + } > + > + ret = getaddrinfo(cfg->hostname, NULL, &hints, &host_info); > + if (ret) { > + fprintf(stderr, "failed to get host info for %s\n", cfg->hostname); > + return ret; > + } > + > + switch (host_info->ai_family) { > + case AF_INET: > + p = inet_ntop(host_info->ai_family, > + &(((struct sockaddr_in *)host_info->ai_addr)->sin_addr), > + addrstr, NVMF_TRADDR_SIZE); > + break; > + case AF_INET6: > + p = inet_ntop(host_info->ai_family, > + &(((struct sockaddr_in6 *)host_info->ai_addr)->sin6_addr), > + addrstr, NVMF_TRADDR_SIZE); > + break; > + default: > + fprintf(stderr, "unrecognized address family (%d) %s\n", > + host_info->ai_family, cfg->hostname); > + ret = -EINVAL; > + goto free_addrinfo; > + } > + > + if (!p) { > + fprintf(stderr, "failed to get traddr for %s\n", cfg->hostname); > + ret = -errno; > + goto free_addrinfo; > + } > + cfg->traddr = strdup(addrstr); > + > +free_addrinfo: > + freeaddrinfo(host_info); > + return ret; > +} > + > static int connect_ctrl(struct nvmf_disc_rsp_page_entry *e) > { > char argstr[BUF_SIZE], *p; > @@ -1230,6 +1283,13 @@ static int discover_from_conf_file(const char *desc, char *argstr, > if (cfg.persistent && !cfg.keep_alive_tmo) > cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO; > > + if (cfg.hostname) { > + ret = hostname2traddr(&cfg); > + if (ret) > + goto out; > + cfg.hostname = NULL; > + } > + I guess you add the NULL to check mutual exclusion later ? Can we check it in build_options ? > err = build_options(argstr, BUF_SIZE, true); > if (err) { > ret = err; > @@ -1259,6 +1319,7 @@ int fabrics_discover(const char *desc, int argc, char **argv, bool connect) > OPT_ARGS(opts) = { > OPT_LIST("transport", 't', &cfg.transport, "transport type"), > OPT_LIST("traddr", 'a', &cfg.traddr, "transport address"), > + OPT_LIST("hostname", 'A', &cfg.hostname, "transport address in a hostname form"), > OPT_LIST("trsvcid", 's', &cfg.trsvcid, "transport service id (e.g. IP port)"), > OPT_LIST("host-traddr", 'w', &cfg.host_traddr, "host traddr (e.g. FC WWN's)"), > OPT_LIST("hostnqn", 'q', &cfg.hostnqn, "user-defined hostnqn (if default not used)"), > @@ -1295,6 +1356,13 @@ int fabrics_discover(const char *desc, int argc, char **argv, bool connect) > } else { > if (cfg.persistent && !cfg.keep_alive_tmo) > cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO; > + > + if (cfg.hostname) { > + ret = hostname2traddr(&cfg); > + if (ret) > + goto out; please NULLify the hostname for Mutual exclusion check. > + } > + > ret = build_options(argstr, BUF_SIZE, true); > if (ret) > goto out; > @@ -1315,6 +1383,7 @@ int fabrics_connect(const char *desc, int argc, char **argv) > OPT_LIST("transport", 't', &cfg.transport, "transport type"), > OPT_LIST("nqn", 'n', &cfg.nqn, "nqn name"), > OPT_LIST("traddr", 'a', &cfg.traddr, "transport address"), > + OPT_LIST("hostname", 'A', &cfg.hostname, "transport address in a hostname form"), > OPT_LIST("trsvcid", 's', &cfg.trsvcid, "transport service id (e.g. IP port)"), > OPT_LIST("host-traddr", 'w', &cfg.host_traddr, "host traddr (e.g. FC WWN's)"), > OPT_LIST("hostnqn", 'q', &cfg.hostnqn, "user-defined hostnqn"), > @@ -1339,6 +1408,12 @@ int fabrics_connect(const char *desc, int argc, char **argv) > if (ret) > goto out; > > + if (cfg.hostname) { > + ret = hostname2traddr(&cfg); > + if (ret) > + goto out; same here. > + } > + > ret = build_options(argstr, BUF_SIZE, false); > if (ret) > goto out; _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr 2020-03-24 9:46 ` Max Gurtovoy @ 2020-03-24 16:21 ` Sagi Grimberg 2020-03-24 17:24 ` Max Gurtovoy 0 siblings, 1 reply; 13+ messages in thread From: Sagi Grimberg @ 2020-03-24 16:21 UTC (permalink / raw) To: Max Gurtovoy, linux-nvme, Keith Busch On 3/24/20 2:46 AM, Max Gurtovoy wrote: > > On 3/24/2020 11:03 AM, Sagi Grimberg wrote: >> Some users would like to use well known hostnames instead >> of remembering ip addresses. So, allow users to pass --hostname >> and we will attempt to resolve against the DNS. >> >> Signed-off-by: Sagi Grimberg <sagi@grimberg.me> >> --- >> fabrics.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 75 insertions(+) >> >> diff --git a/fabrics.c b/fabrics.c >> index a7d628b1f0c9..7bd95c4b0d10 100644 >> --- a/fabrics.c >> +++ b/fabrics.c >> @@ -33,6 +33,10 @@ >> #include <sys/stat.h> >> #include <stddef.h> >> +#include <sys/types.h> >> +#include <arpa/inet.h> >> +#include <netdb.h> >> + >> #include "util/parser.h" >> #include "nvme-ioctl.h" >> #include "nvme-status.h" >> @@ -60,6 +64,7 @@ static struct config { >> char *nqn; >> char *transport; >> char *traddr; >> + char *hostname; >> char *trsvcid; >> char *host_traddr; >> char *hostnqn; >> @@ -857,6 +862,54 @@ static int build_options(char *argstr, int >> max_len, bool discover) >> return 0; >> } >> +static int hostname2traddr(struct config *cfg) >> +{ >> + struct addrinfo *host_info, hints = {.ai_family = AF_UNSPEC}; >> + char addrstr[NVMF_TRADDR_SIZE]; >> + const char *p; >> + int ret; >> + >> + if (cfg->traddr) { >> + fprintf(stderr, "hostname and traddr cannot be passed >> together\n"); >> + return -EINVAL; >> + } >> + >> + ret = getaddrinfo(cfg->hostname, NULL, &hints, &host_info); >> + if (ret) { >> + fprintf(stderr, "failed to get host info for %s\n", >> cfg->hostname); >> + return ret; >> + } >> + >> + switch (host_info->ai_family) { >> + case AF_INET: >> + p = inet_ntop(host_info->ai_family, >> + &(((struct sockaddr_in *)host_info->ai_addr)->sin_addr), >> + addrstr, NVMF_TRADDR_SIZE); >> + break; >> + case AF_INET6: >> + p = inet_ntop(host_info->ai_family, >> + &(((struct sockaddr_in6 *)host_info->ai_addr)->sin6_addr), >> + addrstr, NVMF_TRADDR_SIZE); >> + break; >> + default: >> + fprintf(stderr, "unrecognized address family (%d) %s\n", >> + host_info->ai_family, cfg->hostname); >> + ret = -EINVAL; >> + goto free_addrinfo; >> + } >> + >> + if (!p) { >> + fprintf(stderr, "failed to get traddr for %s\n", cfg->hostname); >> + ret = -errno; >> + goto free_addrinfo; >> + } >> + cfg->traddr = strdup(addrstr); >> + >> +free_addrinfo: >> + freeaddrinfo(host_info); >> + return ret; >> +} >> + >> static int connect_ctrl(struct nvmf_disc_rsp_page_entry *e) >> { >> char argstr[BUF_SIZE], *p; >> @@ -1230,6 +1283,13 @@ static int discover_from_conf_file(const char >> *desc, char *argstr, >> if (cfg.persistent && !cfg.keep_alive_tmo) >> cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO; >> + if (cfg.hostname) { >> + ret = hostname2traddr(&cfg); >> + if (ret) >> + goto out; >> + cfg.hostname = NULL; >> + } >> + > > I guess you add the NULL to check mutual exclusion later ? The NULL assignment here is because we are looping on the conf file entries which can have a different hostname. The other locations are a one-shot so this assignment is not needed. > Can we check it in build_options ? When we get a hostname we resolve the traddr which is what build_options needs (hostname not an option that is passed to the driver). Does that answer your question? _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr 2020-03-24 16:21 ` Sagi Grimberg @ 2020-03-24 17:24 ` Max Gurtovoy 2020-03-24 19:11 ` Sagi Grimberg 0 siblings, 1 reply; 13+ messages in thread From: Max Gurtovoy @ 2020-03-24 17:24 UTC (permalink / raw) To: Sagi Grimberg, linux-nvme, Keith Busch On 3/24/2020 6:21 PM, Sagi Grimberg wrote: > > > On 3/24/20 2:46 AM, Max Gurtovoy wrote: >> >> On 3/24/2020 11:03 AM, Sagi Grimberg wrote: >>> Some users would like to use well known hostnames instead >>> of remembering ip addresses. So, allow users to pass --hostname >>> and we will attempt to resolve against the DNS. >>> >>> Signed-off-by: Sagi Grimberg <sagi@grimberg.me> >>> --- >>> fabrics.c | 75 >>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 75 insertions(+) >>> >>> diff --git a/fabrics.c b/fabrics.c >>> index a7d628b1f0c9..7bd95c4b0d10 100644 >>> --- a/fabrics.c >>> +++ b/fabrics.c >>> @@ -33,6 +33,10 @@ >>> #include <sys/stat.h> >>> #include <stddef.h> >>> +#include <sys/types.h> >>> +#include <arpa/inet.h> >>> +#include <netdb.h> >>> + >>> #include "util/parser.h" >>> #include "nvme-ioctl.h" >>> #include "nvme-status.h" >>> @@ -60,6 +64,7 @@ static struct config { >>> char *nqn; >>> char *transport; >>> char *traddr; >>> + char *hostname; >>> char *trsvcid; >>> char *host_traddr; >>> char *hostnqn; >>> @@ -857,6 +862,54 @@ static int build_options(char *argstr, int >>> max_len, bool discover) >>> return 0; >>> } >>> +static int hostname2traddr(struct config *cfg) >>> +{ >>> + struct addrinfo *host_info, hints = {.ai_family = AF_UNSPEC}; >>> + char addrstr[NVMF_TRADDR_SIZE]; >>> + const char *p; >>> + int ret; >>> + >>> + if (cfg->traddr) { >>> + fprintf(stderr, "hostname and traddr cannot be passed >>> together\n"); >>> + return -EINVAL; >>> + } >>> + >>> + ret = getaddrinfo(cfg->hostname, NULL, &hints, &host_info); >>> + if (ret) { >>> + fprintf(stderr, "failed to get host info for %s\n", >>> cfg->hostname); >>> + return ret; >>> + } >>> + >>> + switch (host_info->ai_family) { >>> + case AF_INET: >>> + p = inet_ntop(host_info->ai_family, >>> + &(((struct sockaddr_in *)host_info->ai_addr)->sin_addr), >>> + addrstr, NVMF_TRADDR_SIZE); >>> + break; >>> + case AF_INET6: >>> + p = inet_ntop(host_info->ai_family, >>> + &(((struct sockaddr_in6 *)host_info->ai_addr)->sin6_addr), >>> + addrstr, NVMF_TRADDR_SIZE); >>> + break; >>> + default: >>> + fprintf(stderr, "unrecognized address family (%d) %s\n", >>> + host_info->ai_family, cfg->hostname); >>> + ret = -EINVAL; >>> + goto free_addrinfo; >>> + } >>> + >>> + if (!p) { >>> + fprintf(stderr, "failed to get traddr for %s\n", >>> cfg->hostname); >>> + ret = -errno; >>> + goto free_addrinfo; >>> + } >>> + cfg->traddr = strdup(addrstr); >>> + >>> +free_addrinfo: >>> + freeaddrinfo(host_info); >>> + return ret; >>> +} >>> + >>> static int connect_ctrl(struct nvmf_disc_rsp_page_entry *e) >>> { >>> char argstr[BUF_SIZE], *p; >>> @@ -1230,6 +1283,13 @@ static int discover_from_conf_file(const char >>> *desc, char *argstr, >>> if (cfg.persistent && !cfg.keep_alive_tmo) >>> cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO; >>> + if (cfg.hostname) { >>> + ret = hostname2traddr(&cfg); >>> + if (ret) >>> + goto out; >>> + cfg.hostname = NULL; >>> + } >>> + >> >> I guess you add the NULL to check mutual exclusion later ? > > The NULL assignment here is because we are looping on the conf > file entries which can have a different hostname. The other > locations are a one-shot so this assignment is not needed. > >> Can we check it in build_options ? > > When we get a hostname we resolve the traddr which is what > build_options needs (hostname not an option that is passed to > the driver). > > Does that answer your question? I meant to prevent a user to send them both in the command line (no to send it to the driver). _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr 2020-03-24 17:24 ` Max Gurtovoy @ 2020-03-24 19:11 ` Sagi Grimberg 2020-03-24 22:57 ` Max Gurtovoy 0 siblings, 1 reply; 13+ messages in thread From: Sagi Grimberg @ 2020-03-24 19:11 UTC (permalink / raw) To: Max Gurtovoy, linux-nvme, Keith Busch >>>> static int connect_ctrl(struct nvmf_disc_rsp_page_entry *e) >>>> { >>>> char argstr[BUF_SIZE], *p; >>>> @@ -1230,6 +1283,13 @@ static int discover_from_conf_file(const char >>>> *desc, char *argstr, >>>> if (cfg.persistent && !cfg.keep_alive_tmo) >>>> cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO; >>>> + if (cfg.hostname) { >>>> + ret = hostname2traddr(&cfg); >>>> + if (ret) >>>> + goto out; >>>> + cfg.hostname = NULL; >>>> + } >>>> + >>> >>> I guess you add the NULL to check mutual exclusion later ? >> >> The NULL assignment here is because we are looping on the conf >> file entries which can have a different hostname. The other >> locations are a one-shot so this assignment is not needed. >> >>> Can we check it in build_options ? >> >> When we get a hostname we resolve the traddr which is what >> build_options needs (hostname not an option that is passed to >> the driver). >> >> Does that answer your question? > > I meant to prevent a user to send them both in the command line (no to > send it to the driver). build_options is just converting options to an argstr, not sure its a good idea to move that there... _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr 2020-03-24 19:11 ` Sagi Grimberg @ 2020-03-24 22:57 ` Max Gurtovoy 0 siblings, 0 replies; 13+ messages in thread From: Max Gurtovoy @ 2020-03-24 22:57 UTC (permalink / raw) To: Sagi Grimberg, linux-nvme, Keith Busch On 3/24/2020 9:11 PM, Sagi Grimberg wrote: > >>>>> static int connect_ctrl(struct nvmf_disc_rsp_page_entry *e) >>>>> { >>>>> char argstr[BUF_SIZE], *p; >>>>> @@ -1230,6 +1283,13 @@ static int discover_from_conf_file(const >>>>> char *desc, char *argstr, >>>>> if (cfg.persistent && !cfg.keep_alive_tmo) >>>>> cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO; >>>>> + if (cfg.hostname) { >>>>> + ret = hostname2traddr(&cfg); >>>>> + if (ret) >>>>> + goto out; >>>>> + cfg.hostname = NULL; >>>>> + } >>>>> + >>>> >>>> I guess you add the NULL to check mutual exclusion later ? >>> >>> The NULL assignment here is because we are looping on the conf >>> file entries which can have a different hostname. The other >>> locations are a one-shot so this assignment is not needed. >>> >>>> Can we check it in build_options ? >>> >>> When we get a hostname we resolve the traddr which is what >>> build_options needs (hostname not an option that is passed to >>> the driver). >>> >>> Does that answer your question? >> >> I meant to prevent a user to send them both in the command line (no >> to send it to the driver). > > build_options is just converting options to an argstr, not sure its a > good idea to move that there... You right, I was just thinking out loud. We can check it before calling the resolution function hostname2traddr. . _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr 2020-03-24 9:03 ` [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr Sagi Grimberg 2020-03-24 9:46 ` Max Gurtovoy @ 2020-04-01 19:39 ` Keith Busch 2020-04-01 20:33 ` Sagi Grimberg 1 sibling, 1 reply; 13+ messages in thread From: Keith Busch @ 2020-04-01 19:39 UTC (permalink / raw) To: Sagi Grimberg; +Cc: linux-nvme On Tue, Mar 24, 2020 at 02:03:24AM -0700, Sagi Grimberg wrote: > Some users would like to use well known hostnames instead > of remembering ip addresses. So, allow users to pass --hostname > and we will attempt to resolve against the DNS. > > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > --- > fabrics.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 75 insertions(+) > > diff --git a/fabrics.c b/fabrics.c > index a7d628b1f0c9..7bd95c4b0d10 100644 > --- a/fabrics.c > +++ b/fabrics.c > @@ -33,6 +33,10 @@ > #include <sys/stat.h> > #include <stddef.h> > > +#include <sys/types.h> > +#include <arpa/inet.h> > +#include <netdb.h> > + > #include "util/parser.h" > #include "nvme-ioctl.h" > #include "nvme-status.h" > @@ -60,6 +64,7 @@ static struct config { > char *nqn; > char *transport; > char *traddr; > + char *hostname; > char *trsvcid; > char *host_traddr; > char *hostnqn; > @@ -857,6 +862,54 @@ static int build_options(char *argstr, int max_len, bool discover) > return 0; > } The code looks fine. I realize 'hostname' in this context is referring to the remote host and resolves the target's transport address, but it just sounds potentially confusing given we have "host_traddr". I'm not sure of a better name, though, so if this is normal convention, then okay. > +static int hostname2traddr(struct config *cfg) > +{ > + struct addrinfo *host_info, hints = {.ai_family = AF_UNSPEC}; > + char addrstr[NVMF_TRADDR_SIZE]; > + const char *p; > + int ret; > + > + if (cfg->traddr) { > + fprintf(stderr, "hostname and traddr cannot be passed together\n"); > + return -EINVAL; > + } > + > + ret = getaddrinfo(cfg->hostname, NULL, &hints, &host_info); > + if (ret) { > + fprintf(stderr, "failed to get host info for %s\n", cfg->hostname); > + return ret; > + } > + > + switch (host_info->ai_family) { > + case AF_INET: > + p = inet_ntop(host_info->ai_family, > + &(((struct sockaddr_in *)host_info->ai_addr)->sin_addr), > + addrstr, NVMF_TRADDR_SIZE); > + break; > + case AF_INET6: > + p = inet_ntop(host_info->ai_family, > + &(((struct sockaddr_in6 *)host_info->ai_addr)->sin6_addr), > + addrstr, NVMF_TRADDR_SIZE); > + break; > + default: > + fprintf(stderr, "unrecognized address family (%d) %s\n", > + host_info->ai_family, cfg->hostname); > + ret = -EINVAL; > + goto free_addrinfo; > + } > + > + if (!p) { > + fprintf(stderr, "failed to get traddr for %s\n", cfg->hostname); > + ret = -errno; > + goto free_addrinfo; > + } > + cfg->traddr = strdup(addrstr); > + > +free_addrinfo: > + freeaddrinfo(host_info); > + return ret; > +} > + > static int connect_ctrl(struct nvmf_disc_rsp_page_entry *e) > { > char argstr[BUF_SIZE], *p; > @@ -1230,6 +1283,13 @@ static int discover_from_conf_file(const char *desc, char *argstr, > if (cfg.persistent && !cfg.keep_alive_tmo) > cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO; > > + if (cfg.hostname) { > + ret = hostname2traddr(&cfg); > + if (ret) > + goto out; > + cfg.hostname = NULL; > + } > + > err = build_options(argstr, BUF_SIZE, true); > if (err) { > ret = err; > @@ -1259,6 +1319,7 @@ int fabrics_discover(const char *desc, int argc, char **argv, bool connect) > OPT_ARGS(opts) = { > OPT_LIST("transport", 't', &cfg.transport, "transport type"), > OPT_LIST("traddr", 'a', &cfg.traddr, "transport address"), > + OPT_LIST("hostname", 'A', &cfg.hostname, "transport address in a hostname form"), > OPT_LIST("trsvcid", 's', &cfg.trsvcid, "transport service id (e.g. IP port)"), > OPT_LIST("host-traddr", 'w', &cfg.host_traddr, "host traddr (e.g. FC WWN's)"), > OPT_LIST("hostnqn", 'q', &cfg.hostnqn, "user-defined hostnqn (if default not used)"), > @@ -1295,6 +1356,13 @@ int fabrics_discover(const char *desc, int argc, char **argv, bool connect) > } else { > if (cfg.persistent && !cfg.keep_alive_tmo) > cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO; > + > + if (cfg.hostname) { > + ret = hostname2traddr(&cfg); > + if (ret) > + goto out; > + } > + > ret = build_options(argstr, BUF_SIZE, true); > if (ret) > goto out; > @@ -1315,6 +1383,7 @@ int fabrics_connect(const char *desc, int argc, char **argv) > OPT_LIST("transport", 't', &cfg.transport, "transport type"), > OPT_LIST("nqn", 'n', &cfg.nqn, "nqn name"), > OPT_LIST("traddr", 'a', &cfg.traddr, "transport address"), > + OPT_LIST("hostname", 'A', &cfg.hostname, "transport address in a hostname form"), > OPT_LIST("trsvcid", 's', &cfg.trsvcid, "transport service id (e.g. IP port)"), > OPT_LIST("host-traddr", 'w', &cfg.host_traddr, "host traddr (e.g. FC WWN's)"), > OPT_LIST("hostnqn", 'q', &cfg.hostnqn, "user-defined hostnqn"), > @@ -1339,6 +1408,12 @@ int fabrics_connect(const char *desc, int argc, char **argv) > if (ret) > goto out; > > + if (cfg.hostname) { > + ret = hostname2traddr(&cfg); > + if (ret) > + goto out; > + } > + > ret = build_options(argstr, BUF_SIZE, false); > if (ret) > goto out; > -- > 2.20.1 > _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr 2020-04-01 19:39 ` Keith Busch @ 2020-04-01 20:33 ` Sagi Grimberg 2020-04-01 20:43 ` Keith Busch 0 siblings, 1 reply; 13+ messages in thread From: Sagi Grimberg @ 2020-04-01 20:33 UTC (permalink / raw) To: Keith Busch; +Cc: linux-nvme >> Some users would like to use well known hostnames instead >> of remembering ip addresses. So, allow users to pass --hostname >> and we will attempt to resolve against the DNS. >> >> Signed-off-by: Sagi Grimberg <sagi@grimberg.me> >> --- >> fabrics.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 75 insertions(+) >> >> diff --git a/fabrics.c b/fabrics.c >> index a7d628b1f0c9..7bd95c4b0d10 100644 >> --- a/fabrics.c >> +++ b/fabrics.c >> @@ -33,6 +33,10 @@ >> #include <sys/stat.h> >> #include <stddef.h> >> >> +#include <sys/types.h> >> +#include <arpa/inet.h> >> +#include <netdb.h> >> + >> #include "util/parser.h" >> #include "nvme-ioctl.h" >> #include "nvme-status.h" >> @@ -60,6 +64,7 @@ static struct config { >> char *nqn; >> char *transport; >> char *traddr; >> + char *hostname; >> char *trsvcid; >> char *host_traddr; >> char *hostnqn; >> @@ -857,6 +862,54 @@ static int build_options(char *argstr, int max_len, bool discover) >> return 0; >> } > > The code looks fine. > > I realize 'hostname' in this context is referring to the remote host and > resolves the target's transport address, but it just sounds potentially > confusing given we have "host_traddr". I'm not sure of a better name, > though, so if this is normal convention, then okay. We could do it like ssh that can accept either IP or hostname (we check if string is IP and if not, resort to name resolution) Is that preferable? _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr 2020-04-01 20:33 ` Sagi Grimberg @ 2020-04-01 20:43 ` Keith Busch 0 siblings, 0 replies; 13+ messages in thread From: Keith Busch @ 2020-04-01 20:43 UTC (permalink / raw) To: Sagi Grimberg; +Cc: linux-nvme On Wed, Apr 01, 2020 at 01:33:40PM -0700, Sagi Grimberg wrote: > > > > Some users would like to use well known hostnames instead > > > of remembering ip addresses. So, allow users to pass --hostname > > > and we will attempt to resolve against the DNS. > > > > > > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > > > --- > > > fabrics.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 75 insertions(+) > > > > > > diff --git a/fabrics.c b/fabrics.c > > > index a7d628b1f0c9..7bd95c4b0d10 100644 > > > --- a/fabrics.c > > > +++ b/fabrics.c > > > @@ -33,6 +33,10 @@ > > > #include <sys/stat.h> > > > #include <stddef.h> > > > +#include <sys/types.h> > > > +#include <arpa/inet.h> > > > +#include <netdb.h> > > > + > > > #include "util/parser.h" > > > #include "nvme-ioctl.h" > > > #include "nvme-status.h" > > > @@ -60,6 +64,7 @@ static struct config { > > > char *nqn; > > > char *transport; > > > char *traddr; > > > + char *hostname; > > > char *trsvcid; > > > char *host_traddr; > > > char *hostnqn; > > > @@ -857,6 +862,54 @@ static int build_options(char *argstr, int max_len, bool discover) > > > return 0; > > > } > > > > The code looks fine. > > > > I realize 'hostname' in this context is referring to the remote host and > > resolves the target's transport address, but it just sounds potentially > > confusing given we have "host_traddr". I'm not sure of a better name, > > though, so if this is normal convention, then okay. > > We could do it like ssh that can accept either IP or hostname (we check > if string is IP and if not, resort to name resolution) > > Is that preferable? Yah, that option sounds more appealing to me. _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH rfc 0/2] support ip resolution with hostnames 2020-03-24 9:03 [PATCH rfc 0/2] support ip resolution with hostnames Sagi Grimberg 2020-03-24 9:03 ` [PATCH rfc 1/2] fabrics: add fabrics_ prefix to fabrics operations Sagi Grimberg 2020-03-24 9:03 ` [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr Sagi Grimberg @ 2020-03-24 9:06 ` Sagi Grimberg 2020-04-01 6:03 ` Sagi Grimberg 3 siblings, 0 replies; 13+ messages in thread From: Sagi Grimberg @ 2020-03-24 9:06 UTC (permalink / raw) To: linux-nvme, Keith Busch Needless to say that this series is for nvme-cli... _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH rfc 0/2] support ip resolution with hostnames 2020-03-24 9:03 [PATCH rfc 0/2] support ip resolution with hostnames Sagi Grimberg ` (2 preceding siblings ...) 2020-03-24 9:06 ` [PATCH rfc 0/2] support ip resolution with hostnames Sagi Grimberg @ 2020-04-01 6:03 ` Sagi Grimberg 3 siblings, 0 replies; 13+ messages in thread From: Sagi Grimberg @ 2020-04-01 6:03 UTC (permalink / raw) To: linux-nvme, Keith Busch Any other feedback in this approach? _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-04-01 20:43 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-24 9:03 [PATCH rfc 0/2] support ip resolution with hostnames Sagi Grimberg 2020-03-24 9:03 ` [PATCH rfc 1/2] fabrics: add fabrics_ prefix to fabrics operations Sagi Grimberg 2020-03-24 9:03 ` [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr Sagi Grimberg 2020-03-24 9:46 ` Max Gurtovoy 2020-03-24 16:21 ` Sagi Grimberg 2020-03-24 17:24 ` Max Gurtovoy 2020-03-24 19:11 ` Sagi Grimberg 2020-03-24 22:57 ` Max Gurtovoy 2020-04-01 19:39 ` Keith Busch 2020-04-01 20:33 ` Sagi Grimberg 2020-04-01 20:43 ` Keith Busch 2020-03-24 9:06 ` [PATCH rfc 0/2] support ip resolution with hostnames Sagi Grimberg 2020-04-01 6:03 ` Sagi Grimberg
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).