[ofa-general] RE: [PATCH 2/4 v2] multicast multiplexing - implementation of newinfrastructure
Slava Strebkov
slavas at voltaire.com
Mon Jul 20 03:10:58 PDT 2009
-----Original Message-----
From: Sasha Khapyorsky [mailto:sashakvolt at gmail.com] On Behalf Of Sasha
Khapyorsky
Sent: Sunday, July 19, 2009 6:51 PM
To: Slava Strebkov
Cc: general at lists.openfabrics.org
Subject: Re: [PATCH 2/4 v2] multicast multiplexing - implementation of
newinfrastructure
On 13:57 Sun 28 Jun , Slava Strebkov wrote:
>
> diff --git a/opensm/include/opensm/osm_mcm_info.h
b/opensm/include/opensm/osm_mcm_info.h
> index dec607f..2310d25 100644
> --- a/opensm/include/opensm/osm_mcm_info.h
> +++ b/opensm/include/opensm/osm_mcm_info.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2004-2007 Voltaire, Inc. All rights reserved.
> + * Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
> * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights
reserved.
> * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
> *
> @@ -47,6 +47,7 @@
> #include <iba/ib_types.h>
> #include <complib/cl_qlist.h>
> #include <opensm/osm_base.h>
> +#include <opensm/osm_sa.h>
>
> #ifdef __cplusplus
> # define BEGIN_C_DECLS extern "C" {
> @@ -71,7 +72,7 @@ BEGIN_C_DECLS
> *
> * SYNOPSIS
> */
> -typedef struct osm_mcm_info {
> + typedef struct osm_mcm_info {
> cl_list_item_t list_item;
> ib_net16_t mlid;
> } osm_mcm_info_t;
> @@ -131,6 +132,34 @@ void osm_mcm_info_delete(IN osm_mcm_info_t *
const p_mcm);
> *
> * SEE ALSO
> *********/
> -
> +/****f* OpenSM: Multicast Group Holder
/osm_mgrp_holder_get_mlid_by_mgid
> +* NAME
> +* osm_mgrp_holder_get_mlid_by_mgid
> +*
> +* DESCRIPTION
> +* Searches holder which contains mgroup with given mgid
> +* Returns mlid of the found holder
> +*
> +* SYNOPSIS
> +*/
> +ib_net16_t osm_mgrp_holder_get_mlid_by_mgid(IN osm_sa_t * sa,
> + IN const ib_gid_t * const
p_mgid);
Why *sa and not *subn? And why should it be in osm_mcm_info.h (which
just describes *port*'s mcm related list)?
-This may be moved to another file, or perhaps it's better to create a
new file for this stuff?
> +/*
> +* PARAMETERS
> +*
> +* p_sa
> +* [in] Pointer to sa object
> +*
> +* p_mgid
> +* [in] pointer to mgid
> +*
> +* RETURN VALUES
> +* mlid of found holder, or zero.
> +*
> +* NOTES
> +*
> +* SEE ALSO
> +*
> +*********/
> END_C_DECLS
> #endif /* _OSM_MCM_INFO_H_ */
> diff --git a/opensm/include/opensm/osm_multicast.h
b/opensm/include/opensm/osm_multicast.h
> index 02b63bd..9f0cd96 100644
> --- a/opensm/include/opensm/osm_multicast.h
> +++ b/opensm/include/opensm/osm_multicast.h
> @@ -214,6 +214,8 @@ typedef struct osm_mgrp {
> uint32_t last_change_id;
> uint32_t last_tree_id;
> unsigned full_members;
> + cl_fmap_item_t mgid_item;
> + cl_list_item_t mgrp_item;
> } osm_mgrp_t;
> /*
> * FIELDS
> @@ -254,6 +256,12 @@ typedef struct osm_mgrp {
> * last_tree_id
> * the last change id used for building the current tree.
> *
> +* mgid_item
> +* fleximap item for map linkage, sorted by mgid.
> +*
> +* mgrp_item
> +* list item for mgroups pointer list.
> +*
> * SEE ALSO
> *********/
>
> @@ -572,6 +580,295 @@ osm_mgrp_apply_func(const osm_mgrp_t * const
p_mgrp,
> * SEE ALSO
> * Multicast Group
> *********/
> +/****f* OpenSM: Multicast Group Holder /osm_mgrp_holder_new
> +* NAME
> +* osm_mgrp_holder_new
> +*
> +* DESCRIPTION
> +* Allocates and initializes a Multicast Group Holder for use.
> +*
> +* SYNOPSIS
> +*/
> +osm_mgrp_holder_t *osm_mgrp_holder_new(IN osm_subn_t * p_subn,
> + IN ib_gid_t * p_mgid,
> + IN ib_net16_t mlid);
> +/*
> +* PARAMETERS
> +* p_subn
> +* (in) pointer to osm_subnet
> +* p_mgid
> +* (in) pointer to mgid
What is "mgid" there? "Common" or "real"?
> +* mlid
> +* [in] Multicast LID for this multicast group holder.
> +*
> +* RETURN VALUES
> +* pointer to initialized osm_mgrp_holder_t
> +* or NULL, if unsuccessful
> +*
> +* SEE ALSO
> +* Multicast Group Holder, osm_mgrp_holder_delete
> +*********/
> +/****f* OpenSM: Multicast Group Holder /osm_mgrp_holder_delete
> +* NAME
> +* osm_mgrp_holder_delete
> +*
> +* DESCRIPTION
> +* Removes entry from array of holders
> +* Removes port from mgroup port list
> +*
> +* SYNOPSIS
> +*/
> +void osm_mgrp_holder_delete(IN osm_subn_t * p_subn, IN ib_net16_t
mlid);
> +/*
> +* PARAMETERS
> +*
> +* p_subn
> +* [in] Pointer to osm_subnet
> +*
> +* mlid
> +* [in] holder's mlid
> +*
> +* RETURN VALUES
> +* None.
> +*
> +* NOTES
> +*
> +* SEE ALSO
> +*
> +*********/
> +/****f* OpenSM: Multicast Group Holder /osm_mgrp_holder_add_mgrp_port
> +* NAME
> +* osm_mgrp_holder_add_mgrp_port
> +*
> +* DESCRIPTION
> +* Allocates osm_mgrp_port_t for new port joined to mgroup with
mlid of this holder,
> +* (or / and) adds mgroup to mgroup map of existed
osm_mgrp_port_t object.
> +*
> +* SYNOPSIS
> +*/
> +ib_api_status_t osm_mgrp_holder_add_mgrp_port(IN osm_mgrp_holder_t *
> + p_mgrp_holder,
> + IN osm_mgrp_t * p_mgrp,
> + IN ib_net64_t port_guid);
Why do you need both mgrp_holder and mgrp?
> +/*
> +* PARAMETERS
> +* p_mgrp_holder
> +* (in) pointer to osm_mgrp_holder_t
> +* p_mgrp
> +* (in) pointer to osm_mgrp_t
> +*
> +* RETURN VALUES
> +* IB_SUCCESS or
> +* IB_INSUFFICIENT_MEMORY
> +*
> +* SEE ALSO
> +* Multicast Group Holder, osm_mgrp_holder_delete_mgrp_port
> +*********/
> +
> +/****f* OpenSM: Multicast Group Holder
/osm_mgrp_holder_delete_mgrp_port
> +* NAME
> +* osm_mgrp_holder_delete_mgrp_port
> +*
> +* DESCRIPTION
> +* Deletes osm_mgrp_port_t for specified port
> +*
> +* SYNOPSIS
> +*/
> +void osm_mgrp_holder_delete_mgrp_port(IN osm_mgrp_holder_t *
p_mgrp_holder,
> + IN osm_mgrp_t * p_mgrp,
> + IN ib_net64_t port_guid);
Ditto.
> +/*
> +* PARAMETERS
> +* p_mgrp_holder
> +* [in] Pointer to an osm_mgrp_holder_t object.
> +*
> +* p_mgrp
> +* (in) Pointer to osm_mgrp_t object
> +*
> +* port_guid
> +* [in] Port guid of the departing port.
> +*
> +* RETURN VALUES
> +* None.
> +*
> +* NOTES
> +*
> +* SEE ALSO
> + Multicast Group Holder,osm_holder_add_mgrp_port
> +*********/
> +
> +/****f* OpenSM: Multicast Group Holder /osm_mgrp_holder_remove_port
> +* NAME
> +* osm_mgrp_holder_remove_port
> +*
> +* DESCRIPTION
> +* Removes osm_mgrp_port_t from mgrp_port_map of holder
> +* Removes port from mgroup port list
> +*
> +* SYNOPSIS
> +*/
> +void osm_mgrp_holder_remove_port(IN osm_subn_t * const p_subn,
> + IN osm_log_t * const p_log,
> + IN osm_mgrp_holder_t * const
p_mgrp_holder,
> + IN const ib_net64_t port_guid);
> +/*
> +* PARAMETERS
> +*
> +* p_subn
> +* [in] Pointer to the subnet object
> +*
> +* p_log
> +* [in] The log object pointer
> +*
> +* p_mgrp_holder
> +* [in] Pointer to an osm_mgrp_holder_t object.
> +*
> +* port_guid
> +* [in] Port guid of the departing port.
> +*
> +* RETURN VALUES
> +* None.
> +*
> +* NOTES
> +*
> +* SEE ALSO
> +*
> +*********/
> +
> +/****f* OpenSM: Multicast Group Holder /osm_mgrp_holder_add_mgrp
> +* NAME
> +* osm_mgrp_holder_add_mgrp
> +*
> +* DESCRIPTION
> +* Adds mgroup to holder according to its mgid
> +*
> +*
> +* SYNOPSIS
> +*/
> +void osm_mgrp_holder_add_mgrp(IN osm_mgrp_holder_t * p_mgrp_holder,
> + IN osm_mgrp_t * p_mgrp,
> + IN osm_log_t * const p_log);
> +/*
> +* PARAMETERS
> +*
> +* p_mgrp_holder
> +* [in] Pointer to an osm_mgrp_holder_t object.
> +*
> +* p_mgrp
> +* [in] mgroup to add.
> +*
> +* RETURN VALUES
> +* None.
> +*
> +* NOTES
> +* Updates common_mgid when holder is being reused
> +* SEE ALSO
> +* Multicast Group Holder,osm_mgrp_holder_delete_mgrp
> +*********/
> +
> +/****f* OpenSM: Multicast Group Holder /osm_mgrp_holder_delete_mgrp
> +* NAME
> +* osm_mgrp_holder_delete_mgrp
> +*
> +* DESCRIPTION
> +* Deletes mgroup from holder according to its mgid
> +*
> +*
> +* SYNOPSIS
> +*/
> +void osm_mgrp_holder_delete_mgrp(IN osm_mgrp_holder_t *
p_mgrp_holder,
> + IN osm_mgrp_t * p_mgrp);
> +/*
> +* PARAMETERS
> +*
> +* p_mgrp_holder
> +* [in] Pointer to an osm_mgrp_holder_t object.
> +*
> +* p_mgrp
> +* [in] mgroup to delete.
> +*
> +* RETURN VALUES
> +* None.
> +*
> +* NOTES
> +*
> +* SEE ALSO
> +* Multicast Group Holder,osm_mgrp_holder_add_mgrp
> +*********/
> +
> +/* Multicast Group Holder, osm_mgrp_holder_delete_port
> +*********/
To what is this comment related to?
-to be deleted
> +
> +/****f* OpenSM: Multicast Group Holder
/osm_mgrp_holder_prepare_common_mgid
> +* NAME
> +* osm_mgrp_holder_prepare_common_mgid
> +*
> +* DESCRIPTION
> +* Prepares mgid, which is common for all mgroups in this holder
> +*
> +* SYNOPSIS
> +*/
> +void osm_mgrp_holder_prepare_common_mgid(IN const ib_gid_t * const
p_mgid,
> + OUT ib_gid_t * p_common_mgid);
> +/*
> +* PARAMETERS
> +*
> +* p_mgid
> +* [in] Pointer to mgid
> +*
> +* p_common_mgid
> +* [out] common mgid
> +*
> +* RETURN VALUES
> +* None.
> +*
> +* NOTES
> +*
> +* SEE ALSO
> +*
> +*********/
> +/****f* OpenSM: Subnet/osm_get_mgrp_by_mlid
> +* NAME
> +* osm_get_mgrp_by_mlid
> +*
> +* DESCRIPTION
> +* The looks for the given multicast group in the subnet table
by mlid.
> +* NOTE: this code is not thread safe. Need to grab the lock
before
> +* calling it.
> +*
> +* SYNOPSIS
> +*/
> +static inline
> + struct osm_mgrp_holder *osm_get_mgrp_holder_by_mlid(osm_subn_t
const
> + *p_subn,
> + ib_net16_t mlid)
> +{
> + return p_subn->mgroup_holders[cl_ntoh16(mlid) -
IB_LID_MCAST_START_HO];
> +}
> +
> +/*
> +* PARAMETERS
> +* p_subn
> +* [in] Pointer to an osm_subn_t object
> +*
> +* mlid
> +* [in] The multicast group mlid in network order
> +*
> +* RETURN VALUES
> +* The multicast group structure pointer if found. NULL
otherwise.
> +*********/
> +
> +static inline ib_net16_t osm_mgrp_holder_get_mlid(IN
osm_mgrp_holder_t *
> + const p_mgrp_holder)
> +{
> + return (p_mgrp_holder->mlid);
> +}
> +
> +static inline boolean_t osm_mgrp_holder_is_empty(IN const
osm_mgrp_holder_t *
> + const p_mgrp_holder)
> +{
> + return (cl_qmap_count(&p_mgrp_holder->mgrp_port_map) == 0);
> +}
>
> END_C_DECLS
> #endif /* _OSM_MULTICAST_H_ */
> diff --git a/opensm/include/opensm/osm_subnet.h
b/opensm/include/opensm/osm_subnet.h
> index 59a32ad..6713d4d 100644
> --- a/opensm/include/opensm/osm_subnet.h
> +++ b/opensm/include/opensm/osm_subnet.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2004-2008 Voltaire, Inc. All rights reserved.
> + * Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
> * Copyright (c) 2002-2008 Mellanox Technologies LTD. All rights
reserved.
> * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
> * Copyright (c) 2008 Xsigo Systems Inc. All rights reserved.
> @@ -509,6 +509,7 @@ typedef struct osm_subn {
> boolean_t coming_out_of_standby;
> unsigned need_update;
> void *mgroups[IB_LID_MCAST_END_HO - IB_LID_MCAST_START_HO + 1];
> + void *mgroup_holders[IB_LID_MCAST_END_HO - IB_LID_MCAST_START_HO
+ 1];
> } osm_subn_t;
> /*
> * FIELDS
> @@ -633,6 +634,10 @@ typedef struct osm_subn {
> * Array of pointers to all Multicast Group objects in the
subnet.
> * Indexed by MLID offset from base MLID.
> *
> +* mgroup_holders
> +* Array of pointers to all Multicast Group Holder objects
in the subnet.
> +* Indexed by MLID offset from base MLID.
> +*
> * SEE ALSO
> * Subnet object
> *********/
> diff --git a/opensm/opensm/osm_multicast.c
b/opensm/opensm/osm_multicast.c
> index ae0a818..d2a19ea 100644
> --- a/opensm/opensm/osm_multicast.c
> +++ b/opensm/opensm/osm_multicast.c
> @@ -44,10 +44,12 @@
>
> #include <stdlib.h>
> #include <string.h>
> +#include <opensm/osm_sa.h>
> #include <opensm/osm_multicast.h>
> #include <opensm/osm_mcm_port.h>
> #include <opensm/osm_mtree.h>
> #include <opensm/osm_inform.h>
> +#include <opensm/osm_mcm_info.h>
> #include <arpa/inet.h>
>
>
/**********************************************************************
> @@ -74,6 +76,31 @@ void osm_mgrp_delete(IN osm_mgrp_t * p_mgrp)
> free(p_mgrp);
> }
>
> +void osm_mgrp_delete_group(IN osm_subn_t * p_subn, IN osm_mgrp_t *
p_mgrp)
> +{
> + osm_mcm_port_t *p_mcm_port;
> + osm_mcm_port_t *p_next_mcm_port;
> +
> + CL_ASSERT(p_mgrp);
> +
> + osm_mgrp_holder_t *p_mgrp_holder =
> + osm_get_mgrp_holder_by_mlid(p_subn, p_mgrp->mlid);
> + p_next_mcm_port =
> + (osm_mcm_port_t *) cl_qmap_head(&p_mgrp->mcm_port_tbl);
> + while (p_next_mcm_port !=
> + (osm_mcm_port_t *) cl_qmap_end(&p_mgrp->mcm_port_tbl)) {
> + p_mcm_port = p_next_mcm_port;
> + p_next_mcm_port =
> + (osm_mcm_port_t *)
cl_qmap_next(&p_mcm_port->map_item);
> + osm_mgrp_holder_delete_mgrp_port(p_mgrp_holder, p_mgrp,
> +
p_mcm_port->port_gid.unicast.
> + interface_id);
> + osm_mcm_port_delete(p_mcm_port);
> + }
> + osm_mgrp_holder_delete_mgrp(p_mgrp_holder, p_mgrp);
> + free(p_mgrp);
> +}
> +
>
/**********************************************************************
>
**********************************************************************/
> osm_mgrp_t *osm_mgrp_new(IN const ib_net16_t mlid)
> @@ -378,3 +405,203 @@ static osm_mgrp_port_t
*osm_mgrp_port_new(ib_net64_t port_guid)
> cl_qlist_init(&p_mgrp_port->mgroups);
> return p_mgrp_port;
> }
> +
> +void osm_mgrp_holder_delete(IN osm_subn_t * p_subn, ib_net16_t mlid)
> +{
> + osm_mgrp_port_t *p_osm_mgr_port;
> + cl_map_item_t *p_item;
> + cl_fmap_item_t *p_fitem;
> + osm_mgrp_t *p_mgrp;
> +
> + osm_mgrp_holder_t *p_mgrp_holder =
> + p_subn->mgroup_holders[cl_ntoh16(mlid) -
IB_LID_MCAST_START_HO];
> + p_item = cl_qmap_head(&p_mgrp_holder->mgrp_port_map);
> + while (p_item != cl_qmap_end(&p_mgrp_holder->mgrp_port_map)) {
> + p_osm_mgr_port = (osm_mgrp_port_t *) p_item;
> + cl_qlist_remove_all(&p_osm_mgr_port->mgroups);
> + cl_qmap_remove_item(&p_mgrp_holder->mgrp_port_map,
p_item);
> + p_item = cl_qmap_head(&p_mgrp_holder->mgrp_port_map);
> + free(p_osm_mgr_port);
> + }
I'm not sure that I understand your data model very well (maybe it would
be a good idea to describe and implement this as separate patch). But do
you have port lists in both mgrp and mgrp_holder? Assuming so, why? This
code looks pretty overloaded.
> + p_fitem = cl_fmap_head(&p_mgrp_holder->mgrp_map);
> + while (p_fitem != cl_fmap_end(&p_mgrp_holder->mgrp_map)) {
> + p_mgrp =
> + (osm_mgrp_t *) PARENT_STRUCT(p_fitem, osm_mgrp_t,
> + mgid_item);
> + osm_mgrp_delete_group(p_subn, p_mgrp);
> + p_fitem = cl_fmap_head(&p_mgrp_holder->mgrp_map);
> + }
> + /* destroy the mtree_node structure */
> + osm_mtree_destroy(p_mgrp_holder->p_root);
> + p_subn->mgroup_holders[cl_ntoh16(mlid) - IB_LID_MCAST_START_HO]
= NULL;
> + free(p_mgrp_holder);
> +}
> +
> +osm_mgrp_holder_t *osm_mgrp_holder_new(IN osm_subn_t * p_subn,
> + ib_gid_t * p_mgid, ib_net16_t
mlid)
> +{
> + osm_mgrp_holder_t *p_mgrp_holder;
> + p_mgrp_holder =
> + p_subn->mgroup_holders[cl_ntoh16(mlid) -
IB_LID_MCAST_START_HO] =
> + (osm_mgrp_holder_t *) malloc(sizeof(*p_mgrp_holder));
> + if (!p_mgrp_holder)
> + return NULL;
> +
> + memset(p_mgrp_holder, 0, sizeof(*p_mgrp_holder));
> + p_mgrp_holder->mlid = mlid;
> + cl_fmap_init(&p_mgrp_holder->mgrp_map, __mgid_cmp);
> + cl_qmap_init(&p_mgrp_holder->mgrp_port_map);
> + osm_mgrp_holder_prepare_common_mgid(p_mgid,
> +
&p_mgrp_holder->common_mgid);
> + return p_mgrp_holder;
> +}
> +
> +ib_net16_t osm_mgrp_holder_get_mlid_by_mgid(IN osm_sa_t * sa,
> + IN const ib_gid_t * const
p_mgid)
> +{
> + int i;
> + ib_gid_t common_mgid;
> + osm_mgrp_holder_t *p_mgrp_holder;
> +
> + OSM_LOG_ENTER(sa->p_log);
> +
> + osm_mgrp_holder_prepare_common_mgid(p_mgid, &common_mgid);
> + for (i = 0; i <= sa->p_subn->max_mcast_lid_ho -
IB_LID_MCAST_START_HO;
> + i++) {
> + if (sa->p_subn->mgroup_holders[i]) {
> + p_mgrp_holder =
> + (osm_mgrp_holder_t *)
sa->p_subn->mgroup_holders[i];
> + if (!memcmp
> + (&p_mgrp_holder->common_mgid, &common_mgid,
> + sizeof(ib_gid_t))) {
> + char gid_str[INET6_ADDRSTRLEN];
> + OSM_LOG(sa->p_log, OSM_LOG_DEBUG,
> + "Found holder 0x%X for MGID
%s\n",
> + cl_ntoh16(p_mgrp_holder->mlid),
> + inet_ntop(AF_INET6, p_mgid->raw,
> + gid_str,
sizeof(gid_str)));
> + OSM_LOG_EXIT(sa->p_log);
> + return p_mgrp_holder->mlid;
> + }
> + }
> + }
Hmm, why do we need linear search her? What was a purpose of storing
mgrps in fleximap?
-Each fleximap contains mgroups with same MLID.
Linear search compares "common" mgid only.
> + OSM_LOG_EXIT(sa->p_log);
> + return 0;
+}
> +
> +void osm_mgrp_holder_remove_port(osm_subn_t * subn, osm_log_t *
p_log,
> + osm_mgrp_holder_t * p_mgrp_holder,
> + ib_net64_t port_guid)
> +{
> + osm_mgrp_t *p_mgrp;
> + cl_list_item_t *p_item;
> +
> + OSM_LOG_ENTER(p_log);
> +
> + osm_mgrp_port_t *p_mgrp_port = (osm_mgrp_port_t *)
> + cl_qmap_remove(&p_mgrp_holder->mgrp_port_map, port_guid);
> + if (p_mgrp_port !=
> + (osm_mgrp_port_t *)
cl_qmap_end(&p_mgrp_holder->mgrp_port_map)) {
> + char gid_str[INET6_ADDRSTRLEN];
> + OSM_LOG(p_log, OSM_LOG_DEBUG,
> + "port 0x%" PRIx64 " removed from mlid 0x%X\n",
> + port_guid, cl_ntoh16(p_mgrp_holder->mlid));
> + while ((p_item =
> + cl_qlist_remove_head(&p_mgrp_port->mgroups)) !=
> + cl_qlist_end(&p_mgrp_port->mgroups)) {
> + p_mgrp =
> + (osm_mgrp_t *) PARENT_STRUCT(p_item,
osm_mgrp_t,
> + mgrp_item);
> + OSM_LOG(p_log, OSM_LOG_DEBUG,
> + "removing mgrp mgid %s from port 0x%"
PRIx64
> + "\n", inet_ntop(AF_INET6,
> +
p_mgrp->mcmember_rec.mgid.raw,
> + gid_str,
sizeof(gid_str)),
> + cl_ntoh64(port_guid));
> + osm_mgrp_delete_port(subn, p_log, p_mgrp,
port_guid);
> + }
> + free(p_mgrp_port);
> + }
> + OSM_LOG_EXIT(p_log);
> +}
> +
> +void osm_mgrp_holder_add_mgrp(osm_mgrp_holder_t * p_mgrp_holder,
> + osm_mgrp_t * p_mgrp, osm_log_t * p_log)
> +{
> + cl_fmap_item_t *p_fitem;
> + char gid_str[INET6_ADDRSTRLEN];
> +
> + OSM_LOG_ENTER(p_log);
> +
> + if (p_mgrp_holder->to_be_deleted) {
> + /* this is re-used mgrp_holder, need to update
common_mgid */
> +
osm_mgrp_holder_prepare_common_mgid(&p_mgrp->mcmember_rec.mgid,
> + &p_mgrp_holder->
> + common_mgid);
> + }
> + p_fitem = cl_fmap_insert(&p_mgrp_holder->mgrp_map,
> + &p_mgrp->mcmember_rec.mgid,
> + &p_mgrp->mgid_item);
Ok, starting to understand (hopefully :)) - you have fleximap only per
mgrp_holder. Why? Wouldn't it better to have single (per subnet) mgrp
fleximap? I guess this will cover all common search cases and will
simplify a data model dramatically.
- I don't think so. In case of single fleximap search will always take
logN.
Having fleximap divided into groups per MLID will give logN in a worst
case.
Also, keeping mgroups per MLID simplifies queries like saquery -m <MLID>
Also (unrelated directly to this patch series). I have some local
cleanup/simplification changes in multicast stuff, in particular it
eliminates the need in all those 'to_be_deleted', 'tree_id', etc. flags.
I think this could help you with simpler implementation.
Sasha
-Your changes are already in git?
Slava
> + CL_ASSERT(p_item == &p_mgrp->mgid_item);
> +
> + OSM_LOG(p_log, OSM_LOG_DEBUG,
> + "mgrp with MGID:%s added to holder with mlid = 0x%X\n",
> + inet_ntop(AF_INET6, p_mgrp->mcmember_rec.mgid.raw,
gid_str,
> + sizeof(gid_str)),
cl_ntoh16(p_mgrp_holder->mlid));
> + p_mgrp_holder->last_change_id++;
> + OSM_LOG_EXIT(p_log);
> +}
> +
> +ib_api_status_t osm_mgrp_holder_add_mgrp_port(osm_mgrp_holder_t *
p_mgrp_holder,
> + osm_mgrp_t * p_mgrp,
> + ib_net64_t port_guid)
> +{
> + osm_mgrp_port_t *p_mgrp_port = (osm_mgrp_port_t *)
> + cl_qmap_get(&p_mgrp_holder->mgrp_port_map, port_guid);
> + if (p_mgrp_port ==
> + (osm_mgrp_port_t *)
cl_qmap_end(&p_mgrp_holder->mgrp_port_map)) {
> + /* new port to mlid */
> + p_mgrp_port = osm_mgrp_port_new(port_guid);
> + if (!p_mgrp_port) {
> + return IB_INSUFFICIENT_MEMORY;
> + }
> + cl_qmap_insert(&p_mgrp_holder->mgrp_port_map,
> + p_mgrp_port->port_guid,
&p_mgrp_port->guid_item);
> + cl_qlist_insert_tail(&p_mgrp_port->mgroups,
&p_mgrp->mgrp_item);
> + } else {
> + cl_qlist_insert_tail(&p_mgrp_port->mgroups,
&p_mgrp->mgrp_item);
> + }
> + return IB_SUCCESS;
> +}
> +
> +void osm_mgrp_holder_delete_mgrp_port(osm_mgrp_holder_t *
p_mgrp_holder,
> + osm_mgrp_t * p_mgrp, ib_net64_t
port_guid)
> +{
> + osm_mgrp_port_t *p_mgrp_port = (osm_mgrp_port_t *)
> + cl_qmap_get(&p_mgrp_holder->mgrp_port_map, port_guid);
> + if (p_mgrp_port !=
> + (osm_mgrp_port_t *)
cl_qmap_end(&p_mgrp_holder->mgrp_port_map)) {
> + cl_qlist_remove_item(&p_mgrp_port->mgroups,
&p_mgrp->mgrp_item);
> + if (0 == cl_qlist_count(&p_mgrp_port->mgroups)) {
> + /* No mgroups registered on this port for
current mlid */
> +
cl_qmap_remove_item(&p_mgrp_holder->mgrp_port_map,
> + &p_mgrp_port->guid_item);
> + free(p_mgrp_port);
> + }
> + p_mgrp_holder->last_change_id++;
> + }
> +}
> +
> +void osm_mgrp_holder_delete_mgrp(osm_mgrp_holder_t * p_mgrp_holder,
> + osm_mgrp_t * p_mgrp)
> +{
> + p_mgrp->to_be_deleted = 1;
> + cl_fmap_remove_item(&p_mgrp_holder->mgrp_map,
&p_mgrp->mgid_item);
> + if (0 == cl_fmap_count(&p_mgrp_holder->mgrp_map)) {
> + /* No more mgroups on this mlid */
> + p_mgrp_holder->to_be_deleted = 1;
> + p_mgrp_holder->last_tree_id = 0;
> + p_mgrp_holder->last_change_id = 0;
> + memset(&p_mgrp_holder->common_mgid, 0,
sizeof(ib_gid_t));
> + }
> +}
> --
> 1.5.5
>
More information about the general
mailing list