May I get feedback for this?
For the first time I use fcloop, I set:
# echo "wwnn=0x3,wwpn=0x1" > /sys/class/fcloop/ctl/add_target_port
However, I would not be able to move forward if I use "0x3" or "0x1" for nvme-fc
target or host further. Instead, the address and port should be
0x0000000000000003 and 0x0000000000000001.
This patch would sync the requirements of input format for nvme-fc and
nvme-fcloop, unless this would break existing test suite (e.g., blktest).
Thank you very much!
Dongli Zhang
On 5/25/20 9:21 PM, Dongli Zhang wrote:
> The nvme host and target verify the wwnn and wwpn format via
> nvme_fc_parse_traddr(). For instance, it is required that the length of
> wwnn to be either 21 ("nn-0x") or 19 (nn-).
>
> Add this verification to nvme-fcloop so that the input should always be in
> hex and the length of input should always be 18.
>
> Otherwise, the user may use e.g. 0x2 to create fcloop local port, while
> 0x0000000000000002 is required for nvme host and target. This makes the
> requirement of format confusing.
>
> Signed-off-by: Dongli Zhang <dongli.zh...@oracle.com>
> ---
> drivers/nvme/target/fcloop.c | 29 +++++++++++++++++++++++------
> 1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
> index f69ce66e2d44..14124e6d4bf2 100644
> --- a/drivers/nvme/target/fcloop.c
> +++ b/drivers/nvme/target/fcloop.c
> @@ -43,6 +43,17 @@ static const match_table_t opt_tokens = {
> { NVMF_OPT_ERR, NULL }
> };
>
> +static int fcloop_verify_addr(substring_t *s)
> +{
> + size_t blen = s->to - s->from + 1;
> +
> + if (strnlen(s->from, blen) != NVME_FC_TRADDR_HEXNAMELEN + 2 ||
> + strncmp(s->from, "0x", 2))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static int
> fcloop_parse_options(struct fcloop_ctrl_options *opts,
> const char *buf)
> @@ -64,14 +75,16 @@ fcloop_parse_options(struct fcloop_ctrl_options *opts,
> opts->mask |= token;
> switch (token) {
> case NVMF_OPT_WWNN:
> - if (match_u64(args, &token64)) {
> + if (fcloop_verify_addr(args) ||
> + match_u64(args, &token64)) {
> ret = -EINVAL;
> goto out_free_options;
> }
> opts->wwnn = token64;
> break;
> case NVMF_OPT_WWPN:
> - if (match_u64(args, &token64)) {
> + if (fcloop_verify_addr(args) ||
> + match_u64(args, &token64)) {
> ret = -EINVAL;
> goto out_free_options;
> }
> @@ -92,14 +105,16 @@ fcloop_parse_options(struct fcloop_ctrl_options *opts,
> opts->fcaddr = token;
> break;
> case NVMF_OPT_LPWWNN:
> - if (match_u64(args, &token64)) {
> + if (fcloop_verify_addr(args) ||
> + match_u64(args, &token64)) {
> ret = -EINVAL;
> goto out_free_options;
> }
> opts->lpwwnn = token64;
> break;
> case NVMF_OPT_LPWWPN:
> - if (match_u64(args, &token64)) {
> + if (fcloop_verify_addr(args) ||
> + match_u64(args, &token64)) {
> ret = -EINVAL;
> goto out_free_options;
> }
> @@ -141,14 +156,16 @@ fcloop_parse_nm_options(struct device *dev, u64 *nname,
> u64 *pname,
> token = match_token(p, opt_tokens, args);
> switch (token) {
> case NVMF_OPT_WWNN:
> - if (match_u64(args, &token64)) {
> + if (fcloop_verify_addr(args) ||
> + match_u64(args, &token64)) {
> ret = -EINVAL;
> goto out_free_options;
> }
> *nname = token64;
> break;
> case NVMF_OPT_WWPN:
> - if (match_u64(args, &token64)) {
> + if (fcloop_verify_addr(args) ||
> + match_u64(args, &token64)) {
> ret = -EINVAL;
> goto out_free_options;
> }
>
