[ofa-general] Re: [PATCH 3/3] osm: QoS - parsing port names
Sasha Khapyorsky
sashak at voltaire.com
Sat Oct 13 13:25:59 PDT 2007
Hi Yevgeny,
On 11:01 Tue 09 Oct , Yevgeny Kliteynik wrote:
> Added CA-by-name hash to the QoS policy object and
Why it is called "CA"-by-name? In the code below I see that hash is
created for all nodes (including switches and routers).
> as port names are parsed they use this hash to locate
> that actual port that the name refers to.
> For now I prefer to keep this hash local, so it's part
> of QoS policy object.
> When the same parser will be used for partitions too,
> this hash will be moved to be part of the subnet object.
>
> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
> ---
> opensm/include/opensm/osm_qos_policy.h | 3 +-
> opensm/opensm/osm_qos_parser.y | 73 +++++++++++++++++++++++++++-----
> opensm/opensm/osm_qos_policy.c | 36 +++++++++++++---
> 3 files changed, 94 insertions(+), 18 deletions(-)
>
> diff --git a/opensm/include/opensm/osm_qos_policy.h b/opensm/include/opensm/osm_qos_policy.h
> index 30c2e6d..5c32896 100644
> --- a/opensm/include/opensm/osm_qos_policy.h
> +++ b/opensm/include/opensm/osm_qos_policy.h
> @@ -49,6 +49,7 @@
>
> #include <iba/ib_types.h>
> #include <complib/cl_list.h>
> +#include <opensm/st.h>
> #include <opensm/osm_port.h>
> #include <opensm/osm_partition.h>
> #include <opensm/osm_sa_path_record.h>
> @@ -72,7 +73,6 @@ typedef struct _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 (.../.../...) */
> uint8_t node_types; /* node types bitmask */
> cl_qmap_t port_map;
> } osm_qos_port_group_t;
> @@ -148,6 +148,7 @@ typedef struct _osm_qos_policy_t {
> cl_list_t qos_match_rules; /* list of osm_qos_match_rule_t */
> osm_qos_level_t *p_default_qos_level; /* default QoS level */
> osm_subn_t *p_subn; /* osm subnet object */
> + st_table * p_ca_hash; /* hash of CAs by node description */
> } osm_qos_policy_t;
>
> /***************************************************/
> diff --git a/opensm/opensm/osm_qos_parser.y b/opensm/opensm/osm_qos_parser.y
> index 2405519..cf342d3 100644
> --- a/opensm/opensm/osm_qos_parser.y
> +++ b/opensm/opensm/osm_qos_parser.y
> @@ -603,23 +603,74 @@ port_group_use_start: TK_USE {
>
> port_group_port_name: port_group_port_name_start string_list {
> /* 'port-name' in 'port-group' - any num of instances */
> - cl_list_iterator_t list_iterator;
> - char * tmp_str;
> -
> - list_iterator = cl_list_head(&tmp_parser_struct.str_list);
> - while( list_iterator != cl_list_end(&tmp_parser_struct.str_list) )
> + cl_list_iterator_t list_iterator;
> + osm_node_t * p_node;
> + osm_physp_t * p_physp;
> + unsigned port_num;
> + char * name_str;
> + char * tmp_str;
> + char * host_str;
> + char * ca_str;
> + char * port_str;
> + char * node_desc = (char*)malloc(IB_NODE_DESCRIPTION_SIZE + 1);
> +
> + /* parsing port name strings */
> + for (list_iterator = cl_list_head(&tmp_parser_struct.str_list);
> + list_iterator != cl_list_end(&tmp_parser_struct.str_list);
> + list_iterator = cl_list_next(list_iterator))
> {
> tmp_str = (char*)cl_list_obj(list_iterator);
> + if (tmp_str && *tmp_str)
> + {
> + name_str = tmp_str;
> + host_str = strtok (name_str,"/");
> + ca_str = strtok (NULL, "/");
> + port_str = strtok (NULL, "/");
> +
> + if (!host_str || !(*host_str) ||
> + !ca_str || !(*ca_str) ||
> + !port_str || !(*port_str) ||
> + (port_str[0] != 'p' && port_str[0] != 'P')) {
> + yyerror("illegal port name");
> + free(tmp_str);
> + free(node_desc);
> + cl_list_remove_all(&tmp_parser_struct.str_list);
> + return 1;
> + }
>
> - /*
> - * TODO: parse port name strings
> - */
> + if (!(port_num = strtoul(&port_str[1],NULL,0))) {
> + yyerror("illegal port number in port name");
> + free(tmp_str);
> + free(node_desc);
> + cl_list_remove_all(&tmp_parser_struct.str_list);
> + return 1;
> + }
>
> - if (tmp_str)
> - cl_list_insert_tail(&p_current_port_group->port_name_list,tmp_str);
> - list_iterator = cl_list_next(list_iterator);
> + sprintf(node_desc,"%s %s",host_str,ca_str);
> + free(tmp_str);
> +
> + if (st_lookup(p_qos_policy->p_ca_hash,
> + (st_data_t)node_desc,
> + (st_data_t*)&p_node))
I am not following this. Hash key is generated as "host_str ca_str", but
below where hash table is filled NodeDescription string is used. Why
this should be same?
> + {
> + /* we found the node, now get the right port */
> + CL_ASSERT(p_node);
Why this CL_ASSERT() needed?
> + p_physp = osm_node_get_physp_ptr(p_node, port_num);
> + if (!p_physp) {
> + yyerror("port number out of range in port name");
> + free(tmp_str);
> + free(node_desc);
> + cl_list_remove_all(&tmp_parser_struct.str_list);
> + return 1;
> + }
> + /* we found the port, now add it to guid table */
> + __parser_add_port_to_port_map(&p_current_port_group->port_map,
> + p_physp);
> + }
> + }
> }
> cl_list_remove_all(&tmp_parser_struct.str_list);
> + free(node_desc);
> }
> ;
>
> diff --git a/opensm/opensm/osm_qos_policy.c b/opensm/opensm/osm_qos_policy.c
> index 51dd7b9..0d7235f 100644
> --- a/opensm/opensm/osm_qos_policy.c
> +++ b/opensm/opensm/osm_qos_policy.c
> @@ -59,6 +59,31 @@
> /***************************************************
> ***************************************************/
>
> +static void
> +__build_cabyname_hash(osm_qos_policy_t * p_qos_policy)
> +{
> + osm_node_t * p_node;
> + cl_qmap_t * p_node_guid_tbl = &p_qos_policy->p_subn->node_guid_tbl;
> +
> + p_qos_policy->p_ca_hash = st_init_strtable();
> + CL_ASSERT(p_qos_policy->p_ca_hash);
> +
> + if (!p_node_guid_tbl || !cl_qmap_count(p_node_guid_tbl))
> + return;
> +
> + for (p_node = (osm_node_t *) cl_qmap_head(p_node_guid_tbl);
> + p_node != (osm_node_t *) cl_qmap_end(p_node_guid_tbl);
> + p_node = (osm_node_t *) cl_qmap_next(&p_node->map_item)) {
> + if (p_node->node_info.node_type == IB_NODE_TYPE_CA)
> + st_insert(p_qos_policy->p_ca_hash,
> + (st_data_t)p_node->print_desc,
> + (st_data_t)p_node);
Hmm, why do you think NodeDescription will be unique for each node in a
fabric?
Sasha
> + }
> +}
> +
> +/***************************************************
> + ***************************************************/
> +
> static boolean_t
> __is_num_in_range_arr(uint64_t ** range_arr,
> unsigned range_arr_len, uint64_t num)
> @@ -127,8 +152,6 @@ osm_qos_port_group_t *osm_qos_policy_port_group_create()
> return NULL;
>
> memset(p, 0, sizeof(osm_qos_port_group_t));
> -
> - cl_list_init(&p->port_name_list, 10);
> cl_qmap_init(&p->port_map);
>
> return p;
> @@ -150,10 +173,6 @@ void osm_qos_policy_port_group_destroy(osm_qos_port_group_t * p)
> if (p->use)
> free(p->use);
>
> - 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);
> -
> p_port = (osm_qos_port_t *) cl_qmap_head(&p->port_map);
> while (p_port != (osm_qos_port_t *) cl_qmap_end(&p->port_map))
> {
> @@ -423,6 +442,8 @@ osm_qos_policy_t * osm_qos_policy_create(osm_subn_t * p_subn)
> cl_list_init(&p_qos_policy->qos_match_rules, 10);
>
> p_qos_policy->p_subn = p_subn;
> + __build_cabyname_hash(p_qos_policy);
> +
> return p_qos_policy;
> }
>
> @@ -495,6 +516,9 @@ void osm_qos_policy_destroy(osm_qos_policy_t * p_qos_policy)
> cl_list_remove_all(&p_qos_policy->qos_match_rules);
> cl_list_destroy(&p_qos_policy->qos_match_rules);
>
> + if (p_qos_policy->p_ca_hash)
> + st_free_table(p_qos_policy->p_ca_hash);
> +
> free(p_qos_policy);
>
> p_qos_policy = NULL;
> --
> 1.5.1.4
>
>
More information about the general
mailing list