Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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!
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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
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.
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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
> >> 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
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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
>> ---
>> 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.
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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, , >> + (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...
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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
> ---
> 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
>
>
signature.asc
Description: PGP signature
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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, , > > >>> + (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 > > > signature.asc Description: PGP signature
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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
---
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...
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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
> >> ---
> >> 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
signature.asc
Description: PGP signature
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
On Wed, Nov 28, 2018 at 01:34:14PM -0600, Steve Wise wrote: > ... > > >>> + rd_prepare_msg(rd, RDMA_NLDEV_CMD_NEWLINK, , > >>> +(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 > signature.asc Description: PGP signature
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
... >>> + rd_prepare_msg(rd, RDMA_NLDEV_CMD_NEWLINK, , >>> + (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).
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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
>> ---
>> 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, ,
>> + (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, ,
>> + (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 },
>> +{
Re: [PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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
> ---
> 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, ,
> +(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, ,
> +(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);
>
[PATCH RFC iproute2-next 1/2] rdma: add 'link add/delete' commands
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
---
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, ,
+ (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, ,
+ (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))
