[ofa-general] Bugs in opensm/libvendor
Mike Heinz
michael.heinz at qlogic.com
Mon Dec 15 08:07:52 PST 2008
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.
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?
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