Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* Re: [PATCH v2] IB/rdmavt: Fix variable shadowing issue in rvt_create_cq
From: Nathan Chancellor @ 2019-07-10 17:07 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Dennis Dalessandro, Mike Marciniszyn, Doug Ledford,
	Jason Gunthorpe, Kamenee Arumugam, linux-rdma, linux-kernel,
	clang-built-linux, Nick Desaulniers
In-Reply-To: <20190710170322.GA5072@iweiny-DESK2.sc.intel.com>

On Wed, Jul 10, 2019 at 10:03:23AM -0700, Ira Weiny wrote:
> What version of the kernel was this found on?
> 
> I don't see the problem with 5.2.  AFAICS there is no 'err' in the function
> scope and the if scoped 'err' is initialized properly on line 239.
> 
> Ira
> 

$ git describe --contains 239b0e52d8aa
next-20190709~84^2~57

I should probably be better about adding 'PATCH -next' to my patches,
sorry for the confusion!

Cheers,
Nathan

^ permalink raw reply

* Re: [PATCH v2] IB/rdmavt: Fix variable shadowing issue in rvt_create_cq
From: Ira Weiny @ 2019-07-10 17:03 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Dennis Dalessandro, Mike Marciniszyn, Doug Ledford,
	Jason Gunthorpe, Kamenee Arumugam, linux-rdma, linux-kernel,
	clang-built-linux, Nick Desaulniers
In-Reply-To: <20190709230552.61842-1-natechancellor@gmail.com>

On Tue, Jul 09, 2019 at 04:05:53PM -0700, Nathan Chancellor wrote:
> clang warns:
> 
> drivers/infiniband/sw/rdmavt/cq.c:260:7: warning: variable 'err' is used
> uninitialized whenever 'if' condition is true
> [-Wsometimes-uninitialized]
>                 if (err)
>                     ^~~
> drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs
> here
>         return err;
>                ^~~
> drivers/infiniband/sw/rdmavt/cq.c:260:3: note: remove the 'if' if its
> condition is always false
>                 if (err)
>                 ^~~~~~~~
> drivers/infiniband/sw/rdmavt/cq.c:253:7: warning: variable 'err' is used
> uninitialized whenever 'if' condition is true
> [-Wsometimes-uninitialized]
>                 if (!cq->ip) {
>                     ^~~~~~~
> drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs
> here
>         return err;
>                ^~~
> drivers/infiniband/sw/rdmavt/cq.c:253:3: note: remove the 'if' if its
> condition is always false
>                 if (!cq->ip) {
>                 ^~~~~~~~~~~~~~
> drivers/infiniband/sw/rdmavt/cq.c:211:9: note: initialize the variable
> 'err' to silence this warning
>         int err;
>                ^
>                 = 0
> 2 warnings generated.

What version of the kernel was this found on?

I don't see the problem with 5.2.  AFAICS there is no 'err' in the function
scope and the if scoped 'err' is initialized properly on line 239.

Ira

> 
> The function scoped err variable is uninitialized when the flow jumps
> into the if statement. The if scoped err variable shadows the function
> scoped err variable, preventing the err assignments within the if
> statement to be reflected at the function level, which will cause
> uninitialized use when the goto statements are taken.
> 
> Just remove the if scoped err declaration so that there is only one
> copy of the err variable for this function.
> 
> Fixes: 239b0e52d8aa ("IB/hfi1: Move rvt_cq_wc struct into uapi directory")
> Link: https://github.com/ClangBuiltLinux/linux/issues/594
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> 
> v1 -> v2:
> 
> * Updated the wording of the commit message to use proper terms like
>   scoping and shadowing, thanks to review from Nick (let me know if the
>   wording isn't up to snuff).
> 
>  drivers/infiniband/sw/rdmavt/cq.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rdmavt/cq.c b/drivers/infiniband/sw/rdmavt/cq.c
> index fac87b13329d..a85571a4cf57 100644
> --- a/drivers/infiniband/sw/rdmavt/cq.c
> +++ b/drivers/infiniband/sw/rdmavt/cq.c
> @@ -247,8 +247,6 @@ int rvt_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
>  	 * See rvt_mmap() for details.
>  	 */
>  	if (udata && udata->outlen >= sizeof(__u64)) {
> -		int err;
> -
>  		cq->ip = rvt_create_mmap_info(rdi, sz, udata, u_wc);
>  		if (!cq->ip) {
>  			err = -ENOMEM;
> -- 
> 2.22.0
> 

^ permalink raw reply

* Re: [PATCH v2] IB/rdmavt: Fix variable shadowing issue in rvt_create_cq
From: Jason Gunthorpe @ 2019-07-10 17:02 UTC (permalink / raw)
  To: Nathan Chancellor, Mike Marciniszyn
  Cc: Dennis Dalessandro, Mike Marciniszyn, Doug Ledford,
	Kamenee Arumugam, linux-rdma, linux-kernel, clang-built-linux,
	Nick Desaulniers
In-Reply-To: <20190709230552.61842-1-natechancellor@gmail.com>

On Tue, Jul 09, 2019 at 04:05:53PM -0700, Nathan Chancellor wrote:
> clang warns:
> 
> drivers/infiniband/sw/rdmavt/cq.c:260:7: warning: variable 'err' is used
> uninitialized whenever 'if' condition is true
> [-Wsometimes-uninitialized]
>                 if (err)
>                     ^~~
> drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs
> here
>         return err;
>                ^~~
> drivers/infiniband/sw/rdmavt/cq.c:260:3: note: remove the 'if' if its
> condition is always false
>                 if (err)
>                 ^~~~~~~~
> drivers/infiniband/sw/rdmavt/cq.c:253:7: warning: variable 'err' is used
> uninitialized whenever 'if' condition is true
> [-Wsometimes-uninitialized]
>                 if (!cq->ip) {
>                     ^~~~~~~
> drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs
> here
>         return err;
>                ^~~
> drivers/infiniband/sw/rdmavt/cq.c:253:3: note: remove the 'if' if its
> condition is always false
>                 if (!cq->ip) {
>                 ^~~~~~~~~~~~~~
> drivers/infiniband/sw/rdmavt/cq.c:211:9: note: initialize the variable
> 'err' to silence this warning
>         int err;
>                ^
>                 = 0
> 2 warnings generated.
> 
> The function scoped err variable is uninitialized when the flow jumps
> into the if statement. The if scoped err variable shadows the function
> scoped err variable, preventing the err assignments within the if
> statement to be reflected at the function level, which will cause
> uninitialized use when the goto statements are taken.
> 
> Just remove the if scoped err declaration so that there is only one
> copy of the err variable for this function.
> 
> Fixes: 239b0e52d8aa ("IB/hfi1: Move rvt_cq_wc struct into uapi directory")
> Link: https://github.com/ClangBuiltLinux/linux/issues/594
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>

Applied to for-next with Mike's ack

Thanks,
Jason

^ permalink raw reply

* Re: [PATCH] [net-next] IB/hfi1: removed shadowed 'err' variable
From: Jason Gunthorpe @ 2019-07-10 16:58 UTC (permalink / raw)
  To: Marciniszyn, Mike
  Cc: Arnd Bergmann, Dalessandro, Dennis, Doug Ledford,
	Arumugam, Kamenee, Shamir Rabinovitch, Gal Pressman,
	Leon Romanovsky, linux-rdma@vger.kernel.org,
	linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com
In-Reply-To: <32E1700B9017364D9B60AED9960492BC70E0B90C@fmsmsx120.amr.corp.intel.com>

On Wed, Jul 10, 2019 at 04:54:33PM +0000, Marciniszyn, Mike wrote:
> > Subject: [PATCH] [net-next] IB/hfi1: removed shadowed 'err' variable
> > 
> > I can't think of any reason for the inner variable declaration, so
> > remove it to avoid the issue.
> > 
> 
> I agree!
> 
> > Fixes: 239b0e52d8aa ("IB/hfi1: Move rvt_cq_wc struct into uapi directory")
> 
> Thanks for catching this!
> 
> Acked-by: Mike Marciniszyn <mike.marciniszyn@intel.com>

Thanks, I'm going to take Nathan's identical patch though, it arrived
first

Jason

^ permalink raw reply

* RE: [PATCH] [net-next] IB/hfi1: removed shadowed 'err' variable
From: Marciniszyn, Mike @ 2019-07-10 16:54 UTC (permalink / raw)
  To: Arnd Bergmann, Dalessandro, Dennis, Doug Ledford, Jason Gunthorpe
  Cc: Arumugam, Kamenee, Jason Gunthorpe, Shamir Rabinovitch,
	Gal Pressman, Leon Romanovsky, linux-rdma@vger.kernel.org,
	linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com
In-Reply-To: <20190710130802.1878874-1-arnd@arndb.de>

> Subject: [PATCH] [net-next] IB/hfi1: removed shadowed 'err' variable
> 
> I can't think of any reason for the inner variable declaration, so
> remove it to avoid the issue.
> 

I agree!

> Fixes: 239b0e52d8aa ("IB/hfi1: Move rvt_cq_wc struct into uapi directory")

Thanks for catching this!

Acked-by: Mike Marciniszyn <mike.marciniszyn@intel.com>

^ permalink raw reply

* Re: [PATCH v4 00/25] InfiniBand Transport (IBTRS) and Network Block Device (IBNBD)
From: Sagi Grimberg @ 2019-07-10 16:25 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Danil Kipnis, Jack Wang, linux-block, linux-rdma, axboe,
	Christoph Hellwig, bvanassche, dledford, Roman Pen, gregkh
In-Reply-To: <20190710135519.GA4051@ziepe.ca>


>> Another question, from what I understand from the code, the client
>> always rdma_writes data on writes (with imm) from a remote pool of
>> server buffers dedicated to it. Essentially all writes are immediate (no
>> rdma reads ever). How is that different than using send wrs to a set of
>> pre-posted recv buffers (like all others are doing)? Is it faster?
> 
> RDMA WRITE only is generally a bit faster, and if you use a buffer
> pool in a smart way it is possible to get very good data packing.

There is no packing, its used exactly as send/recv, but with a remote
buffer pool (pool of 512K buffers) and the client selects one and rdma
write with imm to it.

> With
> SEND the number of recvq entries dictates how big the rx buffer can
> be, or you waste even more memory by using partial send buffers..

This is exactly how it used here.

> A scheme like this seems like a high performance idea, but on the
> other side, I have no idea how you could possibly manage invalidations
> efficiently with a shared RX buffer pool...

There are no invalidations, this remote server pool is registered once
and long lived with the session.

> The RXer has to push out an invalidation for the shared buffer pool
> MR, but we don't have protocols for partial MR invalidation.
> 
> Which is back to my earlier thought that the main reason this perfoms
> better is because it doesn't have synchronous MR invalidation.

This issue only exists on the client side. The server never
invalidates any of its buffers.

> Maybe this is fine, but it needs to be made very clear that it uses
> this insecure operating model to get higher performance..

I still do not understand why this should give any notice-able
performance advantage.

^ permalink raw reply

* Re: [PATCH v4 00/25] InfiniBand Transport (IBTRS) and Network Block Device (IBNBD)
From: Danil Kipnis @ 2019-07-10 14:55 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Jack Wang, linux-block, linux-rdma, axboe, Christoph Hellwig,
	Sagi Grimberg, bvanassche, jgg, dledford, Roman Pen, gregkh
In-Reply-To: <20190709110036.GQ7034@mtr-leonro.mtl.com>

Hi Leon,

thanks for the feedback!

On Tue, Jul 9, 2019 at 1:00 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Tue, Jul 09, 2019 at 11:55:03AM +0200, Danil Kipnis wrote:
> > Hallo Doug, Hallo Jason, Hallo Jens, Hallo Greg,
> >
> > Could you please provide some feedback to the IBNBD driver and the
> > IBTRS library?
> > So far we addressed all the requests provided by the community and
> > continue to maintain our code up-to-date with the upstream kernel
> > while having an extra compatibility layer for older kernels in our
> > out-of-tree repository.
> > I understand that SRP and NVMEoF which are in the kernel already do
> > provide equivalent functionality for the majority of the use cases.
> > IBNBD on the other hand is showing higher performance and more
> > importantly includes the IBTRS - a general purpose library to
> > establish connections and transport BIO-like read/write sg-lists over
> > RDMA, while SRP is targeting SCSI and NVMEoF is addressing NVME. While
> > I believe IBNBD does meet the kernel coding standards, it doesn't have
> > a lot of users, while SRP and NVMEoF are widely accepted. Do you think
> > it would make sense for us to rework our patchset and try pushing it
> > for staging tree first, so that we can proof IBNBD is well maintained,
> > beneficial for the eco-system, find a proper location for it within
> > block/rdma subsystems? This would make it easier for people to try it
> > out and would also be a huge step for us in terms of maintenance
> > effort.
> > The names IBNBD and IBTRS are in fact misleading. IBTRS sits on top of
> > RDMA and is not bound to IB (We will evaluate IBTRS with ROCE in the
> > near future). Do you think it would make sense to rename the driver to
> > RNBD/RTRS?
>
> It is better to avoid "staging" tree, because it will lack attention of
> relevant people and your efforts will be lost once you will try to move
> out of staging. We are all remembering Lustre and don't want to see it
> again.
>
> Back then, you was asked to provide support for performance superiority.

I have only theories of why ibnbd is showing better numbers than nvmeof:
1. The way we utilize the MQ framework in IBNBD. We promise to have
queue_depth (say 512) requests on each of the num_cpus hardware queues
of each device, but in fact we have only queue_depth for the whole
"session" toward a given server. The moment we have queue_depth
inflights we need stop the queue (on a device on a cpu) we get more
requests on. We need to start them again after some requests are
completed. We maintain per cpu lists of stopped HW queues, a bitmap
showing which lists are not empty, etc. to wake them up in a
round-robin fashion to avoid starvation of any devices.
2. We only do rdma writes with imm. A server reserves queue_depth of
max_io_size buffers for a given client. The client manages those
himself. Client uses imm field to tell to the server which buffer has
been written (and where) and server uses the imm field to send back
errno. If our max_io_size is 64K and queue_depth 512 and client only
issues 4K IOs all the time, then 60*512K memory is wasted. On the
other hand we do no buffer allocation/registration in io path on
server side. Server sends rdma addresses and keys to those
preregistered buffers on connection establishment and
deallocates/unregisters them when a session is closed. That's for
writes. For reads, client registers user buffers (after fr) and sends
the addresses and keys to the server (with an rdma write with imm).
Server rdma writes into those buffers. Client does the
unregistering/invalidation and completes the request.

> Can you please share any numbers with us?
Apart from github
(https://github.com/ionos-enterprise/ibnbd/tree/master/performance/v4-v5.2-rc3)
the performance results for v5.2-rc3 on two different systems can be
accessed under dcd.ionos.com/ibnbd-performance-report. The page allows
to filter out test scenarios interesting for comparison.

>
> Thanks

^ permalink raw reply

* Re: [PATCH -next] rdma/siw: Add missing dependencies on LIBCRC32C and DMA_VIRT_OPS
From: Jason Gunthorpe @ 2019-07-10 14:46 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Bernard Metzler, Doug Ledford, linux-rdma, linux-kernel,
	linux-next
In-Reply-To: <20190710133930.26591-1-geert@linux-m68k.org>

On Wed, Jul 10, 2019 at 03:39:30PM +0200, Geert Uytterhoeven wrote:
> If LIBCRC32C and DMA_VIRT_OPS are not enabled:
> 
>     drivers/infiniband/sw/siw/siw_main.o: In function `siw_newlink':
>     siw_main.c:(.text+0x35c): undefined reference to `dma_virt_ops'
>     drivers/infiniband/sw/siw/siw_qp_rx.o: In function `siw_csum_update':
>     siw_qp_rx.c:(.text+0x16): undefined reference to `crc32c'
> 
> Fix the first issue by adding a select of DMA_VIRT_OPS.
> Fix the second issue by replacing the unneeded dependency on
> CRYPTO_CRC32 by a dependency on LIBCRC32C.
> 
> Reported-by: noreply@ellerman.id.au (first issue)
> Fixes: c0cf5bdde46c664d ("rdma/siw: addition to kernel build environment")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>  drivers/infiniband/sw/siw/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/siw/Kconfig b/drivers/infiniband/sw/siw/Kconfig
> index 94f684174ce3556e..b622fc62f2cd6d46 100644
> +++ b/drivers/infiniband/sw/siw/Kconfig
> @@ -1,6 +1,7 @@
>  config RDMA_SIW
>  	tristate "Software RDMA over TCP/IP (iWARP) driver"
> -	depends on INET && INFINIBAND && CRYPTO_CRC32
> +	depends on INET && INFINIBAND && LIBCRC32C

Is this the best practice? 

siw is using both the libcrc32c API and the
'crypto_alloc_shash("crc32c", 0, 0);' version. Is it right to get that
transitively through LIBCRC32C?

Jason

^ permalink raw reply

* [PATCH rdma-core 3/4] pyverbs: Avoid casting pointers to object type
From: Noa Osherovich @ 2019-07-10 14:22 UTC (permalink / raw)
  To: dledford, jgg, leonro; +Cc: linux-rdma, Maxim Chicherin, Maor Gottlieb
In-Reply-To: <20190710142251.9396-1-noaos@mellanox.com>

From: Maxim Chicherin <maximc@mellanox.com>

When casting a C pointer into <object> type python assumes that the
head of the struct is the reference count. The reference count is
incremented every time another variable points to this object.
The result is that the first member of the C struct, which is wrapped
by python object, is incremented. E.g. PD holds ibv_pd pointer, so the
first field of ibv_pd which is ibv_context address is incremented by
1, meaning ibv_context pointer holds invalid address.

Signed-off-by: Maxim Chicherin <maximc@mellanox.com>
Reviewd-by: Maor Gottlieb <maorg@mellanox.com>
---
 pyverbs/cq.pyx | 23 +++--------------------
 pyverbs/pd.pyx |  4 ----
 pyverbs/qp.pyx | 39 +++++++++++++++++++--------------------
 pyverbs/wr.pyx | 17 +++++------------
 4 files changed, 27 insertions(+), 56 deletions(-)

diff --git a/pyverbs/cq.pyx b/pyverbs/cq.pyx
index dd475759337c..dc09924e88a9 100644
--- a/pyverbs/cq.pyx
+++ b/pyverbs/cq.pyx
@@ -59,10 +59,6 @@ cdef class CompChannel(PyverbsCM):
         if isinstance(obj, CQ) or isinstance(obj, CQEX):
             self.cqs.add(obj)
 
-    @property
-    def channel(self):
-        return <object>self.cc
-
 
 cdef class CQ(PyverbsCM):
     """
@@ -84,8 +80,7 @@ cdef class CQ(PyverbsCM):
         """
         if channel is not None:
             self.cq = v.ibv_create_cq(context.context, cqe, <void*>cq_context,
-                                      <v.ibv_comp_channel*>channel.channel,
-                                      comp_vector)
+                                      channel.cc, comp_vector)
             channel.add_ref(self)
         else:
             self.cq = v.ibv_create_cq(context.context, cqe, <void*>cq_context,
@@ -162,10 +157,6 @@ cdef class CQ(PyverbsCM):
         """
         v.ibv_ack_cq_events(self.cq, num_events)
 
-    @property
-    def _cq(self):
-        return <object>self.cq
-
     def __str__(self):
         print_format = '{:22}: {:<20}\n'
         return 'CQ\n' +\
@@ -215,9 +206,9 @@ cdef class CqInitAttrEx(PyverbsObject):
     def comp_channel(self):
         return self.channel
     @comp_channel.setter
-    def comp_channel(self, val):
+    def comp_channel(self, CompChannel val):
         self.channel = val
-        self.attr.channel = <v.ibv_comp_channel*>val
+        self.attr.channel = val.cc
 
     @property
     def comp_vector(self):
@@ -364,14 +355,6 @@ cdef class CQEX(PyverbsCM):
     def wr_id(self, val):
         self.cq.wr_id = val
 
-    @property
-    def _cq(self):
-        return <object>self.cq
-
-    @property
-    def _ibv_cq(self):
-        return <object>self.ibv_cq
-
     def __str__(self):
         print_format = '{:<22}: {:<20}\n'
         return 'Extended CQ:\n' +\
diff --git a/pyverbs/pd.pyx b/pyverbs/pd.pyx
index 4b5dc139c59f..7cd0876682b2 100644
--- a/pyverbs/pd.pyx
+++ b/pyverbs/pd.pyx
@@ -67,7 +67,3 @@ cdef class PD(PyverbsCM):
             self.qps.add(obj)
         else:
             raise PyverbsError('Unrecognized object type')
-
-    @property
-    def _pd(self):
-        return <object>self.pd
diff --git a/pyverbs/qp.pyx b/pyverbs/qp.pyx
index b33993a106e9..47a2158a5acc 100644
--- a/pyverbs/qp.pyx
+++ b/pyverbs/qp.pyx
@@ -101,9 +101,9 @@ cdef class QPInitAttr(PyverbsObject):
         self.attr.qp_context = <void*>qp_context
         if scq is not None:
             if type(scq) is CQ:
-                self.attr.send_cq = <v.ibv_cq*>scq._cq
+                self.attr.send_cq = (<CQ>rcq).cq
             elif type(scq) is CQEX:
-                self.attr.send_cq = <v.ibv_cq*>scq._ibv_cq
+                self.attr.send_cq = (<CQEX>rcq).ibv_cq
             else:
                 raise PyverbsUserError('Expected CQ/CQEX, got {t}'.\
                                        format(t=type(scq)))
@@ -111,9 +111,9 @@ cdef class QPInitAttr(PyverbsObject):
 
         if rcq is not None:
             if type(rcq) is CQ:
-                self.attr.recv_cq = <v.ibv_cq*>rcq._cq
+                self.attr.recv_cq = (<CQ>rcq).cq
             elif type(rcq) is CQEX:
-                self.attr.recv_cq = <v.ibv_cq*>rcq._ibv_cq
+                self.attr.recv_cq = (<CQEX>rcq).ibv_cq
             else:
                 raise PyverbsUserError('Expected CQ/CQEX, got {t}'.\
                                        format(t=type(rcq)))
@@ -129,9 +129,9 @@ cdef class QPInitAttr(PyverbsObject):
     @send_cq.setter
     def send_cq(self, val):
         if type(val) is CQ:
-            self.attr.send_cq = <v.ibv_cq*>val._cq
+            self.attr.send_cq = (<CQ>val).cq
         elif type(val) is CQEX:
-            self.attr.send_cq = <v.ibv_cq*>val._ibv_cq
+            self.attr.send_cq = (<CQEX>val).ibv_cq
         self.scq = val
 
     @property
@@ -140,9 +140,9 @@ cdef class QPInitAttr(PyverbsObject):
     @recv_cq.setter
     def recv_cq(self, val):
         if type(val) is CQ:
-            self.attr.recv_cq = <v.ibv_cq*>val._cq
+            self.attr.recv_cq = (<CQ>val).cq
         elif type(val) is CQEX:
-            self.attr.recv_cq = <v.ibv_cq*>val._ibv_cq
+            self.attr.recv_cq = (<CQEX>val).ibv_cq
         self.rcq = val
 
     @property
@@ -218,9 +218,9 @@ cdef class QPInitAttrEx(PyverbsObject):
         _copy_caps(cap, self)
         if scq is not None:
             if type(scq) is CQ:
-                self.attr.send_cq = <v.ibv_cq*>scq._cq
+                self.attr.send_cq = (<CQ>rcq).cq
             elif type(scq) is CQEX:
-                self.attr.send_cq = <v.ibv_cq*>scq._ibv_cq
+                self.attr.send_cq = (<CQEX>rcq).ibv_cq
             else:
                 raise PyverbsUserError('Expected CQ/CQEX, got {t}'.\
                                        format(t=type(scq)))
@@ -228,12 +228,12 @@ cdef class QPInitAttrEx(PyverbsObject):
 
         if rcq is not None:
             if type(rcq) is CQ:
-                self.attr.recv_cq = <v.ibv_cq*>rcq._cq
+                self.attr.recv_cq = (<CQ>rcq).cq
             elif type(rcq) is CQEX:
-                self.attr.recv_cq = <v.ibv_cq*>rcq._ibv_cq
+                self.attr.recv_cq = (<CQEX>rcq).ibv_cq
             else:
                 raise PyverbsUserError('Expected CQ/CQEX, got {t}'.\
-                                       format(type(rcq)))
+                                       format(t=type(rcq)))
         self.rcq = rcq
 
         self.attr.srq = NULL  # Until SRQ support is added
@@ -247,7 +247,6 @@ cdef class QPInitAttrEx(PyverbsObject):
             raise PyverbsUserError('XRCD and RSS are not yet supported in pyverbs')
         self.attr.comp_mask = comp_mask
         if pd is not None:
-            self.attr.pd = <v.ibv_pd*>pd._pd
             self.pd = pd
         self.attr.create_flags = create_flags
         self.attr.max_tso_header = max_tso_header
@@ -259,9 +258,9 @@ cdef class QPInitAttrEx(PyverbsObject):
     @send_cq.setter
     def send_cq(self, val):
         if type(val) is CQ:
-            self.attr.send_cq = <v.ibv_cq*>val._cq
+            self.attr.send_cq = (<CQ>val).cq
         elif type(val) is CQEX:
-            self.attr.send_cq = <v.ibv_cq*>val._ibv_cq
+            self.attr.send_cq = (<CQEX>val).ibv_cq
         self.scq = val
 
     @property
@@ -270,9 +269,9 @@ cdef class QPInitAttrEx(PyverbsObject):
     @recv_cq.setter
     def recv_cq(self, val):
         if type(val) is CQ:
-            self.attr.recv_cq = <v.ibv_cq*>val._cq
+            self.attr.recv_cq = (<CQ>val).cq
         elif type(val) is CQEX:
-            self.attr.recv_cq = <v.ibv_cq*>val._ibv_cq
+            self.attr.recv_cq = (<CQEX>val).ibv_cq
         self.rcq = val
 
     @property
@@ -311,8 +310,8 @@ cdef class QPInitAttrEx(PyverbsObject):
     def pd(self):
         return self.pd
     @pd.setter
-    def pd(self, val):
-        self.attr.pd = <v.ibv_pd*>val._pd
+    def pd(self, PD val):
+        self.attr.pd = <v.ibv_pd*>val.pd
         self.pd = val
 
     @property
diff --git a/pyverbs/wr.pyx b/pyverbs/wr.pyx
index 2dc766282db3..3003224a8f32 100644
--- a/pyverbs/wr.pyx
+++ b/pyverbs/wr.pyx
@@ -53,10 +53,7 @@ cdef class SGE(PyverbsCM):
         cdef char *sg_data
         cdef int off = offset
         sg_data = <char*>(self.sge.addr + off)
-        return <object>sg_data[:length]
-
-    def _get_sge(self):
-        return <object>self.sge
+        return sg_data[:length]
 
     def __str__(self):
         print_format = '{:22}: {:<20}\n'
@@ -104,7 +101,7 @@ cdef class RecvWR(PyverbsCM):
         if self.recv_wr.sg_list == NULL:
             raise PyverbsRDMAErrno('Failed to malloc SG buffer')
         dst = self.recv_wr.sg_list
-        copy_sg_array(<object>dst, sg, num_sge)
+        copy_sg_array(dst, sg, num_sge)
         self.recv_wr.num_sge = num_sge
         self.recv_wr.wr_id = wr_id
         if next_wr is not None:
@@ -166,7 +163,7 @@ cdef class SendWR(PyverbsCM):
         if self.send_wr.sg_list == NULL:
             raise PyverbsRDMAErrno('Failed to malloc SG buffer')
         dst = self.send_wr.sg_list
-        copy_sg_array(<object>dst, sg, num_sge)
+        copy_sg_array(dst, sg, num_sge)
         self.send_wr.num_sge = num_sge
         self.send_wr.wr_id = wr_id
         if next_wr is not None:
@@ -298,13 +295,9 @@ def send_flags_to_str(flags):
     return flags_str
 
 
-cdef copy_sg_array(dst_obj, sg, num_sge):
-    cdef v.ibv_sge *dst = <v.ibv_sge*>dst_obj
+cdef copy_sg_array(v.ibv_sge *dst, sg, num_sge):
     cdef v.ibv_sge *src
     for i in range(num_sge):
-        # Avoid 'storing unsafe C derivative of temporary Python' errors
-        # that will occur if we merge the two following lines.
-        tmp = sg[i]._get_sge()
-        src = <v.ibv_sge*>tmp
+        src = (<SGE>sg[i]).sge
         memcpy(dst, src, sizeof(v.ibv_sge))
         dst += 1
-- 
2.21.0


^ permalink raw reply related

* [PATCH rdma-core 0/4] pyverbs fixes
From: Noa Osherovich @ 2019-07-10 14:22 UTC (permalink / raw)
  To: dledford, jgg, leonro; +Cc: linux-rdma, Noa Osherovich

The following is a series of fixes to pyverbs:
The first two patches clean the compilation warning introduces in
Fedora 30.
The third patch fixes a wrong casting.
The last one fixes the handling of bad work requests in post send and
post recv operations.

Maxim Chicherin (1):
  pyverbs: Avoid casting pointers to object type

Noa Osherovich (3):
  pyverbs: Fix Cython future warning during build
  build: Remove warning-causing compilation flag from pyverbs
  pyverbs: Fix assignments of bad work requests

 CMakeLists.txt                   |  7 +++++
 buildlib/pyverbs_functions.cmake |  7 ++++-
 pyverbs/addr.pxd                 |  2 ++
 pyverbs/base.pxd                 |  2 ++
 pyverbs/cq.pxd                   |  3 ++
 pyverbs/cq.pyx                   | 23 ++------------
 pyverbs/device.pxd               |  2 ++
 pyverbs/libibverbs_enums.pxd     |  3 ++
 pyverbs/mr.pxd                   |  2 ++
 pyverbs/pd.pxd                   |  3 ++
 pyverbs/pd.pyx                   |  4 ---
 pyverbs/qp.pxd                   |  3 ++
 pyverbs/qp.pyx                   | 54 ++++++++++++++++++--------------
 pyverbs/wr.pxd                   |  2 ++
 pyverbs/wr.pyx                   | 17 +++-------
 15 files changed, 73 insertions(+), 61 deletions(-)

-- 
2.21.0


^ permalink raw reply

* [PATCH rdma-core 1/4] pyverbs: Fix Cython future warning during build
From: Noa Osherovich @ 2019-07-10 14:22 UTC (permalink / raw)
  To: dledford, jgg, leonro; +Cc: linux-rdma, Noa Osherovich, Maor Gottlieb
In-Reply-To: <20190710142251.9396-1-noaos@mellanox.com>

Since Cython 0.29, a language_level directive is required in pxd
files. This currently appears as a FutureWarning e.g.:
FutureWarning: Cython directive 'language_level' not set, using 2 for
now (Py2). This will change in a later release!

This patch updates pyverbs' pxd files with this directive.

Signed-off-by: Noa Osherovich <noaos@mellanox.com>
Reviewd-by: Maor Gottlieb <maorg@mellanox.com>
---
 pyverbs/addr.pxd             | 2 ++
 pyverbs/base.pxd             | 2 ++
 pyverbs/cq.pxd               | 3 +++
 pyverbs/device.pxd           | 2 ++
 pyverbs/libibverbs_enums.pxd | 3 +++
 pyverbs/mr.pxd               | 2 ++
 pyverbs/pd.pxd               | 3 +++
 pyverbs/qp.pxd               | 3 +++
 pyverbs/wr.pxd               | 2 ++
 9 files changed, 22 insertions(+)

diff --git a/pyverbs/addr.pxd b/pyverbs/addr.pxd
index 389c2d5bdb2e..e7322e8d7fdd 100644
--- a/pyverbs/addr.pxd
+++ b/pyverbs/addr.pxd
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
 # Copyright (c) 2018, Mellanox Technologies. All rights reserved. See COPYING file
 
+#cython: language_level=3
+
 from .base cimport PyverbsObject, PyverbsCM
 from pyverbs cimport libibverbs as v
 
diff --git a/pyverbs/base.pxd b/pyverbs/base.pxd
index fa661edb5315..e85f7c020e1c 100644
--- a/pyverbs/base.pxd
+++ b/pyverbs/base.pxd
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
 # Copyright (c) 2019, Mellanox Technologies. All rights reserved.
 
+#cython: language_level=3
+
 cdef class PyverbsObject(object):
     cdef object __weakref__
     cdef object logger
diff --git a/pyverbs/cq.pxd b/pyverbs/cq.pxd
index 0e3bcdfffb7e..9b8df5dcae39 100644
--- a/pyverbs/cq.pxd
+++ b/pyverbs/cq.pxd
@@ -1,5 +1,8 @@
 # SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
 # Copyright (c) 2019, Mellanox Technologies. All rights reserved.
+
+#cython: language_level=3
+
 from pyverbs.base cimport PyverbsObject, PyverbsCM
 cimport pyverbs.libibverbs as v
 
diff --git a/pyverbs/device.pxd b/pyverbs/device.pxd
index 3cb52bde4603..44c8bc3cbcbc 100644
--- a/pyverbs/device.pxd
+++ b/pyverbs/device.pxd
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
 # Copyright (c) 2018, Mellanox Technologies. All rights reserved. See COPYING file
 
+#cython: language_level=3
+
 from .base cimport PyverbsObject, PyverbsCM
 cimport pyverbs.libibverbs as v
 
diff --git a/pyverbs/libibverbs_enums.pxd b/pyverbs/libibverbs_enums.pxd
index 85b5092c486f..c347ef31dd2b 100644
--- a/pyverbs/libibverbs_enums.pxd
+++ b/pyverbs/libibverbs_enums.pxd
@@ -1,6 +1,9 @@
 # SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
 # Copyright (c) 2018, Mellanox Technologies. All rights reserved.
 
+#cython: language_level=3
+
+
 cdef extern from '<infiniband/verbs.h>':
 
     cpdef enum ibv_transport_type:
diff --git a/pyverbs/mr.pxd b/pyverbs/mr.pxd
index 2d76f2dfbe7c..fb46611e6f42 100644
--- a/pyverbs/mr.pxd
+++ b/pyverbs/mr.pxd
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
 # Copyright (c) 2019, Mellanox Technologies. All rights reserved. See COPYING file
 
+#cython: language_level=3
+
 from pyverbs.base cimport PyverbsCM
 from . cimport libibverbs as v
 
diff --git a/pyverbs/pd.pxd b/pyverbs/pd.pxd
index 07c9158b27eb..e0861b301b7c 100644
--- a/pyverbs/pd.pxd
+++ b/pyverbs/pd.pxd
@@ -1,5 +1,8 @@
 # SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
 # Copyright (c) 2019, Mellanox Technologies. All rights reserved.
+
+#cython: language_level=3
+
 from pyverbs.device cimport Context
 cimport pyverbs.libibverbs as v
 from .base cimport PyverbsCM
diff --git a/pyverbs/qp.pxd b/pyverbs/qp.pxd
index d85bc28992ad..29b9ec4a0221 100644
--- a/pyverbs/qp.pxd
+++ b/pyverbs/qp.pxd
@@ -1,5 +1,8 @@
 # SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
 # Copyright (c) 2019 Mellanox Technologies, Inc. All rights reserved.
+
+#cython: language_level=3
+
 from pyverbs.base cimport PyverbsObject, PyverbsCM
 cimport pyverbs.libibverbs as v
 
diff --git a/pyverbs/wr.pxd b/pyverbs/wr.pxd
index 64b16091116a..e259249ef7f8 100644
--- a/pyverbs/wr.pxd
+++ b/pyverbs/wr.pxd
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
 # Copyright (c) 2019 Mellanox Technologies, Inc. All rights reserved. See COPYING file
 
+#cython: language_level=3
+
 from .base cimport PyverbsCM
 from pyverbs cimport libibverbs as v
 
-- 
2.21.0


^ permalink raw reply related

* [PATCH rdma-core 4/4] pyverbs: Fix assignments of bad work requests
From: Noa Osherovich @ 2019-07-10 14:22 UTC (permalink / raw)
  To: dledford, jgg, leonro; +Cc: linux-rdma, Noa Osherovich, Maor Gottlieb
In-Reply-To: <20190710142251.9396-1-noaos@mellanox.com>

The bad work request content wasn't properly copied from the C
object, fix copying to contain the bad work request data.

Signed-off-by: Noa Osherovich <noaos@mellanox.com>
Reviewd-by: Maor Gottlieb <maorg@mellanox.com>
---
 pyverbs/qp.pyx | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/pyverbs/qp.pyx b/pyverbs/qp.pyx
index 47a2158a5acc..ad89c94c002e 100644
--- a/pyverbs/qp.pyx
+++ b/pyverbs/qp.pyx
@@ -14,6 +14,9 @@ cimport pyverbs.libibverbs as v
 from pyverbs.pd cimport PD
 
 
+cdef extern from 'string.h':
+    void *memcpy(void *dest, const void *src, size_t n);
+
 cdef class QPCap(PyverbsObject):
     def __cinit__(self, max_send_wr=1, max_recv_wr=10, max_send_sge=1,
                       max_recv_sge=1, max_inline_data=0):
@@ -963,10 +966,12 @@ cdef class QP(PyverbsCM):
         :return: None
         """
         cdef v.ibv_recv_wr *my_bad_wr
+        # In order to provide a pointer to a pointer, use a temporary cdef'ed
+        # variable.
         rc = v.ibv_post_recv(self.qp, &wr.recv_wr, &my_bad_wr)
         if rc != 0:
-            if bad_wr is not None:
-                bad_wr.wr = <object>my_bad_wr
+            if (bad_wr):
+                memcpy(&bad_wr.recv_wr, my_bad_wr, sizeof(bad_wr.recv_wr))
             raise PyverbsRDMAErrno('Failed to post recv (returned {rc})'.
                                    format(rc=rc))
 
@@ -978,11 +983,13 @@ cdef class QP(PyverbsCM):
                        case of a failure
         :return: None
         """
+        # In order to provide a pointer to a pointer, use a temporary cdef'ed
+        # variable.
         cdef v.ibv_send_wr *my_bad_wr
         rc = v.ibv_post_send(self.qp, &wr.send_wr, &my_bad_wr)
         if rc != 0:
-            if bad_wr is not None:
-                bad_wr.wr = <object>my_bad_wr
+            if (bad_wr):
+                memcpy(&bad_wr.send_wr, my_bad_wr, sizeof(bad_wr.send_wr))
             raise PyverbsRDMAErrno('Failed to post send (returned {rc})'.
                                    format(rc=rc))
 
-- 
2.21.0


^ permalink raw reply related

* [PATCH rdma-core 2/4] build: Remove warning-causing compilation flag from pyverbs
From: Noa Osherovich @ 2019-07-10 14:22 UTC (permalink / raw)
  To: dledford, jgg, leonro; +Cc: linux-rdma, Noa Osherovich
In-Reply-To: <20190710142251.9396-1-noaos@mellanox.com>

The -fvar-tracking-assignment flag is causing the following
compilation warning:

note: variable tracking size limit exceeded with '-fvar-tracking-assignments', retrying without

Since it's a debug flag and not necessary for pyverbs functionality,
remove it from pyverbs' build.

Signed-off-by: Noa Osherovich <noaos@mellanox.com>
Reviewd-by: Leon Romanovsky <leonro@mellanox.com>
---
 CMakeLists.txt                   | 7 +++++++
 buildlib/pyverbs_functions.cmake | 7 ++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8d2e357c78af..f2cb5c306c04 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -182,6 +182,13 @@ endif()
 
 #-------------------------
 # Setup the basic C compiler
+# Some compilation flags are not supported in clang, lets allow users to know
+# whether gcc or clang is used.
+if ("${CMAKE_C_COMPILER_ID}" STREQUAL "Clang")
+  set(IS_CLANG_BUILD TRUE)
+else()
+  set(IS_CLANG_BUILD FALSE)
+endif()
 RDMA_BuildType()
 include_directories(${BUILD_INCLUDE})
 
diff --git a/buildlib/pyverbs_functions.cmake b/buildlib/pyverbs_functions.cmake
index 1966cf3ba1a3..81cdd86fc020 100644
--- a/buildlib/pyverbs_functions.cmake
+++ b/buildlib/pyverbs_functions.cmake
@@ -16,8 +16,13 @@ function(rdma_cython_module PY_MODULE)
 
     string(REGEX REPLACE "\\.so$" "" SONAME "${FILENAME}${CMAKE_PYTHON_SO_SUFFIX}")
     add_library(${SONAME} SHARED ${CFILE})
+    # We need to disable -fvar-tracking-assignments. It's only supported in gcc
+    # so make sure we're not using clang before doing that.
+    if (NOT ${IS_CLANG_BUILD})
+      set(PYVERBS_DEBUG_FLAGS "-fno-var-tracking-assignments")
+    endif()
     set_target_properties(${SONAME} PROPERTIES
-	    COMPILE_FLAGS "${CMAKE_C_FLAGS} -fPIC -fno-strict-aliasing -Wno-unused-function -Wno-redundant-decls -Wno-shadow -Wno-cast-function-type -Wno-implicit-fallthrough -Wno-unknown-warning -Wno-unknown-warning-option"
+        COMPILE_FLAGS "${CMAKE_C_FLAGS} -fPIC -fno-strict-aliasing -Wno-unused-function -Wno-redundant-decls -Wno-shadow -Wno-cast-function-type -Wno-implicit-fallthrough -Wno-unknown-warning -Wno-unknown-warning-option ${PYVERBS_DEBUG_FLAGS}"
       LIBRARY_OUTPUT_DIRECTORY "${BUILD_PYTHON}/${PY_MODULE}"
       PREFIX "")
     target_link_libraries(${SONAME} LINK_PRIVATE ${PYTHON_LIBRARIES} ibverbs)
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH] [net-next] net/mlx5e: avoid uninitialized variable use
From: Tariq Toukan @ 2019-07-10 14:22 UTC (permalink / raw)
  To: Arnd Bergmann, Saeed Mahameed, Leon Romanovsky, David S. Miller
  Cc: Tariq Toukan, Eran Ben Elisha, Boris Pismenny,
	netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com
In-Reply-To: <20190710130638.1846846-1-arnd@arndb.de>



On 7/10/2019 4:06 PM, Arnd Bergmann wrote:
> clang points to a variable being used in an unexpected
> code path:
> 
> drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c:251:2: warning: variable 'rec_seq_sz' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
>          default:
>          ^~~~~~~
> drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c:255:46: note: uninitialized use occurs here
>          skip_static_post = !memcmp(rec_seq, &rn_be, rec_seq_sz);
>                                                      ^~~~~~~~~~
> 
>  From looking at the function logic, it seems that there is no
> sensible way to continue here, so just return early and hope
> for the best.
> 
> Fixes: d2ead1f360e8 ("net/mlx5e: Add kTLS TX HW offload support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
> index 3f5f4317a22b..5c08891806f0 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
> @@ -250,6 +250,7 @@ tx_post_resync_params(struct mlx5e_txqsq *sq,
>   	}
>   	default:
>   		WARN_ON(1);
> +		return;
>   	}
>   
>   	skip_static_post = !memcmp(rec_seq, &rn_be, rec_seq_sz);
> 

Reviewed-by: Tariq Toukan <tariqt@mellanox.com>

Thanks!

^ permalink raw reply

* Re: [PATCH v4 00/25] InfiniBand Transport (IBTRS) and Network Block Device (IBNBD)
From: Jason Gunthorpe @ 2019-07-10 13:55 UTC (permalink / raw)
  To: Sagi Grimberg
  Cc: Danil Kipnis, Jack Wang, linux-block, linux-rdma, axboe,
	Christoph Hellwig, bvanassche, dledford, Roman Pen, gregkh
In-Reply-To: <a8f2f1d2-b5d9-92fc-40c8-090af0487723@grimberg.me>

On Tue, Jul 09, 2019 at 12:45:57PM -0700, Sagi Grimberg wrote:

> Another question, from what I understand from the code, the client
> always rdma_writes data on writes (with imm) from a remote pool of
> server buffers dedicated to it. Essentially all writes are immediate (no
> rdma reads ever). How is that different than using send wrs to a set of
> pre-posted recv buffers (like all others are doing)? Is it faster?

RDMA WRITE only is generally a bit faster, and if you use a buffer
pool in a smart way it is possible to get very good data packing. With
SEND the number of recvq entries dictates how big the rx buffer can
be, or you waste even more memory by using partial send buffers..

A scheme like this seems like a high performance idea, but on the
other side, I have no idea how you could possibly manage invalidations
efficiently with a shared RX buffer pool...

The RXer has to push out an invalidation for the shared buffer pool
MR, but we don't have protocols for partial MR invalidation.

Which is back to my earlier thought that the main reason this perfoms
better is because it doesn't have synchronous MR invalidation.

Maybe this is fine, but it needs to be made very clear that it uses
this insecure operating model to get higher performance..

Jason

^ permalink raw reply

* [PATCH -next] rdma/siw: Add missing dependencies on LIBCRC32C and DMA_VIRT_OPS
From: Geert Uytterhoeven @ 2019-07-10 13:39 UTC (permalink / raw)
  To: Bernard Metzler, Doug Ledford, Jason Gunthorpe
  Cc: linux-rdma, linux-kernel, linux-next, Geert Uytterhoeven

If LIBCRC32C and DMA_VIRT_OPS are not enabled:

    drivers/infiniband/sw/siw/siw_main.o: In function `siw_newlink':
    siw_main.c:(.text+0x35c): undefined reference to `dma_virt_ops'
    drivers/infiniband/sw/siw/siw_qp_rx.o: In function `siw_csum_update':
    siw_qp_rx.c:(.text+0x16): undefined reference to `crc32c'

Fix the first issue by adding a select of DMA_VIRT_OPS.
Fix the second issue by replacing the unneeded dependency on
CRYPTO_CRC32 by a dependency on LIBCRC32C.

Reported-by: noreply@ellerman.id.au (first issue)
Fixes: c0cf5bdde46c664d ("rdma/siw: addition to kernel build environment")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/infiniband/sw/siw/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/siw/Kconfig b/drivers/infiniband/sw/siw/Kconfig
index 94f684174ce3556e..b622fc62f2cd6d46 100644
--- a/drivers/infiniband/sw/siw/Kconfig
+++ b/drivers/infiniband/sw/siw/Kconfig
@@ -1,6 +1,7 @@
 config RDMA_SIW
 	tristate "Software RDMA over TCP/IP (iWARP) driver"
-	depends on INET && INFINIBAND && CRYPTO_CRC32
+	depends on INET && INFINIBAND && LIBCRC32C
+	select DMA_VIRT_OPS
 	help
 	This driver implements the iWARP RDMA transport over
 	the Linux TCP/IP network stack. It enables a system with a
-- 
2.17.1


^ permalink raw reply related

* Re: [rdma 14/16] RDMA/irdma: Add ABI definitions
From: Jason Gunthorpe @ 2019-07-10 13:32 UTC (permalink / raw)
  To: Henry Orosco
  Cc: Saleem, Shiraz, Leon Romanovsky, Kirsher, Jeffrey T,
	dledford@redhat.com, davem@davemloft.net, Ismail, Mustafa,
	linux-rdma@vger.kernel.org, netdev@vger.kernel.org,
	nhorman@redhat.com, sassmann@redhat.com, poswald@suse.com,
	Ertman, David M
In-Reply-To: <20190709205613.GA7440@horosco-MOBL2.amr.corp.intel.com>

On Tue, Jul 09, 2019 at 03:56:13PM -0500, Henry Orosco wrote:
> On Mon, Jul 08, 2019 at 02:13:39PM +0000, Jason Gunthorpe wrote:
> > On Sat, Jul 06, 2019 at 04:15:20PM +0000, Saleem, Shiraz wrote:
> > > > Subject: Re: [rdma 14/16] RDMA/irdma: Add ABI definitions
> > > > 
> > > > On Fri, Jul 05, 2019 at 04:42:19PM +0000, Saleem, Shiraz wrote:
> > > > > > Subject: Re: [rdma 14/16] RDMA/irdma: Add ABI definitions
> > > > > >
> > > > > > On Thu, Jul 04, 2019 at 10:40:21AM +0300, Leon Romanovsky wrote:
> > > > > > > On Wed, Jul 03, 2019 at 07:12:57PM -0700, Jeff Kirsher wrote:
> > > > > > > > From: Mustafa Ismail <mustafa.ismail@intel.com>
> > > > > > > >
> > > > > > > > Add ABI definitions for irdma.
> > > > > > > >
> > > > > > > > Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com>
> > > > > > > > Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > > > > > > > include/uapi/rdma/irdma-abi.h | 130
> > > > > > > > ++++++++++++++++++++++++++++++++++
> > > > > > > >  1 file changed, 130 insertions(+)  create mode 100644
> > > > > > > > include/uapi/rdma/irdma-abi.h
> > > > > > > >
> > > > > > > > diff --git a/include/uapi/rdma/irdma-abi.h
> > > > > > > > b/include/uapi/rdma/irdma-abi.h new file mode 100644 index
> > > > > > > > 000000000000..bdfbda4c829e
> > > > > > > > +++ b/include/uapi/rdma/irdma-abi.h
> > > > > > > > @@ -0,0 +1,130 @@
> > > > > > > > +/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
> > > > > > > > +/* Copyright (c) 2006 - 2019 Intel Corporation.  All rights reserved.
> > > > > > > > + * Copyright (c) 2005 Topspin Communications.  All rights reserved.
> > > > > > > > + * Copyright (c) 2005 Cisco Systems.  All rights reserved.
> > > > > > > > + * Copyright (c) 2005 Open Grid Computing, Inc. All rights reserved.
> > > > > > > > + */
> > > > > > > > +
> > > > > > > > +#ifndef IRDMA_ABI_H
> > > > > > > > +#define IRDMA_ABI_H
> > > > > > > > +
> > > > > > > > +#include <linux/types.h>
> > > > > > > > +
> > > > > > > > +/* irdma must support legacy GEN_1 i40iw kernel
> > > > > > > > + * and user-space whose last ABI ver is 5  */ #define
> > > > > > > > +IRDMA_ABI_VER
> > > > > > > > +6
> > > > > > >
> > > > > > > Can you please elaborate about it more?
> > > > > > > There is no irdma code in RDMA yet, so it makes me wonder why new
> > > > > > > define shouldn't start from 1.
> > > > > >
> > > > > > It is because they are ABI compatible with the current user space,
> > > > > > which raises the question why we even have this confusing header file..
> > > > >
> > > > > It is because we need to support current providers/i40iw user-space.
> > > > > Our user-space patch series will introduce a new provider (irdma)
> > > > > whose ABI ver. is also 6 (capable of supporting X722 and which will
> > > > > work with i40iw driver on older kernels) and removes providers/i40iw from rdma-
> > > > core.
> > > > 
> > > > Why on earth would we do that?
> > > > 
> > > A unified library providers/irdma to go in hand with the driver irdma and uses the ABI header.
> > > It can support the new network device e810 and existing x722 iWARP device. It obsoletes
> > > providers/i40iw and extends its ABI. So why keep providers/i40iw around in rdma-core?
> > 
> > Why rewrite a perfectly good userspace that is compatible with the
> > future and past kernels?
> > 
> > Is there something so wrong with the userspace provider to need this?
> >
> 
> Yes, the issue is that providers/i40iw was never designed to work with a unified driver
> which supports multiple hardware generations.

But Shiraz said it works fine with the new kernel driver.. So what is
actually the problem?

Jason

^ permalink raw reply

* [PATCH] [net-next] IB/hfi1: removed shadowed 'err' variable
From: Arnd Bergmann @ 2019-07-10 13:07 UTC (permalink / raw)
  To: Dennis Dalessandro, Mike Marciniszyn, Doug Ledford,
	Jason Gunthorpe
  Cc: Arnd Bergmann, Kamenee Arumugam, Jason Gunthorpe,
	Shamir Rabinovitch, Gal Pressman, Leon Romanovsky, linux-rdma,
	linux-kernel, clang-built-linux

As clang reports, rvt_create_cq() may return an uninitialized
variable, because the 'err' variable is shadowed by another
local declaration:

drivers/infiniband/sw/rdmavt/cq.c:260:7: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
                if (err)
                    ^~~
drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs here
        return err;
               ^~~
drivers/infiniband/sw/rdmavt/cq.c:260:3: note: remove the 'if' if its condition is always false
                if (err)
                ^~~~~~~~
drivers/infiniband/sw/rdmavt/cq.c:253:7: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
                if (!cq->ip) {
                    ^~~~~~~
drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs here
        return err;
               ^~~
drivers/infiniband/sw/rdmavt/cq.c:253:3: note: remove the 'if' if its condition is always false
                if (!cq->ip) {
                ^~~~~~~~~~~~~~
drivers/infiniband/sw/rdmavt/cq.c:211:9: note: initialize the variable 'err' to silence this warning
        int err;
               ^
                = 0

I can't think of any reason for the inner variable declaration, so
remove it to avoid the issue.

Fixes: 239b0e52d8aa ("IB/hfi1: Move rvt_cq_wc struct into uapi directory")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/infiniband/sw/rdmavt/cq.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/infiniband/sw/rdmavt/cq.c b/drivers/infiniband/sw/rdmavt/cq.c
index fac87b13329d..a85571a4cf57 100644
--- a/drivers/infiniband/sw/rdmavt/cq.c
+++ b/drivers/infiniband/sw/rdmavt/cq.c
@@ -247,8 +247,6 @@ int rvt_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
 	 * See rvt_mmap() for details.
 	 */
 	if (udata && udata->outlen >= sizeof(__u64)) {
-		int err;
-
 		cq->ip = rvt_create_mmap_info(rdi, sz, udata, u_wc);
 		if (!cq->ip) {
 			err = -ENOMEM;
-- 
2.20.0


^ permalink raw reply related

* [PATCH] [net-next] net/mlx5e: avoid uninitialized variable use
From: Arnd Bergmann @ 2019-07-10 13:06 UTC (permalink / raw)
  To: Saeed Mahameed, Leon Romanovsky, David S. Miller
  Cc: Arnd Bergmann, Tariq Toukan, Eran Ben Elisha, Boris Pismenny,
	netdev, linux-rdma, linux-kernel, clang-built-linux

clang points to a variable being used in an unexpected
code path:

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c:251:2: warning: variable 'rec_seq_sz' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
        default:
        ^~~~~~~
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c:255:46: note: uninitialized use occurs here
        skip_static_post = !memcmp(rec_seq, &rn_be, rec_seq_sz);
                                                    ^~~~~~~~~~

From looking at the function logic, it seems that there is no
sensible way to continue here, so just return early and hope
for the best.

Fixes: d2ead1f360e8 ("net/mlx5e: Add kTLS TX HW offload support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
index 3f5f4317a22b..5c08891806f0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
@@ -250,6 +250,7 @@ tx_post_resync_params(struct mlx5e_txqsq *sq,
 	}
 	default:
 		WARN_ON(1);
+		return;
 	}
 
 	skip_static_post = !memcmp(rec_seq, &rn_be, rec_seq_sz);
-- 
2.20.0


^ permalink raw reply related

* Re: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and cookie helper functions
From: Gal Pressman @ 2019-07-10 12:19 UTC (permalink / raw)
  To: Michal Kalderon, ariel.elior, jgg, dledford; +Cc: linux-rdma, davem, netdev
In-Reply-To: <20190709141735.19193-2-michal.kalderon@marvell.com>

On 09/07/2019 17:17, Michal Kalderon wrote:
> Create some common API's for adding entries to a xa_mmap.
> Searching for an entry and freeing one.
> 
> The code was copied from the efa driver almost as is, just renamed
> function to be generic and not efa specific.
> 
> Signed-off-by: Ariel Elior <ariel.elior@marvell.com>
> Signed-off-by: Michal Kalderon <michal.kalderon@marvell.com>

Reviewed-by: Gal Pressman <galpress@amazon.com>

^ permalink raw reply

* Re: [PATCH v6 rdma-next 2/6] RDMA/efa: Use the common mmap_xa helpers
From: Gal Pressman @ 2019-07-10 12:09 UTC (permalink / raw)
  To: Michal Kalderon, ariel.elior, jgg, dledford; +Cc: linux-rdma, davem, netdev
In-Reply-To: <20190709141735.19193-3-michal.kalderon@marvell.com>

On 09/07/2019 17:17, Michal Kalderon wrote:
> Remove the functions related to managing the mmap_xa database.
> This code was copied to the ib_core. Use the common API's instead.
> 
> Signed-off-by: Ariel Elior <ariel.elior@marvell.com>
> Signed-off-by: Michal Kalderon <michal.kalderon@marvell.com>

Thanks Michal,
Acked-by: Gal Pressman <galpress@amazon.com>

^ permalink raw reply

* Re: [PATCH net-next v3] net/mlx5e: Convert single case statement switch statements into if statements
From: Leon Romanovsky @ 2019-07-10  9:31 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Saeed Mahameed, David S. Miller, Boris Pismenny, netdev,
	linux-rdma, linux-kernel, clang-built-linux, Nick Desaulniers
In-Reply-To: <20190710060614.6155-1-natechancellor@gmail.com>

On Tue, Jul 09, 2019 at 11:06:15PM -0700, Nathan Chancellor wrote:
> During the review of commit 1ff2f0fa450e ("net/mlx5e: Return in default
> case statement in tx_post_resync_params"), Leon and Nick pointed out
> that the switch statements can be converted to single if statements
> that return early so that the code is easier to follow.
>
> Suggested-by: Leon Romanovsky <leon@kernel.org>
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---

Thanks again,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

^ permalink raw reply

* Re:  Re: [PATCH] RDMA/siw: Print error code while kthread_create failed
From: Bernard Metzler @ 2019-07-10  8:38 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: YueHaibing, dledford, jgg, linux-kernel, linux-rdma
In-Reply-To: <20190710043554.GA7034@mtr-leonro.mtl.com>

-----"Leon Romanovsky" <leon@kernel.org> wrote: -----

>To: "YueHaibing" <yuehaibing@huawei.com>
>From: "Leon Romanovsky" <leon@kernel.org>
>Date: 07/10/2019 06:36AM
>Cc: bmt@zurich.ibm.com, dledford@redhat.com, jgg@ziepe.ca,
>linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org
>Subject: [EXTERNAL] Re: [PATCH] RDMA/siw: Print error code while
>kthread_create failed
>
>On Wed, Jul 10, 2019 at 09:50:09AM +0800, YueHaibing wrote:
>> In iw_create_tx_threads(), if we failed to create kthread,
>> we should print the 'rv', this fix gcc warning:
>>
>> drivers/infiniband/sw/siw/siw_main.c: In function
>'siw_create_tx_threads':
>> drivers/infiniband/sw/siw/siw_main.c:91:11: warning:
>>  variable 'rv' set but not used [-Wunused-but-set-variable]
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> ---
>>  drivers/infiniband/sw/siw/siw_main.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/sw/siw/siw_main.c
>b/drivers/infiniband/sw/siw/siw_main.c
>> index fd2552a..2a70830d 100644
>> --- a/drivers/infiniband/sw/siw/siw_main.c
>> +++ b/drivers/infiniband/sw/siw/siw_main.c
>> @@ -101,7 +101,8 @@ static int siw_create_tx_threads(void)
>>  		if (IS_ERR(siw_tx_thread[cpu])) {
>>  			rv = PTR_ERR(siw_tx_thread[cpu]);
>>  			siw_tx_thread[cpu] = NULL;
>> -			pr_info("Creating TX thread for CPU %d failed", cpu);
>> +			pr_info("Creating TX thread for CPU%d failed %d\n",
>> +				cpu, rv);
>
>Delete this print together with variable, failure to create kthread
>is basic failure, which affect performance only. The whole kthread
>creation spam in this driver looked suspicious during submission
>and it continues to be.
>
>Thanks

Right, I agree with Leon. Better remove all those printouts. We
already have a warning if we cannot start any thread. Also
stopping those threads is not worth spamming the console. I just
forgot to remove after Leon's comment. Would it be possible
to apply the following?

Thanks a lot!
Bernard.

From e4ca3d4dec86bb5731f8e3cb0cdd01e84b315d80 Mon Sep 17 00:00:00 2001
From: Bernard Metzler <bmt@zurich.ibm.com>
Date: Wed, 10 Jul 2019 10:03:17 +0200
Subject: [PATCH] remove kthread create/destroy printouts

Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com>
---
 drivers/infiniband/sw/siw/siw_main.c  | 4 +---
 drivers/infiniband/sw/siw/siw_qp_tx.c | 4 ----
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_main.c b/drivers/infiniband/sw/siw/siw_main.c
index fd2552a9091d..f55c4e80aea4 100644
--- a/drivers/infiniband/sw/siw/siw_main.c
+++ b/drivers/infiniband/sw/siw/siw_main.c
@@ -88,7 +88,7 @@ static void siw_device_cleanup(struct ib_device *base_dev)
 
 static int siw_create_tx_threads(void)
 {
-	int cpu, rv, assigned = 0;
+	int cpu, assigned = 0;
 
 	for_each_online_cpu(cpu) {
 		/* Skip HT cores */
@@ -99,9 +99,7 @@ static int siw_create_tx_threads(void)
 			kthread_create(siw_run_sq, (unsigned long *)(long)cpu,
 				       "siw_tx/%d", cpu);
 		if (IS_ERR(siw_tx_thread[cpu])) {
-			rv = PTR_ERR(siw_tx_thread[cpu]);
 			siw_tx_thread[cpu] = NULL;
-			pr_info("Creating TX thread for CPU %d failed", cpu);
 			continue;
 		}
 		kthread_bind(siw_tx_thread[cpu], cpu);
diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c b/drivers/infiniband/sw/siw/siw_qp_tx.c
index 2c3d250ee57c..fff02b56d38a 100644
--- a/drivers/infiniband/sw/siw/siw_qp_tx.c
+++ b/drivers/infiniband/sw/siw/siw_qp_tx.c
@@ -1200,8 +1200,6 @@ int siw_run_sq(void *data)
 	init_llist_head(&tx_task->active);
 	init_waitqueue_head(&tx_task->waiting);
 
-	pr_info("Started siw TX thread on CPU %u\n", nr_cpu);
-
 	while (1) {
 		struct llist_node *fifo_list = NULL;
 
@@ -1239,8 +1237,6 @@ int siw_run_sq(void *data)
 			siw_sq_resume(qp);
 		}
 	}
-	pr_info("Stopped siw TX thread on CPU %u\n", nr_cpu);
-
 	return 0;
 }
 
-- 
2.17.2


 



^ permalink raw reply related

* Re: [PATCH v6 rdma-next 0/6] RDMA/qedr: Use the doorbell overflow recovery mechanism for RDMA
From: Gal Pressman @ 2019-07-10  7:32 UTC (permalink / raw)
  To: Michal Kalderon, ariel.elior, jgg, dledford
  Cc: linux-rdma, davem, netdev, sleybo
In-Reply-To: <20190709141735.19193-1-michal.kalderon@marvell.com>

On 09/07/2019 17:17, Michal Kalderon wrote:
> This patch series uses the doorbell overflow recovery mechanism
> introduced in
> commit 36907cd5cd72 ("qed: Add doorbell overflow recovery mechanism")
> for rdma ( RoCE and iWARP )
> 
> The first three patches modify the core code to contain helper
> functions for managing mmap_xa inserting, getting and freeing
> entries. The code was taken almost as is from the efa driver.
> There is still an open discussion on whether we should take
> this even further and make the entire mmap generic. Until a
> decision is made, I only created the database API and modified
> the efa and qedr driver to use it. The doorbell recovery code will be based
> on the common code.
> 
> Efa driver was compile tested only.

For the whole series:
Tested-by: Gal Pressman <galpress@amazon.com>

^ permalink raw reply

* [PATCH iproute2-rc 4/8] rdma: Add rdma statistic counter per-port auto mode support
From: Leon Romanovsky @ 2019-07-10  7:24 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Leon Romanovsky, netdev, David Ahern, Mark Zhang,
	RDMA mailing list
In-Reply-To: <20190710072455.9125-1-leon@kernel.org>

From: Mark Zhang <markz@mellanox.com>

With per-QP statistic counter support, a user is allowed to monitor
specific QPs categories, which are bound to/unbound from counters
dynamically allocated/deallocated.

In per-port "auto" mode, QPs are bound to counters automatically
according to common criteria. For example a per "type"(qp type)
scheme, where in each process all QPs have same qp type are bind
automatically to a single counter.
Currently only "type" (qp type) is supported. Examples:

$ rdma statistic qp set link mlx5_2/1 auto type on
$ rdma statistic qp set link mlx5_2/1 auto off

Signed-off-by: Mark Zhang <markz@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 rdma/stat.c  | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 rdma/utils.c |  1 +
 2 files changed, 88 insertions(+)

diff --git a/rdma/stat.c b/rdma/stat.c
index 0c239851..ad1cc063 100644
--- a/rdma/stat.c
+++ b/rdma/stat.c
@@ -14,12 +14,17 @@ static int stat_help(struct rd *rd)
 	pr_out("       %s statistic OBJECT show\n", rd->filename);
 	pr_out("       %s statistic OBJECT show link [ DEV/PORT_INDEX ] [ FILTER-NAME FILTER-VALUE ]\n", rd->filename);
 	pr_out("       %s statistic OBJECT mode\n", rd->filename);
+	pr_out("       %s statistic OBJECT set COUNTER_SCOPE [DEV/PORT_INDEX] auto {CRITERIA | off}\n", rd->filename);
 	pr_out("where  OBJECT: = { qp }\n");
+	pr_out("       CRITERIA : = { type }\n");
+	pr_out("       COUNTER_SCOPE: = { link | dev }\n");
 	pr_out("Examples:\n");
 	pr_out("       %s statistic qp show\n", rd->filename);
 	pr_out("       %s statistic qp show link mlx5_2/1\n", rd->filename);
 	pr_out("       %s statistic qp mode\n", rd->filename);
 	pr_out("       %s statistic qp mode link mlx5_0\n", rd->filename);
+	pr_out("       %s statistic qp set link mlx5_2/1 auto type on\n", rd->filename);
+	pr_out("       %s statistic qp set link mlx5_2/1 auto off\n", rd->filename);
 
 	return 0;
 }
@@ -381,6 +386,87 @@ static int stat_qp_show(struct rd *rd)
 	return rd_exec_cmd(rd, cmds, "parameter");
 }
 
+static int stat_qp_set_link_auto_sendmsg(struct rd *rd, uint32_t mask)
+{
+	uint32_t seq;
+
+	rd_prepare_msg(rd, RDMA_NLDEV_CMD_STAT_SET,
+		       &seq, (NLM_F_REQUEST | NLM_F_ACK));
+
+	mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
+	mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_PORT_INDEX, rd->port_idx);
+	mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_STAT_RES, RDMA_NLDEV_ATTR_RES_QP);
+	mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_STAT_MODE,
+			 RDMA_COUNTER_MODE_AUTO);
+	mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_STAT_AUTO_MODE_MASK, mask);
+
+	return rd_sendrecv_msg(rd, seq);
+}
+
+static int stat_one_qp_set_link_auto_off(struct rd *rd)
+{
+	return stat_qp_set_link_auto_sendmsg(rd, 0);
+}
+
+static int stat_one_qp_set_auto_type_on(struct rd *rd)
+{
+	return stat_qp_set_link_auto_sendmsg(rd, RDMA_COUNTER_MASK_QP_TYPE);
+}
+
+static int stat_one_qp_set_link_auto_type(struct rd *rd)
+{
+	const struct rd_cmd cmds[] = {
+		{ NULL,		stat_help },
+		{ "on",		stat_one_qp_set_auto_type_on },
+		{ 0 }
+	};
+
+	return rd_exec_cmd(rd, cmds, "parameter");
+}
+
+static int stat_one_qp_set_link_auto(struct rd *rd)
+{
+	const struct rd_cmd cmds[] = {
+		{ NULL,		stat_one_qp_link_get_mode },
+		{ "off",	stat_one_qp_set_link_auto_off },
+		{ "type",	stat_one_qp_set_link_auto_type },
+		{ 0 }
+	};
+
+	return rd_exec_cmd(rd, cmds, "parameter");
+}
+
+static int stat_one_qp_set_link(struct rd *rd)
+{
+	const struct rd_cmd cmds[] = {
+		{ NULL,		stat_one_qp_link_get_mode },
+		{ "auto",	stat_one_qp_set_link_auto },
+		{ 0 }
+	};
+
+	if (!rd->port_idx)
+		return 0;
+
+	return rd_exec_cmd(rd, cmds, "parameter");
+}
+
+static int stat_qp_set_link(struct rd *rd)
+{
+	return rd_exec_link(rd, stat_one_qp_set_link, false);
+}
+
+static int stat_qp_set(struct rd *rd)
+{
+	const struct rd_cmd cmds[] = {
+		{ NULL,		stat_help },
+		{ "link",	stat_qp_set_link },
+		{ "help",	stat_help },
+		{ 0 }
+	};
+
+	return rd_exec_cmd(rd, cmds, "parameter");
+}
+
 static int stat_qp(struct rd *rd)
 {
 	const struct rd_cmd cmds[] =  {
@@ -388,6 +474,7 @@ static int stat_qp(struct rd *rd)
 		{ "show",	stat_qp_show },
 		{ "list",	stat_qp_show },
 		{ "mode",	stat_qp_get_mode },
+		{ "set",	stat_qp_set },
 		{ "help",	stat_help },
 		{ 0 }
 	};
diff --git a/rdma/utils.c b/rdma/utils.c
index 9c885ad7..aed1a3d0 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -445,6 +445,7 @@ static const enum mnl_attr_data_type nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_STAT_HWCOUNTER_ENTRY_VALUE] = MNL_TYPE_U64,
 	[RDMA_NLDEV_ATTR_STAT_MODE] = MNL_TYPE_U32,
 	[RDMA_NLDEV_ATTR_STAT_RES] = MNL_TYPE_U32,
+	[RDMA_NLDEV_ATTR_STAT_AUTO_MODE_MASK] = MNL_TYPE_U32,
 };
 
 int rd_attr_check(const struct nlattr *attr, int *typep)
-- 
2.20.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox