***SPAM*** Re: ***SPAM*** Re: [ofa-general] Bugs in opensm/libvendor
Hal Rosenstock
hal.rosenstock at gmail.com
Mon Dec 15 08:13:25 PST 2008
On Mon, Dec 15, 2008 at 11:11 AM, Hal Rosenstock
<hal.rosenstock at gmail.com> wrote:
> Mike,
>
> On Mon, Dec 15, 2008 at 11:07 AM, Mike Heinz <michael.heinz at qlogic.com> wrote:
>> Sasha, Hal,
>>
>> Reviewing the spec again, on page 915, there's an "X" in the required
>> column for numb_path and GETTABLE requests. Also, looking at the code,
>> this query is being sent as a IB_MAD_METHOD_GETTABLE, not
>> IB_MAD_METHOD_GET, so the number of paths requested needs to be
>> specified.
>
> Yes, that is the spec compliant view. OpenSM and a variety of tools
> rely on the extension to the spec where numb paths 0 means unlimited
> and supports a wider variety of queries here.
>
>> Since this function doesn't support returning multiple paths to the
>> caller, perhaps the correct fix is to change the method to
>> IB_MAD_METHOD_GET?
>
> Not IMO due to the above.
IMO the best solution is to extend the API for these to include
num_paths and support explicit settting of this as well as 0 so
backward compatibility is not lost.
-- Hal
> -- Hal
>
>> The SM we're testing against it Qlogic's.
>>
>> --
>> Michael Heinz
>> Principal Engineer, Qlogic Corporation
>> King of Prussia, Pennsylvania
>>
>> -----Original Message-----
>> From: Sasha Khapyorsky [mailto:sashak at voltaire.com]
>> Sent: Monday, December 15, 2008 10:39 AM
>> To: Mike Heinz
>> Cc: general at lists.openfabrics.org; John Russo; Hal Rosenstock
>> Subject: Re: [ofa-general] Bugs in opensm/libvendor
>>
>> On 09:29 Mon 15 Dec , Mike Heinz wrote:
>>>
>>> That's a good question - and I'm going to ask around and double check.
>>> My first reaction was that you have to specify how many paths you want
>>
>>> from the query - but you're right, the spec doesn't say that.
>>
>> Yes, it looks like this (but I cannot understand "why" :( ). But even
>> more strange (IMHO) limitation is mandatory SGID - actually it should
>> make illegal such GetTable queries as all-to-all, SLID-to-all, etc.. I
>> thought that it is permitted.
>>
>>> I'm going to do some research on my end. Are you saying that
>>> IB_MAD_ATTR_PATH_RECORD should only ever return a single path?
>>
>> With GetTable? I think it shouldn't (for some queries it will - such as
>> SLID + DLID).
>>
>> Sasha
>>
>>>
>>> --
>>> Michael Heinz
>>> Principal Engineer, Qlogic Corporation King of Prussia, Pennsylvania
>>>
>>> -----Original Message-----
>>> From: Sasha Khapyorsky [mailto:sashak at voltaire.com]
>>> Sent: Monday, December 15, 2008 10:18 AM
>>> To: Mike Heinz
>>> Cc: general at lists.openfabrics.org; John Russo; Hal Rosenstock
>>> Subject: Re: [ofa-general] Bugs in opensm/libvendor
>>>
>>> Hi Mike,
>>>
>>> On 12:31 Wed 10 Dec , Mike Heinz wrote:
>>> > While experimenting with the APIs in opensm/libvendor, I was unable
>>> > to
>>>
>>> > get the path record queries to work. Reviewing the error logs from
>>> > the
>>>
>>> > SM, I discovered that the APIs were not setting the required
>>> > num_path field.
>>>
>>> Actually this part of spec is not 100% clear for me - the only thing I
>>
>>> can see is that in table 207 (p.915 - PathRecord) is that SGID and
>>> NumbPath parameters are marked as "required for GetTable request".
>>> This leave me with some questions:
>>>
>>> - Could SLID be used in GetTable request instead of SGID (as
>> implemented
>>> now in opensm/libvendor)? Maybe not, but then I would expect some
>>> explicit mention about this. If yes, what is the meaning of NumbPath
>>> then?
>>>
>>> - What is the reason for such limitation.
>>>
>>> Do you or anybody on the list could clarify this?
>>>
>>> > Here's the fix:
>>>
>>> About the patch.
>>>
>>> White spaces are mangled.
>>>
>>> >
>>> > --- osm_vendor_ibumad_sa.bak 2008-12-10 13:21:22.000000000 -0500
>>> > +++ osm_vendor_ibumad_sa.c 2008-12-10 13:24:42.000000000 -0500
>>> > @@ -615,7 +615,7 @@
>>> > sa_mad_data.attr_offset =
>>> > ib_get_attr_offset(sizeof(ib_path_rec_t));
>>> > sa_mad_data.comp_mask =
>>> > - (IB_PR_COMPMASK_DGID | IB_PR_COMPMASK_SGID);
>>> > + (IB_PR_COMPMASK_DGID | IB_PR_COMPMASK_SGID |
>>> > IB_PR_COMPMASK_NUMBPATH);
>>> > sa_mad_data.p_attr = &path_rec;
>>> > ib_gid_set_default(&path_rec.dgid,
>>> > ((osmv_guid_pair_t *)
>>> > (p_query_req-> @@ -625,6 +625,7 @@
>>> > ((osmv_guid_pair_t *)
>>> > (p_query_req->
>>> >
>>> > p_query_input))->
>>> > src_guid);
>>> > + path_rec.num_path = 1;
>>>
>>> Why should this be '1'?
>>>
>>> Sasha
>>>
>>> > break;
>>> >
>>> > case OSMV_QUERY_PATH_REC_BY_GIDS:
>>> > @@ -634,7 +635,7 @@
>>> > sa_mad_data.attr_offset =
>>> > ib_get_attr_offset(sizeof(ib_path_rec_t));
>>> > sa_mad_data.comp_mask =
>>> > - (IB_PR_COMPMASK_DGID | IB_PR_COMPMASK_SGID);
>>> > + (IB_PR_COMPMASK_DGID | IB_PR_COMPMASK_SGID |
>>> > IB_PR_COMPMASK_NUMBPATH);
>>> > sa_mad_data.p_attr = &path_rec;
>>> > memcpy(&path_rec.dgid,
>>> > &((osmv_gid_pair_t *)
>>> > (p_query_req->p_query_input))-> @@ -642,6 +643,7 @@
>>> > memcpy(&path_rec.sgid,
>>> > &((osmv_gid_pair_t *)
>>> > (p_query_req->p_query_input))->
>>> > src_gid, sizeof(ib_gid_t));
>>> > + path_rec.num_path = 1;
>>> > break;
>>> >
>>> > case OSMV_QUERY_PATH_REC_BY_LIDS:
>>> > @@ -652,13 +654,14 @@
>>> > sa_mad_data.attr_offset =
>>> > ib_get_attr_offset(sizeof(ib_path_rec_t));
>>> > sa_mad_data.comp_mask =
>>> > - (IB_PR_COMPMASK_DLID | IB_PR_COMPMASK_SLID);
>>> > + (IB_PR_COMPMASK_DLID | IB_PR_COMPMASK_SLID |
>>> > IB_PR_COMPMASK_NUMBPATH);
>>> > sa_mad_data.p_attr = &path_rec;
>>> > path_rec.dlid =
>>> > ((osmv_lid_pair_t *)
>>> (p_query_req->p_query_input))->
>>> > dest_lid;
>>> > path_rec.slid =
>>> > ((osmv_lid_pair_t *)
>>> > (p_query_req->p_query_input))->src_lid;
>>> > + path_rec.num_path = 1;
>>> > break;
>>> >
>>> > case OSMV_QUERY_UD_MULTICAST_SET:
>>> > --- osm_vendor_mlx_sa.bak 2008-12-10 13:21:10.000000000 -0500
>>> > +++ osm_vendor_mlx_sa.c 2008-12-10 13:24:07.000000000 -0500
>>> > @@ -743,7 +743,7 @@
>>> > sa_mad_data.attr_offset =
>>> > ib_get_attr_offset(sizeof(ib_path_rec_t));
>>> > sa_mad_data.comp_mask =
>>> > - (IB_PR_COMPMASK_DGID | IB_PR_COMPMASK_SGID);
>>> > + (IB_PR_COMPMASK_DGID | IB_PR_COMPMASK_SGID |
>>> > IB_PR_COMPMASK_NUMBPATH);
>>> > sa_mad_data.p_attr = &path_rec;
>>> > ib_gid_set_default(&path_rec.dgid,
>>> > ((osmv_guid_pair_t *)
>>> > (p_query_req-> @@ -753,6 +753,7 @@
>>> > ((osmv_guid_pair_t *)
>>> > (p_query_req->
>>> >
>>> > p_query_input))->
>>> > src_guid);
>>> > + path_rec.num_path = 1;
>>> > break;
>>> >
>>> > case OSMV_QUERY_PATH_REC_BY_GIDS:
>>> > @@ -763,7 +764,7 @@
>>> > sa_mad_data.attr_offset =
>>> > ib_get_attr_offset(sizeof(ib_path_rec_t));
>>> > sa_mad_data.comp_mask =
>>> > - (IB_PR_COMPMASK_DGID | IB_PR_COMPMASK_SGID);
>>> > + (IB_PR_COMPMASK_DGID | IB_PR_COMPMASK_SGID |
>>> > IB_PR_COMPMASK_NUMBPATH);
>>> > sa_mad_data.p_attr = &path_rec;
>>> > memcpy(&path_rec.dgid,
>>> > &((osmv_gid_pair_t *)
>>> > (p_query_req->p_query_input))-> @@ -771,6 +772,7 @@
>>> > memcpy(&path_rec.sgid,
>>> > &((osmv_gid_pair_t *)
>>> > (p_query_req->p_query_input))->
>>> > src_gid, sizeof(ib_gid_t));
>>> > + path_rec.num_path = 1;
>>> > break;
>>> >
>>> > case OSMV_QUERY_PATH_REC_BY_LIDS:
>>> > @@ -789,6 +791,7 @@
>>> > dest_lid;
>>> > path_rec.slid =
>>> > ((osmv_lid_pair_t *)
>>> > (p_query_req->p_query_input))->src_lid;
>>> > + path_rec.num_path = 1;
>>> > break;
>>> >
>>> > case OSMV_QUERY_UD_MULTICAST_SET:
>>> >
>>> >
>>> > --
>>> > Michael Heinz
>>> > Principal Engineer, Qlogic Corporation King of Prussia, Pennsylvania
>>> >
>>>
>>> > _______________________________________________
>>> > 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
>>
> _______________________________________________
> 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