diff mbox

Find correct driver for interface additions/removals

Message ID 20160306202721.GA16380@w1.fi
State Superseded
Headers show

Commit Message

Jouni Malinen March 6, 2016, 8:27 p.m. UTC 
On Wed, Feb 10, 2016 at 05:57:27PM +0000, Roy Marples wrote:

>     Interface additions/removals are not guaranteed to be for the
>     driver listening to the kernel events.
>     As such, send the events to wpa_supplicant_event_global()
>     which can then pick the correct interface registered with
>     wpa_supplicant to send the event to.

This seems to break one of the hwsim test cases (autogo_ifdown) since
the interface removal event is now handled before the interface disabled
event. I tried to fix that with the changes below, but that did not
allow the test case to pass since there was an extra interface removal
for the previous instance of the netdev being processed 0.7 seconds
later when the new netdev with the same ifname was in place. This new
event used the old ifindex, but it was still delivered and processed for
the new netdev, thus terminating the P2P group.

I'm not sure what to about this. I cannot apply this patch without that
test case being fixed first. This looks like a real issue, so modifying
the test case does not look appropriate either. Something might need to
filter out delayed RTM_DELLINK events to avoid something like this..


Subject: [PATCH] P2P: Remove group on interface status update

Previously, this was done only on the interface getting disabled event,
but with the changes to the way the interface added/removed are
processed, this status update may arrive first and that would prevent
the interface disabled event from being used.

Signed-off-by: Jouni Malinen <j@w1.fi>
---
 wpa_supplicant/events.c         | 4 ++++
 wpa_supplicant/p2p_supplicant.c | 6 +++---
 wpa_supplicant/p2p_supplicant.h | 2 +-
 3 files changed, 8 insertions(+), 4 deletions(-)

Comments

Roy Marples March 13, 2016, 5:31 p.m. UTC | #1 
On Sunday 06 March 2016 22:27:21 Jouni Malinen wrote:
> On Wed, Feb 10, 2016 at 05:57:27PM +0000, Roy Marples wrote:
> >     Interface additions/removals are not guaranteed to be for the
> >     driver listening to the kernel events.
> >     As such, send the events to wpa_supplicant_event_global()
> >     which can then pick the correct interface registered with
> >     wpa_supplicant to send the event to.
> 
> This seems to break one of the hwsim test cases (autogo_ifdown) since
> the interface removal event is now handled before the interface disabled
> event. I tried to fix that with the changes below, but that did not
> allow the test case to pass since there was an extra interface removal
> for the previous instance of the netdev being processed 0.7 seconds
> later when the new netdev with the same ifname was in place. This new
> event used the old ifindex, but it was still delivered and processed for
> the new netdev, thus terminating the P2P group.

That should not be the case.
I have re-configured my Linux system so that the majority of the tests pass, 
however it takes an awful long time to run them all.
Is there a way to run a specific test by itself relatively easily so I can work 
on this more easily?

> I'm not sure what to about this. I cannot apply this patch without that
> test case being fixed first. This looks like a real issue, so modifying
> the test case does not look appropriate either. Something might need to
> filter out delayed RTM_DELLINK events to avoid something like this..

Fair enough.

Roy
Jouni Malinen March 13, 2016, 7:09 p.m. UTC | #2 
On Sun, Mar 13, 2016 at 05:31:39PM +0000, Roy Marples wrote:
> On Sunday 06 March 2016 22:27:21 Jouni Malinen wrote:
> > This seems to break one of the hwsim test cases (autogo_ifdown) since

> That should not be the case.
> I have re-configured my Linux system so that the majority of the tests pass, 
> however it takes an awful long time to run them all.
> Is there a way to run a specific test by itself relatively easily so I can work 
> on this more easily?

If you are using the VM option:

./vm-run.sh autogo_ifdown

and on the host without VM:

sudo ./run-tests.py autogo_ifdown
diff mbox

Patch

diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 7d792c2..4f9a40e 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -2736,6 +2736,10 @@  wpa_supplicant_event_interface_status(struct wpa_supplicant *wpa_s,
 		break;
 	case EVENT_INTERFACE_REMOVED:
 		wpa_dbg(wpa_s, MSG_DEBUG, "Configured interface was removed");
+#ifdef CONFIG_P2P
+		if (wpas_p2p_interface_unavailable(wpa_s) > 0)
+			break;
+#endif /* CONFIG_P2P */
 		wpa_s->interface_removed = 1;
 		wpa_supplicant_mark_disassoc(wpa_s);
 		wpa_supplicant_set_state(wpa_s, WPA_INTERFACE_DISABLED);
diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
index 5ff758f..ce3cfb5 100644
--- a/wpa_supplicant/p2p_supplicant.c
+++ b/wpa_supplicant/p2p_supplicant.c
@@ -7581,14 +7581,14 @@  int wpas_p2p_cancel(struct wpa_supplicant *wpa_s)
 }
 
 
-void wpas_p2p_interface_unavailable(struct wpa_supplicant *wpa_s)
+int wpas_p2p_interface_unavailable(struct wpa_supplicant *wpa_s)
 {
 	if (wpa_s->current_ssid == NULL || !wpa_s->current_ssid->p2p_group)
-		return;
+		return -1;
 
 	wpa_printf(MSG_DEBUG, "P2P: Remove group due to driver resource not "
 		   "being available anymore");
-	wpas_p2p_group_delete(wpa_s, P2P_GROUP_REMOVAL_UNAVAILABLE);
+	return wpas_p2p_group_delete(wpa_s, P2P_GROUP_REMOVAL_UNAVAILABLE);
 }
 
 
diff --git a/wpa_supplicant/p2p_supplicant.h b/wpa_supplicant/p2p_supplicant.h
index 6a770d2..f968494 100644
--- a/wpa_supplicant/p2p_supplicant.h
+++ b/wpa_supplicant/p2p_supplicant.h
@@ -191,7 +191,7 @@  void wpas_p2p_remain_on_channel_cb(struct wpa_supplicant *wpa_s,
 				   unsigned int freq, unsigned int duration);
 void wpas_p2p_cancel_remain_on_channel_cb(struct wpa_supplicant *wpa_s,
 					  unsigned int freq);
-void wpas_p2p_interface_unavailable(struct wpa_supplicant *wpa_s);
+int wpas_p2p_interface_unavailable(struct wpa_supplicant *wpa_s);
 void wpas_p2p_notif_connected(struct wpa_supplicant *wpa_s);
 void wpas_p2p_notif_disconnected(struct wpa_supplicant *wpa_s);
 int wpas_p2p_notif_pbc_overlap(struct wpa_supplicant *wpa_s);