From: "Steve Wise" <swise@opengridcomputing.com> To: 'Jason Gunthorpe' <jgunthorpe@obsidianresearch.com> Cc: dledford@redhat.com, infinipath@intel.com, sagig@mellanox.com, ogerlitz@mellanox.com, roid@mellanox.com, linux-rdma@vger.kernel.org, eli@mellanox.com, target-devel@vger.kernel.org, linux-nfs@vger.kernel.org, bfields@fieldses.org Subject: RE: [PATCH V6 9/9] isert: Support iWARP transports using FRMRs Date: Fri, 24 Jul 2015 13:48:09 -0500 [thread overview] Message-ID: <00ce01d0c641$493ea300$dbbbe900$@opengridcomputing.com> (raw) In-Reply-To: <20150724165721.GC25480@obsidianresearch.com> > -----Original Message----- > From: Jason Gunthorpe [mailto:jgunthorpe@obsidianresearch.com] > Sent: Friday, July 24, 2015 11:57 AM > To: Steve Wise > Cc: dledford@redhat.com; infinipath@intel.com; sagig@mellanox.com; ogerlitz@mellanox.com; roid@mellanox.com; linux- > rdma@vger.kernel.org; eli@mellanox.com; target-devel@vger.kernel.org; linux-nfs@vger.kernel.org; bfields@fieldses.org > Subject: Re: [PATCH V6 9/9] isert: Support iWARP transports using FRMRs > > On Fri, Jul 24, 2015 at 11:19:05AM -0500, Steve Wise wrote: > > Add new register and unregister functions to be used with devices that > > support FRMRs, provide a local dma lkey, yet do not support DIF/PI. > > > > isert_reg_frmr() only needs to use FRMRs for RDMA READ since RDMA WRITE > > can be handled entirely with the local dma lkey. So for RDMA READ, > > it calls isert_reg_read_frmr(). Otherwise is uses the lkey map service > > isert_map_lkey() for RDMA WRITEs. > > > > isert_reg_read_frmr() will create a linked list of WR triplets of the > > form: INV->FRWR->READ. The number of these triplets is dependent on > > the devices fast reg page list length limit. > > That ordering seems strange, surely it should be > > FRWR->READ->INV > > And use IB_WR_RDMA_READ_WITH_INV if possible? > > ACCESS_REMOTE rkey's should not be left open across the FROM_DEVICE > DMA flush. > You're correct. I was thinking to simplify the IO by always invalidating before re-registering. But it does leave the FRMR registered and exposes a security hole. I'll have to rework this. > > /* assign function handlers */ > > - if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS && > > - dev_attr->device_cap_flags & IB_DEVICE_LOCAL_DMA_LKEY && > > - dev_attr->device_cap_flags & IB_DEVICE_SIGNATURE_HANDOVER) { > > - device->use_fastreg = 1; > > - device->reg_rdma_mem = isert_reg_frmr_pi; > > - device->unreg_rdma_mem = isert_unreg_frmr_pi; > > + cap_flags = dev_attr->device_cap_flags; > > + if (cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS && > > + cap_flags & IB_DEVICE_LOCAL_DMA_LKEY) { > > + if (cap_flags & IB_DEVICE_SIGNATURE_HANDOVER) { > > + device->use_fastreg = 1; > > + device->reg_rdma_mem = isert_reg_frmr_pi; > > + device->unreg_rdma_mem = isert_unreg_frmr_pi; > > + } else { > > + device->use_fastreg = 1; > > + device->reg_rdma_mem = isert_reg_frmr; > > + device->unreg_rdma_mem = isert_unreg_frmr; > > + } > > The use of FRWR for RDMA READ should be iWarp specific, IB shouldn't > pay that overhead. I am expecting to see a cap_rdma_read_rkey or > something in here ? > Ok. But cap_rdma_read_rkey() doesn't really describe the requirement. The requirement is rkey + REMOTE_WRITE. So it is more like rdma_cap_read_requires_remote_write() which is ugly and too long (but descriptive)...
WARNING: multiple messages have this Message-ID (diff)
From: "Steve Wise" <swise@opengridcomputing.com> To: "'Jason Gunthorpe'" <jgunthorpe@obsidianresearch.com> Cc: <dledford@redhat.com>, <infinipath@intel.com>, <sagig@mellanox.com>, <ogerlitz@mellanox.com>, <roid@mellanox.com>, <linux-rdma@vger.kernel.org>, <eli@mellanox.com>, <target-devel@vger.kernel.org>, <linux-nfs@vger.kernel.org>, <bfields@fieldses.org> Subject: RE: [PATCH V6 9/9] isert: Support iWARP transports using FRMRs Date: Fri, 24 Jul 2015 13:48:09 -0500 [thread overview] Message-ID: <00ce01d0c641$493ea300$dbbbe900$@opengridcomputing.com> (raw) In-Reply-To: <20150724165721.GC25480@obsidianresearch.com> > -----Original Message----- > From: Jason Gunthorpe [mailto:jgunthorpe@obsidianresearch.com] > Sent: Friday, July 24, 2015 11:57 AM > To: Steve Wise > Cc: dledford@redhat.com; infinipath@intel.com; sagig@mellanox.com; ogerlitz@mellanox.com; roid@mellanox.com; linux- > rdma@vger.kernel.org; eli@mellanox.com; target-devel@vger.kernel.org; linux-nfs@vger.kernel.org; bfields@fieldses.org > Subject: Re: [PATCH V6 9/9] isert: Support iWARP transports using FRMRs > > On Fri, Jul 24, 2015 at 11:19:05AM -0500, Steve Wise wrote: > > Add new register and unregister functions to be used with devices that > > support FRMRs, provide a local dma lkey, yet do not support DIF/PI. > > > > isert_reg_frmr() only needs to use FRMRs for RDMA READ since RDMA WRITE > > can be handled entirely with the local dma lkey. So for RDMA READ, > > it calls isert_reg_read_frmr(). Otherwise is uses the lkey map service > > isert_map_lkey() for RDMA WRITEs. > > > > isert_reg_read_frmr() will create a linked list of WR triplets of the > > form: INV->FRWR->READ. The number of these triplets is dependent on > > the devices fast reg page list length limit. > > That ordering seems strange, surely it should be > > FRWR->READ->INV > > And use IB_WR_RDMA_READ_WITH_INV if possible? > > ACCESS_REMOTE rkey's should not be left open across the FROM_DEVICE > DMA flush. > You're correct. I was thinking to simplify the IO by always invalidating before re-registering. But it does leave the FRMR registered and exposes a security hole. I'll have to rework this. > > /* assign function handlers */ > > - if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS && > > - dev_attr->device_cap_flags & IB_DEVICE_LOCAL_DMA_LKEY && > > - dev_attr->device_cap_flags & IB_DEVICE_SIGNATURE_HANDOVER) { > > - device->use_fastreg = 1; > > - device->reg_rdma_mem = isert_reg_frmr_pi; > > - device->unreg_rdma_mem = isert_unreg_frmr_pi; > > + cap_flags = dev_attr->device_cap_flags; > > + if (cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS && > > + cap_flags & IB_DEVICE_LOCAL_DMA_LKEY) { > > + if (cap_flags & IB_DEVICE_SIGNATURE_HANDOVER) { > > + device->use_fastreg = 1; > > + device->reg_rdma_mem = isert_reg_frmr_pi; > > + device->unreg_rdma_mem = isert_unreg_frmr_pi; > > + } else { > > + device->use_fastreg = 1; > > + device->reg_rdma_mem = isert_reg_frmr; > > + device->unreg_rdma_mem = isert_unreg_frmr; > > + } > > The use of FRWR for RDMA READ should be iWarp specific, IB shouldn't > pay that overhead. I am expecting to see a cap_rdma_read_rkey or > something in here ? > Ok. But cap_rdma_read_rkey() doesn't really describe the requirement. The requirement is rkey + REMOTE_WRITE. So it is more like rdma_cap_read_requires_remote_write() which is ugly and too long (but descriptive)...
next prev parent reply other threads:[~2015-07-24 18:48 UTC|newest] Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-07-24 16:18 [PATCH V6 0/9] iSER support for iWARP Steve Wise 2015-07-24 16:18 ` Steve Wise [not found] ` <20150724161331.25617.8475.stgit-PBeJgSbIpn97NCTnQtmixQ@public.gmane.org> 2015-07-24 16:18 ` [PATCH V6 1/9] RDMA/iser: Limit sg tablesize and max_sectors to device fastreg max depth Steve Wise 2015-07-24 16:18 ` Steve Wise [not found] ` <20150724161820.25617.63886.stgit-PBeJgSbIpn97NCTnQtmixQ@public.gmane.org> 2015-07-24 16:41 ` Jason Gunthorpe 2015-07-24 16:41 ` Jason Gunthorpe 2015-07-24 18:40 ` Steve Wise 2015-07-24 18:40 ` Steve Wise 2015-07-24 19:14 ` Jason Gunthorpe 2015-07-26 9:58 ` Sagi Grimberg 2015-07-26 9:57 ` Sagi Grimberg 2015-07-26 9:57 ` Sagi Grimberg 2015-07-24 16:18 ` [PATCH V6 2/9] mlx4, mlx5, mthca: Expose max_sge_rd correctly Steve Wise 2015-07-24 16:18 ` Steve Wise 2015-07-24 16:18 ` [PATCH V6 3/9] ipath,qib: " Steve Wise 2015-07-24 16:18 ` Steve Wise 2015-07-24 16:18 ` [PATCH V6 4/9] svcrdma: Use max_sge_rd for destination read depths Steve Wise 2015-07-24 16:18 ` Steve Wise [not found] ` <20150724161837.25617.48584.stgit-PBeJgSbIpn97NCTnQtmixQ@public.gmane.org> 2015-07-24 18:51 ` Steve Wise 2015-07-24 18:51 ` Steve Wise 2015-07-26 9:58 ` Sagi Grimberg [not found] ` <55B4AF63.3040108-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-07-26 10:42 ` Christoph Hellwig 2015-07-26 10:42 ` Christoph Hellwig 2015-07-24 16:18 ` [PATCH V6 6/9] isert: Rename IO functions to more descriptive names Steve Wise 2015-07-24 16:18 ` Steve Wise [not found] ` <20150724161848.25617.26092.stgit-PBeJgSbIpn97NCTnQtmixQ@public.gmane.org> 2015-07-26 10:08 ` Sagi Grimberg 2015-07-26 10:08 ` Sagi Grimberg [not found] ` <55B4B190.7070305-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-07-26 10:43 ` Christoph Hellwig 2015-07-26 10:43 ` Christoph Hellwig [not found] ` <20150726104328.GB18944-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 2015-07-26 11:00 ` Sagi Grimberg 2015-07-26 11:00 ` Sagi Grimberg 2015-07-26 15:53 ` Christoph Hellwig 2015-07-26 16:44 ` Sagi Grimberg [not found] ` <55B4BDE3.8040801-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-07-26 17:32 ` Steve Wise 2015-07-26 17:32 ` Steve Wise 2015-07-26 17:40 ` Sagi Grimberg [not found] ` <55B51B7A.8030008-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-07-26 20:15 ` Steve Wise 2015-07-26 20:15 ` Steve Wise 2015-07-26 20:17 ` Steve Wise 2015-07-26 20:17 ` Steve Wise [not found] ` <55B5404F.5040404-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org> 2015-07-27 21:45 ` Steve Wise 2015-07-27 21:45 ` Steve Wise 2015-08-03 19:32 ` Steve Wise 2015-08-03 19:32 ` Steve Wise 2015-08-04 17:26 ` Sagi Grimberg [not found] ` <55C0F5B7.4040100-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-08-04 17:44 ` Steve Wise 2015-08-04 17:44 ` Steve Wise 2015-08-05 21:23 ` Steve Wise 2015-08-05 21:23 ` Steve Wise 2015-08-06 15:37 ` Sagi Grimberg 2015-08-06 15:37 ` Sagi Grimberg [not found] ` <55C37F43.6080106-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-08-12 22:15 ` Steve Wise 2015-08-12 22:15 ` Steve Wise 2015-08-13 13:09 ` Sagi Grimberg 2015-07-24 16:18 ` [PATCH V6 5/9] RDMA/isert: Limit read depth based on the device max_sge_rd capability Steve Wise 2015-07-24 16:18 ` [PATCH V6 7/9] isert: Use the device's max fastreg page list depth Steve Wise 2015-07-24 16:18 ` [PATCH V6 8/9] isert: Use local_dma_lkey whenever possible Steve Wise [not found] ` <20150724161859.25617.17286.stgit-PBeJgSbIpn97NCTnQtmixQ@public.gmane.org> 2015-07-24 16:49 ` Jason Gunthorpe 2015-07-24 16:49 ` Jason Gunthorpe [not found] ` <20150724164909.GB25480-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2015-07-24 18:41 ` Steve Wise 2015-07-24 18:41 ` Steve Wise 2015-07-24 16:19 ` [PATCH V6 9/9] isert: Support iWARP transports using FRMRs Steve Wise 2015-07-24 16:57 ` Jason Gunthorpe 2015-07-24 18:48 ` Steve Wise [this message] 2015-07-24 18:48 ` Steve Wise 2015-07-24 19:24 ` Jason Gunthorpe 2015-07-24 19:24 ` Jason Gunthorpe 2015-07-24 19:57 ` Steve Wise 2015-07-24 19:57 ` Steve Wise 2015-07-24 22:11 ` Steve Wise 2015-07-24 22:11 ` Steve Wise 2015-07-24 22:38 ` Jason Gunthorpe 2015-07-24 22:38 ` Jason Gunthorpe [not found] ` <20150724192411.GC26225-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2015-07-26 10:23 ` Sagi Grimberg 2015-07-26 10:23 ` Sagi Grimberg 2015-07-27 17:07 ` Jason Gunthorpe
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='00ce01d0c641$493ea300$dbbbe900$@opengridcomputing.com' \ --to=swise@opengridcomputing.com \ --cc=bfields@fieldses.org \ --cc=dledford@redhat.com \ --cc=eli@mellanox.com \ --cc=infinipath@intel.com \ --cc=jgunthorpe@obsidianresearch.com \ --cc=linux-nfs@vger.kernel.org \ --cc=linux-rdma@vger.kernel.org \ --cc=ogerlitz@mellanox.com \ --cc=roid@mellanox.com \ --cc=sagig@mellanox.com \ --cc=target-devel@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.