| Message ID | 1462549405-16003-1-git-send-email-eli@mellanox.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
Hello. On 05/06/2016 06:43 PM, Eli Cohen wrote: > Add two NLA's that allow configuration of Infiniband node or port GUIDs > by referencing the IPoIB net device set over then physical function. The > format to be used is as follows: > > ip link set dev ib0 vf 0 node_guid 00:02:c9:03:00:21:6e:70 > ip link set dev ib0 vf 0 port_guid 00:02:c9:03:00:21:6e:78 > > Issue: 702759 > Change-Id: I5ffb54d6de7bfa8650bf5818f484279914991d6e > Signed-off-by: Eli Cohen <eli@mellanox.com> > --- > ip/iplink.c | 40 ++++++++++++++++++++++++++++++++++++++++ > man/man8/ip-link.8.in | 12 +++++++++++- > 2 files changed, 51 insertions(+), 1 deletion(-) > > diff --git a/ip/iplink.c b/ip/iplink.c > index d2e586b6d133..3f885defdfeb 100644 > --- a/ip/iplink.c > +++ b/ip/iplink.c > @@ -237,6 +237,30 @@ struct iplink_req { > char buf[1024]; > }; > > +static int extract_guid(__u64 *guid, char *arg) > +{ > + __u64 ret; > + int g[8]; > + int err; > + > + err = sscanf(arg, "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", > + g, g + 1, g + 2, g + 3, g + 4, g + 5, g + 6, g + 7); > + if (err != 8) Strange name for a variable, if sscanf() returns # of fields read... In fact, you don't even need this variable. > + return -1; > + > + ret = ((__u64)(g[0]) << 56) | > + ((__u64)(g[1]) << 48) | > + ((__u64)(g[2]) << 40) | > + ((__u64)(g[3]) << 32) | > + ((__u64)(g[4]) << 24) | > + ((__u64)(g[5]) << 16) | > + ((__u64)(g[6]) << 8) | > + ((__u64)(g[7])); > + *guid = ret; > + > + return 0; > +} > + > static int iplink_parse_vf(int vf, int *argcp, char ***argvp, > struct iplink_req *req, int dev_index) > { [...] MBR, Sergei
On Fri, 6 May 2016 10:43:25 -0500 Eli Cohen <eli@mellanox.com> wrote: > Add two NLA's that allow configuration of Infiniband node or port GUIDs > by referencing the IPoIB net device set over then physical function. The > format to be used is as follows: > > ip link set dev ib0 vf 0 node_guid 00:02:c9:03:00:21:6e:70 > ip link set dev ib0 vf 0 port_guid 00:02:c9:03:00:21:6e:78 > > Issue: 702759 > Change-Id: I5ffb54d6de7bfa8650bf5818f484279914991d6e > Signed-off-by: Eli Cohen <eli@mellanox.com> I am not that familiar with Infiniband, but the documentation seems to use a non-colon form: # ip link set dev ib0 vf 0 node_guid 0002c90300216e70 Seems like ip should follow the lead of ibstat and friends.
On Fri, May 06, 2016 at 10:43:25AM -0500, Eli Cohen wrote: > Add two NLA's that allow configuration of Infiniband node or port GUIDs > by referencing the IPoIB net device set over then physical function. The > format to be used is as follows: > > ip link set dev ib0 vf 0 node_guid 00:02:c9:03:00:21:6e:70 > ip link set dev ib0 vf 0 port_guid 00:02:c9:03:00:21:6e:78 > > Issue: 702759 > Change-Id: I5ffb54d6de7bfa8650bf5818f484279914991d6e These two lines are not needed to be part of commit message.
Hi Stephen, ibstat is usually used with MLNX_OFED distributions and I don't think it is a part of libibverbs. Moreover, in a previous patch I sent https://patchwork.ozlabs.org/patch/596492/ I documented in the change log the same format I mention in this patch. So if I change the format here we will have the changelog in the kernel tree invalid. How do you think this should be addressed? -----Original Message----- From: Stephen Hemminger [mailto:stephen@networkplumber.org] Sent: Friday, May 06, 2016 1:54 PM To: Eli Cohen <eli@mellanox.com> Cc: shemminger@osdl.org; netdev@vger.kernel.org Subject: Re: [PATCH] Add support for configuring Infiniband GUIDs On Fri, 6 May 2016 10:43:25 -0500 Eli Cohen <eli@mellanox.com> wrote: > Add two NLA's that allow configuration of Infiniband node or port > GUIDs by referencing the IPoIB net device set over then physical > function. The format to be used is as follows: > > ip link set dev ib0 vf 0 node_guid 00:02:c9:03:00:21:6e:70 ip link set > dev ib0 vf 0 port_guid 00:02:c9:03:00:21:6e:78 > > Issue: 702759 > Change-Id: I5ffb54d6de7bfa8650bf5818f484279914991d6e > Signed-off-by: Eli Cohen <eli@mellanox.com> I am not that familiar with Infiniband, but the documentation seems to use a non-colon form: # ip link set dev ib0 vf 0 node_guid 0002c90300216e70 Seems like ip should follow the lead of ibstat and friends.
Hi Stephen, can you please comment on this or are you going to apply this? On Mon, May 09, 2016 at 03:52:02PM +0000, Eli Cohen wrote: > Hi Stephen, > > ibstat is usually used with MLNX_OFED distributions and I don't think it is a part of libibverbs. > Moreover, in a previous patch I sent https://patchwork.ozlabs.org/patch/596492/ I documented in the change log the same format I mention in this patch. So if I change the format here we will have the changelog in the kernel tree invalid. > > How do you think this should be addressed? >
Hi Stephen, can you please comment on this or can we move forward with submitting this patch? Thanks, Eli On Fri, May 13, 2016 at 10:52:25PM +0300, Eli Cohen wrote: > Hi Stephen, > > can you please comment on this or are you going to apply this? > > On Mon, May 09, 2016 at 03:52:02PM +0000, Eli Cohen wrote: > > Hi Stephen, > > > > ibstat is usually used with MLNX_OFED distributions and I don't think it is a part of libibverbs. > > Moreover, in a previous patch I sent https://patchwork.ozlabs.org/patch/596492/ I documented in the change log the same format I mention in this patch. So if I change the format here we will have the changelog in the kernel tree invalid. > > > > How do you think this should be addressed? > >
Hi Sergei, I am not following on your comment. Are you referring to the variable 'g'? It's just a short name to make the code more readable. I also don't follow your comment on the fact that sscanf returns the number of arguments read. Please explain. Eli -----Original Message----- From: Sergei Shtylyov [mailto:sergei.shtylyov@cogentembedded.com] Sent: Friday, May 06, 2016 12:05 PM To: Eli Cohen <eli@mellanox.com>; shemminger@osdl.org Cc: netdev@vger.kernel.org Subject: Re: [PATCH] Add support for configuring Infiniband GUIDs Hello. On 05/06/2016 06:43 PM, Eli Cohen wrote: > Add two NLA's that allow configuration of Infiniband node or port > GUIDs by referencing the IPoIB net device set over then physical > function. The format to be used is as follows: > > ip link set dev ib0 vf 0 node_guid 00:02:c9:03:00:21:6e:70 ip link set > dev ib0 vf 0 port_guid 00:02:c9:03:00:21:6e:78 > > Issue: 702759 > Change-Id: I5ffb54d6de7bfa8650bf5818f484279914991d6e > Signed-off-by: Eli Cohen <eli@mellanox.com> > --- > ip/iplink.c | 40 ++++++++++++++++++++++++++++++++++++++++ > man/man8/ip-link.8.in | 12 +++++++++++- > 2 files changed, 51 insertions(+), 1 deletion(-) > > diff --git a/ip/iplink.c b/ip/iplink.c index > d2e586b6d133..3f885defdfeb 100644 > --- a/ip/iplink.c > +++ b/ip/iplink.c > @@ -237,6 +237,30 @@ struct iplink_req { > char buf[1024]; > }; > > +static int extract_guid(__u64 *guid, char *arg) { > + __u64 ret; > + int g[8]; > + int err; > + > + err = sscanf(arg, "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", > + g, g + 1, g + 2, g + 3, g + 4, g + 5, g + 6, g + 7); > + if (err != 8) Strange name for a variable, if sscanf() returns # of fields read... In fact, you don't even need this variable. > + return -1; > + > + ret = ((__u64)(g[0]) << 56) | > + ((__u64)(g[1]) << 48) | > + ((__u64)(g[2]) << 40) | > + ((__u64)(g[3]) << 32) | > + ((__u64)(g[4]) << 24) | > + ((__u64)(g[5]) << 16) | > + ((__u64)(g[6]) << 8) | > + ((__u64)(g[7])); > + *guid = ret; > + > + return 0; > +} > + > static int iplink_parse_vf(int vf, int *argcp, char ***argvp, > struct iplink_req *req, int dev_index) > { [...] MBR, Sergei
diff --git a/ip/iplink.c b/ip/iplink.c index d2e586b6d133..3f885defdfeb 100644 --- a/ip/iplink.c +++ b/ip/iplink.c @@ -237,6 +237,30 @@ struct iplink_req { char buf[1024]; }; +static int extract_guid(__u64 *guid, char *arg) +{ + __u64 ret; + int g[8]; + int err; + + err = sscanf(arg, "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", + g, g + 1, g + 2, g + 3, g + 4, g + 5, g + 6, g + 7); + if (err != 8) + return -1; + + ret = ((__u64)(g[0]) << 56) | + ((__u64)(g[1]) << 48) | + ((__u64)(g[2]) << 40) | + ((__u64)(g[3]) << 32) | + ((__u64)(g[4]) << 24) | + ((__u64)(g[5]) << 16) | + ((__u64)(g[6]) << 8) | + ((__u64)(g[7])); + *guid = ret; + + return 0; +} + static int iplink_parse_vf(int vf, int *argcp, char ***argvp, struct iplink_req *req, int dev_index) { @@ -383,6 +407,22 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp, invarg("Invalid \"state\" value\n", *argv); ivl.vf = vf; addattr_l(&req->n, sizeof(*req), IFLA_VF_LINK_STATE, &ivl, sizeof(ivl)); + } else if (matches(*argv, "node_guid") == 0) { + struct ifla_vf_guid ivg; + + NEXT_ARG(); + ivg.vf = vf; + if (extract_guid(&ivg.guid, *argv)) + return -1; + addattr_l(&req->n, sizeof(*req), IFLA_VF_IB_NODE_GUID, &ivg, sizeof(ivg)); + } else if (matches(*argv, "port_guid") == 0) { + struct ifla_vf_guid ivg; + + NEXT_ARG(); + ivg.vf = vf; + if (extract_guid(&ivg.guid, *argv)) + return -1; + addattr_l(&req->n, sizeof(*req), IFLA_VF_IB_PORT_GUID, &ivg, sizeof(ivg)); } else { /* rewind arg */ PREV_ARG(); diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in index 805511423ef2..e143a5ec8a9a 100644 --- a/man/man8/ip-link.8.in +++ b/man/man8/ip-link.8.in @@ -143,7 +143,11 @@ ip-link \- network device configuration .br .RB "[ " state " { " auto " | " enable " | " disable " } ]" .br -.RB "[ " trust " { " on " | " off " } ] ]" +.RB "[ " trust " { " on " | " off " } ]" +.br +.RB "[ " node_guid " eui64 ]" +.br +.RB "[ " port_guid " eui64 ] ]" .br .in -9 .RB "[ " master @@ -1033,6 +1037,12 @@ sent by the VF. .BI trust " on|off" - trust the specified VF user. This enables that VF user can set a specific feature which may impact security and/or performance. (e.g. VF multicast promiscuous mode) +.sp +.BI node_guid " eui64" +- configure node GUID for the VF. +.sp +.BI port_guid " eui64" +- configure port GUID for the VF. .in -8 .TP
Add two NLA's that allow configuration of Infiniband node or port GUIDs by referencing the IPoIB net device set over then physical function. The format to be used is as follows: ip link set dev ib0 vf 0 node_guid 00:02:c9:03:00:21:6e:70 ip link set dev ib0 vf 0 port_guid 00:02:c9:03:00:21:6e:78 Issue: 702759 Change-Id: I5ffb54d6de7bfa8650bf5818f484279914991d6e Signed-off-by: Eli Cohen <eli@mellanox.com> --- ip/iplink.c | 40 ++++++++++++++++++++++++++++++++++++++++ man/man8/ip-link.8.in | 12 +++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-)