diff mbox series

[3/5] OCE: enable only if the iface neither P2P group nor AP iface

Message ID 1535305504-7763-3-git-send-email-andrei.otcheretianski@intel.com
State Changes Requested
Headers show
Series None | expand

Commit Message

Andrei Otcheretianski Aug. 26, 2018, 5:45 p.m. UTC 
From: Roee Zamir <roee.zamir@intel.com>

OCE shouldn't be enabled in these cases.
Check these before enabling OCE.

Signed-off-by: Roee Zamir <roee.zamir@intel.com>
---
 wpa_supplicant/wpa_supplicant.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Jouni Malinen Sept. 2, 2018, 3:04 p.m. UTC | #1 
On Sun, Aug 26, 2018 at 08:45:02PM +0300, Andrei Otcheretianski wrote:
> OCE shouldn't be enabled in these cases.

Why?

> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
> @@ -5670,7 +5670,14 @@ static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
>  	hs20_init(wpa_s);
>  #endif /* CONFIG_HS20 */
>  #ifdef CONFIG_MBO
> -	if (wpa_s->conf->oce) {
> +	if (wpa_s->conf->oce
> +#ifdef CONFIG_P2P
> +	&& wpa_s->p2p_group_interface == NOT_P2P_GROUP_INTERFACE
> +#endif /* CONFIG_P2P */
> +#ifdef CONFIG_AP
> +	&& !wpa_s->ap_iface
> +#endif
> +	) {
>  		if ((wpa_s->conf->oce & OCE_STA) &&
>  		    (wpa_s->drv_flags & WPA_DRIVER_FLAGS_OCE_STA))
>  			wpa_s->enable_oce = OCE_STA;

Please note that this is followed by code checking for OCE_STA_CFON and
that STA-CFON case would include "mobile AP" and P2P GO support.. Sure,
that is not yet supported, but this change in the implementation would
make the source look pretty odd..
Andrei Otcheretianski Sept. 3, 2018, 7:19 a.m. UTC | #2 
> > OCE shouldn't be enabled in these cases.
> 
> Why?

In the next patch ("OCE: send scan params") we enable OCE scan if OCE_STA flag in wpa_s->enable_oce is set.
This would enable some scan optimizations that may harm P2P connection (like probe request deferral / suppression)
The same applies for AP case - for OBSS scan. Anyway, it doesn't make sense to set OCE_STA flag on AP interface, no?

> 
> Please note that this is followed by code checking for OCE_STA_CFON and that
> STA-CFON case would include "mobile AP" and P2P GO support.. Sure, that is not
> yet supported, but this change in the implementation would make the source
> look pretty odd..

Yeah, you're right.. I didn't notice that.
This change is relevant only for the OCE_STA case in the next "if".
I'll submit a fixed version.

Andrei

> 
> --
> Jouni Malinen                                            PGP id EFC895FA
Andrei Otcheretianski Sept. 4, 2018, 10:18 a.m. UTC | #3 
>[...]
> I'll submit a fixed version.

After rethinking it - this patch is indeed incorrect.
Please drop this one.

Andrei
diff mbox series

Patch

diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 8bd2727..b95dada 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -5670,7 +5670,14 @@  static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
 	hs20_init(wpa_s);
 #endif /* CONFIG_HS20 */
 #ifdef CONFIG_MBO
-	if (wpa_s->conf->oce) {
+	if (wpa_s->conf->oce
+#ifdef CONFIG_P2P
+	&& wpa_s->p2p_group_interface == NOT_P2P_GROUP_INTERFACE
+#endif /* CONFIG_P2P */
+#ifdef CONFIG_AP
+	&& !wpa_s->ap_iface
+#endif
+	) {
 		if ((wpa_s->conf->oce & OCE_STA) &&
 		    (wpa_s->drv_flags & WPA_DRIVER_FLAGS_OCE_STA))
 			wpa_s->enable_oce = OCE_STA;