[LEDE-DEV] [PATCH 1/1 V3] netifd: track when wdev setup fails

[Felix Fietkau]

On 2016-08-11 15:02, Eduardo Abinader wrote:
> On 11.08.2016 14:16, Felix Fietkau wrote:
>> On 2016-08-11 14:02, Eduardo Abinader wrote:
>>> When netifd failed to load a valid configuration, after an invalid one,
>>> it was not possible to setup the wireless device. This patch
>>> aims to track this situation and behave acordingly, by keeping
>>> track of failed setup without affecting autostart behavior. Also
>>> block the restart of the wdev, when not applied.
>>>
>>> Signed-off-by: Eduardo Abinader <eduardoabinader at gmail.com>
>>> ---
>>>  wireless.c | 26 ++++++++++++++++++++------
>>>  wireless.h |  1 +
>>>  2 files changed, 21 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/wireless.c b/wireless.c
>>> index 34dd328..1212a77 100644
>>> --- a/wireless.c
>>> +++ b/wireless.c
>>> @@ -287,7 +287,7 @@ __wireless_device_set_up(struct wireless_device *wdev)
>>>  	if (wdev->disabled)
>>>  		return;
>>>  
>>> -	if (wdev->state != IFS_DOWN || config_init)
>>> +	if ((wdev->config_state != IFC_RELOAD) && (wdev->state != IFS_DOWN || config_init))
>>>  		return;
>>>  
>>>  	free(wdev->prev_config);
>> This part looks like it should be dropped now.
>> 
> 
> Based on your next comment, right? I mean, replace by the proper
> placement of earlier return of __wireless_device_set_up on
> reload/autostart/retry_setup, ok?
Yes.

>>> @@ -681,6 +693,7 @@ wireless_device_create(struct wireless_driver *drv, const char *name, struct blo
>>>  	wdev->config_state = IFC_NORMAL;
>>>  	wdev->name = strcpy(name_buf, name);
>>>  	wdev->config = data;
>>> +	wdev->retry_setup_failed = false;
>>>  	wdev->config_autostart = true;
>>>  	wdev->autostart = wdev->config_autostart;
>>>  	INIT_LIST_HEAD(&wdev->script_proc);
>>> @@ -991,6 +1004,7 @@ wireless_start_pending(void)
>>>  	struct wireless_device *wdev;
>>>  
>>>  	vlist_for_each_element(&wireless_devices, wdev, node)
>>> -		if (wdev->autostart)
>>> +		if (wdev->autostart && wdev->state == IFS_DOWN)
>> Why did you make this change?
>> 
> 
> To avoid settinp up wdev while a teardown is still in progress, I
> noticed the previous hostapd instance was still running, so it seems
> more appropriate to defer it to wireless_device_check_script_tasks.
> Should it be a separate patch?
I think the issue you're trying to work around here is actually
introduced by that other change above that I suggested removing.
If that's the case, you can drop this one as well.

- Felix