netdev - RE: [RFC PATCH v5 09/16] RDMA/irdma: Implement device supported verb APIs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9DD61F30A802C4429A01CA4200E302A7DCD485DF@fmsmsx124.amr.corp.intel.com>
Date:   Tue, 21 Apr 2020 00:29:43 +0000
From:   "Saleem, Shiraz" <shiraz.saleem@...el.com>
To:     Leon Romanovsky <leon@...nel.org>,
        "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:     "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "jgg@...pe.ca" <jgg@...pe.ca>,
        "Ismail, Mustafa" <mustafa.ismail@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "nhorman@...hat.com" <nhorman@...hat.com>,
        "sassmann@...hat.com" <sassmann@...hat.com>
Subject: RE: [RFC PATCH v5 09/16] RDMA/irdma: Implement device supported
 verb APIs

> Subject: Re: [RFC PATCH v5 09/16] RDMA/irdma: Implement device supported
> verb APIs
> 
> On Fri, Apr 17, 2020 at 10:12:44AM -0700, Jeff Kirsher wrote:
> > From: Mustafa Ismail <mustafa.ismail@...el.com>
> >
> > Implement device supported verb APIs. The supported APIs vary based on
> > the underlying transport the ibdev is registered as (i.e. iWARP or
> > RoCEv2).
> >
> > Signed-off-by: Mustafa Ismail <mustafa.ismail@...el.com>
> > Signed-off-by: Shiraz Saleem <shiraz.saleem@...el.com>
> > ---
> >  drivers/infiniband/hw/irdma/verbs.c     | 4555 +++++++++++++++++++++++
> >  drivers/infiniband/hw/irdma/verbs.h     |  213 ++
> >  include/uapi/rdma/ib_user_ioctl_verbs.h |    1 +
> >  3 files changed, 4769 insertions(+)
> >  create mode 100644 drivers/infiniband/hw/irdma/verbs.c
> >  create mode 100644 drivers/infiniband/hw/irdma/verbs.h
> 
> <...>
> 
> > +static int irdma_destroy_qp(struct ib_qp *ibqp, struct ib_udata
> > +*udata) {
> > +	struct irdma_qp *iwqp = to_iwqp(ibqp);
> > +
> > +	iwqp->destroyed = 1;
> > +	if (iwqp->ibqp_state >= IB_QPS_INIT && iwqp->ibqp_state <
> IB_QPS_RTS)
> > +		irdma_next_iw_state(iwqp, IRDMA_QP_STATE_ERROR, 0, 0, 0);
> > +
> > +	if (!iwqp->user_mode) {
> > +		if (iwqp->iwscq) {
> > +			irdma_clean_cqes(iwqp, iwqp->iwscq);
> > +			if (iwqp->iwrcq != iwqp->iwscq)
> > +				irdma_clean_cqes(iwqp, iwqp->iwrcq);
> > +		}
> > +	}
> > +
> > +	irdma_remove_push_mmap_entries(iwqp);
> > +	irdma_free_lsmm_rsrc(iwqp);
> > +	irdma_rem_ref(&iwqp->ibqp);
> 
> No, please ensure that call to destroy_qp is kfree QP without any need in reference
> counting. We need this to move QP allocation to be IB/core responsibility. I hope
> that all other verbs objects (with MR as
> exception) follow the same pattern: create->kzalloc->destroy>kfree.

Yes. I did see the other verb objects allocation move to IB core
responsibility but not QP. Since we are headed in that direction,
I do think it's a reasonable expectation to make destroy QP
synchronous in providers. We ll look to change it in next rev.

Thank you Leon for taking the time to review and provide
feedback.

Shiraz