***SPAM*** Re: [ofa-general] Bugs in opensm/libvendor
Hal Rosenstock
hal.rosenstock at gmail.com
Mon Dec 15 08:11:00 PST 2008
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.
-- 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
>
More information about the general
mailing list