Re: [RFC rdma-core] verbs: add ibv_export_to_fd man page

[Yuval Shaia <yuval.shaia@oracle.com>]

On Tue, Jul 02, 2019 at 07:48:07PM -0300, Jason Gunthorpe wrote:
> On Wed, Jun 26, 2019 at 03:46:39PM +0300, Yuval Shaia wrote:
> > On Wed, Jun 26, 2019 at 11:36:14AM +0300, Shamir Rabinovitch wrote:
> > > Add the ibv_export_to_fd man page.
> > 
> > This is RFC but still suggesting to give some words here.
> > 
> > Also, subject is incorrect since man page is for all functions involved in
> > the shared-obj mechanism, not only the export_to_fd.
> > 
> > > 
> > > Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@oracle.com>
> > >  libibverbs/man/ibv_export_to_fd.3.md | 109 +++++++++++++++++++++++++++
> > >  1 file changed, 109 insertions(+)
> > >  create mode 100644 libibverbs/man/ibv_export_to_fd.3.md
> > > 
> > > diff --git a/libibverbs/man/ibv_export_to_fd.3.md b/libibverbs/man/ibv_export_to_fd.3.md
> > > new file mode 100644
> > > index 00000000..8e3f0fb2
> > > +++ b/libibverbs/man/ibv_export_to_fd.3.md
> > > @@ -0,0 +1,109 @@
> > > +---
> > > +date: 2018-06-26
> > > +footer: libibverbs
> > > +header: "Libibverbs Programmer's Manual"
> > > +layout: page
> > > +license: 'Licensed under the OpenIB.org BSD license (FreeBSD Variant) - See COPYING.md'
> > > +section: 3
> > > +title: ibv_export_to_fd
> > > +tagline: Verbs
> > > +---
> > > +
> > > +# NAME
> > > +
> > > +**ibv_export_to_fd**, **ibv_import_pd**, **ibv_import_mr** - export & import ib hw objects.
> > > +
> > > +# SYNOPSIS
> > > +
> > > +```c
> > > +#include <infiniband/verbs.h>
> > > +
> > > +int ibv_export_to_fd(uint32_t fd,
> > > +                     uint32_t *new_handle,
> > > +                     struct ibv_context *context,
> > > +                     enum uverbs_default_objects type,
> > > +                     uint32_t handle);
> 
> This should probably be some internal function and the exports should
> be type safe just like the imports.

So you suggesting something like this (instead of passing handle as arg):

int ibv_export_pd(uint32_t fd,
		  uint32_t *new_handle,
		  struct ibv_context *context,
		  struct ib_pd* pd);

int ibv_export_mr(uint32_t fd,
		  uint32_t *new_handle,
		  struct ibv_context *context,
		  struct ib_mr* mr);

So the handle is taken internally from the pd or mr  arg.

Are you still ok with new_handle? asking as this is what is used in the
ibv_import_xxx functions.

> 
> > > +struct ibv_pd *ibv_import_pd(struct ibv_context *context,
> > > +                             uint32_t fd,
> > > +                             uint32_t handle);
> > > +
> > > +struct ibv_mr *ibv_import_mr(struct ibv_context *context,
> > > +                             uint32_t fd,
> > > +                             uint32_t handle);
> > > +
> > > +uint32_t ibv_context_to_fd(struct ibv_context *context);
> > > +
> > > +uint32_t ibv_pd_to_handle(struct ibv_pd *pd);
> > > +
> > > +uint32_t ibv_mr_to_handle(struct ibv_mr *mr);
> > 
> > Do you know if extra stuff besides this new file needs to be done so i can
> > do ex man ibv_context_to_fd and get this man page?
> 
> Yes, they need to be setup in cmake with aliases.

Will take care of it, thanks.

> 
> I think this man page is kind of terse for such a complicated
> thing. 
> 
> Ie it doesn't talk about what happens when close() or ibv_destroy_X()
> is called.

We've mentioned that the returned object is like a regular object returned
from (ex) ibv_create_pd and should be destroyed with the corresponding
destroy function.
We can add a note saying that the HW object will be destroyed only when all
reference to it will be destroyed.
Is that enough?

> 
> Jason