[ofa-general] [PATCH] opensm/include/iba/ib_types.h: fix DataDetails definitions based on 1.2 and 1.2.1 specification

Ira Weiny weiny2 at llnl.gov
Wed Mar 12 14:10:15 PDT 2008


On Wed, 12 Mar 2008 13:21:24 -0700
Hal Rosenstock <hrosenstock at xsigo.com> wrote:

> On Wed, 2008-03-12 at 12:54 -0700, Ira Weiny wrote:
> > Hey Hal,
> > 
> > On Wed, 12 Mar 2008 11:07:02 -0700
> > Hal Rosenstock <hrosenstock at xsigo.com> wrote:
> > 
> > > On Wed, 2008-03-12 at 10:23 -0700, Ira Weiny wrote:
> > > > While making changes to the DataDetails for trap 144 I noticed that trap 256 and 259 were wrong.
> > > > 
> > > > This patch should fix them acording to both the 1.2 and 1.2.1 spec.
> > > > 
> > > > IRa
> > > > 
> > > > 
> > > > >From 9ad1430729151fab371b98fce82e28b33c49f036 Mon Sep 17 00:00:00 2001
> > > > From: Ira K. Weiny <weiny2 at llnl.gov>
> > > > Date: Mon, 10 Mar 2008 13:09:45 -0700
> > > > Subject: [PATCH] opensm/include/iba/ib_types.h: fix DataDetails definitions based on 1.2 and
> > > > 1.2.1 specification
> > > > 
> > > > Signed-off-by: Ira K. Weiny <weiny2 at llnl.gov>
> > > > ---
> > > >  opensm/include/iba/ib_types.h |   12 +++++++-----
> > > >  1 files changed, 7 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/opensm/include/iba/ib_types.h b/opensm/include/iba/ib_types.h
> > > > index a026ac7..f80d0d5 100644
> > > > --- a/opensm/include/iba/ib_types.h
> > > > +++ b/opensm/include/iba/ib_types.h
> > > > @@ -7160,13 +7160,13 @@ typedef struct _ib_mad_notice_attr	// Total Size calc  Accumulated
> > > >  		struct _ntc_256 {	// total: 54
> > > >  			ib_net16_t pad1;	// 2
> > > >  			ib_net16_t lid;	// 2
> > > > -			ib_net16_t pad2;	// 2
> > > > +			ib_net16_t dr_slid;	// 2
> > > >  			uint8_t method;	// 1
> > > > -			uint8_t pad3;	// 1
> > > > +			uint8_t pad2;	// 1
> > > >  			ib_net16_t attr_id;	// 2
> > > >  			ib_net32_t attr_mod;	// 4
> > > >  			ib_net64_t mkey;	// 8
> > > > -			uint8_t dr_slid;	// 1
> > > > +			uint8_t pad3;	// 1
> > > >  			uint8_t dr_trunc_hop;	// 1
> > > >  			uint8_t dr_rtn_path[30];	// 30
> > > >  		} PACK_SUFFIX ntc_256;
> > > > @@ -7189,9 +7189,11 @@ typedef struct _ib_mad_notice_attr	// Total Size calc  Accumulated
> > > >  			ib_net16_t data_valid;	// 2
> > > >  			ib_net16_t lid1;	// 2
> > > >  			ib_net16_t lid2;	// 2
> > > > -			ib_net32_t key;	// 4
> > > > +			ib_net16_t key;	// 4
> > > 
> > > Isn't key still 32 bits ?
> > 
> > In 1.2.1, I see on page 825 Table 140 "Notice DataDetails for Trap 259"
> > 
> > Field  Length (bits)   Description
> > PKEY   16              P_Key
> > 
> > In 1.2, the table is on page 817 Table 139.
> 
> It can be QKey also (trap 258). PKey is encapsulated in the 32 bits as
> indicated in the description.

But trap 257 and 258 have a separate structure:

		struct _ntc_257_258	// violation of p/q_key // 49
		{
			ib_net16_t pad1;	// 2
			ib_net16_t lid1;	// 2
			ib_net16_t lid2;	// 2
			ib_net32_t key;	// 2
			uint8_t sl;	// 1
			ib_net32_t qp1;	// 4
			ib_net32_t qp2;	// 4
			ib_gid_t gid1;	// 16
			ib_gid_t gid2;	// 16
		} PACK_SUFFIX ntc_257_258;

Upon closer inspection I have revised the patch to update the comment section
as well.

> 
> >   I assumed it was just a copy/paste error in the code?
> 
> > > 
> > > >  			uint8_t sl;	// 1
> > > > -			ib_net32_t qp1;	// 4
> > > > +			uint8_t qp1_msb;	// 1
> > > > +			ib_net16_t qp1_lsb;	// 2
> > > > +			uint8_t pad;	// 1
> > > >  			uint8_t qp2_msb;	// 1
> > > >  			ib_net16_t qp2_lsb;	// 2
> > > 
> > > I think splitting up QPN like this would make use harder.
> > > 
> > 
> > We could get rid of that.  But qp2 is split so I figured there was precedence
> > to use msb/lsb.  Also I like the fact it is more explicit which bits are the
> > qp.  I thought some might use the 32bit value including the pad accidentally.
> 
> Maybe there's precedence but it might be bad predecence. I think it
> makes it harder to do any endian conversions. It does clarify the bits
> but that can be done another way (e.g. comments).
> 
> > It's your call,
> 
> Not mine anymore :-)
> 

Ok, I have changed this in the new patch as well.  Tell me what you think.

Ira

>From 0476b0d102ada643db9abf6271aae8a540a417d7 Mon Sep 17 00:00:00 2001
From: Ira K. Weiny <weiny2 at llnl.gov>
Date: Mon, 10 Mar 2008 13:09:45 -0700
Subject: [PATCH] opensm/include/iba/ib_types.h: fix DataDetails definitions based on 1.2 and
1.2.1 specification

Signed-off-by: Ira K. Weiny <weiny2 at llnl.gov>
---
 opensm/include/iba/ib_types.h |   22 +++++++++++++---------
 1 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/opensm/include/iba/ib_types.h b/opensm/include/iba/ib_types.h
index 649ef1c..e160200 100644
--- a/opensm/include/iba/ib_types.h
+++ b/opensm/include/iba/ib_types.h
@@ -7158,13 +7158,13 @@ typedef struct _ib_mad_notice_attr	// Total Size calc  Accumulated
 		struct _ntc_256 {	// total: 54
 			ib_net16_t pad1;	// 2
 			ib_net16_t lid;	// 2
-			ib_net16_t pad2;	// 2
+			ib_net16_t dr_slid;	// 2
 			uint8_t method;	// 1
-			uint8_t pad3;	// 1
+			uint8_t pad2;	// 1
 			ib_net16_t attr_id;	// 2
 			ib_net32_t attr_mod;	// 4
 			ib_net64_t mkey;	// 8
-			uint8_t dr_slid;	// 1
+			uint8_t pad3;	// 1
 			uint8_t dr_trunc_hop;	// 1
 			uint8_t dr_rtn_path[30];	// 30
 		} PACK_SUFFIX ntc_256;
@@ -7182,16 +7182,14 @@ typedef struct _ib_mad_notice_attr	// Total Size calc  Accumulated
 			ib_gid_t gid2;	// 16
 		} PACK_SUFFIX ntc_257_258;
 
-		struct _ntc_259	// p/q_key violation with sw info 53
+		struct _ntc_259	// pkey violation from switch 51
 		{
 			ib_net16_t data_valid;	// 2
 			ib_net16_t lid1;	// 2
 			ib_net16_t lid2;	// 2
-			ib_net32_t key;	// 4
-			uint8_t sl;	// 1
-			ib_net32_t qp1;	// 4
-			uint8_t qp2_msb;	// 1
-			ib_net16_t qp2_lsb;	// 2
+			ib_net16_t pkey;	// 2
+			ib_net32_t sl_qp1; // 4b sl, 4b pad, 24b qp1
+			ib_net32_t qp2; // 8b pad, 24b qp2
 			ib_gid_t gid1;	// 16
 			ib_gid_t gid2;	// 16
 			ib_net16_t sw_lid;	// 2
@@ -7205,6 +7203,12 @@ typedef struct _ib_mad_notice_attr	// Total Size calc  Accumulated
 } PACK_SUFFIX ib_mad_notice_attr_t;
 #include <complib/cl_packoff.h>
 
+/**
+ * Trap 259 masks
+ */
+#define TRAP_259_MASK_SL (CL_HTON32(0xF0000000))
+#define TRAP_259_MASK_QP (CL_HTON32(0x00FFFFFF))
+
 /****f* IBA Base: Types/ib_notice_is_generic
 * NAME
 *	ib_notice_is_generic
-- 
1.5.1




More information about the general mailing list