Message ID | CAFED-jkMKsLY6ECMdr-8zqxgrL8QcquBny26jOsQCh5tXek23A@mail.gmail.com |
---|---|
State | Not Applicable |
Headers | show |
On Sat, Apr 02, 2016 at 07:20:45PM +0200, Janusz Dziedzic wrote: > This is test with hwsim: > diff --git a/tests/hwsim/start.sh b/tests/hwsim/start.sh > @@ -100,7 +100,7 @@ else > -test -f /proc/modules && sudo modprobe mac80211_hwsim radios=7 > channels=$NUM_CH support_p2p_device=0 > +test -f /proc/modules && sudo modprobe mac80211_hwsim radios=7 > channels=$NUM_CH support_p2p_device=1 > janusz@dell:~/work/hostap/tests/hwsim$ sudo ./run-tests.py ibss_5ghz > START ibss_5ghz 1/1 > FAIL ibss_5ghz 5.19917 2016-04-02 19:13:49.208650 > > And error: No regdom change event I cannot reproduce this with such a change in start.sh. Are you saying this fails for you every time with "No regdom change event"?
On 24 April 2016 at 10:42, Jouni Malinen <j@w1.fi> wrote: > On Sat, Apr 02, 2016 at 07:20:45PM +0200, Janusz Dziedzic wrote: >> This is test with hwsim: > >> diff --git a/tests/hwsim/start.sh b/tests/hwsim/start.sh >> @@ -100,7 +100,7 @@ else >> -test -f /proc/modules && sudo modprobe mac80211_hwsim radios=7 >> channels=$NUM_CH support_p2p_device=0 >> +test -f /proc/modules && sudo modprobe mac80211_hwsim radios=7 >> channels=$NUM_CH support_p2p_device=1 > >> janusz@dell:~/work/hostap/tests/hwsim$ sudo ./run-tests.py ibss_5ghz >> START ibss_5ghz 1/1 >> FAIL ibss_5ghz 5.19917 2016-04-02 19:13:49.208650 >> >> And error: No regdom change event > > I cannot reproduce this with such a change in start.sh. Are you saying > this fails for you every time with "No regdom change event"? > Yes, exactly. In ibss_5ghz.log I see we only get this event for p2p-dev-wlan1/p2p-dev-wlan0. Kernel I am using: 4.5.0-rc6-ath+ BR Janusz > -- > Jouni Malinen PGP id EFC895FA
> -----Original Message----- > From: Hostap [mailto:hostap-bounces@lists.infradead.org] On Behalf Of > Janusz Dziedzic > Sent: Monday, April 25, 2016 07:58 > To: Jouni Malinen > Cc: hostap@lists.infradead.org; Janusz Dziedzic > Subject: Re: [PATCH] wpa_supplicant: send event to all interfaces > > On 24 April 2016 at 10:42, Jouni Malinen <j@w1.fi> wrote: > > On Sat, Apr 02, 2016 at 07:20:45PM +0200, Janusz Dziedzic wrote: > >> This is test with hwsim: > > > >> diff --git a/tests/hwsim/start.sh b/tests/hwsim/start.sh @@ -100,7 > >> +100,7 @@ else -test -f /proc/modules && sudo modprobe > mac80211_hwsim > >> radios=7 channels=$NUM_CH support_p2p_device=0 > >> +test -f /proc/modules && sudo modprobe mac80211_hwsim radios=7 > >> channels=$NUM_CH support_p2p_device=1 > > > >> janusz@dell:~/work/hostap/tests/hwsim$ sudo ./run-tests.py ibss_5ghz > >> START ibss_5ghz 1/1 FAIL ibss_5ghz 5.19917 2016-04-02 > 19:13:49.208650 > >> > >> And error: No regdom change event > > > > I cannot reproduce this with such a change in start.sh. Are you saying > > this fails for you every time with "No regdom change event"? > > > Yes, exactly. > In ibss_5ghz.log I see we only get this event for p2p-dev-wlan1/p2p-dev- > wlan0. > I think that is the same issue that was addressed in http://lists.shmoo.com/pipermail/hostap/2015-June/033085.html The attached patch also handles this issue. Regards, Ilan.
On Mon, Apr 25, 2016 at 06:57:59AM +0200, Janusz Dziedzic wrote: > On 24 April 2016 at 10:42, Jouni Malinen <j@w1.fi> wrote: > > On Sat, Apr 02, 2016 at 07:20:45PM +0200, Janusz Dziedzic wrote: > >> This is test with hwsim: > > > >> diff --git a/tests/hwsim/start.sh b/tests/hwsim/start.sh > >> @@ -100,7 +100,7 @@ else > >> -test -f /proc/modules && sudo modprobe mac80211_hwsim radios=7 > >> channels=$NUM_CH support_p2p_device=0 > >> +test -f /proc/modules && sudo modprobe mac80211_hwsim radios=7 > >> channels=$NUM_CH support_p2p_device=1 Ah.. I was not looking at this closely enough. This start.sh change does not actually do anything for my main test setup, since I use a VM and a kernel without any modules.. > In ibss_5ghz.log I see we only get this event for p2p-dev-wlan1/p2p-dev-wlan0. I can see this now with vm/vm-run.sh handling the support_p2p_device modification. Now that I can reproduce this issue, lets try to figure out what's the best way of addressing this. Your patch is doing quite a bit more than just modifying the specfici CTRL-EVENT-REGDOM-CHANGE event. Was there a reason for doing that or is that all just trying to make these test cases pass? Would the simpler patch that Ilan sent today work for the cases where you can see this issue with remote hosts? It is simply finding the highest level parent to get the IFNAME=wlan0 style prefix for the event regardless of whether the P2P Device interface is used.
On 25 April 2016 at 10:55, Jouni Malinen <j@w1.fi> wrote: > On Mon, Apr 25, 2016 at 06:57:59AM +0200, Janusz Dziedzic wrote: >> On 24 April 2016 at 10:42, Jouni Malinen <j@w1.fi> wrote: >> > On Sat, Apr 02, 2016 at 07:20:45PM +0200, Janusz Dziedzic wrote: >> >> This is test with hwsim: >> > >> >> diff --git a/tests/hwsim/start.sh b/tests/hwsim/start.sh >> >> @@ -100,7 +100,7 @@ else >> >> -test -f /proc/modules && sudo modprobe mac80211_hwsim radios=7 >> >> channels=$NUM_CH support_p2p_device=0 >> >> +test -f /proc/modules && sudo modprobe mac80211_hwsim radios=7 >> >> channels=$NUM_CH support_p2p_device=1 > > Ah.. I was not looking at this closely enough. This start.sh change does > not actually do anything for my main test setup, since I use a VM and a > kernel without any modules.. > >> In ibss_5ghz.log I see we only get this event for p2p-dev-wlan1/p2p-dev-wlan0. > > I can see this now with vm/vm-run.sh handling the support_p2p_device > modification. > > Now that I can reproduce this issue, lets try to figure out what's the > best way of addressing this. Your patch is doing quite a bit more than > just modifying the specfici CTRL-EVENT-REGDOM-CHANGE event. Was there a > reason for doing that or is that all just trying to make these test > cases pass? Would the simpler patch that Ilan sent today work for the > cases where you can see this issue with remote hosts? It is simply > finding the highest level parent to get the IFNAME=wlan0 style prefix > for the event regardless of whether the P2P Device interface is used. > I also remove for_each() from wpa_supplicant_update_channel_list() while this function will be called now for each interface. So, seems this for_each() was a workaround for a real issue (not sending this message to all ifaces). This is general question: how should we handle events when ifidx==-1 and wdev_id is not set. BR Janusz > -- > Jouni Malinen PGP id EFC895FA
On Mon, Apr 25, 2016 at 11:14:12AM +0200, Janusz Dziedzic wrote: > I also remove for_each() from wpa_supplicant_update_channel_list() > while this function will be called now for each interface. > So, seems this for_each() was a workaround for a real issue (not > sending this message to all ifaces). Please see commit 0f4bccdbbe642fc620d2b70f60c8ddc4ad09a5db ('Refactor channel list update event in wpa_supplicant'). That dl_list_for_each() in wpa_supplicant_update_channel_list() would seem to be needed to call wpas_p2p_update_channel_list() at the end of the updates. The changes you are proposing in this patch would seem to undo at least parts of that earlier fix. As such, I'm not really keen on applying this without much clearer justification on this fixing something. In other words, I'd much rather use the simpler patch to allow the test cases pass unless someone can go through all the possible channel list update sequences in more detail. > This is general question: > how should we handle events when ifidx==-1 and wdev_id is not set. Which events are these (other than the channel list change indication)?
On 28 April 2016 at 20:11, Jouni Malinen <j@w1.fi> wrote: > On Mon, Apr 25, 2016 at 11:14:12AM +0200, Janusz Dziedzic wrote: >> I also remove for_each() from wpa_supplicant_update_channel_list() >> while this function will be called now for each interface. >> So, seems this for_each() was a workaround for a real issue (not >> sending this message to all ifaces). > > Please see commit 0f4bccdbbe642fc620d2b70f60c8ddc4ad09a5db ('Refactor > channel list update event in wpa_supplicant'). That dl_list_for_each() > in wpa_supplicant_update_channel_list() would seem to be needed to call > wpas_p2p_update_channel_list() at the end of the updates. The changes > you are proposing in this patch would seem to undo at least parts of > that earlier fix. As such, I'm not really keen on applying this without > much clearer justification on this fixing something. In other words, I'd > much rather use the simpler patch to allow the test cases pass unless > someone can go through all the possible channel list update sequences in > more detail. > fixing something: In my opinion remove workarounds, applied before :) But I understand this could have "unknown" impact on other parts of code, so simple patch is also fine for me. BR Janusz >> This is general question: >> how should we handle events when ifidx==-1 and wdev_id is not set. > > Which events are these (other than the channel list change indication)? > > -- > Jouni Malinen PGP id EFC895FA
On Mon, Apr 25, 2016 at 06:35:05AM +0000, Peer, Ilan wrote: > I think that is the same issue that was addressed in http://lists.shmoo.com/pipermail/hostap/2015-June/033085.html > > The attached patch also handles this issue. Thanks, applied.
diff --git a/tests/hwsim/start.sh b/tests/hwsim/start.sh index a2885e4..c0d1366 100755 --- a/tests/hwsim/start.sh +++ b/tests/hwsim/start.sh @@ -100,7 +100,7 @@ else NUM_CH=1 fi -test -f /proc/modules && sudo modprobe mac80211_hwsim radios=7 channels=$NUM_CH support_p2p_device=0 +test -f /proc/modules && sudo modprobe mac80211_hwsim radios=7 channels=$NUM_CH support_p2p_device=1