[PATCH rdma-next 00/31] RDMA fixes and refactoring for 4.15

[PATCH rdma-next 00/31] RDMA fixes and refactoring for 4.15

[Leon Romanovsky]

Hi Doug,

Please consider to take the following series to your coming pull request.

The patches are available in the git repository at:
  git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git tags/rdma-next-2017-11-14

	Thanks
---------------------------------------

Artemy Kovalyov (1):
  IB/umem: Fix use of npages/nmap fields

Daniel Jurgens (1):
  IB/cm: Add debug prints to ib_cm

Erez Alfasi (1):
  IB/mlx4: Remove unused ibpd parameter

Erez Shitrit (2):
  IB/ipoib: Avoid memory leak if neigh destination was changed
  IB/ipoib: Update pathrec field if not valid record

Majd Dibbiny (1):
  IB/core: Make sure that PSN does not overflow

Matan Barak (1):
  IB/core: Fix memory leak in cm_req_handler error flows

Parav Pandit (24):
  IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled
  IB/{core/cm}: Fix dmac query for IPv6 link local destination
  IB/core: Depend on IPv6 stack to resolve link local address
  IB/core: Avoid exporting module internal function
  RDMA/cma: Fix consider size of destination address
  RDMA/cma: Set default GID type as RoCE when resolving RoCE route
  RDMA/cma: Simplify netdev check
  RDMA/cma: Avoid setting path record type twice
  RDMA/cma: Introduce and use helper functions to init work
  IB/core: Avoid unnecessary type cast
  IB/core: Refactor to avoid unnecessary check on GID lookup miss
  IB/rxe: Avoid passing unused index pointer which is optional
  IB/core: Avoid exporting module internal ib_find_gid_by_filter()
  RDMA/core: Avoid redundant memcpy in resolving address
  IB/core: Removed unused function
  RDMA/{core, cma}: Simplify rdma_translate_ip
  RDMA/core: Avoid copying ifindex twice
  IB/{core, ipoib}: Simplify ib_find_gid to search only for IB link
    layer
  IB/{cm, umad}: Fix honor av init error
  IB/cm: Fix honor address handle attribute init error
  IB/cm: Fix avoid sleep while spin lock is held
  IB/{core, cm, cma, ipoib}: Rename ib_init_ah_from_path to
    ib_init_ah_attr_from_path
  IB/{core, umad, cm}: Rename ib_init_ah_from_wc to
    ib_init_ah_attr_from_wc
  IB/cm: Refactor to avoid setting path record software only fields

 drivers/infiniband/core/addr.c            |  65 ++--------
 drivers/infiniband/core/cache.c           |  25 ++--
 drivers/infiniband/core/cm.c              | 191 ++++++++++++++++++++----------
 drivers/infiniband/core/cma.c             | 118 +++++++++---------
 drivers/infiniband/core/core_priv.h       |  13 --
 drivers/infiniband/core/device.c          |  16 +--
 drivers/infiniband/core/sa_query.c        |   8 +-
 drivers/infiniband/core/security.c        |  10 +-
 drivers/infiniband/core/umem.c            |   2 +-
 drivers/infiniband/core/user_mad.c        |  10 +-
 drivers/infiniband/core/verbs.c           | 183 ++++++++++++++--------------
 drivers/infiniband/hw/mlx4/qp.c           |   4 +-
 drivers/infiniband/sw/rxe/rxe_recv.c      |   3 +-
 drivers/infiniband/ulp/ipoib/ipoib_ib.c   |   3 +-
 drivers/infiniband/ulp/ipoib/ipoib_main.c |  62 +++++++---
 include/rdma/ib_addr.h                    |   5 +-
 include/rdma/ib_sa.h                      |  10 +-
 include/rdma/ib_verbs.h                   |  11 +-
 18 files changed, 379 insertions(+), 360 deletions(-)

--
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 01/31] IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Currently two RoCE default GIDs are added whenever HCA supports RoCEv1
and RoCEv2. Default GIDs are created using unique GUID and well known
prefix as defined by the IB spec 1.3 section 4.1.1 and 3.5.10.

For few HCAs GUID is derived from the MAC address of the netdev. Due to
this scheme, default RoCEv2 GID resemebles link-local IPv6 based GID.
Even when IPv6 is disabled at compile time in kernel, default RoCEv2 GID
can get added. When such RoCEv2 GID is used, offload HCA starts using
IPv6 packets for QP.
This leads to sending IPv6 packets in network from a host which has IPv6
disabled kernel.

This can be seen with below simple test.
ib_send_bw -x 1 -d mlx5_1
ib_send_bw -x 1 -d mlx5_1 <ip_v4_addr>

It may not be a good idea to send/receive IPv6 packets on kernel which
has IPv6 disabled.

Therefore lets honor networking stack's IPv6 disabled setting.
This patch avoids adding RoCEv2 based default GID when IPv6 is
disabled in kernel.

ibv_devinfo -v output can be seen as below on IPv6 disabled kernel as
below with one IPv4 address assign to netdevice.

GID[  0]: fe80:0000:0000:0000:268a:07ff:fe55:4661
GID[  2]: 0000:0000:0000:0000:0000:ffff:c3a8:0124
GID[  3]: 0000:0000:0000:0000:0000:ffff:c3a8:0124

This also ensures that any RoCE specific legacy application who has
assumed GID indexes for IP addresses start from 2, still continue to
have such IP based GIDs assigned from start index of 2.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cache.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
index 77515638c55c..e5bba898741d 100644
--- a/drivers/infiniband/core/cache.c
+++ b/drivers/infiniband/core/cache.c
@@ -670,6 +670,15 @@ void ib_cache_gid_set_default_gid(struct ib_device *ib_dev, u8 port,
 	memset(&gid_attr, 0, sizeof(gid_attr));
 	gid_attr.ndev = ndev;
 
+	/* Default GID is created using unique GUID and local subnet prefix,
+	 * as described in section 4.1.1 and 3.5.10 in IB spec 1.3.
+	 * Therefore don't create RoCEv2 default GID based on it that
+	 * resembles as IPv6 GID based on link local address when IPv6 is
+	 * disabled in kernel.
+	 */
+	if (!IS_ENABLED(CONFIG_IPV6))
+		gid_type_mask &= ~BIT(IB_GID_TYPE_ROCE_UDP_ENCAP);
+
 	for (gid_type = 0; gid_type < IB_GID_TYPE_SIZE; ++gid_type) {
 		int ix;
 		union ib_gid current_gid;
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 01/31] IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled

[Or Gerlitz]

On Tue, Nov 14, 2017 at 2:51 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Did you want to say "IB/core: Do not add RoCEv2 default GID when IPv6
is disabled" (i.e remove the word "Fix" and Cap the "do")
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 01/31] IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled

[Parav Pandit]

> -----Original Message-----
> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> Sent: Tuesday, November 14, 2017 3:25 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: Doug Ledford <dledford@redhat.com>; linux-rdma@vger.kernel.org; Leon
> Romanovsky <leonro@mellanox.com>
> Subject: Re: [PATCH rdma-next 01/31] IB/core: Fix do not add RoCEv2 default
> GID when IPv6 is disabled
> 
> On Tue, Nov 14, 2017 at 2:51 PM, Leon Romanovsky <leon@kernel.org> wrote:
> > From: Parav Pandit <parav@mellanox.com>
> 
> Did you want to say "IB/core: Do not add RoCEv2 default GID when IPv6 is
> disabled" (i.e remove the word "Fix" and Cap the "do")
Sounds good. Either we can do v1 or Doug can change while merging depending on comments on other patches.

Re: [PATCH rdma-next 01/31] IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled

[Jason Gunthorpe]

On Tue, Nov 14, 2017 at 02:51:48PM +0200, Leon Romanovsky wrote:

> +	/* Default GID is created using unique GUID and local subnet prefix,
> +	 * as described in section 4.1.1 and 3.5.10 in IB spec 1.3.
> +	 * Therefore don't create RoCEv2 default GID based on it that
> +	 * resembles as IPv6 GID based on link local address when IPv6 is
> +	 * disabled in kernel.
> +	 */
> +	if (!IS_ENABLED(CONFIG_IPV6))
> +		gid_type_mask &= ~BIT(IB_GID_TYPE_ROCE_UDP_ENCAP);

There is also a sysctl to globally disable ipv6, I would think it
would be better to check than then a CONFIG symbol?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 01/31] IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled

[Parav Pandit]

Hi Jason,

> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgg-uk2M96/98Pc@public.gmane.org]
> Sent: Tuesday, November 14, 2017 6:18 PM
> To: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Parav
> Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Subject: Re: [PATCH rdma-next 01/31] IB/core: Fix do not add RoCEv2 default
> GID when IPv6 is disabled
> 
> On Tue, Nov 14, 2017 at 02:51:48PM +0200, Leon Romanovsky wrote:
> 
> > +	/* Default GID is created using unique GUID and local subnet prefix,
> > +	 * as described in section 4.1.1 and 3.5.10 in IB spec 1.3.
> > +	 * Therefore don't create RoCEv2 default GID based on it that
> > +	 * resembles as IPv6 GID based on link local address when IPv6 is
> > +	 * disabled in kernel.
> > +	 */
> > +	if (!IS_ENABLED(CONFIG_IPV6))
> > +		gid_type_mask &= ~BIT(IB_GID_TYPE_ROCE_UDP_ENCAP);
> 
> There is also a sysctl to globally disable ipv6, I would think it would be better to
> check than then a CONFIG symbol?
> 
Currently in rest of the IB stack in cma.c, addr.c we check for IS_ENABLED(CONFIG_IPV6).
May be when all of them are changed, this can also shift to honoring sysctl.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 01/31] IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled

[Jason Gunthorpe]

On Wed, Nov 15, 2017 at 03:59:26AM +0000, Parav Pandit wrote:

> Currently in rest of the IB stack in cma.c, addr.c we check for
> IS_ENABLED(CONFIG_IPV6).  May be when all of them are changed, this
> can also shift to honoring sysctl.

I think most of the rest of the stack is guarding things that would
cause linking problems when ipv6 is compiled out, those cannot be
changed to sysctl.

Since this clearly should be sysctl based it should start out that
way.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 01/31] IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled

[Parav Pandit]

> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgg-uk2M96/98Pc@public.gmane.org]
> Sent: Tuesday, November 14, 2017 10:41 PM
> To: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>; Doug Ledford
> <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH rdma-next 01/31] IB/core: Fix do not add RoCEv2 default
> GID when IPv6 is disabled
> 
> On Wed, Nov 15, 2017 at 03:59:26AM +0000, Parav Pandit wrote:
> 
> > Currently in rest of the IB stack in cma.c, addr.c we check for
> > IS_ENABLED(CONFIG_IPV6).  May be when all of them are changed, this
> > can also shift to honoring sysctl.
> 
> I think most of the rest of the stack is guarding things that would cause linking
> problems when ipv6 is compiled out, those cannot be changed to sysctl.
> 
> Since this clearly should be sysctl based it should start out that way.
Ok. But I will not be able to provide such fix anytime soon.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next, 01/31] IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled

[Jason Gunthorpe]

On Tue, Nov 14, 2017 at 02:51:48PM +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Currently two RoCE default GIDs are added whenever HCA supports RoCEv1
> and RoCEv2. Default GIDs are created using unique GUID and well known
> prefix as defined by the IB spec 1.3 section 4.1.1 and 3.5.10.
> 
> For few HCAs GUID is derived from the MAC address of the netdev. Due to
> this scheme, default RoCEv2 GID resemebles link-local IPv6 based GID.
> Even when IPv6 is disabled at compile time in kernel, default RoCEv2 GID
> can get added. When such RoCEv2 GID is used, offload HCA starts using
> IPv6 packets for QP.
> This leads to sending IPv6 packets in network from a host which has IPv6
> disabled kernel.
> 
> This can be seen with below simple test.
> ib_send_bw -x 1 -d mlx5_1
> ib_send_bw -x 1 -d mlx5_1 <ip_v4_addr>
> 
> It may not be a good idea to send/receive IPv6 packets on kernel which
> has IPv6 disabled.
> 
> Therefore lets honor networking stack's IPv6 disabled setting.
> This patch avoids adding RoCEv2 based default GID when IPv6 is
> disabled in kernel.
> 
> ibv_devinfo -v output can be seen as below on IPv6 disabled kernel as
> below with one IPv4 address assign to netdevice.
> 
> GID[  0]: fe80:0000:0000:0000:268a:07ff:fe55:4661
> GID[  2]: 0000:0000:0000:0000:0000:ffff:c3a8:0124
> GID[  3]: 0000:0000:0000:0000:0000:ffff:c3a8:0124
> 
> This also ensures that any RoCE specific legacy application who has
> assumed GID indexes for IP addresses start from 2, still continue to
> have such IP based GIDs assigned from start index of 2.

I'm not going to take this patch.. The entire concept is wrong.

rocev2 should *NOT* be creating a default IPv6 address, ever. So
turning that off is simply the wrong solution to this problem.

rocev2 is IP based and must exclusively follow the IP address list in
the host.

It must never invent its own IP that is not associated with a netdev.

Creating a rocev1 compatability DGID does makes sense, but that is a
*DGID* and it must never be co-mingled with rocev2 packets.

> Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

BTW, Leon when you send these patches please make sure the Reviewer's
are CC'd as they should see and have a chance to respond to review
comments.. I added Eli manually in this case..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next, 01/31] IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

On Mon, Dec 18, 2017 at 01:48:18PM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 14, 2017 at 02:51:48PM +0200, Leon Romanovsky wrote:
> > Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Reviewed-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> BTW, Leon when you send these patches please make sure the Reviewer's
> are CC'd as they should see and have a chance to respond to review
> comments.. I added Eli manually in this case..

I will try to pay more attention to it.

Thanks

>
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH rdma-next 02/31] IB/{core/cm}: Fix dmac query for IPv6 link local destination

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

ib_init_ah_from_wc() is querying dmac address for IPv6 link local
destination address and for all other address type without any association
with netdevice.
For resolving IPv6 link local address to its mac address, it is must to
provide the link local interface index as required by IPv6 stack.
Currently rdma_cm based RoCE applications fails to establish connection
using IPv6 link local addresses. This problem is described at [1].

Also, for rest of the IPv4 and IPv6 addresses, it is necessary to
consider netdevice from which the MAD packet is received.

Therefore, this patch refers to the GID index from wc. Each RoCE GID is
associated with respective netdevice. Patch uses the netdevice of such GID
entry to ensure that dmac resolve happens using netdevice of
received packet.
RoCE GID table management ensures that right upper netdevices of the
physical netdevices are added. Therefore init_ah_from_wc doesn't need to
perform such check.

Now that route lookup is done based on the netdevice of the GID entry,
simplify code to not have ifindex and vlan pointers.
As part of that, refactor to have netdevice as input parameter.
This is already discussed at [2].

Finally ib_init_ah_from_wc resolves dmac for unicast GID in similar way
as what ib_resolve_eth_dmac() does. So ib_resolve_eth_dmac is refactored
to split for unicast and non unicast GIDs, so that it can be reused by
ib_init_ah_from_wc.
While we are at refactoring ib_resolve_eth_dmac(), it is further
simplified
(a) to avoid hoplimit as optional parameter, as there is only one
user who always queries hoplimit.
(b) for empty line.
(c) avoided zero initialization of ret.
(d) removed as exported symbol as only ib core uses it.

For IPv6, this is tested using simple rping test as below.
rping -sv -a ::0
rping -c -a fe80::268a:7ff:fe55:4661%ens2f1 -C 1 -v -d

[1] https://www.spinics.net/lists/linux-rdma/msg45690.html
[2] https://www.spinics.net/lists/linux-rdma/msg45710.html

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reported-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/addr.c  |  24 ++-----
 drivers/infiniband/core/verbs.c | 136 +++++++++++++++++-----------------------
 include/rdma/ib_addr.h          |   2 +-
 3 files changed, 64 insertions(+), 98 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index f4e8185bccd3..da4469c38eac 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -761,27 +761,23 @@ static void resolve_cb(int status, struct sockaddr *src_addr,
 
 int rdma_addr_find_l2_eth_by_grh(const union ib_gid *sgid,
 				 const union ib_gid *dgid,
-				 u8 *dmac, u16 *vlan_id, int *if_index,
+				 u8 *dmac, const struct net_device *ndev,
 				 int *hoplimit)
 {
-	int ret = 0;
 	struct rdma_dev_addr dev_addr;
 	struct resolve_cb_context ctx;
-	struct net_device *dev;
-
 	union {
 		struct sockaddr     _sockaddr;
 		struct sockaddr_in  _sockaddr_in;
 		struct sockaddr_in6 _sockaddr_in6;
 	} sgid_addr, dgid_addr;
-
+	int ret;
 
 	rdma_gid2ip(&sgid_addr._sockaddr, sgid);
 	rdma_gid2ip(&dgid_addr._sockaddr, dgid);
 
 	memset(&dev_addr, 0, sizeof(dev_addr));
-	if (if_index)
-		dev_addr.bound_dev_if = *if_index;
+	dev_addr.bound_dev_if = ndev->ifindex;
 	dev_addr.net = &init_net;
 
 	ctx.addr = &dev_addr;
@@ -798,19 +794,9 @@ int rdma_addr_find_l2_eth_by_grh(const union ib_gid *sgid,
 		return ret;
 
 	memcpy(dmac, dev_addr.dst_dev_addr, ETH_ALEN);
-	dev = dev_get_by_index(&init_net, dev_addr.bound_dev_if);
-	if (!dev)
-		return -ENODEV;
-	if (if_index)
-		*if_index = dev_addr.bound_dev_if;
-	if (vlan_id)
-		*vlan_id = rdma_vlan_dev_vlan_id(dev);
-	if (hoplimit)
-		*hoplimit = dev_addr.hoplimit;
-	dev_put(dev);
-	return ret;
+	*hoplimit = dev_addr.hoplimit;
+	return 0;
 }
-EXPORT_SYMBOL(rdma_addr_find_l2_eth_by_grh);
 
 int rdma_addr_find_smac_by_sgid(union ib_gid *sgid, u8 *smac, u16 *vlan_id)
 {
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 3fb8fb6cc824..54b56c4fcc38 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -481,6 +481,40 @@ int ib_get_gids_from_rdma_hdr(const union rdma_network_hdr *hdr,
 }
 EXPORT_SYMBOL(ib_get_gids_from_rdma_hdr);
 
+/* Resolve destination mac address and hop limit for unicast destination
+ * GID entry, considering the source GID entry as well.
+ * ah_attribute must have have valid port_num, sgid_index.
+ */
+static int ib_resolve_unicast_gid_dmac(struct ib_device *device,
+				       struct rdma_ah_attr *ah_attr)
+{
+	struct ib_gid_attr sgid_attr;
+	struct ib_global_route *grh;
+	int hop_limit = 0xff;
+	union ib_gid sgid;
+	int ret;
+
+	grh = rdma_ah_retrieve_grh(ah_attr);
+
+	ret = ib_query_gid(device,
+			   rdma_ah_get_port_num(ah_attr),
+			   grh->sgid_index,
+			   &sgid, &sgid_attr);
+	if (ret || !sgid_attr.ndev) {
+		if (!ret)
+			ret = -ENXIO;
+		return ret;
+	}
+
+	ret = rdma_addr_find_l2_eth_by_grh(&sgid, &grh->dgid,
+					   ah_attr->roce.dmac,
+					   sgid_attr.ndev, &hop_limit);
+	dev_put(sgid_attr.ndev);
+
+	grh->hop_limit = hop_limit;
+	return ret;
+}
+
 /*
  * This function creates ah from the incoming packet.
  * Incoming packet has dgid of the receiver node on which this code is
@@ -490,9 +524,6 @@ EXPORT_SYMBOL(ib_get_gids_from_rdma_hdr);
  * as sgid and, sgid is used as dgid because sgid contains destinations
  * GID whom to respond to.
  *
- * This is why when calling rdma_addr_find_l2_eth_by_grh() function, the
- * position of arguments dgid and sgid do not match the order of the
- * parameters.
  */
 int ib_init_ah_from_wc(struct ib_device *device, u8 port_num,
 		       const struct ib_wc *wc, const struct ib_grh *grh,
@@ -523,57 +554,33 @@ int ib_init_ah_from_wc(struct ib_device *device, u8 port_num,
 	if (ret)
 		return ret;
 
+	rdma_ah_set_sl(ah_attr, wc->sl);
+	rdma_ah_set_port_num(ah_attr, port_num);
+
 	if (rdma_protocol_roce(device, port_num)) {
-		int if_index = 0;
 		u16 vlan_id = wc->wc_flags & IB_WC_WITH_VLAN ?
 				wc->vlan_id : 0xffff;
-		struct net_device *idev;
-		struct net_device *resolved_dev;
 
 		if (!(wc->wc_flags & IB_WC_GRH))
 			return -EPROTOTYPE;
 
-		if (!device->get_netdev)
-			return -EOPNOTSUPP;
-
-		idev = device->get_netdev(device, port_num);
-		if (!idev)
-			return -ENODEV;
-
-		ret = rdma_addr_find_l2_eth_by_grh(&dgid, &sgid,
-						   ah_attr->roce.dmac,
-						   wc->wc_flags & IB_WC_WITH_VLAN ?
-						   NULL : &vlan_id,
-						   &if_index, &hoplimit);
-		if (ret) {
-			dev_put(idev);
-			return ret;
-		}
-
-		resolved_dev = dev_get_by_index(&init_net, if_index);
-		rcu_read_lock();
-		if (resolved_dev != idev && !rdma_is_upper_dev_rcu(idev,
-								   resolved_dev))
-			ret = -EHOSTUNREACH;
-		rcu_read_unlock();
-		dev_put(idev);
-		dev_put(resolved_dev);
-		if (ret)
-			return ret;
-
-		ret = get_sgid_index_from_eth(device, port_num, vlan_id,
-					      &dgid, gid_type, &gid_index);
+		ret = get_sgid_index_from_eth(device, port_num,
+					      vlan_id, &dgid,
+					      gid_type, &gid_index);
 		if (ret)
 			return ret;
-	}
 
-	rdma_ah_set_dlid(ah_attr, wc->slid);
-	rdma_ah_set_sl(ah_attr, wc->sl);
-	rdma_ah_set_path_bits(ah_attr, wc->dlid_path_bits);
-	rdma_ah_set_port_num(ah_attr, port_num);
+		flow_class = be32_to_cpu(grh->version_tclass_flow);
+		rdma_ah_set_grh(ah_attr, &sgid,
+				flow_class & 0xFFFFF,
+				(u8)gid_index, hoplimit,
+				(flow_class >> 20) & 0xFF);
+		return ib_resolve_unicast_gid_dmac(device, ah_attr);
+	} else {
+		rdma_ah_set_dlid(ah_attr, wc->slid);
+		rdma_ah_set_path_bits(ah_attr, wc->dlid_path_bits);
 
-	if (wc->wc_flags & IB_WC_GRH) {
-		if (!rdma_cap_eth_ah(device, port_num)) {
+		if (wc->wc_flags & IB_WC_GRH) {
 			if (dgid.global.interface_id != cpu_to_be64(IB_SA_WELL_KNOWN_GUID)) {
 				ret = ib_find_cached_gid_by_port(device, &dgid,
 								 IB_GID_TYPE_IB,
@@ -584,16 +591,15 @@ int ib_init_ah_from_wc(struct ib_device *device, u8 port_num,
 			} else {
 				gid_index = 0;
 			}
-		}
-
-		flow_class = be32_to_cpu(grh->version_tclass_flow);
-		rdma_ah_set_grh(ah_attr, &sgid,
-				flow_class & 0xFFFFF,
-				(u8)gid_index, hoplimit,
-				(flow_class >> 20) & 0xFF);
 
+			flow_class = be32_to_cpu(grh->version_tclass_flow);
+			rdma_ah_set_grh(ah_attr, &sgid,
+					flow_class & 0xFFFFF,
+					(u8)gid_index, hoplimit,
+					(flow_class >> 20) & 0xFF);
+		}
+		return 0;
 	}
-	return 0;
 }
 EXPORT_SYMBOL(ib_init_ah_from_wc);
 
@@ -1290,34 +1296,8 @@ static int ib_resolve_eth_dmac(struct ib_device *device,
 					(char *)ah_attr->roce.dmac);
 		}
 	} else {
-		union ib_gid		sgid;
-		struct ib_gid_attr	sgid_attr;
-		int			ifindex;
-		int			hop_limit;
-
-		ret = ib_query_gid(device,
-				   rdma_ah_get_port_num(ah_attr),
-				   grh->sgid_index,
-				   &sgid, &sgid_attr);
-
-		if (ret || !sgid_attr.ndev) {
-			if (!ret)
-				ret = -ENXIO;
-			goto out;
-		}
-
-		ifindex = sgid_attr.ndev->ifindex;
-
-		ret =
-		rdma_addr_find_l2_eth_by_grh(&sgid, &grh->dgid,
-					     ah_attr->roce.dmac,
-					     NULL, &ifindex, &hop_limit);
-
-		dev_put(sgid_attr.ndev);
-
-		grh->hop_limit = hop_limit;
+		ret = ib_resolve_unicast_gid_dmac(device, ah_attr);
 	}
-out:
 	return ret;
 }
 
diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
index 18c564f60e93..d5c3bbb84608 100644
--- a/include/rdma/ib_addr.h
+++ b/include/rdma/ib_addr.h
@@ -134,7 +134,7 @@ int rdma_addr_size(struct sockaddr *addr);
 int rdma_addr_find_smac_by_sgid(union ib_gid *sgid, u8 *smac, u16 *vlan_id);
 int rdma_addr_find_l2_eth_by_grh(const union ib_gid *sgid,
 				 const union ib_gid *dgid,
-				 u8 *smac, u16 *vlan_id, int *if_index,
+				 u8 *dmac, const struct net_device *ndev,
 				 int *hoplimit);
 
 static inline u16 ib_addr_get_pkey(struct rdma_dev_addr *dev_addr)
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next, 02/31] IB/{core/cm}: Fix dmac query for IPv6 link local destination

[Jason Gunthorpe]

On Tue, Nov 14, 2017 at 02:51:49PM +0200, Leon Romanovsky wrote:

> +static int ib_resolve_unicast_gid_dmac(struct ib_device *device,
> +				       struct rdma_ah_attr *ah_attr)
> +{
> +	struct ib_gid_attr sgid_attr;
> +	struct ib_global_route *grh;
> +	int hop_limit = 0xff;
> +	union ib_gid sgid;
> +	int ret;
> +
> +	grh = rdma_ah_retrieve_grh(ah_attr);
> +
> +	ret = ib_query_gid(device,
> +			   rdma_ah_get_port_num(ah_attr),
> +			   grh->sgid_index,
> +			   &sgid, &sgid_attr);

[..]

> +		ret = get_sgid_index_from_eth(device, port_num,
> +					      vlan_id, &dgid,
> +					      gid_type, &gid_index);
>  		if (ret)
>  			return ret;
> +		flow_class = be32_to_cpu(grh->version_tclass_flow);
> +		rdma_ah_set_grh(ah_attr, &sgid,
> +				flow_class & 0xFFFFF,
> +				(u8)gid_index, hoplimit,
> +				(flow_class >> 20) & 0xFF);
> +		return ib_resolve_unicast_gid_dmac(device, ah_attr);

This patch looks like a major improvement, so I am going to take it.

However, I dislike this flow here, calling get_sgid_index_from_eth to
get an index then converting the index to a ib_gid_attr via
ib_query_gid is silly.

We should have support APIs that go directly from the WC & GRH to a
gid_attr without the racy inbetween step of gid_index.

Can you send a followup to fix it?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 03/31] IB/core: Depend on IPv6 stack to resolve link local address

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Currently ib_resolve_eth_dmac() bypasses IPv6 stack for resolving link
local destination mac address and translates the IPv6 link local GID
to MAC address, which is not always correct.
Static link local neighbour entry can be setup using
ip -6 neigh add <link_local_addr> lladdr <mac_addr> dev eth0
Current code bypasses such administratively set entries and IPV6 stack.

IPv6 RFC 4861 defines neighbour discovery protocol and is already
implemented by the kernel.

Now that rdma_addr_find_l2_eth_by_grh() supports resolving link local
address to find destination mac address, lets make use of it.
This aligns it to how the rest of the IPv6 stack resolves link local
destination IPv6 address.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/verbs.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 54b56c4fcc38..75ebd74f8bbd 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -506,9 +506,20 @@ static int ib_resolve_unicast_gid_dmac(struct ib_device *device,
 		return ret;
 	}
 
+	/* If destination is link local and source GID is RoCEv1,
+	 * IP stack is not used.
+	 */
+	if (rdma_link_local_addr((struct in6_addr *)grh->dgid.raw) &&
+	    sgid_attr.gid_type == IB_GID_TYPE_ROCE) {
+		rdma_get_ll_mac((struct in6_addr *)grh->dgid.raw,
+				ah_attr->roce.dmac);
+		goto done;
+	}
+
 	ret = rdma_addr_find_l2_eth_by_grh(&sgid, &grh->dgid,
 					   ah_attr->roce.dmac,
 					   sgid_attr.ndev, &hop_limit);
+done:
 	dev_put(sgid_attr.ndev);
 
 	grh->hop_limit = hop_limit;
@@ -1280,11 +1291,6 @@ static int ib_resolve_eth_dmac(struct ib_device *device,
 
 	grh = rdma_ah_retrieve_grh(ah_attr);
 
-	if (rdma_link_local_addr((struct in6_addr *)grh->dgid.raw)) {
-		rdma_get_ll_mac((struct in6_addr *)grh->dgid.raw,
-				ah_attr->roce.dmac);
-		return 0;
-	}
 	if (rdma_is_multicast_addr((struct in6_addr *)ah_attr->grh.dgid.raw)) {
 		if (ipv6_addr_v4mapped((struct in6_addr *)ah_attr->grh.dgid.raw)) {
 			__be32 addr = 0;
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 03/31] IB/core: Depend on IPv6 stack to resolve link local address

[Jason Gunthorpe]

On Tue, Nov 14, 2017 at 02:51:50PM +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Currently ib_resolve_eth_dmac() bypasses IPv6 stack for resolving link
> local destination mac address and translates the IPv6 link local GID
> to MAC address, which is not always correct.
> Static link local neighbour entry can be setup using
> ip -6 neigh add <link_local_addr> lladdr <mac_addr> dev eth0
> Current code bypasses such administratively set entries and IPV6 stack.

At the very least this description makes little sense.

The code is inspecting a DGID, and when in rocev1 mode, the DGID is
*NOT* an IPv6 address and the correct thing to do is what it is doing
now.

In rocev2 mode the DGID becomes and IPv6 address and you should do ND.

Has this distinction been captured in the patch?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 03/31] IB/core: Depend on IPv6 stack to resolve link local address

[Parav Pandit]

Hi Jason,

> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgg-uk2M96/98Pc@public.gmane.org]
> Sent: Tuesday, November 14, 2017 6:33 PM
> To: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Parav
> Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Subject: Re: [PATCH rdma-next 03/31] IB/core: Depend on IPv6 stack to resolve
> link local address
> 
> On Tue, Nov 14, 2017 at 02:51:50PM +0200, Leon Romanovsky wrote:
> > From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > Currently ib_resolve_eth_dmac() bypasses IPv6 stack for resolving link
> > local destination mac address and translates the IPv6 link local GID
> > to MAC address, which is not always correct.
> > Static link local neighbour entry can be setup using ip -6 neigh add
> > <link_local_addr> lladdr <mac_addr> dev eth0 Current code bypasses
> > such administratively set entries and IPV6 stack.
> 
> At the very least this description makes little sense.
> 
> The code is inspecting a DGID, and when in rocev1 mode, the DGID is
And it is inspecting SGID type too as rocev1 or v2 using below check.
sgid_attr.gid_type == IB_GID_TYPE_ROCE

> *NOT* an IPv6 address and the correct thing to do is what it is doing now.
> 
Yes. It does what it does today. When SGID is RoCEv1, it doesn't do ND.

> In rocev2 mode the DGID becomes and IPv6 address and you should do ND.
> 
> Has this distinction been captured in the patch?
Yes. When SGID type is RoCEv2, it performs ARP/ND.
This is described in commit log - it uses rdma_addr_find_l2_eth_by_grh() which performs ND.

> 
> Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 03/31] IB/core: Depend on IPv6 stack to resolve link local address

[Jason Gunthorpe]

On Wed, Nov 15, 2017 at 04:04:37AM +0000, Parav Pandit wrote:

> > Has this distinction been captured in the patch?
> Yes. When SGID type is RoCEv2, it performs ARP/ND.

Good then..

> This is described in commit log - it uses
> rdma_addr_find_l2_eth_by_grh() which performs ND.

I find your commit messages very hard to understand.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 03/31] IB/core: Depend on IPv6 stack to resolve link local address

[Parav Pandit]

> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgg-uk2M96/98Pc@public.gmane.org]
> Sent: Tuesday, November 14, 2017 10:41 PM
> To: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>; Doug Ledford
> <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH rdma-next 03/31] IB/core: Depend on IPv6 stack to resolve
> link local address
> 
> On Wed, Nov 15, 2017 at 04:04:37AM +0000, Parav Pandit wrote:
> 
> > > Has this distinction been captured in the patch?
> > Yes. When SGID type is RoCEv2, it performs ARP/ND.
> 
> Good then..
> 
> > This is described in commit log - it uses
> > rdma_addr_find_l2_eth_by_grh() which performs ND.
> 
> I find your commit messages very hard to understand.
> 
Thanks Jason for the input. I will improve upon in future commits.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 04/31] IB/core: Avoid exporting module internal function

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

ib_security_modify_qp and ib_security_pkey_access are core internal
function. So avoid exporting them.
ib_security_pkey_access is used only when secuirty hooks are enabled so
avoid defining it otherwise.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/core_priv.h | 13 -------------
 drivers/infiniband/core/security.c  | 10 ++++------
 2 files changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index a1d687a664f8..6c4541af54bb 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -213,11 +213,6 @@ int ib_get_cached_subnet_prefix(struct ib_device *device,
 				u64              *sn_pfx);
 
 #ifdef CONFIG_SECURITY_INFINIBAND
-int ib_security_pkey_access(struct ib_device *dev,
-			    u8 port_num,
-			    u16 pkey_index,
-			    void *sec);
-
 void ib_security_destroy_port_pkey_list(struct ib_device *device);
 
 void ib_security_cache_change(struct ib_device *device,
@@ -240,14 +235,6 @@ int ib_mad_agent_security_setup(struct ib_mad_agent *agent,
 void ib_mad_agent_security_cleanup(struct ib_mad_agent *agent);
 int ib_mad_enforce_security(struct ib_mad_agent_private *map, u16 pkey_index);
 #else
-static inline int ib_security_pkey_access(struct ib_device *dev,
-					  u8 port_num,
-					  u16 pkey_index,
-					  void *sec)
-{
-	return 0;
-}
-
 static inline void ib_security_destroy_port_pkey_list(struct ib_device *device)
 {
 }
diff --git a/drivers/infiniband/core/security.c b/drivers/infiniband/core/security.c
index 23278ed5be45..209d0574fc1f 100644
--- a/drivers/infiniband/core/security.c
+++ b/drivers/infiniband/core/security.c
@@ -620,12 +620,11 @@ int ib_security_modify_qp(struct ib_qp *qp,
 	}
 	return ret;
 }
-EXPORT_SYMBOL(ib_security_modify_qp);
 
-int ib_security_pkey_access(struct ib_device *dev,
-			    u8 port_num,
-			    u16 pkey_index,
-			    void *sec)
+static int ib_security_pkey_access(struct ib_device *dev,
+				   u8 port_num,
+				   u16 pkey_index,
+				   void *sec)
 {
 	u64 subnet_prefix;
 	u16 pkey;
@@ -642,7 +641,6 @@ int ib_security_pkey_access(struct ib_device *dev,
 
 	return security_ib_pkey_access(sec, subnet_prefix, pkey);
 }
-EXPORT_SYMBOL(ib_security_pkey_access);
 
 static int ib_mad_agent_security_change(struct notifier_block *nb,
 					unsigned long event,
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 05/31] IB/mlx4: Remove unused ibpd parameter

[Leon Romanovsky]

From: Erez Alfasi <ereza-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Remove unused ibpd parameter from create_qp_rss() function.

Signed-off-by: Erez Alfasi <ereza-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/hw/mlx4/qp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 013049bcdb53..dc1c61745ad8 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -725,7 +725,7 @@ static int set_qp_rss(struct mlx4_ib_dev *dev, struct mlx4_ib_rss *rss_ctx,
 	return 0;
 }
 
-static int create_qp_rss(struct mlx4_ib_dev *dev, struct ib_pd *ibpd,
+static int create_qp_rss(struct mlx4_ib_dev *dev,
 			 struct ib_qp_init_attr *init_attr,
 			 struct mlx4_ib_create_qp_rss *ucmd,
 			 struct mlx4_ib_qp *qp)
@@ -848,7 +848,7 @@ static struct ib_qp *_mlx4_ib_create_qp_rss(struct ib_pd *pd,
 	qp->pri.vid = 0xFFFF;
 	qp->alt.vid = 0xFFFF;
 
-	err = create_qp_rss(to_mdev(pd->device), pd, init_attr, &ucmd, qp);
+	err = create_qp_rss(to_mdev(pd->device), init_attr, &ucmd, qp);
 	if (err) {
 		kfree(qp);
 		return ERR_PTR(err);
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 06/31] IB/ipoib: Avoid memory leak if neigh destination was changed

[Leon Romanovsky]

From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

If from some reason the SM responses to path query request with response
that doesn't contain the exact SGID, ipoib should warn and change that
part of the response before push it to the path record DB.
Otherwise, new record will be added to the path record DB with no access
to them from the ipoib.

Demonstration of the bug is as the follow:
ipoib wants to send to GID fe80:0000:0000:0000:0002:c903:00ef:5ee2, it
creates new record in the DB with that gid as a key, and issues a new
request to the sm.
Now, the SM from some reason returns path-record with other SGID (for
example, fe80:0000:0000:0001:0002:c903:00ef:5ee2 that contains the local
subnet prefix) now ipoib will overwrite the current entry with the new
one, and if new request to the original GID arrives ipoib  will not find
it in the DB (was overwritten) and will create new record that in its
turn will also be overwritten by the response from the SM, and so on
till the driver eats all the device memory.

Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 12b7f911f0e5..b173d618c59c 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -775,6 +775,16 @@ static void path_rec_completion(int status,
 	spin_lock_irqsave(&priv->lock, flags);
 
 	if (!IS_ERR_OR_NULL(ah)) {
+		/* check there is no mismatch from the request */
+		if (memcmp(pathrec->dgid.raw, path->pathrec.dgid.raw,
+			   sizeof(union ib_gid))) {
+			pr_warn("%s got PathRec for gid %pI6 while asked for %pI6\n",
+				dev->name, pathrec->dgid.raw, path->pathrec.dgid.raw);
+			/* overwrite the response from the sm  before copy to the db */
+			memcpy(pathrec->dgid.raw, path->pathrec.dgid.raw,
+			       sizeof(union ib_gid));
+		}
+
 		path->pathrec = *pathrec;
 
 		old_ah   = path->ah;
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 06/31] IB/ipoib: Avoid memory leak if neigh destination was changed

[Or Gerlitz]

On Tue, Nov 14, 2017 at 2:51 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> If from some reason the SM responses to path query request with response
> that doesn't contain the exact SGID, ipoib should warn and change that
> part of the response before push it to the path record DB.
> Otherwise, new record will be added to the path record DB with no access
> to them from the ipoib.
>
> Demonstration of the bug is as the follow:

this series contains bunch of fixes, any reason to direct them to
-next (4.16) and not -rc (4.15)

Also, in most of the cases ppl didn't provide a Fixes: line with the
offending patch, any reason to skip that?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 06/31] IB/ipoib: Avoid memory leak if neigh destination was changed

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1549 bytes --]

On Tue, Nov 14, 2017 at 11:51:09PM +0200, Or Gerlitz wrote:
> On Tue, Nov 14, 2017 at 2:51 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > If from some reason the SM responses to path query request with response
> > that doesn't contain the exact SGID, ipoib should warn and change that
> > part of the response before push it to the path record DB.
> > Otherwise, new record will be added to the path record DB with no access
> > to them from the ipoib.
> >
> > Demonstration of the bug is as the follow:
>
> this series contains bunch of fixes, any reason to direct them to
> -next (4.16) and not -rc (4.15)

Doug didn't send his pull request yet and most probably we will have two
pull requests in this merge window, one with already accepted code and
another with fixes from Denny and us. So all these patches are targeted
for 4.15.


>
> Also, in most of the cases ppl didn't provide a Fixes: line with the
> offending patch, any reason to skip that?

There are two main reasons:
1. I didn't request to add Fixes for refactoring code (most Parav's
patches), because I don't want that code to be picked for stable@ trees
by Greg's and Sasha's automatic scripts.
2. Sometimes, it is not clear the origin of the bug and the chances to
have wrong Fixes are higher than to have right ones.

Everything else is my mistake during review. Can you point me to
specific commits where you would like to see Fixes with right Fixes? I'll add.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-next 06/31] IB/ipoib: Avoid memory leak if neigh destination was changed

[Jason Gunthorpe]

On Tue, Nov 14, 2017 at 02:51:53PM +0200, Leon Romanovsky wrote:
> From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> If from some reason the SM responses to path query request with response
> that doesn't contain the exact SGID, ipoib should warn and change that
> part of the response before push it to the path record DB.
> Otherwise, new record will be added to the path record DB with no access
> to them from the ipoib.
> 
> Demonstration of the bug is as the follow:
> ipoib wants to send to GID fe80:0000:0000:0000:0002:c903:00ef:5ee2, it
> creates new record in the DB with that gid as a key, and issues a new
> request to the sm.
> Now, the SM from some reason returns path-record with other SGID (for
> example, fe80:0000:0000:0001:0002:c903:00ef:5ee2 that contains the local
> subnet prefix) now ipoib will overwrite the current entry with the new
> one, and if new request to the original GID arrives ipoib  will not find
> it in the DB (was overwritten) and will create new record that in its
> turn will also be overwritten by the response from the SM, and so on
> till the driver eats all the device memory.
> 
> Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index 12b7f911f0e5..b173d618c59c 100644
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -775,6 +775,16 @@ static void path_rec_completion(int status,
>  	spin_lock_irqsave(&priv->lock, flags);
>  
>  	if (!IS_ERR_OR_NULL(ah)) {
> +		/* check there is no mismatch from the request */
> +		if (memcmp(pathrec->dgid.raw, path->pathrec.dgid.raw,
> +			   sizeof(union ib_gid))) {
> +			pr_warn("%s got PathRec for gid %pI6 while asked for %pI6\n",
> +				dev->name, pathrec->dgid.raw, path->pathrec.dgid.raw);
> +			/* overwrite the response from the sm  before copy to the db */
> +			memcpy(pathrec->dgid.raw, path->pathrec.dgid.raw,
> +			       sizeof(union ib_gid));

This doesn't seem like it should be a warning, and replacing the
good DGID from the SA with local garbage seems really wrong.

The SA should be free to return a correct DGID, for instance by
replacing a old or link local subnet prefix with the current value of
the subnet prefix, or by returning a different gid index for the
destination depending on mulitpath requirements.

So the right thing to do here is to somehow make ipoib use the data
from the SA.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 06/31] IB/ipoib: Avoid memory leak if neigh destination was changed

[Erez Shitrit]

On Wed, Nov 15, 2017 at 2:15 AM, Jason Gunthorpe <jgg-uk2M96/98Pc@public.gmane.org> wrote:
> On Tue, Nov 14, 2017 at 02:51:53PM +0200, Leon Romanovsky wrote:
>> From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>
>> If from some reason the SM responses to path query request with response
>> that doesn't contain the exact SGID, ipoib should warn and change that
>> part of the response before push it to the path record DB.
>> Otherwise, new record will be added to the path record DB with no access
>> to them from the ipoib.
>>
>> Demonstration of the bug is as the follow:
>> ipoib wants to send to GID fe80:0000:0000:0000:0002:c903:00ef:5ee2, it
>> creates new record in the DB with that gid as a key, and issues a new
>> request to the sm.
>> Now, the SM from some reason returns path-record with other SGID (for
>> example, fe80:0000:0000:0001:0002:c903:00ef:5ee2 that contains the local
>> subnet prefix) now ipoib will overwrite the current entry with the new
>> one, and if new request to the original GID arrives ipoib  will not find
>> it in the DB (was overwritten) and will create new record that in its
>> turn will also be overwritten by the response from the SM, and so on
>> till the driver eats all the device memory.
>>
>> Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
>> index 12b7f911f0e5..b173d618c59c 100644
>> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
>> @@ -775,6 +775,16 @@ static void path_rec_completion(int status,
>>       spin_lock_irqsave(&priv->lock, flags);
>>
>>       if (!IS_ERR_OR_NULL(ah)) {
>> +             /* check there is no mismatch from the request */
>> +             if (memcmp(pathrec->dgid.raw, path->pathrec.dgid.raw,
>> +                        sizeof(union ib_gid))) {
>> +                     pr_warn("%s got PathRec for gid %pI6 while asked for %pI6\n",
>> +                             dev->name, pathrec->dgid.raw, path->pathrec.dgid.raw);
>> +                     /* overwrite the response from the sm  before copy to the db */
>> +                     memcpy(pathrec->dgid.raw, path->pathrec.dgid.raw,
>> +                            sizeof(union ib_gid));
>
> This doesn't seem like it should be a warning, and replacing the
> good DGID from the SA with local garbage seems really wrong.

IPoIB is L2 driver and should act as a pipe and cannot add its opinion
about what is a good or a garbage GID.
If the host is asking for specific mac address, IPoIB should return
the response for the original request/mac and should not decide what
is good for the host.
This issue can happen from several issues, sm-bug or bad
configuration, i don't think IPoIB is the place to fix that.
So, what left for IPoIB is to protect itself from endless memory leak,
and this what the patch tries to do.

>
> The SA should be free to return a correct DGID, for instance by
> replacing a old or link local subnet prefix with the current value of
> the subnet prefix, or by returning a different gid index for the
> destination depending on mulitpath requirements.
>
> So the right thing to do here is to somehow make ipoib use the data
> from the SA.
>
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 06/31] IB/ipoib: Avoid memory leak if neigh destination was changed

[Jason Gunthorpe]

On Wed, Nov 15, 2017 at 09:01:07AM +0200, Erez Shitrit wrote:
> >>       if (!IS_ERR_OR_NULL(ah)) {
> >> +             /* check there is no mismatch from the request */
> >> +             if (memcmp(pathrec->dgid.raw, path->pathrec.dgid.raw,
> >> +                        sizeof(union ib_gid))) {
> >> +                     pr_warn("%s got PathRec for gid %pI6 while asked for %pI6\n",
> >> +                             dev->name, pathrec->dgid.raw, path->pathrec.dgid.raw);
> >> +                     /* overwrite the response from the sm  before copy to the db */
> >> +                     memcpy(pathrec->dgid.raw, path->pathrec.dgid.raw,
> >> +                            sizeof(union ib_gid));
> >
> > This doesn't seem like it should be a warning, and replacing the
> > good DGID from the SA with local garbage seems really wrong.
> 
> IPoIB is L2 driver and should act as a pipe and cannot add its opinion
> about what is a good or a garbage GID.

Which is why ipoib should not override the result from the SA.

It needs to work in a way where it looks up a DGID from the LLADDR and
gets back a wire GRH and LRH which come from SA.

> If the host is asking for specific mac address, IPoIB should return
> the response for the original request/mac and should not decide what
> is good for the host.

No, it is up to the SA to return the GRH to put in the packets, it
should never be changed in any way by a ULP.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 06/31] IB/ipoib: Avoid memory leak if neigh destination was changed

[Erez Shitrit]

On Wed, Nov 15, 2017 at 5:39 PM, Jason Gunthorpe <jgg-uk2M96/98Pc@public.gmane.org> wrote:
> On Wed, Nov 15, 2017 at 09:01:07AM +0200, Erez Shitrit wrote:
>> >>       if (!IS_ERR_OR_NULL(ah)) {
>> >> +             /* check there is no mismatch from the request */
>> >> +             if (memcmp(pathrec->dgid.raw, path->pathrec.dgid.raw,
>> >> +                        sizeof(union ib_gid))) {
>> >> +                     pr_warn("%s got PathRec for gid %pI6 while asked for %pI6\n",
>> >> +                             dev->name, pathrec->dgid.raw, path->pathrec.dgid.raw);
>> >> +                     /* overwrite the response from the sm  before copy to the db */
>> >> +                     memcpy(pathrec->dgid.raw, path->pathrec.dgid.raw,
>> >> +                            sizeof(union ib_gid));
>> >
>> > This doesn't seem like it should be a warning, and replacing the
>> > good DGID from the SA with local garbage seems really wrong.
>>
>> IPoIB is L2 driver and should act as a pipe and cannot add its opinion
>> about what is a good or a garbage GID.
>
> Which is why ipoib should not override the result from the SA.

It override only the DGID in the record, and that for IPoIB uses only,
not the ah which is the important.
otherwise if the host will re-ask again for that DGID it will not be
found in IPoIB database, and new record will be created and so on till
the end of all the device memory.

>
> It needs to work in a way where it looks up a DGID from the LLADDR and
> gets back a wire GRH and LRH which come from SA.

For that there is the ah object, this object is used as it came from the SA.

>
>> If the host is asking for specific mac address, IPoIB should return
>> the response for the original request/mac and should not decide what
>> is good for the host.
>
> No, it is up to the SA to return the GRH to put in the packets, it
> should never be changed in any way by a ULP.

The change is only for the ULP, the user/host has no idea of that, and
it gets the same ah that came from the SA.

>
> Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 06/31] IB/ipoib: Avoid memory leak if neigh destination was changed

[Håkon Bugge]

> On 16 Nov 2017, at 09:30, Erez Shitrit <erezsh-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
> 
> On Wed, Nov 15, 2017 at 5:39 PM, Jason Gunthorpe <jgg-uk2M96/98Pc@public.gmane.org> wrote:
>> On Wed, Nov 15, 2017 at 09:01:07AM +0200, Erez Shitrit wrote:
>>>>>      if (!IS_ERR_OR_NULL(ah)) {
>>>>> +             /* check there is no mismatch from the request */
>>>>> +             if (memcmp(pathrec->dgid.raw, path->pathrec.dgid.raw,
>>>>> +                        sizeof(union ib_gid))) {
>>>>> +                     pr_warn("%s got PathRec for gid %pI6 while asked for %pI6\n",
>>>>> +                             dev->name, pathrec->dgid.raw, path->pathrec.dgid.raw);
>>>>> +                     /* overwrite the response from the sm  before copy to the db */
>>>>> +                     memcpy(pathrec->dgid.raw, path->pathrec.dgid.raw,
>>>>> +                            sizeof(union ib_gid));
>>>> 
>>>> This doesn't seem like it should be a warning, and replacing the
>>>> good DGID from the SA with local garbage seems really wrong.
>>> 
>>> IPoIB is L2 driver and should act as a pipe and cannot add its opinion
>>> about what is a good or a garbage GID.
>> 
>> Which is why ipoib should not override the result from the SA.
> 
> It override only the DGID in the record, and that for IPoIB uses only,
> not the ah which is the important.
> otherwise if the host will re-ask again for that DGID it will not be
> found in IPoIB database, and new record will be created and so on till
> the end of all the device memory.

Isn’t the correct solution to mask away the 16-bit subnet prefix when looking up a unicast DGID in the database?


Thxs, Håkon

> 
>> 
>> It needs to work in a way where it looks up a DGID from the LLADDR and
>> gets back a wire GRH and LRH which come from SA.
> 
> For that there is the ah object, this object is used as it came from the SA.
> 
>> 
>>> If the host is asking for specific mac address, IPoIB should return
>>> the response for the original request/mac and should not decide what
>>> is good for the host.
>> 
>> No, it is up to the SA to return the GRH to put in the packets, it
>> should never be changed in any way by a ULP.
> 
> The change is only for the ULP, the user/host has no idea of that, and
> it gets the same ah that came from the SA.
> 
>> 
>> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 06/31] IB/ipoib: Avoid memory leak if neigh destination was changed

[Jason Gunthorpe]

On Thu, Nov 16, 2017 at 06:28:42PM +0100, Håkon Bugge wrote:

> > It override only the DGID in the record, and that for IPoIB uses
> > only, not the ah which is the important.  otherwise if the host
> > will re-ask again for that DGID it will not be found in IPoIB
> > database, and new record will be created and so on till the end of
> > all the device memory.
> 
> Isn’t the correct solution to mask away the 16-bit subnet prefix
> when looking up a unicast DGID in the database?

No, the SA can also alter the GUID portion to point to a GUID
alias. The spec envisioned using this for global multipathing.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 06/31] IB/ipoib: Avoid memory leak if neigh destination was changed

[Jason Gunthorpe]

On Thu, Nov 16, 2017 at 10:30:15AM +0200, Erez Shitrit wrote:

> > It needs to work in a way where it looks up a DGID from the LLADDR and
> > gets back a wire GRH and LRH which come from SA.
> 
> For that there is the ah object, this object is used as it came from the SA.

Okay, that makes sense. This is such an important point, the patch
description really should have explored it.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next, 06/31] IB/ipoib: Avoid memory leak if neigh destination was changed

[Jason Gunthorpe]

On Tue, Nov 14, 2017 at 02:51:53PM +0200, Leon Romanovsky wrote:
> From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> If from some reason the SM responses to path query request with response
> that doesn't contain the exact SGID, ipoib should warn and change that
> part of the response before push it to the path record DB.
> Otherwise, new record will be added to the path record DB with no access
> to them from the ipoib.
> 
> Demonstration of the bug is as the follow:
> ipoib wants to send to GID fe80:0000:0000:0000:0002:c903:00ef:5ee2, it
> creates new record in the DB with that gid as a key, and issues a new
> request to the sm.
> Now, the SM from some reason returns path-record with other SGID (for
> example, fe80:0000:0000:0001:0002:c903:00ef:5ee2 that contains the local
> subnet prefix) now ipoib will overwrite the current entry with the new
> one, and if new request to the original GID arrives ipoib  will not find
> it in the DB (was overwritten) and will create new record that in its
> turn will also be overwritten by the response from the SM, and so on
> till the driver eats all the device memory.
> 
> Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index 12b7f911f0e5..b173d618c59c 100644
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -775,6 +775,16 @@ static void path_rec_completion(int status,
>  	spin_lock_irqsave(&priv->lock, flags);
>  
>  	if (!IS_ERR_OR_NULL(ah)) {
> +		/* check there is no mismatch from the request */
> +		if (memcmp(pathrec->dgid.raw, path->pathrec.dgid.raw,
> +			   sizeof(union ib_gid))) {
> +			pr_warn("%s got PathRec for gid %pI6 while asked for %pI6\n",
> +				dev->name, pathrec->dgid.raw, path->pathrec.dgid.raw);
> +			/* overwrite the response from the sm  before copy to the db */
> +			memcpy(pathrec->dgid.raw, path->pathrec.dgid.raw,
> +			       sizeof(union ib_gid));
> +		}
> +
>  		path->pathrec = *pathrec;

I applied this, but converted the pr_warn to ipoib_dbg (it is not an
error for the SA to change the DGID), reworded the commit message and
comment:

		/*
		 * pathrec.dgid is used as the database key from the LLADDR,
		 * it must remain unchanged even if the SA returns a different
		 * GID to use in the AH.
		 */
		if (memcmp(pathrec->dgid.raw, path->pathrec.dgid.raw,
			   sizeof(union ib_gid))) {
			ipoib_dbg(
				priv,
				"%s got PathRec for gid %pI6 while asked for %pI6\n",
				dev->name, pathrec->dgid.raw,
				path->pathrec.dgid.raw);
			memcpy(pathrec->dgid.raw, path->pathrec.dgid.raw,
			       sizeof(union ib_gid));
		}
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 07/31] IB/ipoib: Update pathrec field if not valid record

[Leon Romanovsky]

From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

In case that the PathRecord is not valid (SM changed its network prefix)
ipoib will continue issue PathQuery requests with the same parameters
that are in its database, which are no longer valid anymore.

Now the driver in that case will re-initialize the record from a valid
place (the priv structure keeps the updated values), and a valid request
will be issued.

Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Alex Vesker <valex-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 49 +++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index b173d618c59c..33f592cee5d8 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -850,6 +850,23 @@ static void path_rec_completion(int status,
 	}
 }
 
+static void init_path_rec(struct ipoib_dev_priv *priv, struct ipoib_path *path,
+			  void *gid)
+{
+	path->dev = priv->dev;
+
+	if (rdma_cap_opa_ah(priv->ca, priv->port))
+		path->pathrec.rec_type = SA_PATH_REC_TYPE_OPA;
+	else
+		path->pathrec.rec_type = SA_PATH_REC_TYPE_IB;
+
+	memcpy(path->pathrec.dgid.raw, gid, sizeof(union ib_gid));
+	path->pathrec.sgid	    = priv->local_gid;
+	path->pathrec.pkey	    = cpu_to_be16(priv->pkey);
+	path->pathrec.numb_path     = 1;
+	path->pathrec.traffic_class = priv->broadcast->mcmember.traffic_class;
+}
+
 static struct ipoib_path *path_rec_create(struct net_device *dev, void *gid)
 {
 	struct ipoib_dev_priv *priv = ipoib_priv(dev);
@@ -862,21 +879,11 @@ static struct ipoib_path *path_rec_create(struct net_device *dev, void *gid)
 	if (!path)
 		return NULL;
 
-	path->dev = dev;
-
 	skb_queue_head_init(&path->queue);
 
 	INIT_LIST_HEAD(&path->neigh_list);
 
-	if (rdma_cap_opa_ah(priv->ca, priv->port))
-		path->pathrec.rec_type = SA_PATH_REC_TYPE_OPA;
-	else
-		path->pathrec.rec_type = SA_PATH_REC_TYPE_IB;
-	memcpy(path->pathrec.dgid.raw, gid, sizeof (union ib_gid));
-	path->pathrec.sgid	    = priv->local_gid;
-	path->pathrec.pkey	    = cpu_to_be16(priv->pkey);
-	path->pathrec.numb_path     = 1;
-	path->pathrec.traffic_class = priv->broadcast->mcmember.traffic_class;
+	init_path_rec(priv, path, gid);
 
 	return path;
 }
@@ -1005,6 +1012,10 @@ static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev,
 
 	spin_lock_irqsave(&priv->lock, flags);
 
+	/* no broadcast means that all paths are (going to be) not valid */
+	if (!priv->broadcast)
+		goto drop_and_unlock;
+
 	path = __path_find(dev, phdr->hwaddr + 4);
 	if (!path || !path->valid) {
 		int new_path = 0;
@@ -1014,6 +1025,10 @@ static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev,
 			new_path = 1;
 		}
 		if (path) {
+			if (!new_path)
+				/* make sure there is no changes in the existing path record */
+				init_path_rec(priv, path, phdr->hwaddr + 4);
+
 			if (skb_queue_len(&path->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
 				push_pseudo_header(skb, phdr->hwaddr);
 				__skb_queue_tail(&path->queue, skb);
@@ -1030,8 +1045,7 @@ static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev,
 			} else
 				__path_add(dev, path);
 		} else {
-			++dev->stats.tx_dropped;
-			dev_kfree_skb_any(skb);
+			goto drop_and_unlock;
 		}
 
 		spin_unlock_irqrestore(&priv->lock, flags);
@@ -1051,11 +1065,16 @@ static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev,
 		push_pseudo_header(skb, phdr->hwaddr);
 		__skb_queue_tail(&path->queue, skb);
 	} else {
-		++dev->stats.tx_dropped;
-		dev_kfree_skb_any(skb);
+		goto drop_and_unlock;
 	}
 
 	spin_unlock_irqrestore(&priv->lock, flags);
+	return;
+
+drop_and_unlock:
+	++dev->stats.tx_dropped;
+	dev_kfree_skb_any(skb);
+	spin_unlock_irqrestore(&priv->lock, flags);
 }
 
 static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 08/31] RDMA/cma: Fix consider size of destination address

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

While copying destination address in statistics, consider size of
destination address.

Fixes: ce117ffac2e9 ("RDMA/cma: Export AF_IB statistics")
Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 1fdb473b5df7..9841e7ca5c06 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -4431,7 +4431,7 @@ static int cma_get_id_stats(struct sk_buff *skb, struct netlink_callback *cb)
 					  RDMA_NL_RDMA_CM_ATTR_SRC_ADDR))
 				goto out;
 			if (ibnl_put_attr(skb, nlh,
-					  rdma_addr_size(cma_src_addr(id_priv)),
+					  rdma_addr_size(cma_dst_addr(id_priv)),
 					  cma_dst_addr(id_priv),
 					  RDMA_NL_RDMA_CM_ATTR_DST_ADDR))
 				goto out;
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 08/31] RDMA/cma: Fix consider size of destination address

[Or Gerlitz]

On Tue, Nov 14, 2017 at 2:51 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> While copying destination address in statistics, consider size of
> destination address.
>
> Fixes: ce117ffac2e9 ("RDMA/cma: Export AF_IB statistics")
> Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/infiniband/core/cma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 1fdb473b5df7..9841e7ca5c06 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -4431,7 +4431,7 @@ static int cma_get_id_stats(struct sk_buff *skb, struct netlink_callback *cb)

Leon, isn't that the code you want to remove in your other patch?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 08/31] RDMA/cma: Fix consider size of destination address

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]

On Tue, Nov 14, 2017 at 11:18:41PM +0200, Or Gerlitz wrote:
> On Tue, Nov 14, 2017 at 2:51 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > While copying destination address in statistics, consider size of
> > destination address.
> >
> > Fixes: ce117ffac2e9 ("RDMA/cma: Export AF_IB statistics")
> > Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> >  drivers/infiniband/core/cma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> > index 1fdb473b5df7..9841e7ca5c06 100644
> > --- a/drivers/infiniband/core/cma.c
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -4431,7 +4431,7 @@ static int cma_get_id_stats(struct sk_buff *skb, struct netlink_callback *cb)
>
> Leon, isn't that the code you want to remove in your other patch?

Yes, I want, but those two parallel things, I posted RFC to see if
anyone actually is using it and if everything goes smooth, I'll post
proper patch for next cycle (4.16).

Right now, code exists in the tree and Parav has fix to it (he is not
using it too).

Thanks

> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH rdma-next 09/31] IB/core: Make sure that PSN does not overflow

[Leon Romanovsky]

From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The rq/sq->psn is 24 bits as defined in the IB spec,therefore we mask
out the 8 most significant bits to avoid overflow in modify qp.

Signed-off-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/verbs.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 75ebd74f8bbd..584b6029cb44 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -1321,6 +1321,7 @@ static int ib_resolve_eth_dmac(struct ib_device *device,
 int ib_modify_qp_with_udata(struct ib_qp *qp, struct ib_qp_attr *attr,
 			    int attr_mask, struct ib_udata *udata)
 {
+	u8 port = attr_mask & IB_QP_PORT ? attr->port_num : qp->port;
 	int ret;
 
 	if (attr_mask & IB_QP_AV) {
@@ -1328,6 +1329,21 @@ int ib_modify_qp_with_udata(struct ib_qp *qp, struct ib_qp_attr *attr,
 		if (ret)
 			return ret;
 	}
+
+	if (rdma_ib_or_roce(qp->device, port)) {
+		if (attr_mask & IB_QP_RQ_PSN && attr->rq_psn & ~0xffffff) {
+			pr_warn("%s: %s rq_psn overflow, masking to 24 bits\n",
+				__func__, qp->device->name);
+			attr->rq_psn &= 0xffffff;
+		}
+
+		if (attr_mask & IB_QP_SQ_PSN && attr->sq_psn & ~0xffffff) {
+			pr_warn("%s: %s sq_psn overflow, masking to 24 bits\n",
+				__func__, qp->device->name);
+			attr->sq_psn &= 0xffffff;
+		}
+	}
+
 	ret = ib_security_modify_qp(qp, attr, attr_mask, udata);
 	if (!ret && (attr_mask & IB_QP_PORT))
 		qp->port = attr->port_num;
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 09/31] IB/core: Make sure that PSN does not overflow

[Or Gerlitz]

On Tue, Nov 14, 2017 at 2:51 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> The rq/sq->psn is 24 bits as defined in the IB spec,therefore we mask
> out the 8 most significant bits to avoid overflow in modify qp.
>
> Signed-off-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/infiniband/core/verbs.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index 75ebd74f8bbd..584b6029cb44 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -1321,6 +1321,7 @@ static int ib_resolve_eth_dmac(struct ib_device *device,
>  int ib_modify_qp_with_udata(struct ib_qp *qp, struct ib_qp_attr *attr,
>                             int attr_mask, struct ib_udata *udata)
>  {
> +       u8 port = attr_mask & IB_QP_PORT ? attr->port_num : qp->port;
>         int ret;
>
>         if (attr_mask & IB_QP_AV) {
> @@ -1328,6 +1329,21 @@ int ib_modify_qp_with_udata(struct ib_qp *qp, struct ib_qp_attr *attr,
>                 if (ret)
>                         return ret;
>         }
> +
> +       if (rdma_ib_or_roce(qp->device, port)) {
> +               if (attr_mask & IB_QP_RQ_PSN && attr->rq_psn & ~0xffffff) {
> +                       pr_warn("%s: %s rq_psn overflow, masking to 24 bits\n",
> +                               __func__, qp->device->name);
> +                       attr->rq_psn &= 0xffffff;
> +               }
> +
> +               if (attr_mask & IB_QP_SQ_PSN && attr->sq_psn & ~0xffffff) {
> +                       pr_warn("%s: %s sq_psn overflow, masking to 24 bits\n",
> +                               __func__, qp->device->name);
> +                       attr->sq_psn &= 0xffffff;
> +               }
> +       }
> +

Isn't this what we tried to push in the past and Jason/Sean rejected [1]?

[1] https://marc.info/?l=linux-rdma&m=142309249413868&w=2


>         ret = ib_security_modify_qp(qp, attr, attr_mask, udata);
>         if (!ret && (attr_mask & IB_QP_PORT))
>                 qp->port = attr->port_num;
> --
> 2.15.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 09/31] IB/core: Make sure that PSN does not overflow

[Jason Gunthorpe]

On Tue, Nov 14, 2017 at 11:23:07PM +0200, Or Gerlitz wrote:
> > +
> > +       if (rdma_ib_or_roce(qp->device, port)) {
> > +               if (attr_mask & IB_QP_RQ_PSN && attr->rq_psn & ~0xffffff) {
> > +                       pr_warn("%s: %s rq_psn overflow, masking to 24 bits\n",
> > +                               __func__, qp->device->name);
> > +                       attr->rq_psn &= 0xffffff;
> > +               }
> > +
> > +               if (attr_mask & IB_QP_SQ_PSN && attr->sq_psn & ~0xffffff) {
> > +                       pr_warn("%s: %s sq_psn overflow, masking to 24 bits\n",
> > +                               __func__, qp->device->name);
> > +                       attr->sq_psn &= 0xffffff;
> > +               }
> > +       }
> > +
> 
> Isn't this what we tried to push in the past and Jason/Sean rejected [1]?
> 
> [1] https://marc.info/?l=linux-rdma&m=142309249413868&w=2

No, this version is what I asked for originally in that thread:

  I think we'd be better off keeping the current behavior of masking
  the PSN, not enforcing the limit.

The issue was because the link'd version returned an error.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 09/31] IB/core: Make sure that PSN does not overflow

[Jason Gunthorpe]

On Tue, Nov 14, 2017 at 02:51:56PM +0200, Leon Romanovsky wrote:
> From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> The rq/sq->psn is 24 bits as defined in the IB spec,therefore we mask
> out the 8 most significant bits to avoid overflow in modify qp.
> 
> Signed-off-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>  drivers/infiniband/core/verbs.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Applied to -next, thanks

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 10/31] IB/core: Fix memory leak in cm_req_handler error flows

[Leon Romanovsky]

From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

In cm_req_handler error flows, sometimes cm_id_priv->timewait_info
isn't free'd.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Mukesh Kacker <mukesh.kacker-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index f6b159d79977..fe7a5e0d79b0 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1842,8 +1842,7 @@ static int cm_req_handler(struct cm_work *work)
 	listen_cm_id_priv = cm_match_req(work, cm_id_priv);
 	if (!listen_cm_id_priv) {
 		ret = -EINVAL;
-		kfree(cm_id_priv->timewait_info);
-		goto destroy;
+		goto free_timeinfo;
 	}
 
 	cm_id_priv->id.cm_handler = listen_cm_id_priv->id.cm_handler;
@@ -1945,6 +1944,8 @@ static int cm_req_handler(struct cm_work *work)
 rejected:
 	atomic_dec(&cm_id_priv->refcount);
 	cm_deref_id(listen_cm_id_priv);
+free_timeinfo:
+	kfree(cm_id_priv->timewait_info);
 destroy:
 	ib_destroy_cm_id(cm_id);
 	return ret;
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 10/31] IB/core: Fix memory leak in cm_req_handler error flows

[Or Gerlitz]

On Tue, Nov 14, 2017 at 2:51 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> In cm_req_handler error flows, sometimes cm_id_priv->timewait_info
> isn't free'd.

Other patches in this series use IB/cm: prefix, best if you align to
that across the place
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 11/31] IB/cm: Add debug prints to ib_cm

[Leon Romanovsky]

From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Add debug prints to the error paths in the connection manager control
flows, to help debug connection management problems.

Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cm.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index fe7a5e0d79b0..cbb385e02947 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1841,6 +1841,8 @@ static int cm_req_handler(struct cm_work *work)
 
 	listen_cm_id_priv = cm_match_req(work, cm_id_priv);
 	if (!listen_cm_id_priv) {
+		pr_debug("%s: local_id %d, no listen_cm_id_priv\n", __func__,
+			 be32_to_cpu(cm_id->local_id));
 		ret = -EINVAL;
 		goto free_timeinfo;
 	}
@@ -1998,6 +2000,8 @@ int ib_send_cm_rep(struct ib_cm_id *cm_id,
 	spin_lock_irqsave(&cm_id_priv->lock, flags);
 	if (cm_id->state != IB_CM_REQ_RCVD &&
 	    cm_id->state != IB_CM_MRA_REQ_SENT) {
+		pr_debug("%s: local_comm_id %d, cm_id->state: %d\n", __func__,
+			 be32_to_cpu(cm_id_priv->id.local_id), cm_id->state);
 		ret = -EINVAL;
 		goto out;
 	}
@@ -2064,6 +2068,8 @@ int ib_send_cm_rtu(struct ib_cm_id *cm_id,
 	spin_lock_irqsave(&cm_id_priv->lock, flags);
 	if (cm_id->state != IB_CM_REP_RCVD &&
 	    cm_id->state != IB_CM_MRA_REP_SENT) {
+		pr_debug("%s: local_id %d, cm_id->state %d\n", __func__,
+			 be32_to_cpu(cm_id->local_id), cm_id->state);
 		ret = -EINVAL;
 		goto error;
 	}
@@ -2171,6 +2177,8 @@ static int cm_rep_handler(struct cm_work *work)
 	cm_id_priv = cm_acquire_id(rep_msg->remote_comm_id, 0);
 	if (!cm_id_priv) {
 		cm_dup_rep_handler(work);
+		pr_debug("%s: remote_comm_id %d, no cm_id_priv\n", __func__,
+			 be32_to_cpu(rep_msg->remote_comm_id));
 		return -EINVAL;
 	}
 
@@ -2184,6 +2192,10 @@ static int cm_rep_handler(struct cm_work *work)
 	default:
 		spin_unlock_irq(&cm_id_priv->lock);
 		ret = -EINVAL;
+		pr_debug("%s: cm_id_priv->id.state: %d, local_comm_id %d, remote_comm_id %d\n",
+			 __func__, cm_id_priv->id.state,
+			 be32_to_cpu(rep_msg->local_comm_id),
+			 be32_to_cpu(rep_msg->remote_comm_id));
 		goto error;
 	}
 
@@ -2197,6 +2209,8 @@ static int cm_rep_handler(struct cm_work *work)
 		spin_unlock(&cm.lock);
 		spin_unlock_irq(&cm_id_priv->lock);
 		ret = -EINVAL;
+		pr_debug("%s: Failed to insert remote id %d\n", __func__,
+			 be32_to_cpu(rep_msg->remote_comm_id));
 		goto error;
 	}
 	/* Check for a stale connection. */
@@ -2214,6 +2228,10 @@ static int cm_rep_handler(struct cm_work *work)
 			     IB_CM_REJ_STALE_CONN, CM_MSG_RESPONSE_REP,
 			     NULL, 0);
 		ret = -EINVAL;
+		pr_debug("%s: Stale connection. local_comm_id %d, remote_comm_id %d\n",
+			 __func__, be32_to_cpu(rep_msg->local_comm_id),
+			 be32_to_cpu(rep_msg->remote_comm_id));
+
 		if (cur_cm_id_priv) {
 			cm_id = &cur_cm_id_priv->id;
 			ib_send_cm_dreq(cm_id, NULL, 0);
@@ -2360,6 +2378,8 @@ int ib_send_cm_dreq(struct ib_cm_id *cm_id,
 	cm_id_priv = container_of(cm_id, struct cm_id_private, id);
 	spin_lock_irqsave(&cm_id_priv->lock, flags);
 	if (cm_id->state != IB_CM_ESTABLISHED) {
+		pr_debug("%s: local_id %d, cm_id->state: %d\n", __func__,
+			 be32_to_cpu(cm_id->local_id), cm_id->state);
 		ret = -EINVAL;
 		goto out;
 	}
@@ -2429,6 +2449,8 @@ int ib_send_cm_drep(struct ib_cm_id *cm_id,
 	if (cm_id->state != IB_CM_DREQ_RCVD) {
 		spin_unlock_irqrestore(&cm_id_priv->lock, flags);
 		kfree(data);
+		pr_debug("%s: local_id %d, cm_idcm_id->state(%d) != IB_CM_DREQ_RCVD\n",
+			 __func__, be32_to_cpu(cm_id->local_id), cm_id->state);
 		return -EINVAL;
 	}
 
@@ -2494,6 +2516,9 @@ static int cm_dreq_handler(struct cm_work *work)
 		atomic_long_inc(&work->port->counter_group[CM_RECV_DUPLICATES].
 				counter[CM_DREQ_COUNTER]);
 		cm_issue_drep(work->port, work->mad_recv_wc);
+		pr_debug("%s: no cm_id_priv, local_comm_id %d, remote_comm_id %d\n",
+			 __func__, be32_to_cpu(dreq_msg->local_comm_id),
+			 be32_to_cpu(dreq_msg->remote_comm_id));
 		return -EINVAL;
 	}
 
@@ -2536,6 +2561,9 @@ static int cm_dreq_handler(struct cm_work *work)
 				counter[CM_DREQ_COUNTER]);
 		goto unlock;
 	default:
+		pr_debug("%s: local_id %d, cm_id_priv->id.state: %d\n",
+			 __func__, be32_to_cpu(cm_id_priv->id.local_id),
+			 cm_id_priv->id.state);
 		goto unlock;
 	}
 	cm_id_priv->id.state = IB_CM_DREQ_RCVD;
@@ -2639,6 +2667,8 @@ int ib_send_cm_rej(struct ib_cm_id *cm_id,
 		cm_enter_timewait(cm_id_priv);
 		break;
 	default:
+		pr_debug("%s: local_id %d, cm_id->state: %d\n", __func__,
+			 be32_to_cpu(cm_id_priv->id.local_id), cm_id->state);
 		ret = -EINVAL;
 		goto out;
 	}
@@ -2749,6 +2779,9 @@ static int cm_rej_handler(struct cm_work *work)
 		/* fall through */
 	default:
 		spin_unlock_irq(&cm_id_priv->lock);
+		pr_debug("%s: local_id %d, cm_id_priv->id.state: %d\n",
+			 __func__, be32_to_cpu(cm_id_priv->id.local_id),
+			 cm_id_priv->id.state);
 		ret = -EINVAL;
 		goto out;
 	}
@@ -2812,6 +2845,9 @@ int ib_send_cm_mra(struct ib_cm_id *cm_id,
 		}
 		/* fall through */
 	default:
+		pr_debug("%s: local_id %d, cm_id_priv->id.state: %d\n",
+			 __func__, be32_to_cpu(cm_id_priv->id.local_id),
+			 cm_id_priv->id.state);
 		ret = -EINVAL;
 		goto error1;
 	}
@@ -2913,6 +2949,9 @@ static int cm_mra_handler(struct cm_work *work)
 				counter[CM_MRA_COUNTER]);
 		/* fall through */
 	default:
+		pr_debug("%s local_id %d, cm_id_priv->id.state: %d\n",
+			 __func__, be32_to_cpu(cm_id_priv->id.local_id),
+			 cm_id_priv->id.state);
 		goto out;
 	}
 
@@ -3693,6 +3732,7 @@ static void cm_work_handler(struct work_struct *_work)
 		ret = cm_timewait_handler(work);
 		break;
 	default:
+		pr_debug("cm_event.event: 0x%x\n", work->cm_event.event);
 		ret = -EINVAL;
 		break;
 	}
@@ -3728,6 +3768,8 @@ static int cm_establish(struct ib_cm_id *cm_id)
 		ret = -EISCONN;
 		break;
 	default:
+		pr_debug("%s: local_id %d, cm_id->state: %d\n", __func__,
+			 be32_to_cpu(cm_id->local_id), cm_id->state);
 		ret = -EINVAL;
 		break;
 	}
@@ -3925,6 +3967,9 @@ static int cm_init_qp_init_attr(struct cm_id_private *cm_id_priv,
 		ret = 0;
 		break;
 	default:
+		pr_debug("%s: local_id %d, cm_id_priv->id.state: %d\n",
+			 __func__, be32_to_cpu(cm_id_priv->id.local_id),
+			 cm_id_priv->id.state);
 		ret = -EINVAL;
 		break;
 	}
@@ -3972,6 +4017,9 @@ static int cm_init_qp_rtr_attr(struct cm_id_private *cm_id_priv,
 		ret = 0;
 		break;
 	default:
+		pr_debug("%s: local_id %d, cm_id_priv->id.state: %d\n",
+			 __func__, be32_to_cpu(cm_id_priv->id.local_id),
+			 cm_id_priv->id.state);
 		ret = -EINVAL;
 		break;
 	}
@@ -4031,6 +4079,9 @@ static int cm_init_qp_rts_attr(struct cm_id_private *cm_id_priv,
 		ret = 0;
 		break;
 	default:
+		pr_debug("%s: local_id %d, cm_id_priv->id.state: %d\n",
+			 __func__, be32_to_cpu(cm_id_priv->id.local_id),
+			 cm_id_priv->id.state);
 		ret = -EINVAL;
 		break;
 	}
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 11/31] IB/cm: Add debug prints to ib_cm

[Or Gerlitz]

On Tue, Nov 14, 2017 at 2:51 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> Add debug prints to the error paths in the connection manager control
> flows, to help debug connection management problems.

Sean, the CM author and maintainer since 2.6.12/13 (12 years) is not copied



>
> Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/infiniband/core/cm.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> index fe7a5e0d79b0..cbb385e02947 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -1841,6 +1841,8 @@ static int cm_req_handler(struct cm_work *work)
>
>         listen_cm_id_priv = cm_match_req(work, cm_id_priv);
>         if (!listen_cm_id_priv) {
> +               pr_debug("%s: local_id %d, no listen_cm_id_priv\n", __func__,
> +                        be32_to_cpu(cm_id->local_id));
>                 ret = -EINVAL;
>                 goto free_timeinfo;
>         }
> @@ -1998,6 +2000,8 @@ int ib_send_cm_rep(struct ib_cm_id *cm_id,
>         spin_lock_irqsave(&cm_id_priv->lock, flags);
>         if (cm_id->state != IB_CM_REQ_RCVD &&
>             cm_id->state != IB_CM_MRA_REQ_SENT) {
> +               pr_debug("%s: local_comm_id %d, cm_id->state: %d\n", __func__,
> +                        be32_to_cpu(cm_id_priv->id.local_id), cm_id->state);
>                 ret = -EINVAL;
>                 goto out;
>         }
> @@ -2064,6 +2068,8 @@ int ib_send_cm_rtu(struct ib_cm_id *cm_id,
>         spin_lock_irqsave(&cm_id_priv->lock, flags);
>         if (cm_id->state != IB_CM_REP_RCVD &&
>             cm_id->state != IB_CM_MRA_REP_SENT) {
> +               pr_debug("%s: local_id %d, cm_id->state %d\n", __func__,
> +                        be32_to_cpu(cm_id->local_id), cm_id->state);
>                 ret = -EINVAL;
>                 goto error;
>         }
> @@ -2171,6 +2177,8 @@ static int cm_rep_handler(struct cm_work *work)
>         cm_id_priv = cm_acquire_id(rep_msg->remote_comm_id, 0);
>         if (!cm_id_priv) {
>                 cm_dup_rep_handler(work);
> +               pr_debug("%s: remote_comm_id %d, no cm_id_priv\n", __func__,
> +                        be32_to_cpu(rep_msg->remote_comm_id));
>                 return -EINVAL;
>         }
>
> @@ -2184,6 +2192,10 @@ static int cm_rep_handler(struct cm_work *work)
>         default:
>                 spin_unlock_irq(&cm_id_priv->lock);
>                 ret = -EINVAL;
> +               pr_debug("%s: cm_id_priv->id.state: %d, local_comm_id %d, remote_comm_id %d\n",
> +                        __func__, cm_id_priv->id.state,
> +                        be32_to_cpu(rep_msg->local_comm_id),
> +                        be32_to_cpu(rep_msg->remote_comm_id));
>                 goto error;
>         }
>
> @@ -2197,6 +2209,8 @@ static int cm_rep_handler(struct cm_work *work)
>                 spin_unlock(&cm.lock);
>                 spin_unlock_irq(&cm_id_priv->lock);
>                 ret = -EINVAL;
> +               pr_debug("%s: Failed to insert remote id %d\n", __func__,
> +                        be32_to_cpu(rep_msg->remote_comm_id));
>                 goto error;
>         }
>         /* Check for a stale connection. */
> @@ -2214,6 +2228,10 @@ static int cm_rep_handler(struct cm_work *work)
>                              IB_CM_REJ_STALE_CONN, CM_MSG_RESPONSE_REP,
>                              NULL, 0);
>                 ret = -EINVAL;
> +               pr_debug("%s: Stale connection. local_comm_id %d, remote_comm_id %d\n",
> +                        __func__, be32_to_cpu(rep_msg->local_comm_id),
> +                        be32_to_cpu(rep_msg->remote_comm_id));
> +
>                 if (cur_cm_id_priv) {
>                         cm_id = &cur_cm_id_priv->id;
>                         ib_send_cm_dreq(cm_id, NULL, 0);
> @@ -2360,6 +2378,8 @@ int ib_send_cm_dreq(struct ib_cm_id *cm_id,
>         cm_id_priv = container_of(cm_id, struct cm_id_private, id);
>         spin_lock_irqsave(&cm_id_priv->lock, flags);
>         if (cm_id->state != IB_CM_ESTABLISHED) {
> +               pr_debug("%s: local_id %d, cm_id->state: %d\n", __func__,
> +                        be32_to_cpu(cm_id->local_id), cm_id->state);
>                 ret = -EINVAL;
>                 goto out;
>         }
> @@ -2429,6 +2449,8 @@ int ib_send_cm_drep(struct ib_cm_id *cm_id,
>         if (cm_id->state != IB_CM_DREQ_RCVD) {
>                 spin_unlock_irqrestore(&cm_id_priv->lock, flags);
>                 kfree(data);
> +               pr_debug("%s: local_id %d, cm_idcm_id->state(%d) != IB_CM_DREQ_RCVD\n",
> +                        __func__, be32_to_cpu(cm_id->local_id), cm_id->state);
>                 return -EINVAL;
>         }
>
> @@ -2494,6 +2516,9 @@ static int cm_dreq_handler(struct cm_work *work)
>                 atomic_long_inc(&work->port->counter_group[CM_RECV_DUPLICATES].
>                                 counter[CM_DREQ_COUNTER]);
>                 cm_issue_drep(work->port, work->mad_recv_wc);
> +               pr_debug("%s: no cm_id_priv, local_comm_id %d, remote_comm_id %d\n",
> +                        __func__, be32_to_cpu(dreq_msg->local_comm_id),
> +                        be32_to_cpu(dreq_msg->remote_comm_id));
>                 return -EINVAL;
>         }
>
> @@ -2536,6 +2561,9 @@ static int cm_dreq_handler(struct cm_work *work)
>                                 counter[CM_DREQ_COUNTER]);
>                 goto unlock;
>         default:
> +               pr_debug("%s: local_id %d, cm_id_priv->id.state: %d\n",
> +                        __func__, be32_to_cpu(cm_id_priv->id.local_id),
> +                        cm_id_priv->id.state);
>                 goto unlock;
>         }
>         cm_id_priv->id.state = IB_CM_DREQ_RCVD;
> @@ -2639,6 +2667,8 @@ int ib_send_cm_rej(struct ib_cm_id *cm_id,
>                 cm_enter_timewait(cm_id_priv);
>                 break;
>         default:
> +               pr_debug("%s: local_id %d, cm_id->state: %d\n", __func__,
> +                        be32_to_cpu(cm_id_priv->id.local_id), cm_id->state);
>                 ret = -EINVAL;
>                 goto out;
>         }
> @@ -2749,6 +2779,9 @@ static int cm_rej_handler(struct cm_work *work)
>                 /* fall through */
>         default:
>                 spin_unlock_irq(&cm_id_priv->lock);
> +               pr_debug("%s: local_id %d, cm_id_priv->id.state: %d\n",
> +                        __func__, be32_to_cpu(cm_id_priv->id.local_id),
> +                        cm_id_priv->id.state);
>                 ret = -EINVAL;
>                 goto out;
>         }
> @@ -2812,6 +2845,9 @@ int ib_send_cm_mra(struct ib_cm_id *cm_id,
>                 }
>                 /* fall through */
>         default:
> +               pr_debug("%s: local_id %d, cm_id_priv->id.state: %d\n",
> +                        __func__, be32_to_cpu(cm_id_priv->id.local_id),
> +                        cm_id_priv->id.state);
>                 ret = -EINVAL;
>                 goto error1;
>         }
> @@ -2913,6 +2949,9 @@ static int cm_mra_handler(struct cm_work *work)
>                                 counter[CM_MRA_COUNTER]);
>                 /* fall through */
>         default:
> +               pr_debug("%s local_id %d, cm_id_priv->id.state: %d\n",
> +                        __func__, be32_to_cpu(cm_id_priv->id.local_id),
> +                        cm_id_priv->id.state);
>                 goto out;
>         }
>
> @@ -3693,6 +3732,7 @@ static void cm_work_handler(struct work_struct *_work)
>                 ret = cm_timewait_handler(work);
>                 break;
>         default:
> +               pr_debug("cm_event.event: 0x%x\n", work->cm_event.event);
>                 ret = -EINVAL;
>                 break;
>         }
> @@ -3728,6 +3768,8 @@ static int cm_establish(struct ib_cm_id *cm_id)
>                 ret = -EISCONN;
>                 break;
>         default:
> +               pr_debug("%s: local_id %d, cm_id->state: %d\n", __func__,
> +                        be32_to_cpu(cm_id->local_id), cm_id->state);
>                 ret = -EINVAL;
>                 break;
>         }
> @@ -3925,6 +3967,9 @@ static int cm_init_qp_init_attr(struct cm_id_private *cm_id_priv,
>                 ret = 0;
>                 break;
>         default:
> +               pr_debug("%s: local_id %d, cm_id_priv->id.state: %d\n",
> +                        __func__, be32_to_cpu(cm_id_priv->id.local_id),
> +                        cm_id_priv->id.state);
>                 ret = -EINVAL;
>                 break;
>         }
> @@ -3972,6 +4017,9 @@ static int cm_init_qp_rtr_attr(struct cm_id_private *cm_id_priv,
>                 ret = 0;
>                 break;
>         default:
> +               pr_debug("%s: local_id %d, cm_id_priv->id.state: %d\n",
> +                        __func__, be32_to_cpu(cm_id_priv->id.local_id),
> +                        cm_id_priv->id.state);
>                 ret = -EINVAL;
>                 break;
>         }
> @@ -4031,6 +4079,9 @@ static int cm_init_qp_rts_attr(struct cm_id_private *cm_id_priv,
>                 ret = 0;
>                 break;
>         default:
> +               pr_debug("%s: local_id %d, cm_id_priv->id.state: %d\n",
> +                        __func__, be32_to_cpu(cm_id_priv->id.local_id),
> +                        cm_id_priv->id.state);
>                 ret = -EINVAL;
>                 break;
>         }
> --
> 2.15.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 12/31] IB/umem: Fix use of npages/nmap fields

[Leon Romanovsky]

From: Artemy Kovalyov <artemyko-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

In ib_umem structure npages holds original number of sg entries, while
nmap is number of DMA blocks returned by dma_map_sg.

Fixes: c5d76f130b28 ('IB/core: Add umem function to read data from user-space')
Signed-off-by: Artemy Kovalyov <artemyko-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/umem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
index 21e60b1e2ff4..c20f6bf587ec 100644
--- a/drivers/infiniband/core/umem.c
+++ b/drivers/infiniband/core/umem.c
@@ -352,7 +352,7 @@ int ib_umem_copy_from(void *dst, struct ib_umem *umem, size_t offset,
 		return -EINVAL;
 	}
 
-	ret = sg_pcopy_to_buffer(umem->sg_head.sgl, umem->nmap, dst, length,
+	ret = sg_pcopy_to_buffer(umem->sg_head.sgl, umem->npages, dst, length,
 				 offset + ib_umem_offset(umem));
 
 	if (ret < 0)
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 13/31] RDMA/cma: Set default GID type as RoCE when resolving RoCE route

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

As the function name suggests cma_resolve_iboe_route() resolves RoCE
route. However, its default GID type is IB_GID_TYPE_IB and not
IB_GID_TYPE_ROCE, even though both are mapped to the same enum value.
Change default GID type to IB_GID_TYPE_ROCE.

cma_iboe_set_mgid() is updated to reflect the RoCEv2 GID check.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cma.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 9841e7ca5c06..e63a51b68ed5 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2528,7 +2528,7 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 	struct cma_work *work;
 	int ret;
 	struct net_device *ndev = NULL;
-	enum ib_gid_type gid_type = IB_GID_TYPE_IB;
+	enum ib_gid_type gid_type = IB_GID_TYPE_ROCE;
 	u8 default_roce_tos = id_priv->cma_dev->default_roce_tos[id_priv->id.port_num -
 					rdma_start_port(id_priv->cma_dev->device)];
 	u8 tos = id_priv->tos_set ? id_priv->tos : default_roce_tos;
@@ -4009,8 +4009,10 @@ static void cma_iboe_set_mgid(struct sockaddr *addr, union ib_gid *mgid,
 	} else if (addr->sa_family == AF_INET6) {
 		memcpy(mgid, &sin6->sin6_addr, sizeof *mgid);
 	} else {
-		mgid->raw[0] = (gid_type == IB_GID_TYPE_IB) ? 0xff : 0;
-		mgid->raw[1] = (gid_type == IB_GID_TYPE_IB) ? 0x0e : 0;
+		mgid->raw[0] =
+			(gid_type == IB_GID_TYPE_ROCE_UDP_ENCAP) ? 0 : 0xff;
+		mgid->raw[1] =
+			(gid_type == IB_GID_TYPE_ROCE_UDP_ENCAP) ? 0 : 0x0e;
 		mgid->raw[2] = 0;
 		mgid->raw[3] = 0;
 		mgid->raw[4] = 0;
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 14/31] RDMA/cma: Simplify netdev check

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Current code checks for NULL ndev twice where 2nd check is always
invalid given the fact that during route resolving stage, device address
must be bound to netdevice interface.

This patch simplifies such check.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cma.c | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index e63a51b68ed5..168f1230756b 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2527,8 +2527,10 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 	struct rdma_addr *addr = &route->addr;
 	struct cma_work *work;
 	int ret;
-	struct net_device *ndev = NULL;
+	struct net_device *ndev;
 	enum ib_gid_type gid_type = IB_GID_TYPE_ROCE;
+	unsigned long supported_gids;
+
 	u8 default_roce_tos = id_priv->cma_dev->default_roce_tos[id_priv->id.port_num -
 					rdma_start_port(id_priv->cma_dev->device)];
 	u8 tos = id_priv->tos_set ? id_priv->tos : default_roce_tos;
@@ -2549,30 +2551,25 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 
 	route->num_paths = 1;
 
-	if (addr->dev_addr.bound_dev_if) {
-		unsigned long supported_gids;
-
-		ndev = dev_get_by_index(&init_net, addr->dev_addr.bound_dev_if);
-		if (!ndev) {
-			ret = -ENODEV;
-			goto err2;
-		}
-
-		supported_gids = roce_gid_type_mask_support(id_priv->id.device,
-							    id_priv->id.port_num);
-		gid_type = cma_route_gid_type(addr->dev_addr.network,
-					      supported_gids,
-					      id_priv->gid_type);
-		route->path_rec->rec_type =
-			sa_conv_gid_to_pathrec_type(gid_type);
-		sa_path_set_ndev(route->path_rec, &init_net);
-		sa_path_set_ifindex(route->path_rec, ndev->ifindex);
+	if (!addr->dev_addr.bound_dev_if) {
+		ret = -ENODEV;
+		goto err2;
 	}
+
+	ndev = dev_get_by_index(&init_net, addr->dev_addr.bound_dev_if);
 	if (!ndev) {
 		ret = -ENODEV;
 		goto err2;
 	}
 
+	supported_gids = roce_gid_type_mask_support(id_priv->id.device,
+						    id_priv->id.port_num);
+	gid_type = cma_route_gid_type(addr->dev_addr.network,
+				      supported_gids,
+				      id_priv->gid_type);
+	route->path_rec->rec_type = sa_conv_gid_to_pathrec_type(gid_type);
+	sa_path_set_ndev(route->path_rec, &init_net);
+	sa_path_set_ifindex(route->path_rec, ndev->ifindex);
 	sa_path_set_dmac(route->path_rec, addr->dev_addr.dst_dev_addr);
 
 	rdma_ip2gid((struct sockaddr *)&id_priv->id.route.addr.src_addr,
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 14/31] RDMA/cma: Simplify netdev check

[Knut Omang]

On Tue, 2017-11-14 at 14:52 +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Current code checks for NULL ndev twice where 2nd check is always
> invalid given the fact that during route resolving stage, device address
> must be bound to netdevice interface.
> 
> This patch simplifies such check.
> 
> Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/infiniband/core/cma.c | 35 ++++++++++++++++-------------------
>  1 file changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index e63a51b68ed5..168f1230756b 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -2527,8 +2527,10 @@ static int cma_resolve_iboe_route(struct rdma_id_private
> *id_priv)
>  	struct rdma_addr *addr = &route->addr;
>  	struct cma_work *work;
>  	int ret;
> -	struct net_device *ndev = NULL;
> +	struct net_device *ndev;
>  	enum ib_gid_type gid_type = IB_GID_TYPE_ROCE;
> +	unsigned long supported_gids;
> +
>  	u8 default_roce_tos = id_priv->cma_dev->default_roce_tos[id_priv->id.port_num -
>  					rdma_start_port(id_priv->cma_dev->device)];
>  	u8 tos = id_priv->tos_set ? id_priv->tos : default_roce_tos;
> @@ -2549,30 +2551,25 @@ static int cma_resolve_iboe_route(struct rdma_id_private
> *id_priv)
>  
>  	route->num_paths = 1;
>  
> -	if (addr->dev_addr.bound_dev_if) {
> -		unsigned long supported_gids;
> -
> -		ndev = dev_get_by_index(&init_net, addr->dev_addr.bound_dev_if);
> -		if (!ndev) {
> -			ret = -ENODEV;
> -			goto err2;
> -		}
> -
> -		supported_gids = roce_gid_type_mask_support(id_priv->id.device,
> -							    id_priv->id.port_num);
> -		gid_type = cma_route_gid_type(addr->dev_addr.network,
> -					      supported_gids,
> -					      id_priv->gid_type);
> -		route->path_rec->rec_type =
> -			sa_conv_gid_to_pathrec_type(gid_type);
> -		sa_path_set_ndev(route->path_rec, &init_net);
> -		sa_path_set_ifindex(route->path_rec, ndev->ifindex);
> +	if (!addr->dev_addr.bound_dev_if) {
> +		ret = -ENODEV;
> +		goto err2;
>  	}
> +
> +	ndev = dev_get_by_index(&init_net, addr->dev_addr.bound_dev_if);
>  	if (!ndev) {
>  		ret = -ENODEV;
>  		goto err2;
>  	}
>  
> +	supported_gids = roce_gid_type_mask_support(id_priv->id.device,
> +						    id_priv->id.port_num);
> +	gid_type = cma_route_gid_type(addr->dev_addr.network,
> +				      supported_gids,
> +				      id_priv->gid_type);
> +	route->path_rec->rec_type = sa_conv_gid_to_pathrec_type(gid_type);
> +	sa_path_set_ndev(route->path_rec, &init_net);
> +	sa_path_set_ifindex(route->path_rec, ndev->ifindex);
>  	sa_path_set_dmac(route->path_rec, addr->dev_addr.dst_dev_addr);
>  
>  	rdma_ip2gid((struct sockaddr *)&id_priv->id.route.addr.src_addr,

Reviewed-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Knut
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 14/31] RDMA/cma: Simplify netdev check

[Jason Gunthorpe]

On Fri, Jan 19, 2018 at 09:30:59AM +0100, Knut Omang wrote:

> >  	rdma_ip2gid((struct sockaddr *)&id_priv->id.route.addr.src_addr,
> 
> Reviewed-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Hey Knut,

You are a bit lat here, these patches were all applied in Novemeber :)

I welcome your reviews, and recommend you check patchworks for what is
current?

https://patchwork.kernel.org/project/linux-rdma/list/

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 15/31] RDMA/cma: Avoid setting path record type twice

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Avoid setting path record type twice for RoCE.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cma.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 168f1230756b..f986597201fa 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2567,7 +2567,11 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 	gid_type = cma_route_gid_type(addr->dev_addr.network,
 				      supported_gids,
 				      id_priv->gid_type);
+	/* Use the hint from IP Stack to select GID Type */
+	if (gid_type < ib_network_to_gid_type(addr->dev_addr.network))
+		gid_type = ib_network_to_gid_type(addr->dev_addr.network);
 	route->path_rec->rec_type = sa_conv_gid_to_pathrec_type(gid_type);
+
 	sa_path_set_ndev(route->path_rec, &init_net);
 	sa_path_set_ifindex(route->path_rec, ndev->ifindex);
 	sa_path_set_dmac(route->path_rec, addr->dev_addr.dst_dev_addr);
@@ -2577,11 +2581,6 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 	rdma_ip2gid((struct sockaddr *)&id_priv->id.route.addr.dst_addr,
 		    &route->path_rec->dgid);
 
-	/* Use the hint from IP Stack to select GID Type */
-	if (gid_type < ib_network_to_gid_type(addr->dev_addr.network))
-		gid_type = ib_network_to_gid_type(addr->dev_addr.network);
-	route->path_rec->rec_type = sa_conv_gid_to_pathrec_type(gid_type);
-
 	if (((struct sockaddr *)&id_priv->id.route.addr.dst_addr)->sa_family != AF_IB)
 		/* TODO: get the hoplimit from the inet/inet6 device */
 		route->path_rec->hop_limit = addr->dev_addr.hoplimit;
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 15/31] RDMA/cma: Avoid setting path record type twice

[Knut Omang]

On Tue, 2017-11-14 at 14:52 +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Avoid setting path record type twice for RoCE.
> 
> Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/infiniband/core/cma.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 168f1230756b..f986597201fa 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -2567,7 +2567,11 @@ static int cma_resolve_iboe_route(struct rdma_id_private
> *id_priv)
>  	gid_type = cma_route_gid_type(addr->dev_addr.network,
>  				      supported_gids,
>  				      id_priv->gid_type);
> +	/* Use the hint from IP Stack to select GID Type */
> +	if (gid_type < ib_network_to_gid_type(addr->dev_addr.network))
> +		gid_type = ib_network_to_gid_type(addr->dev_addr.network);
>  	route->path_rec->rec_type = sa_conv_gid_to_pathrec_type(gid_type);
> +
>  	sa_path_set_ndev(route->path_rec, &init_net);
>  	sa_path_set_ifindex(route->path_rec, ndev->ifindex);
>  	sa_path_set_dmac(route->path_rec, addr->dev_addr.dst_dev_addr);
> @@ -2577,11 +2581,6 @@ static int cma_resolve_iboe_route(struct rdma_id_private
> *id_priv)
>  	rdma_ip2gid((struct sockaddr *)&id_priv->id.route.addr.dst_addr,
>  		    &route->path_rec->dgid);
>  
> -	/* Use the hint from IP Stack to select GID Type */
> -	if (gid_type < ib_network_to_gid_type(addr->dev_addr.network))
> -		gid_type = ib_network_to_gid_type(addr->dev_addr.network);
> -	route->path_rec->rec_type = sa_conv_gid_to_pathrec_type(gid_type);
> -
>  	if (((struct sockaddr *)&id_priv->id.route.addr.dst_addr)->sa_family != AF_IB)
>  		/* TODO: get the hoplimit from the inet/inet6 device */
>  		route->path_rec->hop_limit = addr->dev_addr.hoplimit;

Reviewed-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Knut
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 16/31] RDMA/cma: Introduce and use helper functions to init work

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Introduce and user helper functions to initialize work for address
resolved and route resolved event that avoid code duplication at few
places.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cma.c | 53 ++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 28 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index f986597201fa..48f593bafe33 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2413,6 +2413,26 @@ static void cma_ndev_work_handler(struct work_struct *_work)
 	kfree(work);
 }
 
+static void cma_init_resolve_route_work(struct cma_work *work,
+					struct rdma_id_private *id_priv)
+{
+	work->id = id_priv;
+	INIT_WORK(&work->work, cma_work_handler);
+	work->old_state = RDMA_CM_ROUTE_QUERY;
+	work->new_state = RDMA_CM_ROUTE_RESOLVED;
+	work->event.event = RDMA_CM_EVENT_ROUTE_RESOLVED;
+}
+
+static void cma_init_resolve_addr_work(struct cma_work *work,
+				       struct rdma_id_private *id_priv)
+{
+	work->id = id_priv;
+	INIT_WORK(&work->work, cma_work_handler);
+	work->old_state = RDMA_CM_ADDR_QUERY;
+	work->new_state = RDMA_CM_ADDR_RESOLVED;
+	work->event.event = RDMA_CM_EVENT_ADDR_RESOLVED;
+}
+
 static int cma_resolve_ib_route(struct rdma_id_private *id_priv, int timeout_ms)
 {
 	struct rdma_route *route = &id_priv->id.route;
@@ -2423,11 +2443,7 @@ static int cma_resolve_ib_route(struct rdma_id_private *id_priv, int timeout_ms)
 	if (!work)
 		return -ENOMEM;
 
-	work->id = id_priv;
-	INIT_WORK(&work->work, cma_work_handler);
-	work->old_state = RDMA_CM_ROUTE_QUERY;
-	work->new_state = RDMA_CM_ROUTE_RESOLVED;
-	work->event.event = RDMA_CM_EVENT_ROUTE_RESOLVED;
+	cma_init_resolve_route_work(work, id_priv);
 
 	route->path_rec = kmalloc(sizeof *route->path_rec, GFP_KERNEL);
 	if (!route->path_rec) {
@@ -2482,11 +2498,7 @@ static int cma_resolve_iw_route(struct rdma_id_private *id_priv, int timeout_ms)
 	if (!work)
 		return -ENOMEM;
 
-	work->id = id_priv;
-	INIT_WORK(&work->work, cma_work_handler);
-	work->old_state = RDMA_CM_ROUTE_QUERY;
-	work->new_state = RDMA_CM_ROUTE_RESOLVED;
-	work->event.event = RDMA_CM_EVENT_ROUTE_RESOLVED;
+	cma_init_resolve_route_work(work, id_priv);
 	queue_work(cma_wq, &work->work);
 	return 0;
 }
@@ -2540,9 +2552,6 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 	if (!work)
 		return -ENOMEM;
 
-	work->id = id_priv;
-	INIT_WORK(&work->work, cma_work_handler);
-
 	route->path_rec = kzalloc(sizeof *route->path_rec, GFP_KERNEL);
 	if (!route->path_rec) {
 		ret = -ENOMEM;
@@ -2602,11 +2611,7 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 		goto err2;
 	}
 
-	work->old_state = RDMA_CM_ROUTE_QUERY;
-	work->new_state = RDMA_CM_ROUTE_RESOLVED;
-	work->event.event = RDMA_CM_EVENT_ROUTE_RESOLVED;
-	work->event.status = 0;
-
+	cma_init_resolve_route_work(work, id_priv);
 	queue_work(cma_wq, &work->work);
 
 	return 0;
@@ -2786,11 +2791,7 @@ static int cma_resolve_loopback(struct rdma_id_private *id_priv)
 	rdma_addr_get_sgid(&id_priv->id.route.addr.dev_addr, &gid);
 	rdma_addr_set_dgid(&id_priv->id.route.addr.dev_addr, &gid);
 
-	work->id = id_priv;
-	INIT_WORK(&work->work, cma_work_handler);
-	work->old_state = RDMA_CM_ADDR_QUERY;
-	work->new_state = RDMA_CM_ADDR_RESOLVED;
-	work->event.event = RDMA_CM_EVENT_ADDR_RESOLVED;
+	cma_init_resolve_addr_work(work, id_priv);
 	queue_work(cma_wq, &work->work);
 	return 0;
 err:
@@ -2816,11 +2817,7 @@ static int cma_resolve_ib_addr(struct rdma_id_private *id_priv)
 	rdma_addr_set_dgid(&id_priv->id.route.addr.dev_addr, (union ib_gid *)
 		&(((struct sockaddr_ib *) &id_priv->id.route.addr.dst_addr)->sib_addr));
 
-	work->id = id_priv;
-	INIT_WORK(&work->work, cma_work_handler);
-	work->old_state = RDMA_CM_ADDR_QUERY;
-	work->new_state = RDMA_CM_ADDR_RESOLVED;
-	work->event.event = RDMA_CM_EVENT_ADDR_RESOLVED;
+	cma_init_resolve_addr_work(work, id_priv);
 	queue_work(cma_wq, &work->work);
 	return 0;
 err:
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 17/31] IB/core: Avoid unnecessary type cast

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Type cast from void to struct find_gid_index_context is not needed.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/verbs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 584b6029cb44..0cac32736189 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -421,8 +421,7 @@ static bool find_gid_index(const union ib_gid *gid,
 			   const struct ib_gid_attr *gid_attr,
 			   void *context)
 {
-	struct find_gid_index_context *ctx =
-		(struct find_gid_index_context *)context;
+	struct find_gid_index_context *ctx = context;
 
 	if (ctx->gid_type != gid_attr->gid_type)
 		return false;
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 18/31] IB/core: Refactor to avoid unnecessary check on GID lookup miss

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Currently on every gid entry comparison miss found variable is checked;
which is not needed as those two comparison fail already indicate that
GID is not found yet.
So refactor to avoid such check and copy the GID index when found.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cache.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
index e5bba898741d..72643d5a7f6d 100644
--- a/drivers/infiniband/core/cache.c
+++ b/drivers/infiniband/core/cache.c
@@ -573,27 +573,24 @@ static int ib_cache_gid_find_by_filter(struct ib_device *ib_dev,
 		struct ib_gid_attr attr;
 
 		if (table->data_vec[i].props & GID_TABLE_ENTRY_INVALID)
-			goto next;
+			continue;
 
 		if (memcmp(gid, &table->data_vec[i].gid, sizeof(*gid)))
-			goto next;
+			continue;
 
 		memcpy(&attr, &table->data_vec[i].attr, sizeof(attr));
 
-		if (filter(gid, &attr, context))
+		if (filter(gid, &attr, context)) {
 			found = true;
-
-next:
-		if (found)
+			if (index)
+				*index = i;
 			break;
+		}
 	}
 	read_unlock_irqrestore(&table->rwlock, flags);
 
 	if (!found)
 		return -ENOENT;
-
-	if (index)
-		*index = i;
 	return 0;
 }
 
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 19/31] IB/rxe: Avoid passing unused index pointer which is optional

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

While searching for GID, returned index is not used, so avoid passing
pointer during invocation.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_recv.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
index fb8c83e055e1..4c3f899241d4 100644
--- a/drivers/infiniband/sw/rxe/rxe_recv.c
+++ b/drivers/infiniband/sw/rxe/rxe_recv.c
@@ -336,7 +336,6 @@ static int rxe_match_dgid(struct rxe_dev *rxe, struct sk_buff *skb)
 {
 	union ib_gid dgid;
 	union ib_gid *pdgid;
-	u16 index;
 
 	if (skb->protocol == htons(ETH_P_IP)) {
 		ipv6_addr_set_v4mapped(ip_hdr(skb)->daddr,
@@ -348,7 +347,7 @@ static int rxe_match_dgid(struct rxe_dev *rxe, struct sk_buff *skb)
 
 	return ib_find_cached_gid_by_port(&rxe->ib_dev, pdgid,
 					  IB_GID_TYPE_ROCE_UDP_ENCAP,
-					  1, rxe->ndev, &index);
+					  1, rxe->ndev, NULL);
 }
 
 /* rxe_rcv is called from the interface driver */
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 20/31] IB/core: Avoid exporting module internal ib_find_gid_by_filter()

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

ib_find_gid_by_filter() is used only by ib_core, therefore avoid
exporting it.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cache.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
index 72643d5a7f6d..e56f9d6893ee 100644
--- a/drivers/infiniband/core/cache.c
+++ b/drivers/infiniband/core/cache.c
@@ -889,7 +889,6 @@ int ib_find_gid_by_filter(struct ib_device *device,
 					   port_num, filter,
 					   context, index);
 }
-EXPORT_SYMBOL(ib_find_gid_by_filter);
 
 int ib_get_cached_pkey(struct ib_device *device,
 		       u8                port_num,
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 21/31] RDMA/core: Avoid redundant memcpy in resolving address

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

rdma_resolve_ip works on the rdma_dev_addr pointer even from worker
thread context. Therefore there is no need to store that in context.
At the same time, resolve_cb() callback function invokes memcpy on
source and destination addresses which are essentially same.
Therefore both are removed.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/addr.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index da4469c38eac..6679dd43fe08 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -744,7 +744,6 @@ void rdma_addr_cancel(struct rdma_dev_addr *addr)
 EXPORT_SYMBOL(rdma_addr_cancel);
 
 struct resolve_cb_context {
-	struct rdma_dev_addr *addr;
 	struct completion comp;
 	int status;
 };
@@ -752,9 +751,6 @@ struct resolve_cb_context {
 static void resolve_cb(int status, struct sockaddr *src_addr,
 	     struct rdma_dev_addr *addr, void *context)
 {
-	if (!status)
-		memcpy(((struct resolve_cb_context *)context)->addr,
-		       addr, sizeof(struct rdma_dev_addr));
 	((struct resolve_cb_context *)context)->status = status;
 	complete(&((struct resolve_cb_context *)context)->comp);
 }
@@ -780,7 +776,6 @@ int rdma_addr_find_l2_eth_by_grh(const union ib_gid *sgid,
 	dev_addr.bound_dev_if = ndev->ifindex;
 	dev_addr.net = &init_net;
 
-	ctx.addr = &dev_addr;
 	init_completion(&ctx.comp);
 	ret = rdma_resolve_ip(&self, &sgid_addr._sockaddr, &dgid_addr._sockaddr,
 			&dev_addr, 1000, resolve_cb, &ctx);
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 22/31] IB/core: Removed unused function

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

rdma_addr_find_smac_by_sgid() is exported symbol not used by any kernel
module. Therefore its removed.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/addr.c | 23 -----------------------
 include/rdma/ib_addr.h         |  1 -
 2 files changed, 24 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 6679dd43fe08..e0086901d64c 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -793,29 +793,6 @@ int rdma_addr_find_l2_eth_by_grh(const union ib_gid *sgid,
 	return 0;
 }
 
-int rdma_addr_find_smac_by_sgid(union ib_gid *sgid, u8 *smac, u16 *vlan_id)
-{
-	int ret = 0;
-	struct rdma_dev_addr dev_addr;
-	union {
-		struct sockaddr     _sockaddr;
-		struct sockaddr_in  _sockaddr_in;
-		struct sockaddr_in6 _sockaddr_in6;
-	} gid_addr;
-
-	rdma_gid2ip(&gid_addr._sockaddr, sgid);
-
-	memset(&dev_addr, 0, sizeof(dev_addr));
-	dev_addr.net = &init_net;
-	ret = rdma_translate_ip(&gid_addr._sockaddr, &dev_addr, vlan_id);
-	if (ret)
-		return ret;
-
-	memcpy(smac, dev_addr.src_dev_addr, ETH_ALEN);
-	return ret;
-}
-EXPORT_SYMBOL(rdma_addr_find_smac_by_sgid);
-
 static int netevent_callback(struct notifier_block *self, unsigned long event,
 	void *ctx)
 {
diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
index d5c3bbb84608..3d17073ea338 100644
--- a/include/rdma/ib_addr.h
+++ b/include/rdma/ib_addr.h
@@ -131,7 +131,6 @@ void rdma_copy_addr(struct rdma_dev_addr *dev_addr,
 
 int rdma_addr_size(struct sockaddr *addr);
 
-int rdma_addr_find_smac_by_sgid(union ib_gid *sgid, u8 *smac, u16 *vlan_id);
 int rdma_addr_find_l2_eth_by_grh(const union ib_gid *sgid,
 				 const union ib_gid *dgid,
 				 u8 *dmac, const struct net_device *ndev,
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 23/31] RDMA/{core, cma}: Simplify rdma_translate_ip

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Since no caller needs vlan, rdma_translate_ip is simplified to avoid
vlan pointer.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/addr.c | 11 +++--------
 drivers/infiniband/core/cma.c  |  4 ++--
 include/rdma/ib_addr.h         |  2 +-
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index e0086901d64c..0ae18608761c 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -243,8 +243,7 @@ void rdma_copy_addr(struct rdma_dev_addr *dev_addr,
 EXPORT_SYMBOL(rdma_copy_addr);
 
 int rdma_translate_ip(const struct sockaddr *addr,
-		      struct rdma_dev_addr *dev_addr,
-		      u16 *vlan_id)
+		      struct rdma_dev_addr *dev_addr)
 {
 	struct net_device *dev;
 
@@ -267,8 +266,6 @@ int rdma_translate_ip(const struct sockaddr *addr,
 
 		rdma_copy_addr(dev_addr, dev, NULL);
 		dev_addr->bound_dev_if = dev->ifindex;
-		if (vlan_id)
-			*vlan_id = rdma_vlan_dev_vlan_id(dev);
 		dev_put(dev);
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
@@ -280,8 +277,6 @@ int rdma_translate_ip(const struct sockaddr *addr,
 					  dev, 1)) {
 				rdma_copy_addr(dev_addr, dev, NULL);
 				dev_addr->bound_dev_if = dev->ifindex;
-				if (vlan_id)
-					*vlan_id = rdma_vlan_dev_vlan_id(dev);
 				break;
 			}
 		}
@@ -481,7 +476,7 @@ static int addr_resolve_neigh(struct dst_entry *dst,
 	if (dst->dev->flags & IFF_LOOPBACK) {
 		int ret;
 
-		ret = rdma_translate_ip(dst_in, addr, NULL);
+		ret = rdma_translate_ip(dst_in, addr);
 		if (!ret)
 			memcpy(addr->dst_dev_addr, addr->src_dev_addr,
 			       MAX_ADDR_LEN);
@@ -558,7 +553,7 @@ static int addr_resolve(struct sockaddr *src_in,
 	}
 
 	if (ndev->flags & IFF_LOOPBACK) {
-		ret = rdma_translate_ip(dst_in, addr, NULL);
+		ret = rdma_translate_ip(dst_in, addr);
 		/*
 		 * Put the loopback device and get the translated
 		 * device instead.
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 48f593bafe33..19769ea4d71f 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -601,7 +601,7 @@ static int cma_translate_addr(struct sockaddr *addr, struct rdma_dev_addr *dev_a
 	int ret;
 
 	if (addr->sa_family != AF_IB) {
-		ret = rdma_translate_ip(addr, dev_addr, NULL);
+		ret = rdma_translate_ip(addr, dev_addr);
 	} else {
 		cma_translate_ib((struct sockaddr_ib *) addr, dev_addr);
 		ret = 0;
@@ -2131,7 +2131,7 @@ static int iw_conn_req_handler(struct iw_cm_id *cm_id,
 	mutex_lock_nested(&conn_id->handler_mutex, SINGLE_DEPTH_NESTING);
 	conn_id->state = RDMA_CM_CONNECT;
 
-	ret = rdma_translate_ip(laddr, &conn_id->id.route.addr.dev_addr, NULL);
+	ret = rdma_translate_ip(laddr, &conn_id->id.route.addr.dev_addr);
 	if (ret) {
 		mutex_unlock(&conn_id->handler_mutex);
 		rdma_destroy_id(new_cm_id);
diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
index 3d17073ea338..dc0b642e0175 100644
--- a/include/rdma/ib_addr.h
+++ b/include/rdma/ib_addr.h
@@ -94,7 +94,7 @@ struct rdma_dev_addr {
  * The dev_addr->net field must be initialized.
  */
 int rdma_translate_ip(const struct sockaddr *addr,
-		      struct rdma_dev_addr *dev_addr, u16 *vlan_id);
+		      struct rdma_dev_addr *dev_addr);
 
 /**
  * rdma_resolve_ip - Resolve source and destination IP addresses to
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 24/31] RDMA/core: Avoid copying ifindex twice

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

rdma_copy_addr copies the ifndex to bound_dev_if.
Therefore avoid copying it again after rdma_copy_addr call is completed.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/addr.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 0ae18608761c..a5b4cf030c11 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -265,7 +265,6 @@ int rdma_translate_ip(const struct sockaddr *addr,
 			return -EADDRNOTAVAIL;
 
 		rdma_copy_addr(dev_addr, dev, NULL);
-		dev_addr->bound_dev_if = dev->ifindex;
 		dev_put(dev);
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
@@ -276,7 +275,6 @@ int rdma_translate_ip(const struct sockaddr *addr,
 					  &((const struct sockaddr_in6 *)addr)->sin6_addr,
 					  dev, 1)) {
 				rdma_copy_addr(dev_addr, dev, NULL);
-				dev_addr->bound_dev_if = dev->ifindex;
 				break;
 			}
 		}
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 25/31] IB/{core, ipoib}: Simplify ib_find_gid to search only for IB link layer

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Currently there are no users of ib_find_gid for RoCE transport. It is
only used by IPoIB.
Therefore its simplified to ignore RoCE ports and GID type check which
was previously done for every port.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/device.c        | 16 +++-------------
 drivers/infiniband/ulp/ipoib/ipoib_ib.c |  3 +--
 include/rdma/ib_verbs.h                 |  3 +--
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 84fc32a2c8b3..bb2686d56d3c 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -1017,32 +1017,22 @@ EXPORT_SYMBOL(ib_modify_port);
 
 /**
  * ib_find_gid - Returns the port number and GID table index where
- *   a specified GID value occurs.
+ *   a specified GID value occurs. Its searches only for IB link layer.
  * @device: The device to query.
  * @gid: The GID value to search for.
- * @gid_type: Type of GID.
  * @ndev: The ndev related to the GID to search for.
  * @port_num: The port number of the device where the GID value was found.
  * @index: The index into the GID table where the GID was found.  This
  *   parameter may be NULL.
  */
 int ib_find_gid(struct ib_device *device, union ib_gid *gid,
-		enum ib_gid_type gid_type, struct net_device *ndev,
-		u8 *port_num, u16 *index)
+		struct net_device *ndev, u8 *port_num, u16 *index)
 {
 	union ib_gid tmp_gid;
 	int ret, port, i;
 
 	for (port = rdma_start_port(device); port <= rdma_end_port(device); ++port) {
-		if (rdma_cap_roce_gid_table(device, port)) {
-			if (!ib_find_cached_gid_by_port(device, gid, gid_type, port,
-							ndev, index)) {
-				*port_num = port;
-				return 0;
-			}
-		}
-
-		if (gid_type != IB_GID_TYPE_IB)
+		if (rdma_cap_roce_gid_table(device, port))
 			continue;
 
 		for (i = 0; i < device->port_immutable[port].gid_tbl_len; ++i) {
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
index 3b96cdaf9a83..b9fa21790da9 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
@@ -1085,8 +1085,7 @@ static bool ipoib_dev_addr_changed_valid(struct ipoib_dev_priv *priv)
 
 	netif_addr_unlock_bh(priv->dev);
 
-	err = ib_find_gid(priv->ca, &search_gid, IB_GID_TYPE_IB,
-			  priv->dev, &port, &index);
+	err = ib_find_gid(priv->ca, &search_gid, priv->dev, &port, &index);
 
 	netif_addr_lock_bh(priv->dev);
 
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index fd84cda5ed7c..cfd837049a32 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2836,8 +2836,7 @@ int ib_modify_port(struct ib_device *device,
 		   struct ib_port_modify *port_modify);
 
 int ib_find_gid(struct ib_device *device, union ib_gid *gid,
-		enum ib_gid_type gid_type, struct net_device *ndev,
-		u8 *port_num, u16 *index);
+		struct net_device *ndev, u8 *port_num, u16 *index);
 
 int ib_find_pkey(struct ib_device *device,
 		 u8 port_num, u16 pkey, u16 *index);
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 26/31] IB/{cm, umad}: Fix honor av init error

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

cm_init_av_for_response depends on ib_init_ah_from_wc() whose return
status is ignored.
ib_init_ah_from_wc() can fail and its return status should be honored as
done in this patch.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cm.c       | 34 +++++++++++++++++++++-------------
 drivers/infiniband/core/user_mad.c | 10 +++++++---
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index cbb385e02947..9a60d21ed0c3 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -452,13 +452,13 @@ static void cm_set_private_data(struct cm_id_private *cm_id_priv,
 	cm_id_priv->private_data_len = private_data_len;
 }
 
-static void cm_init_av_for_response(struct cm_port *port, struct ib_wc *wc,
-				    struct ib_grh *grh, struct cm_av *av)
+static int cm_init_av_for_response(struct cm_port *port, struct ib_wc *wc,
+				   struct ib_grh *grh, struct cm_av *av)
 {
 	av->port = port;
 	av->pkey_index = wc->pkey_index;
-	ib_init_ah_from_wc(port->cm_dev->ib_device, port->port_num, wc,
-			   grh, &av->ah_attr);
+	return ib_init_ah_from_wc(port->cm_dev->ib_device, port->port_num, wc,
+				  grh, &av->ah_attr);
 }
 
 static int cm_init_av_by_path(struct sa_path_rec *path, struct cm_av *av,
@@ -1826,9 +1826,11 @@ static int cm_req_handler(struct cm_work *work)
 
 	cm_id_priv = container_of(cm_id, struct cm_id_private, id);
 	cm_id_priv->id.remote_id = req_msg->local_comm_id;
-	cm_init_av_for_response(work->port, work->mad_recv_wc->wc,
-				work->mad_recv_wc->recv_buf.grh,
-				&cm_id_priv->av);
+	ret = cm_init_av_for_response(work->port, work->mad_recv_wc->wc,
+				      work->mad_recv_wc->recv_buf.grh,
+				      &cm_id_priv->av);
+	if (ret)
+		goto destroy;
 	cm_id_priv->timewait_info = cm_create_timewait_info(cm_id_priv->
 							    id.local_id);
 	if (IS_ERR(cm_id_priv->timewait_info)) {
@@ -3171,9 +3173,11 @@ static int cm_lap_handler(struct cm_work *work)
 
 	cm_id_priv->id.lap_state = IB_CM_LAP_RCVD;
 	cm_id_priv->tid = lap_msg->hdr.tid;
-	cm_init_av_for_response(work->port, work->mad_recv_wc->wc,
-				work->mad_recv_wc->recv_buf.grh,
-				&cm_id_priv->av);
+	ret = cm_init_av_for_response(work->port, work->mad_recv_wc->wc,
+				      work->mad_recv_wc->recv_buf.grh,
+				      &cm_id_priv->av);
+	if (ret)
+		goto unlock;
 	cm_init_av_by_path(param->alternate_path, &cm_id_priv->alt_av,
 			   cm_id_priv);
 	ret = atomic_inc_and_test(&cm_id_priv->work_count);
@@ -3426,6 +3430,7 @@ static int cm_sidr_req_handler(struct cm_work *work)
 	struct cm_id_private *cm_id_priv, *cur_cm_id_priv;
 	struct cm_sidr_req_msg *sidr_req_msg;
 	struct ib_wc *wc;
+	int ret;
 
 	cm_id = ib_create_cm_id(work->port->cm_dev->ib_device, NULL, NULL);
 	if (IS_ERR(cm_id))
@@ -3438,9 +3443,12 @@ static int cm_sidr_req_handler(struct cm_work *work)
 	wc = work->mad_recv_wc->wc;
 	cm_id_priv->av.dgid.global.subnet_prefix = cpu_to_be64(wc->slid);
 	cm_id_priv->av.dgid.global.interface_id = 0;
-	cm_init_av_for_response(work->port, work->mad_recv_wc->wc,
-				work->mad_recv_wc->recv_buf.grh,
-				&cm_id_priv->av);
+	ret = cm_init_av_for_response(work->port, work->mad_recv_wc->wc,
+				      work->mad_recv_wc->recv_buf.grh,
+				      &cm_id_priv->av);
+	if (ret)
+		goto out;
+
 	cm_id_priv->id.remote_id = sidr_req_msg->request_id;
 	cm_id_priv->tid = sidr_req_msg->hdr.tid;
 	atomic_inc(&cm_id_priv->work_count);
diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index 4b64dd02e090..3179fed4b408 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -246,10 +246,14 @@ static void recv_handler(struct ib_mad_agent *agent,
 	if (packet->mad.hdr.grh_present) {
 		struct rdma_ah_attr ah_attr;
 		const struct ib_global_route *grh;
+		int ret;
 
-		ib_init_ah_from_wc(agent->device, agent->port_num,
-				   mad_recv_wc->wc, mad_recv_wc->recv_buf.grh,
-				   &ah_attr);
+		ret = ib_init_ah_from_wc(agent->device, agent->port_num,
+					 mad_recv_wc->wc,
+					 mad_recv_wc->recv_buf.grh,
+					 &ah_attr);
+		if (ret)
+			goto err2;
 
 		grh = rdma_ah_read_grh(&ah_attr);
 		packet->mad.hdr.gid_index = grh->sgid_index;
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 27/31] IB/cm: Fix honor address handle attribute init error

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

cm_init_av_by_path depends on ib_init_ah_from_path to initialize ah
attribute and ib_init_ah_from_path() can fail, such error should not
be ignored.

This patch honors ah attribute init error.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cm.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 9a60d21ed0c3..d12c50ee2b89 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -494,8 +494,11 @@ static int cm_init_av_by_path(struct sa_path_rec *path, struct cm_av *av,
 		return ret;
 
 	av->port = port;
-	ib_init_ah_from_path(cm_dev->ib_device, port->port_num, path,
-			     &av->ah_attr);
+	ret = ib_init_ah_from_path(cm_dev->ib_device, port->port_num, path,
+				   &av->ah_attr);
+	if (ret)
+		return ret;
+
 	av->timeout = path->packet_life_time + 1;
 
 	spin_lock_irqsave(&cm.lock, flags);
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 27/31] IB/cm: Fix honor address handle attribute init error

[Or Gerlitz]

On Tue, Nov 14, 2017 at 2:52 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> cm_init_av_by_path depends on ib_init_ah_from_path to initialize ah
> attribute and ib_init_ah_from_path() can fail, such error should not
> be ignored.
>
> This patch honors ah attribute init error.
>
> Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

any reason not to point out the commit that introduced the bug?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 27/31] IB/cm: Fix honor address handle attribute init error

[Parav Pandit]

> -----Original Message-----
> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> Sent: Tuesday, November 14, 2017 3:14 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: Doug Ledford <dledford@redhat.com>; linux-rdma@vger.kernel.org; Hefty,
> Sean <sean.hefty@intel.com>; Leon Romanovsky <leonro@mellanox.com>
> Subject: Re: [PATCH rdma-next 27/31] IB/cm: Fix honor address handle attribute
> init error
> 
> On Tue, Nov 14, 2017 at 2:52 PM, Leon Romanovsky <leon@kernel.org> wrote:
> > From: Parav Pandit <parav@mellanox.com>
> >
> > cm_init_av_by_path depends on ib_init_ah_from_path to initialize ah
> > attribute and ib_init_ah_from_path() can fail, such error should not
> > be ignored.
> >
> > This patch honors ah attribute init error.
> >
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> 
> any reason not to point out the commit that introduced the bug?
No particular reason. I am not sure few years back it was bug or not. In current code it appear so.
So I continued without Fixes commit line.

Re: [PATCH rdma-next 27/31] IB/cm: Fix honor address handle attribute init error

[Or Gerlitz]

On Wed, Nov 15, 2017 at 12:13 AM, Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> From: Or Gerlitz [mailto:gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]

>> any reason not to point out the commit that introduced the bug?
> No particular reason. I am not sure few years back it was bug or not. In current code it appear so.
> So I continued without Fixes commit line.


The idea is that you do the git blaming and such work to get the Fixes
line, and later, the maintainer
of the sub-system and maintainers of stable kernel use that to decide
if/where they want this to go
beyond the -rc kernel you are fixing
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 27/31] IB/cm: Fix honor address handle attribute init error

[Parav Pandit]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1459 bytes --]



> -----Original Message-----
> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> Sent: Tuesday, November 14, 2017 4:18 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: Doug Ledford <dledford@redhat.com>; linux-rdma@vger.kernel.org; Hefty,
> Sean <sean.hefty@intel.com>; Leon Romanovsky <leonro@mellanox.com>
> Subject: Re: [PATCH rdma-next 27/31] IB/cm: Fix honor address handle attribute
> init error
> 
> On Wed, Nov 15, 2017 at 12:13 AM, Parav Pandit <parav@mellanox.com>
> wrote:
> >> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> 
> >> any reason not to point out the commit that introduced the bug?
> > No particular reason. I am not sure few years back it was bug or not. In current
> code it appear so.
> > So I continued without Fixes commit line.
> 
> 
> The idea is that you do the git blaming and such work to get the Fixes line, and
> later, the maintainer of the sub-system and maintainers of stable kernel use that
> to decide if/where they want this to go beyond the -rc kernel you are fixing
Yes,  I understand. But would that be a bug in initial commit, which transport was it handling.. without error code check what could have gone wrong..
I do not have answer for those questions. So I omitted the Fixes line.
Wherever I knew for sure in this or other series, I have added.
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH rdma-next 27/31] IB/cm: Fix honor address handle attribute init error

[Jason Gunthorpe]

On Tue, Nov 14, 2017 at 10:27:42PM +0000, Parav Pandit wrote:

> > On Wed, Nov 15, 2017 at 12:13 AM, Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > wrote:
> > >> From: Or Gerlitz [mailto:gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > 
> > >> any reason not to point out the commit that introduced the bug?
> > > No particular reason. I am not sure few years back it was bug or not. In current
> > code it appear so.
> > > So I continued without Fixes commit line.
> > 
> > 
> > The idea is that you do the git blaming and such work to get the
> > Fixes line, and later, the maintainer of the sub-system and
> > maintainers of stable kernel use that to decide if/where they want
> > this to go beyond the -rc kernel you are fixing

> Yes, I understand. But would that be a bug in initial commit, which
> transport was it handling.. without error code check what could have
> gone wrong..  I do not have answer for those questions. So I omitted
> the Fixes line.  Wherever I knew for sure in this or other series, I
> have added.

While the fixes line is nice, the more important question is if any of
these patches should have a 'Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org' ?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 27/31] IB/cm: Fix honor address handle attribute init error

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1696 bytes --]

On Tue, Nov 14, 2017 at 04:38:41PM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 14, 2017 at 10:27:42PM +0000, Parav Pandit wrote:
>
> > > On Wed, Nov 15, 2017 at 12:13 AM, Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > wrote:
> > > >> From: Or Gerlitz [mailto:gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > >
> > > >> any reason not to point out the commit that introduced the bug?
> > > > No particular reason. I am not sure few years back it was bug or not. In current
> > > code it appear so.
> > > > So I continued without Fixes commit line.
> > >
> > >
> > > The idea is that you do the git blaming and such work to get the
> > > Fixes line, and later, the maintainer of the sub-system and
> > > maintainers of stable kernel use that to decide if/where they want
> > > this to go beyond the -rc kernel you are fixing
>
> > Yes, I understand. But would that be a bug in initial commit, which
> > transport was it handling.. without error code check what could have
> > gone wrong..  I do not have answer for those questions. So I omitted
> > the Fixes line.  Wherever I knew for sure in this or other series, I
> > have added.
>
> While the fixes line is nice, the more important question is if any of
> these patches should have a 'Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org' ?

No, this is why I didn't ask to add Fixes line, because stable@ takes
patches in semi-automatic mode by looking after Fixes line.

Thanks


>
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-next 27/31] IB/cm: Fix honor address handle attribute init error

[Or Gerlitz]

On Wed, Nov 15, 2017 at 7:31 AM, Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> On Tue, Nov 14, 2017 at 04:38:41PM -0700, Jason Gunthorpe wrote:
>> On Tue, Nov 14, 2017 at 10:27:42PM +0000, Parav Pandit wrote:
>>
>> > > On Wed, Nov 15, 2017 at 12:13 AM, Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> > > wrote:
>> > > >> From: Or Gerlitz [mailto:gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
>> > >
>> > > >> any reason not to point out the commit that introduced the bug?
>> > > > No particular reason. I am not sure few years back it was bug or not. In current
>> > > code it appear so.
>> > > > So I continued without Fixes commit line.
>> > >
>> > >
>> > > The idea is that you do the git blaming and such work to get the
>> > > Fixes line, and later, the maintainer of the sub-system and
>> > > maintainers of stable kernel use that to decide if/where they want
>> > > this to go beyond the -rc kernel you are fixing
>>
>> > Yes, I understand. But would that be a bug in initial commit, which
>> > transport was it handling.. without error code check what could have
>> > gone wrong..  I do not have answer for those questions. So I omitted
>> > the Fixes line.  Wherever I knew for sure in this or other series, I
>> > have added.
>>
>> While the fixes line is nice, the more important question is if any of
>> these patches should have a 'Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org' ?
>
> No, this is why I didn't ask to add Fixes line, because stable@ takes
> patches in semi-automatic mode by looking after Fixes line.

And what's wrong with that? IMHO the right thing to do for fixes is

(1) putng the Fixes line
(2) cc the author of the offending patch

This way you can get them a chance to review the fix and point out to
you problems in the patch that may e.g introduce a 2nd problem or
a side effect you don't envision. This is what we do on netdev,
practically always.

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 27/31] IB/cm: Fix honor address handle attribute init error

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 3080 bytes --]

On Wed, Nov 15, 2017 at 09:59:23AM +0200, Or Gerlitz wrote:
> On Wed, Nov 15, 2017 at 7:31 AM, Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> > On Tue, Nov 14, 2017 at 04:38:41PM -0700, Jason Gunthorpe wrote:
> >> On Tue, Nov 14, 2017 at 10:27:42PM +0000, Parav Pandit wrote:
> >>
> >> > > On Wed, Nov 15, 2017 at 12:13 AM, Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >> > > wrote:
> >> > > >> From: Or Gerlitz [mailto:gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> >> > >
> >> > > >> any reason not to point out the commit that introduced the bug?
> >> > > > No particular reason. I am not sure few years back it was bug or not. In current
> >> > > code it appear so.
> >> > > > So I continued without Fixes commit line.
> >> > >
> >> > >
> >> > > The idea is that you do the git blaming and such work to get the
> >> > > Fixes line, and later, the maintainer of the sub-system and
> >> > > maintainers of stable kernel use that to decide if/where they want
> >> > > this to go beyond the -rc kernel you are fixing
> >>
> >> > Yes, I understand. But would that be a bug in initial commit, which
> >> > transport was it handling.. without error code check what could have
> >> > gone wrong..  I do not have answer for those questions. So I omitted
> >> > the Fixes line.  Wherever I knew for sure in this or other series, I
> >> > have added.
> >>
> >> While the fixes line is nice, the more important question is if any of
> >> these patches should have a 'Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org' ?
> >
> > No, this is why I didn't ask to add Fixes line, because stable@ takes
> > patches in semi-automatic mode by looking after Fixes line.
>
> And what's wrong with that? IMHO the right thing to do for fixes is
>
> (1) putng the Fixes line
> (2) cc the author of the offending patch
>
> This way you can get them a chance to review the fix and point out to
> you problems in the patch that may e.g introduce a 2nd problem or
> a side effect you don't envision. This is what we do on netdev,
> practically always.

Always, are you sure?

In the patch c1c1d86bdea4 ("net/mlxfw: Properly handle dependancy with
non-loadable mlx5"), you didn't add Fixes tag and didn't add relevant
people (Yotam, Arnd and Jakub).

It is how it looks in my mailer:
Date: Sun,  2 Jul 2017 18:57:28 +0300
From: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
To: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Saeed Mahameed <saeedm-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>, mlxsw@mellanox.com, Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Subject: [PATCH net-next] net/mlxfw: Properly handle dependancy with non-loadable mlx5

Thanks

>
> Or.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

In case of LAP are used for RoCE, it can lead to a problem of sleeping a
context while spin lock is held in below flow.

cm_lap_handler
	->spin_lock
	-> <..switch_case..>
	-> cm_init_av_for_response
		-> ib_init_ah_from_wc
			-> rdma_addr_find_l2_eth_by_grh
				wait_for_completion()

Therefore ah attribute initialization is done for incoming lap requests
outside of the lock context.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cm.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index d12c50ee2b89..19fd1699dc52 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -3130,6 +3130,12 @@ static int cm_lap_handler(struct cm_work *work)
 	if (!cm_id_priv)
 		return -EINVAL;
 
+	ret = cm_init_av_for_response(work->port, work->mad_recv_wc->wc,
+				      work->mad_recv_wc->recv_buf.grh,
+				      &cm_id_priv->av);
+	if (ret)
+		goto deref;
+
 	param = &work->cm_event.param.lap_rcvd;
 	memset(&work->path[0], 0, sizeof(work->path[1]));
 	cm_path_set_rec_type(work->port->cm_dev->ib_device,
@@ -3176,11 +3182,6 @@ static int cm_lap_handler(struct cm_work *work)
 
 	cm_id_priv->id.lap_state = IB_CM_LAP_RCVD;
 	cm_id_priv->tid = lap_msg->hdr.tid;
-	ret = cm_init_av_for_response(work->port, work->mad_recv_wc->wc,
-				      work->mad_recv_wc->recv_buf.grh,
-				      &cm_id_priv->av);
-	if (ret)
-		goto unlock;
 	cm_init_av_by_path(param->alternate_path, &cm_id_priv->alt_av,
 			   cm_id_priv);
 	ret = atomic_inc_and_test(&cm_id_priv->work_count);
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Or Gerlitz]

On Tue, Nov 14, 2017 at 2:52 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> In case of LAP are used for RoCE

AFAIK APM is not supported with RoCE, isn't that? if yes, on what
HW/RoCE mode and how do you test that?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Parav Pandit]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1126 bytes --]



> -----Original Message-----
> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> Sent: Tuesday, November 14, 2017 3:16 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: Doug Ledford <dledford@redhat.com>; linux-rdma@vger.kernel.org; Hefty,
> Sean <sean.hefty@intel.com>; Leon Romanovsky <leonro@mellanox.com>
> Subject: Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is
> held
> 
> On Tue, Nov 14, 2017 at 2:52 PM, Leon Romanovsky <leon@kernel.org> wrote:
> > From: Parav Pandit <parav@mellanox.com>
> >
> > In case of LAP are used for RoCE
> 
> AFAIK APM is not supported with RoCE, isn't that? if yes, on what HW/RoCE
> mode and how do you test that?
This particular patch in this series is not tested on RoCE.
APM is not supported but injecting such frame in network is not impossible.
We don’t have particular checks to reject such frame for RoCE.
So I prefer to fix the issue before getting such hang/oops report when issue is evident in code.
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Or Gerlitz]

On Wed, Nov 15, 2017 at 12:16 AM, Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> From: Or Gerlitz [mailto:gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
>> Subject: Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is
>>> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

>>> In case of LAP are used for RoCE

>> AFAIK APM is not supported with RoCE, isn't that? if yes, on what HW/RoCE
>> mode and how do you test that?

> This particular patch in this series is not tested on RoCE.
> APM is not supported but injecting such frame in network is not impossible.
> We don’t have particular checks to reject such frame for RoCE.
> So I prefer to fix the issue before getting such hang/oops report when issue is evident in code.

mentioning testing, in this series you did bunch of fixes for the IB
core, CM and CMA.

Would be good to add a section in the cover letter mentioning under which type
of testing the modified code has gone through.

Seems that you did a hell of hardening to the IB core, cool!!
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Parav Pandit]

> -----Original Message-----
> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> Sent: Tuesday, November 14, 2017 4:21 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: Doug Ledford <dledford@redhat.com>; linux-rdma@vger.kernel.org; Hefty,
> Sean <sean.hefty@intel.com>; Leon Romanovsky <leonro@mellanox.com>
> Subject: Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is
> held
> 
> On Wed, Nov 15, 2017 at 12:16 AM, Parav Pandit <parav@mellanox.com>
> wrote:
> >> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> >> Subject: Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while
> >> spin lock is
> >>> From: Parav Pandit <parav@mellanox.com>
> 
> >>> In case of LAP are used for RoCE
> 
> >> AFAIK APM is not supported with RoCE, isn't that? if yes, on what
> >> HW/RoCE mode and how do you test that?
> 
> > This particular patch in this series is not tested on RoCE.
> > APM is not supported but injecting such frame in network is not impossible.
> > We don’t have particular checks to reject such frame for RoCE.
> > So I prefer to fix the issue before getting such hang/oops report when issue is
> evident in code.
> 
> mentioning testing, in this series you did bunch of fixes for the IB core, CM and
> CMA.
> 
> Would be good to add a section in the cover letter mentioning under which type
> of testing the modified code has gone through.
It's been tested for RoCEv1, v2, IPoIB with ConnectX4, ConnectX3 HCA with perftest, rping, krping tools.
In few individual patch, it's been mentioned how is this tested.

> 
> Seems that you did a hell of hardening to the IB core, cool!!
Thanks Or.

Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Or Gerlitz]

On Wed, Nov 15, 2017 at 12:32 AM, Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> From: Or Gerlitz [mailto:gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]

>> >>> In case of LAP are used for RoCE

>> >> AFAIK APM is not supported with RoCE, isn't that? if yes, on what
>> >> HW/RoCE mode and how do you test that?

>> > This particular patch in this series is not tested on RoCE.
>> > APM is not supported but injecting such frame in network is not impossible.
>> > We don’t have particular checks to reject such frame for RoCE.
>> > So I prefer to fix the issue before getting such hang/oops report when issue is
>> evident in code.
>>
>> mentioning testing, in this series you did bunch of fixes for the IB core, CM and
>> CMA.
>>
>> Would be good to add a section in the cover letter mentioning under which type
>> of testing the modified code has gone through.

> It's been tested for RoCEv1, v2, IPoIB with ConnectX4, ConnectX3 HCA with perftest, rping, krping tools.
> In few individual patch, it's been mentioned how is this tested.

Please also add more info (and in the cover letter please) on IPv4 vs
IPv6, unicast vs multicast, etc.
You touched the CMA and addr modules quite heavily on all these areas,
need to cover them in testing.

did you test APM? I don't think it would be correct to change the CM APM code
without proper testing.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Parav Pandit]

> -----Original Message-----
> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> Sent: Wednesday, November 15, 2017 1:53 AM
> To: Parav Pandit <parav@mellanox.com>
> Cc: Doug Ledford <dledford@redhat.com>; linux-rdma@vger.kernel.org; Hefty,
> Sean <sean.hefty@intel.com>; Leon Romanovsky <leonro@mellanox.com>
> Subject: Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is
> held
> 
> On Wed, Nov 15, 2017 at 12:32 AM, Parav Pandit <parav@mellanox.com>
> wrote:
> >> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> 
> >> >>> In case of LAP are used for RoCE
> 
> >> >> AFAIK APM is not supported with RoCE, isn't that? if yes, on what
> >> >> HW/RoCE mode and how do you test that?
> 
> >> > This particular patch in this series is not tested on RoCE.
> >> > APM is not supported but injecting such frame in network is not impossible.
> >> > We don’t have particular checks to reject such frame for RoCE.
> >> > So I prefer to fix the issue before getting such hang/oops report
> >> > when issue is
> >> evident in code.
> >>
> >> mentioning testing, in this series you did bunch of fixes for the IB
> >> core, CM and CMA.
> >>
> >> Would be good to add a section in the cover letter mentioning under
> >> which type of testing the modified code has gone through.
> 
> > It's been tested for RoCEv1, v2, IPoIB with ConnectX4, ConnectX3 HCA with
> perftest, rping, krping tools.
> > In few individual patch, it's been mentioned how is this tested.
> 
> Please also add more info (and in the cover letter please) on IPv4 vs IPv6, unicast
> vs multicast, etc.
Is there a template that you can share that linux-rdma follows that I can use to mention what tests are done?
Any example past cover letters that I can refer as reference?

Depending on how much information you expect in cover letter, this patchset can have many revisions just for cover letter.
So it would be best for you to share the template that I can fill in for v1.

> You touched the CMA and addr modules quite heavily on all these areas, need to
> cover them in testing.
Which are those tests that I should execute apart from above summary?

> 
> did you test APM? I don't think it would be correct to change the CM APM code
> without proper testing.
As I already told this particular patch is not tested. All others are needless to say tested.
I do not have APM setup to test this fix.
I am ok to drop this patch and provide fix when crash occurs later on.

Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 3560 bytes --]

On Wed, Nov 15, 2017 at 11:10:54PM +0000, Parav Pandit wrote:
> > -----Original Message-----
> > From: Or Gerlitz [mailto:gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > Sent: Wednesday, November 15, 2017 1:53 AM
> > To: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Cc: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Hefty,
> > Sean <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Subject: Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is
> > held
> >
> > On Wed, Nov 15, 2017 at 12:32 AM, Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > wrote:
> > >> From: Or Gerlitz [mailto:gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> >
> > >> >>> In case of LAP are used for RoCE
> >
> > >> >> AFAIK APM is not supported with RoCE, isn't that? if yes, on what
> > >> >> HW/RoCE mode and how do you test that?
> >
> > >> > This particular patch in this series is not tested on RoCE.
> > >> > APM is not supported but injecting such frame in network is not impossible.
> > >> > We don’t have particular checks to reject such frame for RoCE.
> > >> > So I prefer to fix the issue before getting such hang/oops report
> > >> > when issue is
> > >> evident in code.
> > >>
> > >> mentioning testing, in this series you did bunch of fixes for the IB
> > >> core, CM and CMA.
> > >>
> > >> Would be good to add a section in the cover letter mentioning under
> > >> which type of testing the modified code has gone through.
> >
> > > It's been tested for RoCEv1, v2, IPoIB with ConnectX4, ConnectX3 HCA with
> > perftest, rping, krping tools.
> > > In few individual patch, it's been mentioned how is this tested.
> >
> > Please also add more info (and in the cover letter please) on IPv4 vs IPv6, unicast
> > vs multicast, etc.
> Is there a template that you can share that linux-rdma follows that I can use to mention what tests are done?
> Any example past cover letters that I can refer as reference?
>
> Depending on how much information you expect in cover letter, this patchset can have many revisions just for cover letter.
> So it would be best for you to share the template that I can fill in for v1.

Parav, relax
There is no need to respin.

I reread responses and there are two types of response/disputes:
1. Jason vs. Erez. Jason is in favor of having memory leak as long as
IPoIB doesn't filter input (in our case wrong input), Erez in favor
of fixing memory leak. I'm with Erez.
2. Or vs. rest of the world. No doubts that the commit messages and
cover letter are important, but they are not the most important part
of this series to justify resubmission.

And yes, you should listen to the feedback provided here and improve
your commit messages in next submissions.

>
> > You touched the CMA and addr modules quite heavily on all these areas, need to
> > cover them in testing.
> Which are those tests that I should execute apart from above summary?
>
> >
> > did you test APM? I don't think it would be correct to change the CM APM code
> > without proper testing.
> As I already told this particular patch is not tested. All others are needless to say tested.
> I do not have APM setup to test this fix.
> I am ok to drop this patch and provide fix when crash occurs later on.

Not everything is possible to test, it doesn't mean that we shouldn't fix it.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Or Gerlitz]

On Thu, Nov 16, 2017 at 1:10 AM, Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> From: Or Gerlitz [mailto:gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]

>> Please also add more info (and in the cover letter please) on IPv4 vs IPv6, unicast
>> vs multicast, etc.

> Is there a template that you can share that linux-rdma follows that I can use to mention what tests are done?
> Any example past cover letters that I can refer as reference?

I suggest you ask the sub-system maintainers (Doug, Jason) what do they expect
for such a heavy duty change set.


> Depending on how much information you expect in cover letter, this patchset can have many revisions just for cover letter.
> So it would be best for you to share the template that I can fill in for v1.

You are still on V0 and complaining on many revisions...

>> You touched the CMA and addr modules quite heavily on all these areas, need to
>> cover them in testing.

> Which are those tests that I should execute apart from above summary?

If there is rdma verification suit @ mlnx that runs rdma user/kernel cm/cma
based applications on ib/roce environments, can run through it.

>> did you test APM? I don't think it would be correct to change the CM APM code
>> without proper testing.

> As I already told this particular patch is not tested. All others are needless to say tested.
> I do not have APM setup to test this fix.
> I am ok to drop this patch and provide fix when crash occurs later on.

I suggest you ask the CM maintainer (Sean) for his opinion on that matter.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Parav Pandit]

> -----Original Message-----
> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> Sent: Thursday, November 16, 2017 5:50 AM
> To: Parav Pandit <parav@mellanox.com>
> Cc: Doug Ledford <dledford@redhat.com>; linux-rdma@vger.kernel.org; Hefty,
> Sean <sean.hefty@intel.com>; Leon Romanovsky <leonro@mellanox.com>
> Subject: Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is
> held
> 
> On Thu, Nov 16, 2017 at 1:10 AM, Parav Pandit <parav@mellanox.com> wrote:
> >> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> 
> >> Please also add more info (and in the cover letter please) on IPv4 vs
> >> IPv6, unicast vs multicast, etc.
> 
> > Is there a template that you can share that linux-rdma follows that I can use to
> mention what tests are done?
> > Any example past cover letters that I can refer as reference?
> 
> I suggest you ask the sub-system maintainers (Doug, Jason) what do they expect
> for such a heavy duty change set.
> 
> 
> > Depending on how much information you expect in cover letter, this patchset
> can have many revisions just for cover letter.
> > So it would be best for you to share the template that I can fill in for v1.
> 
> You are still on V0 and complaining on many revisions...
> 
> >> You touched the CMA and addr modules quite heavily on all these
> >> areas, need to cover them in testing.
> 
> > Which are those tests that I should execute apart from above summary?
> 
> If there is rdma verification suit @ mlnx that runs rdma user/kernel cm/cma
> based applications on ib/roce environments, can run through it.
> 
I have run such verification suit in RoCE environment.
IB was not possible to run due to certain temporary setup constrains.
However as I mentioned before,
IB, perftests, rping, krping, iperf (IPoIB) were done for IB in VM and non VM servers.
Same tests with RoCEv1, RoCEv2, IPv4 multicast with mckey, and tests related to IPv6 bug fixes are also ran.
Certain errors related to timings are injected in code to test them for error path.

> >> did you test APM? I don't think it would be correct to change the CM
> >> APM code without proper testing.
> 
> > As I already told this particular patch is not tested. All others are needless to
> say tested.
> > I do not have APM setup to test this fix.
> > I am ok to drop this patch and provide fix when crash occurs later on.
> 
> I suggest you ask the CM maintainer (Sean) for his opinion on that matter.
You have already added him in CC list so if he has specifics I will let him respond.
I haven't seen test confirmation from Roland for similar fix submitted last time in other cm handler.

Leon,
Once adequate review is done, let me know on how to proceed.
I have additional 6 patches in one patchset that fixes two bugs for RoCE; without which certain users applications are broken for RoCE.
Those fixes depend on this series.

RE: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Amrani, Ram]

> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> In case of LAP are used for RoCE, it can lead to a problem of sleeping a
> context while spin lock is held in below flow.
> 
> cm_lap_handler
> 	->spin_lock
> 	-> <..switch_case..>
> 	-> cm_init_av_for_response
> 		-> ib_init_ah_from_wc
> 			-> rdma_addr_find_l2_eth_by_grh
> 				wait_for_completion()
> 
> Therefore ah attribute initialization is done for incoming lap requests
> outside of the lock context.
> 
> Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Should you add a "Fixes:..."?
Pardon my ignorance, but what is LAP?

Thanks,
Ram

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

On Sun, Nov 19, 2017 at 08:42:17AM +0000, Amrani, Ram wrote:
> > From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > In case of LAP are used for RoCE, it can lead to a problem of sleeping a
> > context while spin lock is held in below flow.
> >
> > cm_lap_handler
> > 	->spin_lock
> > 	-> <..switch_case..>
> > 	-> cm_init_av_for_response
> > 		-> ib_init_ah_from_wc
> > 			-> rdma_addr_find_l2_eth_by_grh
> > 				wait_for_completion()
> >
> > Therefore ah attribute initialization is done for incoming lap requests
> > outside of the lock context.
> >
> > Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Should you add a "Fixes:..."?

It wasn't clear to us the origin of the problem hence we decided to avoid Fixes line.

> Pardon my ignorance, but what is LAP?

It should be LAG.

>
> Thanks,
> Ram
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Or Gerlitz]

On Sun, Nov 19, 2017 at 12:52 PM, Leon Romanovsky wrote:
> On Sun, Nov 19, 2017 at 08:42:17AM +0000, Amrani, Ram wrote:

>> Pardon my ignorance, but what is LAP?

> It should be LAG.

No! This is CM LAP (see IBTA spec ch. 12)
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 28/31] IB/cm: Fix avoid sleep while spin lock is held

[Amrani, Ram]

> No! This is CM LAP (see IBTA spec ch. 12)

Got it. Thanks.

Ram

[PATCH rdma-next 29/31] IB/{core, cm, cma, ipoib}: Rename ib_init_ah_from_path to ib_init_ah_attr_from_path

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Since ib_init_ah_from_path initializes the address handle attribute, it is
renamed to reflect so.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cm.c              |  4 ++--
 drivers/infiniband/core/cma.c             |  7 ++++---
 drivers/infiniband/core/sa_query.c        |  8 ++++----
 drivers/infiniband/ulp/ipoib/ipoib_main.c |  3 ++-
 include/rdma/ib_sa.h                      | 10 +++++-----
 5 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 19fd1699dc52..3bb82c6ec88d 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -494,8 +494,8 @@ static int cm_init_av_by_path(struct sa_path_rec *path, struct cm_av *av,
 		return ret;
 
 	av->port = port;
-	ret = ib_init_ah_from_path(cm_dev->ib_device, port->port_num, path,
-				   &av->ah_attr);
+	ret = ib_init_ah_attr_from_path(cm_dev->ib_device, port->port_num, path,
+					&av->ah_attr);
 	if (ret)
 		return ret;
 
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 19769ea4d71f..5ab646a24089 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -3396,9 +3396,10 @@ static int cma_sidr_rep_handler(struct ib_cm_id *cm_id,
 			event.status = ret;
 			break;
 		}
-		ib_init_ah_from_path(id_priv->id.device, id_priv->id.port_num,
-				     id_priv->id.route.path_rec,
-				     &event.param.ud.ah_attr);
+		ib_init_ah_attr_from_path(id_priv->id.device,
+					  id_priv->id.port_num,
+					  id_priv->id.route.path_rec,
+					  &event.param.ud.ah_attr);
 		event.param.ud.qp_num = rep->qpn;
 		event.param.ud.qkey = rep->qkey;
 		event.event = RDMA_CM_EVENT_ESTABLISHED;
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index ab5e1024fea9..7cd37e77debb 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -1227,9 +1227,9 @@ static u8 get_src_path_mask(struct ib_device *device, u8 port_num)
 	return src_path_mask;
 }
 
-int ib_init_ah_from_path(struct ib_device *device, u8 port_num,
-			 struct sa_path_rec *rec,
-			 struct rdma_ah_attr *ah_attr)
+int ib_init_ah_attr_from_path(struct ib_device *device, u8 port_num,
+			      struct sa_path_rec *rec,
+			      struct rdma_ah_attr *ah_attr)
 {
 	int ret;
 	u16 gid_index;
@@ -1341,7 +1341,7 @@ int ib_init_ah_from_path(struct ib_device *device, u8 port_num,
 
 	return 0;
 }
-EXPORT_SYMBOL(ib_init_ah_from_path);
+EXPORT_SYMBOL(ib_init_ah_attr_from_path);
 
 static int alloc_mad(struct ib_sa_query *query, gfp_t gfp_mask)
 {
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 33f592cee5d8..71a66a7d265e 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -768,7 +768,8 @@ static void path_rec_completion(int status,
 	if (!status) {
 		struct rdma_ah_attr av;
 
-		if (!ib_init_ah_from_path(priv->ca, priv->port, pathrec, &av))
+		if (!ib_init_ah_attr_from_path(priv->ca, priv->port,
+					       pathrec, &av))
 			ah = ipoib_create_ah(dev, priv->pd, &av);
 	}
 
diff --git a/include/rdma/ib_sa.h b/include/rdma/ib_sa.h
index 1f7f604db5aa..811cfcfcbe3d 100644
--- a/include/rdma/ib_sa.h
+++ b/include/rdma/ib_sa.h
@@ -549,12 +549,12 @@ int ib_init_ah_from_mcmember(struct ib_device *device, u8 port_num,
 			     struct rdma_ah_attr *ah_attr);
 
 /**
- * ib_init_ah_from_path - Initialize address handle attributes based on an SA
- *   path record.
+ * ib_init_ah_attr_from_path - Initialize address handle attributes based on
+ *   an SA path record.
  */
-int ib_init_ah_from_path(struct ib_device *device, u8 port_num,
-			 struct sa_path_rec *rec,
-			 struct rdma_ah_attr *ah_attr);
+int ib_init_ah_attr_from_path(struct ib_device *device, u8 port_num,
+			      struct sa_path_rec *rec,
+			      struct rdma_ah_attr *ah_attr);
 
 /**
  * ib_sa_pack_path - Conert a path record from struct ib_sa_path_rec
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 30/31] IB/{core, umad, cm}: Rename ib_init_ah_from_wc to ib_init_ah_attr_from_wc

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Currently ib_init_ah_from_wc initializes address handle attributes and
not the address handle object itself.
To avoid confusion between ah_attr vs ah, ib_init_ah_from_wc is
renamed to ib_init_ah_attr_from_wc to reflect that its initialzes
ah_attr.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cm.c       |  5 +++--
 drivers/infiniband/core/user_mad.c |  8 ++++----
 drivers/infiniband/core/verbs.c    | 12 ++++++------
 include/rdma/ib_verbs.h            |  8 ++++----
 4 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 3bb82c6ec88d..e5d96bd3c377 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -457,8 +457,9 @@ static int cm_init_av_for_response(struct cm_port *port, struct ib_wc *wc,
 {
 	av->port = port;
 	av->pkey_index = wc->pkey_index;
-	return ib_init_ah_from_wc(port->cm_dev->ib_device, port->port_num, wc,
-				  grh, &av->ah_attr);
+	return ib_init_ah_attr_from_wc(port->cm_dev->ib_device,
+				       port->port_num, wc,
+				       grh, &av->ah_attr);
 }
 
 static int cm_init_av_by_path(struct sa_path_rec *path, struct cm_av *av,
diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index 3179fed4b408..3ef8ee5d4aa2 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -248,10 +248,10 @@ static void recv_handler(struct ib_mad_agent *agent,
 		const struct ib_global_route *grh;
 		int ret;
 
-		ret = ib_init_ah_from_wc(agent->device, agent->port_num,
-					 mad_recv_wc->wc,
-					 mad_recv_wc->recv_buf.grh,
-					 &ah_attr);
+		ret = ib_init_ah_attr_from_wc(agent->device, agent->port_num,
+					      mad_recv_wc->wc,
+					      mad_recv_wc->recv_buf.grh,
+					      &ah_attr);
 		if (ret)
 			goto err2;
 
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 0cac32736189..b53fb0e98751 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -526,7 +526,7 @@ static int ib_resolve_unicast_gid_dmac(struct ib_device *device,
 }
 
 /*
- * This function creates ah from the incoming packet.
+ * This function initializes address handle attributes from the incoming packet.
  * Incoming packet has dgid of the receiver node on which this code is
  * getting executed and, sgid contains the GID of the sender.
  *
@@ -535,9 +535,9 @@ static int ib_resolve_unicast_gid_dmac(struct ib_device *device,
  * GID whom to respond to.
  *
  */
-int ib_init_ah_from_wc(struct ib_device *device, u8 port_num,
-		       const struct ib_wc *wc, const struct ib_grh *grh,
-		       struct rdma_ah_attr *ah_attr)
+int ib_init_ah_attr_from_wc(struct ib_device *device, u8 port_num,
+			    const struct ib_wc *wc, const struct ib_grh *grh,
+			    struct rdma_ah_attr *ah_attr)
 {
 	u32 flow_class;
 	u16 gid_index;
@@ -611,7 +611,7 @@ int ib_init_ah_from_wc(struct ib_device *device, u8 port_num,
 		return 0;
 	}
 }
-EXPORT_SYMBOL(ib_init_ah_from_wc);
+EXPORT_SYMBOL(ib_init_ah_attr_from_wc);
 
 struct ib_ah *ib_create_ah_from_wc(struct ib_pd *pd, const struct ib_wc *wc,
 				   const struct ib_grh *grh, u8 port_num)
@@ -619,7 +619,7 @@ struct ib_ah *ib_create_ah_from_wc(struct ib_pd *pd, const struct ib_wc *wc,
 	struct rdma_ah_attr ah_attr;
 	int ret;
 
-	ret = ib_init_ah_from_wc(pd->device, port_num, wc, grh, &ah_attr);
+	ret = ib_init_ah_attr_from_wc(pd->device, port_num, wc, grh, &ah_attr);
 	if (ret)
 		return ERR_PTR(ret);
 
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index cfd837049a32..60c3268c8c04 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2904,7 +2904,7 @@ int ib_get_gids_from_rdma_hdr(const union rdma_network_hdr *hdr,
 int ib_get_rdma_header_version(const union rdma_network_hdr *hdr);
 
 /**
- * ib_init_ah_from_wc - Initializes address handle attributes from a
+ * ib_init_ah_attr_from_wc - Initializes address handle attributes from a
  *   work completion.
  * @device: Device on which the received message arrived.
  * @port_num: Port on which the received message arrived.
@@ -2914,9 +2914,9 @@ int ib_get_rdma_header_version(const union rdma_network_hdr *hdr);
  * @ah_attr: Returned attributes that can be used when creating an address
  *   handle for replying to the message.
  */
-int ib_init_ah_from_wc(struct ib_device *device, u8 port_num,
-		       const struct ib_wc *wc, const struct ib_grh *grh,
-		       struct rdma_ah_attr *ah_attr);
+int ib_init_ah_attr_from_wc(struct ib_device *device, u8 port_num,
+			    const struct ib_wc *wc, const struct ib_grh *grh,
+			    struct rdma_ah_attr *ah_attr);
 
 /**
  * ib_create_ah_from_wc - Creates an address handle associated with the
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 31/31] IB/cm: Refactor to avoid setting path record software only fields

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

When path ah_attr initialization from path record
fails, ib_cm_send_rej() uses av.ah_attr fields to send out reject
message. In such cases initialization of path record software fields
is not needed. Code is simplified for same.

Additionally in current code in cm_req_handler, when ib_get_cached_gid
fails for a given sgid_index of the GID of the GRH of the incoming CM MAD,
error code 12 is sent. This error code refers to primary GID in incoming
CM REQ and not for the GID in in MAD packet.
Therefore code is refactored to send code 5 (unsupported request) for such
error.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cm.c | 92 +++++++++++++++++++++-----------------------
 1 file changed, 44 insertions(+), 48 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index e5d96bd3c377..dc61e66ae7e6 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1868,56 +1868,52 @@ static int cm_req_handler(struct cm_work *work)
 				work->port->port_num,
 				grh->sgid_index,
 				&gid, &gid_attr);
-	if (!ret) {
-		if (gid_attr.ndev) {
-			work->path[0].rec_type =
-				sa_conv_gid_to_pathrec_type(gid_attr.gid_type);
-			sa_path_set_ifindex(&work->path[0],
-					    gid_attr.ndev->ifindex);
-			sa_path_set_ndev(&work->path[0],
-					 dev_net(gid_attr.ndev));
+	if (ret) {
+		if (gid_attr.ndev)
 			dev_put(gid_attr.ndev);
-		} else {
-			cm_path_set_rec_type(work->port->cm_dev->ib_device,
-					     work->port->port_num,
-					     &work->path[0],
-					     &req_msg->primary_local_gid);
-		}
-		if (cm_req_has_alt_path(req_msg))
-			work->path[1].rec_type = work->path[0].rec_type;
-		cm_format_paths_from_req(req_msg, &work->path[0],
-					 &work->path[1]);
-		if (cm_id_priv->av.ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE)
-			sa_path_set_dmac(&work->path[0],
-					 cm_id_priv->av.ah_attr.roce.dmac);
-		work->path[0].hop_limit = grh->hop_limit;
-		ret = cm_init_av_by_path(&work->path[0], &cm_id_priv->av,
-					 cm_id_priv);
+		ib_send_cm_rej(cm_id, IB_CM_REJ_UNSUPPORTED, NULL, 0, NULL, 0);
+		goto rejected;
 	}
+
+	if (gid_attr.ndev) {
+		work->path[0].rec_type =
+			sa_conv_gid_to_pathrec_type(gid_attr.gid_type);
+		sa_path_set_ifindex(&work->path[0],
+				    gid_attr.ndev->ifindex);
+		sa_path_set_ndev(&work->path[0],
+				 dev_net(gid_attr.ndev));
+		dev_put(gid_attr.ndev);
+	} else {
+		cm_path_set_rec_type(work->port->cm_dev->ib_device,
+				     work->port->port_num,
+				     &work->path[0],
+				     &req_msg->primary_local_gid);
+	}
+	if (cm_req_has_alt_path(req_msg))
+		work->path[1].rec_type = work->path[0].rec_type;
+	cm_format_paths_from_req(req_msg, &work->path[0],
+				 &work->path[1]);
+	if (cm_id_priv->av.ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE)
+		sa_path_set_dmac(&work->path[0],
+				 cm_id_priv->av.ah_attr.roce.dmac);
+	work->path[0].hop_limit = grh->hop_limit;
+	ret = cm_init_av_by_path(&work->path[0], &cm_id_priv->av,
+				 cm_id_priv);
 	if (ret) {
-		int err = ib_get_cached_gid(work->port->cm_dev->ib_device,
-					    work->port->port_num, 0,
-					    &work->path[0].sgid,
-					    &gid_attr);
-		if (!err && gid_attr.ndev) {
-			work->path[0].rec_type =
-				sa_conv_gid_to_pathrec_type(gid_attr.gid_type);
-			sa_path_set_ifindex(&work->path[0],
-					    gid_attr.ndev->ifindex);
-			sa_path_set_ndev(&work->path[0],
-					 dev_net(gid_attr.ndev));
-			dev_put(gid_attr.ndev);
-		} else {
-			cm_path_set_rec_type(work->port->cm_dev->ib_device,
-					     work->port->port_num,
-					     &work->path[0],
-					     &req_msg->primary_local_gid);
-		}
-		if (cm_req_has_alt_path(req_msg))
-			work->path[1].rec_type = work->path[0].rec_type;
-		ib_send_cm_rej(cm_id, IB_CM_REJ_INVALID_GID,
-			       &work->path[0].sgid, sizeof work->path[0].sgid,
-			       NULL, 0);
+		int err;
+
+		err = ib_get_cached_gid(work->port->cm_dev->ib_device,
+					work->port->port_num, 0,
+					&work->path[0].sgid,
+					NULL);
+		if (err)
+			ib_send_cm_rej(cm_id, IB_CM_REJ_INVALID_GID,
+				       NULL, 0, NULL, 0);
+		else
+			ib_send_cm_rej(cm_id, IB_CM_REJ_INVALID_GID,
+				       &work->path[0].sgid,
+				       sizeof(work->path[0].sgid),
+				       NULL, 0);
 		goto rejected;
 	}
 	if (cm_req_has_alt_path(req_msg)) {
@@ -1926,7 +1922,7 @@ static int cm_req_handler(struct cm_work *work)
 		if (ret) {
 			ib_send_cm_rej(cm_id, IB_CM_REJ_INVALID_ALT_GID,
 				       &work->path[0].sgid,
-				       sizeof work->path[0].sgid, NULL, 0);
+				       sizeof(work->path[0].sgid), NULL, 0);
 			goto rejected;
 		}
 	}
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 31/31] IB/cm: Refactor to avoid setting path record software only fields

[Or Gerlitz]

On Tue, Nov 14, 2017 at 2:52 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> When path ah_attr initialization from path record
> fails, ib_cm_send_rej() uses av.ah_attr fields to send out reject
> message. In such cases initialization of path record software fields
> is not needed. Code is simplified for same.

is "Code is simplified for same"  cut in the middle, I fail to
understand the sentence.


> Additionally in current code in cm_req_handler, when ib_get_cached_gid
> fails for a given sgid_index of the GID of the GRH of the incoming CM MAD,
> error code 12 is sent. This error code refers to primary GID in incoming
> CM REQ and not for the GID in in MAD packet.
> Therefore code is refactored to send code 5 (unsupported request) for such
> error.

You do two logical changes in one patch, doesn't seem right to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma-next 31/31] IB/cm: Refactor to avoid setting path record software only fields

[Parav Pandit]

> -----Original Message-----
> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> Sent: Tuesday, November 14, 2017 3:13 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: Doug Ledford <dledford@redhat.com>; linux-rdma@vger.kernel.org; Hefty,
> Sean <sean.hefty@intel.com>; Leon Romanovsky <leonro@mellanox.com>
> Subject: Re: [PATCH rdma-next 31/31] IB/cm: Refactor to avoid setting path
> record software only fields
> 
> On Tue, Nov 14, 2017 at 2:52 PM, Leon Romanovsky <leon@kernel.org> wrote:
> > From: Parav Pandit <parav@mellanox.com>
> >
> > When path ah_attr initialization from path record fails,
> > ib_cm_send_rej() uses av.ah_attr fields to send out reject message. In
> > such cases initialization of path record software fields is not
> > needed. Code is simplified for same.
> 
> is "Code is simplified for same"  cut in the middle, I fail to understand the
> sentence.
> 
Code is simplified to avoid such initialization.

> 
> > Additionally in current code in cm_req_handler, when ib_get_cached_gid
> > fails for a given sgid_index of the GID of the GRH of the incoming CM
> > MAD, error code 12 is sent. This error code refers to primary GID in
> > incoming CM REQ and not for the GID in in MAD packet.
> > Therefore code is refactored to send code 5 (unsupported request) for
> > such error.
> 
> You do two logical changes in one patch, doesn't seem right to me.
Both logical changes are in same function few lines apart around GID check.
So Hal and I continued to have in one patch.

Re: [PATCH rdma-next 31/31] IB/cm: Refactor to avoid setting path record software only fields

[Or Gerlitz]

On Wed, Nov 15, 2017 at 12:11 AM, Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> From: Or Gerlitz [mailto:gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]

>> On Tue, Nov 14, 2017 at 2:52 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> >
>> > When path ah_attr initialization from path record fails,
>> > ib_cm_send_rej() uses av.ah_attr fields to send out reject message. In
>> > such cases initialization of path record software fields is not
>> > needed. Code is simplified for same.
>>
>> is "Code is simplified for same"  cut in the middle, I fail to understand the
>> sentence.
>>
> Code is simplified to avoid such initialization.

so fix  that in the next version, ok?

>> You do two logical changes in one patch, doesn't seem right to me.

> Both logical changes are in same function few lines apart around GID check.
> So Hal and I continued to have in one patch.


As long as it's not terribly problematic, patch has to carry one logical change,
I don't see why not apply this practice here too.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 00/31] RDMA fixes and refactoring for 4.15

[Jason Gunthorpe]

On Tue, Nov 14, 2017 at 02:51:47PM +0200, Leon Romanovsky wrote:
> Hi Doug,
> 
> Please consider to take the following series to your coming pull request.
> 
> The patches are available in the git repository at:
>   git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git tags/rdma-next-2017-11-14
> 
> 	Thanks

I applied this series excluding:

   IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled

As noted in my reply to that patch.

I revised many of the commit messages, lets try to improve these please.

I also revised this patch:
   IB/ipoib: Avoid memory leak if neigh destination was changed

To use ipoib_debug as noted in my separate message.

Everything is in the wip/jgg-zero-day branch, it will move to for-next
my tomorrow. Let me know if there are any serious problems with my
edits.

Thanks,
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 00/31] RDMA fixes and refactoring for 4.15

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1251 bytes --]

On Mon, Dec 18, 2017 at 04:11:20PM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 14, 2017 at 02:51:47PM +0200, Leon Romanovsky wrote:
> > Hi Doug,
> >
> > Please consider to take the following series to your coming pull request.
> >
> > The patches are available in the git repository at:
> >   git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git tags/rdma-next-2017-11-14
> >
> > 	Thanks
>
> I applied this series excluding:
>
>    IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled
>
> As noted in my reply to that patch.
>
> I revised many of the commit messages, lets try to improve these please.
>
> I also revised this patch:
>    IB/ipoib: Avoid memory leak if neigh destination was changed
>
> To use ipoib_debug as noted in my separate message.
>
> Everything is in the wip/jgg-zero-day branch, it will move to for-next
> my tomorrow. Let me know if there are any serious problems with my
> edits.

I took a glance look on the messages and they seem ok to me.

Thanks a lot.

>
> Thanks,
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-next 00/31] RDMA fixes and refactoring for 4.15

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]

On Mon, Dec 18, 2017 at 04:11:20PM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 14, 2017 at 02:51:47PM +0200, Leon Romanovsky wrote:
> > Hi Doug,
> >
> > Please consider to take the following series to your coming pull request.
> >
> > The patches are available in the git repository at:
> >   git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git tags/rdma-next-2017-11-14
> >
> > 	Thanks
>
> I applied this series excluding:
>
>    IB/core: Fix do not add RoCEv2 default GID when IPv6 is disabled
>
> As noted in my reply to that patch.
>
> I revised many of the commit messages, lets try to improve these please.
>
> I also revised this patch:
>    IB/ipoib: Avoid memory leak if neigh destination was changed
>
> To use ipoib_debug as noted in my separate message.
>
> Everything is in the wip/jgg-zero-day branch, it will move to for-next
> my tomorrow. Let me know if there are any serious problems with my
> edits.

Jason,

I have a hard time to find the commit 09/31
https://patchwork.kernel.org/patch/10057549/

It is marked as superseded, but I wasn't supposed to send any new
version of it.

Thanks

>
> Thanks,
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-next 00/31] RDMA fixes and refactoring for 4.15

[Jason Gunthorpe]

On Tue, Dec 26, 2017 at 08:49:14PM +0200, Leon Romanovsky wrote:
> > Everything is in the wip/jgg-zero-day branch, it will move to for-next
> > my tomorrow. Let me know if there are any serious problems with my
> > edits.
> 
> I have a hard time to find the commit 09/31
> https://patchwork.kernel.org/patch/10057549/
> 
> It is marked as superseded, but I wasn't supposed to send any new
> version of it.

I must have confused it with the other PSN patch you re-sent to go to
-rc (RDMA/cma: Make sure that PSN is not over max allowed)

Can you just mark it as New in patchworks?

Thanks,
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 00/31] RDMA fixes and refactoring for 4.15

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

On Tue, Dec 26, 2017 at 12:49:26PM -0700, Jason Gunthorpe wrote:
> On Tue, Dec 26, 2017 at 08:49:14PM +0200, Leon Romanovsky wrote:
> > > Everything is in the wip/jgg-zero-day branch, it will move to for-next
> > > my tomorrow. Let me know if there are any serious problems with my
> > > edits.
> >
> > I have a hard time to find the commit 09/31
> > https://patchwork.kernel.org/patch/10057549/
> >
> > It is marked as superseded, but I wasn't supposed to send any new
> > version of it.
>
> I must have confused it with the other PSN patch you re-sent to go to
> -rc (RDMA/cma: Make sure that PSN is not over max allowed)
>
> Can you just mark it as New in patchworks?

Done, thanks

>
> Thanks,
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]