| Message ID | 1470928926-4560-1-git-send-email-eduardoabinader@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 2016-08-11 17:22, 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@gmail.com> > --- > wireless.c | 19 ++++++++++++++++--- > wireless.h | 1 + > 2 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/wireless.c b/wireless.c > index 34dd328..67c87f6 100644 > --- a/wireless.c > +++ b/wireless.c > @@ -388,6 +394,10 @@ wireless_device_mark_up(struct wireless_device *wdev) > > D(WIRELESS, "Wireless device '%s' is now up\n", wdev->name); > wdev->state = IFS_UP; > + > + if (wdev->retry_setup_failed) > + wdev->retry_setup_failed = false; > + > vlist_for_each_element(&wdev->interfaces, vif, node) > wireless_interface_handle_link(vif, true); > } wdev->retry_setup_failed = false needs to be moved to wdev_change_config(). You can also make it unconditional, testing if it's true before clearing it is unnecessary. > @@ -681,6 +692,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); > @@ -993,4 +1005,5 @@ wireless_start_pending(void) > vlist_for_each_element(&wireless_devices, wdev, node) > if (wdev->autostart) > __wireless_device_set_up(wdev); > + > } Please remove this empty line. You can also get rid of the wdev->autostart check here, since you moved it to __wireless_device_set_up. - Felix
On Thu, Aug 11, 2016 at 7:27 PM, Felix Fietkau <nbd@nbd.name> wrote: > On 2016-08-11 17:22, 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@gmail.com> >> --- >> wireless.c | 19 ++++++++++++++++--- >> wireless.h | 1 + >> 2 files changed, 17 insertions(+), 3 deletions(-) >> >> diff --git a/wireless.c b/wireless.c >> index 34dd328..67c87f6 100644 >> --- a/wireless.c >> +++ b/wireless.c >> @@ -388,6 +394,10 @@ wireless_device_mark_up(struct wireless_device *wdev) >> >> D(WIRELESS, "Wireless device '%s' is now up\n", wdev->name); >> wdev->state = IFS_UP; >> + >> + if (wdev->retry_setup_failed) >> + wdev->retry_setup_failed = false; >> + >> vlist_for_each_element(&wdev->interfaces, vif, node) >> wireless_interface_handle_link(vif, true); >> } > wdev->retry_setup_failed = false needs to be moved to > wdev_change_config(). You can also make it unconditional, testing if > it's true before clearing it is unnecessary. > it is already, but I am taking this chunk out. Thanks. >> @@ -681,6 +692,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); >> @@ -993,4 +1005,5 @@ wireless_start_pending(void) >> vlist_for_each_element(&wireless_devices, wdev, node) >> if (wdev->autostart) >> __wireless_device_set_up(wdev); >> + >> } > Please remove this empty line. You can also get rid of the > wdev->autostart check here, since you moved it to __wireless_device_set_up. > > - Felix
diff --git a/wireless.c b/wireless.c index 34dd328..67c87f6 100644 --- a/wireless.c +++ b/wireless.c @@ -287,6 +287,12 @@ __wireless_device_set_up(struct wireless_device *wdev) if (wdev->disabled) return; + if (wdev->retry_setup_failed) + return; + + if (!wdev->autostart) + return; + if (wdev->state != IFS_DOWN || config_init) return; @@ -314,9 +320,9 @@ wdev_handle_config_change(struct wireless_device *wdev) switch(state) { case IFC_NORMAL: case IFC_RELOAD: + __wireless_device_set_up(wdev); + wdev->config_state = IFC_NORMAL; - if (wdev->autostart) - __wireless_device_set_up(wdev); break; case IFC_REMOVE: wireless_device_free(wdev); @@ -388,6 +394,10 @@ wireless_device_mark_up(struct wireless_device *wdev) D(WIRELESS, "Wireless device '%s' is now up\n", wdev->name); wdev->state = IFS_UP; + + if (wdev->retry_setup_failed) + wdev->retry_setup_failed = false; + vlist_for_each_element(&wdev->interfaces, vif, node) wireless_interface_handle_link(vif, true); } @@ -399,7 +409,7 @@ wireless_device_retry_setup(struct wireless_device *wdev) return; if (--wdev->retry < 0) - wdev->autostart = false; + wdev->retry_setup_failed = true; __wireless_device_set_down(wdev); } @@ -467,6 +477,7 @@ wdev_change_config(struct wireless_device *wdev, struct wireless_device *wd_new) free(wdev->config); wdev->config = blob_memdup(new_config); wdev->disabled = disabled; + wdev->retry_setup_failed = false; wdev_set_config_state(wdev, IFC_RELOAD); } @@ -681,6 +692,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); @@ -993,4 +1005,5 @@ wireless_start_pending(void) vlist_for_each_element(&wireless_devices, wdev, node) if (wdev->autostart) __wireless_device_set_up(wdev); + } diff --git a/wireless.h b/wireless.h index 665cdb7..403cc86 100644 --- a/wireless.h +++ b/wireless.h @@ -56,6 +56,7 @@ struct wireless_device { bool config_autostart; bool autostart; bool disabled; + bool retry_setup_failed; enum interface_state state; enum interface_config_state config_state;
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@gmail.com> --- wireless.c | 19 ++++++++++++++++--- wireless.h | 1 + 2 files changed, 17 insertions(+), 3 deletions(-)