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 <hal.rosenstock at gmail.com>
<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);
> }
>