[ofa-general] Re: [PATCH 1/2] osm: QoS - replace guid ranges and partition list by port map

Sasha Khapyorsky sashak at voltaire.com
Mon Sep 17 15:10:03 PDT 2007


Hi Yevgeny,

Small comment is below.

On 21:22 Mon 17 Sep     , Yevgeny Kliteynik wrote:
> QoS policy optimization: replacing partition list and guid
> ranges in a port group by port map indexed by port guid.
> The port map is filled at parse time, thus checking whether
> some port belongs to a group becomes a single map query.
> 
> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
> ---
>  opensm/include/opensm/osm_qos_policy.h |   11 ++-
>  opensm/opensm/osm_qos_parser.y         |  115 ++++++++++++++++++++++++++-----
>  opensm/opensm/osm_qos_policy.c         |   59 ++++++++---------
>  3 files changed, 131 insertions(+), 54 deletions(-)
> 
> diff --git a/opensm/include/opensm/osm_qos_policy.h b/opensm/include/opensm/osm_qos_policy.h
> index 0c220ee..680bf71 100644
> --- a/opensm/include/opensm/osm_qos_policy.h
> +++ b/opensm/include/opensm/osm_qos_policy.h
> @@ -50,6 +50,7 @@
>  #include <iba/ib_types.h>
>  #include <complib/cl_list.h>
>  #include <opensm/osm_port.h>
> +#include <opensm/osm_partition.h>
>  #include <opensm/osm_sa_path_record.h>
>  #include <opensm/osm_sa_multipath_record.h>
> 
> @@ -59,17 +60,20 @@
> 
>  /***************************************************/
> 
> +typedef struct _osm_qos_port_t {
> +	cl_map_item_t map_item;
> +	osm_physp_t * p_physp;
> +} osm_qos_port_t;
> +
>  typedef struct _osm_qos_port_group_t {
>  	char *name;			/* single string (this port group name) */
>  	char *use;			/* single string (description) */
>  	cl_list_t port_name_list;	/* list of port names (.../.../...) */
> -	uint64_t **guid_range_arr;	/* array of guid ranges (pair of 64-bit guids) */
> -	unsigned guid_range_len;	/* num of guid ranges in the array */
> -	cl_list_t partition_list;	/* list of partition names */
>  	boolean_t node_type_ca;
>  	boolean_t node_type_switch;
>  	boolean_t node_type_router;
>  	boolean_t node_type_self;
> +	cl_qmap_t port_map;
>  } osm_qos_port_group_t;
> 
>  /***************************************************/
> @@ -147,6 +151,7 @@ typedef struct _osm_qos_policy_t {
> 
>  /***************************************************/
> 
> +osm_qos_port_t *osm_qos_policy_port_create(osm_physp_t * p_physp);
>  osm_qos_port_group_t * osm_qos_policy_port_group_create();
>  void osm_qos_policy_port_group_destroy(osm_qos_port_group_t * p_port_group);
> 
> diff --git a/opensm/opensm/osm_qos_parser.y b/opensm/opensm/osm_qos_parser.y
> index a477084..ca77536 100644
> --- a/opensm/opensm/osm_qos_parser.y
> +++ b/opensm/opensm/osm_qos_parser.y
> @@ -103,6 +103,19 @@ static void __merge_rangearr(
>      uint64_t  ** * p_arr,
>      unsigned     * p_arr_len );
> 
> +static void __parser_add_port_to_port_map(
> +    cl_qmap_t   * p_map,
> +    osm_physp_t * p_physp);
> +
> +static void __parser_add_range_to_port_map(
> +    cl_qmap_t  * p_map,
> +    uint64_t  ** range_arr,
> +    unsigned     range_len);
> +
> +static void __parser_add_map_to_port_map(
> +    cl_qmap_t * p_dmap,
> +    cl_map_t  * p_smap);
> +
>  extern char * __qos_parser_text;
>  extern void __qos_parser_error (char *s);
>  extern int __qos_parser_lex (void);
> @@ -612,24 +625,9 @@ port_group_port_guid:   port_group_port_guid_start list_of_ranges {
>                                                        &range_arr,
>                                                        &range_len );
> 
> -                                if ( !p_current_port_group->guid_range_len )
> -                                {
> -                                    p_current_port_group->guid_range_arr = range_arr;
> -                                    p_current_port_group->guid_range_len = range_len;
> -                                }
> -                                else
> -                                {
> -                                    uint64_t ** new_range_arr;
> -                                    unsigned new_range_len;
> -                                    __merge_rangearr( p_current_port_group->guid_range_arr,
> -                                                      p_current_port_group->guid_range_len,
> -                                                      range_arr,
> -                                                      range_len,
> -                                                      &new_range_arr,
> -                                                      &new_range_len );
> -                                    p_current_port_group->guid_range_arr = new_range_arr;
> -                                    p_current_port_group->guid_range_len = new_range_len;
> -                                }
> +                                __parser_add_range_to_port_map(&p_current_port_group->port_map,
> +                                                               range_arr,
> +                                                               range_len);
>                              }
>                          }
>                          ;
> @@ -643,13 +641,26 @@ port_group_partition:  port_group_partition_start string_list {
>                              /* 'partition' in 'port-group' - any num of instances */
>                              cl_list_iterator_t    list_iterator;
>                              char                * tmp_str;
> +                            osm_prtn_t          * p_prtn;
> 
> +                            /* extract all the ports from the partition
> +                               to the port map of this port group */
>                              list_iterator = cl_list_head(&tmp_parser_struct.str_list);
>                              while( list_iterator != cl_list_end(&tmp_parser_struct.str_list) )
>                              {
>                                  tmp_str = (char*)cl_list_obj(list_iterator);
>                                  if (tmp_str)
> -                                    cl_list_insert_tail(&p_current_port_group->partition_list,tmp_str);
> +                                {
> +                                    p_prtn = osm_prtn_find_by_name(p_qos_policy->p_subn, tmp_str);
> +                                    if (p_prtn)
> +                                    {
> +                                        __parser_add_map_to_port_map(&p_current_port_group->port_map,
> +                                                                     &p_prtn->part_guid_tbl);
> +                                        __parser_add_map_to_port_map(&p_current_port_group->port_map,
> +                                                                     &p_prtn->full_guid_tbl);
> +                                    }
> +                                    free(tmp_str);
> +                                }
>                                  list_iterator = cl_list_next(list_iterator);
>                              }
>                              cl_list_remove_all(&tmp_parser_struct.str_list);
> @@ -2185,3 +2196,69 @@ static void __merge_rangearr(
> 
>  /***************************************************
>   ***************************************************/
> +
> +static void __parser_add_port_to_port_map(
> +    cl_qmap_t   * p_map,
> +    osm_physp_t * p_physp)
> +{
> +    if (p_physp && osm_physp_is_valid(p_physp) &&
> +        cl_qmap_get(p_map, cl_ntoh64(
> +           osm_physp_get_port_guid(p_physp))) == cl_qmap_end(p_map))
> +    {
> +        osm_qos_port_t * p_port = osm_qos_policy_port_create(p_physp);

Here mem allocation can fail and p_port will be NULL. Don't need to
resubmit the patch for this, just send subsequent patch.

Sasha

> +        cl_qmap_insert(p_map,
> +                       cl_ntoh64(osm_physp_get_port_guid(p_physp)),
> +                       &p_port->map_item);
> +    }
> +}
> +
> +/***************************************************
> + ***************************************************/
> +
> +static void __parser_add_range_to_port_map(
> +    cl_qmap_t  * p_map,
> +    uint64_t  ** range_arr,
> +    unsigned     range_len)
> +{
> +    unsigned i;
> +    uint64_t guid_ho;
> +    osm_port_t * p_osm_port;
> +
> +    if (!range_arr || !range_len)
> +        return;
> +
> +    for (i = 0; i < range_len; i++) {
> +         for (guid_ho = range_arr[i][0]; guid_ho <= range_arr[i][1]; guid_ho++) {
> +             p_osm_port =
> +                osm_get_port_by_guid(p_qos_policy->p_subn, cl_hton64(guid_ho));
> +             if (p_osm_port)
> +                 __parser_add_port_to_port_map(p_map, p_osm_port->p_physp);
> +         }
> +         free(range_arr[i]);
> +    }
> +    free(range_arr);
> +}
> +
> +/***************************************************
> + ***************************************************/
> +
> +static void __parser_add_map_to_port_map(
> +    cl_qmap_t * p_dmap,
> +    cl_map_t  * p_smap)
> +{
> +    cl_map_iterator_t map_iterator;
> +    osm_physp_t * p_physp;
> +
> +    if (!p_dmap || !p_smap)
> +        return;
> +
> +    map_iterator = cl_map_head(p_smap);
> +    while (map_iterator != cl_map_end(p_smap)) {
> +        p_physp = (osm_physp_t*)cl_map_obj(map_iterator);
> +        __parser_add_port_to_port_map(p_dmap, p_physp);
> +        map_iterator = cl_map_next(map_iterator);
> +    }
> +}
> +
> +/***************************************************
> + ***************************************************/
> diff --git a/opensm/opensm/osm_qos_policy.c b/opensm/opensm/osm_qos_policy.c
> index b2d1622..d1b227f 100644
> --- a/opensm/opensm/osm_qos_policy.c
> +++ b/opensm/opensm/osm_qos_policy.c
> @@ -101,6 +101,27 @@ static void __free_single_element(void *p_element, void *context)
>  		free(p_element);
>  }
> 
> +static void __free_port_map_element(cl_map_item_t *p_element, void *context)
> +{
> +	if (p_element)
> +		free(p_element);
> +}
> +
> +/***************************************************
> + ***************************************************/
> +
> +osm_qos_port_t *osm_qos_policy_port_create(osm_physp_t *p_physp)
> +{
> +	osm_qos_port_t *p =
> +	    (osm_qos_port_t *) malloc(sizeof(osm_qos_port_t));
> +	if (!p)
> +		return NULL;
> +	memset(p, 0, sizeof(osm_qos_port_t));
> +
> +	p->p_physp = p_physp;
> +	return p;
> +}
> +
>  /***************************************************
>   ***************************************************/
> 
> @@ -114,7 +135,7 @@ osm_qos_port_group_t *osm_qos_policy_port_group_create()
>  	memset(p, 0, sizeof(osm_qos_port_group_t));
> 
>  	cl_list_init(&p->port_name_list, 10);
> -	cl_list_init(&p->partition_list, 10);
> +	cl_qmap_init(&p->port_map);
> 
>  	return p;
>  }
> @@ -124,8 +145,6 @@ osm_qos_port_group_t *osm_qos_policy_port_group_create()
> 
>  void osm_qos_policy_port_group_destroy(osm_qos_port_group_t * p)
>  {
> -	unsigned i;
> -
>  	if (!p)
>  		return;
> 
> @@ -134,18 +153,12 @@ void osm_qos_policy_port_group_destroy(osm_qos_port_group_t * p)
>  	if (p->use)
>  		free(p->use);
> 
> -	for (i = 0; i < p->guid_range_len; i++)
> -		free(p->guid_range_arr[i]);
> -	if (p->guid_range_arr)
> -		free(p->guid_range_arr);
> -
>  	cl_list_apply_func(&p->port_name_list, __free_single_element, NULL);
>  	cl_list_remove_all(&p->port_name_list);
>  	cl_list_destroy(&p->port_name_list);
> 
> -	cl_list_apply_func(&p->partition_list, __free_single_element, NULL);
> -	cl_list_remove_all(&p->partition_list);
> -	cl_list_destroy(&p->partition_list);
> +	cl_qmap_apply_func(&p->port_map,  __free_port_map_element,  NULL);
> +	cl_qmap_remove_all(&p->port_map);
> 
>  	free(p);
>  }
> @@ -491,12 +504,9 @@ __qos_policy_is_port_in_group(osm_subn_t * p_subn,
>  			      osm_qos_port_group_t * p_port_group)
>  {
>  	osm_node_t *p_node = osm_physp_get_node_ptr(p_physp);
> -	osm_prtn_t *p_prtn = NULL;
>  	ib_net64_t port_guid = osm_physp_get_port_guid(p_physp);
>  	uint64_t port_guid_ho = cl_ntoh64(port_guid);
>  	uint8_t node_type = osm_node_get_type(p_node);
> -	cl_list_iterator_t list_iterator;
> -	char *partition_name;
> 
>  	/* check whether this port's type matches any of group's types */
> 
> @@ -506,27 +516,12 @@ __qos_policy_is_port_in_group(osm_subn_t * p_subn,
>  		&& p_port_group->node_type_router))
>  		return TRUE;
> 
> -	/* check whether this port's guid is in range of this group's guids */
> +	/* check whether this port's guid is in group's port map */
> 
> -	if (__is_num_in_range_arr(p_port_group->guid_range_arr,
> -				  p_port_group->guid_range_len, port_guid_ho))
> +	if (cl_qmap_get(&p_port_group->port_map, port_guid_ho) !=
> +	    cl_qmap_end(&p_port_group->port_map))
>  		return TRUE;
> 
> -	/* check whether this port is member of this group's partitions */
> -
> -	list_iterator = cl_list_head(&p_port_group->partition_list);
> -	while (list_iterator != cl_list_end(&p_port_group->partition_list)) {
> -		partition_name = (char *)cl_list_obj(list_iterator);
> -		if (partition_name && strlen(partition_name)) {
> -			p_prtn = osm_prtn_find_by_name(p_subn, partition_name);
> -			if (p_prtn) {
> -				if (osm_prtn_is_guid(p_prtn, port_guid))
> -					return TRUE;
> -			}
> -		}
> -		list_iterator = cl_list_next(list_iterator);
> -	}
> -
>  	/* check whether this port's name matches any of group's names */
> 
>  	/*
> -- 
> 1.5.1.4
> 
> 



More information about the general mailing list