***SPAM*** Re: [ofa-general] [PATCH] opensm/osm_multicast.[ch]: simplify flows, remove unused functions
Hal Rosenstock
hal.rosenstock at gmail.com
Tue Sep 16 06:52:05 PDT 2008
Sasha,
On Mon, Sep 15, 2008 at 6:40 AM, Sasha Khapyorsky <sashak at voltaire.com> wrote:
>
> Simplify flows, remove unused and mean less osm_mgrp_init() functions,
> consolidate notice sending functions.
>
> Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> ---
> opensm/include/opensm/osm_multicast.h | 88 ----------------
> opensm/opensm/osm_multicast.c | 181 ++++++++++-----------------------
> 2 files changed, 56 insertions(+), 213 deletions(-)
>
> diff --git a/opensm/include/opensm/osm_multicast.h b/opensm/include/opensm/osm_multicast.h
> index c0bd16e..c860d4a 100644
> --- a/opensm/include/opensm/osm_multicast.h
> +++ b/opensm/include/opensm/osm_multicast.h
> @@ -81,29 +81,6 @@ BEGIN_C_DECLS
> * Steve King, Intel
> *
> *********/
> -/****f* IBA Base: OpenSM: Multicast Group/osm_get_mcast_req_type_str
> -* NAME
> -* osm_get_mcast_req_type_str
> -*
> -* DESCRIPTION
> -* Returns a string for the specified osm_mcast_req_type_t value.
> -*
> -* SYNOPSIS
> -*/
> -const char *osm_get_mcast_req_type_str(IN osm_mcast_req_type_t req_type);
> -/*
> -* PARAMETERS
> -* req_type
> -* [in] osm_mcast_req_type value
> -*
> -* RETURN VALUES
> -* Pointer to the request type description string.
> -*
> -* NOTES
> -*
> -* SEE ALSO
> -*********/
> -
> /****s* OpenSM: Multicast Group/osm_mcast_mgr_ctxt_t
> * NAME
> * osm_mcast_mgr_ctxt_t
> @@ -483,71 +460,6 @@ osm_mgrp_remove_port(IN osm_subn_t * const p_subn,
> * SEE ALSO
> *********/
>
> -/****f* OpenSM: Multicast Group/osm_mgrp_get_root_switch
> -* NAME
> -* osm_mgrp_get_root_switch
> -*
> -* DESCRIPTION
> -* Returns the "root" switch of this multicast group. The root switch
> -* is at the trunk of the multicast single spanning tree.
> -*
> -* SYNOPSIS
> -*/
> -static inline osm_switch_t *osm_mgrp_get_root_switch(IN const osm_mgrp_t *
> - const p_mgrp)
> -{
> - if (p_mgrp->p_root)
> - return (p_mgrp->p_root->p_sw);
> - else
> - return (NULL);
> -}
> -
> -/*
> -* PARAMETERS
> -* p_mgrp
> -* [in] Pointer to an osm_mgrp_t object.
> -*
> -* RETURN VALUES
> -* Returns the "root" switch of this multicast group. The root switch
> -* is at the trunk of the multicast single spanning tree.
> -*
> -* NOTES
> -*
> -* SEE ALSO
> -* Multicast Group
> -*********/
> -
> -/****f* OpenSM: Multicast Group/osm_mgrp_compute_avg_hops
> -* NAME
> -* osm_mgrp_compute_avg_hops
> -*
> -* DESCRIPTION
> -* Returns the average number of hops from the given to switch
> -* to all member of a multicast group.
> -*
> -* SYNOPSIS
> -*/
> -float
> -osm_mgrp_compute_avg_hops(const osm_mgrp_t * const p_mgrp,
> - const osm_switch_t * const p_sw);
> -/*
> -* PARAMETERS
> -* p_mgrp
> -* [in] Pointer to an osm_mgrp_t object.
> -*
> -* p_sw
> -* [in] Pointer to the switch from which to measure.
> -*
> -* RETURN VALUES
> -* Returns the average number of hops from the given to switch
> -* to all member of a multicast group.
> -*
> -* NOTES
> -*
> -* SEE ALSO
> -* Multicast Group
> -*********/
> -
> /****f* OpenSM: Multicast Group/osm_mgrp_apply_func
> * NAME
> * osm_mgrp_apply_func
> diff --git a/opensm/opensm/osm_multicast.c b/opensm/opensm/osm_multicast.c
> index 77e61ad..b810630 100644
> --- a/opensm/opensm/osm_multicast.c
> +++ b/opensm/opensm/osm_multicast.c
> @@ -51,23 +51,6 @@
>
> /**********************************************************************
> **********************************************************************/
> -/* osm_mcast_req_type_t values converted to test for easier printing. */
> -const static char *mcast_req_type_str[] = {
> - "OSM_MCAST_REQ_TYPE_CREATE",
> - "OSM_MCAST_REQ_TYPE_JOIN",
> - "OSM_MCAST_REQ_TYPE_LEAVE",
> - "OSM_MCAST_REQ_TYPE_SUBNET_CHANGE"
> -};
> -
> -const char *osm_get_mcast_req_type_str(IN osm_mcast_req_type_t req_type)
> -{
> - if (req_type > OSM_MCAST_REQ_TYPE_SUBNET_CHANGE)
> - req_type = OSM_MCAST_REQ_TYPE_SUBNET_CHANGE;
> - return (mcast_req_type_str[req_type]);
> -}
> -
> -/**********************************************************************
> - **********************************************************************/
> void osm_mgrp_delete(IN osm_mgrp_t * const p_mgrp)
> {
> osm_mcm_port_t *p_mcm_port;
> @@ -92,10 +75,13 @@ void osm_mgrp_delete(IN osm_mgrp_t * const p_mgrp)
>
> /**********************************************************************
> **********************************************************************/
> -static void
> -osm_mgrp_init(IN osm_mgrp_t * const p_mgrp, IN const ib_net16_t mlid)
> +osm_mgrp_t *osm_mgrp_new(IN const ib_net16_t mlid)
> {
> - CL_ASSERT(cl_ntoh16(mlid) >= IB_LID_MCAST_START_HO);
> + osm_mgrp_t *p_mgrp;
> +
> + p_mgrp = (osm_mgrp_t *) malloc(sizeof(*p_mgrp));
> + if (!p_mgrp)
> + return NULL;
>
> memset(p_mgrp, 0, sizeof(*p_mgrp));
> cl_qmap_init(&p_mgrp->mcm_port_tbl);
> @@ -103,19 +89,8 @@ osm_mgrp_init(IN osm_mgrp_t * const p_mgrp, IN const ib_net16_t mlid)
> p_mgrp->last_change_id = 0;
> p_mgrp->last_tree_id = 0;
> p_mgrp->to_be_deleted = FALSE;
> -}
>
> -/**********************************************************************
> - **********************************************************************/
> -osm_mgrp_t *osm_mgrp_new(IN const ib_net16_t mlid)
> -{
> - osm_mgrp_t *p_mgrp;
> -
> - p_mgrp = (osm_mgrp_t *) malloc(sizeof(*p_mgrp));
> - if (p_mgrp)
> - osm_mgrp_init(p_mgrp, mlid);
> -
> - return (p_mgrp);
> + return p_mgrp;
> }
>
> /**********************************************************************
> @@ -132,42 +107,39 @@ osm_mcm_port_t *osm_mgrp_add_port(IN osm_mgrp_t * const p_mgrp,
> uint8_t prev_scope;
>
> p_mcm_port = osm_mcm_port_new(p_port_gid, join_state, proxy_join);
> - if (p_mcm_port) {
> - port_guid = p_port_gid->unicast.interface_id;
> + if (!p_mcm_port)
> + return NULL;
> +
> + port_guid = p_port_gid->unicast.interface_id;
> +
> + /*
> + prev_item = cl_qmap_insert(...)
> + Pointer to the item in the map with the specified key. If insertion
> + was successful, this is the pointer to the item. If an item with the
> + specified key already exists in the map, the pointer to that item is
> + returned.
> + */
> + prev_item = cl_qmap_insert(&p_mgrp->mcm_port_tbl,
> + port_guid, &p_mcm_port->map_item);
> +
> + /* if already exists - revert the insertion and only update join state */
> + if (prev_item != &p_mcm_port->map_item) {
> + osm_mcm_port_delete(p_mcm_port);
> + p_mcm_port = (osm_mcm_port_t *) prev_item;
>
> /*
> - prev_item = cl_qmap_insert(...)
> - Pointer to the item in the map with the specified key. If insertion
> - was successful, this is the pointer to the item. If an item with the
> - specified key already exists in the map, the pointer to that item is
> - returned.
> + o15.0.1.11
> + Join state of the end port should be the or of the
> + previous setting with the current one
> */
> - prev_item = cl_qmap_insert(&p_mgrp->mcm_port_tbl,
> - port_guid, &p_mcm_port->map_item);
> -
> - /* if already exists - revert the insertion and only update join state */
> - if (prev_item != &p_mcm_port->map_item) {
> -
> - osm_mcm_port_delete(p_mcm_port);
> - p_mcm_port = (osm_mcm_port_t *) prev_item;
> -
> - /*
> - o15.0.1.11
> - Join state of the end port should be the or of the
> - previous setting with the current one
> - */
> - ib_member_get_scope_state(p_mcm_port->scope_state,
> - &prev_scope,
> - &prev_join_state);
> - p_mcm_port->scope_state =
> - ib_member_set_scope_state(prev_scope,
> - prev_join_state |
> - join_state);
> -
> - } else {
> - /* track the fact we modified the group ports */
> - p_mgrp->last_change_id++;
> - }
> + ib_member_get_scope_state(p_mcm_port->scope_state, &prev_scope,
> + &prev_join_state);
> + p_mcm_port->scope_state =
> + ib_member_set_scope_state(prev_scope,
> + prev_join_state | join_state);
> + } else {
> + /* track the fact we modified the group ports */
> + p_mgrp->last_change_id++;
> }
>
> return (p_mcm_port);
> @@ -243,9 +215,7 @@ __osm_mgrp_apply_func_sub(const osm_mgrp_t * const p_mgrp,
> uint8_t max_children;
> osm_mtree_node_t *p_child_mtn;
>
> - /*
> - Call the user, then recurse.
> - */
> + /* Call the user, then recurse. */
> p_func(p_mgrp, p_mtn, context);
>
> max_children = osm_mtree_node_get_max_children(p_mtn);
> @@ -276,82 +246,43 @@ osm_mgrp_apply_func(const osm_mgrp_t * const p_mgrp,
>
> /**********************************************************************
> **********************************************************************/
> -void
> -osm_mgrp_send_delete_notice(IN osm_subn_t * const p_subn,
> - IN osm_log_t * const p_log, IN osm_mgrp_t * p_mgrp)
> +static void mgrp_send_notice(osm_subn_t *subn, osm_log_t *log,
> + osm_mgrp_t *mgrp, unsigned num)
> {
> ib_mad_notice_attr_t notice;
> ib_api_status_t status;
>
> - OSM_LOG_ENTER(p_log);
> -
> - /* prepare the needed info */
> -
> - /* details of the notice */
> - notice.generic_type = 0x83; /* is generic subn mgt type */
> + notice.generic_type = 0x83; /* generic SubnMgt type */
> ib_notice_set_prod_type_ho(¬ice, 4); /* A Class Manager generator */
> - notice.g_or_v.generic.trap_num = CL_HTON16(67); /* delete of mcg */
> + notice.g_or_v.generic.trap_num = CL_HTON16(num);
> /* The sm_base_lid is saved in network order already. */
> - notice.issuer_lid = p_subn->sm_base_lid;
> + notice.issuer_lid = subn->sm_base_lid;
> /* following o14-12.1.11 and table 120 p726 */
> /* we need to provide the MGID */
> - memcpy(&(notice.data_details.ntc_64_67.gid),
> - &(p_mgrp->mcmember_rec.mgid), sizeof(ib_gid_t));
> + memcpy(¬ice.data_details.ntc_64_67.gid,
> + &mgrp->mcmember_rec.mgid, sizeof(ib_gid_t));
>
> /* According to page 653 - the issuer gid in this case of trap
> is the SM gid, since the SM is the initiator of this trap. */
> - notice.issuer_gid.unicast.prefix = p_subn->opt.subnet_prefix;
> - notice.issuer_gid.unicast.interface_id = p_subn->sm_port_guid;
> + notice.issuer_gid.unicast.prefix = subn->opt.subnet_prefix;
> + notice.issuer_gid.unicast.interface_id = subn->sm_port_guid;
>
> - status = osm_report_notice(p_log, p_subn, ¬ice);
> - if (status != IB_SUCCESS) {
> - OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 7601: "
> + if ((status = osm_report_notice(log, subn, ¬ice)))
> + OSM_LOG(log, OSM_LOG_ERROR, "ERR 7601: "
> "Error sending trap reports (%s)\n",
> ib_get_err_str(status));
> - goto Exit;
> - }
> +}
>
> -Exit:
> - OSM_LOG_EXIT(p_log);
> +void
> +osm_mgrp_send_delete_notice(IN osm_subn_t * const p_subn,
> + IN osm_log_t * const p_log, IN osm_mgrp_t * p_mgrp)
> +{
> + mgrp_send_notice(p_subn, p_log, p_mgrp, 67);
> }
Any reason not to eliminate this extra call level ?
> -/**********************************************************************
> - **********************************************************************/
> void
> osm_mgrp_send_create_notice(IN osm_subn_t * const p_subn,
> IN osm_log_t * const p_log, IN osm_mgrp_t * p_mgrp)
> {
> - ib_mad_notice_attr_t notice;
> - ib_api_status_t status;
> -
> - OSM_LOG_ENTER(p_log);
> -
> - /* prepare the needed info */
> -
> - /* details of the notice */
> - notice.generic_type = 0x83; /* Generic SubnMgt type */
> - ib_notice_set_prod_type_ho(¬ice, 4); /* A Class Manager generator */
> - notice.g_or_v.generic.trap_num = CL_HTON16(66); /* create of mcg */
> - /* The sm_base_lid is saved in network order already. */
> - notice.issuer_lid = p_subn->sm_base_lid;
> - /* following o14-12.1.11 and table 120 p726 */
> - /* we need to provide the MGID */
> - memcpy(&(notice.data_details.ntc_64_67.gid),
> - &(p_mgrp->mcmember_rec.mgid), sizeof(ib_gid_t));
> -
> - /* According to page 653 - the issuer gid in this case of trap
> - is the SM gid, since the SM is the initiator of this trap. */
> - notice.issuer_gid.unicast.prefix = p_subn->opt.subnet_prefix;
> - notice.issuer_gid.unicast.interface_id = p_subn->sm_port_guid;
> -
> - status = osm_report_notice(p_log, p_subn, ¬ice);
> - if (status != IB_SUCCESS) {
> - OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 7602: "
> - "Error sending trap reports (%s)\n",
> - ib_get_err_str(status));
> - goto Exit;
> - }
> -
> -Exit:
> - OSM_LOG_EXIT(p_log);
> + mgrp_send_notice(p_subn, p_log, p_mgrp, 66);
Similarly, any reason not to eliminate this extra calling level ?
-- Hal
> }
> --
> 1.6.0.1.196.g01914
>
> _______________________________________________
> 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
>
More information about the general
mailing list