Mike, On Mon, Dec 15, 2008 at 11:07 AM, Mike Heinz <[email protected]> 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:[email protected]] > Sent: Monday, December 15, 2008 10:39 AM > To: Mike Heinz > Cc: [email protected]; 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:[email protected]] >> Sent: Monday, December 15, 2008 10:18 AM >> To: Mike Heinz >> Cc: [email protected]; 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 >> > [email protected] >> > http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general >> > >> > To unsubscribe, please visit >> > http://openib.org/mailman/listinfo/openib-general > _______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
