***SPAM*** Re: [ofa-general] Re: [PATCH] Add pkey table support to osm_get_all_port_attrs
Hal Rosenstock
hal.rosenstock at gmail.com
Thu Feb 26 04:03:02 PST 2009
Sasha,
On Thu, Feb 26, 2009 at 2:06 AM, Sasha Khapyorsky <sashak at voltaire.com> wrote:
> Hi Hal,
>
> On 10:30 Wed 18 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>
>> ---
>> opensm/libvendor/osm_vendor_ibumad.c | 24 +++++++++++++++++++-----
>> opensm/opensm/main.c | 6 ++++++
>> opensm/osmtest/main.c | 11 +++++++++++
>> opensm/osmtest/osmtest.c | 7 +++++++
>> 4 files changed, 43 insertions(+), 5 deletions(-)
>>
>> diff --git a/opensm/libvendor/osm_vendor_ibumad.c b/opensm/libvendor/osm_vendor_ibumad.c
>> index 734a860..861bfbe 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,21 @@ 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;
>> + attr->num_pkeys = ca.ports[j]->pkeys_size;
>> + if (attr->num_pkeys && attr->p_pkey_table) {
>> + if (attr->num_pkeys < ca.ports[j]->pkeys_size) {
>
> You are doing:
>
> attr->num_pkeys = ca.ports[j]->pkeys_size;
>
> , just two lines above, so this check will be always false.
Oops; I'll fix in the next version.
>> + r = IB_INSUFFICIENT_MEMORY;
>> + OSM_LOG(p_vend->p_log,
>> + OSM_LOG_ERROR,
>> + "ERR 5419: Insufficient memory for pkeys for port %d; need space for %d pkeys\n",
>> + j,
>> + ca.ports[j]->pkeys_size);
>
> Also should it be an error? May be it is just enough to fill requested
> pkey entries?
I agree that being more forgiving is better but then how would it be
known if the pkeys are being truncated ?
Also, it seems to be the style of the API (what is done for ports).
Can't just request an individal port but all ports.
>> + goto Exit;
>> + }
>> + for (k = 0; k < attr->num_pkeys; k++)
>> + attr->p_pkey_table[k] =
>> + cl_hton16(ca.ports[j]->pkeys[k]);
>> + }
>> attr++;
>> if (attr - p_attr_array > *p_num_ports) {
>> done = 1;
>> @@ -601,7 +616,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 73a6274..503d7fa 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
>> @@ -364,6 +365,11 @@ static ib_net64_t get_port_guid(IN osm_opensm_t * p_osm, uint64_t port_guid)
>> uint32_t i, choice = 0;
>> ib_api_status_t status;
>>
>> + for (i = 0; i < num_ports; i++) {
>> + attr_array[i].num_pkeys = 0;
>> + attr_array[i].p_pkey_table = NULL;
>> + }
>> +
>
> Here and below. Just
>
> memset(attr_array, 0, sizeof(attr_array));
>
> would be enough.
Sure; next version.
-- Hal
> Sasha
>
>> /* Call the transport layer for a list of local port GUID values */
>> status = osm_vendor_get_all_port_attr(p_osm->p_vendor, attr_array,
>> &num_ports);
>> diff --git a/opensm/osmtest/main.c b/opensm/osmtest/main.c
>> index b360af6..83c1e13 100644
>> --- a/opensm/osmtest/main.c
>> +++ b/opensm/osmtest/main.c
>> @@ -1,6 +1,7 @@
>> /*
>> * Copyright (c) 2004-2006 Voltaire, Inc. All rights reserved.
>> * Copyright (c) 2002-2005 Mellanox Technologies LTD. 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
>> @@ -217,6 +218,11 @@ static void print_all_guids(IN osmtest_t * p_osmt)
>> ib_port_attr_t attr_array[MAX_LOCAL_IBPORTS];
>> int i;
>>
>> + for (i = 0; i < num_ports; i++) {
>> + attr_array[i].num_pkeys = 0;
>> + attr_array[i].p_pkey_table = NULL;
>> + }
>> +
>> /*
>> Call the transport layer for a list of local port
>> GUID values.
>> @@ -245,6 +251,11 @@ ib_net64_t get_port_guid(IN osmtest_t * p_osmt, uint64_t port_guid)
>> ib_port_attr_t attr_array[MAX_LOCAL_IBPORTS];
>> int i;
>>
>> + for (i = 0; i < num_ports; i++) {
>> + attr_array[i].num_pkeys = 0;
>> + attr_array[i].p_pkey_table = NULL;
>> + }
>> +
>> /*
>> Call the transport layer for a list of local port
>> GUID values.
>> diff --git a/opensm/osmtest/osmtest.c b/opensm/osmtest/osmtest.c
>> index a7b343f..986a8d2 100644
>> --- a/opensm/osmtest/osmtest.c
>> +++ b/opensm/osmtest/osmtest.c
>> @@ -2,6 +2,7 @@
>> * Copyright (c) 2006-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
>> @@ -7096,9 +7097,15 @@ osmtest_bind(IN osmtest_t * p_osmt,
>> ib_api_status_t status;
>> uint32_t num_ports = MAX_LOCAL_IBPORTS;
>> ib_port_attr_t attr_array[MAX_LOCAL_IBPORTS];
>> + int i;
>>
>> OSM_LOG_ENTER(&p_osmt->log);
>>
>> + for (i = 0; i < num_ports; i++) {
>> + attr_array[i].num_pkeys = 0;
>> + attr_array[i].p_pkey_table = NULL;
>> + }
>> +
>> /*
>> * Call the transport layer for a list of local port
>> * GUID values.
>> --
>> 1.5.6.4
>>
> _______________________________________________
> 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