| Message ID | 20181018115120.17893-4-leon@kernel.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Ahern |
| Headers | show |
| Series | rdma: IB device rename | expand |
On 10/18/18 5:51 AM, Leon Romanovsky wrote: > From: Leon Romanovsky <leonro@mellanox.com> > > Enrich rdmatool with an option to rename IB devices, > the command interface follows Iproute2 convention: > "rdma dev set [OLD-DEVNAME] name NEW-DEVNAME" > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com> > --- > rdma/dev.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/rdma/dev.c b/rdma/dev.c > index e2eafe47..760b7fb3 100644 > --- a/rdma/dev.c > +++ b/rdma/dev.c > @@ -14,6 +14,7 @@ > static int dev_help(struct rd *rd) > { > pr_out("Usage: %s dev show [DEV]\n", rd->filename); > + pr_out(" %s dev set [DEV] name DEVNAME\n", rd->filename); > return 0; > } > > @@ -240,17 +241,51 @@ static int dev_one_show(struct rd *rd) > return rd_exec_cmd(rd, cmds, "parameter"); > } > > +static int dev_set_name(struct rd *rd) > +{ > + uint32_t seq; > + > + if (rd_no_arg(rd)) { > + pr_err("Please provide device new name.\n"); > + return -EINVAL; > + } This is redundant with rd_exec_require_dev which is the required path to get to this point.
On Sun, Oct 21, 2018 at 10:02:43AM -0600, David Ahern wrote: > On 10/18/18 5:51 AM, Leon Romanovsky wrote: > > From: Leon Romanovsky <leonro@mellanox.com> > > > > Enrich rdmatool with an option to rename IB devices, > > the command interface follows Iproute2 convention: > > "rdma dev set [OLD-DEVNAME] name NEW-DEVNAME" > > > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com> > > --- > > rdma/dev.c | 35 +++++++++++++++++++++++++++++++++++ > > 1 file changed, 35 insertions(+) > > > > diff --git a/rdma/dev.c b/rdma/dev.c > > index e2eafe47..760b7fb3 100644 > > --- a/rdma/dev.c > > +++ b/rdma/dev.c > > @@ -14,6 +14,7 @@ > > static int dev_help(struct rd *rd) > > { > > pr_out("Usage: %s dev show [DEV]\n", rd->filename); > > + pr_out(" %s dev set [DEV] name DEVNAME\n", rd->filename); > > return 0; > > } > > > > @@ -240,17 +241,51 @@ static int dev_one_show(struct rd *rd) > > return rd_exec_cmd(rd, cmds, "parameter"); > > } > > > > +static int dev_set_name(struct rd *rd) > > +{ > > + uint32_t seq; > > + > > + if (rd_no_arg(rd)) { > > + pr_err("Please provide device new name.\n"); > > + return -EINVAL; > > + } > > This is redundant with rd_exec_require_dev which is the required path to > get to this point. Sorry for being slow in response, I was on vacation. ---------- This print is intended to warn about missing "new name" and it is not checked by rd_exec_require_dev() To emphasize it: diff --git a/rdma/dev.c b/rdma/dev.c index 760b7fb3..468ae686 100644 --- a/rdma/dev.c +++ b/rdma/dev.c @@ -246,6 +246,7 @@ static int dev_set_name(struct rd *rd) uint32_t seq; if (rd_no_arg(rd)) { + pr_err("hello world\n"); pr_err("Please provide device new name.\n"); return -EINVAL; } Produces the following output: [leonro@server iproute2]$ ./rdma/rdma dev set mlx5_0 name hello world Please provide device new name. So how will we progress from here? Should I respin? Thanks
On 10/29/18 4:30 AM, Leon Romanovsky wrote: > > Sorry for being slow in response, I was on vacation. > ---------- > > This print is intended to warn about missing "new name" and it is not > checked by rd_exec_require_dev() > > To emphasize it: > > diff --git a/rdma/dev.c b/rdma/dev.c > index 760b7fb3..468ae686 100644 > --- a/rdma/dev.c > +++ b/rdma/dev.c > @@ -246,6 +246,7 @@ static int dev_set_name(struct rd *rd) > uint32_t seq; > > if (rd_no_arg(rd)) { > + pr_err("hello world\n"); > pr_err("Please provide device new name.\n"); > return -EINVAL; > } > > Produces the following output: > [leonro@server iproute2]$ ./rdma/rdma dev set mlx5_0 name > hello world > Please provide device new name. > > So how will we progress from here? Should I respin? ok, thanks for the explanation. Steve mentioned an update to the man page is needed as well. Please do that in the respin. Thanks,
On Mon, Oct 29, 2018 at 11:07:06AM -0600, David Ahern wrote: > On 10/29/18 4:30 AM, Leon Romanovsky wrote: > > > > Sorry for being slow in response, I was on vacation. > > ---------- > > > > This print is intended to warn about missing "new name" and it is not > > checked by rd_exec_require_dev() > > > > To emphasize it: > > > > diff --git a/rdma/dev.c b/rdma/dev.c > > index 760b7fb3..468ae686 100644 > > --- a/rdma/dev.c > > +++ b/rdma/dev.c > > @@ -246,6 +246,7 @@ static int dev_set_name(struct rd *rd) > > uint32_t seq; > > > > if (rd_no_arg(rd)) { > > + pr_err("hello world\n"); > > pr_err("Please provide device new name.\n"); > > return -EINVAL; > > } > > > > Produces the following output: > > [leonro@server iproute2]$ ./rdma/rdma dev set mlx5_0 name > > hello world > > Please provide device new name. > > > > So how will we progress from here? Should I respin? > > ok, thanks for the explanation. > > Steve mentioned an update to the man page is needed as well. Please do > that in the respin. No problem, will do, thanks > > Thanks,
diff --git a/rdma/dev.c b/rdma/dev.c index e2eafe47..760b7fb3 100644 --- a/rdma/dev.c +++ b/rdma/dev.c @@ -14,6 +14,7 @@ static int dev_help(struct rd *rd) { pr_out("Usage: %s dev show [DEV]\n", rd->filename); + pr_out(" %s dev set [DEV] name DEVNAME\n", rd->filename); return 0; } @@ -240,17 +241,51 @@ static int dev_one_show(struct rd *rd) return rd_exec_cmd(rd, cmds, "parameter"); } +static int dev_set_name(struct rd *rd) +{ + uint32_t seq; + + if (rd_no_arg(rd)) { + pr_err("Please provide device new name.\n"); + return -EINVAL; + } + + rd_prepare_msg(rd, RDMA_NLDEV_CMD_SET, + &seq, (NLM_F_REQUEST | NLM_F_ACK)); + mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx); + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, rd_argv(rd)); + + return rd_send_msg(rd); +} + +static int dev_one_set(struct rd *rd) +{ + const struct rd_cmd cmds[] = { + { NULL, dev_help}, + { "name", dev_set_name}, + { 0 } + }; + + return rd_exec_cmd(rd, cmds, "parameter"); +} + static int dev_show(struct rd *rd) { return rd_exec_dev(rd, dev_one_show); } +static int dev_set(struct rd *rd) +{ + return rd_exec_require_dev(rd, dev_one_set); +} + int cmd_dev(struct rd *rd) { const struct rd_cmd cmds[] = { { NULL, dev_show }, { "show", dev_show }, { "list", dev_show }, + { "set", dev_set }, { "help", dev_help }, { 0 } };