[ofa-general] Re: [PATCHv2] Add pkey table support to osm_get_all_port_attrs

[Sasha Khapyorsky]

Hi Hal,

On 13:44 Fri 27 Feb     , Hal Rosenstock wrote:
> 
> Only supported in osm_vendor_ibumad.c (separate patch for other
> vendor layers)
> Also, update applications using this (osmtest, opensm)
> 
> Signed-off-by: Hal Rosenstock <hal.rosenstock at gmail.com>
> 
> ---
> Changes from v1:
> Only copy number of pkeys indicated
> Also, don't indicate insufficient memory error if insufficient pkey space
> supplied and always return number of pkeys that the port supports
> 
> Note: initialization prior to get_all_port_attrs call not changed
> since it is faster this way
> 
> Other patch for other vendor layers still appropriate following this
> ibutils patch to come 

I think ibutils patch should go first - port_attr_array initialization
will not hurt, but doing it in opposite order will leave ibutils in
broken state.

> 
> diff --git a/opensm/libvendor/osm_vendor_ibumad.c b/opensm/libvendor/osm_vendor_ibumad.c
> index 734a860..7a578ea 100644
> --- a/opensm/libvendor/osm_vendor_ibumad.c
> +++ b/opensm/libvendor/osm_vendor_ibumad.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
> @@ -556,12 +557,13 @@ osm_vendor_get_all_port_attr(IN osm_vendor_t * const p_vend,
>  	umad_ca_t ca;
>  	ib_port_attr_t *attr = p_attr_array;
>  	unsigned done = 0;
> -	int r, i, j;
> +	int r, i, j, k;
>  
>  	OSM_LOG_ENTER(p_vend->p_log);
>  
>  	CL_ASSERT(p_vend && p_num_ports);
>  
> +	r = 0;
>  	if (!*p_num_ports) {
>  		r = IB_INVALID_PARAMETER;
>  		OSM_LOG(p_vend->p_log, OSM_LOG_ERROR, "ERR 5418: "
> @@ -576,9 +578,7 @@ osm_vendor_get_all_port_attr(IN osm_vendor_t * const p_vend,
>  	}
>  
>  	for (i = 0; i < p_vend->ca_count && !done; i++) {
> -		/*
> -		 * For each CA, retrieve the port guids
> -		 */
> +		/* For each CA, retrieve the port attributes */
>  		if (umad_get_ca(p_vend->ca_names[i], &ca) == 0) {
>  			if (ca.node_type < 1 || ca.node_type > 3)
>  				continue;
> @@ -590,6 +590,12 @@ osm_vendor_get_all_port_attr(IN osm_vendor_t * const p_vend,
>  				attr->port_num = ca.ports[j]->portnum;
>  				attr->sm_lid = ca.ports[j]->sm_lid;
>  				attr->link_state = ca.ports[j]->state;
> +				if (attr->num_pkeys && attr->p_pkey_table) {
> +					for (k = 0; k < attr->num_pkeys; k++)
> +						attr->p_pkey_table[k] =
> +							cl_hton16(ca.ports[j]->pkeys[k]);

And when attr->num_pkeys > ca.ports[j]->pkeys_size? It will copy
garbage?

> +				}
> +				attr->num_pkeys = ca.ports[j]->pkeys_size;
>  				attr++;
>  				if (attr - p_attr_array > *p_num_ports) {
>  					done = 1;
> @@ -601,7 +607,6 @@ osm_vendor_get_all_port_attr(IN osm_vendor_t * const p_vend,
>  	}
>  
>  	*p_num_ports = attr - p_attr_array;
> -	r = 0;
>  
>  Exit:
>  	OSM_LOG_EXIT(p_vend->p_log);
> diff --git a/opensm/opensm/main.c b/opensm/opensm/main.c
> index 47fd658..1507fff 100644
> --- a/opensm/opensm/main.c
> +++ b/opensm/opensm/main.c
> @@ -2,6 +2,7 @@
>   * Copyright (c) 2004-2008 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) 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

There should be initialization?

> diff --git a/opensm/osmtest/main.c b/opensm/osmtest/main.c
> index f87e33b..bc8999d 100644

Sasha