From mboxrd@z Thu Jan 1 00:00:00 1970 From: Knut Omang Subject: Re: [PATCH libibverbs v3 1/3] Add new call ibv_cmd_create_ah_ex which supports extra parameters Date: Tue, 20 Sep 2016 16:05:50 +0200 Message-ID: <1474380350.20134.162.camel@oracle.com> References: <550041ef-b105-c079-068c-d96278ec34f1@dev.mellanox.co.il> <1474371459.8837.32.camel@oracle.com> <20160920132945.GO26673@leon.nu> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20160920132945.GO26673-2ukJVAZIZ/Y@public.gmane.org> Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Leon Romanovsky Cc: Yishai Hadas , Doug Ledford , linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mukesh Kacker , Jason Gunthorpe List-Id: linux-rdma@vger.kernel.org On Tue, 2016-09-20 at 16:29 +0300, Leon Romanovsky wrote: > On Tue, Sep 20, 2016 at 01:37:39PM +0200, Knut Omang wrote: > > On Tue, 2016-09-20 at 14:17 +0300, Yishai Hadas wrote: > > > On 9/20/2016 9:22 AM, Knut Omang wrote: > > > > > > > > The original call ibv_cmd_create_ah does not allow vendor specific request or > > > > response parameters to be defined and passed through to kernel space. > > > > > > > > To keep backward compatibility, introduce a new extended call which accepts > > > > cmd and resp parameters similar to other parts of the API. > > > > Reimplement the old call to just use the new extended call. > > > > > > > > The new call is versioned in the shared library as IBVERBS_1.4. > > > > > > > > Signed-off-by: Knut Omang > > > > Reviewed-by: Mukesh Kacker > > > > --- > > > > include/infiniband/driver.h | 4 +++- > > > > src/cmd.c | 50 +++++++++++++++++++++----------------- > > > > src/libibverbs.map | 6 ++++- > > > > 3 files changed, 37 insertions(+), 23 deletions(-) > > > > > > > > diff --git a/include/infiniband/driver.h b/include/infiniband/driver.h > > > > index ea3dade..9356dc8 100644 > > > > --- a/include/infiniband/driver.h > > > > +++ b/include/infiniband/driver.h > > > > @@ -234,6 +234,10 @@ int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr, > > > > struct ibv_recv_wr **bad_wr); > > > > int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah, > > > > struct ibv_ah_attr *attr); > > > > +int ibv_cmd_create_ah_ex(struct ibv_pd *pd, struct ibv_ah *ah, > > > > + struct ibv_ah_attr *attr, > > > > + struct ibv_create_ah *cmd, size_t cmd_size, > > > > + struct ibv_create_ah_resp *resp, size_t resp_size); > > > > int ibv_cmd_destroy_ah(struct ibv_ah *ah); > > > > int ibv_cmd_attach_mcast(struct ibv_qp *qp, const union ibv_gid *gid, uint16_t lid); > > > > int ibv_cmd_detach_mcast(struct ibv_qp *qp, const union ibv_gid *gid, uint16_t lid); > > > > diff --git a/src/cmd.c b/src/cmd.c > > > > index 11f6509..58b9dcd 100644 > > > > --- a/src/cmd.c > > > > +++ b/src/cmd.c > > > > @@ -1493,38 +1493,44 @@ int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr, > > > > return ret; > > > > } > > > > > > > > -int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah, > > > > - struct ibv_ah_attr *attr) > > > > +int ibv_cmd_create_ah_ex(struct ibv_pd *pd, struct ibv_ah *ah, struct ibv_ah_attr *attr, > > > > + struct ibv_create_ah *cmd, size_t cmd_size, > > > > + struct ibv_create_ah_resp *resp, size_t resp_size) > > > > { > > > > - struct ibv_create_ah cmd; > > > > - struct ibv_create_ah_resp resp; > > > > - > > > > - IBV_INIT_CMD_RESP(&cmd, sizeof cmd, CREATE_AH, &resp, sizeof resp); > > > > - cmd.user_handle = (uintptr_t) ah; > > > > - cmd.pd_handle = pd->handle; > > > > - cmd.attr.dlid = attr->dlid; > > > > - cmd.attr.sl = attr->sl; > > > > - cmd.attr.src_path_bits = attr->src_path_bits; > > > > - cmd.attr.static_rate = attr->static_rate; > > > > - cmd.attr.is_global = attr->is_global; > > > > - cmd.attr.port_num = attr->port_num; > > > > - cmd.attr.grh.flow_label = attr->grh.flow_label; > > > > - cmd.attr.grh.sgid_index = attr->grh.sgid_index; > > > > - cmd.attr.grh.hop_limit = attr->grh.hop_limit; > > > > - cmd.attr.grh.traffic_class = attr->grh.traffic_class; > > > > - memcpy(cmd.attr.grh.dgid, attr->grh.dgid.raw, 16); > > > > + IBV_INIT_CMD_RESP(cmd, cmd_size, CREATE_AH, resp, resp_size); > > > > + cmd->user_handle = (uintptr_t) ah; > > > > + cmd->pd_handle = pd->handle; > > > > + cmd->attr.dlid = attr->dlid; > > > > + cmd->attr.sl = attr->sl; > > > > + cmd->attr.src_path_bits = attr->src_path_bits; > > > > + cmd->attr.static_rate = attr->static_rate; > > > > + cmd->attr.is_global = attr->is_global; > > > > + cmd->attr.port_num = attr->port_num; > > > > + cmd->attr.grh.flow_label = attr->grh.flow_label; > > > > + cmd->attr.grh.sgid_index = attr->grh.sgid_index; > > > > + cmd->attr.grh.hop_limit = attr->grh.hop_limit; > > > > + cmd->attr.grh.traffic_class = attr->grh.traffic_class; > > > > + memcpy(cmd->attr.grh.dgid, attr->grh.dgid.raw, 16); > > > > > > > > - if (write(pd->context->cmd_fd, &cmd, sizeof cmd) != sizeof cmd) > > > > + if (write(pd->context->cmd_fd, cmd, cmd_size) != cmd_size) > > > > return errno; > > > > > > > > - (void) VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp); > > > > + (void) VALGRIND_MAKE_MEM_DEFINED(resp, sizeof *resp); > > > > > > > > - ah->handle = resp.handle; > > > > + ah->handle = resp->handle; > > > > ah->context = pd->context; > > > > > > > > return 0; > > > > } > > > > > > > > +int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah, > > > > + struct ibv_ah_attr *attr) > > > > +{ > > > > + struct ibv_create_ah cmd; > > > > + struct ibv_create_ah_resp resp; > > > > + return ibv_cmd_create_ah_ex(pd, ah, attr, &cmd, sizeof(cmd), &resp, sizeof(resp)); > > > > +} > > > > + > > > > int ibv_cmd_destroy_ah(struct ibv_ah *ah) > > > > { > > > > struct ibv_destroy_ah cmd; > > > > diff --git a/src/libibverbs.map b/src/libibverbs.map > > > > index 46744e5..ddba63a 100644 > > > > --- a/src/libibverbs.map > > > > +++ b/src/libibverbs.map > > > > @@ -118,7 +118,6 @@ IBVERBS_1.1 { > > > > ibv_cmd_create_qp_ex2; > > > > ibv_cmd_open_qp; > > > > ibv_cmd_rereg_mr; > > > > - > > > > } IBVERBS_1.0; > > > > > > > > IBVERBS_1.3 { > > > > @@ -130,3 +129,8 @@ IBVERBS_1.3 { > > > > ibv_cmd_destroy_rwq_ind_table; > > > > ibv_query_gid_type; > > > > } IBVERBS_1.1; > > > > + > > > > +IBVERBS_1.4 { > > > Any reason to add 1.4 instead of using 1.3 ? in my understanding the > > > idea is that new symbols for next release will use a new ABI version, > > > this is what 1.3 already supplied. Added Jason for his input as well. > > > > good point - Jason? > > > > > > > > > > + global: > > > > + ibv_cmd_create_ah_ex; > > > When do you plan to contribute the driver code for that ? for now no > > > upstream drivers uses this API, taking some code for future usage with > > > no current usage is not the preferred way. > > > > The plan is ASAP, the kernel side has been approved for release by Oracle's > > legal dep, but the user library is still stuck, and with an undesirable > > license combination given Jason's recent consolidation efforts :-/ > > Jason didn't change licenses. You still can release your driver under > dual-license in similar way to other drivers. The problem is that libsif is stuck under a BSD only license imposed by the first set of lawyers involved, and to be compatible, we need it to be dual license GPLv2 + BSD like the rest of the provider libraries. Knut > > > > Knut > > > > > > > > > > +} IBVERBS_1.3; > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html