diff mbox

[rdma-rc,5/9] IB/mlx4: Handle well-known-gid in mad_demux processing

Message ID 1478375842-21513-6-git-send-email-leonro@mellanox.com (mailing list archive)
State Superseded
Headers show

Commit Message

Leon Romanovsky Nov. 5, 2016, 7:57 p.m. UTC
From: Jack Morgenstein <jackm@dev.mellanox.co.il>

If OpenSM runs over ConnectX-3, and there are ConnectIB VFs active
on the network, the OpenSM will receive QP1 packets containing a GRH
where the destination GID is the "Well-Known GID" -- which is not a GID
in the HCA Port's GID Table.

This GID must be tested-for separately -- and packets which contain
this destination GID should be routed to slave 0 (the PF).

Fixes: 37bfc7c1e83f ('IB/mlx4: SR-IOV multiplex and demultiplex MADs')
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
Signed-off-by: Leon Romanovsky <leon@kernel.org>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/hw/mlx4/mad.c | 16 ++++++++++++----
 include/rdma/ib_verbs.h          |  1 +
 2 files changed, 13 insertions(+), 4 deletions(-)

Comments

Hal Rosenstock Nov. 5, 2016, 9:03 p.m. UTC | #1
On 11/5/2016 3:57 PM, Leon Romanovsky wrote:
> From: Jack Morgenstein <jackm@dev.mellanox.co.il>
> 
> If OpenSM runs over ConnectX-3, and there are ConnectIB VFs active
> on the network, the OpenSM will receive QP1 packets containing a GRH
> where the destination GID is the "Well-Known GID" -- which is not a GID
> in the HCA Port's GID Table.
> 
> This GID must be tested-for separately -- and packets which contain
> this destination GID should be routed to slave 0 (the PF).
> 
> Fixes: 37bfc7c1e83f ('IB/mlx4: SR-IOV multiplex and demultiplex MADs')
> Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
> Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
> Signed-off-by: Leon Romanovsky <leon@kernel.org>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
>  drivers/infiniband/hw/mlx4/mad.c | 16 ++++++++++++----
>  include/rdma/ib_verbs.h          |  1 +
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
> index b8e9013..f0b8272 100644
> --- a/drivers/infiniband/hw/mlx4/mad.c
> +++ b/drivers/infiniband/hw/mlx4/mad.c
> @@ -729,10 +729,18 @@ static int mlx4_ib_demux_mad(struct ib_device *ibdev, u8 port,
>  
>  	/* If a grh is present, we demux according to it */
>  	if (wc->wc_flags & IB_WC_GRH) {
> -		slave = mlx4_ib_find_real_gid(ibdev, port, grh->dgid.global.interface_id);
> -		if (slave < 0) {
> -			mlx4_ib_warn(ibdev, "failed matching grh\n");
> -			return -ENOENT;
> +		if (grh->dgid.global.interface_id ==
> +			cpu_to_be64(IB_SA_WELL_KNOWN_GUID) &&
> +		    grh->dgid.global.subnet_prefix ==
> +			cpu_to_be64(IB_SA_WELL_KNOWN_GID_PREFIX)) {
> +			slave = 0;
> +		} else {
> +			slave = mlx4_ib_find_real_gid(ibdev, port,
> +						      grh->dgid.global.interface_id);
> +			if (slave < 0) {
> +				mlx4_ib_warn(ibdev, "failed matching grh\n");
> +				return -ENOENT;
> +			}
>  		}
>  	}
>  	/* Class-specific handling */
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 467a4b4..da5949f 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -100,6 +100,7 @@ enum rdma_node_type {
>  
>  enum {
>  	/* set the local administered indication */
> +	IB_SA_WELL_KNOWN_GID_PREFIX = 0xfe80000000000000ull,
>  	IB_SA_WELL_KNOWN_GUID	= BIT_ULL(57) | 2,

Isn't the SA well-known GID the concatenation of the subnet prefix
(which isn't necessarily the default link local one) and the GUID
0x0200000000000002 ?

-- Hal

>  };
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
jackm Nov. 6, 2016, 6:41 p.m. UTC | #2
On Sat, 5 Nov 2016 17:03:25 -0400
Hal Rosenstock <hal@dev.mellanox.co.il> wrote:

> Isn't the SA well-known GID the concatenation of the subnet prefix
> (which isn't necessarily the default link local one) and the GUID
> 0x0200000000000002 ?

You are correct regarding the subnet prefix (it should be the port's
current subnet prefix, and not the default subnet prefix).

Regarding the SM GUID, the version of the Virtualization Annex that I
worked with (v17 -- Feb 16, 2015) states:
SM GID	A well-known GID that is associated with the SM,
comprising the concatenation of the Subnet prefix and the GUID 0x2. The
SM GID is never present in any GID Table.

Has the SM GUID value changed (0x2 -->0x0200000000000002) in the Annex
since v17? (I'm not a member of the MGTWG workgroup, so I don't have
access to the most recent version).

-Jack 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Or Gerlitz Nov. 6, 2016, 8:46 p.m. UTC | #3
On Sun, Nov 6, 2016 at 8:41 PM, jackm <jackm@dev.mellanox.co.il> wrote:
> On Sat, 5 Nov 2016 17:03:25 -0400
> Hal Rosenstock <hal@dev.mellanox.co.il> wrote:
>
>> Isn't the SA well-known GID the concatenation of the subnet prefix
>> (which isn't necessarily the default link local one) and the GUID
>> 0x0200000000000002 ?
>
> You are correct regarding the subnet prefix (it should be the port's
> current subnet prefix, and not the default subnet prefix).
>
> Regarding the SM GUID, the version of the Virtualization Annex that I
> worked with (v17 -- Feb 16, 2015) states:
> SM GID  A well-known GID that is associated with the SM,
> comprising the concatenation of the Subnet prefix and the GUID 0x2. The
> SM GID is never present in any GID Table.
>
> Has the SM GUID value changed (0x2 -->0x0200000000000002) in the Annex
> since v17? (I'm not a member of the MGTWG workgroup, so I don't have
> access to the most recent version).

Jack, I see that in Linux we've picked the value Hal is mentioning,
see Eli Cohen's commit a0c1b2a3 "IB/core: Support accessing SA in
virtualized environment". You probably can talk to Eli to get where
things stand.

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Or Gerlitz Nov. 6, 2016, 8:51 p.m. UTC | #4
On Sun, Nov 6, 2016 at 10:46 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Sun, Nov 6, 2016 at 8:41 PM, jackm <jackm@dev.mellanox.co.il> wrote:
>> On Sat, 5 Nov 2016 17:03:25 -0400
>> Hal Rosenstock <hal@dev.mellanox.co.il> wrote:
>>
>>> Isn't the SA well-known GID the concatenation of the subnet prefix
>>> (which isn't necessarily the default link local one) and the GUID
>>> 0x0200000000000002 ?
>>
>> You are correct regarding the subnet prefix (it should be the port's
>> current subnet prefix, and not the default subnet prefix).
>>
>> Regarding the SM GUID, the version of the Virtualization Annex that I
>> worked with (v17 -- Feb 16, 2015) states:
>> SM GID  A well-known GID that is associated with the SM,
>> comprising the concatenation of the Subnet prefix and the GUID 0x2. The
>> SM GID is never present in any GID Table.
>>
>> Has the SM GUID value changed (0x2 -->0x0200000000000002) in the Annex
>> since v17? (I'm not a member of the MGTWG workgroup, so I don't have
>> access to the most recent version).
>
> Jack, I see that in Linux we've picked the value Hal is mentioning,
> see Eli Cohen's commit a0c1b2a3 "IB/core: Support accessing SA in
> virtualized environment". You probably can talk to Eli to get where
> things stand.

yep, looking now on the annex I see the following:

SM GID -- A well-known GID that is associated with the SM, comprising
the concatenation of the Subnet prefix and the GUID
0x0200000000000002. The
SM GID is never present in any GID Table. If the SubnetPrefix is modified
by the SM, the SM GID is updated implicitly
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
jackm Nov. 7, 2016, 2:42 p.m. UTC | #5
On Sat, 5 Nov 2016 17:03:25 -0400
Hal Rosenstock <hal@dev.mellanox.co.il> wrote:

> On 11/5/2016 3:57 PM, Leon Romanovsky wrote:
> > From: Jack Morgenstein <jackm@dev.mellanox.co.il>
> > 
> > If OpenSM runs over ConnectX-3, and there are ConnectIB VFs active
> > on the network, the OpenSM will receive QP1 packets containing a GRH
> > where the destination GID is the "Well-Known GID" -- which is not a
> > GID in the HCA Port's GID Table.
> > 
> > This GID must be tested-for separately -- and packets which contain
> > this destination GID should be routed to slave 0 (the PF).
> > 
> > Fixes: 37bfc7c1e83f ('IB/mlx4: SR-IOV multiplex and demultiplex
> > MADs') Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
> > Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leon@kernel.org>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> >  drivers/infiniband/hw/mlx4/mad.c | 16 ++++++++++++----
> >  include/rdma/ib_verbs.h          |  1 +
> >  2 files changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/infiniband/hw/mlx4/mad.c
> > b/drivers/infiniband/hw/mlx4/mad.c index b8e9013..f0b8272 100644
> > --- a/drivers/infiniband/hw/mlx4/mad.c
> > +++ b/drivers/infiniband/hw/mlx4/mad.c
> > @@ -729,10 +729,18 @@ static int mlx4_ib_demux_mad(struct ib_device
> > *ibdev, u8 port, 
> >  	/* If a grh is present, we demux according to it */
> >  	if (wc->wc_flags & IB_WC_GRH) {
> > -		slave = mlx4_ib_find_real_gid(ibdev, port,
> > grh->dgid.global.interface_id);
> > -		if (slave < 0) {
> > -			mlx4_ib_warn(ibdev, "failed matching
> > grh\n");
> > -			return -ENOENT;
> > +		if (grh->dgid.global.interface_id ==
> > +			cpu_to_be64(IB_SA_WELL_KNOWN_GUID) &&
> > +		    grh->dgid.global.subnet_prefix ==
> > +			cpu_to_be64(IB_SA_WELL_KNOWN_GID_PREFIX)) {
> > +			slave = 0;
> > +		} else {
> > +			slave = mlx4_ib_find_real_gid(ibdev, port,
> > +
> > grh->dgid.global.interface_id);
> > +			if (slave < 0) {
> > +				mlx4_ib_warn(ibdev, "failed
> > matching grh\n");
> > +				return -ENOENT;
> > +			}
> >  		}
> >  	}
> >  	/* Class-specific handling */
> > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > index 467a4b4..da5949f 100644
> > --- a/include/rdma/ib_verbs.h
> > +++ b/include/rdma/ib_verbs.h
> > @@ -100,6 +100,7 @@ enum rdma_node_type {
> >  
> >  enum {
> >  	/* set the local administered indication */
> > +	IB_SA_WELL_KNOWN_GID_PREFIX = 0xfe80000000000000ull,
> >  	IB_SA_WELL_KNOWN_GUID	= BIT_ULL(57) | 2,  
> 
> Isn't the SA well-known GID the concatenation of the subnet prefix
> (which isn't necessarily the default link local one) and the GUID
> 0x0200000000000002 ?
> 
> -- Hal

PLEASE DO NOT APPLY THIS PATCH. Hal is correct, and the patch needs
to be fixed. We will submit an updated (i.e. fixed) version.

-Jack

> 
> >  };
> >  
> >   

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

Patch

diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index b8e9013..f0b8272 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -729,10 +729,18 @@  static int mlx4_ib_demux_mad(struct ib_device *ibdev, u8 port,
 
 	/* If a grh is present, we demux according to it */
 	if (wc->wc_flags & IB_WC_GRH) {
-		slave = mlx4_ib_find_real_gid(ibdev, port, grh->dgid.global.interface_id);
-		if (slave < 0) {
-			mlx4_ib_warn(ibdev, "failed matching grh\n");
-			return -ENOENT;
+		if (grh->dgid.global.interface_id ==
+			cpu_to_be64(IB_SA_WELL_KNOWN_GUID) &&
+		    grh->dgid.global.subnet_prefix ==
+			cpu_to_be64(IB_SA_WELL_KNOWN_GID_PREFIX)) {
+			slave = 0;
+		} else {
+			slave = mlx4_ib_find_real_gid(ibdev, port,
+						      grh->dgid.global.interface_id);
+			if (slave < 0) {
+				mlx4_ib_warn(ibdev, "failed matching grh\n");
+				return -ENOENT;
+			}
 		}
 	}
 	/* Class-specific handling */
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 467a4b4..da5949f 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -100,6 +100,7 @@  enum rdma_node_type {
 
 enum {
 	/* set the local administered indication */
+	IB_SA_WELL_KNOWN_GID_PREFIX = 0xfe80000000000000ull,
 	IB_SA_WELL_KNOWN_GUID	= BIT_ULL(57) | 2,
 };