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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
> > > 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 <[EMAIL PROTECTED]>
> > > ---
> > >  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.

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

-- Hal

> Ira
> _______________________________________________
> general mailing list
> [email protected]
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> 
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to