* Re: SR-IOV with mlx4 on ConnectX-2 fails with DMAR errors
From: Joshua McBeth @ 2016-12-14 15:06 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161213190102.GA15119-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Tue, Dec 13, 2016 at 2:01 PM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
>
> On Tue, Dec 13, 2016 at 01:36:42PM -0500, Joshua McBeth wrote:
> > I bisected the kernel between v4.1 and v4.3.1 by booting each build on
> > the SR-IOV host and attempting to "ping x.x.x.x" with x.x.x.x being
> > the IP address assigned to the Infiniband interface of a remote host
> >
> > At 4be90bc's parent the SR-IOV host is able to ping the remote host,
> > but at 4be90bc the SR-IOV host is not able to ping the remote host
> > (destination host unreachable)
>
> Okay, that makes sense
>
> > The DMAR errors occur in both the kernel built at 4be90bc (not passing
> > ping test) and its parent (passing ping test)
>
> Continuing to bisect until you find the commit that introduces the
> DMAR errors would also be helpful, I think.
I will do this when I find some time and report back with the results.
>
>
>
> > Reverting only the commit 4be90bc from a later kernel (4.8.x) does not
> > enable the SR-IOV host to ping the remote host, which to me suggests
> > that another commit after 4be90bc is also causing my test to fail.
>
> Okay, that does not seem too surprising.
>
> Does this make your 4.8 kernel work? If yes, then I suspect mlx4 has
> broken IB_DEVICE_LOCAL_DMA_LKEY with SRIOV.. Leon? mlx5 has this
> broken, doesn't it?
>
With 4.8.1 and the below applied to the SR-IOV host and guest kernels,
SR-IOV functions in both the SR-IOV host and guests and there are no
DMAR errors emitted. The NFS/RDMA client in the guest does not work
on the SR-IOV virtual function with the NFS/RDMA server of the host on
the SR-IOV physical function, but this may be something else I need to
troubleshoot further, as both IPoIB and synthetic RDMA traffic passes
between the guest, host, and remote node just fine. The remote node's
NFS/RDMA client is additionally able to function with the host's
NFS/RDMA server on the SR-IOV physical function.
>
> It would also be very helpful to try and determine what memory the NIC is
> trying to read.. If it is the ipoib packet or some mlx4 internal
> thing.
How can I determine this?
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index 2be4ea0cda9c19..1346924d27691f 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -243,6 +243,8 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,
> atomic_set(&pd->usecnt, 0);
> pd->flags = flags;
>
> + device->attrs.device_cap_flags = 0;
> +
> if (device->attrs.device_cap_flags & IB_DEVICE_LOCAL_DMA_LKEY)
> pd->local_dma_lkey = device->local_dma_lkey;
> else
>
> Jason
Apologies for duplicates, I am resending with subject for threading.
On Tue, Dec 13, 2016 at 2:01 PM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> On Tue, Dec 13, 2016 at 01:36:42PM -0500, Joshua McBeth wrote:
>> I bisected the kernel between v4.1 and v4.3.1 by booting each build on
>> the SR-IOV host and attempting to "ping x.x.x.x" with x.x.x.x being
>> the IP address assigned to the Infiniband interface of a remote host
>>
>> At 4be90bc's parent the SR-IOV host is able to ping the remote host,
>> but at 4be90bc the SR-IOV host is not able to ping the remote host
>> (destination host unreachable)
>
> Okay, that makes sense
>
>> The DMAR errors occur in both the kernel built at 4be90bc (not passing
>> ping test) and its parent (passing ping test)
>
> Continuing to bisect until you find the commit that introduces the
> DMAR errors would also be helpful, I think.
>
>> Reverting only the commit 4be90bc from a later kernel (4.8.x) does not
>> enable the SR-IOV host to ping the remote host, which to me suggests
>> that another commit after 4be90bc is also causing my test to fail.
>
> Okay, that does not seem too surprising.
>
> Does this make your 4.8 kernel work? If yes, then I suspect mlx4 has
> broken IB_DEVICE_LOCAL_DMA_LKEY with SRIOV.. Leon? mlx5 has this
> broken, doesn't it?
>
> It would also be very helpful to try and determine what memory the NIC is
> trying to read.. If it is the ipoib packet or some mlx4 internal
> thing.
>
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index 2be4ea0cda9c19..1346924d27691f 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -243,6 +243,8 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,
> atomic_set(&pd->usecnt, 0);
> pd->flags = flags;
>
> + device->attrs.device_cap_flags = 0;
> +
> if (device->attrs.device_cap_flags & IB_DEVICE_LOCAL_DMA_LKEY)
> pd->local_dma_lkey = device->local_dma_lkey;
> else
>
> Jason
--
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
^ permalink raw reply
* (unknown),
From: Joshua McBeth @ 2016-12-14 14:58 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Jason Gunthorpe
On Tue, Dec 13, 2016 at 2:01 PM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
>
> On Tue, Dec 13, 2016 at 01:36:42PM -0500, Joshua McBeth wrote:
> > I bisected the kernel between v4.1 and v4.3.1 by booting each build on
> > the SR-IOV host and attempting to "ping x.x.x.x" with x.x.x.x being
> > the IP address assigned to the Infiniband interface of a remote host
> >
> > At 4be90bc's parent the SR-IOV host is able to ping the remote host,
> > but at 4be90bc the SR-IOV host is not able to ping the remote host
> > (destination host unreachable)
>
> Okay, that makes sense
>
> > The DMAR errors occur in both the kernel built at 4be90bc (not passing
> > ping test) and its parent (passing ping test)
>
> Continuing to bisect until you find the commit that introduces the
> DMAR errors would also be helpful, I think.
I will do this when I find some time and report back with the results.
>
>
>
> > Reverting only the commit 4be90bc from a later kernel (4.8.x) does not
> > enable the SR-IOV host to ping the remote host, which to me suggests
> > that another commit after 4be90bc is also causing my test to fail.
>
> Okay, that does not seem too surprising.
>
> Does this make your 4.8 kernel work? If yes, then I suspect mlx4 has
> broken IB_DEVICE_LOCAL_DMA_LKEY with SRIOV.. Leon? mlx5 has this
> broken, doesn't it?
>
With 4.8.1 and the below applied to the SR-IOV host and guest kernels,
SR-IOV functions in both the SR-IOV host and guests and there are no
DMAR errors emitted. The NFS/RDMA client in the guest does not work
on the SR-IOV virtual function with the NFS/RDMA server of the host on
the SR-IOV physical function, but this may be something else I need to
troubleshoot further, as both IPoIB and synthetic RDMA traffic passes
between the guest, host, and remote node just fine. The remote node's
NFS/RDMA client is additionally able to function with the host's
NFS/RDMA server on the SR-IOV physical function.
>
> It would also be very helpful to try and determine what memory the NIC is
> trying to read.. If it is the ipoib packet or some mlx4 internal
> thing.
How can I determine this?
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index 2be4ea0cda9c19..1346924d27691f 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -243,6 +243,8 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,
> atomic_set(&pd->usecnt, 0);
> pd->flags = flags;
>
> + device->attrs.device_cap_flags = 0;
> +
> if (device->attrs.device_cap_flags & IB_DEVICE_LOCAL_DMA_LKEY)
> pd->local_dma_lkey = device->local_dma_lkey;
> else
>
> Jason
--
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
^ permalink raw reply
* Re: [PATCH v6 0/9] SELinux support for Infiniband RDMA
From: Paul Moore @ 2016-12-13 22:17 UTC (permalink / raw)
To: Daniel Jurgens
Cc: Stephen Smalley, chrisw@sous-sol.org, eparis@parisplace.org,
dledford@redhat.com, sean.hefty@intel.com,
hal.rosenstock@gmail.com, linux-rdma@vger.kernel.org,
linux-security-module@vger.kernel.org, selinux@tycho.nsa.gov
In-Reply-To: <DB6PR0501MB2422DCC01A577CDA280C2C82C49B0@DB6PR0501MB2422.eurprd05.prod.outlook.com>
On Tue, Dec 13, 2016 at 11:25 AM, Daniel Jurgens <danielj@mellanox.com> wrote:
> On 12/13/2016 9:01 AM, Stephen Smalley wrote:
>> For the LSM/SELinux bits,
>> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
>>
>> Note that there will be a merge conflict on classmap.h due to commits in
>> the selinux next branch, but that should be easy to resolve.
>>
>> We'll need the patches for the selinux userspace and refpolicy.
>
> Thanks Stephen, I need to rebase the user space and do some patch breakup. I'll start on that soon.
Sorry, I haven't had a chance to look at v6, but considering all our
discussions on the previous versions I don't expect any issues from
me. I was hoping for some more generic hooks/controls, but that
doesn't look to be possible given the nature of RDMA. I also want to
mention again the need for tests; we've talked about this in the past
and while it isn't possible to run the tests without IB hardware, I
would like to see us merge tests into the selinux-testsuite so that
those who do have the required h/w available could run the tests.
Assuming we can sort out the SELinux userspace and and tests by the
end of January, I see no reason why this couldn't go in for v4.11.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* RE: [PATCH V2 08/10] i40iw: Control debug error prints using env variable
From: Nikolova, Tatyana E @ 2016-12-13 20:02 UTC (permalink / raw)
To: Leon Romanovsky
Cc: jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
In-Reply-To: <20161210143421.GC2521-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
Hi Leon,
Please see inline.
> -----Original Message-----
> From: Leon Romanovsky [mailto:leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org]
> Sent: Saturday, December 10, 2016 8:34 AM
> To: Nikolova, Tatyana E <tatyana.e.nikolova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org; dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-
> rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> Subject: Re: [PATCH V2 08/10] i40iw: Control debug error prints using env
> variable
>
> On Fri, Dec 09, 2016 at 11:55:02AM -0600, Tatyana Nikolova wrote:
> > From: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >
> > Debug prints for error paths are off by default. User has the option
> > to turn them on by setting environment variable I40IW_DEBUG in
> command
> > line.
> >
> > Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> Hi Tatyana,
>
> This patch duplicates already existing code in most of providers and libraries
> in rdma-core, while two of our main goals for creating this consolidated
> library were simplification for users and reduce code duplication.
>
> It will be very beneficial if you:
> 1. Use and promote general pr_debug(..), srp_tools has nice piece of code,
> to be general code.
[Tatyana Nikolova] The debug/error printing macros available in rdma-core use different mechanisms to report information, for instance, they set/check one or more variables, or they use a bit mask to enable debug level. They also print to different outputs: stderr/stdout, debug files or syslog.
By promoting general code, do you mean that we need to implement common functions and replace all of the currently used debug macros in rdma-core with the common ones?
Which one of the different debug mechanisms should be used?
Please, provide details on what you are asking for.
> 2. Create one and general place for all rdma-core's environment variables.
[Tatyana Nikolova] Are you suggesting a new common header file with defines for all environmental variables in rdma-core?
For example:
# cat rdma-core/util/env_vars.h
#define MLX5_CQE_SIZE "MLX5_CQE_SIZE"
#define MLX5_SCATTER_TO_CQE "MLX5_SCATTER_TO_CQE"
#define MLX5_SRQ_SIGNATURE "MLX5_SRQ_SIGNATURE"
#define MLX5_QP_SIGNATURE "MLX5_QP_SIGNATURE"
#define MLX5_LOCAL_CPUS "MLX5_LOCAL_CPUS"
#define MLX5_STALL_CQ_POLL "MLX5_STALL_CQ_POLL"
#define MLX5_STALL_NUM_LOOP "MLX5_STALL_NUM_LOOP"
Please explain and provide an example if possible.
Thank you,
Tatyana
>
> This infrastructure will allow us in the future create better manual of all
> variables supported and ensure easy change if neeeded.
>
> Thanks
>
> > ---
> > providers/i40iw/i40iw_umain.c | 11 ++++++---
> > providers/i40iw/i40iw_umain.h | 7 ++++++
> > providers/i40iw/i40iw_uverbs.c | 52
> > +++++++++++++++++++++++-------------------
> > 3 files changed, 44 insertions(+), 26 deletions(-)
> >
> > diff --git a/providers/i40iw/i40iw_umain.c
> > b/providers/i40iw/i40iw_umain.c index 1756e65..a204859 100644
> > --- a/providers/i40iw/i40iw_umain.c
> > +++ b/providers/i40iw/i40iw_umain.c
> > @@ -46,7 +46,7 @@
> > #include "i40iw_umain.h"
> > #include "i40iw-abi.h"
> >
> > -unsigned int i40iw_debug_level;
> > +unsigned int i40iw_dbg;
> >
> > #include <sys/types.h>
> > #include <sys/stat.h>
> > @@ -184,7 +184,7 @@ static struct ibv_context
> *i40iw_ualloc_context(struct ibv_device *ibdev, int cm
> > return &iwvctx->ibv_ctx;
> >
> > err_free:
> > - fprintf(stderr, PFX "%s: failed to allocate context for device.\n",
> __func__);
> > + i40iw_debug("failed to allocate context for device.\n");
> > free(iwvctx);
> >
> > return NULL;
> > @@ -216,6 +216,7 @@ static struct ibv_device_ops i40iw_udev_ops = {
> > struct ibv_device *i40iw_driver_init(const char *uverbs_sys_path, int
> > abi_version) {
> > char value[16];
> > + char *env_val;
> > struct i40iw_udevice *dev;
> > unsigned int vendor, device;
> > int i;
> > @@ -236,9 +237,13 @@ struct ibv_device *i40iw_driver_init(const char
> > *uverbs_sys_path, int abi_versio
> >
> > return NULL;
> > found:
> > + env_val = getenv("I40IW_DEBUG");
> > + if (env_val)
> > + i40iw_dbg = atoi(env_val);
> > +
> > dev = malloc(sizeof(*dev));
> > if (!dev) {
> > - fprintf(stderr, PFX "%s: failed to allocate memory for device
> object\n", __func__);
> > + i40iw_debug("failed to allocate memory for device
> object\n");
> > return NULL;
> > }
> >
> > diff --git a/providers/i40iw/i40iw_umain.h
> > b/providers/i40iw/i40iw_umain.h index 13d3da8..889c006 100644
> > --- a/providers/i40iw/i40iw_umain.h
> > +++ b/providers/i40iw/i40iw_umain.h
> > @@ -72,6 +72,13 @@
> > #define I40E_DB_SHADOW_AREA_SIZE 64
> > #define I40E_DB_CQ_OFFSET 0x40
> >
> > +extern unsigned int i40iw_dbg;
> > +#define i40iw_debug(fmt, args...) \
> > +do { \
> > + if (i40iw_dbg) \
> > + fprintf(stderr, PFX "%s: " fmt, __FUNCTION__, ##args); \ }
> while
> > +(0)
> > +
> > enum i40iw_uhca_type {
> > INTEL_i40iw
> > };
> > diff --git a/providers/i40iw/i40iw_uverbs.c
> > b/providers/i40iw/i40iw_uverbs.c index f6d9196..464900b 100644
> > --- a/providers/i40iw/i40iw_uverbs.c
> > +++ b/providers/i40iw/i40iw_uverbs.c
> > @@ -65,7 +65,7 @@ int i40iw_uquery_device(struct ibv_context *context,
> > struct ibv_device_attr *att
> >
> > ret = ibv_cmd_query_device(context, attr, &i40iw_fw_ver, &cmd,
> sizeof(cmd));
> > if (ret) {
> > - fprintf(stderr, PFX "%s: query device failed and returned
> status code: %d\n", __func__, ret);
> > + i40iw_debug("query device failed and returned status code:
> %d\n",
> > +ret);
> > return ret;
> > }
> >
> > @@ -165,7 +165,7 @@ struct ibv_mr *i40iw_ureg_mr(struct ibv_pd *pd,
> void *addr, size_t length, int a
> > if (ibv_cmd_reg_mr(pd, addr, length, (uintptr_t)addr,
> > access, mr, &cmd.ibv_cmd, sizeof(cmd),
> > &resp, sizeof(resp))) {
> > - fprintf(stderr, PFX "%s: Failed to register memory\n",
> __func__);
> > + i40iw_debug("Failed to register memory\n");
> > free(mr);
> > return NULL;
> > }
> > @@ -264,7 +264,7 @@ struct ibv_cq *i40iw_ucreate_cq(struct ibv_context
> *context, int cqe,
> > &iwucq->mr, ®_mr_cmd.ibv_cmd,
> sizeof(reg_mr_cmd), ®_mr_resp,
> > sizeof(reg_mr_resp));
> > if (ret) {
> > - fprintf(stderr, PFX "%s: failed to pin memory for CQ\n",
> __func__);
> > + i40iw_debug("failed to pin memory for CQ\n");
> > goto err;
> > }
> >
> > @@ -274,7 +274,7 @@ struct ibv_cq *i40iw_ucreate_cq(struct ibv_context
> *context, int cqe,
> > &resp.ibv_resp, sizeof(resp));
> > if (ret) {
> > ibv_cmd_dereg_mr(&iwucq->mr);
> > - fprintf(stderr, PFX "%s: failed to create CQ\n", __func__);
> > + i40iw_debug("failed to create CQ\n");
> > goto err;
> > }
> >
> > @@ -286,7 +286,7 @@ struct ibv_cq *i40iw_ucreate_cq(struct ibv_context
> *context, int cqe,
> > if (!ret)
> > return &iwucq->ibv_cq;
> > else
> > - fprintf(stderr, PFX "%s: failed to initialze CQ, status %d\n",
> __func__, ret);
> > + i40iw_debug("failed to initialze CQ, status %d\n", ret);
> > err:
> > if (info.cq_base)
> > free(info.cq_base);
> > @@ -307,11 +307,11 @@ int i40iw_udestroy_cq(struct ibv_cq *cq)
> >
> > ret = pthread_spin_destroy(&iwucq->lock);
> > if (ret)
> > - return ret;
> > + goto err;
> >
> > ret = ibv_cmd_destroy_cq(cq);
> > if (ret)
> > - return ret;
> > + goto err;
> >
> > ibv_cmd_dereg_mr(&iwucq->mr);
> >
> > @@ -319,6 +319,9 @@ int i40iw_udestroy_cq(struct ibv_cq *cq)
> > free(iwucq);
> >
> > return 0;
> > +err:
> > + i40iw_debug("failed to destroy CQ, status %d\n", ret);
> > + return ret;
> > }
> >
> > /**
> > @@ -344,7 +347,7 @@ int i40iw_upoll_cq(struct ibv_cq *cq, int
> num_entries, struct ibv_wc *entry)
> > if (ret == I40IW_ERR_QUEUE_EMPTY) {
> > break;
> > } else if (ret) {
> > - fprintf(stderr, PFX "%s: Error polling CQ, status
> %d\n", __func__, ret);
> > + i40iw_debug("Error polling CQ, status %d\n", ret);
> > if (!cqe_count)
> > /* Indicate error */
> > cqe_count = -1;
> > @@ -519,7 +522,7 @@ static int i40iw_vmapped_qp(struct i40iw_uqp
> *iwuqp, struct ibv_pd *pd,
> > info->sq = memalign(I40IW_HW_PAGE_SIZE, totalqpsize);
> >
> > if (!info->sq) {
> > - fprintf(stderr, PFX "%s: failed to allocate memory for SQ\n",
> __func__);
> > + i40iw_debug("failed to allocate memory for SQ\n");
> > return 0;
> > }
> >
> > @@ -535,7 +538,7 @@ static int i40iw_vmapped_qp(struct i40iw_uqp
> *iwuqp, struct ibv_pd *pd,
> > IBV_ACCESS_LOCAL_WRITE, &iwuqp->mr,
> ®_mr_cmd.ibv_cmd,
> > sizeof(reg_mr_cmd), ®_mr_resp,
> sizeof(reg_mr_resp));
> > if (ret) {
> > - fprintf(stderr, PFX "%s: failed to pin memory for SQ\n",
> __func__);
> > + i40iw_debug("failed to pin memory for SQ\n");
> > free(info->sq);
> > return 0;
> > }
> > @@ -545,7 +548,7 @@ static int i40iw_vmapped_qp(struct i40iw_uqp
> *iwuqp, struct ibv_pd *pd,
> > ret = ibv_cmd_create_qp(pd, &iwuqp->ibv_qp, attr, &cmd.ibv_cmd,
> sizeof(cmd),
> > &resp->ibv_resp, sizeof(struct
> i40iw_ucreate_qp_resp));
> > if (ret) {
> > - fprintf(stderr, PFX "%s: failed to create QP, status %d\n",
> __func__, ret);
> > + i40iw_debug("failed to create QP, status %d\n", ret);
> > ibv_cmd_dereg_mr(&iwuqp->mr);
> > free(info->sq);
> > return 0;
> > @@ -565,7 +568,7 @@ static int i40iw_vmapped_qp(struct i40iw_uqp
> *iwuqp, struct ibv_pd *pd,
> > map = mmap(NULL, I40IW_HW_PAGE_SIZE, PROT_WRITE |
> PROT_READ, MAP_SHARED,
> > pd->context->cmd_fd, offset);
> > if (map == MAP_FAILED) {
> > - fprintf(stderr, PFX "%s: failed to map push page,
> errno %d\n", __func__, errno);
> > + i40iw_debug("failed to map push page, errno %d\n",
> errno);
> > info->push_wqe = NULL;
> > info->push_db = NULL;
> > } else {
> > @@ -575,7 +578,7 @@ static int i40iw_vmapped_qp(struct i40iw_uqp
> *iwuqp, struct ibv_pd *pd,
> > map = mmap(NULL, I40IW_HW_PAGE_SIZE,
> PROT_WRITE | PROT_READ, MAP_SHARED,
> > pd->context->cmd_fd, offset);
> > if (map == MAP_FAILED) {
> > - fprintf(stderr, PFX "%s: failed to map push
> doorbell, errno %d\n", __func__, errno);
> > + i40iw_debug("failed to map push doorbell,
> errno %d\n", errno);
> > munmap(info->push_wqe,
> I40IW_HW_PAGE_SIZE);
> > info->push_wqe = NULL;
> > info->push_db = NULL;
> > @@ -639,7 +642,7 @@ struct ibv_qp *i40iw_ucreate_qp(struct ibv_pd *pd,
> struct ibv_qp_init_attr *attr
> > int sq_attr, rq_attr;
> >
> > if (attr->qp_type != IBV_QPT_RC) {
> > - fprintf(stderr, PFX "%s: failed to create QP, unsupported QP
> type: 0x%x\n", __func__, attr->qp_type);
> > + i40iw_debug("failed to create QP, unsupported QP type:
> 0x%x\n",
> > +attr->qp_type);
> > return NULL;
> > }
> >
> > @@ -658,8 +661,8 @@ struct ibv_qp *i40iw_ucreate_qp(struct ibv_pd *pd,
> struct ibv_qp_init_attr *attr
> > /* Sanity check QP size before proceeding */
> > sqdepth = i40iw_qp_get_qdepth(sq_attr, attr->cap.max_send_sge,
> attr->cap.max_inline_data);
> > if (!sqdepth) {
> > - fprintf(stderr, PFX "%s: invalid SQ attributes,
> max_send_wr=%d max_send_sge=%d\n",
> > - __func__, attr->cap.max_send_wr, attr-
> >cap.max_send_sge);
> > + i40iw_debug("invalid SQ attributes, max_send_wr=%d
> max_send_sge=%d\n",
> > + attr->cap.max_send_wr, attr->cap.max_send_sge);
> > return NULL;
> > }
> >
> > @@ -691,13 +694,13 @@ struct ibv_qp *i40iw_ucreate_qp(struct ibv_pd
> *pd, struct ibv_qp_init_attr *attr
> > info.sq_wrtrk_array = calloc(sqdepth, sizeof(*info.sq_wrtrk_array));
> >
> > if (!info.sq_wrtrk_array) {
> > - fprintf(stderr, PFX "%s: failed to allocate memory for SQ work
> array\n", __func__);
> > + i40iw_debug("failed to allocate memory for SQ work
> array\n");
> > goto err_destroy_lock;
> > }
> >
> > info.rq_wrid_array = calloc(rqdepth, sizeof(*info.rq_wrid_array));
> > if (!info.rq_wrid_array) {
> > - fprintf(stderr, PFX "%s: failed to allocate memory for RQ
> work array\n", __func__);
> > + i40iw_debug("failed to allocate memory for RQ work
> array\n");
> > goto err_free_sq_wrtrk;
> > }
> >
> > @@ -706,7 +709,7 @@ struct ibv_qp *i40iw_ucreate_qp(struct ibv_pd *pd,
> struct ibv_qp_init_attr *attr
> > status = i40iw_vmapped_qp(iwuqp, pd, attr, &resp, sqdepth,
> rqdepth,
> > &info);
> >
> > if (!status) {
> > - fprintf(stderr, PFX "%s: failed to map QP\n", __func__);
> > + i40iw_debug("failed to map QP\n");
> > goto err_free_rq_wrid;
> > }
> > info.qp_id = resp.qp_id;
> > @@ -772,11 +775,11 @@ int i40iw_udestroy_qp(struct ibv_qp *qp)
> >
> > ret = pthread_spin_destroy(&iwuqp->lock);
> > if (ret)
> > - return ret;
> > + goto err;
> >
> > ret = i40iw_destroy_vmapped_qp(iwuqp, iwuqp->qp.sq_base);
> > if (ret)
> > - return ret;
> > + goto err;
> >
> > if (iwuqp->qp.sq_wrtrk_array)
> > free(iwuqp->qp.sq_wrtrk_array);
> > @@ -792,6 +795,9 @@ int i40iw_udestroy_qp(struct ibv_qp *qp)
> > free(iwuqp);
> >
> > return 0;
> > +err:
> > + i40iw_debug("failed to destroy QP, status %d\n", ret);
> > + return ret;
> > }
> >
> > /**
> > @@ -916,7 +922,7 @@ int i40iw_upost_send(struct ibv_qp *ib_qp, struct
> ibv_send_wr *ib_wr, struct ibv
> > default:
> > /* error */
> > err = -EINVAL;
> > - fprintf(stderr, PFX "%s: post work request failed,
> invalid opcode: 0x%x\n", __func__, ib_wr->opcode);
> > + i40iw_debug("post work request failed, invalid
> opcode: 0x%x\n",
> > +ib_wr->opcode);
> > break;
> > }
> >
> > @@ -960,7 +966,7 @@ int i40iw_upost_recv(struct ibv_qp *ib_qp, struct
> ibv_recv_wr *ib_wr, struct ibv
> > post_recv.sg_list = sg_list;
> > ret = iwuqp->qp.ops.iw_post_receive(&iwuqp->qp,
> &post_recv);
> > if (ret) {
> > - fprintf(stderr, PFX "%s: failed to post receives, status
> %d\n", __func__, ret);
> > + i40iw_debug("failed to post receives, status %d\n",
> ret);
> > if (ret == I40IW_ERR_QP_TOOMANY_WRS_POSTED)
> > err = -ENOMEM;
> > else
> > --
> > 1.8.5.2
> >
> > --
> > 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
^ permalink raw reply
* Re: SR-IOV with mlx4 on ConnectX-2 fails with DMAR errors
From: Jason Gunthorpe @ 2016-12-13 19:01 UTC (permalink / raw)
To: Joshua McBeth, Leon Romanovsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAN27Ff7hEr4u5nyELjwRXb3W_t0TNefuuT4AzdjAoWWGRHqTFw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Dec 13, 2016 at 01:36:42PM -0500, Joshua McBeth wrote:
> I bisected the kernel between v4.1 and v4.3.1 by booting each build on
> the SR-IOV host and attempting to "ping x.x.x.x" with x.x.x.x being
> the IP address assigned to the Infiniband interface of a remote host
>
> At 4be90bc's parent the SR-IOV host is able to ping the remote host,
> but at 4be90bc the SR-IOV host is not able to ping the remote host
> (destination host unreachable)
Okay, that makes sense
> The DMAR errors occur in both the kernel built at 4be90bc (not passing
> ping test) and its parent (passing ping test)
Continuing to bisect until you find the commit that introduces the
DMAR errors would also be helpful, I think.
> Reverting only the commit 4be90bc from a later kernel (4.8.x) does not
> enable the SR-IOV host to ping the remote host, which to me suggests
> that another commit after 4be90bc is also causing my test to fail.
Okay, that does not seem too surprising.
Does this make your 4.8 kernel work? If yes, then I suspect mlx4 has
broken IB_DEVICE_LOCAL_DMA_LKEY with SRIOV.. Leon? mlx5 has this
broken, doesn't it?
It would also be very helpful to try and determine what memory the NIC is
trying to read.. If it is the ipoib packet or some mlx4 internal
thing.
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 2be4ea0cda9c19..1346924d27691f 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -243,6 +243,8 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,
atomic_set(&pd->usecnt, 0);
pd->flags = flags;
+ device->attrs.device_cap_flags = 0;
+
if (device->attrs.device_cap_flags & IB_DEVICE_LOCAL_DMA_LKEY)
pd->local_dma_lkey = device->local_dma_lkey;
else
Jason
--
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
^ permalink raw reply related
* Re: SR-IOV with mlx4 on ConnectX-2 fails with DMAR errors
From: Joshua McBeth @ 2016-12-13 18:36 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161213165441.GB11099-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
I bisected the kernel between v4.1 and v4.3.1 by booting each build on
the SR-IOV host and attempting to "ping x.x.x.x" with x.x.x.x being
the IP address assigned to the Infiniband interface of a remote host
At 4be90bc's parent the SR-IOV host is able to ping the remote host,
but at 4be90bc the SR-IOV host is not able to ping the remote host
(destination host unreachable)
The DMAR errors occur in both the kernel built at 4be90bc (not passing
ping test) and its parent (passing ping test)
Reverting only the commit 4be90bc from a later kernel (4.8.x) does not
enable the SR-IOV host to ping the remote host, which to me suggests
that another commit after 4be90bc is also causing my test to fail.
On Tue, Dec 13, 2016 at 11:54 AM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> On Mon, Dec 12, 2016 at 10:57:58PM -0500, Joshua McBeth wrote:
>> Regarding this issue, I discovered earlier kernels were working with
>> SR-IOV enabled and bisected to the commit
>> 4be90bc60df47f6268b594c4fb6c90f0ff2f519f ("IB/mad: Remove
>> ib_get_dma_mr calls").
>>
>> However, this commit seems to be part of a larger refactoring patch
>> set and reverting just this commit does not resolve the DMAR errors
>> nor enable the 'ib0' interface to function.
>
> ?? if you bisected correctly then reverting the ID'd patch should
> solve the problem.
>
> Jason
>
--
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
^ permalink raw reply
* Re: [PATCH V2 18/22] bnxt_re: Support for DCB
From: Jason Gunthorpe @ 2016-12-13 16:56 UTC (permalink / raw)
To: Selvin Xavier
Cc: Or Gerlitz, Doug Ledford,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Netdev List, Eddie Wai, Devesh Sharma, Somnath Kotur,
Sriharsha Basavapatna
In-Reply-To: <CA+sbYW1irBd0cTqJJSGJWRbBi-iFzvX3JpoTfF_daU47EqNtAg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Dec 13, 2016 at 11:55:55AM +0530, Selvin Xavier wrote:
> v1 eth_type is not defined. All vendor drivers have their own definition.
Send a cleanup patch?
Jason
--
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
^ permalink raw reply
* Re: SR-IOV with mlx4 on ConnectX-2 fails with DMAR errors
From: Jason Gunthorpe @ 2016-12-13 16:54 UTC (permalink / raw)
To: Joshua McBeth; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAN27Ff74Ov8bgsZP1QJJOwvWc5_KOW_U=PsP+=Z87AJz+VSAOw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Dec 12, 2016 at 10:57:58PM -0500, Joshua McBeth wrote:
> Regarding this issue, I discovered earlier kernels were working with
> SR-IOV enabled and bisected to the commit
> 4be90bc60df47f6268b594c4fb6c90f0ff2f519f ("IB/mad: Remove
> ib_get_dma_mr calls").
>
> However, this commit seems to be part of a larger refactoring patch
> set and reverting just this commit does not resolve the DMAR errors
> nor enable the 'ib0' interface to function.
?? if you bisected correctly then reverting the ID'd patch should
solve the problem.
Jason
--
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
^ permalink raw reply
* Re: [PATCH v6 0/9] SELinux support for Infiniband RDMA
From: Daniel Jurgens @ 2016-12-13 16:25 UTC (permalink / raw)
To: Stephen Smalley, chrisw-69jw2NvuJkxg9hUCZPvPmw@public.gmane.org,
paul-r2n+y4ga6xFZroRs9YW3xA@public.gmane.org,
eparis-FjpueFixGhCM4zKIHC2jIg@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org
In-Reply-To: <a56c4be8-f908-4774-dd1c-c6bbab5c1b03@tycho.nsa.gov>
On 12/13/2016 9:01 AM, Stephen Smalley wrote:
> On 11/23/2016 09:17 AM, Dan Jurgens wrote:
>> From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>
>> Infiniband applications access HW from user-space -- traffic is generated
>> directly by HW, bypassing the kernel. Consequently, Infiniband Partitions,
>> which are associated directly with HW transport endpoints, are a natural
>> choice for enforcing granular mandatory access control for Infiniband. QPs may
>> only send or receives packets tagged with the corresponding partition key
>> (PKey). The PKey is not a cryptographic key; it's a 16 bit number identifying
>> the partition.
>>
>> Every Infiniband fabric is controlled by a central Subnet Manager (SM). The SM
>> provisions the partitions by assigning each port with the partitions it can
>> access. In addition, the SM tags each port with a subnet prefix, which
>> identifies the subnet. Determining which users are allowed to access which
>> partition keys on a given subnet forms an effective policy for isolating users
>> on the fabric. Any application that attempts to send traffic on a given subnet
>> is automatically subject to the policy, regardless of which device and port it
>> uses. SM software configures the subnet through a privileged Subnet Management
>> Interface (SMI), which is presented by each Infiniband port. Thus, the SMI must
>> also be controlled to prevent unauthorized changes to fabric configuration and
>> partitioning.
>>
>> To support access control for IB partitions and subnet management, security
>> contexts must be provided for two new types of objects - PKeys and IB ports.
>>
>> A PKey label consists of a subnet prefix and a range of PKey values and is
>> similar to the labeling mechanism for netports. Each Infiniband port can reside
>> on a different subnet. So labeling the PKey values for specific subnet prefixes
>> provides the user maximum flexibility, as PKey values may be determined
>> independently for different subnets. There is a single access vector for PKeys
>> called "access".
>>
>> An Infiniband port is labeled by device name and port number. There is a single
>> access vector for IB ports called "manage_subnet".
>>
>> Because RDMA allows kernel bypass, enforcement must be done during connection
>> setup. Communication over RDMA requires a send and receive queue, collectively
>> known as a Queue Pair (QP). A QP must be initialized by privileged system calls
>> before it can be used to send or receive data. During initialization the user
>> must provide the PKey and port the QP will use; at this time access control can
>> be enforced.
>>
>> Because there is a possibility that the enforcement settings or security
>> policy can change, a means of notifying the ib_core module of such changes
>> is required. To facilitate this a generic notification callback mechanism
>> is added to the LSM. One callback is registered for checking the QP PKey
>> associations when the policy changes. Mad agents also register a callback,
>> they cache the permission to send and receive SMPs to avoid another per
>> packet call to the LSM.
>>
>> Because frequent accesses to the same PKey's SID is expected a cache is
>> implemented which is very similar to the netport cache.
>>
>> In order to properly enforce security when changes to the PKey table or
>> security policy or enforcement occur ib_core must track which QPs are
>> using which port, pkey index, and alternate path for every IB device.
>> This makes operations that used to be atomic transactional.
>>
>> When modifying a QP, ib_core must associate it with the PKey index, port,
>> and alternate path specified. If the QP was already associated with
>> different settings, the QP is added to the new list prior to the
>> modification. If the modify succeeds then the old listing is removed. If
>> the modify fails the new listing is removed and the old listing remains
>> unchanged.
>>
>> When destroying a QP the ib_qp structure is freed by the decive specific
>> driver (i.e. mlx4_ib) if the 'destroy' is successful. This requires storing
>> security related information in a separate structure. When a 'destroy'
>> request is in process the ib_qp structure is in an undefined state so if
>> there are changes to the security policy or PKey table, the security checks
>> cannot reset the QP if it doesn't have permission for the new setting. If
>> the 'destroy' fails, security for that QP must be enforced again and its
>> status in the list is restored. If the 'destroy' succeeds the security info
>> can be cleaned up and freed.
>>
>> There are a number of locks required to protect the QP security structure
>> and the QP to device/port/pkey index lists. If multiple locks are required,
>> the safe locking order is: QP security structure mutex first, followed by
>> any list locks needed, which are sorted first by port followed by pkey
>> index.
>>
>> ---
>> v2:
>> - Use void* blobs in the LSM hooks. Paul Moore
>> - Make the policy change callback generic. Yuval Shaia, Paul Moore
>> - Squash LSM changes into the patches where the calls are added. Paul Moore
>> - Don't add new initial SIDs. Stephen Smalley
>> - Squash MAD agent PKey and SMI patches and move logic to IB security. Dan Jurgens
>> - Changed ib_end_port to ib_port. Paul Moore
>> - Changed ib_port access vector from smp to manage_subnet. Paul Moore
>> - Added pkey and ib_port details to the audit log. Paul Moore
>> - See individual patches for more detail.
>>
>> v3:
>> - ib_port -> ib_endport. Paul Moore
>> - use notifier chains for LSM notifications. Paul Moore
>> - reorder parameters in hooks to put security blob first. Paul Moore
>> - Don't treat device name as untrusted string in audit log. Paul Moore
>>
>> v4:
>> - Added separate AVC callback for LSM notifier. Paul Moore
>> - Removed unneeded braces in ocontext_read. Paul Moore
>>
>> v5:
>> - Fix link error when CONFIG_SECURITY is not set. Build Robot
>> - Strip issue and Gerrit-Id: Leon Romanovsky
>>
>> v6:
>> - Whitespace and bracket cleanup. James Morris
>> - Cleanup error flow in sel_pkey_sid_slow. James Morris
>>
>> Daniel Jurgens (9):
>> IB/core: IB cache enhancements to support Infiniband security
>> IB/core: Enforce PKey security on QPs
>> selinux lsm IB/core: Implement LSM notification system
>> IB/core: Enforce security on management datagrams
>> selinux: Create policydb version for Infiniband support
>> selinux: Allocate and free infiniband security hooks
>> selinux: Implement Infiniband PKey "Access" access vector
>> selinux: Add IB Port SMP access vector
>> selinux: Add a cache for quicker retreival of PKey SIDs
>>
>> drivers/infiniband/core/Makefile | 3 +-
>> drivers/infiniband/core/cache.c | 57 ++-
>> drivers/infiniband/core/core_priv.h | 115 ++++++
>> drivers/infiniband/core/device.c | 86 +++++
>> drivers/infiniband/core/mad.c | 52 ++-
>> drivers/infiniband/core/security.c | 709 +++++++++++++++++++++++++++++++++++
>> drivers/infiniband/core/uverbs_cmd.c | 20 +-
>> drivers/infiniband/core/verbs.c | 27 +-
>> include/linux/lsm_audit.h | 15 +
>> include/linux/lsm_hooks.h | 35 ++
>> include/linux/security.h | 50 +++
>> include/rdma/ib_mad.h | 4 +
>> include/rdma/ib_verbs.h | 49 +++
>> security/Kconfig | 9 +
>> security/lsm_audit.c | 16 +
>> security/security.c | 59 +++
>> security/selinux/Makefile | 2 +-
>> security/selinux/hooks.c | 86 ++++-
>> security/selinux/ibpkey.c | 245 ++++++++++++
>> security/selinux/include/classmap.h | 4 +
>> security/selinux/include/ibpkey.h | 31 ++
>> security/selinux/include/objsec.h | 11 +
>> security/selinux/include/security.h | 7 +-
>> security/selinux/selinuxfs.c | 2 +
>> security/selinux/ss/policydb.c | 129 ++++++-
>> security/selinux/ss/policydb.h | 27 +-
>> security/selinux/ss/services.c | 81 ++++
>> 27 files changed, 1886 insertions(+), 45 deletions(-)
>> create mode 100644 drivers/infiniband/core/security.c
>> create mode 100644 security/selinux/ibpkey.c
>> create mode 100644 security/selinux/include/ibpkey.h
> For the LSM/SELinux bits,
> Acked-by: Stephen Smalley <sds-+05T5uksL2qpZYMLLGbcSA@public.gmane.org>
>
> Note that there will be a merge conflict on classmap.h due to commits in
> the selinux next branch, but that should be easy to resolve.
>
> We'll need the patches for the selinux userspace and refpolicy.
Thanks Stephen, I need to rebase the user space and do some patch breakup. I'll start on that soon.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
_______________________________________________
Selinux mailing list
Selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org
To unsubscribe, send email to Selinux-leave-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
To get help, send an email containing "help" to Selinux-request-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
^ permalink raw reply
* [PATCH resend] IB/core: fix unmap_sg argument
From: Sebastian Ott @ 2016-12-13 15:10 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock; +Cc: linux-rdma, linux-kernel
In-Reply-To: <alpine.LFD.2.20.1612021439450.3570@schleppi>
Hi,
using dapltest on s390 I ran into the following warning:
[ 20.781709] mlx4_core 0000:00:00.0: DMA-API: device driver frees DMA sg list with different entry count [map count=2] [unmap count=1]
[ 20.781760] ------------[ cut here ]------------
[ 20.781767] WARNING: CPU: 4 PID: 1063 at lib/dma-debug.c:1141 check_unmap+0x658/0xa08
[ 20.781769] Modules linked in: rdma_ucm ib_ucm ib_uverbs rdma_cm configfs ib_cm iw_cm [...]
[ 20.781797] CPU: 4 PID: 1063 Comm: dapltest Tainted: G W 4.9.0-rc7-00039-g43c4f67 #65
[ 20.781799] Hardware name: IBM 2964 N96 704 (LPAR)
[ 20.781801] task: 00000000dd8e4008 task.stack: 00000000f1448000
[ 20.781803] Krnl PSW : 0404c00180000000 000000000060af08 (check_unmap+0x658/0xa08)
[ 20.781806] R:0 T:1 IO:0 EX:0 Key:0 M:1 W:0 P:0 AS:3 CC:0 PM:0 RI:0 EA:3
[ 20.781809] Krnl GPRS: 0000000000000002 00000000dd8e4008 0000000000000079 0000000000a1d4f0
[ 20.781811] 000000000060af04 0000000000000000 0000000000000001 0000000000000001
[ 20.781813] 00000000f8e8e880 07000000008f2210 0000000001e47700 00000000f7a11298
[ 20.781814] 00000000f144ba68 00000000008f21f8 000000000060af04 00000000f144b960
[ 20.781822] Krnl Code: 000000000060aef8: c0200022fcc0 larl %r2,a6a878
000000000060aefe: c0e5ffe474dd brasl %r14,2998b8
#000000000060af04: a7f40001 brc 15,60af06
>000000000060af08: c0200022f976 larl %r2,a6a1f4
000000000060af0e: c0e5ffe474d5 brasl %r14,2998b8
000000000060af14: 4120b050 la %r2,80(%r11)
000000000060af18: a7390000 lghi %r3,0
000000000060af1c: c0e5ffde780a brasl %r14,1d9f30
[ 20.781847] Call Trace:
[ 20.781848] ([<000000000060af04>] check_unmap+0x654/0xa08)
[ 20.781851] [<000000000060b6bc>] debug_dma_unmap_sg+0xf4/0x160
[ 20.781873] [<000003ff82245738>] __ib_umem_release+0x98/0x1a8 [ib_core]
[ 20.781881] [<000003ff82245f3e>] ib_umem_release+0x5e/0x1d0 [ib_core]
[ 20.781891] [<000003ff80139976>] mlx4_ib_destroy_qp+0x47e/0x550 [mlx4_ib]
[ 20.781898] [<000003ff8222ad2e>] ib_destroy_qp+0x116/0x188 [ib_core]
[ 20.781902] [<000003ff80052fba>] ib_uverbs_destroy_qp+0xb2/0x1a0 [ib_uverbs]
[ 20.781905] [<000003ff8004cada>] ib_uverbs_write+0x20a/0x488 [ib_uverbs]
[ 20.781910] [<0000000000352756>] __vfs_write+0x36/0x138
[ 20.781912] [<000000000035348c>] vfs_write+0xbc/0x1a0
[ 20.781914] [<0000000000354a96>] SyS_write+0x66/0xc0
[ 20.781918] [<000000000087d796>] system_call+0xd6/0x270
[ 20.781919] INFO: lockdep is turned off.
[ 20.781921] Last Breaking-Event-Address:
[ 20.781922] [<000000000060af04>] check_unmap+0x654/0xa08
[ 20.781924] ---[ end trace bd581c43b9bebeea ]---
[ 20.781925] Mapped at:
[ 20.781929] [<000000000060b4a6>] debug_dma_map_sg+0x5e/0x180
[ 20.781938] [<000003ff82245d12>] ib_umem_get+0x4ca/0x610 [ib_core]
[ 20.781943] [<000003ff80136df2>] create_qp_common.isra.15+0x572/0x1010 [mlx4_ib]
[ 20.781949] [<000003ff8013929e>] mlx4_ib_create_qp+0x1de/0x438 [mlx4_ib]
[ 20.781953] [<000003ff8004f52c>] create_qp.isra.11+0x44c/0x7f0 [ib_uverbs]
The following patch fixes this:
---->8
>From ec91646d8c14e2a8dd2b62187084dab32ef8a56b Mon Sep 17 00:00:00 2001
From: Sebastian Ott <sebott@linux.vnet.ibm.com>
Date: Fri, 2 Dec 2016 11:15:05 +0100
Subject: [PATCH] IB/core: fix unmap_sg argument
__ib_umem_release calls dma_unmap_sg with a different number of
sg_entries than ib_umem_get uses for dma_map_sg. This might cause
trouble for implementations that merge sglist entries and results
in the following dma debug complaint:
DMA-API: device driver frees DMA sg list with different entry
count [map count=2] [unmap count=1]
Fix it by using the correct value.
Signed-off-by: Sebastian Ott <sebott@linux.vnet.ibm.com>
---
drivers/infiniband/core/umem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
index 84b4eff..1e62a5f 100644
--- a/drivers/infiniband/core/umem.c
+++ b/drivers/infiniband/core/umem.c
@@ -51,7 +51,7 @@ static void __ib_umem_release(struct ib_device *dev, struct ib_umem *umem, int d
if (umem->nmap > 0)
ib_dma_unmap_sg(dev, umem->sg_head.sgl,
- umem->nmap,
+ umem->npages,
DMA_BIDIRECTIONAL);
for_each_sg(umem->sg_head.sgl, sg, umem->npages, i) {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v6 0/9] SELinux support for Infiniband RDMA
From: Stephen Smalley @ 2016-12-13 15:04 UTC (permalink / raw)
To: Dan Jurgens, chrisw-69jw2NvuJkxg9hUCZPvPmw,
paul-r2n+y4ga6xFZroRs9YW3xA, eparis-FjpueFixGhCM4zKIHC2jIg,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
selinux-+05T5uksL2qpZYMLLGbcSA
In-Reply-To: <1479910651-43246-1-git-send-email-danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
On 11/23/2016 09:17 AM, Dan Jurgens wrote:
> From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> Infiniband applications access HW from user-space -- traffic is generated
> directly by HW, bypassing the kernel. Consequently, Infiniband Partitions,
> which are associated directly with HW transport endpoints, are a natural
> choice for enforcing granular mandatory access control for Infiniband. QPs may
> only send or receives packets tagged with the corresponding partition key
> (PKey). The PKey is not a cryptographic key; it's a 16 bit number identifying
> the partition.
>
> Every Infiniband fabric is controlled by a central Subnet Manager (SM). The SM
> provisions the partitions by assigning each port with the partitions it can
> access. In addition, the SM tags each port with a subnet prefix, which
> identifies the subnet. Determining which users are allowed to access which
> partition keys on a given subnet forms an effective policy for isolating users
> on the fabric. Any application that attempts to send traffic on a given subnet
> is automatically subject to the policy, regardless of which device and port it
> uses. SM software configures the subnet through a privileged Subnet Management
> Interface (SMI), which is presented by each Infiniband port. Thus, the SMI must
> also be controlled to prevent unauthorized changes to fabric configuration and
> partitioning.
>
> To support access control for IB partitions and subnet management, security
> contexts must be provided for two new types of objects - PKeys and IB ports.
>
> A PKey label consists of a subnet prefix and a range of PKey values and is
> similar to the labeling mechanism for netports. Each Infiniband port can reside
> on a different subnet. So labeling the PKey values for specific subnet prefixes
> provides the user maximum flexibility, as PKey values may be determined
> independently for different subnets. There is a single access vector for PKeys
> called "access".
>
> An Infiniband port is labeled by device name and port number. There is a single
> access vector for IB ports called "manage_subnet".
>
> Because RDMA allows kernel bypass, enforcement must be done during connection
> setup. Communication over RDMA requires a send and receive queue, collectively
> known as a Queue Pair (QP). A QP must be initialized by privileged system calls
> before it can be used to send or receive data. During initialization the user
> must provide the PKey and port the QP will use; at this time access control can
> be enforced.
>
> Because there is a possibility that the enforcement settings or security
> policy can change, a means of notifying the ib_core module of such changes
> is required. To facilitate this a generic notification callback mechanism
> is added to the LSM. One callback is registered for checking the QP PKey
> associations when the policy changes. Mad agents also register a callback,
> they cache the permission to send and receive SMPs to avoid another per
> packet call to the LSM.
>
> Because frequent accesses to the same PKey's SID is expected a cache is
> implemented which is very similar to the netport cache.
>
> In order to properly enforce security when changes to the PKey table or
> security policy or enforcement occur ib_core must track which QPs are
> using which port, pkey index, and alternate path for every IB device.
> This makes operations that used to be atomic transactional.
>
> When modifying a QP, ib_core must associate it with the PKey index, port,
> and alternate path specified. If the QP was already associated with
> different settings, the QP is added to the new list prior to the
> modification. If the modify succeeds then the old listing is removed. If
> the modify fails the new listing is removed and the old listing remains
> unchanged.
>
> When destroying a QP the ib_qp structure is freed by the decive specific
> driver (i.e. mlx4_ib) if the 'destroy' is successful. This requires storing
> security related information in a separate structure. When a 'destroy'
> request is in process the ib_qp structure is in an undefined state so if
> there are changes to the security policy or PKey table, the security checks
> cannot reset the QP if it doesn't have permission for the new setting. If
> the 'destroy' fails, security for that QP must be enforced again and its
> status in the list is restored. If the 'destroy' succeeds the security info
> can be cleaned up and freed.
>
> There are a number of locks required to protect the QP security structure
> and the QP to device/port/pkey index lists. If multiple locks are required,
> the safe locking order is: QP security structure mutex first, followed by
> any list locks needed, which are sorted first by port followed by pkey
> index.
>
> ---
> v2:
> - Use void* blobs in the LSM hooks. Paul Moore
> - Make the policy change callback generic. Yuval Shaia, Paul Moore
> - Squash LSM changes into the patches where the calls are added. Paul Moore
> - Don't add new initial SIDs. Stephen Smalley
> - Squash MAD agent PKey and SMI patches and move logic to IB security. Dan Jurgens
> - Changed ib_end_port to ib_port. Paul Moore
> - Changed ib_port access vector from smp to manage_subnet. Paul Moore
> - Added pkey and ib_port details to the audit log. Paul Moore
> - See individual patches for more detail.
>
> v3:
> - ib_port -> ib_endport. Paul Moore
> - use notifier chains for LSM notifications. Paul Moore
> - reorder parameters in hooks to put security blob first. Paul Moore
> - Don't treat device name as untrusted string in audit log. Paul Moore
>
> v4:
> - Added separate AVC callback for LSM notifier. Paul Moore
> - Removed unneeded braces in ocontext_read. Paul Moore
>
> v5:
> - Fix link error when CONFIG_SECURITY is not set. Build Robot
> - Strip issue and Gerrit-Id: Leon Romanovsky
>
> v6:
> - Whitespace and bracket cleanup. James Morris
> - Cleanup error flow in sel_pkey_sid_slow. James Morris
>
> Daniel Jurgens (9):
> IB/core: IB cache enhancements to support Infiniband security
> IB/core: Enforce PKey security on QPs
> selinux lsm IB/core: Implement LSM notification system
> IB/core: Enforce security on management datagrams
> selinux: Create policydb version for Infiniband support
> selinux: Allocate and free infiniband security hooks
> selinux: Implement Infiniband PKey "Access" access vector
> selinux: Add IB Port SMP access vector
> selinux: Add a cache for quicker retreival of PKey SIDs
>
> drivers/infiniband/core/Makefile | 3 +-
> drivers/infiniband/core/cache.c | 57 ++-
> drivers/infiniband/core/core_priv.h | 115 ++++++
> drivers/infiniband/core/device.c | 86 +++++
> drivers/infiniband/core/mad.c | 52 ++-
> drivers/infiniband/core/security.c | 709 +++++++++++++++++++++++++++++++++++
> drivers/infiniband/core/uverbs_cmd.c | 20 +-
> drivers/infiniband/core/verbs.c | 27 +-
> include/linux/lsm_audit.h | 15 +
> include/linux/lsm_hooks.h | 35 ++
> include/linux/security.h | 50 +++
> include/rdma/ib_mad.h | 4 +
> include/rdma/ib_verbs.h | 49 +++
> security/Kconfig | 9 +
> security/lsm_audit.c | 16 +
> security/security.c | 59 +++
> security/selinux/Makefile | 2 +-
> security/selinux/hooks.c | 86 ++++-
> security/selinux/ibpkey.c | 245 ++++++++++++
> security/selinux/include/classmap.h | 4 +
> security/selinux/include/ibpkey.h | 31 ++
> security/selinux/include/objsec.h | 11 +
> security/selinux/include/security.h | 7 +-
> security/selinux/selinuxfs.c | 2 +
> security/selinux/ss/policydb.c | 129 ++++++-
> security/selinux/ss/policydb.h | 27 +-
> security/selinux/ss/services.c | 81 ++++
> 27 files changed, 1886 insertions(+), 45 deletions(-)
> create mode 100644 drivers/infiniband/core/security.c
> create mode 100644 security/selinux/ibpkey.c
> create mode 100644 security/selinux/include/ibpkey.h
For the LSM/SELinux bits,
Acked-by: Stephen Smalley <sds-+05T5uksL2qpZYMLLGbcSA@public.gmane.org>
Note that there will be a merge conflict on classmap.h due to commits in
the selinux next branch, but that should be easy to resolve.
We'll need the patches for the selinux userspace and refpolicy.
--
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
^ permalink raw reply
* Re: [PATCH v6 5/9] selinux: Create policydb version for Infiniband support
From: Daniel Jurgens @ 2016-12-13 14:40 UTC (permalink / raw)
To: Stephen Smalley, chrisw-69jw2NvuJkxg9hUCZPvPmw@public.gmane.org,
paul-r2n+y4ga6xFZroRs9YW3xA@public.gmane.org,
eparis-FjpueFixGhCM4zKIHC2jIg@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org
In-Reply-To: <edb7cebd-cf0a-f115-741e-6d4e96edb0bd@tycho.nsa.gov>
On 12/13/2016 8:35 AM, Stephen Smalley wrote:
> On 11/23/2016 09:17 AM, Dan Jurgens wrote:
>> From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>
>> Support for Infiniband requires the addition of two new object contexts,
>> one for infiniband PKeys and another IB Ports. Added handlers to read
>> and write the new ocontext types when reading or writing a binary policy
>> representation.
>>
>> Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> I assume you have libsepol/checkpolicy patches for this as well?
>
Yes, I plan to submit them once the kernel changes are accepted.
_______________________________________________
Selinux mailing list
Selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org
To unsubscribe, send email to Selinux-leave-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
To get help, send an email containing "help" to Selinux-request-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
^ permalink raw reply
* Re: [PATCH v6 3/9] selinux lsm IB/core: Implement LSM notification system
From: Daniel Jurgens @ 2016-12-13 14:38 UTC (permalink / raw)
To: Stephen Smalley, chrisw-69jw2NvuJkxg9hUCZPvPmw@public.gmane.org,
paul-r2n+y4ga6xFZroRs9YW3xA@public.gmane.org,
eparis-FjpueFixGhCM4zKIHC2jIg@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org
In-Reply-To: <8165ef76-d620-5904-e38e-90d258a30683@tycho.nsa.gov>
On 12/13/2016 8:26 AM, Stephen Smalley wrote:
> On 11/23/2016 09:17 AM, Dan Jurgens wrote:
>> @@ -177,6 +177,8 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
>> avc_ss_reset(0);
>> selnl_notify_setenforce(selinux_enforcing);
>> selinux_status_update_setenforce(selinux_enforcing);
>> + if (!selinux_enforcing)
>> + call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
> Why do you need this notification? When switching from permissive to
> enforcing, you need (and already get) a notification since you may need
> to revoke previously granted permissions. But what action do you need
> to take on switching to permissive?
MAD (management datagram) Agents cache if they are allowed to send and receive subnet management protocol (SMP) datagrams. Without this notification they will still drop all SMPs in permissive mode if they weren't allowed in enforcing mode. This is handled in [PATCH v6 4/9] IB/core: Enforce security on management datagrams.
_______________________________________________
Selinux mailing list
Selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org
To unsubscribe, send email to Selinux-leave-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
To get help, send an email containing "help" to Selinux-request-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
^ permalink raw reply
* Re: [PATCH v6 5/9] selinux: Create policydb version for Infiniband support
From: Stephen Smalley @ 2016-12-13 14:38 UTC (permalink / raw)
To: Dan Jurgens, chrisw-69jw2NvuJkxg9hUCZPvPmw,
paul-r2n+y4ga6xFZroRs9YW3xA, eparis-FjpueFixGhCM4zKIHC2jIg,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
selinux-+05T5uksL2qpZYMLLGbcSA
In-Reply-To: <1479910651-43246-6-git-send-email-danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
On 11/23/2016 09:17 AM, Dan Jurgens wrote:
> From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> Support for Infiniband requires the addition of two new object contexts,
> one for infiniband PKeys and another IB Ports. Added handlers to read
> and write the new ocontext types when reading or writing a binary policy
> representation.
>
> Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
I assume you have libsepol/checkpolicy patches for this as well?
>
> ---
> v2:
> - Shorten ib_end_port to ib_port. Paul Moore
> - Added bounds checking to port number. Paul Moore
> - Eliminated {} in OCON_PKEY case statement. Yuval Shaia
>
> v3:
> - ib_port -> ib_endport. Paul Moore
>
> v4:
> - removed unneeded brackets in ocontext_read. Paul Moore
> ---
> security/selinux/include/security.h | 3 +-
> security/selinux/ss/policydb.c | 129 +++++++++++++++++++++++++++++++-----
> security/selinux/ss/policydb.h | 27 +++++---
> 3 files changed, 135 insertions(+), 24 deletions(-)
>
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 308a286..6bb9b0a 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -36,10 +36,11 @@
> #define POLICYDB_VERSION_DEFAULT_TYPE 28
> #define POLICYDB_VERSION_CONSTRAINT_NAMES 29
> #define POLICYDB_VERSION_XPERMS_IOCTL 30
> +#define POLICYDB_VERSION_INFINIBAND 31
>
> /* Range of policy versions we understand*/
> #define POLICYDB_VERSION_MIN POLICYDB_VERSION_BASE
> -#define POLICYDB_VERSION_MAX POLICYDB_VERSION_XPERMS_IOCTL
> +#define POLICYDB_VERSION_MAX POLICYDB_VERSION_INFINIBAND
>
> /* Mask for just the mount related flags */
> #define SE_MNTMASK 0x0f
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index d719db4..24e16da 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -17,6 +17,11 @@
> *
> * Added support for the policy capability bitmap
> *
> + * Update: Mellanox Techonologies
> + *
> + * Added Infiniband support
> + *
> + * Copyright (C) 2016 Mellanox Techonologies
> * Copyright (C) 2007 Hewlett-Packard Development Company, L.P.
> * Copyright (C) 2004-2005 Trusted Computer Solutions, Inc.
> * Copyright (C) 2003 - 2004 Tresys Technology, LLC
> @@ -76,81 +81,86 @@ static struct policydb_compat_info policydb_compat[] = {
> {
> .version = POLICYDB_VERSION_BASE,
> .sym_num = SYM_NUM - 3,
> - .ocon_num = OCON_NUM - 1,
> + .ocon_num = OCON_NUM - 3,
> },
> {
> .version = POLICYDB_VERSION_BOOL,
> .sym_num = SYM_NUM - 2,
> - .ocon_num = OCON_NUM - 1,
> + .ocon_num = OCON_NUM - 3,
> },
> {
> .version = POLICYDB_VERSION_IPV6,
> .sym_num = SYM_NUM - 2,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_NLCLASS,
> .sym_num = SYM_NUM - 2,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_MLS,
> .sym_num = SYM_NUM,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_AVTAB,
> .sym_num = SYM_NUM,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_RANGETRANS,
> .sym_num = SYM_NUM,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_POLCAP,
> .sym_num = SYM_NUM,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_PERMISSIVE,
> .sym_num = SYM_NUM,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_BOUNDARY,
> .sym_num = SYM_NUM,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_FILENAME_TRANS,
> .sym_num = SYM_NUM,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_ROLETRANS,
> .sym_num = SYM_NUM,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_NEW_OBJECT_DEFAULTS,
> .sym_num = SYM_NUM,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_DEFAULT_TYPE,
> .sym_num = SYM_NUM,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_CONSTRAINT_NAMES,
> .sym_num = SYM_NUM,
> - .ocon_num = OCON_NUM,
> + .ocon_num = OCON_NUM - 2,
> },
> {
> .version = POLICYDB_VERSION_XPERMS_IOCTL,
> .sym_num = SYM_NUM,
> + .ocon_num = OCON_NUM - 2,
> + },
> + {
> + .version = POLICYDB_VERSION_INFINIBAND,
> + .sym_num = SYM_NUM,
> .ocon_num = OCON_NUM,
> },
> };
> @@ -2222,6 +2232,60 @@ static int ocontext_read(struct policydb *p, struct policydb_compat_info *info,
> goto out;
> break;
> }
> + case OCON_PKEY:
> + rc = next_entry(nodebuf, fp, sizeof(u32) * 6);
> + if (rc)
> + goto out;
> +
> + c->u.pkey.subnet_prefix = be64_to_cpu(*((__be64 *)nodebuf));
> + /* The subnet prefix is stored as an IPv6
> + * address in the policy.
> + *
> + * Check that the lower 2 DWORDS are 0.
> + */
> + if (nodebuf[2] || nodebuf[3]) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + if (nodebuf[4] > 0xffff ||
> + nodebuf[5] > 0xffff) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + c->u.pkey.low_pkey = le32_to_cpu(nodebuf[4]);
> + c->u.pkey.high_pkey = le32_to_cpu(nodebuf[5]);
> +
> + rc = context_read_and_validate(&c->context[0],
> + p,
> + fp);
> + if (rc)
> + goto out;
> + break;
> + case OCON_IB_ENDPORT:
> + rc = next_entry(buf, fp, sizeof(u32) * 2);
> + if (rc)
> + goto out;
> + len = le32_to_cpu(buf[0]);
> +
> + rc = str_read(&c->u.ib_endport.dev_name, GFP_KERNEL, fp, len);
> + if (rc)
> + goto out;
> +
> + if (buf[1] > 0xff || buf[1] == 0) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + c->u.ib_endport.port_num = le32_to_cpu(buf[1]);
> +
> + rc = context_read_and_validate(&c->context[0],
> + p,
> + fp);
> + if (rc)
> + goto out;
> + break;
> }
> }
> }
> @@ -3151,6 +3215,41 @@ static int ocontext_write(struct policydb *p, struct policydb_compat_info *info,
> if (rc)
> return rc;
> break;
> + case OCON_PKEY:
> + *((__be64 *)nodebuf) = cpu_to_be64(c->u.pkey.subnet_prefix);
> +
> + /*
> + * The low order 2 bits were confirmed to be 0
> + * when the policy was loaded. Write them out
> + * as zero
> + */
> + nodebuf[2] = 0;
> + nodebuf[3] = 0;
> +
> + nodebuf[4] = cpu_to_le32(c->u.pkey.low_pkey);
> + nodebuf[5] = cpu_to_le32(c->u.pkey.high_pkey);
> +
> + rc = put_entry(nodebuf, sizeof(u32), 6, fp);
> + if (rc)
> + return rc;
> + rc = context_write(p, &c->context[0], fp);
> + if (rc)
> + return rc;
> + break;
> + case OCON_IB_ENDPORT:
> + len = strlen(c->u.ib_endport.dev_name);
> + buf[0] = cpu_to_le32(len);
> + buf[1] = cpu_to_le32(c->u.ib_endport.port_num);
> + rc = put_entry(buf, sizeof(u32), 2, fp);
> + if (rc)
> + return rc;
> + rc = put_entry(c->u.ib_endport.dev_name, 1, len, fp);
> + if (rc)
> + return rc;
> + rc = context_write(p, &c->context[0], fp);
> + if (rc)
> + return rc;
> + break;
> }
> }
> }
> diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> index 725d594..edb329d 100644
> --- a/security/selinux/ss/policydb.h
> +++ b/security/selinux/ss/policydb.h
> @@ -187,6 +187,15 @@ struct ocontext {
> u32 addr[4];
> u32 mask[4];
> } node6; /* IPv6 node information */
> + struct {
> + u64 subnet_prefix;
> + u16 low_pkey;
> + u16 high_pkey;
> + } pkey;
> + struct {
> + char *dev_name;
> + u8 port_num;
> + } ib_endport;
> } u;
> union {
> u32 sclass; /* security class for genfs */
> @@ -215,14 +224,16 @@ struct genfs {
> #define SYM_NUM 8
>
> /* object context array indices */
> -#define OCON_ISID 0 /* initial SIDs */
> -#define OCON_FS 1 /* unlabeled file systems */
> -#define OCON_PORT 2 /* TCP and UDP port numbers */
> -#define OCON_NETIF 3 /* network interfaces */
> -#define OCON_NODE 4 /* nodes */
> -#define OCON_FSUSE 5 /* fs_use */
> -#define OCON_NODE6 6 /* IPv6 nodes */
> -#define OCON_NUM 7
> +#define OCON_ISID 0 /* initial SIDs */
> +#define OCON_FS 1 /* unlabeled file systems */
> +#define OCON_PORT 2 /* TCP and UDP port numbers */
> +#define OCON_NETIF 3 /* network interfaces */
> +#define OCON_NODE 4 /* nodes */
> +#define OCON_FSUSE 5 /* fs_use */
> +#define OCON_NODE6 6 /* IPv6 nodes */
> +#define OCON_PKEY 7 /* Infiniband PKeys */
> +#define OCON_IB_ENDPORT 8 /* Infiniband end ports */
> +#define OCON_NUM 9
>
> /* The policy database */
> struct policydb {
>
--
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
^ permalink raw reply
* Re: [PATCH v6 3/9] selinux lsm IB/core: Implement LSM notification system
From: Stephen Smalley @ 2016-12-13 14:29 UTC (permalink / raw)
To: Dan Jurgens, chrisw-69jw2NvuJkxg9hUCZPvPmw,
paul-r2n+y4ga6xFZroRs9YW3xA, eparis-FjpueFixGhCM4zKIHC2jIg,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
selinux-+05T5uksL2qpZYMLLGbcSA
In-Reply-To: <1479910651-43246-4-git-send-email-danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
On 11/23/2016 09:17 AM, Dan Jurgens wrote:
> From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> Add a generic notificaiton mechanism in the LSM. Interested consumers
> can register a callback with the LSM and security modules can produce
> events.
>
> Because access to Infiniband QPs are enforced in the setup phase of a
> connection security should be enforced again if the policy changes.
> Register infiniband devices for policy change notification and check all
> QPs on that device when the notification is received.
>
> Add a call to the notification mechanism from SELinux when the AVC
> cache changes or setenforce is cleared.
>
> Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> ---
> v2:
> - new patch that has the generic notification, replaces selinux and
> IB/core patches related to the ib_flush callback. Yuval Shaia and Paul
> Moore
>
> v3:
> - use notifier chains. Paul Moore
>
> v4:
> - Seperate avc callback for LSM notifier. Paul Moore
>
> v5:
> - Fix link error when CONFIG_SECURITY is not set. Build Robot
> ---
> drivers/infiniband/core/device.c | 53 ++++++++++++++++++++++++++++++++++++++++
> include/linux/security.h | 23 +++++++++++++++++
> security/security.c | 20 +++++++++++++++
> security/selinux/hooks.c | 11 +++++++++
> security/selinux/selinuxfs.c | 2 ++
> 5 files changed, 109 insertions(+)
>
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 5b42e83..7b6fd06 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -39,6 +39,8 @@
> #include <linux/init.h>
> #include <linux/mutex.h>
> #include <linux/netdevice.h>
> +#include <linux/security.h>
> +#include <linux/notifier.h>
> #include <rdma/rdma_netlink.h>
> #include <rdma/ib_addr.h>
> #include <rdma/ib_cache.h>
> @@ -82,6 +84,14 @@ static LIST_HEAD(client_list);
> static DEFINE_MUTEX(device_mutex);
> static DECLARE_RWSEM(lists_rwsem);
>
> +static int ib_security_change(struct notifier_block *nb, unsigned long event,
> + void *lsm_data);
> +static void ib_policy_change_task(struct work_struct *work);
> +static DECLARE_WORK(ib_policy_change_work, ib_policy_change_task);
> +
> +static struct notifier_block ibdev_lsm_nb = {
> + .notifier_call = ib_security_change,
> +};
>
> static int ib_device_check_mandatory(struct ib_device *device)
> {
> @@ -344,6 +354,40 @@ static int setup_port_pkey_list(struct ib_device *device)
> return 0;
> }
>
> +static void ib_policy_change_task(struct work_struct *work)
> +{
> + struct ib_device *dev;
> +
> + down_read(&lists_rwsem);
> + list_for_each_entry(dev, &device_list, core_list) {
> + int i;
> +
> + for (i = rdma_start_port(dev); i <= rdma_end_port(dev); i++) {
> + u64 sp;
> + int ret = ib_get_cached_subnet_prefix(dev,
> + i,
> + &sp);
> +
> + WARN_ONCE(ret,
> + "ib_get_cached_subnet_prefix err: %d, this should never happen here\n",
> + ret);
> + ib_security_cache_change(dev, i, sp);
> + }
> + }
> + up_read(&lists_rwsem);
> +}
> +
> +static int ib_security_change(struct notifier_block *nb, unsigned long event,
> + void *lsm_data)
> +{
> + if (event != LSM_POLICY_CHANGE)
> + return NOTIFY_DONE;
> +
> + schedule_work(&ib_policy_change_work);
> +
> + return NOTIFY_OK;
> +}
> +
> /**
> * ib_register_device - Register an IB device with IB core
> * @device:Device to register
> @@ -1075,10 +1119,18 @@ static int __init ib_core_init(void)
> goto err_sa;
> }
>
> + ret = register_lsm_notifier(&ibdev_lsm_nb);
> + if (ret) {
> + pr_warn("Couldn't register LSM notifier. ret %d\n", ret);
> + goto err_ibnl_clients;
> + }
> +
> ib_cache_setup();
>
> return 0;
>
> +err_ibnl_clients:
> + ib_remove_ibnl_clients();
> err_sa:
> ib_sa_cleanup();
> err_mad:
> @@ -1098,6 +1150,7 @@ static int __init ib_core_init(void)
>
> static void __exit ib_core_cleanup(void)
> {
> + unregister_lsm_notifier(&ibdev_lsm_nb);
> ib_cache_cleanup();
> ib_remove_ibnl_clients();
> ib_sa_cleanup();
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 342ca4c..0a5de0c 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -69,6 +69,10 @@ struct audit_krule;
> struct user_namespace;
> struct timezone;
>
> +enum lsm_event {
> + LSM_POLICY_CHANGE,
> +};
> +
> /* These functions are in security/commoncap.c */
> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
> int cap, int audit);
> @@ -161,6 +165,10 @@ struct security_mnt_opts {
> int num_mnt_opts;
> };
>
> +int call_lsm_notifier(enum lsm_event event, void *data);
> +int register_lsm_notifier(struct notifier_block *nb);
> +int unregister_lsm_notifier(struct notifier_block *nb);
> +
> static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
> {
> opts->mnt_opts = NULL;
> @@ -377,6 +385,21 @@ int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
> struct security_mnt_opts {
> };
>
> +static inline int call_lsm_notifier(enum lsm_event event, void *data)
> +{
> + return 0;
> +}
> +
> +static inline int register_lsm_notifier(struct notifier_block *nb)
> +{
> + return 0;
> +}
> +
> +static inline int unregister_lsm_notifier(struct notifier_block *nb)
> +{
> + return 0;
> +}
> +
> static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
> {
> }
> diff --git a/security/security.c b/security/security.c
> index 7d3bf2f..40326d4 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -33,6 +33,8 @@
> /* Maximum number of letters for an LSM name string */
> #define SECURITY_NAME_MAX 10
>
> +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
> +
> /* Boot-time LSM user choice */
> static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
> CONFIG_DEFAULT_SECURITY;
> @@ -98,6 +100,24 @@ int __init security_module_enable(const char *module)
> return !strcmp(module, chosen_lsm);
> }
>
> +int call_lsm_notifier(enum lsm_event event, void *data)
> +{
> + return atomic_notifier_call_chain(&lsm_notifier_chain, event, data);
> +}
> +EXPORT_SYMBOL(call_lsm_notifier);
> +
> +int register_lsm_notifier(struct notifier_block *nb)
> +{
> + return atomic_notifier_chain_register(&lsm_notifier_chain, nb);
> +}
> +EXPORT_SYMBOL(register_lsm_notifier);
> +
> +int unregister_lsm_notifier(struct notifier_block *nb)
> +{
> + return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb);
> +}
> +EXPORT_SYMBOL(unregister_lsm_notifier);
> +
> /*
> * Hook list operation macros.
> *
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 09fd610..2d7a7c1 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -170,6 +170,14 @@ static int selinux_netcache_avc_callback(u32 event)
> return 0;
> }
>
> +static int selinux_lsm_notifier_avc_callback(u32 event)
> +{
> + if (event == AVC_CALLBACK_RESET)
> + call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
> +
> + return 0;
> +}
> +
> /*
> * initialise the security for the init task
> */
> @@ -6325,6 +6333,9 @@ static __init int selinux_init(void)
> if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
> panic("SELinux: Unable to register AVC netcache callback\n");
>
> + if (avc_add_callback(selinux_lsm_notifier_avc_callback, AVC_CALLBACK_RESET))
> + panic("SELinux: Unable to register AVC LSM notifier callback\n");
> +
> if (selinux_enforcing)
> printk(KERN_DEBUG "SELinux: Starting in enforcing mode\n");
> else
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 72c145d..d3f9192 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -177,6 +177,8 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
> avc_ss_reset(0);
> selnl_notify_setenforce(selinux_enforcing);
> selinux_status_update_setenforce(selinux_enforcing);
> + if (!selinux_enforcing)
> + call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
Why do you need this notification? When switching from permissive to
enforcing, you need (and already get) a notification since you may need
to revoke previously granted permissions. But what action do you need
to take on switching to permissive?
> }
> length = count;
> out:
>
--
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
^ permalink raw reply
* Re: [PATCH rdma-rc 2/5] IB/rxe: Fix handling of erroneous WR
From: Leon Romanovsky @ 2016-12-13 12:20 UTC (permalink / raw)
To: Bart Van Assche
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yonatan Cohen
In-Reply-To: <ba254635-c8f9-7b3b-eb73-60075d079542-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]
On Tue, Dec 13, 2016 at 09:04:44AM +0100, Bart Van Assche wrote:
> On 12/13/2016 08:44 AM, Leon Romanovsky wrote:
> > On Tue, Dec 13, 2016 at 08:03:03AM +0100, Bart Van Assche wrote:
> > > On 11/16/2016 09:39 AM, Leon Romanovsky wrote:
> > > > @@ -745,13 +746,17 @@ int rxe_requester(void *arg)
> > > > wqe->status = IB_WC_LOC_PROT_ERR;
> > > > wqe->state = wqe_state_error;
> > > >
> > > > -complete:
> > > > - if (qp_type(qp) != IB_QPT_RC) {
> > > > - while (rxe_completer(qp) == 0)
> > > > - ;
> > > > - }
> > > > -
> > > > - return 0;
> > > > + /*
> > > > + * IBA Spec. Section 10.7.3.1 SIGNALED COMPLETIONS
> > > > + * ---------8<---------8<-------------
> > > > + * ...Note that if a completion error occurs, a Work Completion
> > > > + * will always be generated, even if the signaling
> > > > + * indicator requests an Unsignaled Completion.
> > > > + * ---------8<---------8<-------------
> > > > + */
> > > > + wqe->wr.send_flags |= IB_SEND_SIGNALED;
> > > > + __rxe_do_task(&qp->comp.task);
> > > > + return -EAGAIN;
> > >
> > > Hello Leon and Yonatan,
> > >
> > > Sorry for the late reply but I think setting IB_SEND_SIGNALED for WQE's
> > > reporting a completion error is wrong.
> >
> > I'm not clear about it. I didn't find in spec what to do with IB_SEND_SIGNALED
> > flag in case of error.
> >
> > According to spec:
> > "C10-91: The CI shall generate a CQE when a Work Request
> > completed under any of the following conditions:
> > • The Work Request completed in error"
>
> Hello Leon,
>
> To me that paragraph from the spec means that do_complete() is wrong. And
> once do_complete() is fixed, callers that set the WQE status to "error" no
> longer have to set IB_SEND_SIGNALED.
Thanks,
It looks like a right thing to do. Yonatan is handling it.
>
> Bart.
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH V2 00/22] Broadcom RoCE Driver (bnxt_re)
From: Selvin Xavier @ 2016-12-13 8:36 UTC (permalink / raw)
To: Or Gerlitz
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Netdev List
In-Reply-To: <CAJ3xEMh98kC1KXGf7uHKD-H91f_NiZXaz-3yTtwQ2s-D7rYqMQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Dec 13, 2016 at 1:29 PM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> I made some quick on-the-surface static checkers etc rub on the new
> driver (Doug, I used
> the bits in your github bnxt_re branch), there are bunch (tons...) of
> smatch [1] and sparse [2]
> complaints along with few checkpatch [3] things too. So... addressing
> them before this goes upstream?
Thank you for pointing it out. I will include this cleanup also as a
part of my v3 series.
--
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
^ permalink raw reply
* Re: [PATCH rdma-rc 2/5] IB/rxe: Fix handling of erroneous WR
From: Bart Van Assche @ 2016-12-13 8:04 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yonatan Cohen
In-Reply-To: <20161213074441.GE8204-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
On 12/13/2016 08:44 AM, Leon Romanovsky wrote:
> On Tue, Dec 13, 2016 at 08:03:03AM +0100, Bart Van Assche wrote:
>> On 11/16/2016 09:39 AM, Leon Romanovsky wrote:
>>> @@ -745,13 +746,17 @@ int rxe_requester(void *arg)
>>> wqe->status = IB_WC_LOC_PROT_ERR;
>>> wqe->state = wqe_state_error;
>>>
>>> -complete:
>>> - if (qp_type(qp) != IB_QPT_RC) {
>>> - while (rxe_completer(qp) == 0)
>>> - ;
>>> - }
>>> -
>>> - return 0;
>>> + /*
>>> + * IBA Spec. Section 10.7.3.1 SIGNALED COMPLETIONS
>>> + * ---------8<---------8<-------------
>>> + * ...Note that if a completion error occurs, a Work Completion
>>> + * will always be generated, even if the signaling
>>> + * indicator requests an Unsignaled Completion.
>>> + * ---------8<---------8<-------------
>>> + */
>>> + wqe->wr.send_flags |= IB_SEND_SIGNALED;
>>> + __rxe_do_task(&qp->comp.task);
>>> + return -EAGAIN;
>>
>> Hello Leon and Yonatan,
>>
>> Sorry for the late reply but I think setting IB_SEND_SIGNALED for WQE's
>> reporting a completion error is wrong.
>
> I'm not clear about it. I didn't find in spec what to do with IB_SEND_SIGNALED
> flag in case of error.
>
> According to spec:
> "C10-91: The CI shall generate a CQE when a Work Request
> completed under any of the following conditions:
> • The Work Request completed in error"
Hello Leon,
To me that paragraph from the spec means that do_complete() is wrong.
And once do_complete() is fixed, callers that set the WQE status to
"error" no longer have to set IB_SEND_SIGNALED.
Bart.
--
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
^ permalink raw reply
* Re: [PATCH V2 00/22] Broadcom RoCE Driver (bnxt_re)
From: Or Gerlitz @ 2016-12-13 7:59 UTC (permalink / raw)
To: Doug Ledford, Selvin Xavier
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Netdev List
In-Reply-To: <23e26353-4317-2836-9f94-d1fc3274a770-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Dec 13, 2016 at 1:52 AM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 12/9/2016 1:47 AM, Selvin Xavier wrote:
>> This series introduces the RoCE driver for the Broadcom
>> NetXtreme-E 10/25/40/50 gigabit RoCE HCAs.
>> This driver is dependent on the bnxt_en NIC driver and is
>> based on the bnxt_re branch in Doug's repository. bnxt_en changes
>> required for this patch series is already available in this branch.
>>
>> I am preparing a git repository with these changes as per Jason's
>> comment and will share the details later today.
>>
>> v1-> v2:
>> * The license text in each file updated to reflect Dual license.
>> * Makefile and Kconfig changes are pushed to the last patch
>> * Moved bnxt_re_uverbs_abi.h to include/uapi/rdma folder
>> * Remove duplicate structure definitions from bnxt_re_hsi.h as
>> it is available in the corresponding bnxt_en header file (bnxt_hsi.h)
>> * Removed some unused code reported during code review.
>> * Fixed few sparse warnings
>>
>> Doug,
>> Please review and consider applying this to linux-rdma repository.
>
> There are outstanding review comments to be addressed still yet, and the
> v2 patchset doesn't compile for me in 0day testing. I'm going to bounce
> this one to 4.11.
I made some quick on-the-surface static checkers etc rub on the new
driver (Doug, I used
the bits in your github bnxt_re branch), there are bunch (tons...) of
smatch [1] and sparse [2]
complaints along with few checkpatch [3] things too. So... addressing
them before this goes upstream?
BTW net-next's drivers/net/ethernet/broadcom/bnxt (where you put the
pre patches for the
RDMA driver) is pretty much clean of that
Or.
[1] smatch
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_main.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_debugfs.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:185
bnxt_qplib_del_sgid() warn: impossible condition '(*sgid_tbl->hw_id +
index == -1) => (0-65535 == (-1))'
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:107
bnxt_qplib_rcfw_send_message() warn: test_bit() takes a bit number
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:116
bnxt_qplib_rcfw_send_message() warn: test_bit() takes a bit number
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:148
bnxt_qplib_rcfw_send_message() error: double lock 'irqsave:flags'
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:204
bnxt_qplib_rcfw_send_message() error: double unlock 'irqsave:flags'
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:121
bnxt_re_unregister_netdev() warn: variable dereferenced before check
'rdev' (see line 118)
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:140
bnxt_re_register_netdev() warn: variable dereferenced before check
'rdev' (see line 137)
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:155 bnxt_re_free_msix()
warn: variable dereferenced before check 'rdev' (see line 152)
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:173
bnxt_re_request_msix() warn: variable dereferenced before check 'rdev'
(see line 171)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:172
bnxt_qplib_alloc_init_hwq() warn: unsigned '*elements' is never less
than zero.
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:387
bnxt_qplib_deinit_rcfw() warn: test_bit() takes a bit number
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:488
bnxt_qplib_alloc_sgid_tbl() warn: double check that we're allocating
correct size: 2 vs 4
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:688
bnxt_qplib_alloc_dpi_tbl() warn: consider using resource_size() here
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:496
bnxt_qplib_init_rcfw() warn: test_bit() takes a bit number
./include/linux/mm.h:1385 stack_guard_page_end() warn: bitwise AND
condition is false here
./include/linux/mm.h:1379 vma_growsup() warn: bitwise AND condition is
false here
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:522 show_rev() info:
ignoring unreachable code.
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:835 bnxt_re_dev_stop()
warn: was && intended here instead of ||?
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:1048 bnxt_re_ib_reg()
warn: curly braces intended?
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:1050 bnxt_re_ib_reg()
warn: inconsistent indenting
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:1190 bnxt_re_task() warn:
curly braces intended?
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1639 __flush_sq() warn:
variable dereferenced before check 'budget' (see line 1621)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1852
bnxt_qplib_cq_process_res_ud() warn: shift has higher precedence than
mask
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1861
bnxt_qplib_cq_process_res_ud() warn: inconsistent indenting
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:2015
bnxt_qplib_cq_process_terminal() warn: variable dereferenced before
check 'budget' (see line 1997)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1607
bnxt_re_build_qp1_send_v2 warn: unused return: size =
ib_ud_header_pack()
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1681
bnxt_re_build_qp1_shadow_qp_recv() warn: unsigned 'sge.size' is never
less than zero.
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2066
bnxt_re_post_recv_shadow_qp warn: unused return: payload_sz =
bnxt_re_build_sgl()
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2242
bnxt_re_create_cq() warn: variable dereferenced before check
'cq->umem' (see line 2192)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2456
bnxt_re_is_loopback_packet() warn: curly braces intended?
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2512
bnxt_re_process_raw_qp_pkt_rx() warn: impossible condition '(pkt_type
< 0) => (0-255 < 0)'
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2578
bnxt_re_process_raw_qp_pkt_rx warn: unused return: rc =
bnxt_re_post_send_shadow_qp()
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2838 bnxt_re_dereg_mr
warn: unused return: rc = bnxt_qplib_free_fast_reg_page_list()
[2] sparse
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_main.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_debugfs.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:111:33: warning: cast to
restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:114:30: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:114:30: warning: cast to
restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:277:28: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:277:28: expected
restricted __le32 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:277:28: got restricted
__be32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:278:28: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:278:28: expected
restricted __le32 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:278:28: got restricted
__be32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:279:28: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:279:28: expected
restricted __le32 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:279:28: got restricted
__be32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:280:28: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:280:28: expected
restricted __le32 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:280:28: got restricted
__be32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:282:34: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:282:34: expected
restricted __le16 [addressable] [assigned] [usertype] vlan
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:282:34: got restricted
__le32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:289:32: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:289:32: expected
restricted __le16 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:289:32: got restricted
__be16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:290:32: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:290:32: expected
restricted __le16 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:290:32: got restricted
__be16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:291:32: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:291:32: expected
restricted __le16 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:291:32: got restricted
__be16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:810:18: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:810:18: expected
restricted __le16 [addressable] [assigned] [usertype] cos0
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:810:18: got unsigned
short [unsigned] [short] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:811:18: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:811:18: expected
restricted __le16 [addressable] [assigned] [usertype] cos1
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:811:18: got unsigned
short [unsigned] [short] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:133:22: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:136:24: warning:
restricted __le16 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:136:24: warning: cast to
restricted __le16
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:777:25: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:777:25: expected
restricted __le32 [addressable] [assigned] [usertype] modify_mask
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:777:25: got restricted
__le64 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:822:30: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:822:30: expected
unsigned char [unsigned] [addressable] [assigned] [usertype] path_mtu
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:822:30: got restricted
__le16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:953:26: warning:
restricted __le16 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:953:26: warning: cast to
restricted __le16
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:956:26: warning:
restricted __le16 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:970:26: warning: cast to
restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:970:26: warning: cast
from restricted __le16
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1211:34: warning:
invalid assignment: +=
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1211:34: left side has type int
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1211:34: right side has
type restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1333:24: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1333:24: expected
unsigned int [unsigned] [usertype] temp32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1333:24: got restricted
__le32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1343:46: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1343:46: expected
unsigned long long [unsigned] [long] [long long] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1343:46: got restricted
__le64 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1363:24: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1363:24: expected
unsigned int [unsigned] [addressable] [usertype] temp32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1363:24: got restricted
__le32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1533:27: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1533:27: expected
restricted __le32 [addressable] [assigned] [usertype] cq_fco_cnq_id
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1533:27: got restricted
__le16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1796:21: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1796:21: warning: cast
to restricted __le64
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1849:21: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1849:21: warning: cast
to restricted __le64
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1852:39: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1904:21: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1904:21: warning: cast
to restricted __le64
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1917:34: warning: cast
to restricted __le16
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1917:34: warning: cast
from restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1015:22: warning:
context imbalance in 'bnxt_qplib_lock_cqs' - wrong count at exit
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1030:28: warning:
context imbalance in 'bnxt_qplib_unlock_cqs' - unexpected unlock
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:410:72: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:410:72: expected
unsigned long long [unsigned] [long] [long long] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:410:72: got restricted
__le64 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:418:56: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:418:56: expected
unsigned long long [unsigned] [long] [long long] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:418:56: got restricted
__le64 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:729:6: warning: symbol
'bnxt_qplib_cleanup_pkey_tbl' was not declared. Should it be static?
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1725:47: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1725:47: expected
unsigned int [unsigned] [usertype] imm_data_or_inv_key
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1725:47: got
restricted __be32 [usertype] imm_data
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1755:47: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1755:47: expected
unsigned int [unsigned] [usertype] imm_data_or_inv_key
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1755:47: got
restricted __be32 [usertype] imm_data
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2627:25: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2627:25: expected
restricted __be32 [usertype] imm_data
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2627:25: got unsigned
int [unsigned] [usertype] immdata_or_invrkey
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2696:41: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2696:41: expected
restricted __be32 [usertype] imm_data
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2696:41: got unsigned
int [unsigned] [usertype] immdata_or_invrkey
[3] checkpatch
CHECK: Macro argument reuse 'req' - possible side-effects?
CHECK: Macro argument reuse 'prod' - possible side-effects?
CHECK: Macro argument reuse 'dma_addr' - possible side-effects?
CHECK: Macro argument reuse 'rdev' - possible side-effects?
CHECK: Please don't use multiple blank lines
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Please don't use multiple blank lines
CHECK: DEFINE_MUTEX definition without comment
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Alignment should match open parenthesis
CHECK: Please use a blank line after function/struct/union/enum declarations
--
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
^ permalink raw reply
* Re: [PATCH rdma-rc 2/5] IB/rxe: Fix handling of erroneous WR
From: Leon Romanovsky @ 2016-12-13 7:44 UTC (permalink / raw)
To: Bart Van Assche
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yonatan Cohen
In-Reply-To: <5bd83de1-64d3-e5a9-1c58-cca52d89d64a-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]
On Tue, Dec 13, 2016 at 08:03:03AM +0100, Bart Van Assche wrote:
> On 11/16/2016 09:39 AM, Leon Romanovsky wrote:
> > @@ -745,13 +746,17 @@ int rxe_requester(void *arg)
> > wqe->status = IB_WC_LOC_PROT_ERR;
> > wqe->state = wqe_state_error;
> >
> > -complete:
> > - if (qp_type(qp) != IB_QPT_RC) {
> > - while (rxe_completer(qp) == 0)
> > - ;
> > - }
> > -
> > - return 0;
> > + /*
> > + * IBA Spec. Section 10.7.3.1 SIGNALED COMPLETIONS
> > + * ---------8<---------8<-------------
> > + * ...Note that if a completion error occurs, a Work Completion
> > + * will always be generated, even if the signaling
> > + * indicator requests an Unsignaled Completion.
> > + * ---------8<---------8<-------------
> > + */
> > + wqe->wr.send_flags |= IB_SEND_SIGNALED;
> > + __rxe_do_task(&qp->comp.task);
> > + return -EAGAIN;
>
> Hello Leon and Yonatan,
>
> Sorry for the late reply but I think setting IB_SEND_SIGNALED for WQE's
> reporting a completion error is wrong.
I'm not clear about it. I didn't find in spec what to do with IB_SEND_SIGNALED
flag in case of error.
According to spec:
"C10-91: The CI shall generate a CQE when a Work Request
completed under any of the following conditions:
• The Work Request completed in error"
> I noticed this because this change
> conflicts with Doug's rxe branch on github. Have you considered to apply
> the following (untested) patch instead of setting the IB_SEND_SIGNALED
> flag?
>
> --- a/drivers/infiniband/sw/rxe/rxe_comp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_comp.c
> @@ -418,7 +418,7 @@ static void do_complete(struct rxe_qp *qp, struct rxe_send_w
> qe *wqe)
>
> if ((qp->sq_sig_type == IB_SIGNAL_ALL_WR) ||
> (wqe->wr.send_flags & IB_SEND_SIGNALED) ||
> - (qp->req.state == QP_STATE_ERROR)) {
> + wqe->status != IB_WC_SUCCESS) {
> make_send_cqe(qp, wqe, &cqe);
> advance_consumer(qp->sq.queue);
> rxe_cq_post(qp->scq, &cqe, 0);
>
> Bart.
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH rdma-rc 2/5] IB/rxe: Fix handling of erroneous WR
From: Bart Van Assche @ 2016-12-13 7:03 UTC (permalink / raw)
To: Leon Romanovsky, dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yonatan Cohen
In-Reply-To: <1479285558-19627-3-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On 11/16/2016 09:39 AM, Leon Romanovsky wrote:
> @@ -745,13 +746,17 @@ int rxe_requester(void *arg)
> wqe->status = IB_WC_LOC_PROT_ERR;
> wqe->state = wqe_state_error;
>
> -complete:
> - if (qp_type(qp) != IB_QPT_RC) {
> - while (rxe_completer(qp) == 0)
> - ;
> - }
> -
> - return 0;
> + /*
> + * IBA Spec. Section 10.7.3.1 SIGNALED COMPLETIONS
> + * ---------8<---------8<-------------
> + * ...Note that if a completion error occurs, a Work Completion
> + * will always be generated, even if the signaling
> + * indicator requests an Unsignaled Completion.
> + * ---------8<---------8<-------------
> + */
> + wqe->wr.send_flags |= IB_SEND_SIGNALED;
> + __rxe_do_task(&qp->comp.task);
> + return -EAGAIN;
Hello Leon and Yonatan,
Sorry for the late reply but I think setting IB_SEND_SIGNALED for WQE's
reporting a completion error is wrong. I noticed this because this change
conflicts with Doug's rxe branch on github. Have you considered to apply
the following (untested) patch instead of setting the IB_SEND_SIGNALED
flag?
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -418,7 +418,7 @@ static void do_complete(struct rxe_qp *qp, struct rxe_send_w
qe *wqe)
if ((qp->sq_sig_type == IB_SIGNAL_ALL_WR) ||
(wqe->wr.send_flags & IB_SEND_SIGNALED) ||
- (qp->req.state == QP_STATE_ERROR)) {
+ wqe->status != IB_WC_SUCCESS) {
make_send_cqe(qp, wqe, &cqe);
advance_consumer(qp->sq.queue);
rxe_cq_post(qp->scq, &cqe, 0);
Bart.
--
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
^ permalink raw reply
* Re: [PATCH V2 00/22] Broadcom RoCE Driver (bnxt_re)
From: Or Gerlitz @ 2016-12-13 6:54 UTC (permalink / raw)
To: Selvin Xavier; +Cc: dledford, linux-rdma, netdev
In-Reply-To: <1481266096-23331-1-git-send-email-selvin.xavier@broadcom.com>
On 12/9/2016 8:47 AM, Selvin Xavier wrote:
> This series introduces the RoCE driver for the Broadcom
> NetXtreme-E 10/25/40/50 gigabit RoCE HCAs.
> This driver is dependent on the bnxt_en NIC driver and is
> based on the bnxt_re branch in Doug's repository. bnxt_en changes
> required for this patch series is already available in this branch.
>
> I am preparing a git repository with these changes as per Jason's
> comment and will share the details later today.
>
> v1-> v2:
> * The license text in each file updated to reflect Dual license.
> * Makefile and Kconfig changes are pushed to the last patch
> * Moved bnxt_re_uverbs_abi.h to include/uapi/rdma folder
> * Remove duplicate structure definitions from bnxt_re_hsi.h as
> it is available in the corresponding bnxt_en header file (bnxt_hsi.h)
> * Removed some unused code reported during code review.
> * Fixed few sparse warnings
>
> Doug, Please review and consider applying this to linux-rdma repository.
Hi,
I made some quick on-the-surface static checkers etc rub on the new
driver (Doug, I used the bits in your github bnxt_re branch), there are
bunch (tons...) of smatch [1] and sparse [2]complaints along with few
checkpatch [3] things too. So... addressing them before this goes upstream?
BTW net-next's drivers/net/ethernet/broadcom/bnxt (where you put the pre
patches for the RDMA driver) is pretty much clean of that
Or.
[1] smatch
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_main.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_debugfs.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:185 bnxt_qplib_del_sgid()
warn: impossible condition '(*sgid_tbl->hw_id + index == -1) => (0-65535
== (-1))'
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:107
bnxt_qplib_rcfw_send_message() warn: test_bit() takes a bit number
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:116
bnxt_qplib_rcfw_send_message() warn: test_bit() takes a bit number
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:148
bnxt_qplib_rcfw_send_message() error: double lock 'irqsave:flags'
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:204
bnxt_qplib_rcfw_send_message() error: double unlock 'irqsave:flags'
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:121
bnxt_re_unregister_netdev() warn: variable dereferenced before check
'rdev' (see line 118)
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:140
bnxt_re_register_netdev() warn: variable dereferenced before check
'rdev' (see line 137)
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:155 bnxt_re_free_msix()
warn: variable dereferenced before check 'rdev' (see line 152)
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:173 bnxt_re_request_msix()
warn: variable dereferenced before check 'rdev' (see line 171)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:172
bnxt_qplib_alloc_init_hwq() warn: unsigned '*elements' is never less
than zero.
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:387
bnxt_qplib_deinit_rcfw() warn: test_bit() takes a bit number
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:488
bnxt_qplib_alloc_sgid_tbl() warn: double check that we're allocating
correct size: 2 vs 4
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:688
bnxt_qplib_alloc_dpi_tbl() warn: consider using resource_size() here
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:496
bnxt_qplib_init_rcfw() warn: test_bit() takes a bit number
./include/linux/mm.h:1385 stack_guard_page_end() warn: bitwise AND
condition is false here
./include/linux/mm.h:1379 vma_growsup() warn: bitwise AND condition is
false here
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:522 show_rev() info:
ignoring unreachable code.
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:835 bnxt_re_dev_stop()
warn: was && intended here instead of ||?
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:1048 bnxt_re_ib_reg() warn:
curly braces intended?
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:1050 bnxt_re_ib_reg() warn:
inconsistent indenting
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:1190 bnxt_re_task() warn:
curly braces intended?
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1639 __flush_sq() warn:
variable dereferenced before check 'budget' (see line 1621)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1852
bnxt_qplib_cq_process_res_ud() warn: shift has higher precedence than mask
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1861
bnxt_qplib_cq_process_res_ud() warn: inconsistent indenting
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:2015
bnxt_qplib_cq_process_terminal() warn: variable dereferenced before
check 'budget' (see line 1997)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1607
bnxt_re_build_qp1_send_v2 warn: unused return: size = ib_ud_header_pack()
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1681
bnxt_re_build_qp1_shadow_qp_recv() warn: unsigned 'sge.size' is never
less than zero.
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2066
bnxt_re_post_recv_shadow_qp warn: unused return: payload_sz =
bnxt_re_build_sgl()
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2242
bnxt_re_create_cq() warn: variable dereferenced before check 'cq->umem'
(see line 2192)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2456
bnxt_re_is_loopback_packet() warn: curly braces intended?
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2512
bnxt_re_process_raw_qp_pkt_rx() warn: impossible condition '(pkt_type <
0) => (0-255 < 0)'
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2578
bnxt_re_process_raw_qp_pkt_rx warn: unused return: rc =
bnxt_re_post_send_shadow_qp()
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2838 bnxt_re_dereg_mr
warn: unused return: rc = bnxt_qplib_free_fast_reg_page_list()
[2] sparse
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_main.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_debugfs.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:111:33: warning: cast to
restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:114:30: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:114:30: warning: cast to
restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:277:28: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:277:28: expected
restricted __le32 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:277:28: got restricted
__be32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:278:28: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:278:28: expected
restricted __le32 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:278:28: got restricted
__be32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:279:28: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:279:28: expected
restricted __le32 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:279:28: got restricted
__be32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:280:28: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:280:28: expected
restricted __le32 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:280:28: got restricted
__be32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:282:34: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:282:34: expected
restricted __le16 [addressable] [assigned] [usertype] vlan
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:282:34: got restricted
__le32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:289:32: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:289:32: expected
restricted __le16 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:289:32: got restricted
__be16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:290:32: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:290:32: expected
restricted __le16 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:290:32: got restricted
__be16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:291:32: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:291:32: expected
restricted __le16 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:291:32: got restricted
__be16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:810:18: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:810:18: expected
restricted __le16 [addressable] [assigned] [usertype] cos0
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:810:18: got unsigned short
[unsigned] [short] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:811:18: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:811:18: expected
restricted __le16 [addressable] [assigned] [usertype] cos1
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:811:18: got unsigned short
[unsigned] [short] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:133:22: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:136:24: warning:
restricted __le16 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:136:24: warning: cast to
restricted __le16
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:777:25: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:777:25: expected
restricted __le32 [addressable] [assigned] [usertype] modify_mask
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:777:25: got restricted
__le64 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:822:30: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:822:30: expected unsigned
char [unsigned] [addressable] [assigned] [usertype] path_mtu
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:822:30: got restricted
__le16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:953:26: warning:
restricted __le16 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:953:26: warning: cast to
restricted __le16
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:956:26: warning:
restricted __le16 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:970:26: warning: cast to
restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:970:26: warning: cast from
restricted __le16
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1211:34: warning: invalid
assignment: +=
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1211:34: left side has
type int
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1211:34: right side has
type restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1333:24: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1333:24: expected unsigned
int [unsigned] [usertype] temp32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1333:24: got restricted
__le32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1343:46: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1343:46: expected unsigned
long long [unsigned] [long] [long long] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1343:46: got restricted
__le64 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1363:24: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1363:24: expected unsigned
int [unsigned] [addressable] [usertype] temp32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1363:24: got restricted
__le32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1533:27: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1533:27: expected
restricted __le32 [addressable] [assigned] [usertype] cq_fco_cnq_id
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1533:27: got restricted
__le16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1796:21: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1796:21: warning: cast to
restricted __le64
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1849:21: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1849:21: warning: cast to
restricted __le64
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1852:39: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1904:21: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1904:21: warning: cast to
restricted __le64
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1917:34: warning: cast to
restricted __le16
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1917:34: warning: cast
from restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1015:22: warning: context
imbalance in 'bnxt_qplib_lock_cqs' - wrong count at exit
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1030:28: warning: context
imbalance in 'bnxt_qplib_unlock_cqs' - unexpected unlock
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:410:72: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:410:72: expected unsigned
long long [unsigned] [long] [long long] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:410:72: got restricted
__le64 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:418:56: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:418:56: expected unsigned
long long [unsigned] [long] [long long] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:418:56: got restricted
__le64 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:729:6: warning: symbol
'bnxt_qplib_cleanup_pkey_tbl' was not declared. Should it be static?
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1725:47: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1725:47: expected
unsigned int [unsigned] [usertype] imm_data_or_inv_key
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1725:47: got restricted
__be32 [usertype] imm_data
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1755:47: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1755:47: expected
unsigned int [unsigned] [usertype] imm_data_or_inv_key
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1755:47: got restricted
__be32 [usertype] imm_data
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2627:25: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2627:25: expected
restricted __be32 [usertype] imm_data
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2627:25: got unsigned
int [unsigned] [usertype] immdata_or_invrkey
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2696:41: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2696:41: expected
restricted __be32 [usertype] imm_data
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2696:41: got unsigned
int [unsigned] [usertype] immdata_or_invrkey
[3] checkpatch
CHECK: Macro argument reuse 'req' - possible side-effects?
CHECK: Macro argument reuse 'prod' - possible side-effects?
CHECK: Macro argument reuse 'dma_addr' - possible side-effects?
CHECK: Macro argument reuse 'rdev' - possible side-effects?
CHECK: Please don't use multiple blank lines
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Please don't use multiple blank lines
CHECK: DEFINE_MUTEX definition without comment
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Alignment should match open parenthesis
CHECK: Please use a blank line after function/struct/union/enum declarations
^ permalink raw reply
* Re: [PATCH V2 00/22] Broadcom RoCE Driver (bnxt_re)
From: Michael Chan @ 2016-12-13 6:41 UTC (permalink / raw)
To: Selvin Xavier; +Cc: jtoppins, Doug Ledford, linux-rdma, Netdev
In-Reply-To: <CA+sbYW2fF01N5ZQfaH6Uj7Q63SHwdo1gEwR=eoB6vB1ktQaiag@mail.gmail.com>
On Mon, Dec 12, 2016 at 8:52 PM, Selvin Xavier
<selvin.xavier@broadcom.com> wrote:
>> CHECK drivers/infiniband/hw/bnxtre/bnxt_qplib_rcfw.c
>> CHECK drivers/infiniband/hw/bnxtre/bnxt_qplib_sp.c
>> CHECK drivers/infiniband/hw/bnxtre/bnxt_qplib_fp.c
>> drivers/infiniband/hw/bnxtre/bnxt_qplib_fp.c:1015:22: warning: context
>> imbalance in 'bnxt_qplib_lock_cqs' - wrong count at exit
>> drivers/infiniband/hw/bnxtre/bnxt_qplib_fp.c:1030:28: warning: context
>> imbalance in 'bnxt_qplib_unlock_cqs' - unexpected unlock
> The above two are false positives, since locking and unlocking are
> handled in two separate functions. This is a wrapper to lock/unlock
> both SQ and RQ CQ locks. Functionally it is ok since
> bnxt_qplib_unlock_cqs is called just after the critical section and
> both locks are freed in order. I think we can ignore this warning.
>
>
You can use __releases() and __acquires() macros to denote these cases
for sparse.
^ permalink raw reply
* Re: [PATCH V2 18/22] bnxt_re: Support for DCB
From: Selvin Xavier @ 2016-12-13 6:25 UTC (permalink / raw)
To: Or Gerlitz
Cc: Doug Ledford, linux-rdma@vger.kernel.org, Linux Netdev List,
Eddie Wai, Devesh Sharma, Somnath Kotur, Sriharsha Basavapatna
In-Reply-To: <CAJ3xEMjcKRZpBMEBwQMZO5OqbVzH2=Q3FOAyYUVAz+ouw1jTNQ@mail.gmail.com>
On Sat, Dec 10, 2016 at 7:20 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Fri, Dec 9, 2016 at 8:48 AM, Selvin Xavier
> <selvin.xavier@broadcom.com> wrote:
>> This patch queries the configured RoCE APP Priority on the host
>> using the dcbnl API and programs the RoCE FW with the corresponding
>> Traffic Class(es) for the priority.
>
>> +#define BNXT_RE_ROCE_V1_ETH_TYPE 0x8915
>> +#define BNXT_RE_ROCE_V2_PORT_NO 4791
>
> I believe these two are defined already, try # git grep on each under include
Thanks Or for your comments.
V2 port number is defined in ib_verbs.h. i will include this in the
next patch set.
v1 eth_type is not defined. All vendor drivers have their own definition.
Thanks,
Selvin
^ permalink raw reply
* Re: [PATCH V2 13/22] bnxt_re: Support QP verbs
From: Selvin Xavier @ 2016-12-13 6:08 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Eddie Wai, Devesh Sharma,
Somnath Kotur, Sriharsha Basavapatna
In-Reply-To: <20161212182737.GC8204-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
On Mon, Dec 12, 2016 at 11:57 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> It can help to review if you break this function into smaller pieces and
> get rid of switch->switch->if construction.
Thanks Leon. I will address this and your previous comments in v3 patch set.
--
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
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox