Re: [PATCH] opensm: Add infrastructure support for PortInfo IsMulticastPkeyTrapSuppressionSupported

[Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>]

On 09:15 Tue 13 Oct     , Hal Rosenstock wrote:
> On Mon, Oct 12, 2009 at 2:45 PM, Sasha Khapyorsky <sashak-smomgflXvObQFizaE/u3fw@public.gmane.orgm> wrote:
> > On 10:46 Fri 09 Oct     , Hal Rosenstock wrote:
> >>
> >> Per published MgtWG errata RefID 4576
> >>
> >> Signed-off-by: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> ---
> >> diff --git a/opensm/include/iba/ib_types.h b/opensm/include/iba/ib_types.h
> >> index 25ed35f..c820372 100644
> >> --- a/opensm/include/iba/ib_types.h
> >> +++ b/opensm/include/iba/ib_types.h
> >> @@ -4432,7 +4432,7 @@ typedef struct _ib_port_info {
> >>       ib_net16_t p_key_violations;
> >>       ib_net16_t q_key_violations;
> >>       uint8_t guid_cap;
> >> -     uint8_t subnet_timeout; /* cli_rereg(1b), resrv(2b), timeout(5b) */
> >> +     uint8_t subnet_timeout; /* cli_rereg(1b), mcast_pkey_trap_suppr(1b), resrv(1b), timeout(5b) */
> >>       uint8_t resp_time_value;
> >>       uint8_t error_threshold; /* local phy errors(4b), overrun errors(4b) */
> >>       ib_net16_t max_credit_hint;
> >> @@ -5282,7 +5282,7 @@ ib_port_info_set_timeout(IN ib_port_info_t * const p_pi,
> >>  {
> >>       CL_ASSERT(timeout <= 0x1F);
> >>       p_pi->subnet_timeout =
> >> -         (uint8_t) ((p_pi->subnet_timeout & 0x80) | (timeout & 0x1F));
> >> +         (uint8_t) ((p_pi->subnet_timeout & 0xC0) | (timeout & 0x1F));
> >
> > Why not 'p_pi->subnet_timeout & 0xe0' then (IOW why to bother with
> > reserved bits reset)?
> 
> Per this erratum (RefID 4576), this bit is no longer reserved. It is
> now used for MulticastPKeyTrapSuppressionEnabled.

It is bit 6 and I'm about bit 5 which still be reserved - We don't need
to clear this. IOW I think that here (and in other similar places) should
be:

	p_pi->subnet_timeout = (p_pi->subnet_timeout & 0xe0) | (timeout & 0x1f);

> 
> >
> >>  }
> >>
> >>  /*
> >> @@ -5317,7 +5317,7 @@ ib_port_info_set_client_rereg(IN ib_port_info_t * const p_pi,
> >>       CL_ASSERT(client_rereg <= 0x1);
> >>       p_pi->subnet_timeout =
> >>           (uint8_t) ((p_pi->
> >> -                     subnet_timeout & 0x1F) | ((client_rereg << 7) & 0x80));
> >> +                     subnet_timeout & 0x5F) | ((client_rereg << 7) & 0x80));
> >
> > Ditto.
> 
> See above.
> 
> >>  }
> >>
> >>  /*
> >> @@ -5336,6 +5336,43 @@ ib_port_info_set_client_rereg(IN ib_port_info_t * const p_pi,
> >>  * SEE ALSO
> >>  *********/
> >>
> >> +/****f* IBA Base: Types/ib_port_info_set_mcast_pkey_trap_suppress
> >> +* NAME
> >> +*    ib_port_info_set_mcast_pkey_trap_suppress
> >> +*
> >> +* DESCRIPTION
> >> +*    Sets the encoded multicast pkey trap suppresion enabled bit value
> >> +*    in the PortInfo attribute.
> >> +*
> >> +* SYNOPSIS
> >> +*/
> >> +static inline void OSM_API
> >> +ib_port_info_set_mcast_pkey_trap_suppress(IN ib_port_info_t * const p_pi,
> >> +                                       IN const uint8_t trap_suppress)
> >> +{
> >> +     CL_ASSERT(trap_suppress <= 0x1);
> >> +     p_pi->subnet_timeout =
> >> +         (uint8_t) ((p_pi->
> >> +                     subnet_timeout & 0x9F) | ((trap_suppress << 6) & 0x40));
> >
> > Ditto.
> 
> See above.
> 
> > And whoudn't this:
> >
> >        p_pi->subnet_timeout = (p_pi->subnet_timeout & 0xbf) |
> >                (trap_suppress << 6)
> >
> > just work?
> 
> Sure, Do you want an updated patch with this change ?

Yes, please.

Sasha
--
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