| Message ID | 20190124031041.29585.54467.stgit@scvm10.sc.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | IB/hfi1: Add OPFN | expand |
On Wed, 2019-01-23 at 19:20 -0800, Dennis Dalessandro wrote: > This is the second in a set of patch series to add in TID RDMA. This series > is the patches to support OPFN which is the feature negotiation protocol used > by TID RDMA. This is totally hidden from users and does not impact any API. It > done under the hood at the driver level. > > At a high level OPFN enables exchanging parameters between two hosts using > IB compare and swap requests to a special virtual address. The request uses > a reserved IB work request opcode (see patch 3). I looked it over, and I didn't see any red flags. None of the obvious NAK type stuff anyway. My only concern is a matter of precedent. You're hijacking a bit to represent a single driver handled, in-band negotiation. Given that it's only on OPA links, and I suspect you're going to say it's backward compatible and won't prevent older drivers from talking to newer drivers, I'm OK with that generally. But I wonder if you should be doing one bit, one extension, or a single "I have in- band, driver local features to discuss" bit and then the negotiation protocol handles what features are available. I'm thinking of this from a future extension standpoint. If you guys have plans for more changes like this, then something other than one bit per feature might be the better option.
> -----Original Message----- > From: Doug Ledford [mailto:dledford@redhat.com] > Sent: Wednesday, January 30, 2019 2:32 PM > To: Dalessandro, Dennis <dennis.dalessandro@intel.com>; jgg@ziepe.ca > Cc: Dixit, Ashutosh <ashutosh.dixit@intel.com>; linux-rdma@vger.kernel.org; > Mitko Haralanov <mitko.haralanov@intel.com>; Marciniszyn, Mike > <mike.marciniszyn@intel.com>; Wan, Kaike <kaike.wan@intel.com> > Subject: Re: [PATCH for-next 0/6] IB/hfi1: Add OPFN > > On Wed, 2019-01-23 at 19:20 -0800, Dennis Dalessandro wrote: > > This is the second in a set of patch series to add in TID RDMA. This > > series is the patches to support OPFN which is the feature negotiation > > protocol used by TID RDMA. This is totally hidden from users and does > > not impact any API. It done under the hood at the driver level. > > > > At a high level OPFN enables exchanging parameters between two hosts > > using IB compare and swap requests to a special virtual address. The > > request uses a reserved IB work request opcode (see patch 3). > > I looked it over, and I didn't see any red flags. None of the obvious NAK type > stuff anyway. My only concern is a matter of precedent. > You're hijacking a bit to represent a single driver handled, in-band > negotiation. Given that it's only on OPA links, and I suspect you're going to > say it's backward compatible and won't prevent older drivers from talking to > newer drivers, I'm OK with that generally. But I wonder if you should be > doing one bit, one extension, or a single "I have in- band, driver local features > to discuss" bit and then the negotiation protocol handles what features are > available. I'm thinking of this from a future extension standpoint. If you guys > have plans for more changes like this, then something other than one bit per > feature might be the better option. > We are doing a single "I have in-band, driver local features to discuss" approach here (one bit for all future features) and future extension can be easily done by adding features in OPFN itself. Kaike
On Thu, 2019-01-31 at 12:35 +0000, Wan, Kaike wrote: > > -----Original Message----- > > From: Doug Ledford [mailto:dledford@redhat.com] > > Sent: Wednesday, January 30, 2019 2:32 PM > > To: Dalessandro, Dennis <dennis.dalessandro@intel.com>; jgg@ziepe.ca > > Cc: Dixit, Ashutosh <ashutosh.dixit@intel.com>; linux-rdma@vger.kernel.org; > > Mitko Haralanov <mitko.haralanov@intel.com>; Marciniszyn, Mike > > <mike.marciniszyn@intel.com>; Wan, Kaike <kaike.wan@intel.com> > > Subject: Re: [PATCH for-next 0/6] IB/hfi1: Add OPFN > > > > On Wed, 2019-01-23 at 19:20 -0800, Dennis Dalessandro wrote: > > > This is the second in a set of patch series to add in TID RDMA. This > > > series is the patches to support OPFN which is the feature negotiation > > > protocol used by TID RDMA. This is totally hidden from users and does > > > not impact any API. It done under the hood at the driver level. > > > > > > At a high level OPFN enables exchanging parameters between two hosts > > > using IB compare and swap requests to a special virtual address. The > > > request uses a reserved IB work request opcode (see patch 3). > > > > I looked it over, and I didn't see any red flags. None of the obvious NAK type > > stuff anyway. My only concern is a matter of precedent. > > You're hijacking a bit to represent a single driver handled, in-band > > negotiation. Given that it's only on OPA links, and I suspect you're going to > > say it's backward compatible and won't prevent older drivers from talking to > > newer drivers, I'm OK with that generally. But I wonder if you should be > > doing one bit, one extension, or a single "I have in- band, driver local features > > to discuss" bit and then the negotiation protocol handles what features are > > available. I'm thinking of this from a future extension standpoint. If you guys > > have plans for more changes like this, then something other than one bit per > > feature might be the better option. > > > We are doing a single "I have in-band, driver local features to discuss" approach here (one bit for all future features) and future extension can be easily done by adding features in OPFN itself. Ok. Mainly because of the way you had things named, I took it to be a bit per feature. I'm glad that's not the case. Thanks for the clarification.
On Wed, 2019-01-23 at 19:20 -0800, Dennis Dalessandro wrote: > This is the second in a set of patch series to add in TID RDMA. This series > is the patches to support OPFN which is the feature negotiation protocol used > by TID RDMA. This is totally hidden from users and does not impact any API. It > done under the hood at the driver level. > > At a high level OPFN enables exchanging parameters between two hosts using > IB compare and swap requests to a special virtual address. The request uses > a reserved IB work request opcode (see patch 3). > > --- > > Kaike Wan (5): > IB/hfi1: Add OPFN helper functions for TID RDMA feature > IB/hfi1: OPFN interface > IB/hfi1, IB/rdmavt: Allow for extending of QP's s_ack_queue > IB/hfi1: Integrate OPFN into RC transactions > IB/hfi1: Add static trace for OPFN > > Mitko Haralanov (1): > IB/hfi1: OPFN support discovery Series merged into wip/dl-hfi1-tid, thanks.
This is the second in a set of patch series to add in TID RDMA. This series is the patches to support OPFN which is the feature negotiation protocol used by TID RDMA. This is totally hidden from users and does not impact any API. It done under the hood at the driver level. At a high level OPFN enables exchanging parameters between two hosts using IB compare and swap requests to a special virtual address. The request uses a reserved IB work request opcode (see patch 3). --- Kaike Wan (5): IB/hfi1: Add OPFN helper functions for TID RDMA feature IB/hfi1: OPFN interface IB/hfi1, IB/rdmavt: Allow for extending of QP's s_ack_queue IB/hfi1: Integrate OPFN into RC transactions IB/hfi1: Add static trace for OPFN Mitko Haralanov (1): IB/hfi1: OPFN support discovery drivers/infiniband/hw/hfi1/Makefile | 1 drivers/infiniband/hw/hfi1/chip.c | 11 + drivers/infiniband/hw/hfi1/chip.h | 3 drivers/infiniband/hw/hfi1/hfi.h | 3 drivers/infiniband/hw/hfi1/init.c | 10 + drivers/infiniband/hw/hfi1/opfn.c | 318 +++++++++++++++++++++++++++++++ drivers/infiniband/hw/hfi1/opfn.h | 85 ++++++++ drivers/infiniband/hw/hfi1/qp.c | 13 + drivers/infiniband/hw/hfi1/rc.c | 87 ++++++-- drivers/infiniband/hw/hfi1/ruc.c | 16 +- drivers/infiniband/hw/hfi1/tid_rdma.c | 217 +++++++++++++++++++++ drivers/infiniband/hw/hfi1/tid_rdma.h | 29 +++ drivers/infiniband/hw/hfi1/trace.h | 1 drivers/infiniband/hw/hfi1/trace_rx.h | 107 ---------- drivers/infiniband/hw/hfi1/trace_tid.h | 332 ++++++++++++++++++++++++++++++++ drivers/infiniband/hw/hfi1/uc.c | 3 drivers/infiniband/hw/hfi1/verbs.c | 2 drivers/infiniband/hw/hfi1/verbs.h | 8 + include/rdma/rdma_vt.h | 10 + 19 files changed, 1115 insertions(+), 141 deletions(-) create mode 100644 drivers/infiniband/hw/hfi1/opfn.c create mode 100644 drivers/infiniband/hw/hfi1/opfn.h create mode 100644 drivers/infiniband/hw/hfi1/trace_tid.h -- -Denny