On Mon, Dec 15, 2008 at 11:11 AM, Hal Rosenstock <[email protected]> wrote: > 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.
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:[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 > _______________________________________________ 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
