RE: [PATCH 1/1] ibacm: Add support for pathrecord query through netlink

2015-05-19 Thread Wan, Kaike
 
  +static inline int acm_nl_supported_sa_mad(struct ib_sa_mad *sa_mad) {
  +   return (sa_mad-rmpp_version == 0 
 
 I think you want something like:
 
 !(rmpp_flags  RMPP_ACTIVE)
 
 Instead of checking for version = 0.


Will be changed to:

!(sa_mad-rmpp_flags  UMAD_RMPP_FLAG_ACTIVE)

in next version
 
  +   sa_mad-method == UMAD_METHOD_GET 
  +   sa_mad-attr_id == htons(UMAD_SA_ATTR_PATH_REC)); }

--
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 v8 0/6] Generic logging helpers

2015-05-19 Thread Doug Ledford
On Tue, 2015-05-19 at 12:48 +0300, Sagi Grimberg wrote:
 This small set adds some meaningful verbosity to some
 of the core/rdma_cm enumerated events. Its useful to
 see the meaning of the opcodes instead of revisiting the
 code for every new status/event that left the cache in
 our brain.
 
 Many thanks to all the reviewers!

Hi Sagi,

I grabbed v7 already, but I manually added Sean's typo catch to it when
I picked it up.  So it's the same as your v8.

 Changes from v7:
 - Fixed typo for IB_WC_REM_INV_RD_REQ_ERR wc status
 
 Changes from v6:
 - Lowered case for unrecognised event/status print
 - Fixed typo in IB_WC_REM_INV_REQ_ERR string
 
 Changes from v5:
 - Changed message strings to match the xprtrdma style
 - checkpatch nit
 
 Changes from v4:
 - Split up ULP changes to separate patches
 
 Changes from v3:
 - Protect against holes in string arrays
 - Restored __attribute_const__ to functions prototype
 - Made string arrays rodata
 
 Changes from v2:
 - Cast to size_t instead of unsigned
 - Style fix in string arrays declaration
 - Removed redundant __attribute_const__
 
 Changes from v1:
 - Changed helper names ib_wc_status_msg and rdma_event_msg
 - Cast input arguments to protect string buffer access
 - Add svcrdma to the party
 
 Changes from v0 (RFC):
 - Moved string arrays to .c files
 - Changed string helpers from macros to exported functions
 - Aligned rds to generic helpers as well
 
 Sagi Grimberg (6):
   IB/core, cma: Nice log-friendly string helpers
   IB/srp: Align to generic logging helpers
   IB/iser: Align to generic logging helpers
   iser-target: Align to generic logging helpers
   xprtrdma, svcrdma: Switch to generic logging helpers
   RDS: Switch to generic logging helpers
 
  drivers/infiniband/core/cma.c|   28 +
  drivers/infiniband/core/verbs.c  |   65 +
  drivers/infiniband/ulp/iser/iser_verbs.c |   28 ++---
  drivers/infiniband/ulp/isert/ib_isert.c  |   19 --
  drivers/infiniband/ulp/srp/ib_srp.c  |   16 +++--
  include/rdma/ib_verbs.h  |4 +
  include/rdma/rdma_cm.h   |2 +
  net/rds/af_rds.c |9 ---
  net/rds/ib.h |1 -
  net/rds/ib_cm.c  |   36 +---
  net/rds/ib_recv.c|4 +-
  net/rds/ib_send.c|   38 +
  net/rds/rdma_transport.c |   34 +--
  net/rds/rds.h|1 -
  net/sunrpc/xprtrdma/frwr_ops.c   |4 +-
  net/sunrpc/xprtrdma/svc_rdma_transport.c |   29 ++
  net/sunrpc/xprtrdma/verbs.c  |   90 
 ++
  17 files changed, 174 insertions(+), 234 deletions(-)
 
 --
 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


-- 
Doug Ledford dledf...@redhat.com
  GPG KeyID: 0E572FDD



signature.asc
Description: This is a digitally signed message part


RE: [PATCH][RESEND] IB/cma: Resolve AF_IB UD support

2015-05-19 Thread Hefty, Sean
 Resent due to mailer issues

Next time, please put this comment under the '---', so that it doesn't end up 
in the patch description.

 Support for using UD and AF_IB is currently broken.  The
 IB_CM_SIDR_REQ_RECEIVED
 message is not handled properly in cma_save_net_info() and we end up
 falling
 into code that will try and process the request as ipv4/ipv6, which will
 end
 up failing.
 
 The resolution is to add a check for the SIDR_REQ and call
 cma_save_ib_info()
 with a NULL path record.  Change cma_save_ib_info() to copy the src sib
 info
 from the listen_id when the path record is NULL.
 
 Reported-by: Hari Shankar hari.shan...@netapp.com
 Signed-off-by: Matt Finlay m...@mellanox.com

Acked-by: Sean Hefty sean.he...@intel.com
--
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 v4 for-next 04/12] IB/ipoib: Return IPoIB devices matching connection parameters

2015-05-19 Thread Jason Gunthorpe
On Sun, May 17, 2015 at 08:51:00AM +0300, Haggai Eran wrote:

 +#if IS_ENABLED(CONFIG_IPV6)
 + struct sockaddr_in6 *addr_in6 = (struct sockaddr_in6 *)addr;
 +#endif
 + __be32 ret_addr;
 +
 + switch (addr-sa_family) {
 + case AF_INET:
 + in_dev = in_dev_get(dev);
 + if (!in_dev)
 + return false;
 +
 + ret_addr = inet_confirm_addr(net, in_dev, 0,
 +  addr_in-sin_addr.s_addr,
 +  RT_SCOPE_HOST);
 + in_dev_put(in_dev);
 + if (ret_addr)
 + return true;
 +
 + break;
 +#if IS_ENABLED(CONFIG_IPV6)
 + case AF_INET6:
 + if (ipv6_chk_addr(net, addr_in6-sin6_addr, dev, 1))
 + return true;
 +
 + break;
 +#endif

Can you use

  if (IS_ENABLED(CONFIG_IPV6))

At the call site instead of the #if guards?

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 3/3] IB/sa: route SA pathrecord query through netlink

2015-05-19 Thread Hefty, Sean
  N?r??yb?X??ǧv?^?)޺{.n?+{??ٚ?{ay?ʇڙ?,j
??f???h???z??w???
 
 ???j:+v???w?j?m
zZ+?ݢj??!
 
 Does anyone else ever wonder what this garbage is on some of the
 messages from @intel folks?

I believe it's some weird formatting of:

 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

I have no idea why we get it.  I send/receive in plain text. 


Re: rdma kernel tree

2015-05-19 Thread Yann Droneaud
Hi,

Le mardi 19 mai 2015 à 15:49 -0400, Doug Ledford a écrit :
 On Tue, 2015-05-19 at 21:47 +0300, Or Gerlitz wrote:
  On Tue, May 19, 2015 at 3:22 PM, Doug Ledford dledf...@redhat.com wrote:
   On Tue, 2015-05-19 at 07:35 +0300, Or Gerlitz wrote:
I pulled that via a bundle from patchworks.  I'll double check it.
Did  you check it out? fixed it out?
  
   I took a look now, you've rebased to-be-rebased/for-4.2 to 4.1-rc4 and
  
   Correct.
  
   it seems this is what you are going to push into the kernel.org treem
  
   Correct.
  
   but this series is still there with the zillion tested/reviewed/etc
   signature per one 2-3 patch, I think we've agreed this needs to be
   addressed prior to the upstream push, right?
  
   Incorrect.  What you objected to before was the large Cc: list in the
   patches.  That is gone.  What is there now is just the reviewed-by: list
   of three people, and the tested-by list of two people.  As the entire
   patch set as a whole was reviewed and tested by those people, it seems
   accurate to me.
  
  Doug, I have never ever seen a patch set (specifically the 15~23 part
  of it) with that level of simplicity
 
 That portion of the patchset didn't start out with that level of
 simplicity per patch, it evolved to that because it made review
 *significantly* easier.  It's very simple to review a patch that does:
 
 Add 1 helper
 Replace tests in code with just that 1 helper
 
 because you can scroll through that patch and know that every line being
 replaced is related to that one helper.  If you want to know every line
 that was replaced with rdma_cap_iw_cm, you go to that one patch and it's
 all listed very easy to read.
 
 On the other hand, when you squash all those patches together, review
 becomes much harder because if you want to see what a single helper
 does, you have to sift through all of the other helper changes and hope
 you find the right ones, and that you found all of the right ones.
 
   and
  signature/reviewers/tested-by/etc inflation.
 
 I added those myself as part of an automated addition.  It applied to
 the entire series, so it was put on each patch.  The people that
 tested/reviewed the series did not do so to individual patches, they hit
 all of them.  And as was pointed out a couple of weeks ago on an earlier
 patchset I picked up, it is generally good behavior to give attribution
 where it is due to encourage people to participate.
 

I agree with all the above replies from Doug:

- small patches are easier to read and review: I prefer reviewing 10
littles patches than one big patch.

- Signed-off-by:, Reviewed-by:, Tested-by: and Cc: are required too
so that when it come to modifying existing code, we can contact people
previously involved for reviews, tests, advice, etc.

(I usually add Link: with a reference to the e-mail so that one commit
 can be traced back to the related patch submission and discussions  
 (cover-letter !), and, to be able to trace back all patch iterations,
 I add a link to the previous patchset submission, and so on).

Regards.

-- 
Yann Droneaud
OPTEYA


--
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 3/3] IB/sa: route SA pathrecord query through netlink

2015-05-19 Thread Hefty, Sean
 I'm not sure there is much use case for letting user space get
 involved in arbitary MAD traffic..

I was thinking more about easily supporting other queries - ServiceRecords, 
MCMemberRecords, etc.  The only use case I'm currently aware of is PathRecords, 
however.

 Over generalizing feels like over design and doesn't let us add
 valuable information that could help push policy decisions into user
 space, like passing the IP and TOS, QP type, etc when available, to
 userspace.

I agree.  This level of control would be better.  The current security model of 
IB seems hopelessly broken.  IMO, all path data should come from privileged 
users, without any ability of the app to modify it.

Changing the protocol shouldn't be a big deal, though if you wanted to make my 
life easy, we could just adopt the ibacm sockets protocol directly.  :)

- Sean
--
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 v4 for-next 04/14] IB/core: Add default GID for RoCE GID table

2015-05-19 Thread Jason Gunthorpe
On Tue, May 19, 2015 at 05:27:07PM +0300, Matan Barak wrote:
 When RoCE is used, a default GID address should be generated
 for every supported RoCE type. These default GID addresses are
 generated based on the IPv6 link-local address, but in contrast
 to the GID based on the regular IPv6 link-local (as we generate
 GID per IP address), these GIDs are also available if the net
 device is down (in order to support loopback).
 Moreover, these default GID addresses can't be deleted.

That addrconf stuff needs to be in a dedicated patch

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


[PATCH v2 0/4] Sending kernel pathrecord query to user cache server

2015-05-19 Thread kaike . wan
From: Kaike Wan kaike@intel.com

A SA cache is undeniably critical for fabric scalability and performance.
In user space, the ibacm application provides a good example of pathrecord
cache for address and route resolution. With the recent implementation of
the provider architecture, ibacm offers more extensibility as a SA cache.
In kernel, ipoib implements its own small cache for pathrecords, which is
however not available for general use. Furthermore, the implementation of
a SA cache in user space offers better flexibility, larger capacity, and
more robustness for the system.

In this patch series, a mechanism is implemented to allow ib_sa to
send pathrecord query to a user application (eg ibacm) through netlink.
Potentially, this mechanism could be easily extended to other SA queries.

With a customized test implemented in rdma_cm module (not included in this
series), it was shown that the time to retrieve 1 million pathrecords
dropped from 46660 jiffies (46.66 seconds) to 16119 jiffies (or 16.119
seconds) on a two-node system, a reduction of more than 60%.

Changes since v1:
- Move kzalloc changes into a separate patch (Patch 3).
- Remove redundant include line (Patch 4). 
- Rename struct rdma_nl_resp_msg as structure ib_nl_resp_msg (Patch 4).

Kaike Wan (4):
  IB/netlink: Add defines for MAD requests through netlink
  IB/core: Check the presence of netlink multicast group listeners
  IB/sa: Allocate SA query with kzalloc
  IB/sa: Route SA pathrecord query through netlink

 drivers/infiniband/core/netlink.c  |8 +
 drivers/infiniband/core/sa_query.c |  333 +++-
 include/rdma/rdma_netlink.h|7 +
 include/uapi/rdma/rdma_netlink.h   |7 +
 4 files changed, 350 insertions(+), 5 deletions(-)

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