On Wed, Jun 9, 2010 at 8:10 AM, Sasha Khapyorsky <[email protected]> wrote: > On 06:23 Wed 09 Jun , Hal Rosenstock wrote: >> >> >> >> Signed-off-by: Hal Rosenstock <[email protected]> >> >> --- >> >> opensm/opensm/osm_sa.c | 12 ++++++++++-- >> >> 1 files changed, 10 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/opensm/opensm/osm_sa.c b/opensm/opensm/osm_sa.c >> >> index 0aca81f..8325632 100644 >> >> --- a/opensm/opensm/osm_sa.c >> >> +++ b/opensm/opensm/osm_sa.c >> >> @@ -3,6 +3,7 @@ >> >> * Copyright (c) 2002-2010 Mellanox Technologies LTD. All rights >> >> reserved. >> >> * Copyright (c) 1996-2003 Intel Corporation. All rights reserved. >> >> * Copyright (c) 2008 Xsigo Systems Inc. All rights reserved. >> >> + * Copyright (c) 2010 HNR Consulting. All rights reserved. >> >> * >> >> * This software is available to you under a choice of one of two >> >> * licenses. You may choose to be licensed under the terms of the GNU >> >> @@ -454,8 +455,15 @@ void osm_sa_respond(osm_sa_t *sa, osm_madw_t *madw, >> >> size_t attr_size, >> >> /* C15-0.1.5 - always return SM_Key = 0 (table 185 p 884) */ >> >> resp_sa_mad->sm_key = 0; >> >> >> >> - /* Fill in the offset (paylen will be done by the rmpp SAR) */ >> >> - resp_sa_mad->attr_offset = num_rec ? ib_get_attr_offset(attr_size) >> >> : 0; >> >> +#ifdef DUAL_SIDED_RMPP >> >> + if (resp_sa_mad->method == IB_MAD_METHOD_GETTABLE_RESP || >> >> + resp_sa_mad->method == IB_MAD_METHOD_GETMULTI_RESP) { >> >> +#else >> >> + if (resp_sa_mad->method == IB_MAD_METHOD_GETTABLE_RESP) { >> >> +#endif >> >> + /* Fill in the offset (paylen will be done by the rmpp SAR) >> >> */ >> >> + resp_sa_mad->attr_offset = num_rec ? >> >> ib_get_attr_offset(attr_size) : 0; >> >> + } >> > >> > What is wrong with current implementation? >> >> It's now needed for the debug version of OpenSM not to assert due to: >> >> 7fc6cd3037f07190e483a047f17d37b6bebbb2b3 >> Author: Smith, Stan <[email protected]> >> Date: Fri May 21 10:49:27 2010 -0700 >> >> ib_types.h add debug assert >> >> Add a debug assert to catch incorrect MAD attr offset size. >> This proved to be useful in catching incorrect struct sizes as MAD attrs >> nee >> d to be a multiple of 8 bytes. >> >> Signed-off-by: stan smith <[email protected]> >> Signed-off-by: Sasha Khapyorsky <[email protected]> >> >> as not all SA attributes are multiple of 8 bytes. > > Then it seems that such assertion is incorrect and the patch should be > reverted. Right?
I think the use of ib_get_attr_offset is being "overused". I'd rather see this patch used as it is the simplest way to address that rather than conditionalize ib_get_attr_offset on the SA attribute or would you rather see a patch along those lines ? -- Hal > > Sasha > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
