On 11/21/2018 8:50 PM, Honggang LI wrote: > On Wed, Nov 21, 2018 at 09:01:16AM -0500, Hal Rosenstock wrote: >> On 11/21/2018 1:58 AM, Honggang LI wrote: >>> From: Honggang Li <honli@xxxxxxxxxx> >>> >>> Signed-off-by: Honggang Li <honli@xxxxxxxxxx> >>> --- >>> opensm/osm_resp.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/opensm/osm_resp.c b/opensm/osm_resp.c >>> index 3f270e66..42015564 100644 >>> --- a/opensm/osm_resp.c >>> +++ b/opensm/osm_resp.c >>> @@ -85,8 +85,8 @@ static void resp_make_resp_smp(IN osm_sm_t * sm, IN const ib_smp_t * p_src_smp, >>> if (p_src_smp->mgmt_class == IB_MCLASS_SUBN_DIR) >>> p_dest_smp->status |= IB_SMP_DIRECTION; >>> >>> - p_dest_smp->dr_dlid = p_dest_smp->dr_slid; >>> - p_dest_smp->dr_slid = p_dest_smp->dr_dlid; >>> + p_dest_smp->dr_dlid = p_src_smp->dr_slid; >>> + p_dest_smp->dr_slid = p_src_smp->dr_dlid; >> >> Please elaborate on why you think that this change is needed. Is it just >> by code inspection where a number of dest SMP parameters are updated >> from their src equivalents ? > > yes, just by code inspection. > >> >> Earlier in this code block, the src SMP is copied to the dest SMP so >> that flipping the dest DR LIDs seems just fine to me and has worked for >> more than the last 10-15 years. > > Yes, flipping the dest DR LIDs works. But flipping the src DR LIDs does > not. > > Please let's take random LIDs for illustration. > > p_src_smp->dr_slid = 1 > p_src_smp->dr_dlid = 2 > > > > 60 static void resp_make_resp_smp(IN osm_sm_t * sm, IN const ib_smp_t * p_src_smp, > ..... > 70 > 71 *p_dest_smp = *p_src_smp; > 72 if (p_src_smp->method == IB_MAD_METHOD_GET || > > Line 71 will copy the dest SMP to src SMP, which means: > > p_dest_smp->dr_slid = p_src_smp->dr_slid = 1; > p_dest_smp->dr_dlid = p_src_smp->dr_dlid = 2; > > 88 p_dest_smp->dr_dlid = p_dest_smp->dr_slid; > > line 88 will set p_dest_smp->dr_dlid to 1 > > 89 p_dest_smp->dr_slid = p_dest_smp->dr_dlid; > > line 89 will set p_dest_smp->dr_slid to 1. > > Both p_dest_smp->dr_slid and p_dest_smp->dr_dlid will be set to same > value 1. > > Line 88 and 89 is really just like: > A = B > B = A > > The second line (B = A) is redundant, if you just want to flip the dest > DR LID. I see what you mean. In looking at this further, per IBA vol 1 C14-10.1.1, there is no need to swap DrDLID and DrSLID in the response. Patch to follow shortly. > >> >>> memcpy(&p_dest_smp->data, p_payload, IB_SMP_DATA_SIZE); >>> >>> Exit: >>> >
