Re: [PATCH opensm] osm_resp.c: No need to swap DR [D/S]LIDs in resp_make_resp_smp

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> 
> 
> 
> 
> 
> 



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux