* [PATCH RFC iproute2-next 0/2] Dynamic rdma link creation @ 2018-11-28 16:25 Steve Wise 2018-09-13 17:19 ` [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands Steve Wise 2018-11-28 15:16 ` [PATCH RFC iproute2-next 2/2] rdma: man page update for link add/delete Steve Wise 0 siblings, 2 replies; 17+ messages in thread From: Steve Wise @ 2018-11-28 16:25 UTC (permalink / raw) To: dsahern, leon; +Cc: stephen, netdev, linux-rdma, leon, BMT This series adds rdmatool support for creating/deleting rdma links. This will be used, mainly, by soft rdma drivers to allow adding/deleting rdma links over netdev interfaces. I've tagged this as "RFC" to start reviewing it, and because it is dependent on kernel changes which are still under review [1]. Please comment! Thanks, Steve. [1] https://www.spinics.net/lists/linux-rdma/msg71469.html Steve Wise (2): rdma: add 'link add/delete' commands rdma: man page update for link add/delete man/man8/rdma-link.8 | 51 +++++++++++++++++++++++++ rdma/link.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++ rdma/rdma.h | 1 + rdma/utils.c | 2 +- 4 files changed, 159 insertions(+), 1 deletion(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 16:25 [PATCH RFC iproute2-next 0/2] Dynamic rdma link creation Steve Wise @ 2018-09-13 17:19 ` Steve Wise 2018-11-28 18:26 ` Leon Romanovsky 2018-11-28 15:16 ` [PATCH RFC iproute2-next 2/2] rdma: man page update for link add/delete Steve Wise 1 sibling, 1 reply; 17+ messages in thread From: Steve Wise @ 2018-09-13 17:19 UTC (permalink / raw) To: dsahern, leon; +Cc: stephen, netdev, linux-rdma, leon, BMT Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma device to a netdev interface. EG: rdma link add rxe_eth0 type rxe dev eth0 rdma link delete rxe_eth0 Signed-off-by: Steve Wise <swise@opengridcomputing.com> --- rdma/link.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ rdma/rdma.h | 1 + rdma/utils.c | 2 +- 3 files changed, 108 insertions(+), 1 deletion(-) diff --git a/rdma/link.c b/rdma/link.c index 7a6d4b7e356d..d4f76b0ce11f 100644 --- a/rdma/link.c +++ b/rdma/link.c @@ -14,6 +14,8 @@ static int link_help(struct rd *rd) { pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename); + pr_out("Usage: %s link add NAME type TYPE dev DEV\n", rd->filename); + pr_out("Usage: %s link delete NAME type TYPE\n", rd->filename); return 0; } @@ -315,10 +317,114 @@ static int link_show(struct rd *rd) return rd_exec_link(rd, link_one_show, true); } +static int link_add_parse_cb(const struct nlmsghdr *nlh, void *data) +{ + return MNL_CB_OK; +} + +static int link_add(struct rd *rd) +{ + char *name; + char *type = NULL; + char *dev = NULL; + uint32_t seq; + int ret; + + if (rd_no_arg(rd)) { + pr_err("No link name was supplied\n"); + return -EINVAL; + } + name = rd_argv(rd); + rd_arg_inc(rd); + while (!rd_no_arg(rd)) { + if (rd_argv_match(rd, "type")) { + rd_arg_inc(rd); + type = rd_argv(rd); + } else if (rd_argv_match(rd, "dev")) { + rd_arg_inc(rd); + dev = rd_argv(rd); + } else { + pr_err("Invalid parameter %s\n", rd_argv(rd)); + return -EINVAL; + } + rd_arg_inc(rd); + } + if (!type) { + pr_err("No type was supplied\n"); + return -EINVAL; + } + if (!dev) { + pr_err("No net device was supplied\n"); + return -EINVAL; + } + + rd_prepare_msg(rd, RDMA_NLDEV_CMD_NEWLINK, &seq, + (NLM_F_REQUEST | NLM_F_ACK)); + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, name); + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_LINK_TYPE, type); + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_NDEV_NAME, dev); + ret = rd_send_msg(rd); + if (ret) + return ret; + + ret = rd_recv_msg(rd, link_add_parse_cb, rd, seq); + if (ret) + perror(NULL); + return ret; +} + +static int link_del_parse_cb(const struct nlmsghdr *nlh, void *data) +{ + return MNL_CB_OK; +} + +static int link_del(struct rd *rd) +{ + char *name; + char *type = NULL; + uint32_t seq; + int ret; + + if (rd_no_arg(rd)) { + pr_err("No link type was supplied\n"); + return -EINVAL; + } + name = rd_argv(rd); + rd_arg_inc(rd); + while (!rd_no_arg(rd)) { + if (rd_argv_match(rd, "type")) { + rd_arg_inc(rd); + type = rd_argv(rd); + } else { + pr_err("Invalid parameter %s\n", rd_argv(rd)); + return -EINVAL; + } + rd_arg_inc(rd); + } + if (!type) { + pr_err("No type was supplied\n"); + return -EINVAL; + } + rd_prepare_msg(rd, RDMA_NLDEV_CMD_DELLINK, &seq, + (NLM_F_REQUEST | NLM_F_ACK)); + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, name); + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_LINK_TYPE, type); + ret = rd_send_msg(rd); + if (ret) + return ret; + + ret = rd_recv_msg(rd, link_del_parse_cb, rd, seq); + if (ret) + perror(NULL); + return ret; +} + int cmd_link(struct rd *rd) { const struct rd_cmd cmds[] = { { NULL, link_show }, + { "add", link_add }, + { "delete", link_del }, { "show", link_show }, { "list", link_show }, { "help", link_help }, diff --git a/rdma/rdma.h b/rdma/rdma.h index 547bb5749a39..b5271e423c10 100644 --- a/rdma/rdma.h +++ b/rdma/rdma.h @@ -79,6 +79,7 @@ struct rd_cmd { */ bool rd_no_arg(struct rd *rd); void rd_arg_inc(struct rd *rd); +bool rd_argv_match(struct rd *rd, const char *pattern); char *rd_argv(struct rd *rd); diff --git a/rdma/utils.c b/rdma/utils.c index 61f4aeb1bcf2..41f8f7391279 100644 --- a/rdma/utils.c +++ b/rdma/utils.c @@ -32,7 +32,7 @@ int strcmpx(const char *str1, const char *str2) return strncmp(str1, str2, strlen(str1)); } -static bool rd_argv_match(struct rd *rd, const char *pattern) +bool rd_argv_match(struct rd *rd, const char *pattern) { if (!rd_argc(rd)) return false; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-09-13 17:19 ` [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands Steve Wise @ 2018-11-28 18:26 ` Leon Romanovsky 2018-11-28 19:08 ` Steve Wise 0 siblings, 1 reply; 17+ messages in thread From: Leon Romanovsky @ 2018-11-28 18:26 UTC (permalink / raw) To: Steve Wise; +Cc: dsahern, stephen, netdev, linux-rdma, BMT [-- Attachment #1: Type: text/plain, Size: 4827 bytes --] On Thu, Sep 13, 2018 at 10:19:21AM -0700, Steve Wise wrote: > Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma > device to a netdev interface. > > EG: > > rdma link add rxe_eth0 type rxe dev eth0 > rdma link delete rxe_eth0 > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > --- > rdma/link.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > rdma/rdma.h | 1 + > rdma/utils.c | 2 +- > 3 files changed, 108 insertions(+), 1 deletion(-) > > diff --git a/rdma/link.c b/rdma/link.c > index 7a6d4b7e356d..d4f76b0ce11f 100644 > --- a/rdma/link.c > +++ b/rdma/link.c > @@ -14,6 +14,8 @@ > static int link_help(struct rd *rd) > { > pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename); > + pr_out("Usage: %s link add NAME type TYPE dev DEV\n", rd->filename); I suggest to rename "dev" to be "netdev", because we are using "dev" for ib devices. > + pr_out("Usage: %s link delete NAME type TYPE\n", rd->filename); Why do you need "type" for "delete" command? > return 0; > } > > @@ -315,10 +317,114 @@ static int link_show(struct rd *rd) > return rd_exec_link(rd, link_one_show, true); > } > > +static int link_add_parse_cb(const struct nlmsghdr *nlh, void *data) > +{ > + return MNL_CB_OK; > +} > + > +static int link_add(struct rd *rd) > +{ > + char *name; > + char *type = NULL; > + char *dev = NULL; > + uint32_t seq; > + int ret; > + > + if (rd_no_arg(rd)) { > + pr_err("No link name was supplied\n"); > + return -EINVAL; > + } > + name = rd_argv(rd); > + rd_arg_inc(rd); > + while (!rd_no_arg(rd)) { > + if (rd_argv_match(rd, "type")) { > + rd_arg_inc(rd); > + type = rd_argv(rd); > + } else if (rd_argv_match(rd, "dev")) { > + rd_arg_inc(rd); > + dev = rd_argv(rd); > + } else { > + pr_err("Invalid parameter %s\n", rd_argv(rd)); > + return -EINVAL; > + } > + rd_arg_inc(rd); > + } > + if (!type) { > + pr_err("No type was supplied\n"); > + return -EINVAL; > + } > + if (!dev) { > + pr_err("No net device was supplied\n"); > + return -EINVAL; > + } > + > + rd_prepare_msg(rd, RDMA_NLDEV_CMD_NEWLINK, &seq, > + (NLM_F_REQUEST | NLM_F_ACK)); > + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, name); > + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_LINK_TYPE, type); > + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_NDEV_NAME, dev); > + ret = rd_send_msg(rd); > + if (ret) > + return ret; > + > + ret = rd_recv_msg(rd, link_add_parse_cb, rd, seq); > + if (ret) > + perror(NULL); Why do you need rd_recv_msg()? I think that it is not needed, at least for rename, I didn't need it. https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/tree/rdma/dev.c#n244 > + return ret; > +} > + > +static int link_del_parse_cb(const struct nlmsghdr *nlh, void *data) > +{ > + return MNL_CB_OK; > +} > + > +static int link_del(struct rd *rd) > +{ > + char *name; > + char *type = NULL; > + uint32_t seq; > + int ret; > + > + if (rd_no_arg(rd)) { > + pr_err("No link type was supplied\n"); > + return -EINVAL; > + } > + name = rd_argv(rd); > + rd_arg_inc(rd); > + while (!rd_no_arg(rd)) { > + if (rd_argv_match(rd, "type")) { > + rd_arg_inc(rd); > + type = rd_argv(rd); > + } else { > + pr_err("Invalid parameter %s\n", rd_argv(rd)); > + return -EINVAL; > + } > + rd_arg_inc(rd); > + } > + if (!type) { > + pr_err("No type was supplied\n"); > + return -EINVAL; > + } > + rd_prepare_msg(rd, RDMA_NLDEV_CMD_DELLINK, &seq, > + (NLM_F_REQUEST | NLM_F_ACK)); > + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, name); > + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_LINK_TYPE, type); > + ret = rd_send_msg(rd); > + if (ret) > + return ret; > + > + ret = rd_recv_msg(rd, link_del_parse_cb, rd, seq); > + if (ret) > + perror(NULL); > + return ret; The same question as above. > +} > + > int cmd_link(struct rd *rd) > { > const struct rd_cmd cmds[] = { > { NULL, link_show }, > + { "add", link_add }, > + { "delete", link_del }, > { "show", link_show }, > { "list", link_show }, > { "help", link_help }, > diff --git a/rdma/rdma.h b/rdma/rdma.h > index 547bb5749a39..b5271e423c10 100644 > --- a/rdma/rdma.h > +++ b/rdma/rdma.h > @@ -79,6 +79,7 @@ struct rd_cmd { > */ > bool rd_no_arg(struct rd *rd); > void rd_arg_inc(struct rd *rd); > +bool rd_argv_match(struct rd *rd, const char *pattern); > > char *rd_argv(struct rd *rd); > > diff --git a/rdma/utils.c b/rdma/utils.c > index 61f4aeb1bcf2..41f8f7391279 100644 > --- a/rdma/utils.c > +++ b/rdma/utils.c > @@ -32,7 +32,7 @@ int strcmpx(const char *str1, const char *str2) > return strncmp(str1, str2, strlen(str1)); > } > > -static bool rd_argv_match(struct rd *rd, const char *pattern) > +bool rd_argv_match(struct rd *rd, const char *pattern) > { > if (!rd_argc(rd)) > return false; > -- > 1.8.3.1 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 18:26 ` Leon Romanovsky @ 2018-11-28 19:08 ` Steve Wise 2018-11-28 19:34 ` Steve Wise 2018-11-28 20:04 ` Leon Romanovsky 0 siblings, 2 replies; 17+ messages in thread From: Steve Wise @ 2018-11-28 19:08 UTC (permalink / raw) To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma, BMT On 11/28/2018 12:26 PM, Leon Romanovsky wrote: > On Thu, Sep 13, 2018 at 10:19:21AM -0700, Steve Wise wrote: >> Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma >> device to a netdev interface. >> >> EG: >> >> rdma link add rxe_eth0 type rxe dev eth0 >> rdma link delete rxe_eth0 >> >> Signed-off-by: Steve Wise <swise@opengridcomputing.com> >> --- >> rdma/link.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> rdma/rdma.h | 1 + >> rdma/utils.c | 2 +- >> 3 files changed, 108 insertions(+), 1 deletion(-) >> >> diff --git a/rdma/link.c b/rdma/link.c >> index 7a6d4b7e356d..d4f76b0ce11f 100644 >> --- a/rdma/link.c >> +++ b/rdma/link.c >> @@ -14,6 +14,8 @@ >> static int link_help(struct rd *rd) >> { >> pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename); >> + pr_out("Usage: %s link add NAME type TYPE dev DEV\n", rd->filename); > I suggest to rename "dev" to be "netdev", because we are using "dev" for > ib devices. Yea ok. >> + pr_out("Usage: %s link delete NAME type TYPE\n", rd->filename); > Why do you need "type" for "delete" command? Because the type is used in the kernel to find the appropriate link ops. I could change the kernel side to search all types for the device name to delete? >> return 0; >> } >> >> @@ -315,10 +317,114 @@ static int link_show(struct rd *rd) >> return rd_exec_link(rd, link_one_show, true); >> } >> >> +static int link_add_parse_cb(const struct nlmsghdr *nlh, void *data) >> +{ >> + return MNL_CB_OK; >> +} >> + >> +static int link_add(struct rd *rd) >> +{ >> + char *name; >> + char *type = NULL; >> + char *dev = NULL; >> + uint32_t seq; >> + int ret; >> + >> + if (rd_no_arg(rd)) { >> + pr_err("No link name was supplied\n"); >> + return -EINVAL; >> + } >> + name = rd_argv(rd); >> + rd_arg_inc(rd); >> + while (!rd_no_arg(rd)) { >> + if (rd_argv_match(rd, "type")) { >> + rd_arg_inc(rd); >> + type = rd_argv(rd); >> + } else if (rd_argv_match(rd, "dev")) { >> + rd_arg_inc(rd); >> + dev = rd_argv(rd); >> + } else { >> + pr_err("Invalid parameter %s\n", rd_argv(rd)); >> + return -EINVAL; >> + } >> + rd_arg_inc(rd); >> + } >> + if (!type) { >> + pr_err("No type was supplied\n"); >> + return -EINVAL; >> + } >> + if (!dev) { >> + pr_err("No net device was supplied\n"); >> + return -EINVAL; >> + } >> + >> + rd_prepare_msg(rd, RDMA_NLDEV_CMD_NEWLINK, &seq, >> + (NLM_F_REQUEST | NLM_F_ACK)); >> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, name); >> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_LINK_TYPE, type); >> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_NDEV_NAME, dev); >> + ret = rd_send_msg(rd); >> + if (ret) >> + return ret; >> + >> + ret = rd_recv_msg(rd, link_add_parse_cb, rd, seq); >> + if (ret) >> + perror(NULL); > Why do you need rd_recv_msg()? I think that it is not needed, at least > for rename, I didn't need it. > https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/tree/rdma/dev.c#n244 To get the response of if it was successfully added. It provides the errno value. >> + return ret; >> +} >> + >> +static int link_del_parse_cb(const struct nlmsghdr *nlh, void *data) >> +{ >> + return MNL_CB_OK; >> +} >> + >> +static int link_del(struct rd *rd) >> +{ >> + char *name; >> + char *type = NULL; >> + uint32_t seq; >> + int ret; >> + >> + if (rd_no_arg(rd)) { >> + pr_err("No link type was supplied\n"); >> + return -EINVAL; >> + } >> + name = rd_argv(rd); >> + rd_arg_inc(rd); >> + while (!rd_no_arg(rd)) { >> + if (rd_argv_match(rd, "type")) { >> + rd_arg_inc(rd); >> + type = rd_argv(rd); >> + } else { >> + pr_err("Invalid parameter %s\n", rd_argv(rd)); >> + return -EINVAL; >> + } >> + rd_arg_inc(rd); >> + } >> + if (!type) { >> + pr_err("No type was supplied\n"); >> + return -EINVAL; >> + } >> + rd_prepare_msg(rd, RDMA_NLDEV_CMD_DELLINK, &seq, >> + (NLM_F_REQUEST | NLM_F_ACK)); >> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, name); >> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_LINK_TYPE, type); >> + ret = rd_send_msg(rd); >> + if (ret) >> + return ret; >> + >> + ret = rd_recv_msg(rd, link_del_parse_cb, rd, seq); >> + if (ret) >> + perror(NULL); >> + return ret; > The same question as above. > >> +} >> + >> int cmd_link(struct rd *rd) >> { >> const struct rd_cmd cmds[] = { >> { NULL, link_show }, >> + { "add", link_add }, >> + { "delete", link_del }, >> { "show", link_show }, >> { "list", link_show }, >> { "help", link_help }, >> diff --git a/rdma/rdma.h b/rdma/rdma.h >> index 547bb5749a39..b5271e423c10 100644 >> --- a/rdma/rdma.h >> +++ b/rdma/rdma.h >> @@ -79,6 +79,7 @@ struct rd_cmd { >> */ >> bool rd_no_arg(struct rd *rd); >> void rd_arg_inc(struct rd *rd); >> +bool rd_argv_match(struct rd *rd, const char *pattern); >> >> char *rd_argv(struct rd *rd); >> >> diff --git a/rdma/utils.c b/rdma/utils.c >> index 61f4aeb1bcf2..41f8f7391279 100644 >> --- a/rdma/utils.c >> +++ b/rdma/utils.c >> @@ -32,7 +32,7 @@ int strcmpx(const char *str1, const char *str2) >> return strncmp(str1, str2, strlen(str1)); >> } >> >> -static bool rd_argv_match(struct rd *rd, const char *pattern) >> +bool rd_argv_match(struct rd *rd, const char *pattern) >> { >> if (!rd_argc(rd)) >> return false; >> -- >> 1.8.3.1 >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 19:08 ` Steve Wise @ 2018-11-28 19:34 ` Steve Wise 2018-11-28 20:02 ` Leon Romanovsky 2018-11-28 20:04 ` Leon Romanovsky 1 sibling, 1 reply; 17+ messages in thread From: Steve Wise @ 2018-11-28 19:34 UTC (permalink / raw) To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma, BMT ... >>> + rd_prepare_msg(rd, RDMA_NLDEV_CMD_NEWLINK, &seq, >>> + (NLM_F_REQUEST | NLM_F_ACK)); >>> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, name); >>> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_LINK_TYPE, type); >>> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_NDEV_NAME, dev); >>> + ret = rd_send_msg(rd); >>> + if (ret) >>> + return ret; >>> + >>> + ret = rd_recv_msg(rd, link_add_parse_cb, rd, seq); >>> + if (ret) >>> + perror(NULL); >> Why do you need rd_recv_msg()? I think that it is not needed, at least >> for rename, I didn't need it. >> https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/tree/rdma/dev.c#n244 > To get the response of if it was successfully added. It provides the > errno value. If I don't do the rd_recv_msg, then adding the same name twice fails without any error notification. Ditto for deleting a non-existent link. So the rd_recv_msg() allows getting the failure reason (and detecting the failure). ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 19:34 ` Steve Wise @ 2018-11-28 20:02 ` Leon Romanovsky 2018-11-28 20:08 ` Leon Romanovsky 0 siblings, 1 reply; 17+ messages in thread From: Leon Romanovsky @ 2018-11-28 20:02 UTC (permalink / raw) To: Steve Wise; +Cc: dsahern, stephen, netdev, linux-rdma, BMT [-- Attachment #1: Type: text/plain, Size: 1263 bytes --] On Wed, Nov 28, 2018 at 01:34:14PM -0600, Steve Wise wrote: > ... > > >>> + rd_prepare_msg(rd, RDMA_NLDEV_CMD_NEWLINK, &seq, > >>> + (NLM_F_REQUEST | NLM_F_ACK)); > >>> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, name); > >>> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_LINK_TYPE, type); > >>> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_NDEV_NAME, dev); > >>> + ret = rd_send_msg(rd); > >>> + if (ret) > >>> + return ret; > >>> + > >>> + ret = rd_recv_msg(rd, link_add_parse_cb, rd, seq); > >>> + if (ret) > >>> + perror(NULL); > >> Why do you need rd_recv_msg()? I think that it is not needed, at least > >> for rename, I didn't need it. > >> https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/tree/rdma/dev.c#n244 > > To get the response of if it was successfully added. It provides the > > errno value. > If I don't do the rd_recv_msg, then adding the same name twice fails > without any error notification. Ditto for deleting a non-existent > link. So the rd_recv_msg() allows getting the failure reason (and > detecting the failure). > Shouldn't extack provide such information as part of NLM_F_ACK flag? just shooting into the air, will take more close look tomorrow. Thanks > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 20:02 ` Leon Romanovsky @ 2018-11-28 20:08 ` Leon Romanovsky 2018-11-28 20:23 ` Steve Wise 0 siblings, 1 reply; 17+ messages in thread From: Leon Romanovsky @ 2018-11-28 20:08 UTC (permalink / raw) To: Steve Wise; +Cc: dsahern, stephen, netdev, linux-rdma, BMT [-- Attachment #1: Type: text/plain, Size: 1644 bytes --] On Wed, Nov 28, 2018 at 10:02:04PM +0200, Leon Romanovsky wrote: > On Wed, Nov 28, 2018 at 01:34:14PM -0600, Steve Wise wrote: > > ... > > > > >>> + rd_prepare_msg(rd, RDMA_NLDEV_CMD_NEWLINK, &seq, > > >>> + (NLM_F_REQUEST | NLM_F_ACK)); > > >>> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, name); > > >>> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_LINK_TYPE, type); > > >>> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_NDEV_NAME, dev); > > >>> + ret = rd_send_msg(rd); > > >>> + if (ret) > > >>> + return ret; > > >>> + > > >>> + ret = rd_recv_msg(rd, link_add_parse_cb, rd, seq); > > >>> + if (ret) > > >>> + perror(NULL); > > >> Why do you need rd_recv_msg()? I think that it is not needed, at least > > >> for rename, I didn't need it. > > >> https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/tree/rdma/dev.c#n244 > > > To get the response of if it was successfully added. It provides the > > > errno value. > > If I don't do the rd_recv_msg, then adding the same name twice fails > > without any error notification. Ditto for deleting a non-existent > > link. So the rd_recv_msg() allows getting the failure reason (and > > detecting the failure). > > > > Shouldn't extack provide such information as part of NLM_F_ACK flag? > > just shooting into the air, will take more close look tomorrow. OK, it was easier than I thought. You are right, need both send and receive to get the reason. Can you prepare general function and update rename part too? Something like send_receive(...) with dummy callback for receive path. Thanks > > Thanks > > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 20:08 ` Leon Romanovsky @ 2018-11-28 20:23 ` Steve Wise 0 siblings, 0 replies; 17+ messages in thread From: Steve Wise @ 2018-11-28 20:23 UTC (permalink / raw) To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma, BMT On 11/28/2018 2:08 PM, Leon Romanovsky wrote: > On Wed, Nov 28, 2018 at 10:02:04PM +0200, Leon Romanovsky wrote: >> On Wed, Nov 28, 2018 at 01:34:14PM -0600, Steve Wise wrote: >>> ... >>> >>>>>> + rd_prepare_msg(rd, RDMA_NLDEV_CMD_NEWLINK, &seq, >>>>>> + (NLM_F_REQUEST | NLM_F_ACK)); >>>>>> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, name); >>>>>> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_LINK_TYPE, type); >>>>>> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_NDEV_NAME, dev); >>>>>> + ret = rd_send_msg(rd); >>>>>> + if (ret) >>>>>> + return ret; >>>>>> + >>>>>> + ret = rd_recv_msg(rd, link_add_parse_cb, rd, seq); >>>>>> + if (ret) >>>>>> + perror(NULL); >>>>> Why do you need rd_recv_msg()? I think that it is not needed, at least >>>>> for rename, I didn't need it. >>>>> https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/tree/rdma/dev.c#n244 >>>> To get the response of if it was successfully added. It provides the >>>> errno value. >>> If I don't do the rd_recv_msg, then adding the same name twice fails >>> without any error notification. Ditto for deleting a non-existent >>> link. So the rd_recv_msg() allows getting the failure reason (and >>> detecting the failure). >>> >> Shouldn't extack provide such information as part of NLM_F_ACK flag? >> >> just shooting into the air, will take more close look tomorrow. > OK, it was easier than I thought. > > You are right, need both send and receive to get the reason. > > Can you prepare general function and update rename part too? > Something like send_receive(...) with dummy callback for receive path. > > Thanks Sure, I'll whip something up for the next version of the patch series... ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 19:08 ` Steve Wise 2018-11-28 19:34 ` Steve Wise @ 2018-11-28 20:04 ` Leon Romanovsky 2018-11-28 20:07 ` Steve Wise 1 sibling, 1 reply; 17+ messages in thread From: Leon Romanovsky @ 2018-11-28 20:04 UTC (permalink / raw) To: Steve Wise; +Cc: dsahern, stephen, netdev, linux-rdma, BMT [-- Attachment #1: Type: text/plain, Size: 1524 bytes --] On Wed, Nov 28, 2018 at 01:08:05PM -0600, Steve Wise wrote: > > > On 11/28/2018 12:26 PM, Leon Romanovsky wrote: > > On Thu, Sep 13, 2018 at 10:19:21AM -0700, Steve Wise wrote: > >> Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma > >> device to a netdev interface. > >> > >> EG: > >> > >> rdma link add rxe_eth0 type rxe dev eth0 > >> rdma link delete rxe_eth0 > >> > >> Signed-off-by: Steve Wise <swise@opengridcomputing.com> > >> --- > >> rdma/link.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >> rdma/rdma.h | 1 + > >> rdma/utils.c | 2 +- > >> 3 files changed, 108 insertions(+), 1 deletion(-) > >> > >> diff --git a/rdma/link.c b/rdma/link.c > >> index 7a6d4b7e356d..d4f76b0ce11f 100644 > >> --- a/rdma/link.c > >> +++ b/rdma/link.c > >> @@ -14,6 +14,8 @@ > >> static int link_help(struct rd *rd) > >> { > >> pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename); > >> + pr_out("Usage: %s link add NAME type TYPE dev DEV\n", rd->filename); > > I suggest to rename "dev" to be "netdev", because we are using "dev" for > > ib devices. > > Yea ok. > > >> + pr_out("Usage: %s link delete NAME type TYPE\n", rd->filename); > > Why do you need "type" for "delete" command? > > Because the type is used in the kernel to find the appropriate link > ops. I could change the kernel side to search all types for the device > name to delete? I would say, yes. It makes "delete" operation more natural. Thanks [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 20:04 ` Leon Romanovsky @ 2018-11-28 20:07 ` Steve Wise 2018-11-28 20:13 ` Leon Romanovsky 0 siblings, 1 reply; 17+ messages in thread From: Steve Wise @ 2018-11-28 20:07 UTC (permalink / raw) To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma, BMT On 11/28/2018 2:04 PM, Leon Romanovsky wrote: > On Wed, Nov 28, 2018 at 01:08:05PM -0600, Steve Wise wrote: >> >> On 11/28/2018 12:26 PM, Leon Romanovsky wrote: >>> On Thu, Sep 13, 2018 at 10:19:21AM -0700, Steve Wise wrote: >>>> Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma >>>> device to a netdev interface. >>>> >>>> EG: >>>> >>>> rdma link add rxe_eth0 type rxe dev eth0 >>>> rdma link delete rxe_eth0 >>>> >>>> Signed-off-by: Steve Wise <swise@opengridcomputing.com> >>>> --- >>>> rdma/link.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> rdma/rdma.h | 1 + >>>> rdma/utils.c | 2 +- >>>> 3 files changed, 108 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/rdma/link.c b/rdma/link.c >>>> index 7a6d4b7e356d..d4f76b0ce11f 100644 >>>> --- a/rdma/link.c >>>> +++ b/rdma/link.c >>>> @@ -14,6 +14,8 @@ >>>> static int link_help(struct rd *rd) >>>> { >>>> pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename); >>>> + pr_out("Usage: %s link add NAME type TYPE dev DEV\n", rd->filename); >>> I suggest to rename "dev" to be "netdev", because we are using "dev" for >>> ib devices. >> Yea ok. >> >>>> + pr_out("Usage: %s link delete NAME type TYPE\n", rd->filename); >>> Why do you need "type" for "delete" command? >> Because the type is used in the kernel to find the appropriate link >> ops. I could change the kernel side to search all types for the device >> name to delete? > I would say, yes. > It makes "delete" operation more natural. > > Thanks Perhaps. Note: 'ip link delete' takes a type as well... ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 20:07 ` Steve Wise @ 2018-11-28 20:13 ` Leon Romanovsky 2018-11-28 20:18 ` Steve Wise 0 siblings, 1 reply; 17+ messages in thread From: Leon Romanovsky @ 2018-11-28 20:13 UTC (permalink / raw) To: Steve Wise; +Cc: dsahern, stephen, netdev, linux-rdma, BMT [-- Attachment #1: Type: text/plain, Size: 2002 bytes --] On Wed, Nov 28, 2018 at 02:07:29PM -0600, Steve Wise wrote: > > > On 11/28/2018 2:04 PM, Leon Romanovsky wrote: > > On Wed, Nov 28, 2018 at 01:08:05PM -0600, Steve Wise wrote: > >> > >> On 11/28/2018 12:26 PM, Leon Romanovsky wrote: > >>> On Thu, Sep 13, 2018 at 10:19:21AM -0700, Steve Wise wrote: > >>>> Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma > >>>> device to a netdev interface. > >>>> > >>>> EG: > >>>> > >>>> rdma link add rxe_eth0 type rxe dev eth0 > >>>> rdma link delete rxe_eth0 > >>>> > >>>> Signed-off-by: Steve Wise <swise@opengridcomputing.com> > >>>> --- > >>>> rdma/link.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >>>> rdma/rdma.h | 1 + > >>>> rdma/utils.c | 2 +- > >>>> 3 files changed, 108 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/rdma/link.c b/rdma/link.c > >>>> index 7a6d4b7e356d..d4f76b0ce11f 100644 > >>>> --- a/rdma/link.c > >>>> +++ b/rdma/link.c > >>>> @@ -14,6 +14,8 @@ > >>>> static int link_help(struct rd *rd) > >>>> { > >>>> pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename); > >>>> + pr_out("Usage: %s link add NAME type TYPE dev DEV\n", rd->filename); > >>> I suggest to rename "dev" to be "netdev", because we are using "dev" for > >>> ib devices. > >> Yea ok. > >> > >>>> + pr_out("Usage: %s link delete NAME type TYPE\n", rd->filename); > >>> Why do you need "type" for "delete" command? > >> Because the type is used in the kernel to find the appropriate link > >> ops. I could change the kernel side to search all types for the device > >> name to delete? > > I would say, yes. > > It makes "delete" operation more natural. > > > > Thanks > > Perhaps. > > Note: 'ip link delete' takes a type as well... According to man section, yes. According to various guides, no. https://docs.fedoraproject.org/en-US/Fedora/20/html/Networking_Guide/sec-Configure_802_1Q_VLAN_Tagging_ip_Commands.html Thanks > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 20:13 ` Leon Romanovsky @ 2018-11-28 20:18 ` Steve Wise 2018-11-28 22:17 ` Jason Gunthorpe 0 siblings, 1 reply; 17+ messages in thread From: Steve Wise @ 2018-11-28 20:18 UTC (permalink / raw) To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma, BMT On 11/28/2018 2:13 PM, Leon Romanovsky wrote: > On Wed, Nov 28, 2018 at 02:07:29PM -0600, Steve Wise wrote: >> >> On 11/28/2018 2:04 PM, Leon Romanovsky wrote: >>> On Wed, Nov 28, 2018 at 01:08:05PM -0600, Steve Wise wrote: >>>> On 11/28/2018 12:26 PM, Leon Romanovsky wrote: >>>>> On Thu, Sep 13, 2018 at 10:19:21AM -0700, Steve Wise wrote: >>>>>> Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma >>>>>> device to a netdev interface. >>>>>> >>>>>> EG: >>>>>> >>>>>> rdma link add rxe_eth0 type rxe dev eth0 >>>>>> rdma link delete rxe_eth0 >>>>>> >>>>>> Signed-off-by: Steve Wise <swise@opengridcomputing.com> >>>>>> --- >>>>>> rdma/link.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>> rdma/rdma.h | 1 + >>>>>> rdma/utils.c | 2 +- >>>>>> 3 files changed, 108 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/rdma/link.c b/rdma/link.c >>>>>> index 7a6d4b7e356d..d4f76b0ce11f 100644 >>>>>> --- a/rdma/link.c >>>>>> +++ b/rdma/link.c >>>>>> @@ -14,6 +14,8 @@ >>>>>> static int link_help(struct rd *rd) >>>>>> { >>>>>> pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename); >>>>>> + pr_out("Usage: %s link add NAME type TYPE dev DEV\n", rd->filename); >>>>> I suggest to rename "dev" to be "netdev", because we are using "dev" for >>>>> ib devices. >>>> Yea ok. >>>> >>>>>> + pr_out("Usage: %s link delete NAME type TYPE\n", rd->filename); >>>>> Why do you need "type" for "delete" command? >>>> Because the type is used in the kernel to find the appropriate link >>>> ops. I could change the kernel side to search all types for the device >>>> name to delete? >>> I would say, yes. >>> It makes "delete" operation more natural. >>> >>> Thanks >> Perhaps. >> >> Note: 'ip link delete' takes a type as well... > According to man section, yes. > According to various guides, no. > https://docs.fedoraproject.org/en-US/Fedora/20/html/Networking_Guide/sec-Configure_802_1Q_VLAN_Tagging_ip_Commands.html > > Thanks It does make sense to not require type. The name must be unique so that should be enough. I'll have to respin the kernel side though... Thanks, Steve. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 20:18 ` Steve Wise @ 2018-11-28 22:17 ` Jason Gunthorpe 2018-11-28 22:21 ` Steve Wise 0 siblings, 1 reply; 17+ messages in thread From: Jason Gunthorpe @ 2018-11-28 22:17 UTC (permalink / raw) To: Steve Wise; +Cc: Leon Romanovsky, dsahern, stephen, netdev, linux-rdma, BMT On Wed, Nov 28, 2018 at 02:18:55PM -0600, Steve Wise wrote: > > > On 11/28/2018 2:13 PM, Leon Romanovsky wrote: > > On Wed, Nov 28, 2018 at 02:07:29PM -0600, Steve Wise wrote: > >> > >> On 11/28/2018 2:04 PM, Leon Romanovsky wrote: > >>> On Wed, Nov 28, 2018 at 01:08:05PM -0600, Steve Wise wrote: > >>>> On 11/28/2018 12:26 PM, Leon Romanovsky wrote: > >>>>> On Thu, Sep 13, 2018 at 10:19:21AM -0700, Steve Wise wrote: > >>>>>> Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma > >>>>>> device to a netdev interface. > >>>>>> > >>>>>> EG: > >>>>>> > >>>>>> rdma link add rxe_eth0 type rxe dev eth0 > >>>>>> rdma link delete rxe_eth0 > >>>>>> > >>>>>> Signed-off-by: Steve Wise <swise@opengridcomputing.com> > >>>>>> rdma/link.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >>>>>> rdma/rdma.h | 1 + > >>>>>> rdma/utils.c | 2 +- > >>>>>> 3 files changed, 108 insertions(+), 1 deletion(-) > >>>>>> > >>>>>> diff --git a/rdma/link.c b/rdma/link.c > >>>>>> index 7a6d4b7e356d..d4f76b0ce11f 100644 > >>>>>> +++ b/rdma/link.c > >>>>>> @@ -14,6 +14,8 @@ > >>>>>> static int link_help(struct rd *rd) > >>>>>> { > >>>>>> pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename); > >>>>>> + pr_out("Usage: %s link add NAME type TYPE dev DEV\n", rd->filename); > >>>>> I suggest to rename "dev" to be "netdev", because we are using "dev" for > >>>>> ib devices. > >>>> Yea ok. > >>>> > >>>>>> + pr_out("Usage: %s link delete NAME type TYPE\n", rd->filename); > >>>>> Why do you need "type" for "delete" command? > >>>> Because the type is used in the kernel to find the appropriate link > >>>> ops. I could change the kernel side to search all types for the device > >>>> name to delete? > >>> I would say, yes. > >>> It makes "delete" operation more natural. > >>> > >>> Thanks > >> Perhaps. > >> > >> Note: 'ip link delete' takes a type as well... > > According to man section, yes. > > According to various guides, no. > > https://docs.fedoraproject.org/en-US/Fedora/20/html/Networking_Guide/sec-Configure_802_1Q_VLAN_Tagging_ip_Commands.html > > > > Thanks > > It does make sense to not require type. The name must be unique so that > should be enough. I'll have to respin the kernel side though... The delete_link really should be an operation on the ib_device, not the link_ops thing. That directly prevents mis-matching function callbacks.. Jason ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 22:17 ` Jason Gunthorpe @ 2018-11-28 22:21 ` Steve Wise 2018-11-28 22:25 ` Jason Gunthorpe 0 siblings, 1 reply; 17+ messages in thread From: Steve Wise @ 2018-11-28 22:21 UTC (permalink / raw) To: Jason Gunthorpe Cc: Leon Romanovsky, dsahern, stephen, netdev, linux-rdma, BMT On 11/28/2018 4:17 PM, Jason Gunthorpe wrote: > On Wed, Nov 28, 2018 at 02:18:55PM -0600, Steve Wise wrote: >> >> On 11/28/2018 2:13 PM, Leon Romanovsky wrote: >>> On Wed, Nov 28, 2018 at 02:07:29PM -0600, Steve Wise wrote: >>>> On 11/28/2018 2:04 PM, Leon Romanovsky wrote: >>>>> On Wed, Nov 28, 2018 at 01:08:05PM -0600, Steve Wise wrote: >>>>>> On 11/28/2018 12:26 PM, Leon Romanovsky wrote: >>>>>>> On Thu, Sep 13, 2018 at 10:19:21AM -0700, Steve Wise wrote: >>>>>>>> Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma >>>>>>>> device to a netdev interface. >>>>>>>> >>>>>>>> EG: >>>>>>>> >>>>>>>> rdma link add rxe_eth0 type rxe dev eth0 >>>>>>>> rdma link delete rxe_eth0 >>>>>>>> >>>>>>>> Signed-off-by: Steve Wise <swise@opengridcomputing.com> >>>>>>>> rdma/link.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>>>> rdma/rdma.h | 1 + >>>>>>>> rdma/utils.c | 2 +- >>>>>>>> 3 files changed, 108 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/rdma/link.c b/rdma/link.c >>>>>>>> index 7a6d4b7e356d..d4f76b0ce11f 100644 >>>>>>>> +++ b/rdma/link.c >>>>>>>> @@ -14,6 +14,8 @@ >>>>>>>> static int link_help(struct rd *rd) >>>>>>>> { >>>>>>>> pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename); >>>>>>>> + pr_out("Usage: %s link add NAME type TYPE dev DEV\n", rd->filename); >>>>>>> I suggest to rename "dev" to be "netdev", because we are using "dev" for >>>>>>> ib devices. >>>>>> Yea ok. >>>>>> >>>>>>>> + pr_out("Usage: %s link delete NAME type TYPE\n", rd->filename); >>>>>>> Why do you need "type" for "delete" command? >>>>>> Because the type is used in the kernel to find the appropriate link >>>>>> ops. I could change the kernel side to search all types for the device >>>>>> name to delete? >>>>> I would say, yes. >>>>> It makes "delete" operation more natural. >>>>> >>>>> Thanks >>>> Perhaps. >>>> >>>> Note: 'ip link delete' takes a type as well... >>> According to man section, yes. >>> According to various guides, no. >>> https://docs.fedoraproject.org/en-US/Fedora/20/html/Networking_Guide/sec-Configure_802_1Q_VLAN_Tagging_ip_Commands.html >>> >>> Thanks >> It does make sense to not require type. The name must be unique so that >> should be enough. I'll have to respin the kernel side though... > The delete_link really should be an operation on the ib_device, not > the link_ops thing. > > That directly prevents mis-matching function callbacks.. > > Jason Looking at the rtnetlink newlink/dellink, I see they cache the link_ops ptr in the net_device struct. So when the link is deleted, then appropriate driver-specific dellink function can be called after finding the device to be deleted. Should I do something along these lines? IE add a struct rdma_link_ops pointer to struct ib_device. Steve. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 22:21 ` Steve Wise @ 2018-11-28 22:25 ` Jason Gunthorpe 2018-11-28 22:51 ` Steve Wise 0 siblings, 1 reply; 17+ messages in thread From: Jason Gunthorpe @ 2018-11-28 22:25 UTC (permalink / raw) To: Steve Wise; +Cc: Leon Romanovsky, dsahern, stephen, netdev, linux-rdma, BMT On Wed, Nov 28, 2018 at 04:21:48PM -0600, Steve Wise wrote: > >> It does make sense to not require type. The name must be unique so that > >> should be enough. I'll have to respin the kernel side though... > > The delete_link really should be an operation on the ib_device, not > > the link_ops thing. > > > > That directly prevents mis-matching function callbacks.. > > > > Jason > Looking at the rtnetlink newlink/dellink, I see they cache the link_ops > ptr in the net_device struct. So when the link is deleted, then > appropriate driver-specific dellink function can be called after finding > the device to be deleted. Should I do something along these lines? IE > add a struct rdma_link_ops pointer to struct ib_device. I don't see a problem with that either.. Jason ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands 2018-11-28 22:25 ` Jason Gunthorpe @ 2018-11-28 22:51 ` Steve Wise 0 siblings, 0 replies; 17+ messages in thread From: Steve Wise @ 2018-11-28 22:51 UTC (permalink / raw) To: Jason Gunthorpe Cc: Leon Romanovsky, dsahern, stephen, netdev, linux-rdma, BMT On 11/28/2018 4:25 PM, Jason Gunthorpe wrote: > On Wed, Nov 28, 2018 at 04:21:48PM -0600, Steve Wise wrote: > >>>> It does make sense to not require type. The name must be unique so that >>>> should be enough. I'll have to respin the kernel side though... >>> The delete_link really should be an operation on the ib_device, not >>> the link_ops thing. >>> >>> That directly prevents mis-matching function callbacks.. >>> >>> Jason >> Looking at the rtnetlink newlink/dellink, I see they cache the link_ops >> ptr in the net_device struct. So when the link is deleted, then >> appropriate driver-specific dellink function can be called after finding >> the device to be deleted. Should I do something along these lines? IE >> add a struct rdma_link_ops pointer to struct ib_device. > I don't see a problem with that either.. > > Jason Ok, I'll respin the kernel and user patches tomorrow. Thanks! ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH RFC iproute2-next 2/2] rdma: man page update for link add/delete 2018-11-28 16:25 [PATCH RFC iproute2-next 0/2] Dynamic rdma link creation Steve Wise 2018-09-13 17:19 ` [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands Steve Wise @ 2018-11-28 15:16 ` Steve Wise 1 sibling, 0 replies; 17+ messages in thread From: Steve Wise @ 2018-11-28 15:16 UTC (permalink / raw) To: dsahern, leon; +Cc: stephen, netdev, linux-rdma, leon, BMT Update the 'rdma link' man page with 'link add/delete' info. Signed-off-by: Steve Wise <swise@opengridcomputing.com> --- man/man8/rdma-link.8 | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/man/man8/rdma-link.8 b/man/man8/rdma-link.8 index 97dd8bb994d2..b994c326dc34 100644 --- a/man/man8/rdma-link.8 +++ b/man/man8/rdma-link.8 @@ -23,6 +23,20 @@ rdma-link \- rdma link configuration .RI "[ " DEV/PORT_INDEX " ]" .ti -8 +.B rdma link add +.BR NAME +.BR type +.BR TYPE +.BR dev +.BR NETDEV + +.ti -8 +.B rdma link delete +.RI NAME +.BR type +.BR TYPE + +.ti -8 .B rdma link help .SH "DESCRIPTION" @@ -33,6 +47,33 @@ rdma-link \- rdma link configuration - specifies the RDMA link to show. If this argument is omitted all links are listed. +.SS rdma link add NAME type TYPE dev NETDEV - add an rdma link for the specified type to the network device +.sp +.BR NAME +- specifies the new name of the rdma link to add + +.BR TYPE +- specifies which rdma type to use. Link types: +.sp +.in +8 +.B rxe +- Soft RoCE driver +.sp +.B siw +- Soft iWARP driver +.in -8 + +.BR NETDEV +- specifies the network device to which the link is bound + +.SS rdma link delete NAME type TYPE - delete an rdma link for the specified type +.PP +.BR NAME +- specifies the name of the rdma link to delete +.PP +.BR TYPE +- specifies which rdma type to use + .SH "EXAMPLES" .PP rdma link show @@ -45,6 +86,16 @@ rdma link show mlx5_2/1 Shows the state of specified rdma link. .RE .PP +rdma link add siw_eth0 type rxe dev eth0 +.RS 4 +Adds a RXE link named siw_eth0 to network device eth0 +.RE +.PP +rdma link del siw_eth0 type rxe +.RS 4 +Removes RXE link siw_eth0 +.RE +.PP .SH SEE ALSO .BR rdma (8), -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-11-29 9:54 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-11-28 16:25 [PATCH RFC iproute2-next 0/2] Dynamic rdma link creation Steve Wise 2018-09-13 17:19 ` [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands Steve Wise 2018-11-28 18:26 ` Leon Romanovsky 2018-11-28 19:08 ` Steve Wise 2018-11-28 19:34 ` Steve Wise 2018-11-28 20:02 ` Leon Romanovsky 2018-11-28 20:08 ` Leon Romanovsky 2018-11-28 20:23 ` Steve Wise 2018-11-28 20:04 ` Leon Romanovsky 2018-11-28 20:07 ` Steve Wise 2018-11-28 20:13 ` Leon Romanovsky 2018-11-28 20:18 ` Steve Wise 2018-11-28 22:17 ` Jason Gunthorpe 2018-11-28 22:21 ` Steve Wise 2018-11-28 22:25 ` Jason Gunthorpe 2018-11-28 22:51 ` Steve Wise 2018-11-28 15:16 ` [PATCH RFC iproute2-next 2/2] rdma: man page update for link add/delete Steve Wise
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).