Re: [PATCH v5 1/4] IB/netlink: Add defines for local service requests through netlink

2015-06-11 Thread Jason Gunthorpe
On Thu, Jun 11, 2015 at 11:45:46PM +, Wan, Kaike wrote: > Apparently, to achieve this goal, we need the following changes: I don't expect anything more than a proper hole in the UAPI to make this work. I already have patches that implement full kernel support for the 6-path format, and global

Re: [PATCH for-next V5 00/12] Move RoCE GID management to IB/Core

2015-06-12 Thread Jason Gunthorpe
On Fri, Jun 12, 2015 at 03:29:25PM +0300, Or Gerlitz wrote: > I understand the email thread went down into further details from this > point and on, but still, I'd like to stop here and ask for short > clarification -- my understanding of things re this reader-writer > scheme is the following: >

Re: [PATCH 08/11] IB/cma: Add net_dev and private data checks to RDMA CM

2015-06-15 Thread Jason Gunthorpe
On Mon, Jun 15, 2015 at 11:47:13AM +0300, Haggai Eran wrote: > Instead of relying on a the ib_cm module to check an incoming CM request's > private data header, add these checks to the RDMA CM module. This allows a > following patch to to clean up the ib_cm interface and remove the code that > look

Re: [PATCH 02/11] IB/ipoib: Return IPoIB devices matching connection parameters

2015-06-15 Thread Jason Gunthorpe
On Mon, Jun 15, 2015 at 11:47:07AM +0300, Haggai Eran wrote: > +/* Called with an RCU read lock taken */ Add _rcu to the name? That is the standard convention. > +/* returns an IPoIB netdev on top a given ipoib device matching a pkey_index > + * and address, if one exists. */ > +static struct ne

Re: [PATCH 06/41] IB/hfi1: add char device instantiation code

2015-06-15 Thread Jason Gunthorpe
On Sun, Jun 14, 2015 at 11:58:00PM +0300, Or Gerlitz wrote: > On Fri, Jun 12, 2015 at 2:09 AM, Mike Marciniszyn > wrote: > > +++ b/drivers/infiniband/hw/hfi1/device.c > > > +int __init dev_init(void) > > +{ > > + int ret; > > + > > + ret = alloc_chrdev_region(&hfi1_dev, 0, HFI1_NMINOR

Re: [PATCH v5 1/4] IB/netlink: Add defines for local service requests through netlink

2015-06-15 Thread Jason Gunthorpe
On Fri, Jun 12, 2015 at 05:24:32AM +, Weiny, Ira wrote: > > PATH_USE_* is a better name. But I think the defines should be: > > PATH_USE_ANY (or ALL?) How about PATH_USE_CONNECTED? > Why not treat unknown type as "ANY/ALL" and return all 6 if > possible. If not possible/supported (ie like

Re: [PATCH 06/41] IB/hfi1: add char device instantiation code

2015-06-15 Thread Jason Gunthorpe
On Mon, Jun 15, 2015 at 06:11:02PM +, Hefty, Sean wrote: > > Was Al Viro's comment on qib addressed here? That would be a > > showstopper for me.. > > Do you have a link to this comment? I'm missing a bunch of messages from > this thread and can't find anything from Al in the logs. Author:

Re: [PATCH 04/11] IB/cm: Expose DGID in SIDR request events

2015-06-15 Thread Jason Gunthorpe
On Mon, Jun 15, 2015 at 09:32:53PM +, Hefty, Sean wrote: > > drivers/infiniband/core/cm.c | 7 +++ > > include/rdma/ib_cm.h | 2 ++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c > > index c5f5f89e274a..46f99ec

Re: [PATCH 06/41] IB/hfi1: add char device instantiation code

2015-06-17 Thread Jason Gunthorpe
On Wed, Jun 17, 2015 at 12:05:08PM +, Marciniszyn, Mike wrote: > Jason, what is your take on ioctl vs. write. Well, I think the global view keeps changing, so I'm not sure what is trendy now.. But personally, I hate seeing write() used to emulate ioctl() because 'ioctl is bad'. ie if you are

Re: [PATCH 04/11] IB/cm: Expose DGID in SIDR request events

2015-06-17 Thread Jason Gunthorpe
On Tue, Jun 16, 2015 at 02:25:07PM +0300, Haggai Eran wrote: > > But how is that going to work? How is the sender to know it should be > > sending a GRH with the CM message? > > If the admin wants to use SIDR with alias GIDs, they will need to > configure the system to enable GRH for such GMPs. (

Re: [PATCH 08/11] IB/cma: Add net_dev and private data checks to RDMA CM

2015-06-17 Thread Jason Gunthorpe
On Tue, Jun 16, 2015 at 08:26:26AM +0300, Haggai Eran wrote: > On 15/06/2015 20:08, Jason Gunthorpe wrote: > > On Mon, Jun 15, 2015 at 11:47:13AM +0300, Haggai Eran wrote: > >> Instead of relying on a the ib_cm module to check an incoming CM request's > >> private

Re: [PATCH REPOST libibverbs] Add IP and TCP/UDP TX checksum offload support

2015-06-18 Thread Jason Gunthorpe
On Sun, Jun 14, 2015 at 01:13:04PM +0300, Or Gerlitz wrote: > From: Moshe Lazer > > Add a device capability flag IB_DEVICE_IP_SUM to denote checksum offload > support. Devices should set this flag if they support insertion of IP, TCP > and UDP checksums on outgoing IP packets sent over IB UD or E

Re: [PATCH RFC] IB: Add rdma_cap_ib_switch helper and use where appropriate

2015-06-18 Thread Jason Gunthorpe
On Thu, Jun 18, 2015 at 04:18:27PM -0400, Hal Rosenstock wrote: > In addition, an ib_switch helper was added to ib_verbs.h > based on the is_switch device bit rather than node_type > (although those should be consistent). This is basically fine, but it would be nicer to change the port handling s

Re: [PATCHv2 RFC] IB: Add rdma_cap_ib_switch helper and use where appropriate

2015-06-22 Thread Jason Gunthorpe
nder the covers of using > rdma_[start end]_port rather than the open coding > previously used. > > Signed-off-by: Hal Rosenstock Looks pretty good now. Reviewed-By: Jason Gunthorpe Although a bitfield isn't my preference: > index 986fddb..b0f898e 10064

Re: [PATCH for-next V5 1/5] IB/uverbs: Fix reference counting usage of event files

2015-06-24 Thread Jason Gunthorpe
On Mon, Jun 22, 2015 at 05:47:14PM +0300, Yishai Hadas wrote: > fd_install(resp.async_fd, filp); > @@ -386,6 +376,7 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file, > return in_len; > > err_file: > + ib_uverbs_free_async_event_file(file); > fput(filp); This lo

Re: [PATCH for-next V5 3/5] IB/uverbs: Enable device removal when there are active user space applications

2015-06-24 Thread Jason Gunthorpe
On Mon, Jun 22, 2015 at 05:47:16PM +0300, Yishai Hadas wrote: > +++ b/drivers/infiniband/core/uverbs_main.c > @@ -137,7 +137,12 @@ static void ib_uverbs_release_dev(struct kref *ref) > struct ib_uverbs_device *dev = > container_of(ref, struct ib_uverbs_device, ref); > > -

Re: [PATCH for-next V5 3/5] IB/uverbs: Enable device removal when there are active user space applications

2015-06-25 Thread Jason Gunthorpe
On Thu, Jun 25, 2015 at 04:51:49PM +0300, Yishai Hadas wrote: > On 6/24/2015 9:25 PM, Jason Gunthorpe wrote: > >Is not holding the RCU lock while ib_uverbs_release_dev is reading > >ib_dev. The barriers in kref are not strong enough to guarentee the RCU > >protected data will

Re: [PATCH for-next V5 1/5] IB/uverbs: Fix reference counting usage of event files

2015-06-25 Thread Jason Gunthorpe
On Thu, Jun 25, 2015 at 02:46:02PM +0300, Yishai Hadas wrote: > On 6/24/2015 8:57 PM, Jason Gunthorpe wrote: > >On Mon, Jun 22, 2015 at 05:47:14PM +0300, Yishai Hadas wrote: > >>fd_install(resp.async_fd, filp); > >>@@ -386,6 +376,7 @@ ssize_t ib_uverbs_get_co

Re: [PATCH for-next V6 00/10] Move RoCE GID management to IB/Core

2015-06-25 Thread Jason Gunthorpe
On Thu, Jun 25, 2015 at 11:34:43AM +0300, Or Gerlitz wrote: > So... are we finally OK wrt the feedback you provided? I've been looking at Yishai's series, I though it was almost good to go, but the error flows are still wrong :( For Matan's patch, I only looked briefly, merging it with the othe

Re: [PATCH RFC 2/2] RDMA/isert: Support iWARP transport

2015-06-25 Thread Jason Gunthorpe
On Thu, Jun 25, 2015 at 10:39:23AM -0500, Steve Wise wrote: > + /* > + * IWARP transports need REMOTE_WRITE for MRs used as the target of > + * an RDMA_READ. Since the DMA MR is used for all ports, then if > + * any port is running IWARP, add REMOTE_WRITE. > + */ > + if

Re: [PATCH RFC 2/2] RDMA/isert: Support iWARP transport

2015-06-25 Thread Jason Gunthorpe
On Thu, Jun 25, 2015 at 06:45:56PM +, Hefty, Sean wrote: > > How would you envision doing this? At the time a MR is registered the > > device driver doesn't know if the application will be doing > > RDMA reads or not on that MR. > > I was thinking of checking for REMOTE_READ, but that doesn't

Re: [PATCH RFC 2/2] RDMA/isert: Support iWARP transport

2015-06-25 Thread Jason Gunthorpe
On Thu, Jun 25, 2015 at 02:25:49PM -0500, Steve Wise wrote: > To stage the changes we could introduce a new function that returns > the needed ib_access_flags value given the desired opcodes. Then > have a series that changes all the existing ULPs to make use of this > new function. I wouldn't b

Re: [PATCH RFC] RDMA/core: add rdma_get_dma_mr()

2015-06-25 Thread Jason Gunthorpe
On Thu, Jun 25, 2015 at 04:29:17PM -0500, Steve Wise wrote: > - * ib_dma_mapping_error - check a DMA addr for error > + * rdma_mr_roles - possible roles a MR will be used for > + * > + * This allows a transport independent RDMA application to > + * create MRs that are usable for all the desired ro

Re: [PATCH v6 4/4] IB/sa: Route SA pathrecord query through netlink

2015-06-25 Thread Jason Gunthorpe
On Fri, Jun 12, 2015 at 10:06:06AM -0400, kaike@intel.com wrote: > This patch routes a SA pathrecord query to netlink first and processes the > response appropriately. If a failure is returned, the request will be sent > through IB. The decision whether to route the request to netlink first is

Re: [PATCH RFC] RDMA/core: add rdma_get_dma_mr()

2015-06-29 Thread Jason Gunthorpe
On Mon, Jun 29, 2015 at 08:47:55AM -0500, Steve Wise wrote: > > On 26/06/2015 00:29, Steve Wise wrote: > > > +enum rdma_mr_roles { > > > + RDMA_MRR_RECV = 1, > > > + RDMA_MRR_SEND = (1<<1), > > > + RDMA_MRR_READ_SOURCE= (1<<2), > > > + RDMA_MRR_READ_

Re: [PATCH for-next V5 1/5] IB/uverbs: Fix reference counting usage of event files

2015-06-29 Thread Jason Gunthorpe
On Sun, Jun 28, 2015 at 05:33:04PM +0300, Yishai Hadas wrote: > You are wrong here, we have here balanced put, the first is done as > part of fput(filp) -> ib_uverbs_event_close_file -> > kref_put(&file->ref, ib_uverbs_release_event_file) and the second at > the end of this function as part of the

Re: [PATCH RFC 2/2] RDMA/isert: Support iWARP transport

2015-06-29 Thread Jason Gunthorpe
On Sat, Jun 27, 2015 at 12:12:11PM +0300, Sagi Grimberg wrote: > >Also since we have the new rdma_cap_read_multi_sge() helper, I > >thought I should use it. :) > > I think that reading the exact device caps max_sge, max_sge_is better > and more straight forward... Right, rdma_cap_read_multi_sge

Re: [PATCH] mlx4, mlx5, mthca: Expose max_sge_rd correctly

2015-06-29 Thread Jason Gunthorpe
On Mon, Jun 29, 2015 at 01:14:50PM -0500, Steve Wise wrote: > > On 6/29/2015 1:12 PM, Sagi Grimberg wrote: > >Applications must not assume that max_sge and max_sge_rd > >are the same, Hence expose max_sge_rd correctly as well. > > > >Reported-by: Steve Wise > >Signed-off-by: Sagi Grimberg > > H

Re: [PATCH for-next V5 1/5] IB/uverbs: Fix reference counting usage of event files

2015-06-29 Thread Jason Gunthorpe
On Tue, Jun 30, 2015 at 12:22:02AM +0300, Yishai Hadas wrote: > It should be: > kref_put(&uverbs_file->async_file->ref, ib_uverbs_release_event_file) > instead of: > kref_put(&uverbs_file->async_file->ref, ib_uverbs_release_file); Right > Please note that in that approach we duplicate above line

Re: [PATCH V2 3/5] RDMA/core: transport-independent access flags

2015-06-30 Thread Jason Gunthorpe
On Mon, Jun 29, 2015 at 04:36:18PM -0500, Steve Wise wrote: > +int rdma_device_access_flags(struct ib_pd *pd, int roles, int attrs) > +{ > + int access_flags = attrs; No RDMA_MRR_SEND ? > + if (roles & RDMA_MRR_RECV) > + access_flags |= IB_ACCESS_LOCAL_WRITE; > + > + if (r

Re: [PATCH V2 3/5] RDMA/core: transport-independent access flags

2015-06-30 Thread Jason Gunthorpe
> NFSRDMA currently checks the transport type to decide how to set the > access flags for memory registration. With the new services > exported in this series, we can change/simplify NFSRDMA to not have > to know the transport type. It would be excellent if this series actually went through and g

Re: [PATCH V2 4/5] RDMA/iser: support iWARP devices

2015-06-30 Thread Jason Gunthorpe
On Tue, Jun 30, 2015 at 09:33:21AM -0500, Steve Wise wrote: > I prefer to decouple the iSER changes with this core work. > Jason/Sean... thoughts? I could do the iSER w/o patch 3, and the > follow up with a series that includes our final solution on > transport independent memory registration and

Re: [PATCH V2 2/5] ipath,qib: Expose max_sge_rd correctly

2015-06-30 Thread Jason Gunthorpe
On Mon, Jun 29, 2015 at 04:36:13PM -0500, Steve Wise wrote: > Applications must not assume that max_sge and max_sge_rd > are the same, Hence expose max_sge_rd correctly as well. Chuck, Now that this works, can we change NFS RDMA and get rid of the rdma_cap_read_multi_sge stuff? Thanks, Jason --

Re: [PATCH for-next V6 1/5] IB/uverbs: Fix reference counting usage of event files

2015-06-30 Thread Jason Gunthorpe
On Tue, Jun 30, 2015 at 01:26:03PM +0300, Yishai Hadas wrote: > Signed-off-by: Yishai Hadas > Signed-off-by: Shachar Raindel Reviewed-By: Jason Gunthorpe > err_file: > + ib_uverbs_free_async_event_file(file); > fput(filp); > +void ib_uverbs_free_async

Re: [PATCH for-next V6 2/5] IB/uverbs: Explicitly pass ib_dev to uverbs commands

2015-06-30 Thread Jason Gunthorpe
y: Shachar Raindel Reviewed-By: Jason Gunthorpe Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next V6 3/5] IB/uverbs: Enable device removal when there are active user space applications

2015-06-30 Thread Jason Gunthorpe
On Tue, Jun 30, 2015 at 01:26:05PM +0300, Yishai Hadas wrote: > struct ib_uverbs_device { > - struct kref ref; > + struct kref comp_ref; > + struct kref free_ref; So.. I was looking at this, and there

Re: [PATCH v7 4/4] IB/sa: Route SA pathrecord query through netlink

2015-06-30 Thread Jason Gunthorpe
On Tue, Jun 30, 2015 at 09:45:55AM -0400, kaike@intel.com wrote: > +static inline int ib_nl_is_good_resolve_resp(const struct nlmsghdr *nlh) > +{ > + const struct nlattr *head, *curr; > + int len, rem; > + > + if (nlh->nlmsg_flags & RDMA_NL_LS_F_ERR) > + return 0; > + >

Re: [PATCH V3 4/4] RDMA/isert: Support iWARP transport

2015-07-02 Thread Jason Gunthorpe
On Thu, Jul 02, 2015 at 09:28:46AM +0300, Sagi Grimberg wrote: > Or has a good point. > The DMA mkey in target mode is discrete and not sent to any peer. That doesn't mean the peer cannot guess it. Using the right permission is clearly a stronger protection, we shouldn't weaken IB just to accomm

Re: [PATCH v7 1/4] IB/netlink: Add defines for local service requests through netlink

2015-07-03 Thread Jason Gunthorpe
On Tue, Jun 30, 2015 at 09:45:52AM -0400, kaike@intel.com wrote: > @@ -7,12 +7,14 @@ enum { > RDMA_NL_RDMA_CM = 1, > RDMA_NL_NES, > RDMA_NL_C4IW, > + RDMA_NL_SA, I think this should be RDMA_NL_LS to be consistent with the rest, the SA resolve OP should be something like:

Re: [PATCH v7 4/4] IB/sa: Route SA pathrecord query through netlink

2015-07-03 Thread Jason Gunthorpe
On Tue, Jun 30, 2015 at 09:45:55AM -0400, kaike@intel.com wrote: I still think this is long and rambly, but that is mostly a style preference, I think you should look at it and remove some of the left over stuff, like we really don't need a rough in for other responses at this point. > +#defi

Re: [PATCH for-next V6 3/5] IB/uverbs: Enable device removal when there are active user space applications

2015-07-06 Thread Jason Gunthorpe
On Mon, Jul 06, 2015 at 05:08:08PM +0300, Yishai Hadas wrote: > The patch that introduces this bug was added 5 years ago by Alex Chiang and > Signed-off-by: Roland Dreier. > > Look at commit ID:2a72f212263701b927559f6850446421d5906c41, it can be seen > also at: > http://git.kernel.org/cgit/linux/

Re: [PATCH v7 1/4] IB/netlink: Add defines for local service requests through netlink

2015-07-06 Thread Jason Gunthorpe
> > > +/* Local service status attribute */ > > > +enum { > > > + LS_NLA_STATUS_SUCCESS = 0, > > > + LS_NLA_STATUS_EINVAL, > > > + LS_NLA_STATUS_ENODATA, > > > + LS_NLA_STATUS_MAX > > > +}; > > > > So, this is never used, there seems to be a bunch of never used stuff > > - please audit everything

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

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 rdma_device

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

Re: [PATCH v1 01/12] IB/core: pass client data to remove() callbacks

2015-07-08 Thread Jason Gunthorpe
On Mon, Jun 22, 2015 at 03:42:30PM +0300, Haggai Eran wrote: > An ib_client callback that is called with the lists_rwsem locked only for > read is protected from changes to the IB client lists, but not from > ib_unregister_device() freeing its client data. This is because > ib_unregister_device() w

Re: [PATCH v1 02/12] IB/core: Find the network device matching connection parameters

2015-07-08 Thread Jason Gunthorpe
ms(struct ib_device *dev, u8 port, > + u16 pkey, const union ib_gid *gid, > + const struct sockaddr *addr); I feel like this has been repated a few times now, but kdocs should be with the function body, not in the header. Rev

Re: [PATCH v1 01/12] IB/core: pass client data to remove() callbacks

2015-07-08 Thread Jason Gunthorpe
On Wed, Jul 08, 2015 at 02:29:10PM -0600, Jason Gunthorpe wrote: > On Mon, Jun 22, 2015 at 03:42:30PM +0300, Haggai Eran wrote: > > An ib_client callback that is called with the lists_rwsem locked only for > > read is protected from changes to the IB client list

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 di

Re: [PATCH v1 03/12] IB/ipoib: Return IPoIB devices matching connection parameters

2015-07-08 Thread Jason Gunthorpe
On Mon, Jun 22, 2015 at 03:42:32PM +0300, Haggai Eran wrote: > + if (net_dev) { > + ipoib_warn(priv, "matching net_dev found: %s\n", > +net_dev->name); Is that a debug print? > + default: > + dev_warn(&dev->dev, "dupl

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 f

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

2015-07-09 Thread Jason Gunthorpe
On Thu, Jul 09, 2015 at 02:02:03PM +0300, Sagi Grimberg wrote: > We have protocol that involves remote memory keys transfer in their > standards so I don't see how we can remove it altogether from ULPs. This is why I've been talking about local and remote MRs differently. A Local MR is one where

Re: [PATCH 06/41] IB/hfi1: add char device instantiation code

2015-07-09 Thread Jason Gunthorpe
On Wed, Jul 08, 2015 at 09:42:39PM +, Marciniszyn, Mike wrote: > > netlink is a reasonable low speed format to use for this kind of > > serialization, > > either via the common mux or via your own char device. > > > > A couple of follow-up netlink questions. > > 1. I assume you are talking

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

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 i

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: kernel memory registration (was: RDMA/core: Transport-independent access flags)

2015-07-10 Thread Jason Gunthorpe
On Fri, Jul 10, 2015 at 11:55:29AM +0300, Sagi Grimberg wrote: > IMHO, memory registration is memory registration. The fact that we are > distinguishing between local and remote might be a sign that this might > be a wrong direction to take. Sorry. I belive they are very different, yes, if you si

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. A

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

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

2015-07-10 Thread Jason Gunthorpe
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 is we don't inadvertently open a security hole on

Re: [PATCH v1 09/12] xprtrdma: Prepare rpcrdma_ep_post() for RDMA_NOMSG calls

2015-07-10 Thread Jason Gunthorpe
On Fri, Jul 10, 2015 at 10:53:40AM -0400, Chuck Lever wrote: > It is certainly possible to examine the device’s max_sge field > in rpcrdma_ep_create() and fail transport creation if the > device’s max_sge is less than RPC_MAX_IOVS. I just want to draw Sagi's attention to this problem, when consid

Re: Kernel fast memory registration API proposal [RFC]

2015-07-13 Thread Jason Gunthorpe
On Fri, Jul 10, 2015 at 12:09:37PM +0300, Sagi Grimberg wrote: > Given the last discussions on our in-kernel memory registration API I > thought I'd propose another approach to address this. I assume you can put your new indirect registrations under this API without changing the callers? >

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 o

Re: kernel memory registration (was: RDMA/core: Transport-independent access flags)

2015-07-13 Thread Jason Gunthorpe
On Sat, Jul 11, 2015 at 03:31:53AM -0700, 'Christoph Hellwig' wrote: > I'm not too excited about moving the code in the drivers. The RDMA > subsystem actually has a lot more hardware drivers than ULDs, so moving > logic into them seems like a major step backwards. From my journeys > into the driv

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 i

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 1006

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 netf

Re: [PATCH for-next V6 04/10] IB/core: Add rwsem to allow reading device list or client list

2015-07-13 Thread Jason Gunthorpe
lls and remove from the lists before the remove() calls. > > Signed-off-by: Haggai Eran > Signed-off-by: Matan Barak Looks OK Reviewed-By: Jason Gunthorpe Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord..

Re: [PATCH v1 05/12] IB/cm: Share listening CM IDs

2015-07-13 Thread Jason Gunthorpe
> if (cur_cm_id_priv) { > cm_id->state = IB_CM_IDLE; > + --cm_id_priv->listen_sharecount; Ditto Otherwise I don't see any other mechanical problems with this. Sean said he was happy with the idea right? Reviewed-By: Jason Gunthorpe Jason -- T

Re: [PATCH v1 11/12] IB/cma: Share ib_cm_ids between rdma_cm_ids

2015-07-13 Thread Jason Gunthorpe
On Mon, Jun 22, 2015 at 03:42:40PM +0300, Haggai Eran wrote: > Use ib_cm_id_create_and_listen to create listening IB CM IDs or share ^^^ Is that the wrong name? ib_cm_insert_listen perhaps? I think I've looked at the details in this series I was concerned about, Sean

Re: [PATCH v1 08/12] IB/cma: Add net_dev and private data checks to RDMA CM

2015-07-13 Thread Jason Gunthorpe
On Mon, Jun 22, 2015 at 03:42:37PM +0300, Haggai Eran wrote: > + switch (ib_event->event) { > + case IB_CM_REQ_RECEIVED: > + req->device = req_param->listen_id->device; > + req->port = req_param->port; > + req->local_gid = &req_param->primary_p

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 su

Re: Kernel fast memory registration API proposal [RFC]

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 08:33:47AM -0700, Christoph Hellwig wrote: > On Tue, Jul 14, 2015 at 11:39:24AM +0300, Sagi Grimberg wrote: > > This is exactly what I don't want to do. I don't think that implicit > > posting is a good idea for reasons that I mentioned earlier: > > > > "This is where I hav

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: Kernel fast memory registration API proposal [RFC]

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 07:12:01PM +0300, Sagi Grimberg wrote: > >The ULP doesn't care if it needs to reserver the slot, and it generally > >doesn't care about the notification either unless it needs to handle an > >error. > > That's generally correct. But the ULP may want to suppress notification

Re: Kernel fast memory registration API proposal [RFC]

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 07:46:50PM +0300, Sagi Grimberg wrote: > >I'm really disappointed by the negative emails on this subject.. > I'm really not trying to be negative. I'm hearing you out, and I agree > with a lot of what you have to say. I just don't agree with all of it. Sure, I just find it

Re: Kernel fast memory registration API proposal [RFC]

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 07:55:39PM +0300, Sagi Grimberg wrote: > But, if people think that it's better to have an API that does implicit > posting always without notification, and then silently consume error or > flush completions. I can try and look at it as well. Can we do FMR transparently if

Re: kernel memory registration

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 11:24:26AM +0300, Sagi Grimberg wrote: > > > > >>Having a few schemes availabe in the core code that the driver can chose > >>from seems like a much more sensible option. > > > >I think that makes sense, but several of the schemes we are working > >with are effectively sing

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 d

Re: [PATCH v1 05/12] IB/cm: Share listening CM IDs

2015-07-14 Thread Jason Gunthorpe
On Tue, Jul 14, 2015 at 11:45:15AM +0300, Haggai Eran wrote: > > Reviewed-By: Jason Gunthorpe > Thanks. > Can I add it with the modifications above? Yep Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord..

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 li

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 hidd

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 opt

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

2015-07-14 Thread Jason Gunthorpe
is appears to be a sane way to advance indirect MR.. Jason >From 5a9799bf487d822daae5a8b8f3b197f1dda1db45 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 14 Jul 2015 14:27:37 -0600 Subject: [PATCH] IB/core: Guarantee that a local_dma_lkey is available Every single ULP requires a l

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* (con

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 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 ow

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 diff

Re: Kernel fast memory registration API proposal [RFC]

2015-07-15 Thread Jason Gunthorpe
On Wed, Jul 15, 2015 at 12:32:33AM -0700, Christoph Hellwig wrote: > int rdma_create_mr(struct ib_pd *pd, enum rdma_mr_type mr, > u32 max_pages, int flags); > > > * array from a SG list > > * @mr: memory region > > * @sg: sg list > > * @sg_nents:number of el

Re: Kernel fast memory registration API proposal [RFC]

2015-07-15 Thread Jason Gunthorpe
On Wed, Jul 15, 2015 at 10:32:55AM -0400, Chuck Lever wrote: > I would rather not build a non-deterministic delay into the > unmap interface. Using a pool or having map do an implicit > unmap are both solutions I’d rather avoid. Can you explain how NFS is using FMR today? When does it unmap a FMR

Re: Kernel fast memory registration API proposal [RFC]

2015-07-15 Thread Jason Gunthorpe
On Wed, Jul 15, 2015 at 11:01:46AM +0300, Sagi Grimberg wrote: > On 7/14/2015 8:09 PM, Jason Gunthorpe wrote: > >On Tue, Jul 14, 2015 at 07:55:39PM +0300, Sagi Grimberg wrote: > > > >>But, if people think that it's better to have an API that does implicit > >&

Re: [PATCH v1 08/12] IB/cma: Add net_dev and private data checks to RDMA CM

2015-07-15 Thread Jason Gunthorpe
On Wed, Jul 15, 2015 at 01:57:48PM +0300, Haggai Eran wrote: > On 13/07/2015 21:14, Jason Gunthorpe wrote: > > On Mon, Jun 22, 2015 at 03:42:37PM +0300, Haggai Eran wrote: > >> + switch (ib_event->event) { > >> + case IB_CM_REQ_RECEIVED: > >> +

Re: Kernel fast memory registration API proposal [RFC]

2015-07-15 Thread Jason Gunthorpe
> > - ULP calls a 'rdma_post_close_rkey' helper > > * For FRWR this posts the INVALIDATE > > Note: Some send operations automatically invalidate an rkey (and the > lkey for IB?). This is intended to avoid having to post the > invalidate WR explicitly. Namely IB_WR_READ_WITH_INV and > IB_W

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 l

Re: Kernel fast memory registration API proposal [RFC]

2015-07-15 Thread Jason Gunthorpe
On Wed, Jul 15, 2015 at 11:33:39AM +0300, Sagi Grimberg wrote: > >Call this rdma_mr to fit the scheme we use for "generic" APIs in the > >RDMA stack? > > Umm, I think this can become weird given all other primitives have > ib_ prefix. I'd prefer to keep that prefix to stay consistent, and have >

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

2015-07-15 Thread Jason Gunthorpe
On Wed, Jul 15, 2015 at 05:19:26AM -0700, 'Christoph Hellwig' wrote: > 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; > > >+ > > >+ r

Re: [PATCH v1 08/12] IB/cma: Add net_dev and private data checks to RDMA CM

2015-07-15 Thread Jason Gunthorpe
On Wed, Jul 15, 2015 at 08:27:06PM +, Liran Liss wrote: > If you want to restrict a container to a specific set of pkeys, use > cgroups. Ideally yes, but in the absence of a cgroup the set of pkeys assigned to the container via ipoib is a reasonable alternate. > This would apply both to CM MA

Re: Kernel fast memory registration API proposal [RFC]

2015-07-15 Thread Jason Gunthorpe
On Wed, Jul 15, 2015 at 05:25:11PM -0400, Chuck Lever wrote: > NFS READ and WRITE data payloads are mapped with ib_map_phys_mr() > just before the RPC is sent, and those payloads are unmapped > with ib_unmap_fmr() as soon as the client sees the server’s RPC > reply. Okay.. but.. ib_unmap_fmr is t

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

2015-07-15 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: https://github.com/jgunthor

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

  1   2   3   4   5   6   7   8   9   10   >