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

[Hal Rosenstock]

On Wed, Aug 5, 2009 at 3:46 PM, Sasha Khapyorsky <sashak at voltaire.com>wrote:

>  On 15:19 Wed 05 Aug     , Hal Rosenstock wrote:
> >
> > -     /* 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);
> >               }
> > +     } else if (sm->p_subn->opt.sweep_on_trap)
> >               osm_sm_signal(sm, OSM_SIGNAL_SWEEP);
> > -     }
>
> For me this part seems simpler in the original code, so I applied this
> patch as:
>
> diff --git a/opensm/opensm/osm_trap_rcv.c b/opensm/opensm/osm_trap_rcv.c
> index bf39926..d2e4202 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
> @@ -547,7 +548,9 @@ 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 &&
> +       if (ib_notice_is_generic(p_ntci) &&
> +           cl_ntoh16(p_ntci->g_or_v.generic.trap_num) == 144 &&
> +           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");
>
> @@ -555,11 +558,10 @@ trap_rcv_process_request(IN osm_sm_t * sm,
>                        CL_PLOCK_ACQUIRE(sm->p_lock);
>                        osm_req_get_node_desc(sm, p_physp);
>                        CL_PLOCK_RELEASE(sm->p_lock);
> -               } else {
> +               } 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 */
>
>
> Hope it is fine for you.


Sure (that's a smaller/simpler change). I'll retest to be sure when it's in
the tree.

There's more coming which may head more towards where I was trying to go but
we'll see what happens with the next steps with this...

-- Hal


>
>
> Sasha
>  _______________________________________________
> 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/ca08dbb9/attachment.html>