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

[Hal Rosenstock]

Sasha,

On Thu, Mar 5, 2009 at 6:55 PM, Sasha Khapyorsky <sashak at voltaire.com> wrote:
> 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.

Fine; I'll shortly send this patch but hope the rest won't be pended
waiting for it to be applied.

> - Show quoted text -
>>
>> 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?

I'll fix this in v3 of this patch once I understand what you mean below.

>> +                             }
>> +                             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?

I don't understand your question.

-- Hal

>> diff --git a/opensm/osmtest/main.c b/opensm/osmtest/main.c
>> index f87e33b..bc8999d 100644
>
> Sasha
> - Show quoted text -
> _______________________________________________
> 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
>