RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-23 Thread Hefty, Sean
The lkey is possibly useful if someone wants to do single op transfers larger than the S/G limit of the SQE. I haven't noticed any ULPs doing that.. That changes how the buffer is identified, which gets back to my question of are we identifying local buffers by address or through some sort of

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-23 Thread Jason Gunthorpe
On Thu, Jul 23, 2015 at 11:30:08PM +, Hefty, Sean wrote: I don't see where usnic, ipath, qib, or opa technically need an lkey at all. The lkey is possibly useful if someone wants to do single op transfers larger than the S/G limit of the SQE. I haven't noticed any ULPs doing that.. qib

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-23 Thread Hefty, Sean
This may be a nit, but IMO, the use of the term 'rkey' versus 'stag' matters. They convey different ways of finding a data buffer. For example, do you locate a buffer using the stag, then verify that the offset + length fits into the target buffer? Yes. HW always uses the stag to

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-23 Thread Steve Wise
...@mellanox.com; linux-rdma@vger.kernel.org; e...@mellanox.com; target-de...@vger.kernel.org; linux-...@vger.kernel.org; trond.mykleb...@primarydata.com; bfie...@fieldses.org; 'Oren Duer' Subject: RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags There is confusion about lkeys and rkeys

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-23 Thread Hefty, Sean
There is confusion about lkeys and rkeys with regard to iWARP. In the iWARP verbs, there is no distinction between an lkey and rkey: they are the same key, called a Steering Tag or STAG. When you create a MR, the lkey == rkey == STAG for iwarp transports. Somewhat related, but really a

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-16 Thread Jason Gunthorpe
On Wed, Jul 15, 2015 at 01:12:57PM -0600, Jason Gunthorpe wrote: I might find time to type this in, but I won't be able to find time to do any testing on the ULPs.. Here is the typing, I'll look more carefully at it later and send it via email:

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-16 Thread 'Christoph Hellwig'
On Wed, Jul 15, 2015 at 01:12:57PM -0600, Jason Gunthorpe wrote: This looks perfect to me. After this we can get rid of the ib_get_dma_mr calls outside of ib_alloc_pd, and eventuall move setting up -local_dma_lkey into the HW driver and kill of ib_get_dma_mr, IB_DEVICE_LOCAL_DMA_LKEY and

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-16 Thread Jason Gunthorpe
On Thu, Jul 16, 2015 at 01:04:02AM -0700, 'Christoph Hellwig' wrote: On Wed, Jul 15, 2015 at 01:12:57PM -0600, Jason Gunthorpe wrote: This looks perfect to me. After this we can get rid of the ib_get_dma_mr calls outside of ib_alloc_pd, and eventuall move setting up -local_dma_lkey into

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-15 Thread Sagi Grimberg
On 7/14/2015 11:29 PM, Jason Gunthorpe wrote: On Tue, Jul 14, 2015 at 12:55:11PM -0700, 'Christoph Hellwig' wrote: On Tue, Jul 14, 2015 at 02:32:31PM -0500, Steve Wise wrote: You mean should not, yea? Ok. I'll check for iWARP. But don't tell me to remove the transport-specific hacks in

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-15 Thread 'Christoph Hellwig'
On Tue, Jul 14, 2015 at 02:29:43PM -0600, Jason Gunthorpe wrote: local_dma_lkey appears to be global, it works with any PD. ib_get_dma_mr is tied to a PD, so it cannot replace local_dma_lkey at the struct device level. ib_alloc_pd is the in-kernel entry point, the UAPI calls

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-15 Thread Sagi Grimberg
On 7/14/2015 8:26 PM, Jason Gunthorpe wrote: On Tue, Jul 14, 2015 at 12:05:53PM +0300, Sagi Grimberg wrote: iser has it too. I have a similar patch with a flag for iser (its behind a bulk of patches that are still pending though). Do we all agree and understand that stuff like this in

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-15 Thread 'Christoph Hellwig'
On Wed, Jul 15, 2015 at 11:47:52AM +0300, Sagi Grimberg wrote: struct ib_pd *ib_alloc_pd(struct ib_device *device) { struct ib_pd *pd; +struct ib_device_attr devattr; +int rc; + +rc = ib_query_device(device, devattr); +if (rc) +return ERR_PTR(rc);

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-15 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 11:50:57PM -0700, 'Christoph Hellwig' wrote: On Tue, Jul 14, 2015 at 02:29:43PM -0600, Jason Gunthorpe wrote: local_dma_lkey appears to be global, it works with any PD. ib_get_dma_mr is tied to a PD, so it cannot replace local_dma_lkey at the struct device level.

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread 'Christoph Hellwig'
On Mon, Jul 13, 2015 at 10:57:48AM -0600, Jason Gunthorpe wrote: Currently various drivers are using ib_get_dma_mr with remote flags unfortunately, e.g. the SRP initiator driver uses it to optimize away memory registrtions for single SGL entry requests. Unconditionally? Ugh. Maybe we do

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Sagi Grimberg
On 7/13/2015 7:50 PM, Jason Gunthorpe wrote: On Sun, Jul 12, 2015 at 10:49:08AM +0300, Sagi Grimberg wrote: On 7/10/2015 10:34 PM, Christoph Hellwig wrote: On Thu, Jul 09, 2015 at 09:52:59AM -0400, Chuck Lever wrote: There is one remaining kernel user of ib_reg_phys_mr() in 4.2: Lustre.

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread 'Christoph Hellwig'
On Mon, Jul 13, 2015 at 03:36:44PM -0400, Tom Talpey wrote: On 7/11/2015 6:25 AM, 'Christoph Hellwig' wrote: I think what we need to support for now is FRMR as the primary target, and FMR as a secondar[y]. FMR is a *very* bad choice, for several reasons. 1) It's not supported by very many

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Sagi Grimberg
On 7/13/2015 11:15 PM, Jason Gunthorpe wrote: On Mon, Jul 13, 2015 at 03:36:44PM -0400, Tom Talpey wrote: On 7/11/2015 6:25 AM, 'Christoph Hellwig' wrote: I think what we need to support for now is FRMR as the primary target, and FMR as a secondar[y]. FMR is a *very* bad choice, for several

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Sagi Grimberg
On 7/14/2015 10:37 AM, 'Christoph Hellwig' wrote: On Mon, Jul 13, 2015 at 03:36:44PM -0400, Tom Talpey wrote: On 7/11/2015 6:25 AM, 'Christoph Hellwig' wrote: I think what we need to support for now is FRMR as the primary target, and FMR as a secondar[y]. FMR is a *very* bad choice, for

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Sagi Grimberg
On 7/14/2015 10:25 AM, 'Christoph Hellwig' wrote: On Mon, Jul 13, 2015 at 10:57:48AM -0600, Jason Gunthorpe wrote: Currently various drivers are using ib_get_dma_mr with remote flags unfortunately, e.g. the SRP initiator driver uses it to optimize away memory registrtions for single SGL entry

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Sagi Grimberg
On 7/14/2015 3:12 PM, Tom Talpey wrote: On 7/14/2015 5:22 AM, Sagi Grimberg wrote: On 7/14/2015 10:37 AM, 'Christoph Hellwig' wrote: On Mon, Jul 13, 2015 at 03:36:44PM -0400, Tom Talpey wrote: On 7/11/2015 6:25 AM, 'Christoph Hellwig' wrote: I think what we need to support for now is FRMR as

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Tom Talpey
On 7/14/2015 5:22 AM, Sagi Grimberg wrote: On 7/14/2015 10:37 AM, 'Christoph Hellwig' wrote: On Mon, Jul 13, 2015 at 03:36:44PM -0400, Tom Talpey wrote: On 7/11/2015 6:25 AM, 'Christoph Hellwig' wrote: I think what we need to support for now is FRMR as the primary target, and FMR as a

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Sagi Grimberg
On 7/14/2015 3:24 PM, Tom Talpey wrote: On 7/14/2015 4:06 AM, Sagi Grimberg wrote: All protocols cares about transferring data and sending messages, so it's not a good enough reason for a poor registration method choice. This just emphasizes why we need to converge to a single method. In my

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Tom Talpey
On 7/14/2015 11:36 AM, 'Christoph Hellwig' wrote: On Tue, Jul 14, 2015 at 12:10:36PM +0300, Sagi Grimberg wrote: Having an API that does FRMR/FMR/PHYS_MR is even worse from the ULP PoV. If you expose an API that might schedule (PHYS_MR) it limits the context that the caller is allowed to call

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 08:36:19AM -0700, 'Christoph Hellwig' wrote: Oh, I had missed that PHYS_MR might sleep. That might be the reasons why everyone is avoiding them despite Tom preferring them over FMR. Yep, almost certainly. But even that is just a legacy of the bad API. Even Sagi's API

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread 'Christoph Hellwig'
On Tue, Jul 14, 2015 at 12:22:14PM +0300, Sagi Grimberg wrote: It's better if you want it fast. I can't stress it enough, but IMO, the fallback should *not* be in the API, but rather in the ULP. Ideally, at some point it won't need to fall back, and we can remove the API. But if all driver

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread 'Christoph Hellwig'
On Tue, Jul 14, 2015 at 12:10:36PM +0300, Sagi Grimberg wrote: Having an API that does FRMR/FMR/PHYS_MR is even worse from the ULP PoV. If you expose an API that might schedule (PHYS_MR) it limits the context that the caller is allowed to call in. I'm 100% against an registration API that

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread 'Christoph Hellwig'
On Tue, Jul 14, 2015 at 12:05:53PM +0300, Sagi Grimberg wrote: iser has it too. I have a similar patch with a flag for iser (its behind a bulk of patches that are still pending though). So instead of this flag can we revisit the need for it? Given how inherently isecture it is maybe a

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Steve Wise
I'm willing to add a proper memory_registration.txt under Documentation/infiniband/ which would capture the items that were brought up here. This would be great. Documentation/infiniband is very sparse... -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 12:45:12PM -0700, 'Christoph Hellwig' wrote: How does IB_DEVICE_LOCAL_DMA_LKEY (and -local_dma_lkey) play into this? Seems like that should be the preferred option if supported. Very good question. Interestingly enough various iWarp driver seem to support this

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Steve Wise
...@mellanox.com; r...@mellanox.com; linux-rdma@vger.kernel.org; e...@mellanox.com; target-de...@vger.kernel.org; linux-...@vger.kernel.org; trond.mykleb...@primarydata.com; bfie...@fieldses.org; 'Oren Duer' Subject: Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags On Tue, Jul

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 04:01:04PM -0500, Steve Wise wrote: Right, a local_dma_lkey is not an rkey, and iwarp requires the rkey for the read destination MR. Further that rkey needs REMOTE_WRITE. BTW: What use is an IB rkey with no REMOTE_ flags set? Can it be used somehow differently

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Steve Wise
...@mellanox.com; ogerl...@mellanox.com; r...@mellanox.com; linux-rdma@vger.kernel.org; e...@mellanox.com; target-de...@vger.kernel.org; linux-...@vger.kernel.org; trond.mykleb...@primarydata.com; bfie...@fieldses.org; 'Oren Duer' Subject: RE: [PATCH V3 1/5] RDMA/core: Transport-independent access

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 02:32:31PM -0500, Steve Wise wrote: Ok. I'll check for iWARP. But don't tell me to remove the transport-specific hacks in this series when I post it! ;) Sure thing! Wonder what the test should be? cap_remote_access_lkey? No idea. Still think this should all be

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Steve Wise
...@mellanox.com; linux-rdma@vger.kernel.org; e...@mellanox.com; target-de...@vger.kernel.org; linux- n...@vger.kernel.org; trond.mykleb...@primarydata.com; bfie...@fieldses.org; 'Oren Duer' Subject: Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags On Tue, Jul 14, 2015 at 02:25

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 02:25:50PM -0500, Steve Wise wrote: The benefit is that we don't have to check for iWARP protocol in the ULP. IB should have to pay the cost of FRMR for lkey on RDMA READ, I'm pretty sure you have to check for iWarp at somepoint.. Jason -- To unsubscribe from this

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread 'Christoph Hellwig'
On Tue, Jul 14, 2015 at 02:25:50PM -0500, Steve Wise wrote: if (device_supports_fastreg device_supports_signature) use FRMR else use DMAMR Shouldn't we just recode it this way? if (device_supports_fastreg) use FRMR else use DMAMR How does

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 12:55:11PM -0700, 'Christoph Hellwig' wrote: On Tue, Jul 14, 2015 at 02:32:31PM -0500, Steve Wise wrote: You mean should not, yea? Ok. I'll check for iWARP. But don't tell me to remove the transport-specific hacks in this series when I post it! ;) Just

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Steve Wise
...@mellanox.com; ogerl...@mellanox.com; r...@mellanox.com; linux-rdma@vger.kernel.org; e...@mellanox.com; target-de...@vger.kernel.org; linux-...@vger.kernel.org; trond.mykleb...@primarydata.com; bfie...@fieldses.org; 'Oren Duer' Subject: Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Steve Wise
I'm sorry Steve for leading you down the wrong path with these flags, I did not fully realize exactly what the iWarp difference was at the start :( Jason No problem. I'll work on iSER target FRMRs and repost the iSER series. Sagi, right now isert only uses FRMRs along with

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Steve Wise
...@mellanox.com; linux-rdma@vger.kernel.org; e...@mellanox.com; target-de...@vger.kernel.org; linux- n...@vger.kernel.org; trond.mykleb...@primarydata.com; bfie...@fieldses.org; 'Oren Duer' Subject: Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags On Tue, Jul 14, 2015 at 02:32

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 02:58:23PM -0500, Steve Wise wrote: The local_dma_lkey can be used in any rdma sge that requires an lkey. No, this is where iWarp doesn't follow the generic API - a local dma lkey cannot be used with iWarp's RDMA_READ WR lkey. In effect RDMA READ requires an *rkey*

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Steve Wise
...@mellanox.com; ogerl...@mellanox.com; r...@mellanox.com; linux-rdma@vger.kernel.org; e...@mellanox.com; target-de...@vger.kernel.org; linux-...@vger.kernel.org; trond.mykleb...@primarydata.com; bfie...@fieldses.org; 'Oren Duer' Subject: Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread 'Christoph Hellwig'
On Tue, Jul 14, 2015 at 02:32:31PM -0500, Steve Wise wrote: You mean should not, yea? Ok. I'll check for iWARP. But don't tell me to remove the transport-specific hacks in this series when I post it! ;) Just curious if there are any holes in this little scheme to deal with the lkey

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Tom Talpey
On 7/14/2015 3:32 PM, Steve Wise wrote: On Tue, Jul 14, 2015 at 02:25:50PM -0500, Steve Wise wrote: The benefit is that we don't have to check for iWARP protocol in the ULP. IB should have to pay the cost of FRMR for lkey on RDMA READ, I'm pretty sure you have to check for iWarp at

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 03:40:49PM -0500, Steve Wise wrote: local_dma_lkey appears to be global, it works with any PD. ib_get_dma_mr is tied to a PD, so it cannot replace local_dma_lkey at the struct device level. ib_alloc_pd is the in-kernel entry point, the UAPI calls

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Steve Wise
On Tue, Jul 14, 2015 at 02:25:50PM -0500, Steve Wise wrote: The benefit is that we don't have to check for iWARP protocol in the ULP. IB should have to pay the cost of FRMR for lkey on RDMA READ, I'm pretty sure you have to check for iWarp at somepoint.. You mean should not, yea?

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 03:54:15PM -0500, Steve Wise wrote: I'm not seeing the benefit of adding pd-local_dma_lkey? pd-device-local_dma_lkey is there for core and ULP use, and we could have old drivers that don't currently have support for local_dma_lkey allocate their own private

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 12:05:53PM +0300, Sagi Grimberg wrote: iser has it too. I have a similar patch with a flag for iser (its behind a bulk of patches that are still pending though). Do we all agree and understand that stuff like this in drivers/infiniband/ulp/iser/iser_verbs.c

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-13 Thread Tom Talpey
On 7/11/2015 6:25 AM, 'Christoph Hellwig' wrote: I think what we need to support for now is FRMR as the primary target, and FMR as a secondar[y]. FMR is a *very* bad choice, for several reasons. 1) It's not supported by very many devices, in fact it might even be thought to be obsolete. 2)

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-13 Thread Jason Gunthorpe
On Fri, Jul 10, 2015 at 11:10:23PM -0400, Doug Ledford wrote: access controls and connection filtering. The app/ULP itself doesn't even need to be filter aware as you can do the filtering in the TCP stack on the primary listening socket using the netfilter tools. Does netfilter work

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-13 Thread Jason Gunthorpe
On Sat, Jul 11, 2015 at 03:25:38AM -0700, 'Christoph Hellwig' wrote: So a FRMR-like API that allows to use FMRs underneath might be a good start. If the imput to that API are S/G lists as in my suggestion we'll get indirect registration support almost for free. That would be an excellent

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-13 Thread Jason Gunthorpe
On Sat, Jul 11, 2015 at 03:17:36AM -0700, 'Christoph Hellwig' wrote: On Fri, Jul 10, 2015 at 01:54:20PM -0600, Jason Gunthorpe wrote: diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c index bac3fb406a74..6ed7e0f6c162 100644 +++

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-13 Thread Jason Gunthorpe
On Sun, Jul 12, 2015 at 10:49:08AM +0300, Sagi Grimberg wrote: On 7/10/2015 10:34 PM, Christoph Hellwig wrote: On Thu, Jul 09, 2015 at 09:52:59AM -0400, Chuck Lever wrote: There is one remaining kernel user of ib_reg_phys_mr() in 4.2: Lustre. It's in the staging tree, which proper in-tree

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-13 Thread Jason Gunthorpe
On Mon, Jul 13, 2015 at 03:36:44PM -0400, Tom Talpey wrote: On 7/11/2015 6:25 AM, 'Christoph Hellwig' wrote: I think what we need to support for now is FRMR as the primary target, and FMR as a secondar[y]. FMR is a *very* bad choice, for several reasons. If an API can transparently support

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-13 Thread Tom Talpey
On 7/13/2015 1:18 PM, Jason Gunthorpe wrote: On Fri, Jul 10, 2015 at 11:10:23PM -0400, Doug Ledford wrote: Black hat server is beyond the scope of this discussion. We cannot assume an all-trusted model here, there are many configurations to deploy NFS/iSCSI that don't assume that. Even if you

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-12 Thread Sagi Grimberg
On 7/10/2015 10:34 PM, Christoph Hellwig wrote: On Thu, Jul 09, 2015 at 09:52:59AM -0400, Chuck Lever wrote: There is one remaining kernel user of ib_reg_phys_mr() in 4.2: Lustre. It's in the staging tree, which proper in-tree code doesn't have to cater for. So as soon as sunrpc is done

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-12 Thread Sagi Grimberg
On 7/11/2015 7:37 PM, Steve Wise wrote: On Jul 10, 2015, at 9:11 AM, Jason Gunthorpe jguntho...@obsidianresearch.com wrote: On Fri, Jul 10, 2015 at 09:22:24AM -0400, Tom Talpey wrote: and it is enabled only when the RDMA Read is active. ??? How is that done? ib_get_dma_mr is defined to

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-11 Thread 'Christoph Hellwig'
On Fri, Jul 10, 2015 at 01:54:20PM -0600, Jason Gunthorpe wrote: diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c index bac3fb406a74..6ed7e0f6c162 100644 --- a/drivers/infiniband/core/verbs.c +++ b/drivers/infiniband/core/verbs.c @@ -1126,6 +1126,12 @@ struct

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-11 Thread 'Christoph Hellwig'
On Thu, Jul 09, 2015 at 11:01:42AM -0600, Jason Gunthorpe wrote: To your point in another message, I'd say, as long as the new API supports FRMR at full speed with no performance penalty we are good. If the other variants out there take a performance hit, then I think that is OK. As you say,

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-11 Thread Steve Wise
On Jul 10, 2015, at 9:11 AM, Jason Gunthorpe jguntho...@obsidianresearch.com wrote: On Fri, Jul 10, 2015 at 09:22:24AM -0400, Tom Talpey wrote: and it is enabled only when the RDMA Read is active. ??? How is that done? ib_get_dma_mr is defined to return a remote usable rkey that is

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-10 Thread Tom Talpey
On 7/9/2015 6:53 PM, Jason Gunthorpe wrote: On Thu, Jul 09, 2015 at 06:18:26PM -0400, Doug Ledford wrote: A lot of this discussion is interesting and has gone off in an area that I think is worth pursuing in the long run. However, in the short run, this patch was a minor

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-10 Thread Doug Ledford
On 07/10/2015 12:11 PM, Jason Gunthorpe wrote: On Fri, Jul 10, 2015 at 09:22:24AM -0400, Tom Talpey wrote: and it is enabled only when the RDMA Read is active. ??? How is that done? ib_get_dma_mr is defined to return a remote usable rkey that is valid from when it it returns until the MR is

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-10 Thread Tom Talpey
On 7/10/2015 1:56 PM, Doug Ledford wrote: On 07/10/2015 12:11 PM, Jason Gunthorpe wrote: On Fri, Jul 10, 2015 at 09:22:24AM -0400, Tom Talpey wrote: and it is enabled only when the RDMA Read is active. ??? How is that done? ib_get_dma_mr is defined to return a remote usable rkey that is

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-10 Thread Chuck Lever
On Jul 10, 2015, at 1:56 PM, Doug Ledford dledf...@redhat.com wrote: On 07/10/2015 12:11 PM, Jason Gunthorpe wrote: On Fri, Jul 10, 2015 at 09:22:24AM -0400, Tom Talpey wrote: and it is enabled only when the RDMA Read is active. ??? How is that done? ib_get_dma_mr is defined to return a

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-10 Thread Jason Gunthorpe
On Fri, Jul 10, 2015 at 09:22:24AM -0400, Tom Talpey wrote: and it is enabled only when the RDMA Read is active. ??? How is that done? ib_get_dma_mr is defined to return a remote usable rkey that is valid from when it it returns until the MR is destroyed. NFS creates one mr early on, it does not

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-10 Thread Christoph Hellwig
On Thu, Jul 09, 2015 at 09:52:59AM -0400, Chuck Lever wrote: There is one remaining kernel user of ib_reg_phys_mr() in 4.2: Lustre. It's in the staging tree, which proper in-tree code doesn't have to cater for. So as soon as sunrpc is done using the interface we can and should kill it off. --

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-10 Thread Jason Gunthorpe
On Fri, Jul 10, 2015 at 02:42:45PM -0400, Tom Talpey wrote: For the proposed iSER patch the problem is very acute, iser makes a single PD and phys MR at boot time for each device. This means there is a single machine wide unchanging rkey that allows remote physical memory access. An attacker

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-10 Thread Jason Gunthorpe
On Fri, Jul 10, 2015 at 01:54:20PM -0600, Jason Gunthorpe wrote: On Fri, Jul 10, 2015 at 02:42:45PM -0400, Tom Talpey wrote: For the proposed iSER patch the problem is very acute, iser makes a single PD and phys MR at boot time for each device. This means there is a single machine wide

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-10 Thread Doug Ledford
On 07/10/2015 04:57 PM, Jason Gunthorpe wrote: On Fri, Jul 10, 2015 at 01:56:36PM -0400, Doug Ledford wrote: Are there security issues? Yes. Are we going to solve them in this patch set? No. Especially since those security issues extend beyond iSER + iWARP. I think my biggest concern

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-10 Thread Doug Ledford
On 07/10/2015 02:42 PM, Tom Talpey wrote: However, it's an extremely bad idea to codify writable privileged rmr's in the API as best practice. So under no circumstance should that become the long term plan. Agree 100%. Which is why I requested a big warning in the dmesg output that pointed a

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-10 Thread Doug Ledford
On 07/10/2015 03:54 PM, Jason Gunthorpe wrote: On Fri, Jul 10, 2015 at 02:42:45PM -0400, Tom Talpey wrote: For the proposed iSER patch the problem is very acute, iser makes a single PD and phys MR at boot time for each device. This means there is a single machine wide unchanging rkey that

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-09 Thread Chuck Lever
On Jul 9, 2015, at 4:46 AM, Sagi Grimberg sa...@dev.mellanox.co.il wrote: On 7/8/2015 8:14 PM, Hefty, Sean wrote: I am still not clear if all of us agree that we need it. Sean and Steve had some disclaimers... A single entry point doesn't help a whole lot if the app must deal with

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-09 Thread Sagi Grimberg
On 7/8/2015 8:14 PM, Hefty, Sean wrote: I am still not clear if all of us agree that we need it. Sean and Steve had some disclaimers... A single entry point doesn't help a whole lot if the app must deal with different behavior based on how the API is used. It is true that different MRs will

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-09 Thread Sagi Grimberg
On 7/9/2015 2:36 AM, Jason Gunthorpe wrote: I'm arguing upper layer protocols should never even see local memory registration, that it is totally irrelevant to them. So yes, you can call that a common approach to memory registration if you like.. Basically it appears there is nothing that NFS

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-09 Thread Sagi Grimberg
On 7/8/2015 11:32 PM, 'Christoph Hellwig' wrote: On Wed, Jul 08, 2015 at 01:08:42PM -0600, Jason Gunthorpe wrote: Then, what is left is all remote MRs and maybe it will be clearer what to do about them then... From looking at that for a while the APIs needed seem pretty simple to me from a

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-09 Thread Jason Gunthorpe
On Thu, Jul 09, 2015 at 04:00:37PM -0400, Tom Talpey wrote: On 7/9/2015 1:01 PM, Jason Gunthorpe wrote: Laid out like this, I think it even means we can nuke the IB DMA API for these cases. rdma_post_read and rdma_post_complete_read are the two points that need dma api calls (cache flushes),

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-09 Thread Tom Talpey
On 7/9/2015 1:01 PM, Jason Gunthorpe wrote: Laid out like this, I think it even means we can nuke the IB DMA API for these cases. rdma_post_read and rdma_post_complete_read are the two points that need dma api calls (cache flushes), and they can just do them internally. This also tells me that

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-09 Thread Jason Gunthorpe
On Thu, Jul 09, 2015 at 06:18:26PM -0400, Doug Ledford wrote: A lot of this discussion is interesting and has gone off in an area that I think is worth pursuing in the long run. However, in the short run, this patch was a minor cleanup/simplification patch. These discussions are moving into

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-09 Thread Doug Ledford
On 07/08/2015 08:03 PM, Jason Gunthorpe wrote: On Wed, Jul 08, 2015 at 01:32:05PM -0700, 'Christoph Hellwig' wrote: On Wed, Jul 08, 2015 at 01:08:42PM -0600, Jason Gunthorpe wrote: Then, what is left is all remote MRs and maybe it will be clearer what to do about them then... From looking at

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-08 Thread Sagi Grimberg
On 7/8/2015 1:20 PM, 'Christoph Hellwig' wrote: On Wed, Jul 08, 2015 at 01:05:28PM +0300, Sagi Grimberg wrote: If we agree to consolidate on a single MR allocation API, I don't see how this wrapper is moving us forward. But if you guys prefer to have it than I don't have a hard objection.

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-08 Thread 'Christoph Hellwig'
On Tue, Jul 07, 2015 at 09:05:15AM -0500, Steve Wise wrote: I took the feedback from Christoph and Jason to mean I should remove ib_get_dma_mr() entirely and pull its guts into rdma_get_dma_mr(), and change all the users of ib_get_dma_mr() to use rdma_get_dma_mr(). So the net result isn't a

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-08 Thread Hefty, Sean
I am still not clear if all of us agree that we need it. Sean and Steve had some disclaimers... A single entry point doesn't help a whole lot if the app must deal with different behavior based on how the API is used. We have a single entry point for post_send, for example, and that makes

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-08 Thread 'Christoph Hellwig'
On Wed, Jul 08, 2015 at 01:08:42PM -0600, Jason Gunthorpe wrote: Then, what is left is all remote MRs and maybe it will be clearer what to do about them then... From looking at that for a while the APIs needed seem pretty simple to me from a consumer perspective: struct rdma_mr

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-08 Thread 'Christoph Hellwig'
On Wed, Jul 08, 2015 at 01:32:05PM -0700, 'Christoph Hellwig' wrote: /* updates *sg if the SG couldn't be fully registered due to offsets */ int rdma_register_sg(struct rdma_mr *mr, struct scatterlist **sg, u32 *pkey, u32 *offset, u32 *len); plus an enum dma_data_direction

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-08 Thread Sagi Grimberg
On 7/8/2015 12:36 AM, Jason Gunthorpe wrote: On Tue, Jul 07, 2015 at 07:27:47PM +0300, Sagi Grimberg wrote: Doesn't it look odd to you? Sure, but the oddness is that rdma_device_access_flags exists at all, not the wrapper. The wrapper is what we want the API to look like, I don't

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-08 Thread Jason Gunthorpe
On Wed, Jul 08, 2015 at 05:38:05PM -0400, Tom Talpey wrote: On 7/8/2015 3:08 PM, Jason Gunthorpe wrote: The MR stuff was never really designed, the HW people provided some capability and the SW side just raw exposed it, thoughtlessly. Jason, I don't disagree that the API can be improved. I

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-08 Thread Jason Gunthorpe
On Wed, Jul 08, 2015 at 01:32:05PM -0700, 'Christoph Hellwig' wrote: On Wed, Jul 08, 2015 at 01:08:42PM -0600, Jason Gunthorpe wrote: Then, what is left is all remote MRs and maybe it will be clearer what to do about them then... From looking at that for a while the APIs needed seem pretty

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-08 Thread Tom Talpey
On 7/8/2015 3:08 PM, Jason Gunthorpe wrote: The MR stuff was never really designed, the HW people provided some capability and the SW side just raw exposed it, thoughtlessly. Jason, I don't disagree that the API can be improved. I have some responses to your statements below though. Why is

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-08 Thread Jason Gunthorpe
On Wed, Jul 08, 2015 at 10:29:56AM +0300, Sagi Grimberg wrote: Sure, but the oddness is that rdma_device_access_flags exists at all, not the wrapper. The wrapper is what we want the API to look like, I don't necessarily agree. The API we'd want is a single API at all the call sites to all

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-07 Thread Jason Gunthorpe
On Tue, Jul 07, 2015 at 09:05:15AM -0500, Steve Wise wrote: I took the feedback from Christoph and Jason to mean I should remove ib_get_dma_mr() entirely and pull its guts into rdma_get_dma_mr(), and change all the users of ib_get_dma_mr() to use rdma_get_dma_mr(). So the net result isn't a

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-07 Thread Sagi Grimberg
On 7/7/2015 7:17 PM, Jason Gunthorpe wrote: On Tue, Jul 07, 2015 at 09:05:15AM -0500, Steve Wise wrote: I took the feedback from Christoph and Jason to mean I should remove ib_get_dma_mr() entirely and pull its guts into rdma_get_dma_mr(), and change all the users of ib_get_dma_mr() to use

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-07 Thread 'Christoph Hellwig'
On Mon, Jul 06, 2015 at 09:23:42AM -0500, Steve Wise wrote: Please add an assert for the values that are allowed for attrs. It also would be highly useful to add a kerneldoc comment describing the function and the parameters. Also __bitwise sparse tricks to ensure the right flags are

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-07 Thread Christoph Hellwig
On Mon, Jul 06, 2015 at 07:17:38PM +0300, Sagi Grimberg wrote: Ok. I'll remove all uses of ib_get_dma_mr()... I meant that rdma_get_dma_mr can go away. I'd prefer to get the needed access_flags and just call existing verb. I strongly disagree. As this series has shown the existing API

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-07 Thread Sagi Grimberg
On 7/7/2015 12:00 PM, Christoph Hellwig wrote: On Mon, Jul 06, 2015 at 07:17:38PM +0300, Sagi Grimberg wrote: Ok. I'll remove all uses of ib_get_dma_mr()... I meant that rdma_get_dma_mr can go away. I'd prefer to get the needed access_flags and just call existing verb. I strongly

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-07 Thread Jason Gunthorpe
On Tue, Jul 07, 2015 at 07:27:47PM +0300, Sagi Grimberg wrote: Doesn't it look odd to you? Sure, but the oddness is that rdma_device_access_flags exists at all, not the wrapper. The wrapper is what we want the API to look like, if we could trivially change the WR format as well then

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-07 Thread Haggai Eran
...@mellanox.com; target- de...@vger.kernel.org; linux-...@vger.kernel.org; trond.mykleb...@primarydata.com; bfie...@fieldses.org Subject: Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags On 07/07/2015 17:17, Steve Wise wrote: -Original Message- From: linux-nfs-ow

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-07 Thread Steve Wise
...@mellanox.com; linux-rdma@vger.kernel.org; e...@mellanox.com; target-de...@vger.kernel.org; linux-...@vger.kernel.org; trond.mykleb...@primarydata.com; bfie...@fieldses.org Subject: Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags On 7/7/2015 12:00 PM, Christoph Hellwig wrote: On Mon

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-07 Thread Steve Wise
@vger.kernel.org; e...@mellanox.com; target- de...@vger.kernel.org; linux-...@vger.kernel.org; trond.mykleb...@primarydata.com; bfie...@fieldses.org Subject: Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags On 06/07/2015 02:22, Steve Wise wrote: +int rdma_device_access_flags(struct

Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-07 Thread Haggai Eran
; r...@mellanox.com; linux-rdma@vger.kernel.org; e...@mellanox.com; target- de...@vger.kernel.org; linux-...@vger.kernel.org; trond.mykleb...@primarydata.com; bfie...@fieldses.org Subject: Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags On 06/07/2015 02:22, Steve Wise wrote

RE: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

2015-07-07 Thread Steve Wise
...@vger.kernel.org; linux-...@vger.kernel.org; trond.mykleb...@primarydata.com; bfie...@fieldses.org Subject: Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags On 07/07/2015 17:17, Steve Wise wrote: -Original Message- From: linux-nfs-ow...@vger.kernel.org [mailto:linux-nfs-ow

  1   2   >