On Sun, Jun 21, 2020 at 11:44:52AM +0300, Yishai Hadas wrote:
> On 6/19/2020 3:50 PM, Jason Gunthorpe wrote:
> > On Wed, Jun 17, 2020 at 10:45:53AM +0300, Yishai Hadas wrote:
> > > Implement the import/unimport MR verbs based on their definition as
> > > described in the man page.
> > >
> > > It uses the query MR KABI to retrieve the original MR properties based
> > > on its given handle.
> > >
> > > Signed-off-by: Yishai Hadas <yishaih@xxxxxxxxxxxx>
> > > libibverbs/cmd_mr.c | 35 +++++++++++++++++++++++++++++++++++
> > > libibverbs/driver.h | 3 +++
> > > libibverbs/libibverbs.map.in | 1 +
> > > providers/mlx5/mlx5.c | 2 ++
> > > providers/mlx5/mlx5.h | 3 +++
> > > providers/mlx5/verbs.c | 24 ++++++++++++++++++++++++
> > > 6 files changed, 68 insertions(+)
> > >
> > > diff --git a/libibverbs/cmd_mr.c b/libibverbs/cmd_mr.c
> > > index cb729b6..6984948 100644
> > > +++ b/libibverbs/cmd_mr.c
> > > @@ -85,3 +85,38 @@ int ibv_cmd_dereg_mr(struct verbs_mr *vmr)
> > > return ret;
> > > return 0;
> > > }
> > > +
> > > +int ibv_cmd_query_mr(struct ibv_pd *pd, struct verbs_mr *vmr,
> > > + uint32_t mr_handle)
> > > +{
> > > + DECLARE_FBCMD_BUFFER(cmd, UVERBS_OBJECT_MR,
> > > + UVERBS_METHOD_QUERY_MR,
> > > + 5, NULL);
> > > + struct ibv_mr *mr = &vmr->ibv_mr;
> > > + uint64_t iova;
> > > + int ret;
> > > +
> > > + fill_attr_in_obj(cmd, UVERBS_ATTR_QUERY_MR_HANDLE, mr_handle);
> > > + fill_attr_out_ptr(cmd, UVERBS_ATTR_QUERY_MR_RESP_LKEY,
> > > + &mr->lkey);
> > > + fill_attr_out_ptr(cmd, UVERBS_ATTR_QUERY_MR_RESP_RKEY,
> > > + &mr->rkey);
> > > + fill_attr_out_ptr(cmd, UVERBS_ATTR_QUERY_MR_RESP_LENGTH,
> > > + &mr->length);
> > > + /* The iova may be used down the road, let's have it ready from kernel */
> > > + fill_attr_out_ptr(cmd, UVERBS_ATTR_QUERY_MR_RESP_IOVA,
> > > + &iova);
> >
> > There isn't much reason to fill the attribute here..
> >
>
> We have defined this attribute from kernel perspective to be mandatory from
> day one as of other attributes above.
> Are you suggesting to change in kernel to let this attribute be optional and
> not fill it here ? other alternative ?
I'm not sure output attributes should be marked as mandatory?
Jason
