| Message ID | 1401306667-24630-1-git-send-email-eduardo.abinader@openbossa.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, May 28, 2014 at 03:51:07PM -0400, Eduardo Abinader wrote: > Upon p2p_group_add failure, deinit_iface should not exclude > all p2p GO interfaces, but the uninitialized one. > > This issue occurs mainly for autonomous groups. > diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c > @@ -3777,7 +3777,7 @@ static void wpa_supplicant_deinit_iface(struct wpa_supplicant *wpa_s, > if (wpa_s == wpa_s->parent) > - wpas_p2p_group_remove(wpa_s, "*"); > + wpas_p2p_group_remove(wpa_s, wpa_s->ifname); Hmm.. This looks a bit odd. That wpa_s == wpa_s->parent condition would indicate that there is no separate group interface in use and as such, there cannot be multiple P2P group instances if this comes from p2p_group_add failure. As such, removing all groups should remove just the failing group in case this comes from p2p_group_add failure. This wildcard is used here on purpose (see commit e83e15ee771988c57b73cb4badf728aaf0943291) and it is needed to handle removal of the parent interface in other cases where there are multiple P2P group interfaces. Could you please share a debug log showing issues with this for p2p_group_add failure?
Hi Jouni, On Sat, May 31, 2014 at 5:56 AM, Jouni Malinen <j@w1.fi> wrote: > On Wed, May 28, 2014 at 03:51:07PM -0400, Eduardo Abinader wrote: >> Upon p2p_group_add failure, deinit_iface should not exclude >> all p2p GO interfaces, but the uninitialized one. >> >> This issue occurs mainly for autonomous groups. > >> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c >> @@ -3777,7 +3777,7 @@ static void wpa_supplicant_deinit_iface(struct wpa_supplicant *wpa_s, >> if (wpa_s == wpa_s->parent) >> - wpas_p2p_group_remove(wpa_s, "*"); >> + wpas_p2p_group_remove(wpa_s, wpa_s->ifname); > > Hmm.. This looks a bit odd. That wpa_s == wpa_s->parent condition would > indicate that there is no separate group interface in use and as such, > there cannot be multiple P2P group instances if this comes from > p2p_group_add failure. As such, removing all groups should remove just > the failing group in case this comes from p2p_group_add failure. > > This wildcard is used here on purpose (see commit > e83e15ee771988c57b73cb4badf728aaf0943291) and it is needed to handle > removal of the parent interface in other cases where there are multiple > P2P group interfaces. > > Could you please share a debug log showing issues with this for > p2p_group_add failure? Here the logs for p2p_group_add failure: http://pastebin.com/U28azbKu I have called p2p_group_add 4 times: 1) p2p_group_add --> Adds a p2p_go intf 2) p2p_group_add --> Remove all (even the previous one) 3) p2p_group_add --> Add a p2p_go intf 4) p2p_group_add --> Remove all (even the previous one) Thanks. > > -- > Jouni Malinen PGP id EFC895FA
On Wed, Jun 04, 2014 at 08:03:58AM -0400, Eduardo Abinader wrote: > On Sat, May 31, 2014 at 5:56 AM, Jouni Malinen <j@w1.fi> wrote: > > On Wed, May 28, 2014 at 03:51:07PM -0400, Eduardo Abinader wrote: > >> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c > >> @@ -3777,7 +3777,7 @@ static void wpa_supplicant_deinit_iface(struct wpa_supplicant *wpa_s, > >> if (wpa_s == wpa_s->parent) > >> - wpas_p2p_group_remove(wpa_s, "*"); > >> + wpas_p2p_group_remove(wpa_s, wpa_s->ifname); > > > > Hmm.. This looks a bit odd. That wpa_s == wpa_s->parent condition would > > indicate that there is no separate group interface in use and as such, > > there cannot be multiple P2P group instances if this comes from > > p2p_group_add failure. As such, removing all groups should remove just > > the failing group in case this comes from p2p_group_add failure. The issue here was in wpa_s->parent not getting yet assigned on the init error path. > > Could you please share a debug log showing issues with this for > > p2p_group_add failure? > > Here the logs for p2p_group_add failure: http://pastebin.com/U28azbKu > > I have called p2p_group_add 4 times: > 1) p2p_group_add --> Adds a p2p_go intf > 2) p2p_group_add --> Remove all (even the previous one) I reproduced this and fixed this by changing wpa_s initialization path to assign the wpa_s->parent pointer as part of the allocation so that the wpa_s->parent comparison in deinit_iface works properly.
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c index 0b871d0..aeac333 100644 --- a/wpa_supplicant/wpa_supplicant.c +++ b/wpa_supplicant/wpa_supplicant.c @@ -3777,7 +3777,7 @@ static void wpa_supplicant_deinit_iface(struct wpa_supplicant *wpa_s, #ifdef CONFIG_P2P if (wpa_s == wpa_s->parent) - wpas_p2p_group_remove(wpa_s, "*"); + wpas_p2p_group_remove(wpa_s, wpa_s->ifname); if (wpa_s == wpa_s->global->p2p_init_wpa_s && wpa_s->global->p2p) { wpa_dbg(wpa_s, MSG_DEBUG, "P2P: Disable P2P since removing " "the management interface is being removed");
Upon p2p_group_add failure, deinit_iface should not exclude all p2p GO interfaces, but the uninitialized one. This issue occurs mainly for autonomous groups. Signed-off-by: Eduardo Abinader <eduardo.abinader@openbossa.org> --- wpa_supplicant/wpa_supplicant.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)