* [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
2018-11-28 16:25 ` Steve Wise
@ 2018-09-13 17:19 ` Steve Wise
-1 siblings, 0 replies; 20+ messages in thread
From: Steve Wise @ 2018-09-13 17:19 UTC (permalink / raw)
To: dsahern; +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] 20+ messages in thread
* [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
@ 2018-09-13 17:19 ` Steve Wise
0 siblings, 0 replies; 20+ 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] 20+ messages in thread
* [PATCH RFC iproute2-next 2/2] rdma: man page update for link add/delete
2018-11-28 16:25 ` Steve Wise
@ 2018-11-28 15:16 ` Steve Wise
-1 siblings, 0 replies; 20+ messages in thread
From: Steve Wise @ 2018-11-28 15:16 UTC (permalink / raw)
To: dsahern; +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] 20+ messages in thread
* [PATCH RFC iproute2-next 2/2] rdma: man page update for link add/delete
@ 2018-11-28 15:16 ` Steve Wise
0 siblings, 0 replies; 20+ 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] 20+ messages in thread
* [PATCH RFC iproute2-next 0/2] Dynamic rdma link creation
@ 2018-11-28 16:25 ` Steve Wise
0 siblings, 0 replies; 20+ messages in thread
From: Steve Wise @ 2018-11-28 16:25 UTC (permalink / raw)
To: dsahern; +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] 20+ messages in thread
* [PATCH RFC iproute2-next 0/2] Dynamic rdma link creation
@ 2018-11-28 16:25 ` Steve Wise
0 siblings, 0 replies; 20+ 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] 20+ messages in thread
* Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
2018-09-13 17:19 ` Steve Wise
(?)
@ 2018-11-28 18:26 ` Leon Romanovsky
2018-11-28 19:08 ` Steve Wise
-1 siblings, 1 reply; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ messages in thread
end of thread, other threads:[~2018-11-29 3:29 UTC | newest]
Thread overview: 20+ 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-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-09-13 17:19 ` 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
2018-11-28 15:16 ` Steve Wise
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.