Re: [PATCH] mlx4: properly mask MGM entry members count

2010-11-30 Thread Aleksey Senin
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

2010-11-30 Thread Or Gerlitz
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

2010-11-30 Thread Nir Muchtar
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Hefty, Sean
 +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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Jason Gunthorpe
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

2010-11-30 Thread Jason Gunthorpe
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)

2010-11-30 Thread Sasha Khapyorsky
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

2010-11-30 Thread Jason Gunthorpe
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)

2010-11-30 Thread Hefty, Sean
 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)

2010-11-30 Thread Smith, Stan
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

2010-11-30 Thread Jason Gunthorpe
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)

2010-11-30 Thread Smith, Stan
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

2010-11-30 Thread Tom Tucker

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