[ofa-general] Re: [PATCH] opensm/osm_trap_rcv.c: Validate trap is 144 before checking for NodeDescription changed

Hal Rosenstock hal.rosenstock at gmail.com
Wed Aug 5 14:48:06 PDT 2009


On Wed, Aug 5, 2009 at 3:10 PM, Hal Rosenstock <hal.rosenstock at gmail.com>wrote:

>
>
>   On Wed, Aug 5, 2009 at 3:04 PM, Sasha Khapyorsky <sashak at voltaire.com>wrote:
>
>>  On 08:47 Tue 04 Aug     , Hal Rosenstock wrote:
>> >
>> > Signed-off-by: Hal Rosenstock <hal.rosenstock at gmail.com>
>> > ---
>> > diff --git a/opensm/opensm/osm_trap_rcv.c b/opensm/opensm/osm_trap_rcv.c
>> > index bf39926..925cb27 100644
>> > --- a/opensm/opensm/osm_trap_rcv.c
>> > +++ b/opensm/opensm/osm_trap_rcv.c
>> > @@ -2,6 +2,7 @@
>> >   * Copyright (c) 2004-2008 Voltaire, Inc. All rights reserved.
>> >   * Copyright (c) 2002-2006 Mellanox Technologies LTD. All rights
>> reserved.
>> >   * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
>> > + * Copyright (c) 2009 HNR Consulting. All rights reserved.
>> >   *
>> >   * This software is available to you under a choice of one of two
>> >   * licenses.  You may choose to be licensed under the terms of the GNU
>> > @@ -546,42 +547,47 @@ trap_rcv_process_request(IN osm_sm_t * sm,
>> >               }
>> >       }
>> >
>> > -     /* Check for node description update. IB Spec v1.2.1 pg 823 */
>> > -     if (p_ntci->data_details.ntc_144.local_changes &
>> TRAP_144_MASK_OTHER_LOCAL_CHANGES &&
>> > -         p_ntci->data_details.ntc_144.change_flgs &
>> TRAP_144_MASK_NODE_DESCRIPTION_CHANGE) {
>> > -             OSM_LOG(sm->p_log, OSM_LOG_INFO, "Trap 144 Node
>> description update\n");
>> > -
>> > -             if (p_physp) {
>> > -                     CL_PLOCK_ACQUIRE(sm->p_lock);
>> > -                     osm_req_get_node_desc(sm, p_physp);
>> > -                     CL_PLOCK_RELEASE(sm->p_lock);
>> > -             } else {
>> > -                     OSM_LOG(sm->p_log, OSM_LOG_ERROR,
>> > -                             "ERR 3812: No physical port found for "
>> > -                             "trap 144: \"node description
>> update\"\n");
>> > +     if (ib_notice_is_generic(p_ntci)) {
>> > +             /* Check for node description update. IB Spec v1.2.1 pg
>> 823 */
>> > +             if (cl_ntoh16(p_ntci->g_or_v.generic.trap_num) == 144) {
>> > +                     if (p_ntci->data_details.ntc_144.local_changes &
>> TRAP_144_MASK_OTHER_LOCAL_CHANGES &&
>> > +                         p_ntci->data_details.ntc_144.change_flgs &
>> TRAP_144_MASK_NODE_DESCRIPTION_CHANGE) {
>> > +                             OSM_LOG(sm->p_log, OSM_LOG_INFO,
>> > +                                     "Trap 144 Node description
>> update\n");
>> > +
>> > +                             if (p_physp) {
>> > +                                     CL_PLOCK_ACQUIRE(sm->p_lock);
>> > +                                     osm_req_get_node_desc(sm,
>> p_physp);
>> > +                                     CL_PLOCK_RELEASE(sm->p_lock);
>> > +                             } else
>> > +                                     OSM_LOG(sm->p_log, OSM_LOG_ERROR,
>> > +                                             "ERR 3812: No physical
>> port found for "
>> > +                                             "trap 144: \"node
>> description update\"\n");
>> > +                     }
>> >               }
>> > -     }
>> >
>> > -     /* do a sweep if we received a trap */
>> > -     if (sm->p_subn->opt.sweep_on_trap) {
>> > -             /* if this is trap number 128 or run_heavy_sweep is TRUE -
>> > -                update the force_heavy_sweep flag of the subnet.
>> > -                Sweep also on traps 144/145 - these traps signal a
>> change of
>> > -                certain port capabilities/system image guid.
>> > -                TODO: In the future this can be changed to just getting
>> > -                PortInfo on this port instead of sweeping the entire
>> subnet. */
>> > -             if (ib_notice_is_generic(p_ntci) &&
>> > -                 (cl_ntoh16(p_ntci->g_or_v.generic.trap_num) == 128 ||
>> > -                  cl_ntoh16(p_ntci->g_or_v.generic.trap_num) == 144 ||
>> > -                  cl_ntoh16(p_ntci->g_or_v.generic.trap_num) == 145 ||
>> > -                  run_heavy_sweep)) {
>> > -                     OSM_LOG(sm->p_log, OSM_LOG_VERBOSE,
>> > -                             "Forcing heavy sweep. Received trap:%u\n",
>> > -
>> cl_ntoh16(p_ntci->g_or_v.generic.trap_num));
>> > +             /* do a sweep if we received a trap */
>> > +             if (sm->p_subn->opt.sweep_on_trap) {
>> > +                     /* if this is trap number 128 or run_heavy_sweep
>> is
>> > +                        TRUE - update the force_heavy_sweep flag of the
>> > +                        subnet. Also, sweep also on traps 144/145 -
>> > +                        these traps signal a change of certain port
>> > +                        capabilities/system image guid.
>> > +                        TODO: In the future this can be changed to just
>> > +                        getting PortInfo on this port instead of
>> sweeping
>> > +                        the entire subnet. */
>> > +                     if (cl_ntoh16(p_ntci->g_or_v.generic.trap_num) ==
>> 128 ||
>> > +                         cl_ntoh16(p_ntci->g_or_v.generic.trap_num) ==
>> 144 ||
>> > +                         cl_ntoh16(p_ntci->g_or_v.generic.trap_num) ==
>> 145 ||
>> > +                         run_heavy_sweep) {
>> > +                             OSM_LOG(sm->p_log, OSM_LOG_VERBOSE,
>> > +                                     "Forcing heavy sweep. Received
>> trap:%u\n",
>> > +
>> cl_ntoh16(p_ntci->g_or_v.generic.trap_num));
>> >
>> > -                     sm->p_subn->force_heavy_sweep = TRUE;
>> > +                             sm->p_subn->force_heavy_sweep = TRUE;
>> > +                     }
>> > +                     osm_sm_signal(sm, OSM_SIGNAL_SWEEP);
>> >               }
>> > -             osm_sm_signal(sm, OSM_SIGNAL_SWEEP);
>>
>> Actually this disables sweep (light) on non generic traps. Was it desired
>> change?
>
>
> It was unintended; I'll resubmit adding that back.
>
> -- Hal
>
>
>> Could you see any potential issues with it?
>
>
In thinking about it, I'm not sure what light sweep on non generic trap
accomplishes anyhow.

-- Hal


>
>>
>> Sasha
>>
>> >       }
>> >
>> >       /* If we reached here due to trap 129/130/131 - do not need to do
>> >
>> _______________________________________________
>> general mailing list
>> general at lists.openfabrics.org
>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>>
>> To unsubscribe, please visit
>> http://openib.org/mailman/listinfo/openib-general
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/general/attachments/20090805/d58833ab/attachment.html>


More information about the general mailing list