nvme-fc: revise TRADDR parsing
authorJames Smart <jsmart2021@gmail.com>
Mon, 17 Jul 2017 20:59:39 +0000 (13:59 -0700)
committerChristoph Hellwig <hch@lst.de>
Tue, 25 Jul 2017 16:05:25 +0000 (18:05 +0200)
The FC-NVME spec hasn't locked down on the format string for TRADDR.
Currently the spec is lobbying for "nn-<16hexdigits>:pn-<16hexdigits>"
where the wwn's are hex values but not prefixed by 0x.

Most implementations so far expect a string format of
"nn-0x<16hexdigits>:pn-0x<16hexdigits>" to be used. The transport
uses the match_u64 parser which requires a leading 0x prefix to set
the base properly. If it's not there, a match will either fail or return
a base 10 value.

The resolution in T11 is pushing out. Therefore, to fix things now and
to cover any eventuality and any implementations already in the field,
this patch adds support for both formats.

The change consists of replacing the token matching routine with a
routine that validates the fixed string format, and then builds
a local copy of the hex name with a 0x prefix before calling
the system parser.

Note: the same parser routine exists in both the initiator and target
transports. Given this is about the only "shared" item, we chose to
replicate rather than create an interdendency on some shared code.

Signed-off-by: James Smart <james.smart@broadcom.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
drivers/nvme/host/fc.c
drivers/nvme/target/fc.c
include/linux/nvme-fc.h

index 5630ca46c3b5d7e11e86a98cadd0e9d3cdffc479..5c2a08ef08bafd8351b0000ac5029620a9f19522 100644 (file)
@@ -2805,66 +2805,70 @@ out_fail:
        return ERR_PTR(ret);
 }
 
-enum {
-       FCT_TRADDR_ERR          = 0,
-       FCT_TRADDR_WWNN         = 1 << 0,
-       FCT_TRADDR_WWPN         = 1 << 1,
-};
 
 struct nvmet_fc_traddr {
        u64     nn;
        u64     pn;
 };
 
-static const match_table_t traddr_opt_tokens = {
-       { FCT_TRADDR_WWNN,      "nn-%s"         },
-       { FCT_TRADDR_WWPN,      "pn-%s"         },
-       { FCT_TRADDR_ERR,       NULL            }
-};
-
 static int
-nvme_fc_parse_address(struct nvmet_fc_traddr *traddr, char *buf)
+__nvme_fc_parse_u64(substring_t *sstr, u64 *val)
 {
-       substring_t args[MAX_OPT_ARGS];
-       char *options, *o, *p;
-       int token, ret = 0;
        u64 token64;
 
-       options = o = kstrdup(buf, GFP_KERNEL);
-       if (!options)
-               return -ENOMEM;
+       if (match_u64(sstr, &token64))
+               return -EINVAL;
+       *val = token64;
 
-       while ((p = strsep(&o, ":\n")) != NULL) {
-               if (!*p)
-                       continue;
+       return 0;
+}
 
-               token = match_token(p, traddr_opt_tokens, args);
-               switch (token) {
-               case FCT_TRADDR_WWNN:
-                       if (match_u64(args, &token64)) {
-                               ret = -EINVAL;
-                               goto out;
-                       }
-                       traddr->nn = token64;
-                       break;
-               case FCT_TRADDR_WWPN:
-                       if (match_u64(args, &token64)) {
-                               ret = -EINVAL;
-                               goto out;
-                       }
-                       traddr->pn = token64;
-                       break;
-               default:
-                       pr_warn("unknown traddr token or missing value '%s'\n",
-                                       p);
-                       ret = -EINVAL;
-                       goto out;
-               }
-       }
+/*
+ * This routine validates and extracts the WWN's from the TRADDR string.
+ * As kernel parsers need the 0x to determine number base, universally
+ * build string to parse with 0x prefix before parsing name strings.
+ */
+static int
+nvme_fc_parse_traddr(struct nvmet_fc_traddr *traddr, char *buf, size_t blen)
+{
+       char name[2 + NVME_FC_TRADDR_HEXNAMELEN + 1];
+       substring_t wwn = { name, &name[sizeof(name)-1] };
+       int nnoffset, pnoffset;
+
+       /* validate it string one of the 2 allowed formats */
+       if (strnlen(buf, blen) == NVME_FC_TRADDR_MAXLENGTH &&
+                       !strncmp(buf, "nn-0x", NVME_FC_TRADDR_OXNNLEN) &&
+                       !strncmp(&buf[NVME_FC_TRADDR_MAX_PN_OFFSET],
+                               "pn-0x", NVME_FC_TRADDR_OXNNLEN)) {
+               nnoffset = NVME_FC_TRADDR_OXNNLEN;
+               pnoffset = NVME_FC_TRADDR_MAX_PN_OFFSET +
+                                               NVME_FC_TRADDR_OXNNLEN;
+       } else if ((strnlen(buf, blen) == NVME_FC_TRADDR_MINLENGTH &&
+                       !strncmp(buf, "nn-", NVME_FC_TRADDR_NNLEN) &&
+                       !strncmp(&buf[NVME_FC_TRADDR_MIN_PN_OFFSET],
+                               "pn-", NVME_FC_TRADDR_NNLEN))) {
+               nnoffset = NVME_FC_TRADDR_NNLEN;
+               pnoffset = NVME_FC_TRADDR_MIN_PN_OFFSET + NVME_FC_TRADDR_NNLEN;
+       } else
+               goto out_einval;
 
-out:
-       kfree(options);
-       return ret;
+       name[0] = '0';
+       name[1] = 'x';
+       name[2 + NVME_FC_TRADDR_HEXNAMELEN] = 0;
+
+       memcpy(&name[2], &buf[nnoffset], NVME_FC_TRADDR_HEXNAMELEN);
+       if (__nvme_fc_parse_u64(&wwn, &traddr->nn))
+               goto out_einval;
+
+       memcpy(&name[2], &buf[pnoffset], NVME_FC_TRADDR_HEXNAMELEN);
+       if (__nvme_fc_parse_u64(&wwn, &traddr->pn))
+               goto out_einval;
+
+       return 0;
+
+out_einval:
+       pr_warn("%s: bad traddr string\n", __func__);
+       return -EINVAL;
 }
 
 static struct nvme_ctrl *
@@ -2878,11 +2882,11 @@ nvme_fc_create_ctrl(struct device *dev, struct nvmf_ctrl_options *opts)
        unsigned long flags;
        int ret;
 
-       ret = nvme_fc_parse_address(&raddr, opts->traddr);
+       ret = nvme_fc_parse_traddr(&raddr, opts->traddr, NVMF_TRADDR_SIZE);
        if (ret || !raddr.nn || !raddr.pn)
                return ERR_PTR(-EINVAL);
 
-       ret = nvme_fc_parse_address(&laddr, opts->host_traddr);
+       ret = nvme_fc_parse_traddr(&laddr, opts->host_traddr, NVMF_TRADDR_SIZE);
        if (ret || !laddr.nn || !laddr.pn)
                return ERR_PTR(-EINVAL);
 
index d5801c150b1cbb41820c837d4732132f3c8151c8..31ca55dfcb1d49f3a1d88f7c6f5d0e7f0ee1e1ea 100644 (file)
@@ -2293,66 +2293,70 @@ nvmet_fc_rcv_fcp_abort(struct nvmet_fc_target_port *target_port,
 }
 EXPORT_SYMBOL_GPL(nvmet_fc_rcv_fcp_abort);
 
-enum {
-       FCT_TRADDR_ERR          = 0,
-       FCT_TRADDR_WWNN         = 1 << 0,
-       FCT_TRADDR_WWPN         = 1 << 1,
-};
 
 struct nvmet_fc_traddr {
        u64     nn;
        u64     pn;
 };
 
-static const match_table_t traddr_opt_tokens = {
-       { FCT_TRADDR_WWNN,      "nn-%s"         },
-       { FCT_TRADDR_WWPN,      "pn-%s"         },
-       { FCT_TRADDR_ERR,       NULL            }
-};
-
 static int
-nvmet_fc_parse_traddr(struct nvmet_fc_traddr *traddr, char *buf)
+__nvme_fc_parse_u64(substring_t *sstr, u64 *val)
 {
-       substring_t args[MAX_OPT_ARGS];
-       char *options, *o, *p;
-       int token, ret = 0;
        u64 token64;
 
-       options = o = kstrdup(buf, GFP_KERNEL);
-       if (!options)
-               return -ENOMEM;
+       if (match_u64(sstr, &token64))
+               return -EINVAL;
+       *val = token64;
 
-       while ((p = strsep(&o, ":\n")) != NULL) {
-               if (!*p)
-                       continue;
+       return 0;
+}
 
-               token = match_token(p, traddr_opt_tokens, args);
-               switch (token) {
-               case FCT_TRADDR_WWNN:
-                       if (match_u64(args, &token64)) {
-                               ret = -EINVAL;
-                               goto out;
-                       }
-                       traddr->nn = token64;
-                       break;
-               case FCT_TRADDR_WWPN:
-                       if (match_u64(args, &token64)) {
-                               ret = -EINVAL;
-                               goto out;
-                       }
-                       traddr->pn = token64;
-                       break;
-               default:
-                       pr_warn("unknown traddr token or missing value '%s'\n",
-                                       p);
-                       ret = -EINVAL;
-                       goto out;
-               }
-       }
+/*
+ * This routine validates and extracts the WWN's from the TRADDR string.
+ * As kernel parsers need the 0x to determine number base, universally
+ * build string to parse with 0x prefix before parsing name strings.
+ */
+static int
+nvme_fc_parse_traddr(struct nvmet_fc_traddr *traddr, char *buf, size_t blen)
+{
+       char name[2 + NVME_FC_TRADDR_HEXNAMELEN + 1];
+       substring_t wwn = { name, &name[sizeof(name)-1] };
+       int nnoffset, pnoffset;
+
+       /* validate it string one of the 2 allowed formats */
+       if (strnlen(buf, blen) == NVME_FC_TRADDR_MAXLENGTH &&
+                       !strncmp(buf, "nn-0x", NVME_FC_TRADDR_OXNNLEN) &&
+                       !strncmp(&buf[NVME_FC_TRADDR_MAX_PN_OFFSET],
+                               "pn-0x", NVME_FC_TRADDR_OXNNLEN)) {
+               nnoffset = NVME_FC_TRADDR_OXNNLEN;
+               pnoffset = NVME_FC_TRADDR_MAX_PN_OFFSET +
+                                               NVME_FC_TRADDR_OXNNLEN;
+       } else if ((strnlen(buf, blen) == NVME_FC_TRADDR_MINLENGTH &&
+                       !strncmp(buf, "nn-", NVME_FC_TRADDR_NNLEN) &&
+                       !strncmp(&buf[NVME_FC_TRADDR_MIN_PN_OFFSET],
+                               "pn-", NVME_FC_TRADDR_NNLEN))) {
+               nnoffset = NVME_FC_TRADDR_NNLEN;
+               pnoffset = NVME_FC_TRADDR_MIN_PN_OFFSET + NVME_FC_TRADDR_NNLEN;
+       } else
+               goto out_einval;
+
+       name[0] = '0';
+       name[1] = 'x';
+       name[2 + NVME_FC_TRADDR_HEXNAMELEN] = 0;
+
+       memcpy(&name[2], &buf[nnoffset], NVME_FC_TRADDR_HEXNAMELEN);
+       if (__nvme_fc_parse_u64(&wwn, &traddr->nn))
+               goto out_einval;
+
+       memcpy(&name[2], &buf[pnoffset], NVME_FC_TRADDR_HEXNAMELEN);
+       if (__nvme_fc_parse_u64(&wwn, &traddr->pn))
+               goto out_einval;
 
-out:
-       kfree(options);
-       return ret;
+       return 0;
+
+out_einval:
+       pr_warn("%s: bad traddr string\n", __func__);
+       return -EINVAL;
 }
 
 static int
@@ -2370,7 +2374,8 @@ nvmet_fc_add_port(struct nvmet_port *port)
 
        /* map the traddr address info to a target port */
 
-       ret = nvmet_fc_parse_traddr(&traddr, port->disc_addr.traddr);
+       ret = nvme_fc_parse_traddr(&traddr, port->disc_addr.traddr,
+                       sizeof(port->disc_addr.traddr));
        if (ret)
                return ret;
 
index 21c37e39e41a2829c77ebac20afcd8a38102ac57..36cca93a5ff273e22be5511b44a33c4d9b3f8796 100644 (file)
@@ -334,5 +334,24 @@ struct fcnvme_ls_disconnect_acc {
 #define NVME_FC_LS_TIMEOUT_SEC         2               /* 2 seconds */
 #define NVME_FC_TGTOP_TIMEOUT_SEC      2               /* 2 seconds */
 
+/*
+ * TRADDR string must be of form "nn-<16hexdigits>:pn-<16hexdigits>"
+ * the string is allowed to be specified with or without a "0x" prefix
+ * infront of the <16hexdigits>.  Without is considered the "min" string
+ * and with is considered the "max" string. The hexdigits may be upper
+ * or lower case.
+ */
+#define NVME_FC_TRADDR_NNLEN           3       /* "?n-" */
+#define NVME_FC_TRADDR_OXNNLEN         5       /* "?n-0x" */
+#define NVME_FC_TRADDR_HEXNAMELEN      16
+#define NVME_FC_TRADDR_MINLENGTH       \
+               (2 * (NVME_FC_TRADDR_NNLEN + NVME_FC_TRADDR_HEXNAMELEN) + 1)
+#define NVME_FC_TRADDR_MAXLENGTH       \
+               (2 * (NVME_FC_TRADDR_OXNNLEN + NVME_FC_TRADDR_HEXNAMELEN) + 1)
+#define NVME_FC_TRADDR_MIN_PN_OFFSET   \
+               (NVME_FC_TRADDR_NNLEN + NVME_FC_TRADDR_HEXNAMELEN + 1)
+#define NVME_FC_TRADDR_MAX_PN_OFFSET   \
+               (NVME_FC_TRADDR_OXNNLEN + NVME_FC_TRADDR_HEXNAMELEN + 1)
+
 
 #endif /* _NVME_FC_H */