On 11/26/2018 9:34 AM, Håkon Bugge wrote: > > >> On 26 Nov 2018, at 14:27, Hal Rosenstock <hal@xxxxxxxxxxxxxxxxxx> wrote: >> >> >> C14-10.1.1 states that the DrSLID and DrDLID in the directed route response >> SMP shall be copied as is from the request SMP. >> >> Pointed-out-by: Honggang Li <honli@xxxxxxxxxx> >> >> Signed-off-by: Hal Rosenstock <hal@xxxxxxxxxxxx> >> --- >> opensm/osm_resp.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/opensm/osm_resp.c b/opensm/osm_resp.c >> index 3f270e6..9a98df9 100644 >> --- a/opensm/osm_resp.c >> +++ b/opensm/osm_resp.c >> @@ -85,8 +85,6 @@ 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; >> memcpy(&p_dest_smp->data, p_payload, IB_SMP_DATA_SIZE); > > This patch is correct, wrt. the IBTA spec, AFAICT. > > However, this bug must have been present for ~1/8 century, and partially DR SMPs may be negative affected by this commit. Any thoughts on that? This routine is only invoked for SMInfo responding and sending TrapRepress. TrapRepress is only LID routed so Dr LIDs don't matter for this case. SMInfo responding can be either LID routed or DR. In the cases of SMInfo DR responding that I checked (2 instances in osm_req.c (osm_req_get and osm_prepare_req_set) where ib_smp_init_new is called and also using sminfo utility), the DrSLID = DrDLID (permissive LID) which is why the current code worked for more than the last 10-15 years. -- Hal > > Well, having raised the issue, then you have my: > > Reviewed-by: Håkon Bugge <haakon.bugge@xxxxxxxxxx> > > > Thxs, Håkon > > > > > >