Hi Hal,

On 07:24 Thu 05 Mar     , Hal Rosenstock wrote:
> 
> Make trap generation sending be a transaction (expect a response)
> and handle incoming trap repress (notice attribute)

I don't see how this works. See below.

> ib_mad_is_response is now changed to better reflect responses
> (not just R bit in MAD header)
> 
> Signed-off-by: Hal Rosenstock <[email protected]>

<snip...>

> @@ -3797,8 +3810,10 @@ static inline boolean_t OSM_API
>  ib_mad_is_response(IN const ib_mad_t * const p_mad)
>  {
>       CL_ASSERT(p_mad);
> -     return ((p_mad->method & IB_MAD_METHOD_RESP_MASK) ==
> -             IB_MAD_METHOD_RESP_MASK);
> +     return (p_mad->method & IB_MAD_METHOD_RESP_MASK || 
> +             p_mad->method == IB_MAD_METHOD_TRAP_REPRESS ||
> +             (p_mad->mgmt_class == IB_MCLASS_BM &&
> +              p_mad->attr_mod & IB_BM_ATTR_MOD_RESP));

Why to mix things? BR is not used anywhere in OpenSM.

>  }
>  
>  /*
> diff --git a/opensm/opensm/osm_req.c b/opensm/opensm/osm_req.c
> index 2e851b2..1a46e50 100644
> --- a/opensm/opensm/osm_req.c
> +++ b/opensm/opensm/osm_req.c
> @@ -246,6 +246,7 @@ int osm_send_trap144(osm_sm_t *sm, ib_net16_t local)
>  
>       madw->mad_addr.dest_lid = pi->master_sm_base_lid;
>       madw->mad_addr.addr_type.smi.source_lid = pi->base_lid;
> +     madw->resp_expected = TRUE;
>       madw->fail_msg = CL_DISP_MSGID_NONE;
>  
>       smp = osm_madw_get_smp_ptr(madw);
> diff --git a/opensm/opensm/osm_sm_mad_ctrl.c b/opensm/opensm/osm_sm_mad_ctrl.c
> index 267ec85..a9124fa 100644
> --- a/opensm/opensm/osm_sm_mad_ctrl.c
> +++ b/opensm/opensm/osm_sm_mad_ctrl.c
> @@ -2,6 +2,7 @@
>   * Copyright (c) 2004-2008 Voltaire, Inc. All rights reserved.
>   * Copyright (c) 2002-2005 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
> @@ -250,10 +251,12 @@ __osm_sm_mad_ctrl_process_get_resp(IN osm_sm_mad_ctrl_t 
> * const p_ctrl,
>       case IB_MAD_ATTR_P_KEY_TABLE:
>               msg_id = OSM_MSG_MAD_PKEY;
>               break;
> +     case IB_MAD_ATTR_NOTICE:
> +             msg_id = OSM_MSG_MAD_NOTICE;
> +             break;

This is GetResp processing, not TrapRepress. And here
osm_trap_rcv_process() is actually scheduled for execution.

>  
>       case IB_MAD_ATTR_GUID_INFO:
>       case IB_MAD_ATTR_CLASS_PORT_INFO:
> -     case IB_MAD_ATTR_NOTICE:
>       case IB_MAD_ATTR_INFORM_INFO:
>       default:
>               cl_atomic_inc(&p_ctrl->p_stats->qp0_mads_rcvd_unknown);
> @@ -537,6 +540,57 @@ Exit:
>   * SEE ALSO
>   *********/
>  
> +/****f* opensm: SM/__osm_sm_mad_ctrl_process_trap_repress
> + * NAME
> + * __osm_sm_mad_ctrl_process_trap_repress
> + *
> + * DESCRIPTION
> + * This function handles method TrapRepress() for received MADs.
> + *
> + * SYNOPSIS
> + */
> +static void
> +__osm_sm_mad_ctrl_process_trap_repress(IN osm_sm_mad_ctrl_t * const p_ctrl,
> +                                    IN osm_madw_t * p_madw)
> +{
> +     ib_smp_t *p_smp;
> +
> +     OSM_LOG_ENTER(p_ctrl->p_log);
> +
> +     p_smp = osm_madw_get_smp_ptr(p_madw);
> +
> +     /*
> +        Note that attr_id (like the rest of the MAD) is in
> +        network byte order.
> +      */
> +     switch (p_smp->attr_id) {
> +     case IB_MAD_ATTR_NOTICE:
> +             break;
> +
> +     default:
> +             cl_atomic_inc(&p_ctrl->p_stats->qp0_mads_rcvd_unknown);
> +             OSM_LOG(p_ctrl->p_log, OSM_LOG_ERROR, "ERR 3105: "
> +                     "Unsupported attribute = 0x%X\n",
> +                     cl_ntoh16(p_smp->attr_id));
> +             osm_dump_dr_smp(p_ctrl->p_log, p_smp, OSM_LOG_ERROR);
> +             break;
> +     }
> +
> +     osm_mad_pool_put(p_ctrl->p_mad_pool, p_madw);
> +
> +     OSM_LOG_EXIT(p_ctrl->p_log);
> +}

And this function which should process TrapRepress method does nothing,
right?

> +
> +/*
> + * PARAMETERS
> + *
> + * RETURN VALUES
> + *
> + * NOTES
> + *
> + * SEE ALSO
> + *********/
> +
>  /****f* opensm: SM/__osm_sm_mad_ctrl_rcv_callback
>   * NAME
>   * __osm_sm_mad_ctrl_rcv_callback
> @@ -624,10 +678,14 @@ __osm_sm_mad_ctrl_rcv_callback(IN osm_madw_t * p_madw,

This (__osm_sm_mad_ctrl_rcv_callback()) function has above

        if (ib_smp_is_response(p_smp) ||
            (p_smp->method == IB_MAD_METHOD_TRAP_REPRESS))

check. Following your ib_mad_is_response() modification it should be
simplified (probably in other places too).

Sasha

>               __osm_sm_mad_ctrl_process_set(p_ctrl, p_madw);
>               break;
>  
> +     case IB_MAD_METHOD_TRAP_REPRESS:
> +             CL_ASSERT(p_req_madw == NULL);
> +             __osm_sm_mad_ctrl_process_trap_repress(p_ctrl, p_madw);
> +             break;
> +
>       case IB_MAD_METHOD_SEND:
>       case IB_MAD_METHOD_REPORT:
>       case IB_MAD_METHOD_REPORT_RESP:
> -     case IB_MAD_METHOD_TRAP_REPRESS:
>       default:
>               cl_atomic_inc(&p_ctrl->p_stats->qp0_mads_rcvd_unknown);
>               OSM_LOG(p_ctrl->p_log, OSM_LOG_ERROR, "ERR 3112: "
> diff --git a/opensm/opensm/osm_trap_rcv.c b/opensm/opensm/osm_trap_rcv.c
> index a5491e1..8a52f1a 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
> @@ -649,18 +650,34 @@ Exit:
>  }
>  
>  /**********************************************************************
> - CURRENTLY WE ARE NOT CREATING TRAPS - SO THIS CALL IN AN ERROR
>  **********************************************************************/
>  static void
>  trap_rcv_process_response(IN osm_sm_t * sm,
>                         IN const osm_madw_t * const p_madw)
>  {
> +     uint8_t payload[sizeof(ib_mad_notice_attr_t)];
> +     ib_smp_t *p_smp;
> +     ib_mad_notice_attr_t *p_ntci = (ib_mad_notice_attr_t *) payload;
> +
> +     CL_ASSERT(p_madw);
> +
> +     if (!osm_log_is_active(sm->p_log, OSM_LOG_VERBOSE))
> +             return;
>  
>       OSM_LOG_ENTER(sm->p_log);
>  
> -     OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 3808: "
> -             "This function is not supported yet\n");
> +     if (p_madw->p_mad->mgmt_class != IB_MCLASS_SUBN_LID &&
> +         p_madw->p_mad->mgmt_class != IB_MCLASS_SUBN_DIR)
> +             goto Exit;
>  
> +     p_smp = osm_madw_get_smp_ptr(p_madw);
> +
> +     memset(payload, 0, sizeof(payload));
> +     memcpy(payload, &p_smp->data, IB_SMP_DATA_SIZE);
> +
> +     osm_dump_notice(sm->p_log, p_ntci, OSM_LOG_VERBOSE);
> +
> +Exit:
>       OSM_LOG_EXIT(sm->p_log);
>  }
>  
_______________________________________________
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