| Message ID | 1375870125-27991-5-git-send-email-ilan.peer@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Aug 07, 2013 at 01:08:41PM +0300, Ilan Peer wrote: > Disconnect a P2P GO interface when the interface is being disabled. Could you please describe what the use case is for this? The only case where EVENT_INTERFACE_DISABLED would show up during GO operation would be when something external sets the group interface down. Why would that terminate the group? Why is that something external not terminating the group through existing means? > diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c > @@ -2566,6 +2566,9 @@ void wpa_supplicant_event(void *ctx, enum wpa_event_type event, > { > struct wpa_supplicant *wpa_s = ctx; > > + if (!wpa_s) > + return; > + > if (wpa_s->wpa_state == WPA_INTERFACE_DISABLED && This looks completely independent change and something that should not be needed in the first place. Where is wpa_supplicant_event() called with ctx == NULL? > @@ -3030,6 +3033,13 @@ void wpa_supplicant_event(void *ctx, enum wpa_event_type event, > break; > case EVENT_INTERFACE_DISABLED: > wpa_dbg(wpa_s, MSG_DEBUG, "Interface was disabled"); > +#ifdef CONFIG_P2P > + if (wpa_s->p2p_group_interface == > + P2P_GROUP_INTERFACE_GO) { > + wpas_p2p_disconnect(wpa_s); > + break; > + } > +#endif /* CONFIG_P2P */ This would cover only the case of a separate P2P group interface. Shouldn't the use-wlan0-netdev-for-group case behave consistently?
> On Wed, Aug 07, 2013 at 01:08:41PM +0300, Ilan Peer wrote: > > Disconnect a P2P GO interface when the interface is being disabled. > > Could you please describe what the use case is for this? The only case where > EVENT_INTERFACE_DISABLED would show up during GO operation would be > when something external sets the group interface down. Why would that > terminate the group? Why is that something external not terminating the group > through existing means? > The use case was triggering rfkill (both SW and HW). This case popped up as part of a testing cycle, where after a toggle in the rfkill state, the result was that the interface was not deleted, but on the other hand the wpa_supplicant did not configure the kernel to re-start the ap functionality again. To fix this, we could have either fixed the flow to reconfigured the kernel to re-start the AP functionality again, or to entirely remove the interface. Assuming that that a P2P group is usually setup for specific use case, i.e., file transfer or WFD, once a P2P group interface is externally disabled, it did not make sense to keep the interface, but rely on the external user to setup it up again in case it was needed. Hope that his make sense. > > diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c @@ > > -2566,6 +2566,9 @@ void wpa_supplicant_event(void *ctx, enum > > wpa_event_type event, { > > struct wpa_supplicant *wpa_s = ctx; > > > > + if (!wpa_s) > > + return; > > > > if (wpa_s->wpa_state == WPA_INTERFACE_DISABLED && > > This looks completely independent change and something that should not be > needed in the first place. Where is wpa_supplicant_event() called with ctx == > NULL? Sorry but I cannot recall why I added this, and cannot reproduce a crash without it. Will remove it. > > > @@ -3030,6 +3033,13 @@ void wpa_supplicant_event(void *ctx, enum > wpa_event_type event, > > break; > > case EVENT_INTERFACE_DISABLED: > > wpa_dbg(wpa_s, MSG_DEBUG, "Interface was disabled"); > > +#ifdef CONFIG_P2P > > + if (wpa_s->p2p_group_interface == > > + P2P_GROUP_INTERFACE_GO) { > > + wpas_p2p_disconnect(wpa_s); > > + break; > > + } > > +#endif /* CONFIG_P2P */ > > This would cover only the case of a separate P2P group interface. > Shouldn't the use-wlan0-netdev-for-group case behave consistently? > Missed this one. Does deleting the network block in case that it is marked as temporary is a sufficient solution? Regards, Ilan.
On Sun, Sep 01, 2013 at 07:38:45PM +0000, Peer, Ilan wrote: > The use case was triggering rfkill (both SW and HW). This case popped up as part of a testing cycle, where after a toggle in the rfkill state, the result was that the interface was not deleted, but on the other hand the wpa_supplicant did not configure the kernel to re-start the ap functionality again. To fix this, we could have either fixed the flow to reconfigured the kernel to re-start the AP functionality again, or to entirely remove the interface. Assuming that that a P2P group is usually setup for specific use case, i.e., file transfer or WFD, once a P2P group interface is externally disabled, it did not make sense to keep the interface, but rely on the external user to setup it up again in case it was needed. Hope that his make sense. OK, that makes sense. > > > case EVENT_INTERFACE_DISABLED: > > > wpa_dbg(wpa_s, MSG_DEBUG, "Interface was disabled"); > > > +#ifdef CONFIG_P2P > > > + if (wpa_s->p2p_group_interface == > > > + P2P_GROUP_INTERFACE_GO) { > > > + wpas_p2p_disconnect(wpa_s); > > > + break; > > > + } > > > +#endif /* CONFIG_P2P */ > > > > This would cover only the case of a separate P2P group interface. > > Shouldn't the use-wlan0-netdev-for-group case behave consistently? > > Missed this one. Does deleting the network block in case that it is marked as temporary is a sufficient solution? No, both of these cases need to do same, i.e., call wpas_p2p_disconnect() (which will result in removing that network block, but also indicating the expected events about group removal). I'll fix this when committing the patch.
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c index bcfac21..1e804e3 100644 --- a/wpa_supplicant/events.c +++ b/wpa_supplicant/events.c @@ -2566,6 +2566,9 @@ void wpa_supplicant_event(void *ctx, enum wpa_event_type event, { struct wpa_supplicant *wpa_s = ctx; + if (!wpa_s) + return; + if (wpa_s->wpa_state == WPA_INTERFACE_DISABLED && event != EVENT_INTERFACE_ENABLED && event != EVENT_INTERFACE_STATUS && @@ -3030,6 +3033,13 @@ void wpa_supplicant_event(void *ctx, enum wpa_event_type event, break; case EVENT_INTERFACE_DISABLED: wpa_dbg(wpa_s, MSG_DEBUG, "Interface was disabled"); +#ifdef CONFIG_P2P + if (wpa_s->p2p_group_interface == + P2P_GROUP_INTERFACE_GO) { + wpas_p2p_disconnect(wpa_s); + break; + } +#endif /* CONFIG_P2P */ wpa_supplicant_mark_disassoc(wpa_s); wpa_supplicant_set_state(wpa_s, WPA_INTERFACE_DISABLED); break;