Re: [PATCH] mlx4: properly mask MGM entry members count
On Mon, Nov 29, 2010 at 7:34 PM, Roland Dreier rdre...@cisco.com wrote: I afraid that you overloaded with dozen of e-mails and missed this question, so I'd ask you again. Is there any reason why this members_count patch cannot be applied? No, it is fine. I will merge it for 2.6.38. However this long thread should (I hope) show how the changelog could be improved -- if the original patch had said why the patch was needed then we could have avoided all this long discussion. - R. Excellent news, thanks. -- 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 V2 5/5] RDMA CM: Netlink Client
Jason Gunthorpe wrote: struct ib_nl_qp // String names for IB devices was a mistake, don't perpetuate it. __u32 ib_dev_if; Jason, Do you have a concrete suggestion and/or sketch for a patch someone can work on to make this enumeration to take place? Or. -- 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 V2 5/5] RDMA CM: Netlink Client
On Mon, 2010-11-29 at 12:11 -0700, Jason Gunthorpe wrote: On Mon, Nov 29, 2010 at 06:16:39PM +0200, Nir Muchtar wrote: Add callbacks and data types for statistics export. One callback is implemented that exports all of the current devices/ids. Add/remove the callback to IB Netlink on init/cleanup. Please include the schema for the messages you are adding to netlink in the comment so other people can review them easily. Looks to me like you have messages of the form: { [IBNL_RDMA_CM_DEVICE_NAME - char[]] [IBNL_RDMA_CM_ID_STATS - struct rdma_cm_id_stats]* }* Yes that's the basic structure. I'll add an explanation next time. As I've said before, I don't want to see this tied to RDMA_CM, that is not general enough for a new userspace API. The use of IBNL_RDMA_CM_DEVICE_NAME is very un-netlink-like and is just an ugly hack to avoid addressing that problem. This is done to save space in the netlink messages. I am open for ideas for improvements. I thought of another possibility: We can make another op for rdma devices only with index mapping. This could create a problems if a device is added/removed between calls though. See my question about your suggestion below. How about messages of the form: { IBNL_QP - struct ib_nl_qp IBNL_QP_ATTR - struct ib_qp_attr (or a reasonable subset) IBNL_QP_CM_INFO u8[] - struct ib_nl_qp_cm_info IBNL_QP_CM_SERVICE_ID u8[] IBNL_RDMA_CM_INFO - struct ib_nl_rdma_cm_info IBNL_RDMA_CM_SRC - u8[] // This is a sockaddr_* IBNL_RDMA_CM_DST - u8[] }+ Meaning there is an array of IBNL_QP messages which contains various attributes. Similar to how everything else in netlink works. struct ib_nl_qp { // String names for IB devices was a mistake, don't perpetuate it. I don't know of an IB device index mapping like the one in netdevice. Am I missing one? Do you mean we should create one? __u32 ib_dev_if; __u32 qp_num; __s32 creator_pid; // -1 for kernel consumer }; struct ib_nl_qp_cm_info { u32 cm_state; // enum ib_cm_state u32 lap_state; }; struct ib_nl_rdma_cm_info { __u32 bound_dev_if; __u32 family; __u32 cm_state; // enum rdma_cm_state }; This captures more information and doesn't tie things to RDMA_CM. My problem with making everything QP related is that not everything necessarily is. For example, when creating rdma cm id's they are still not bound to QP's. I guess you could send all zeros in this case, but as more and more of such exceptions are needed this framework will become a bit unnatural. The current implementation is not tying anything to RDMA CM. It allows other modules to export data exactly the way they need. +static int cma_get_stats(struct sk_buff **nl_skb, int pid) You really have to use netlink_dump_start here, doing it like this will deadlock. See how other places use NLM_F_DUMP as well. Well, I already reviewed netlink_dump_start, and this is how it works as much as I can see (Please correct me if I'm wrong): 1. Allocates an skb 2. Calls its supplied dump cb 3. Calls its supplied done cb if if applicable 4. Appends NLMSG_DONE This appears to be executed synchronously, within the context of the calling thread. So I couldn't figure out how to use it for avoiding long locking times. Anyway, what I tried to achieve is a mechanism that allocates more skb's as they are needed, and separate it from the calling module. Do you know of an inherent way to make netlink_dump_start to do that? +struct rdma_cm_id_stats { + enum rdma_node_type nt; + int port_num; + int bound_dev_if; + __be16 local_port; + __be16 remote_port; + __be32 local_addr[4]; + __be32 remote_addr[4]; + enum rdma_port_space ps; + enum rdma_cm_state cm_state; + u32 qp_num; + pid_t pid; +}; Putting enums in a user/kernel structure like this makes me nervous that we'll have a 32/64 bit problem. It would be more consistent with the uverbs stuff to use explicit fixed width types. Yes you're right. Also, I see now that this is not normally done this way, so I'll drop the enums. Jason Thanks again for all your input. Nir -- 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 0/2] opensm: Bug fixes for torus-2QoS patchset
On 11:03 Fri 17 Sep , Jim Schutt wrote: Hi Sasha, These patches fix bugs discovered during further testing of the torus-2QoS routing module for OpenSM. They apply to your torus-2qos branch. Thanks -- Jim Jim Schutt (2): opensm/osm_torus.c: Add check for invalid topology discovery due to user misconfiguration. opensm/osm_torus.c: Handle calloc() failure on routing engine context creation. opensm/opensm/osm_torus.c | 24 +++- 1 files changed, 23 insertions(+), 1 deletions(-) Applied. Thanks. Sasha -- 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] infiniband-diags/ibtracert: Eliminate direct route (-D) option
On 11:10 Tue 14 Sep , Hal Rosenstock wrote: Direct route does not make sense as an ibtracert option so eliminate it. Signed-off-by: Hal Rosenstock h...@mellanox.com Applied. Thanks. Sasha -- 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 00/13] opensm: Cleanups and more documentation for torus-2QoS patchset
On 15:11 Fri 12 Nov , Jim Schutt wrote: Hi Sasha, These patches clean up and add documentation to the torus-2QoS routing module for OpenSM. They apply on top of my previous bug-fix patchset from September (http://www.spinics.net/lists/linux-rdma/msg05809.html), which applies to your torus-2qos branch. Applied. Thanks. Sasha -- 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] infiniband-diags/saquery.c: In dump_one_mcmember_record, fix flow label endian
On 09:06 Wed 27 Oct , Hal Rosenstock wrote: Already in host endian Signed-off-by: Hal Rosenstock h...@mellanox.com Applied by hands (the patch was malformed). Thanks. Sasha --- diff --git a/infiniband-diags/src/saquery.c b/infiniband-diags/src/saquery.c index cb4fc18..18c5a1c 100644 --- a/infiniband-diags/src/saquery.c +++ b/infiniband-diags/src/saquery.c @@ -1,7 +1,7 @@ /* * Copyright (c) 2006,2007 The Regents of the University of California. * Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved. - * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved. + * Copyright (c) 2002-2010 Mellanox Technologies LTD. All rights reserved. * Copyright (c) 1996-2003 Intel Corporation. All rights reserved. * Copyright (c) 2009 HNR Consulting. All rights reserved. * @@ -461,7 +461,7 @@ static void dump_one_mcmember_record(void *data) inet_ntop(AF_INET6, mr-port_gid.raw, gid, sizeof(gid)), cl_ntoh32(mr-qkey), cl_ntoh16(mr-mlid), mr-mtu, mr-tclass, cl_ntoh16(mr-pkey), mr-rate, mr-pkt_life, sl, -cl_ntoh32(flow), hop, scope, join, mr-proxy_join); +flow, hop, scope, join, mr-proxy_join); } static void dump_multicast_group_record(void *data) -- 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] infiniband-diags/iblinkinfo.c: Limit some queries to switches
On 08:47 Wed 27 Oct , Hal Rosenstock wrote: infiniband-diags/iblinkinfo.c: Limit certain queries to switches iblinkinfo man page says: DESCRIPTION iblinkinfo reports the link info for each port of each switch active in the IB fabric. OPTIONS -S guid Output only the switch specified by guid (hex format) -D direct_route Output only the switch specified by the direct route path. Queries by DR path or GUID should (also) be limited to switches. Signed-off-by: Hal Rosenstock h...@mellanox.com Applied by hands (the patch is malformed). Thanks. Sasha --- diff --git a/infiniband-diags/src/iblinkinfo.c b/infiniband-diags/src/iblinkinfo.c index d0c9b13..b129cb6 100644 --- a/infiniband-diags/src/iblinkinfo.c +++ b/infiniband-diags/src/iblinkinfo.c @@ -2,6 +2,7 @@ * Copyright (c) 2004-2009 Voltaire Inc. All rights reserved. * Copyright (c) 2007 Xsigo Systems Inc. All rights reserved. * Copyright (c) 2008 Lawrence Livermore National Lab. All rights reserved. + * Copyright (c) 2010 Mellanox Technologies LTD. All rights reserved. * * This software is available to you under a choice of one of two * licenses. You may choose to be licensed under the terms of the GNU @@ -387,7 +388,7 @@ int main(int argc, char **argv) if (!all guid_str) { ibnd_node_t *sw = ibnd_find_node_guid(fabric, guid); - if (sw) + if (sw sw-type == IB_NODE_TYPE_SWITCH) print_switch(sw, NULL); else fprintf(stderr, Failed to find switch: %s\n, @@ -402,7 +403,7 @@ int main(int argc, char **argv) mad_decode_field(ni, IB_NODE_GUID_F, (guid)); sw = ibnd_find_node_guid(fabric, guid); - if (sw) + if (sw sw-type == IB_NODE_TYPE_SWITCH) print_switch(sw, NULL); else fprintf(stderr, Failed to find switch: %s\n, dr_path); -- 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 -- 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][TRIVIAL] opensm: Fix some typos
On 09:46 Mon 27 Sep , Hal Rosenstock wrote: Fix some typos in the opensm man page (man/opensm.8.in) and in the partition config doc (doc/partition-config.txt) Signed-off-by: Hal Rosenstock h...@mellanox.com Applied. Thanks. Sasha -- 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] osmtest/osmt_service.c: In osmt_run_service_records_flow, add missing status
On 09:40 Mon 27 Sep , Hal Rosenstock wrote: When deleting ServiceRecord for service_name[6] fails, error status should be set. Signed-off-by: Hal Rosenstock h...@mellanox.com Applied. Thanks. Sasha -- 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] opensm/osm_ucast_ftree: When roots are not connected, update hop count but not lft
On 09:32 Mon 27 Sep , Hal Rosenstock wrote: When roots are not connected, neither hops nor lfts are updated for root switch port 0s. This causes a problem for multicast (looping) where switch port 0s can join. Solution proposed by Yevgeny is to treat this as updn does and update the hop count but not new_lft. Signed-off-by: Hal Rosenstock h...@mellanox.com Applied. Thanks. Sasha -- 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] opensm/osm_trap_rcv.c: No need to check for sweep for trap 145
On 11:38 Thu 17 Jun , Hal Rosenstock wrote: Trap 145 merely carries the SystemImageGUID (and indication that it changed) so there is no need (to even check) for sweep Signed-off-by: Hal Rosenstock hal.rosenst...@gmail.com Applied. Thanks. Sasha -- 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 V2 5/5] RDMA CM: Netlink Client
+struct rdma_cm_id_stats { + enum rdma_node_type nt; + int port_num; + int bound_dev_if; + __be16 local_port; + __be16 remote_port; + __be32 local_addr[4]; + __be32 remote_addr[4]; Please use sockaddr_storage, so that we can expand the rdma_cm beyond ipv4/6 address support. -- 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] opensm/osm_trap_rcv.c: fix possible core dump
On 17:53 Tue 07 Sep , Yevgeny Kliteynik wrote: Handle the case when source physical port for trap 145 wasn't found. Signed-off-by: Yevgeny Kliteynik klit...@dev.mellanox.co.il Applied. Thanks. Sasha -- 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] opensm/osm_ucast_ftree.c: fix small memory leak in error path
On 18:11 Tue 07 Sep , Yevgeny Kliteynik wrote: Signed-off-by: Yevgeny Kliteynik klit...@dev.mellanox.co.il Applied. Thanks. Sasha -- 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] opensm/osm_ucast_ftree.c: fixing another memory leak at error path
On 18:11 Tue 07 Sep , Yevgeny Kliteynik wrote: Signed-off-by: Yevgeny Kliteynik klit...@dev.mellanox.co.il Applied. Thanks. Sasha -- 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] opensm/osm_ucast_lash.c: small bug in calculating allocated size
On 18:15 Tue 07 Sep , Yevgeny Kliteynik wrote: Signed-off-by: Yevgeny Kliteynik klit...@dev.mellanox.co.il Applied. Thanks. Sasha -- 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] opensm/osm_pkey_mgr.c: fixing small memory leak
On 18:15 Tue 07 Sep , Yevgeny Kliteynik wrote: Signed-off-by: Yevgeny Kliteynik klit...@dev.mellanox.co.il Applied. Thanks. Sasha -- 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] opensm/osm_ucast_file.c: closing file descriptor in error path
On 18:16 Tue 07 Sep , Yevgeny Kliteynik wrote: Signed-off-by: Yevgeny Kliteynik klit...@dev.mellanox.co.il Applied. Thanks. Sasha -- 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] opensm/osm_qos_parser_y.y: fixing bunch of memory leaks on invalid values
On 18:17 Tue 07 Sep , Yevgeny Kliteynik wrote: Signed-off-by: Yevgeny Kliteynik klit...@dev.mellanox.co.il Applied. Thanks. Sasha -- 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 v2] opensm/osm_console.c: fix memory and file descriptor leaks
On 22:30 Tue 07 Sep , Yevgeny Kliteynik wrote: Signed-off-by: Yevgeny Kliteynik klit...@dev.mellanox.co.il Applied. Thanks. Sasha -- 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] opensm/st.c: fix potential core dumps
On 18:18 Tue 07 Sep , Yevgeny Kliteynik wrote: If we're out of polynomials, new_size() will return -1 Deal with this in low-level calls. Signed-off-by: Yevgeny Kliteynik klit...@dev.mellanox.co.il Applied. Thanks. Sasha -- 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 V2] libibmad/fields.c: Change all PortCounter names to match the Specification
On 12:13 Wed 08 Sep , Ira Weiny wrote: From: Ira Weiny wei...@llnl.gov Date: Tue, 3 Aug 2010 10:40:56 -0700 Subject: [PATCH V2] libibmad/fields.c: Change all PortCounter names to match the Specification Change from V1: Add name change to scripts in infiniband-diags Signed-off-by: Ira Weiny wei...@llnl.gov Applied. Thanks. Sasha -- 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 V2 5/5] RDMA CM: Netlink Client
On Tue, Nov 30, 2010 at 02:34:59PM +0200, Or Gerlitz wrote: Jason Gunthorpe wrote: struct ib_nl_qp // String names for IB devices was a mistake, don't perpetuate it. __u32 ib_dev_if; Do you have a concrete suggestion and/or sketch for a patch someone can work on to make this enumeration to take place? Oh, it is really easy, just add an ifindex member to struct ib_device. alloc_name can be extended to compute an appropriate index, see how this works in dev_new_index. Add a 'ifindex' to the ib_device's sysfs directory that exports this number. In future we'd want to see a netlink query to get the RDMA device list to replace trundling through sysfs. I think this is really important from a netlink perspective, there are going to be so many places you wan't to refer to an RDMA device. 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 V2 1/5] IB Netlink Infrastructure
On Tue, Nov 30, 2010 at 02:56:41PM +0200, Nir Muchtar wrote: Do you mean adding this to ib_core? I'm not sure we want to autoload ib_core whenever the userspace asks for socket(PF_NETLINK, SOCK_RAW, NETLINK_INFINIBAND) What's wrong with receiving a protocol not supported error? If ib_core is not loaded then there's no data anyway. Am I missing other possible effects of MODULE_ALIAS here? It seems to be kernel policy to annotate this sort of autoload meta-data. If the module is demand loaded or not is a userspace choice. Apps using this should handle both cases. 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] replace (long*)(long) casting with transportable data type (uintptr_t)
On 14:12 Thu 09 Sep , Stan C. Smith wrote: Hello, In osm_vendor_ibumad_sa.c the casting of a pointer to a long causes data truncation problems in 64-bit Windows where sizeof(long) != sizeof(void*). 'uintptr_t' is correct in both operating environments. Wouldn't it be better to remove those additional castings at all? Like below? thank you, stan. signed-off-by: stan smith stan.sm...@intel.com diff --git a/opensm/libvendor/osm_vendor_ibumad_sa.c b/opensm/libvendor/osm_vendor_ibumad_sa.c index 1fdcc47..9180972 100644 --- a/opensm/libvendor/osm_vendor_ibumad_sa.c +++ b/opensm/libvendor/osm_vendor_ibumad_sa.c @@ -85,7 +85,7 @@ __osmv_sa_mad_rcv_cb(IN osm_madw_t * p_madw, /* obtain the sent context since we store it during send in the ni_ctx */ p_query_req_copy = - (osmv_query_req_t *) (long *)(long)(p_req_madw-context.ni_context. + (osmv_query_req_t *) (uintptr_t)(p_req_madw-context.ni_context. node_guid); p_query_req_copy = (osmv_query_req_t *) p_req_madw-context.ni_context.node_guid; /* provide the context of the original request in the result */ @@ -181,7 +181,7 @@ static void __osmv_sa_mad_err_cb(IN void *bind_context, IN osm_madw_t * p_madw) /* Obtain the sent context etc */ p_query_req_copy = - (osmv_query_req_t *) (long *)(long)(p_madw-context.ni_context. + (osmv_query_req_t *) (uintptr_t)(p_madw-context.ni_context. node_guid); p_query_req_copy = (osmv_query_req_t *) p_madw-context.ni_context.node_guid; /* provide the context of the original request in the result */ @@ -433,7 +433,7 @@ __osmv_send_sa_req(IN osmv_sa_bind_info_t * p_bind, } *p_query_req_copy = *p_query_req; p_madw-context.ni_context.node_guid = - (ib_net64_t) (long)p_query_req_copy; + (ib_net64_t) (uintptr_t)p_query_req_copy; p_madw-context.ni_context.node_guid = (ib_net64_t) p_query_req_copy; ? Sasha /* we can support async as well as sync calls */ sync = ((p_query_req-flags OSM_SA_FLAGS_SYNC) == OSM_SA_FLAGS_SYNC); -- 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 V2 5/5] RDMA CM: Netlink Client
On Tue, Nov 30, 2010 at 04:09:31PM +0200, Nir Muchtar wrote: struct ib_nl_qp { // String names for IB devices was a mistake, don't perpetuate it. I don't know of an IB device index mapping like the one in netdevice. Am I missing one? Do you mean we should create one? Yes, definately. It is very easy to do and goes hand-in-hand with the typical netlink protocol design. __u32 ib_dev_if; __u32 qp_num; __s32 creator_pid; // -1 for kernel consumer }; struct ib_nl_qp_cm_info { u32 cm_state; // enum ib_cm_state u32 lap_state; }; struct ib_nl_rdma_cm_info { __u32 bound_dev_if; __u32 family; __u32 cm_state; // enum rdma_cm_state }; This captures more information and doesn't tie things to RDMA_CM. My problem with making everything QP related is that not everything necessarily is. For example, when creating rdma cm id's they are still not bound to QP's. I guess you could send all zeros in this case, but as more and more of such exceptions are needed this framework will become a bit unnatural. The current implementation is not tying anything to RDMA CM. It allows other modules to export data exactly the way they need. Well, I was outlining how I think the QP-centric information can be returned. You are right that we also have non-QP info, like listening objects, and I think that can be best returned with a seperate query. Trying to conflate them seems like it would be trouble. Certainly, as I've described IBNL_QP messages should only refer to active QPs. Remember you can have as many queries as you like, this is just the QP object query. I guess an alternative would be to have many tables: RDMA_CM, QP, and IB_CM and then rely on userspace to 'join' them by ifindex+QPN - but that seems like alot of work in userspace and I think pretty much everyone is going to want to have the joined data. +static int cma_get_stats(struct sk_buff **nl_skb, int pid) You really have to use netlink_dump_start here, doing it like this will deadlock. See how other places use NLM_F_DUMP as well. Well, I already reviewed netlink_dump_start, and this is how it works as much as I can see (Please correct me if I'm wrong): 1. Allocates an skb 2. Calls its supplied dump cb 3. Calls its supplied done cb if if applicable 4. Appends NLMSG_DONE No, this isn't quite right. The dumpcb is also called after userspace calls recvmsg(), which continues the dump once the buffer is freed. The idea is to return a bit of the table on every dump call back. The way it is used is: 1. Userspace does send() 2. Kernel calls netlink_dump_start() 3. netlink_dump_start calls callback which returns non-zero 4. send() returns in userspace 5. Userspace does recv() 6. Kernel copies the data from #3 into userspace 7. netlink_dump calls callback which returns non-zero 8. recv() returns in userspace [...] Thanks again for all your input. Thanks for working on this! 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] replace (long*)(long) casting with transportable data type (uintptr_t)
Wouldn't it be better to remove those additional castings at all? Like below? .. p_query_req_copy = (osmv_query_req_t *) p_req_madw-context.ni_context.node_guid; I think the Windows compiler will complain about data loss on a 32-bit system. -- 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] replace (long*)(long) casting with transportable data type (uintptr_t)
Sasha Khapyorsky wrote: On 14:12 Thu 09 Sep , Stan C. Smith wrote: Hello, In osm_vendor_ibumad_sa.c the casting of a pointer to a long causes data truncation problems in 64-bit Windows where sizeof(long) != sizeof(void*). 'uintptr_t' is correct in both operating environments. Wouldn't it be better to remove those additional castings at all? Like below? Hello, I assume the original casts were there for a reason hence the cumbersome double cast? Mellanox had reported a truncation problem which was fixed with the (uintptr_t) replacement of (long*)(long). It certainly make more sense to use the single cast if it works; no truncation. I'll check with those who discovered the truncation and see if they tried the obvious single cast approach. stan. thank you, stan. signed-off-by: stan smith stan.sm...@intel.com diff --git a/opensm/libvendor/osm_vendor_ibumad_sa.c b/opensm/libvendor/osm_vendor_ibumad_sa.c index 1fdcc47..9180972 100644 --- a/opensm/libvendor/osm_vendor_ibumad_sa.c +++ b/opensm/libvendor/osm_vendor_ibumad_sa.c @@ -85,7 +85,7 @@ __osmv_sa_mad_rcv_cb(IN osm_madw_t * p_madw, /* obtain the sent context since we store it during send in the ni_ctx */p_query_req_copy = -(osmv_query_req_t *) (long *)(long)(p_req_madw-context.ni_context. + (osmv_query_req_t *) (uintptr_t)(p_req_madw-context.ni_context. node_guid); p_query_req_copy = (osmv_query_req_t *) p_req_madw-context.ni_context.node_guid; /* provide the context of the original request in the result */ @@ -181,7 +181,7 @@ static void __osmv_sa_mad_err_cb(IN void *bind_context, IN osm_madw_t * p_madw) /* Obtain the sent context etc */ p_query_req_copy = -(osmv_query_req_t *) (long *)(long)(p_madw-context.ni_context. +(osmv_query_req_t *) (uintptr_t)(p_madw-context.ni_context. node_guid); p_query_req_copy = (osmv_query_req_t *) p_madw-context.ni_context.node_guid; /* provide the context of the original request in the result */ @@ -433,7 +433,7 @@ __osmv_send_sa_req(IN osmv_sa_bind_info_t * p_bind, } *p_query_req_copy = *p_query_req; p_madw-context.ni_context.node_guid = -(ib_net64_t) (long)p_query_req_copy; +(ib_net64_t) (uintptr_t)p_query_req_copy; p_madw-context.ni_context.node_guid = (ib_net64_t) p_query_req_copy; ? Sasha /* we can support async as well as sync calls */ sync = ((p_query_req-flags OSM_SA_FLAGS_SYNC) == OSM_SA_FLAGS_SYNC); -- 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 V2 5/5] RDMA CM: Netlink Client
On Tue, Nov 30, 2010 at 08:13:45AM -0800, Hefty, Sean wrote: +struct rdma_cm_id_stats { + enum rdma_node_type nt; + int port_num; + int bound_dev_if; + __be16 local_port; + __be16 remote_port; + __be32 local_addr[4]; + __be32 remote_addr[4]; Please use sockaddr_storage, so that we can expand the rdma_cm beyond ipv4/6 address support. Actually for netlink the proper thing to do here is to place the sockaddr in a sub-attribute that can be variable-sized. Do not put variably sized data into a struct like this. Further, sockaddr_storage is not actually ABI guaranteed to be constant in size, it is just the current largest sockaddr possible. This is why the sockaddr size in/out parameter associated with all sockaddrs is so important. Don't put sockaddrs in fixed-size structures for user/kernel interfaces. 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] replace (long*)(long) casting with transportable data type (uintptr_t)
Hefty, Sean wrote: Wouldn't it be better to remove those additional castings at all? Like below? .. p_query_req_copy = (osmv_query_req_t *) p_req_madw-context.ni_context.node_guid; I think the Windows compiler will complain about data loss on a 32-bit system. Only the case @ line 433 compiler wise, requires the (uintptr_t) to eliminate the x86 compiler warning: conversion from ptr to ib_net64_t is sign-extended which may result in unexpected runtime behaviors. Otherwise, x64 x86 compilers digest the single cast approach. The real question becomes, are the runtime truncations observed by Mellanox still present with the single cast? @@ -433,7 +433,7 @@ __osmv_send_sa_req(IN osmv_sa_bind_info_t * p_bind, } *p_query_req_copy = *p_query_req; p_madw-context.ni_context.node_guid = - (ib_net64_t) (long)p_query_req_copy; + (ib_net64_t) (uintptr_t)p_query_req_copy; p_madw-context.ni_context.node_guid = (ib_net64_t) p_query_req_copy; -- 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: Problem Pinning Physical Memory
On 11/30/10 9:24 AM, Alan Cook wrote: Tom Tuckert...@... writes: Yes. I removed the new verb and followed Jason's recommendation of adding this support to the core reg_mr support. I used the type bits in the vma struct to determine the type of memory being registered and just did the right thing. I'll repost in the the next day or two. Tom Tom, Couple of questions: I noticed that OFED 1.5.3 was released last week. Are the changes you speak of part of that release? No. If not, is there an alternate branch/project that I should be looking at or into to for the mentioned changes? The patch will be against the top-of-tree Linux kernel. Also, I am inferring that the changes allowing the registering of physical memory will only happen if my application is running in kernel space. Actually, no. Is this correct? or will I be able to register the physical memory from user space now as well? What I implemented was support for mmap'd memory. In practical terms for your application you would write a driver that supported the mmap file op. The driver's mmap routine would ioremap the pci memory of interest and stuff it in the provided vma. The user-mode app then ibv_reg_mr the address and length returned by mmap. Make sense? Tom -- 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 -- 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