diff mbox

cma: resolve to first active IB port

Message ID CAMGffE=mSpm+zuG9wo-GV0HT4-ewg34uvOKCUrPRC61ATjxyVg@mail.gmail.com (mailing list archive)
State Superseded
Headers show

Commit Message

Jinpu Wang Dec. 6, 2016, 11:16 a.m. UTC
Hi Doug, Sean and Hal,

We have problem when connect connection from second port with AF_IB,
when first port is down.
We will fail when resovle route.

It turned out when resolve addr, in cma_resolve_ib_dev, we will choose
the first port if subnet_prefix is the same, in our case we use
default well known prefix.

I proposal fix below to resolve to first acrive IB port:
Note: I will attach another one for easily apply if the format mangled by gmail.
Patch created based on Linux 4.9-rc7.

From b42e51deb8da3c1b596cb4332b6815e5ac8658a7 Mon Sep 17 00:00:00 2001
From: Jack Wang <jinpu.wang@profitbricks.com>
Date: Tue, 6 Dec 2016 09:01:04 +0100
Subject: [PATCH] cma: resolve to first active ib_port

When resolve addr if we don't give src addr, cma core will try to resolve to
ib device on itself, current logic is only check if it has same
subnet_prefix, which is not enough if we use default well known gid,
we should also check if port is active.

Signed-off-by: Jack Wang <jinpu.wang@profitbricks.com>
---
 drivers/infiniband/core/cma.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

                     sgid = gid;
@@ -692,7 +694,9 @@ static int cma_resolve_ib_dev(struct
rdma_id_private *id_priv)
                 }

                 if (!cma_dev && (gid.global.subnet_prefix ==
-                         dgid->global.subnet_prefix)) {
+                    dgid->global.subnet_prefix) &&
+                    (!ib_query_port(cur_dev->device, p, &attr) &&
+                    attr.state == IB_PORT_ACTIVE)) {
                     cma_dev = cur_dev;
                     sgid = gid;
                     id_priv->id.port_num = p;

Comments

Jinpu Wang Dec. 8, 2016, 4:47 p.m. UTC | #1
On Tue, Dec 6, 2016 at 12:16 PM, Jinpu Wang <jinpu.wang@profitbricks.com> wrote:
> Hi Doug, Sean and Hal,
>
> We have problem when connect connection from second port with AF_IB,
> when first port is down.
> We will fail when resovle route.
>
> It turned out when resolve addr, in cma_resolve_ib_dev, we will choose
> the first port if subnet_prefix is the same, in our case we use
> default well known prefix.
>
> I proposal fix below to resolve to first acrive IB port:
> Note: I will attach another one for easily apply if the format mangled by gmail.
> Patch created based on Linux 4.9-rc7.
>
> From b42e51deb8da3c1b596cb4332b6815e5ac8658a7 Mon Sep 17 00:00:00 2001
> From: Jack Wang <jinpu.wang@profitbricks.com>
> Date: Tue, 6 Dec 2016 09:01:04 +0100
> Subject: [PATCH] cma: resolve to first active ib_port
>
> When resolve addr if we don't give src addr, cma core will try to resolve to
> ib device on itself, current logic is only check if it has same
> subnet_prefix, which is not enough if we use default well known gid,
> we should also check if port is active.
>
> Signed-off-by: Jack Wang <jinpu.wang@profitbricks.com>
> ---
>  drivers/infiniband/core/cma.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 2a6fc47..9e46b42 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -684,6 +684,8 @@ static int cma_resolve_ib_dev(struct
> rdma_id_private *id_priv)
>              for (i = 0; !ib_get_cached_gid(cur_dev->device, p, i,
>                                 &gid, NULL);
>                   i++) {
> +                struct ib_port_attr attr;
> +
>                  if (!memcmp(&gid, dgid, sizeof(gid))) {
>                      cma_dev = cur_dev;
>                      sgid = gid;
> @@ -692,7 +694,9 @@ static int cma_resolve_ib_dev(struct
> rdma_id_private *id_priv)
>                  }
>
>                  if (!cma_dev && (gid.global.subnet_prefix ==
> -                         dgid->global.subnet_prefix)) {
> +                    dgid->global.subnet_prefix) &&
> +                    (!ib_query_port(cur_dev->device, p, &attr) &&
> +                    attr.state == IB_PORT_ACTIVE)) {
>                      cma_dev = cur_dev;
>                      sgid = gid;
>                      id_priv->id.port_num = p;
> --
> 2.7.4

ping?
Hefty, Sean Dec. 8, 2016, 5:47 p.m. UTC | #2
> > diff --git a/drivers/infiniband/core/cma.c

> b/drivers/infiniband/core/cma.c

> > index 2a6fc47..9e46b42 100644

> > --- a/drivers/infiniband/core/cma.c

> > +++ b/drivers/infiniband/core/cma.c

> > @@ -684,6 +684,8 @@ static int cma_resolve_ib_dev(struct

> > rdma_id_private *id_priv)

> >              for (i = 0; !ib_get_cached_gid(cur_dev->device, p, i,

> >                                 &gid, NULL);

> >                   i++) {

> > +                struct ib_port_attr attr;

> > +

> >                  if (!memcmp(&gid, dgid, sizeof(gid))) {

> >                      cma_dev = cur_dev;

> >                      sgid = gid;

> > @@ -692,7 +694,9 @@ static int cma_resolve_ib_dev(struct

> > rdma_id_private *id_priv)

> >                  }

> >

> >                  if (!cma_dev && (gid.global.subnet_prefix ==

> > -                         dgid->global.subnet_prefix)) {

> > +                    dgid->global.subnet_prefix) &&

> > +                    (!ib_query_port(cur_dev->device, p, &attr) &&

> > +                    attr.state == IB_PORT_ACTIVE)) {


I do have a concern about accessing non-cached port data as part of this call.  Can we cache the port state and use that instead?
diff mbox

Patch

From b42e51deb8da3c1b596cb4332b6815e5ac8658a7 Mon Sep 17 00:00:00 2001
From: Jack Wang <jinpu.wang@profitbricks.com>
Date: Tue, 6 Dec 2016 09:01:04 +0100
Subject: [PATCH] cma: resolve to first active ib_port

When resolve addr if we don't give src addr, cma core will try to resolve to
ib device on itself, current logic is only check if it has same
subnet_prefix, which is not enough if we use default well known gid,
we should also check if port is active.

Signed-off-by: Jack Wang <jinpu.wang@profitbricks.com>
---
 drivers/infiniband/core/cma.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 2a6fc47..9e46b42 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -684,6 +684,8 @@  static int cma_resolve_ib_dev(struct rdma_id_private *id_priv)
 			for (i = 0; !ib_get_cached_gid(cur_dev->device, p, i,
 						       &gid, NULL);
 			     i++) {
+				struct ib_port_attr attr;
+
 				if (!memcmp(&gid, dgid, sizeof(gid))) {
 					cma_dev = cur_dev;
 					sgid = gid;
@@ -692,7 +694,9 @@  static int cma_resolve_ib_dev(struct rdma_id_private *id_priv)
 				}
 
 				if (!cma_dev && (gid.global.subnet_prefix ==
-						 dgid->global.subnet_prefix)) {
+				    dgid->global.subnet_prefix) &&
+				    (!ib_query_port(cur_dev->device, p, &attr) &&
+				    attr.state == IB_PORT_ACTIVE)) {
 					cma_dev = cur_dev;
 					sgid = gid;
 					id_priv->id.port_num = p;
-- 
2.7.4