[ofa-general] [PATCH 2/4] opensm/main.c rescan subnet configuration after SIGHUP

[Eli Dorfman (Voltaire)]

Sasha Khapyorsky wrote:
> On 11:28 Tue 03 Feb     , Eli Dorfman (Voltaire) wrote:
>> Sasha Khapyorsky wrote:
>>> On 16:32 Mon 26 Jan     , Eli Dorfman (Voltaire) wrote:
>>>>  rescan subnet configuration after SIGHUP
>>>>  call osm_subn_rescan_conf_files() after SIGHUP.
>>>>  this is important when priority is changed and SM is in standby.
>>>>  in that case it will not send capability mask trap and will not become master.
>>>>
>>>> Signed-off-by: Eli Dorfman <elid at voltaire.com>
>>>> ---
>>>>  opensm/opensm/main.c |    1 +
>>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/opensm/opensm/main.c b/opensm/opensm/main.c
>>>> index f786192..0f7b822 100644
>>>> --- a/opensm/opensm/main.c
>>>> +++ b/opensm/opensm/main.c
>>>> @@ -507,6 +507,7 @@ int osm_manager_loop(osm_subn_opt_t * p_opt, osm_opensm_t * p_osm)
>>>>  			osm_hup_flag = 0;
>>>>  			/* a HUP signal should only start a new heavy sweep */
>>>>  			p_osm->subn.force_heavy_sweep = TRUE;
>>>> +			osm_subn_rescan_conf_files(&p_osm->subn);
>>> Is it synchronized with sweep? If regular (scheduled by timer) sweep
>>> starts in a middle of osm_subn_rescan_conf_files() (when QoS parameters
>>> are freed..., etc.). I think it is not.
>>>
>> i assume it is not.
>> what about the the following (though it uses yet another flag...)
>>
>> diff --git a/opensm/include/opensm/osm_subnet.h b/opensm/include/opensm/osm_subnet.h
>> index 8863e47..88c977d 100644
>> --- a/opensm/include/opensm/osm_subnet.h
>> +++ b/opensm/include/opensm/osm_subnet.h
>> @@ -169,6 +169,7 @@ typedef struct osm_subn_opt {
>>  	uint32_t polling_retry_number;
>>  	uint32_t max_msg_fifo_timeout;
>>  	boolean_t force_heavy_sweep;
>> +	boolean_t rescan_conf_file;
>>  	uint8_t log_flags;
>>  	char *dump_files_dir;
>>  	char *log_file;
>> diff --git a/opensm/opensm/main.c b/opensm/opensm/main.c
>> index de38056..f2d7846 100644
>> --- a/opensm/opensm/main.c
>> +++ b/opensm/opensm/main.c
>> @@ -507,7 +507,7 @@ int osm_manager_loop(osm_subn_opt_t * p_opt, osm_opensm_t * p_osm)
>>  			osm_hup_flag = 0;
>>  			/* a HUP signal should only start a new heavy sweep */
>>  			p_osm->subn.force_heavy_sweep = TRUE;
>> -			osm_subn_rescan_conf_files(&p_osm->subn);
>> +			p_osm->subn.rescan_conf_file  = TRUE;
>>  			osm_opensm_sweep(p_osm);
>>  		}
>>  	}
>> diff --git a/opensm/opensm/osm_state_mgr.c b/opensm/opensm/osm_state_mgr.c
>> index fc7ceb9..87a5db9 100644
>> --- a/opensm/opensm/osm_state_mgr.c
>> +++ b/opensm/opensm/osm_state_mgr.c
>> @@ -1042,6 +1042,13 @@ static void do_sweep(osm_sm_t * sm)
>>  	ib_api_status_t status;
>>  	osm_remote_sm_t *p_remote_sm;
>>  
>> +	if (sm->p_subn->rescan_conf_file) {
>> +		if (osm_subn_rescan_conf_files(sm->p_subn) < 0)
>> +			OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 331A: "
>> +				"osm_subn_rescan_conf_file failed\n");
>> +		sm->p_subn->rescan_conf_file = FALSE;
>> +	}
>> +
> 
> What would be wrong with using exiting 'force_heavy_sweep' flag?
> 
'force_heavy_sweep' flag is set in other occasions as well

> Another issue with this patch - config file will be rescanned later
> again (during heavy sweep). It would be really nice to avoid such
> obviously unneeded double parsing.
>
that is correct, but we need a special flag to handle the priority change when SM
is in standby.
In that case a rescan at the beginning of do_sweep is a must, otherwise it will 
simply return without doing anything.
what was the reason of putting rescan not in the beginning of do_sweep().
If none then we can simply rescan as first step.

Eli

> Sasha
> 
>>  	if (sm->p_subn->sm_state != IB_SMINFO_STATE_MASTER &&
>>  	    sm->p_subn->sm_state != IB_SMINFO_STATE_DISCOVERING)
>>  		return;