diff mbox series

[rdma-core,2/4] pyverbs: Introduce ParentDomain class

Message ID 20191114093732.12637-3-noaos@mellanox.com (mailing list archive)
State Not Applicable
Headers show
Series pyverbs: Introduce parent domain | expand

Commit Message

Noa Osherovich Nov. 14, 2019, 9:37 a.m. UTC
From: Edward Srouji <edwards@mellanox.com>

The parent domain object extends the normal PD (protection domain) and
can be used interchangeably with it.
This patch adds ParentDomain class, in order to allow the user to
allocate parent domain in a user-friendly way.

In addition, ParentDomainContext class was added which is used as the
pd_context and it includes PD (protection domain) object, alloc and
free Python functions.
This allows the user to implement allocators in Python that are
callable from the C callbacks wrappers via driver code.

Signed-off-by: Edward Srouji <edwards@mellanox.com>
Reviewd-by: Daria Velikovsky <daria@mellanox.com>
Reviewd-by: Noa Osherovich <noaos@mellanox.com>
---
 pyverbs/libibverbs.pxd                  |  11 ++
 pyverbs/libibverbs_enums.pxd            |   7 ++
 pyverbs/pd.pxd                          |  17 +++
 pyverbs/pd.pyx                          | 150 ++++++++++++++++++++++--
 pyverbs/providers/mlx5/mlx5dv_enums.pxd |  11 ++
 pyverbs/srq.pyx                         |   2 +-
 6 files changed, 187 insertions(+), 11 deletions(-)

Comments

Jason Gunthorpe Nov. 14, 2019, 1:39 p.m. UTC | #1
On Thu, Nov 14, 2019 at 09:37:46AM +0000, Noa Osherovich wrote:
> diff --git a/pyverbs/providers/mlx5/mlx5dv_enums.pxd b/pyverbs/providers/mlx5/mlx5dv_enums.pxd
> index 038a49111a3b..b02da9bf5001 100644
> +++ b/pyverbs/providers/mlx5/mlx5dv_enums.pxd
> @@ -45,3 +45,14 @@ cdef extern from 'infiniband/mlx5dv.h':
>          MLX5DV_FLOW_ACTION_FLAGS_ESP_AES_GCM_SPI_STEERING   = 1 << 2
>          MLX5DV_FLOW_ACTION_FLAGS_ESP_AES_GCM_FULL_OFFLOAD   = 1 << 3
>          MLX5DV_FLOW_ACTION_FLAGS_ESP_AES_GCM_TX_IV_IS_ESN   = 1 << 4
> +
> +    cdef unsigned long long MLX5DV_RES_TYPE_QP
> +    cdef unsigned long long MLX5DV_RES_TYPE_RWQ
> +    cdef unsigned long long MLX5DV_RES_TYPE_DBR
> +    cdef unsigned long long MLX5DV_RES_TYPE_SRQ

These are supposed to be enums, and should be cpdef

Jason
Edward Srouji Nov. 14, 2019, 4:49 p.m. UTC | #2
They are defined in mlx5dv with #define and not enums.
I will change to cpdef though.

-----Original Message-----
From: Jason Gunthorpe <jgg@mellanox.com> 
Sent: Thursday, November 14, 2019 3:40 PM
To: Noa Osherovich <noaos@mellanox.com>
Cc: dledford@redhat.com; Leon Romanovsky <leonro@mellanox.com>; linux-rdma@vger.kernel.org; Edward Srouji <edwards@mellanox.com>; Daria Velikovsky <daria@mellanox.com>
Subject: Re: [PATCH rdma-core 2/4] pyverbs: Introduce ParentDomain class

On Thu, Nov 14, 2019 at 09:37:46AM +0000, Noa Osherovich wrote:
> diff --git a/pyverbs/providers/mlx5/mlx5dv_enums.pxd b/pyverbs/providers/mlx5/mlx5dv_enums.pxd
> index 038a49111a3b..b02da9bf5001 100644
> +++ b/pyverbs/providers/mlx5/mlx5dv_enums.pxd
> @@ -45,3 +45,14 @@ cdef extern from 'infiniband/mlx5dv.h':
>          MLX5DV_FLOW_ACTION_FLAGS_ESP_AES_GCM_SPI_STEERING   = 1 << 2
>          MLX5DV_FLOW_ACTION_FLAGS_ESP_AES_GCM_FULL_OFFLOAD   = 1 << 3
>          MLX5DV_FLOW_ACTION_FLAGS_ESP_AES_GCM_TX_IV_IS_ESN   = 1 << 4
> +
> +    cdef unsigned long long MLX5DV_RES_TYPE_QP
> +    cdef unsigned long long MLX5DV_RES_TYPE_RWQ
> +    cdef unsigned long long MLX5DV_RES_TYPE_DBR
> +    cdef unsigned long long MLX5DV_RES_TYPE_SRQ

These are supposed to be enums, and should be cpdef

Jason
Jason Gunthorpe Nov. 14, 2019, 4:50 p.m. UTC | #3
On Thu, Nov 14, 2019 at 04:49:52PM +0000, Edward Srouji wrote:
> They are defined in mlx5dv with #define and not enums.
> I will change to cpdef though.

It is a mistake they are #define

Jason
Jason Gunthorpe Nov. 21, 2019, 4:31 p.m. UTC | #4
We rely on a C extension for enums larger than integers.

Jason

On Thu, Nov 21, 2019 at 04:28:00PM +0000, Edward Srouji wrote:
>    The C standard specifies that enums are integers (and are guaranteed to
>    be large enough to hold int).
> 
>    These #defines are larger than 32bits (they're 64bits), and even if
>    they're compiled with the driver, the Cythonization (of Pyverbs) would
>    treat the enums as integers and the values would be cut. (Cython throws
>    compilation warnings related to that).
> 
>    The PR was updated.
>      __________________________________________________________________
> 
>    From: Jason Gunthorpe <jgg@mellanox.com>
>    Sent: Thursday, November 14, 2019 6:50 PM
>    To: Edward Srouji <edwards@mellanox.com>
>    Cc: Noa Osherovich <noaos@mellanox.com>; dledford@redhat.com
>    <dledford@redhat.com>; Leon Romanovsky <leonro@mellanox.com>;
>    linux-rdma@vger.kernel.org <linux-rdma@vger.kernel.org>; Daria
>    Velikovsky <daria@mellanox.com>
>    Subject: Re: [PATCH rdma-core 2/4] pyverbs: Introduce ParentDomain
>    class
> 
>    On Thu, Nov 14, 2019 at 04:49:52PM +0000, Edward Srouji wrote:
>    > They are defined in mlx5dv with #define and not enums.
>    > I will change to cpdef though.
>    It is a mistake they are #define
>    Jason
diff mbox series

Patch

diff --git a/pyverbs/libibverbs.pxd b/pyverbs/libibverbs.pxd
index 02137d81e2d3..ad8d8bacc541 100755
--- a/pyverbs/libibverbs.pxd
+++ b/pyverbs/libibverbs.pxd
@@ -454,6 +454,15 @@  cdef extern from 'infiniband/verbs.h':
         ibv_qp_type     qp_type;
         unsigned int    events_completed;
 
+    cdef struct ibv_parent_domain_init_attr:
+        ibv_pd          *pd;
+        uint32_t        comp_mask;
+        void            *(*alloc)(ibv_pd *pd, void *pd_context, size_t size,
+                                  size_t alignment, uint64_t resource_type);
+        void            (*free)(ibv_pd *pd, void *pd_context, void *ptr,
+                                uint64_t resource_type);
+        void            *pd_context;
+
     ibv_device **ibv_get_device_list(int *n)
     void ibv_free_device_list(ibv_device **list)
     ibv_context *ibv_open_device(ibv_device *device)
@@ -540,3 +549,5 @@  cdef extern from 'infiniband/verbs.h':
     int ibv_destroy_srq(ibv_srq *srq)
     int ibv_post_srq_recv(ibv_srq *srq, ibv_recv_wr *recv_wr,
                           ibv_recv_wr **bad_recv_wr)
+    ibv_pd *ibv_alloc_parent_domain(ibv_context *context,
+                                    ibv_parent_domain_init_attr *attr)
diff --git a/pyverbs/libibverbs_enums.pxd b/pyverbs/libibverbs_enums.pxd
index 114915d0a751..66066e2c37fd 100755
--- a/pyverbs/libibverbs_enums.pxd
+++ b/pyverbs/libibverbs_enums.pxd
@@ -401,6 +401,13 @@  cdef extern from '<infiniband/verbs.h>':
     cdef unsigned long long IBV_DEVICE_RAW_SCATTER_FCS
     cdef unsigned long long IBV_DEVICE_PCI_WRITE_END_PADDING
 
+    cpdef enum ibv_parent_domain_init_attr_mask:
+        IBV_PARENT_DOMAIN_INIT_ATTR_ALLOCATORS
+        IBV_PARENT_DOMAIN_INIT_ATTR_PD_CONTEXT
+
+    cdef void *IBV_ALLOCATOR_USE_DEFAULT
+
 
 _IBV_DEVICE_RAW_SCATTER_FCS = IBV_DEVICE_RAW_SCATTER_FCS
 _IBV_DEVICE_PCI_WRITE_END_PADDING = IBV_DEVICE_PCI_WRITE_END_PADDING
+_IBV_ALLOCATOR_USE_DEFAULT = <size_t>IBV_ALLOCATOR_USE_DEFAULT
diff --git a/pyverbs/pd.pxd b/pyverbs/pd.pxd
index 6dd9c2959ed3..cb0b4715ba0d 100644
--- a/pyverbs/pd.pxd
+++ b/pyverbs/pd.pxd
@@ -3,6 +3,7 @@ 
 
 #cython: language_level=3
 
+from pyverbs.base cimport PyverbsObject
 from pyverbs.device cimport Context
 cimport pyverbs.libibverbs as v
 from .base cimport PyverbsCM
@@ -17,3 +18,19 @@  cdef class PD(PyverbsCM):
     cdef object mws
     cdef object ahs
     cdef object qps
+    cdef object parent_domains
+
+cdef class ParentDomainInitAttr(PyverbsObject):
+    cdef v.ibv_parent_domain_init_attr init_attr
+    cdef object pd
+    cdef object alloc
+    cdef object dealloc
+
+cdef class ParentDomain(PD):
+    cdef object protection_domain
+    pass
+
+cdef class ParentDomainContext(PyverbsObject):
+    cdef object p_alloc
+    cdef object p_free
+    cdef object pd
diff --git a/pyverbs/pd.pyx b/pyverbs/pd.pyx
index a4fc76f0fe5b..b639c3dcc74d 100644
--- a/pyverbs/pd.pyx
+++ b/pyverbs/pd.pyx
@@ -2,35 +2,43 @@ 
 # Copyright (c) 2019, Mellanox Technologies. All rights reserved.
 import weakref
 
-from pyverbs.pyverbs_error import PyverbsRDMAError, PyverbsError
+from pyverbs.pyverbs_error import PyverbsError, PyverbsUserError
 from pyverbs.base import PyverbsRDMAErrno
-from pyverbs.device cimport Context, DM
+from pyverbs.device cimport Context
+from libc.stdint cimport uintptr_t
+from libc.errno cimport errno
 from .mr cimport MR, MW, DMMR
 from pyverbs.srq cimport SRQ
 from pyverbs.addr cimport AH
 from pyverbs.qp cimport QP
-from libc.errno cimport errno
 
 
 cdef class PD(PyverbsCM):
-    def __cinit__(self, Context context not None):
+    def __cinit__(self, Context context not None, **kwargs):
         """
         Initializes a PD object. A reference for the creating Context is kept
         so that Python's GC will destroy the objects in the right order.
         :param context: The Context object creating the PD
-        :return: The newly created PD on success
+        :param kwargs: Arguments:
+            * *attr* (object)
+                If provided PD will not be allocated, leaving the allocation to
+                be made by an inheriting class
         """
-        self.pd = v.ibv_alloc_pd(<v.ibv_context*>context.context)
-        if self.pd == NULL:
-            raise PyverbsRDMAErrno('Failed to allocate PD', errno)
+        # If there's a Parent Domain attribute skip PD allocation
+        # since this is done by the Parent Domain class
+        if not kwargs.get('attr'):
+            self.pd = v.ibv_alloc_pd(<v.ibv_context*>context.context)
+            if self.pd == NULL:
+                raise PyverbsRDMAErrno('Failed to allocate PD', errno)
+            self.logger.debug('PD: Allocated ibv_pd')
         self.ctx = context
         context.add_ref(self)
-        self.logger.debug('PD: Allocated ibv_pd')
         self.srqs = weakref.WeakSet()
         self.mrs = weakref.WeakSet()
         self.mws = weakref.WeakSet()
         self.ahs = weakref.WeakSet()
         self.qps = weakref.WeakSet()
+        self.parent_domains = weakref.WeakSet()
 
     def __dealloc__(self):
         """
@@ -48,7 +56,8 @@  cdef class PD(PyverbsCM):
         :return: None
         """
         self.logger.debug('Closing PD')
-        self.close_weakrefs([self.qps, self.ahs, self.mws, self.mrs, self.srqs])
+        self.close_weakrefs([self.parent_domains, self.qps, self.ahs, self.mws,
+                             self.mrs, self.srqs])
         if self.pd != NULL:
             rc = v.ibv_dealloc_pd(self.pd)
             if rc != 0:
@@ -67,5 +76,126 @@  cdef class PD(PyverbsCM):
             self.qps.add(obj)
         elif isinstance(obj, SRQ):
             self.srqs.add(obj)
+        elif isinstance(obj, ParentDomain):
+            self.parent_domains.add(obj)
         else:
             raise PyverbsError('Unrecognized object type')
+
+
+cdef void *pd_alloc(v.ibv_pd *pd, void *pd_context, size_t size,
+                  size_t alignment, v.uint64_t resource_type):
+    """
+    Parent Domain allocator wrapper. This function is used to wrap a
+    user-defined Python alloc function which should be a part of pd_context.
+    :param pd: Parent domain
+    :param pd_context: User-specific context of type ParentDomainContext
+    :param size: Size of the requested buffer
+    :param alignment: Alignment of the requested buffer
+    :param resource_type: Vendor-specific resource type
+    :return: Pointer to the allocated buffer, or NULL to designate an error.
+             It may also return IBV_ALLOCATOR_USE_DEFAULT asking the callee to
+             allocate the buffer using the default allocator.
+
+    """
+    cdef ParentDomainContext pd_ctx
+    pd_ctx = <object>pd_context
+    ptr = <uintptr_t>pd_ctx.p_alloc(pd_ctx.pd, pd_ctx, size, alignment,
+                                    resource_type)
+    return <void*>ptr
+
+
+cdef void pd_free(v.ibv_pd *pd, void *pd_context, void *ptr,
+                     v.uint64_t resource_type):
+    """
+    Parent Domain deallocator wrapper. This function is used to wrap a
+    user-defined Python free function which should be part of pd_context.
+    :param pd: Parent domain
+    :param pd_context: User-specific context of type ParentDomainContext
+    :param ptr: Pointer to the buffer to be freed
+    :param resource_type: Vendor-specific resource type
+    """
+    cdef ParentDomainContext pd_ctx
+    pd_ctx = <object>pd_context
+    pd_ctx.p_free(pd_ctx.pd, pd_ctx, <uintptr_t>ptr, resource_type)
+
+
+cdef class ParentDomainContext(PyverbsObject):
+    def __cinit__(self, PD pd, alloc_func, free_func):
+        """
+        Initializes ParentDomainContext object which is used as a pd_context.
+        It contains the relevant fields in order to allow the user to write
+        alloc and free functions in Python
+        :param pd: PD object that represents the ibv_pd which is passed to the
+                  creation of the Parent Domain
+        :param alloc_func: Python alloc function
+        :param free_func: Python free function
+        """
+        self.pd = pd
+        self.p_alloc = alloc_func
+        self.p_free = free_func
+
+
+cdef class ParentDomainInitAttr(PyverbsObject):
+    def __cinit__(self, PD pd not None, ParentDomainContext pd_context=None):
+        """
+        Represents ibv_parent_domain_init_attr C struct
+        :param pd: PD to initialize the ParentDomain with
+        :param pd_context: ParentDomainContext object including the alloc and
+                          free Python callbacks
+        """
+        self.pd = pd
+        self.init_attr.pd = <v.ibv_pd*>pd.pd
+        if pd_context:
+            self.init_attr.alloc = pd_alloc
+            self.init_attr.free = pd_free
+            self.init_attr.pd_context = <void*>pd_context
+            # The only way to use Python callbacks is to pass the (Python)
+            # functions through pd_context. Hence, we must set PD_CONTEXT
+            # in the comp mask.
+            self.init_attr.comp_mask = v.IBV_PARENT_DOMAIN_INIT_ATTR_PD_CONTEXT | \
+                                       v.IBV_PARENT_DOMAIN_INIT_ATTR_ALLOCATORS
+
+    @property
+    def comp_mask(self):
+        return self.init_attr.comp_mask
+
+
+cdef class ParentDomain(PD):
+    def __cinit__(self, Context context not None, **kwargs):
+        """
+        Initializes ParentDomain object which represents a parent domain of
+        ibv_pd C struct type
+        :param context: Device context
+        :param kwargs: Arguments:
+            * *attr* (object)
+                Attribute of type ParentDomainInitAttr to initialize the
+                ParentDomain with
+        """
+        cdef ParentDomainInitAttr attr
+        attr = kwargs.get('attr')
+        if attr is None:
+            raise PyverbsUserError('ParentDomain must take attr')
+        (<PD>attr.pd).add_ref(self)
+        self.protection_domain = attr.pd
+        self.pd = v.ibv_alloc_parent_domain(context.context, &attr.init_attr)
+        if self.pd == NULL:
+            raise PyverbsRDMAErrno('Failed to allocate Parent Domain')
+        self.logger.debug('Allocated ParentDomain')
+
+    def __dealloc__(self):
+        self.__close(True)
+
+    cpdef close(self):
+        self.__close()
+
+    def __close(self, from_dealloc=False):
+        """
+        The close function can be called either explicitly by the user, or
+        implicitly (from __dealloc__). In the case it was called by dealloc,
+        the close function of the PD would have been already called, thus
+        freeing the PD of this parent domain and no need to dealloc it again
+        :param from_dealloc: Indicates whether the close was called via dealloc
+        """
+        self.logger.debug('Closing ParentDomain')
+        if not from_dealloc:
+            super(ParentDomain, self).close()
diff --git a/pyverbs/providers/mlx5/mlx5dv_enums.pxd b/pyverbs/providers/mlx5/mlx5dv_enums.pxd
index 038a49111a3b..b02da9bf5001 100644
--- a/pyverbs/providers/mlx5/mlx5dv_enums.pxd
+++ b/pyverbs/providers/mlx5/mlx5dv_enums.pxd
@@ -45,3 +45,14 @@  cdef extern from 'infiniband/mlx5dv.h':
         MLX5DV_FLOW_ACTION_FLAGS_ESP_AES_GCM_SPI_STEERING   = 1 << 2
         MLX5DV_FLOW_ACTION_FLAGS_ESP_AES_GCM_FULL_OFFLOAD   = 1 << 3
         MLX5DV_FLOW_ACTION_FLAGS_ESP_AES_GCM_TX_IV_IS_ESN   = 1 << 4
+
+    cdef unsigned long long MLX5DV_RES_TYPE_QP
+    cdef unsigned long long MLX5DV_RES_TYPE_RWQ
+    cdef unsigned long long MLX5DV_RES_TYPE_DBR
+    cdef unsigned long long MLX5DV_RES_TYPE_SRQ
+
+
+_MLX5DV_RES_TYPE_QP = MLX5DV_RES_TYPE_QP
+_MLX5DV_RES_TYPE_RWQ = MLX5DV_RES_TYPE_RWQ
+_MLX5DV_RES_TYPE_DBR = MLX5DV_RES_TYPE_DBR
+_MLX5DV_RES_TYPE_SRQ = MLX5DV_RES_TYPE_SRQ
diff --git a/pyverbs/srq.pyx b/pyverbs/srq.pyx
index 9cad4cafdd83..a60aa5dcb0e5 100755
--- a/pyverbs/srq.pyx
+++ b/pyverbs/srq.pyx
@@ -125,7 +125,7 @@  cdef class SRQ(PyverbsCM):
     def __cinit__(self, object creator not None, object attr not None):
         self.srq = NULL
         self.cq = None
-        if type(creator) == PD:
+        if isinstance(creator, PD):
             self._create_srq(creator, attr)
         elif type(creator) == Context:
             self._create_srq_ex(creator, attr)