RE: [PATCHv3 TRIVIAL] IB/core: Documentation fix to ib_mad_snoop_handler in the MAD header file

2016-01-05 Thread Hefty, Sean
> In ib_mad.h, ib_mad_snoop_handler uses send_buf rather than send_wr The MAD snooping should be removed from the mad stack. There are no in tree users and the only attempt at adding one was rejected.

RE: [PATCHv3 TRIVIAL] IB/core: Documentation fix to ib_mad_snoop_handler in the MAD header file

2016-01-05 Thread Hefty, Sean
> > There are no in tree users and the only attempt at adding one was > rejected. > > There are no in tree users of this but there is your madeye tool (which > is out of tree). This is still a useful debug tool for MADs and there > are people who still use that. It's an out of tree tool.

RE: [PATCH 22/37] IB/rdmavt: Add queue pair data structure to rdmavt

2015-12-10 Thread Hefty, Sean
> >> +struct rvt_rwqe { > >> + u64 wr_id; > >> + u8 num_sge; > >> + struct ib_sge sg_list[0]; > >> +}; > >> + > >> +/* > >> + * This structure is used to contain the head pointer, tail pointer, > >> + * and receive work queue entries as a single memory allocation so > >> + * it can be mmap'ed

RE: [PATCH 03/37] IB/rdmavt: Add protection domain to rdmavt.

2015-12-10 Thread Hefty, Sean
> > How about I reword the comment to say that while we could continue > > allocating PDs being only constrained by system resources, the spec says > > there is a limit so we enforce that? > > I'm ok with that, Sean? That's fine -- To unsubscribe from this list: send the line "unsubscribe

RE: [PATCH V2 1/1] [PATCH 1/1] Ibacm: default pkey for partitioned fabrics

2015-12-10 Thread Hefty, Sean
Thanks - applied -- 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 1/1] Ibacm: default pkey for partitioned fabrics

2015-12-09 Thread Hefty, Sean
> > I mean that the user needs to investigate why the fabric is not working > out of box. > > My point is that an educated admin should _know_ to configure in these > cases and that debug is only when things are broken not by default in > these more complex cases. This means the limitations of

RE: [PATCH 1/1] Ibacm: default pkey for partitioned fabrics

2015-12-09 Thread Hefty, Sean
> pkey[0] at least has the logic that the admin will configure things so > that the default ipoib device reaches the broadest audiance makes the > most sense to me. That is what most sites I've seen want to do. Kaike, will pkey[0] work in the configurations that you're targeting with this

RE: [PATCH 1/1] Ibacm: default pkey for partitioned fabrics

2015-12-09 Thread Hefty, Sean
> >> Example: Compute nodes are assigned pkeys 0x8000 and 0x7fff. A node > >> running the job scheduler has pkeys 0x and 0x8000 (maybe it's > >> also the backup SA). Ibacm would need to select pkey 0x8000 for > >> communication. > > > > I've also seen the reverse, eg 0x is used for

RE: [PATCH 1/1] Ibacm: default pkey for partitioned fabrics

2015-12-09 Thread Hefty, Sean
> > 1. Find the first non-management full-member pkey; I.e. a pkey with the high-order bit set that is not 0x > > 2. If it fails, find pkey 0x; > > Order of 1 and 2 depends on use models for full default partition and > other partitions. Reversing 1 and 2 (full default partition first)

RE: [PATCH 03/37] IB/rdmavt: Add protection domain to rdmavt.

2015-12-08 Thread Hefty, Sean
> > > +struct ib_pd *rvt_alloc_pd(struct ib_device *ibdev, > > > +struct ib_ucontext *context, > > > +struct ib_udata *udata) > > > +{ > > > + struct rvt_dev_info *dev = ib_to_rvt(ibdev); > > > + struct rvt_pd *pd; > > > + struct ib_pd *ret; > > > + > > > +

RE: [PATCH 1/1] Ibacm: default pkey for partitioned fabrics

2015-12-08 Thread Hefty, Sean
> > +* Determine the default pkey for parsing address file as well. > > +* order of preference: first full-member non-management pkey, > > +* 0x, first pkey. > > +*/ > > This really should just be the 0 index pkey, which exactly matches how > IPoIB determines the default

RE: [PATCH 01/37] IB/rdmavt: Create module framework and handle driver registration

2015-12-08 Thread Hefty, Sean
> >> +#define RDMAVT_DRIVER_VERSION "0.1" > >Do we really need driver version? > > Based on these patches thus far. Probably not. We do still have a bit more > development to go here, I added it more as a just in case but am not > steadfast in keeping it. > > What's the general rule here? Should

RE: [PATCH 03/37] IB/rdmavt: Add protection domain to rdmavt.

2015-12-07 Thread Hefty, Sean
> +struct ib_pd *rvt_alloc_pd(struct ib_device *ibdev, > +struct ib_ucontext *context, > +struct ib_udata *udata) > +{ > + struct rvt_dev_info *dev = ib_to_rvt(ibdev); > + struct rvt_pd *pd; > + struct ib_pd *ret; > + > + pd =

RE: [PATCH 00/37] Add rdma verbs transport library

2015-12-07 Thread Hefty, Sean
> The following series implements rdmavt. This is the rdma verbs transport > software library This is going to be an annoying request, but I would rather see this named the IB transport library, unless this framework is usable over iWarp as well. N�r��yb�X��ǧv�^�)޺{.n�+{��ٚ�{ay�

RE: [PATCH 06/37] IB/rdmavt: Add query and modify device stubs

2015-12-07 Thread Hefty, Sean
> +static int rvt_query_device(struct ib_device *ibdev, > + struct ib_device_attr *props, > + struct ib_udata *uhw) > +{ > + /* > + * Return rvt_dev_info.props contents > + */ > + return -EINVAL; ENOSYS on all of the function

RE: [PATCH 01/37] IB/rdmavt: Create module framework and handle driver registration

2015-12-07 Thread Hefty, Sean
> @@ -0,0 +1,89 @@ > +/* > + * > + * This file is provided under a dual BSD/GPLv2 license. When using or > + * redistributing this file, you may do so under either license. > + * > + * GPL LICENSE SUMMARY > + * > + * Copyright(c) 2015 Intel Corporation. I'm guessing that the GPL license text

RE: [PATCH 05/37] IB/rdmavt: Macroize override checks during driver registration

2015-12-07 Thread Hefty, Sean
> +/* > + * Check driver override. If driver passes a value use it, otherwise we > use our > + * own value. > + */ > +#define CDR(rdi, x) \ > + rdi->ibdev.x = rdi->ibdev.x ? : rvt_ ##x This is an extremely obscure name. No one will be able to look at this: > + CDR(rdi, alloc_pd); > +

RE: [PATCH 21/37] IB/rdmavt: Move MR datastructures into rvt

2015-12-07 Thread Hefty, Sean
> include/rdma/rdma_vt.h | 53 > > 1 files changed, 53 insertions(+), 0 deletions(-) > > diff --git a/include/rdma/rdma_vt.h b/include/rdma/rdma_vt.h > index 5112dd7..39a0737 100644 > --- a/include/rdma/rdma_vt.h > +++ b/include/rdma/rdma_vt.h >

RE: [PATCH 22/37] IB/rdmavt: Add queue pair data structure to rdmavt

2015-12-07 Thread Hefty, Sean
> drivers/infiniband/sw/rdmavt/qp.h |5 - > include/rdma/rdma_vt.h| 233 > + > 2 files changed, 233 insertions(+), 5 deletions(-) > > diff --git a/drivers/infiniband/sw/rdmavt/qp.h > b/drivers/infiniband/sw/rdmavt/qp.h > index 4e4709f..c80d326

RE: [PATCH] IB/mad: In validate_mad, validate CM method and attribute

2015-11-12 Thread Hefty, Sean
> > + /* CM attributes other than ClassPortInfo only use Send method > */ > > + if (mad_hdr->mgmt_class == IB_MGMT_CLASS_CM) { > > + if (mad_hdr->attr_id != IB_MGMT_CLASSPORTINFO_ATTR_ID) { > > + if (mad_hdr->method !=

RE: [PATCH] IB/mad: In validate_mad, validate CM method and attribute

2015-11-12 Thread Hefty, Sean
> IIRC responding to Get(CPI) is mandatory? Maybe the drivers are responding? I don't believe that the CM does. -- 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

RE: [PATCH] IB/mad: In validate_mad, validate CM method and attribute

2015-11-12 Thread Hefty, Sean
> Receipt of CM MAD with response method for other than ClassPortInfo > attribute is invalid. > > CM attributes other than ClassPortInfo use send method only > and GetResp is valid for ClassPortInfo attribute. > Note also that the CM ClassPortInfo is not currently supported. > > The SRP

RE: [PATCH libibcm] cmpost.c: Handle ibv_get_device_list returning no IB devices in init()

2015-11-05 Thread Hefty, Sean
Merged - thanks. This is the first patch against the libibcm in over 4 years. Is there a reason why this is being used instead of the librdmacm? I ask because I assumed that the libibcm was basically deprecated. The last release was over 6 years ago. - Sean

RE: [PATCH rdma-RC] IB/cm: Fix rb-tree duplicate free and use-after-free

2015-10-26 Thread Hefty, Sean
> > Sean, I need to close on this patch. What is your position after > > Matan's explanation? > > > > Absent an objection from Sean, I've pulled this in. A use after free > bug is a pretty serious issue, and you've listed an error flow that > triggers it. The only thing bugging me is that this

RE: [PATCH rdma-RC] IB/cm: Fix sleeping while atomic when creating AH from WC

2015-10-20 Thread Hefty, Sean
> Today, cm assumes paths are reversible primary_path->reversible = 1. I can't quickly find a link, but I believe all CM MADs are reversible, per the spec.

RE: [PATCH rdma-RC] IB/cm: Fix sleeping while atomic when creating AH from WC

2015-10-15 Thread Hefty, Sean
> >> ib_create_ah_from_wc needs to resolve the DMAC in order to create the > >> AH (this may result sending an ARP and waiting for response). > >> CM uses this function (which is now sleepable). > > > > This is a significant change to the CM. The CM calls are invoked > assuming that they return

RE: [PATCH rdma-RC] IB/cm: Fix sleeping while atomic when creating AH from WC

2015-10-13 Thread Hefty, Sean
> On Mon, Oct 12, 2015 at 7:42 PM, Hefty, Sean <sean.he...@intel.com> wrote: > >> When IP based addressing was introduced, ib_create_ah_from_wc was > >> changed in order to support a suitable AH. Since this AH should > >> now contains the DMAC (which

RE: [PATCH rdma-RC] IB/cm: Fix sleeping while atomic when creating AH from WC

2015-10-12 Thread Hefty, Sean
> When IP based addressing was introduced, ib_create_ah_from_wc was > changed in order to support a suitable AH. Since this AH should > now contains the DMAC (which isn't a simple derivative of the GID). > In order to find the DMAC, an ARP should sometime be sent. This ARP > is a sleeping context.

RE: [PATCH rdma-RC] IB/cm: Fix rb-tree duplicate free and use-after-free

2015-10-12 Thread Hefty, Sean
> ib_send_cm_sidr_rep could sometimes erase the node from the sidr > (depending on errors in the process). Since ib_send_cm_sidr_rep is > called both from cm_sidr_req_handler and cm_destroy_id, cm_id_priv This should clarify that it is the app calling from the callback, and not a direct call

RE: [PATCH librdmacm] examples: Use gai_strerror rather than perror for [rdma_]getaddrinfo failures

2015-10-06 Thread Hefty, Sean
Thanks - merged all 3 -- 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: libmlx4 and libmlx5 git trees? Who is handling those?

2015-09-28 Thread Hefty, Sean
> Anyway, I had intended, and I've started on, making a change in how > libibverbs is done anyway. The idea that a new release is just a script > that throws a version on and we go is naive. I *will* be doing > pre-release rc tarballs and there will be testing and there will be a > release

RE: libmlx4 and libmlx5 git trees? Who is handling those?

2015-09-28 Thread Hefty, Sean
> >The issue is that basically no one tests the releases of the packages. > > We assume that the maintainer of the package tests it before they release > it :) No one has access to that many pieces of hardware and system configurations. The "community" can't dump the responsibility for testing

RE: [PATCH] infiniband:core:Fix error handling in the function cm_lap_handler

2015-09-18 Thread Hefty, Sean
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c > index ea4db9c..8fab3a7 100644 > --- a/drivers/infiniband/core/cm.c > +++ b/drivers/infiniband/core/cm.c > @@ -2812,7 +2812,12 @@ static int cm_lap_handler(struct cm_work *work) > cm_init_av_for_response(work->port,

RE: [PATCH 0/7] devcg: device cgroup extension for rdma resource

2015-09-11 Thread Hefty, Sean
> > Trying to limit the number of QPs that an app can allocate, > > therefore, just limits how much of the address space an app can use. > > There's no clear link between QP limits and HW resource limits, > > unless you assume a very specific underlying implementation. > > Isn't that the point

RE: [PATCH 0/7] devcg: device cgroup extension for rdma resource

2015-09-11 Thread Hefty, Sean
> So, the existence of resource limitations is fine. That's what we > deal with all the time. The problem usually with this sort of > interfaces which expose implementation details to users directly is > that it severely limits engineering manuevering space. You usually > want your users to

RE: RDMA/CM and multiple QPs

2015-09-10 Thread Hefty, Sean
> right now RDMA/CM works on a QP basis, but seems very awakward if you > want multiple QPs as part of a single logical device, which will be > useful for a lot of modern protocols. For example we will need to check > in the CM handler that we're not getting a different ib_device if we > want to

RE: [PATCH 0/7] devcg: device cgroup extension for rdma resource

2015-09-10 Thread Hefty, Sean
> > In past there has been similar comment to have dedicated cgroup > > controller for RDMA instead of merging with device cgroup. > > I am ok with both the approach, however I prefer to utilize device > > controller instead of spinning of new controller for new devices > > category. > > I

RE: [PATCH] infiniband:core:Fix assumation that the function ib_send_cm_drep never fails in rdma_disconnect

2015-09-08 Thread Hefty, Sean
> This fixes the incorrect assumation that the function ib_send_cm_drep > always runs successfully and never fails in the function rdma_disconnect > by making this call be assigned the variable used to return to callers > of rdma_disconnect in order to make sure that a error code is returned > if

RE: [PATCH] infiniband:core:Fix mutex locking to lock before unlocking in the function cma_ib_mc_handler

2015-09-08 Thread Hefty, Sean
> This fixes mutex locking to lock before unlocking in the function > cma_ib_mc_handler for the mutex handler_mutex as part of the pointer > id_priv before unlocking it after the critical region for event handler > work and execution in order to have actual proper concurrent execution > protection

RE: [PATCH] infiniband:Fix error handling in the function cm_init_av_by_path

2015-09-08 Thread Hefty, Sean
> This fixes error handling in the function cm_init_av_by_path > to properly check if the internal call to the function > ib_init_ah_from_path has failed by returning a error code and > if so return immediately to the caller with this error code in > order to signal/allow the caller to handle the

RE: [PATCH] infiniband:core:Fix error handling in the function join_handler

2015-09-08 Thread Hefty, Sean
> diff --git a/drivers/infiniband/core/multicast.c > b/drivers/infiniband/core/multicast.c > index 2cb865c..9284337 100644 > --- a/drivers/infiniband/core/multicast.c > +++ b/drivers/infiniband/core/multicast.c > @@ -526,8 +526,9 @@ static void join_handler(int status, struct > ib_sa_mcmember_rec

RE: [PATCH v6 0/4] Add network namespace support in the RDMA-CM

2015-08-27 Thread Hefty, Sean
This series looks reasonable to me -- 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 1/3] IB/uverbs: reject invalid or unknown opcodes

2015-08-19 Thread Hefty, Sean
AFAIK, this path is rarely (never?) actually used. I think all the drivers we have can post directly from userspace. I didn't think the ipath or qib drivers post from userspace. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to

RE: [PATCH] IB/ucma: Fix theoretical user triggered use-after-free

2015-08-06 Thread Hefty, Sean
Something like this: CPU A CPU B ucma_destroy_id() wait_for_completion() .. anything ucma_put_ctx()

RE: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-05 Thread Hefty, Sean
+enum ib_csum_cap_flags { + IB_CSUM_RX_TCP_UDP = 1 0, + IB_CSUM_RX_IP_HDR= 1 1, + IB_CSUM_TX_TCP_UDP = 1 2, + IB_CSUM_TX_IP_HDR= 1 3 +}; TPC and UDP should be separate flags. -- To unsubscribe from this list: send the line unsubscribe

RE: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-05 Thread Hefty, Sean
+enum ib_csum_cap_flags { + IB_CSUM_RX_TCP_UDP = 1 0, + IB_CSUM_RX_IP_HDR= 1 1, + IB_CSUM_TX_TCP_UDP = 1 2, + IB_CSUM_TX_IP_HDR= 1 3 +}; TPC and UDP should be separate flags. Can you explain why? For the same reason that you didn't include

RE: [RFC] split struct ib_send_wr

2015-08-04 Thread Hefty, Sean
please take a look at my RFC patch here: http://git.infradead.org/users/hch/scsi.git/commitdiff/751774250b71d a83a26ba8584cff70f5e7bb7b1e the commit contains my explanation, but apparently the patch is too large for the list limit and didn't make it through. This looks like a

RE: [PATCH v2 05/13] IB/cm: Share listening CM IDs

2015-07-27 Thread Hefty, Sean
+/** + * Create a new listening ib_cm_id and listen on the given service ID. + * + * If there's an existing ID listening on that same device and service ID, + * return it. + * + * @device: Device associated with the cm_id. All related communication will + * be associated with the

RE: [PATCH v2 07/13] IB/cma: Helper functions to access port space IDRs

2015-07-27 Thread Hefty, Sean
Add helper functions to access the IDRs by port-space and port number. Pass around the port-space enum in cma.c instead of using pointers to port-space IDRs. Why? drivers/infiniband/core/cma.c | 81 -- - 1 file changed, 60 insertions(+), 21

RE: [PATCH v2 09/13] IB/cma: Add net_dev and private data checks to RDMA CM

2015-07-27 Thread Hefty, Sean
@@ -1040,6 +1040,181 @@ static int cma_save_net_info(struct sockaddr *src_addr, return cma_save_ip_info(src_addr, dst_addr, ib_event, service_id); } +struct cma_req_info { + struct ib_device *device; + int port; + const union ib_gid *local_gid; The use of a pointer to

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 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: rdma_getaddrinfo and GUID

2015-07-23 Thread Hefty, Sean
I'm use cat /sys/class/infiniband/mlx4_0/ports/1/gids/0 fe80::::0002:c903:00ef:6601 In node I'm use gid provided below service null, hints contains af_ib and rai_family|rai_numerichost Ah, yes, the rdma_cm supports GIDs. It does not support GUIDs. Though it's usually

RE: rdma_getaddrinfo and GUID

2015-07-23 Thread Hefty, Sean
Hello, does it possible to use rdma_getaddrinfo and specify in node port GUID? The current code does not support this. Why rdma_getaddrinfo does not return error? Why it returns success and result contains port guid? I don't think I'm following your questions on how

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: rdma_getaddrinfo and GUID

2015-07-22 Thread Hefty, Sean
Hello, does it possible to use rdma_getaddrinfo and specify in node port GUID? The current code does not support this.

RE: [PATCH WIP 01/43] IB: Modify ib_create_mr API

2015-07-22 Thread Hefty, Sean
+enum ib_mr_type { + IB_MR_TYPE_FAST_REG, + IB_MR_TYPE_SIGNATURE, If we're going to go through the trouble of changing everything, I vote for dropping the word 'fast'. It's a marketing term. It's goofy. And the IB spec is goofy for using it. -- To unsubscribe from this list: send

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 v7 1/4] IB/netlink: Add defines for local service requests through netlink

2015-06-30 Thread Hefty, Sean
include/uapi/rdma/rdma_netlink.h | 87 ++ 1 files changed, 87 insertions(+), 0 deletions(-) diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h index 6e4bb42..d2c50e9 100644 --- a/include/uapi/rdma/rdma_netlink.h +++

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

2015-06-30 Thread Hefty, Sean
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 change all the TI kernel users (iser and nfsrdma) along

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

2015-06-30 Thread Hefty, Sean
I suggest to start consolidating to ib_create_mr() that receives an extensible ib_mr_init_attr and additional attributes can be mr_roles and mr_attrs. I think this makes sense, but does it really help? If the end result is that the app and providers basically end up switching on

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

2015-06-29 Thread Hefty, Sean
+enum rdma_mr_roles { + RDMA_MRR_RECV = 1, + RDMA_MRR_SEND = (11), + RDMA_MRR_READ_SOURCE= (12), + RDMA_MRR_READ_SINK = (13), Maybe it's just me, but it took me a second to figure out

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

2015-06-28 Thread Hefty, Sean
I wanted to point out that with this scheme the ULP may sometimes get an MR with a wider set of permissions that it asked for, and I'm not sure that's always safe. Perhaps the ULP wants to guarantee that the MR doesn't allow certain kinds of accesses and doesn't expect the verbs layer to

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

2015-06-25 Thread Hefty, Sean
+* 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 (any_port_is_iwarp(device)) It would be nice to have a new-style cap test

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

2015-06-25 Thread Hefty, Sean
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 work on the initiator side. I guess you could a READ_DEST(SOURCE?

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

2015-06-25 Thread Hefty, Sean
What about moving to something more specific? Encode the allowed verbs in the access flag? This makes more sense to me. Something like: SEND, RECV, INIT READ, INIT WRITE, READ TARGET, WRITE TARGET, etc. We're close to this, but it's not clear, for example, what flags are needed for a

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

2015-06-25 Thread Hefty, Sean
+enum rdma_mr_roles { I would drop naming the enum - it shouldn't be used, as the values are bit flags. + RDMA_MRR_RECV = 1, + RDMA_MRR_SEND = (11), + RDMA_MRR_READ_SOURCE= (12), + RDMA_MRR_READ_SINK = (13), +

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

2015-06-22 Thread Hefty, Sean
Note that there still remain a couple of node type checks in the kernel that we may want to remove. There's an IB CA check in cma.c:rdma_notify This should check for rdma_cap_ib_cm() as well as in rds/ib.c:rds_ib_add_one and rds_ib_laddr_check and Not sure what the underlying reason is for

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

2015-06-22 Thread Hefty, Sean
rds_ib_add_one says: /* Only handle IB (no iWARP) devices */ but not sure comment is 100% accurate as it checks node type != IB CA. rds_ib_laddr_check says: /* rdma_bind_addr will only succeed for IB iWARP devices */ /* due to this, we will claim to support iWARP devices unless we check

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

2015-06-18 Thread Hefty, Sean
In an effort to reform the RDMA core and ULPs to minimize use of node_type in struct ib_device, an additional bit is added to struct ib_device for is_switch (IB switch). This is needed to be initialized by any IB switch device driver. This is a NEW requirement on such device drivers which are

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

2015-06-18 Thread Hefty, Sean
In an effort to reform the RDMA core and ULPs to minimize use of node_type in struct ib_device, an additional bit is added to struct ib_device for is_switch (IB switch). This is needed to be initialized by any IB switch device driver. This is a NEW requirement on such device drivers which are

RE: [PATCH v2 01/49] IB/core: Add OPA Port header definitions

2015-06-17 Thread Hefty, Sean
include/rdma/opa_port_info.h | 433 This matches the current code structure, but is this the best location for this file? Do you have a suggestion? Couldn't it be a header in the hfi1 driver directory ? Isn't it OPA specific ? I don't have a specific suggestion. If the

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

2015-06-17 Thread Hefty, Sean
PSM2 (and PSM) uses this to dialog with the driver for hardware specific setup. I'm suspected at some point in the past, this was ioctl based and changed due to bias against ioctl. Could this be distinguished based on a common command header? -- To unsubscribe from this list: send the line

RE: [PATCH 00/41] Add OPA gen1 driver

2015-06-17 Thread Hefty, Sean
Ummm.. Could we get some more descriptions as to what this code is for? Do we have a new OmniPath protocol here as well or is it IB? Which standards are followed? I think the APIs that the driver uses need to be documented somewhere in particular if new sysfs entries etc are created.

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

2015-06-16 Thread Hefty, Sean
The idea is to allow SIDR request to be sorted by the GID, when we will have alias GIDs for IPoIB. Please limit this series, or at least the early patches in this series, to simply moving the de-mux out of the ib_cm and into the rdma_cm. -- To unsubscribe from this list: send the line

RE: [PATCH 14/14] IB/mad: Add final OPA MAD processing

2015-06-16 Thread Hefty, Sean
You're right that apps can be coded to other CA types, like RNICs and USNICs. However, those are all very different from an IB_CA due to limited queue pair types or limited primitives. If OPA had that same limitation then I would agree it needs a different node type. So this will be my

RE: [PATCH acm] acme.c: Fix IPv6 address handling in inet_any_pton

2015-06-16 Thread Hefty, Sean
Thanks - applied manually acm/src/acme.c |2 +- Path? diff --git a/acm/src/acme.c b/acm/src/acme.c index 9bf7557..d54c8b9 100644 --- a/acm/src/acme.c +++ b/acm/src/acme.c @@ -787,7 +787,7 @@ static int inet_any_pton(char *addr, struct sockaddr This was off by a couple hundred lines.

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

2015-06-15 Thread Hefty, Sean
+int __init dev_init(void) +{ + int ret; + + ret = alloc_chrdev_region(hfi1_dev, 0, HFI1_NMINORS, DRIVER_NAME); + if (ret 0) { + pr_err(Could not allocate chrdev region (err %d)\n, - ret); + goto done; + } + + class

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

2015-06-15 Thread Hefty, Sean
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. - Sean -- To unsubscribe from this list: send the line unsubscribe linux-rdma in

RE: [PATCH v2 01/49] IB/core: Add OPA Port header definitions

2015-06-15 Thread Hefty, Sean
include/rdma/opa_port_info.h | 433 This matches the current code structure, but is this the best location for this file?

RE: [PATCH 03/11] IB/cm: Expose service ID in request events

2015-06-15 Thread Hefty, Sean
Expose the service ID on an incoming CM or SIDR request to the event handler. This will allow the RDMA CM module to de-multiplex connection requests based on the information encoded in the service ID. Signed-off-by: Haggai Eran hagg...@mellanox.com Acked-by: Sean Hefty sean.he...@intel.com

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

2015-06-15 Thread Hefty, Sean
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..46f99ec4080a 100644 --- a/drivers/infiniband/core/cm.c +++

RE: [PATCH 05/11] IB/cm: Share listening CM IDs

2015-06-15 Thread Hefty, Sean
@@ -722,6 +725,7 @@ struct ib_cm_id *ib_create_cm_id(struct ib_device *device, INIT_LIST_HEAD(cm_id_priv-work_list); atomic_set(cm_id_priv-work_count, -1); atomic_set(cm_id_priv-refcount, 1); + cm_id_priv-listen_sharecount = 1; This is setting the listen count before

RE: [PATCH v2 25/49] IB/hfi1: add local mad header

2015-06-15 Thread Hefty, Sean
drivers/infiniband/hw/hfi1/mad.h | 477 ++ 1 file changed, 477 insertions(+) create mode 100644 drivers/infiniband/hw/hfi1/mad.h diff --git a/drivers/infiniband/hw/hfi1/mad.h b/drivers/infiniband/hw/hfi1/mad.h new file mode 100644 index

RE: [PATCH for-next V2 0/9] Add completion timestamping support

2015-06-11 Thread Hefty, Sean
Intel is supporting multicast in hardware. Its just a bad implementation (broadcast and filtering MC groups in the HCA or what was that?) and there is no plan to fix the issues despite the problem being known for quite some time. Also does this mean that libfabric only to supports the

RE: [PATCH 14/14] IB/mad: Add final OPA MAD processing

2015-06-11 Thread Hefty, Sean
cap_is_switch_smi would be a nice refinement to let us drop nodetype. Exactly, we need a bit added to the immutable data bits, and a new cap_ helper, and then nodetype is ready to be retired. Add a bit, drop a u8 ;-) I agree that the node type enum isn't particularly useful and should be

RE: [PATCH 14/14] IB/mad: Add final OPA MAD processing

2015-06-11 Thread Hefty, Sean
I agree that the node type enum isn't particularly useful and should be retired. Are you referring to kernel space or user space or both ? Short term, kernel space. User space needs to keep something around for backwards compatibility. But the in tree code will never expose this value

RE: [PATCH] IB/core: Don't warn on no SA support in event handler

2015-06-10 Thread Hefty, Sean
Registering an event handler is done for a device. This device may have one RoCE port (no SA cap) and one InfiniBand port (has SA cap). Therefore, warning from the event handler about a specific port that doesn't have SA cap is correct but pollutes the kernel log without a need. Maybe

RE: [PATCH for-next V2 0/9] Add completion timestamping support

2015-06-10 Thread Hefty, Sean
There are multiple problems with libfrabric related to the use cases in my area. Most of all the lack of multicast support. Then there is the build up of software bloat on top. The interest here is in low latency operations. Redenzvous and other new features are really not wanted if they

RE: rdmacm issue

2015-06-10 Thread Hefty, Sean
RDMA_CM_EVENT_UNREACHABLE is indicated when there are timeouts in underlying CM protocol exchange. I suspect that the server is really busy and doesn't respond to the low level CM MADs in a timely manner. RDMA CM (and other kernel ULPs like IPoIB and SRP use hard coded local and remote

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

2015-06-10 Thread Hefty, Sean
+/* Local Service Reversible attribute */ struct +rdma_nla_ls_reversible { + __u32 reversible; +}; Isn't __u8 sufficient for reversible ? Certainly enough. However, reversible is __u32 in struct ib_user_path_rec and int in struct ib_sa_path_rec. OK; I hadn't double

RE: [PATCH v4 0/4] Sending kernel pathrecord query to user cache server

2015-06-10 Thread Hefty, Sean
This series does not attempt to optimize the kernel needing to know that a PR has been updated. There are existing mechanisms for that. Does this exist in the kernel? -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org

RE: [PATCH v4 0/4] Sending kernel pathrecord query to user cache server

2015-06-10 Thread Hefty, Sean
Not in the patches themselves but in the general issue when a PR changes. Do you think this needs addressing or are things fine as they are now ? IMO, I think it needs addressing in terms of can the proposed netlink architecture and design accommodate this sort of request in the future? We

RE: [PATCH v4 0/4] Sending kernel pathrecord query to user cache server

2015-06-10 Thread Hefty, Sean
Not directly. IPoIB treats it that way. I guess to be safe. Officially one should register for UnPath/RePath traps. But no one has ever implemented that. To be clear I am agreeing with Hal that having some sort of update signal would be nice. But I don't think that must be done before

RE: [PATCH v4 0/4] Sending kernel pathrecord query to user cache server

2015-06-10 Thread Hefty, Sean
This series does not attempt to optimize the kernel needing to know that a PR has been updated. There are existing mechanisms for that. Does this exist in the kernel? At least some support, yes. For example client reregister marks all IPoIB paths as invalid. Reregister indicates

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

2015-06-10 Thread Hefty, Sean
Sean, this change is needed b/c two drivers have (mlx4 and ocrda) and more two to come soon (mlx5 and soft-Roce) would have the very same logic of constructing the port GID table according to netdev events and such, no point in repeating this logic/code over and over. Matan explained why we

RE: [PATCH v4 0/4] Sending kernel pathrecord query to user cache server

2015-06-10 Thread Hefty, Sean
While this appears to address the current upstream use model for ACM with it's multicast overlay backend where PRs are static, it does not appear to address PR changes. Although this ties into ibacm, from the viewpoint of the kernel, there's no requirement on the user space implementation.

RE: [PATCH 10/14] IB/mad: Add support for additional MAD info to/from drivers

2015-06-08 Thread Hefty, Sean
@@ -1693,8 +1693,11 @@ struct ib_device { u8 port_num, const struct ib_wc *in_wc, const struct ib_grh *in_grh, -

RE: [PATCH] IB/core: Get rid of redundant verb ib_destroy_mr

2015-06-08 Thread Hefty, Sean
--- a/drivers/infiniband/hw/mlx5/mr.c +++ b/drivers/infiniband/hw/mlx5/mr.c @@ -1174,6 +1174,18 @@ static int clean_mr(struct mlx5_ib_mr *mr) int umred = mr-umred; int err; + if (mr-sig) { + if (mlx5_core_destroy_psv(dev-mdev, +

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

2015-06-08 Thread Hefty, Sean
Previously, every vendor implemented its net device notifiers in its own driver. This introduces a huge code duplication as figuring 28 files changed, 2253 insertions(+), 860 deletions(-) How does adding 1400 lines of code help reduce code duplication? Can you please explain and justify

  1   2   3   4   5   6   7   8   9   10   >