[PATCH 1/7] mac80211: allow to get wireless_dev structure from ieee80211_vif

[PATCH 1/7] mac80211: allow to get wireless_dev structure from ieee80211_vif

[Emmanuel Grumbach]

This will allow mac80211 drivers to call cfg80211 APIs with
the right handle.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 include/net/mac80211.h | 13 +++++++++++++
 net/mac80211/util.c    | 12 ++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index a7756e4..157c0f1 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1282,6 +1282,19 @@ static inline bool ieee80211_vif_is_mesh(struct ieee80211_vif *vif)
 struct ieee80211_vif *wdev_to_ieee80211_vif(struct wireless_dev *wdev);
 
 /**
+ * ieee80211_vif_to_wdev - return a wdev struct from a vif
+ * @vif: the vif to get the wdev for
+ *
+ * This can be used by mac80211 drivers with direct cfg80211 APIs
+ * (like the vendor commands) that needs to get the wdev for a vif.
+ *
+ * Note that this function may return %NULL if the given wdev isn't
+ * associated with a vif that the driver knows about (e.g. monitor
+ * or AP_VLAN interfaces.)
+ */
+struct wireless_dev *ieee80211_vif_to_wdev(struct ieee80211_vif *vif);
+
+/**
  * enum ieee80211_key_flags - key flags
  *
  * These flags are used for communication about keys between the driver
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 37d85d3..e664b28 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -745,6 +745,18 @@ struct ieee80211_vif *wdev_to_ieee80211_vif(struct wireless_dev *wdev)
 }
 EXPORT_SYMBOL_GPL(wdev_to_ieee80211_vif);
 
+struct wireless_dev *ieee80211_vif_to_wdev(struct ieee80211_vif *vif)
+{
+	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+
+	if (!ieee80211_sdata_running(sdata) ||
+	    !(sdata->flags & IEEE80211_SDATA_IN_DRIVER))
+		return NULL;
+
+	return &sdata->wdev;
+}
+EXPORT_SYMBOL_GPL(ieee80211_vif_to_wdev);
+
 /*
  * Nothing should have been stuffed into the workqueue during
  * the suspend->resume cycle. Since we can't check each caller
-- 
1.9.1

[PATCH 2/7] mac80211: refactor drop connection/unlock in CSA processing

[Emmanuel Grumbach]

From: Johannes Berg <johannes.berg@intel.com>

The schedule_work()/mutex unlocking code is duplicated many times,
refactor that to a common place in the function.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/mlme.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 539d6a9..1999bc0 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1157,11 +1157,7 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 	if (!conf) {
 		sdata_info(sdata,
 			   "no channel context assigned to vif?, disconnecting\n");
-		ieee80211_queue_work(&local->hw,
-				     &ifmgd->csa_connection_drop_work);
-		mutex_unlock(&local->chanctx_mtx);
-		mutex_unlock(&local->mtx);
-		return;
+		goto drop_connection;
 	}
 
 	chanctx = container_of(conf, struct ieee80211_chanctx, conf);
@@ -1170,11 +1166,7 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 	    !(local->hw.flags & IEEE80211_HW_CHANCTX_STA_CSA)) {
 		sdata_info(sdata,
 			   "driver doesn't support chan-switch with channel contexts\n");
-		ieee80211_queue_work(&local->hw,
-				     &ifmgd->csa_connection_drop_work);
-		mutex_unlock(&local->chanctx_mtx);
-		mutex_unlock(&local->mtx);
-		return;
+		goto drop_connection;
 	}
 
 	ch_switch.timestamp = timestamp;
@@ -1186,11 +1178,7 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 	if (drv_pre_channel_switch(sdata, &ch_switch)) {
 		sdata_info(sdata,
 			   "preparing for channel switch failed, disconnecting\n");
-		ieee80211_queue_work(&local->hw,
-				     &ifmgd->csa_connection_drop_work);
-		mutex_unlock(&local->chanctx_mtx);
-		mutex_unlock(&local->mtx);
-		return;
+		goto drop_connection;
 	}
 
 	res = ieee80211_vif_reserve_chanctx(sdata, &csa_ie.chandef,
@@ -1199,11 +1187,7 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 		sdata_info(sdata,
 			   "failed to reserve channel context for channel switch, disconnecting (err=%d)\n",
 			   res);
-		ieee80211_queue_work(&local->hw,
-				     &ifmgd->csa_connection_drop_work);
-		mutex_unlock(&local->chanctx_mtx);
-		mutex_unlock(&local->mtx);
-		return;
+		goto drop_connection;
 	}
 	mutex_unlock(&local->chanctx_mtx);
 
@@ -1232,6 +1216,11 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 		mod_timer(&ifmgd->chswitch_timer,
 			  TU_TO_EXP_TIME((csa_ie.count - 1) *
 					 cbss->beacon_interval));
+	return;
+ drop_connection:
+	ieee80211_queue_work(&local->hw, &ifmgd->csa_connection_drop_work);
+	mutex_unlock(&local->chanctx_mtx);
+	mutex_unlock(&local->mtx);
 }
 
 static bool
-- 
1.9.1

[PATCH 3/7] mac80211: ignore CSA to same channel

[Emmanuel Grumbach]

From: Johannes Berg <johannes.berg@intel.com>

If the AP is confused and starts doing a CSA to the same channel,
just ignore that request instead of trying to act it out since it
was likely sent in error anyway.

In the case of the bug I was investigating the GO was misbehaving
and sending out a beacon with CSA IEs still included after having
actually done the channel switch.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/ieee80211_i.h |  1 +
 net/mac80211/mlme.c        | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 0266c57..994d1b3 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -453,6 +453,7 @@ struct ieee80211_if_managed {
 	unsigned int flags;
 
 	bool csa_waiting_bcn;
+	bool csa_ignored_same_chan;
 
 	bool beacon_crc_valid;
 	u32 beacon_crc;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 1999bc0..6205206 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1150,6 +1150,17 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 		return;
 	}
 
+	if (cfg80211_chandef_identical(&csa_ie.chandef,
+				       &sdata->vif.bss_conf.chandef)) {
+		if (ifmgd->csa_ignored_same_chan)
+			return;
+		sdata_info(sdata,
+			   "AP %pM tries to chanswitch to same channel, ignore\n",
+			   ifmgd->associated->bssid);
+		ifmgd->csa_ignored_same_chan = true;
+		return;
+	}
+
 	mutex_lock(&local->mtx);
 	mutex_lock(&local->chanctx_mtx);
 	conf = rcu_dereference_protected(sdata->vif.chanctx_conf,
@@ -1194,6 +1205,7 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 	sdata->vif.csa_active = true;
 	sdata->csa_chandef = csa_ie.chandef;
 	sdata->csa_block_tx = csa_ie.mode;
+	ifmgd->csa_ignored_same_chan = false;
 
 	if (sdata->csa_block_tx)
 		ieee80211_stop_vif_queues(local, sdata,
@@ -2076,6 +2088,7 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 
 	sdata->vif.csa_active = false;
 	ifmgd->csa_waiting_bcn = false;
+	ifmgd->csa_ignored_same_chan = false;
 	if (sdata->csa_block_tx) {
 		ieee80211_wake_vif_queues(local, sdata,
 					  IEEE80211_QUEUE_STOP_REASON_CSA);
-- 
1.9.1

[PATCH 4/7] nl80211: ignore HT/VHT capabilities without QoS/WMM

[Emmanuel Grumbach]

From: Johannes Berg <johannes.berg@intel.com>

As HT/VHT depend heavily on QoS/WMM, it's not a good idea to
let userspace add clients that have HT/VHT but not QoS/WMM.
Since it does so in certain cases we've observed (client is
using HT IEs but not QoS/WMM) just ignore the HT/VHT info at
this point and don't pass it down to the drivers which might
unconditionally use it.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/nl80211.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b020853..593513d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4401,6 +4401,16 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 	if (parse_station_flags(info, dev->ieee80211_ptr->iftype, &params))
 		return -EINVAL;
 
+	/* HT/VHT requires QoS, but if we don't have that just ignore HT/VHT
+	 * as userspace might just pass through the capabilities from the IEs
+	 * directly, rather than enforcing this restriction and returning an
+	 * error in this case.
+	 */
+	if (!(params.sta_flags_set & BIT(NL80211_STA_FLAG_WME))) {
+		params.ht_capa = NULL;
+		params.vht_capa = NULL;
+	}
+
 	/* When you run into this, adjust the code below for the new flag */
 	BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 7);
 
-- 
1.9.1

[PATCH 5/7] mac80211: Adjust chan_ctx when assigning reserved vif

[Emmanuel Grumbach]

From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>

It is possible to reserve a narrower channel context during csa (as long as
the chandefs are compatible). However when the reservation is used, the
channel context should be adjusted. Not doing so would result in attempts
to use higher rates than the chan_ctx supports.

Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
Reviewed-by: Luciano Coelho <luciano.coelho@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 net/mac80211/chan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index ff0d2db..aef28fd 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -1008,6 +1008,8 @@ ieee80211_vif_use_reserved_reassign(struct ieee80211_sub_if_data *sdata)
 	if (WARN_ON(!chandef))
 		return -EINVAL;
 
+	ieee80211_change_chanctx(local, new_ctx, chandef);
+
 	vif_chsw[0].vif = &sdata->vif;
 	vif_chsw[0].old_ctx = &old_ctx->conf;
 	vif_chsw[0].new_ctx = &new_ctx->conf;
@@ -1079,6 +1081,8 @@ ieee80211_vif_use_reserved_assign(struct ieee80211_sub_if_data *sdata)
 	if (WARN_ON(!chandef))
 		return -EINVAL;
 
+	ieee80211_change_chanctx(local, new_ctx, chandef);
+
 	list_del(&sdata->reserved_chanctx_list);
 	sdata->reserved_chanctx = NULL;
 
-- 
1.9.1

[PATCH 6/7] mac80211: ask for ECSA IE to be considered for beacon parse CRC

[Emmanuel Grumbach]

From: Johannes Berg <johannes.berg@intel.com>

When a beacon from the AP contains only the ECSA IE, and not a CSA IE
as well, this ECSA IE is not considered for calculating the CRC and
the beacon might be dropped as not being interesting. This is clearly
wrong, it should be handled and the channel switch should be executed.

Fix this by including the ECSA IE ID in the bitmap of interesting IEs.

Reported-by: Gil Tribush <gil.tribush@intel.com>
Reviewed-by: Luciano Coelho <luciano.coelho@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/mlme.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 6205206..ff03961 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -3207,7 +3207,8 @@ static const u64 care_about_ies =
 	(1ULL << WLAN_EID_CHANNEL_SWITCH) |
 	(1ULL << WLAN_EID_PWR_CONSTRAINT) |
 	(1ULL << WLAN_EID_HT_CAPABILITY) |
-	(1ULL << WLAN_EID_HT_OPERATION);
+	(1ULL << WLAN_EID_HT_OPERATION) |
+	(1ULL << WLAN_EID_EXT_CHANSWITCH_ANN);
 
 static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 				     struct ieee80211_mgmt *mgmt, size_t len,
-- 
1.9.1

[PATCH 7/7] mac80211: Count correctly interface types

[Emmanuel Grumbach]

From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>

Previously, interface combination check in mac80211 considered only
interfaces that have channel context. This wouldn't take P2P device interfaces
into account at all. Also for managed interfaces the channel context is bound
upon association and the combination check is performed when the iface is
brought up.
Fix this by counting the numbers of running interfaces instead of number of
interfaces that have channel context.

Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 net/mac80211/util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index e664b28..36d8cb2 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -3245,7 +3245,7 @@ int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 		wdev_iter = &sdata_iter->wdev;
 
 		if (sdata_iter == sdata ||
-		    rcu_access_pointer(sdata_iter->vif.chanctx_conf) == NULL ||
+		    !ieee80211_sdata_running(sdata_iter) ||
 		    local->hw.wiphy->software_iftypes & BIT(wdev_iter->iftype))
 			continue;
 
-- 
1.9.1

Re: [PATCH 5/7] mac80211: Adjust chan_ctx when assigning reserved vif

[Johannes Berg]

On Thu, 2015-03-12 at 08:53 +0200, Emmanuel Grumbach wrote:
> From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> 
> It is possible to reserve a narrower channel context during csa (as long as
> the chandefs are compatible). However when the reservation is used, the
> channel context should be adjusted. Not doing so would result in attempts
> to use higher rates than the chan_ctx supports.

The commit log for this patch doesn't make any sense. I tried to make
sense of it, even with Luca's help, but I can't - are you thinking about
the min_def, which isn't really "the channel context" but just an
auxiliary concept?

I've applied all the other patches, but I rewrote the commit log for 7/7
(where I could figure out what you really meant)

johannes

Re: [PATCH 7/7] mac80211: Count correctly interface types

[Janusz Dziedzic]

On 12 March 2015 at 07:53, Emmanuel Grumbach
<emmanuel.grumbach@intel.com> wrote:
>
> From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
>
> Previously, interface combination check in mac80211 considered only
> interfaces that have channel context. This wouldn't take P2P device interfaces
> into account at all. Also for managed interfaces the channel context is bound
> upon association and the combination check is performed when the iface is
> brought up.
> Fix this by counting the numbers of running interfaces instead of number of
> interfaces that have channel context.
>
> Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> ---
>  net/mac80211/util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index e664b28..36d8cb2 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -3245,7 +3245,7 @@ int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
>                 wdev_iter = &sdata_iter->wdev;
>
>                 if (sdata_iter == sdata ||
> -                   rcu_access_pointer(sdata_iter->vif.chanctx_conf) == NULL ||
> +                   !ieee80211_sdata_running(sdata_iter) ||
>                     local->hw.wiphy->software_iftypes & BIT(wdev_iter->iftype))
>                         continue;
>
Hello,

Seems this one break IBSS case when started from wpa_supplicant at
least for Intel7260 and ath10k where dedicated p2p_device is used.

This is wpa_supplicant log:
1427867673.149187: nl80211: Set mode ifindex 8 iftype 1 (ADHOC)
1427867673.149225: nl80211: Mode change succeeded while interface is down
1427867673.149234: Could not set interface wlan3 flags (UP): Device or
resource busy
1427867673.149237: nl80211: Failed to set interface up after switching mode
1427867673.149238: nl80211: Interface mode change to 1 from 1 failed
1427867673.149240: nl80211: Failed to set interface into IBSS mode
1427867673.149251: wlan3: Association request to the driver failed

BR
Janusz

Re: [PATCH 7/7] mac80211: Count correctly interface types

[Johannes Berg]

On Wed, 2015-04-01 at 08:06 +0200, Janusz Dziedzic wrote:

> Hello,
> 
> Seems this one break IBSS case when started from wpa_supplicant at
> least for Intel7260 and ath10k where dedicated p2p_device is used.
> 
> This is wpa_supplicant log:
> 1427867673.149187: nl80211: Set mode ifindex 8 iftype 1 (ADHOC)
> 1427867673.149225: nl80211: Mode change succeeded while interface is down
> 1427867673.149234: Could not set interface wlan3 flags (UP): Device or
> resource busy

That's actually the correct behaviour, given the interface limitations,
at least for iwlmvm devices:

iw list
[...]
	Supported interface modes:
		 * IBSS
		 * managed
		 * AP
		 * AP/VLAN
		 * monitor
		 * P2P-client
		 * P2P-GO
		 * P2P-device
[...]
	software interface modes (can always be added):
		 * AP/VLAN
		 * monitor
	valid interface combinations:
		 * #{ managed } <= 1, #{ AP, P2P-client, P2P-GO } <= 1, #{ P2P-device } <= 1,
		   total <= 3, #channels <= 2

So there's no combination with P2P-device and IBSS together.

In our device I'm pretty sure the two couldn't be supported together.
Perhaps the supplicant has issues though with how it creates the
P2P-Device? Should probably not be there if you have IBSS, it's pretty
useless anyway then. Or at least not be there if the driver doesn't
advertise support for that combination.

johannes

Re: [PATCH 7/7] mac80211: Count correctly interface types

[Arend van Spriel]

On 04/01/15 09:12, Johannes Berg wrote:
> On Wed, 2015-04-01 at 08:06 +0200, Janusz Dziedzic wrote:
>
>> Hello,
>>
>> Seems this one break IBSS case when started from wpa_supplicant at
>> least for Intel7260 and ath10k where dedicated p2p_device is used.
>>
>> This is wpa_supplicant log:
>> 1427867673.149187: nl80211: Set mode ifindex 8 iftype 1 (ADHOC)
>> 1427867673.149225: nl80211: Mode change succeeded while interface is down
>> 1427867673.149234: Could not set interface wlan3 flags (UP): Device or
>> resource busy
>
> That's actually the correct behaviour, given the interface limitations,
> at least for iwlmvm devices:
>
> iw list
> [...]
> 	Supported interface modes:
> 		 * IBSS
> 		 * managed
> 		 * AP
> 		 * AP/VLAN
> 		 * monitor
> 		 * P2P-client
> 		 * P2P-GO
> 		 * P2P-device
> [...]
> 	software interface modes (can always be added):
> 		 * AP/VLAN
> 		 * monitor
> 	valid interface combinations:
> 		 * #{ managed }<= 1, #{ AP, P2P-client, P2P-GO }<= 1, #{ P2P-device }<= 1,
> 		   total<= 3, #channels<= 2
>
> So there's no combination with P2P-device and IBSS together.
>
> In our device I'm pretty sure the two couldn't be supported together.
> Perhaps the supplicant has issues though with how it creates the
> P2P-Device? Should probably not be there if you have IBSS, it's pretty
> useless anyway then. Or at least not be there if the driver doesn't
> advertise support for that combination.

I think in latest wpa_s the P2P-Device is always created. There used to 
be a driver_param to create it. Not sure if that driver_param could 
still be used to avoid P2P-Device creation.

The interface combinations look confusing to me. Why is there no IBSS or 
monitor listed there?

Regards,
Arend

> johannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] mac80211: Count correctly interface types

[Johannes Berg]

On Wed, 2015-04-01 at 10:36 +0200, Arend van Spriel wrote:

> I think in latest wpa_s the P2P-Device is always created. There used to 
> be a driver_param to create it. Not sure if that driver_param could 
> still be used to avoid P2P-Device creation.

Yeah, I feared this was the case so I added Ilan :)

I think we need to fix that perhaps in some way? I'm pretty sure we
cannot have P2P-Device and IBSS at the same time properly in our device.

> The interface combinations look confusing to me. Why is there no IBSS or 
> monitor listed there?

Only "real" combinations are listed - anything that's in the "Supported
interface modes" is, by default, only supported with a single virtual
interface. Real combinations (of >1 interface) are listed separately.

johannes

Re: [PATCH 7/7] mac80211: Count correctly interface types

[Arend van Spriel]

On 04/01/15 10:44, Johannes Berg wrote:
> On Wed, 2015-04-01 at 10:36 +0200, Arend van Spriel wrote:
>
>> I think in latest wpa_s the P2P-Device is always created. There used to
>> be a driver_param to create it. Not sure if that driver_param could
>> still be used to avoid P2P-Device creation.
>
> Yeah, I feared this was the case so I added Ilan :)
>
> I think we need to fix that perhaps in some way? I'm pretty sure we
> cannot have P2P-Device and IBSS at the same time properly in our device.
>
>> The interface combinations look confusing to me. Why is there no IBSS or
>> monitor listed there?
>
> Only "real" combinations are listed - anything that's in the "Supported
> interface modes" is, by default, only supported with a single virtual
> interface. Real combinations (of>1 interface) are listed separately.

Thanks. That helps.

Regards,
Arend

RE: [PATCH 7/7] mac80211: Count correctly interface types

[Peer, Ilan]

[-- Attachment #1: Type: text/plain, Size: 1475 bytes --]

Hi,

Can you try the attached patch? Set p2p_disabled=1 in your configuration file.

Anyway, this is a quick solution, will try to work on a more complete one that also checks the combinations.

Ilan.

> -----Original Message-----
> From: Arend van Spriel [mailto:arend@broadcom.com]
> Sent: Wednesday, April 01, 2015 12:16
> To: Johannes Berg
> Cc: Janusz Dziedzic; Peer, Ilan; Grumbach, Emmanuel; linux-
> wireless@vger.kernel.org; Otcheretianski, Andrei
> Subject: Re: [PATCH 7/7] mac80211: Count correctly interface types
> 
> On 04/01/15 10:44, Johannes Berg wrote:
> > On Wed, 2015-04-01 at 10:36 +0200, Arend van Spriel wrote:
> >
> >> I think in latest wpa_s the P2P-Device is always created. There used
> >> to be a driver_param to create it. Not sure if that driver_param
> >> could still be used to avoid P2P-Device creation.
> >
> > Yeah, I feared this was the case so I added Ilan :)
> >
> > I think we need to fix that perhaps in some way? I'm pretty sure we
> > cannot have P2P-Device and IBSS at the same time properly in our device.
> >
> >> The interface combinations look confusing to me. Why is there no IBSS
> >> or monitor listed there?
> >
> > Only "real" combinations are listed - anything that's in the
> > "Supported interface modes" is, by default, only supported with a
> > single virtual interface. Real combinations (of>1 interface) are listed
> separately.
> 
> Thanks. That helps.
> 
> Regards,
> Arend

[-- Attachment #2: 0001-P2P-Do-not-create-a-P2P-Device-interface-is-P2P-is-d.patch --]
[-- Type: application/octet-stream, Size: 1045 bytes --]

From 1f12bf18bf0a862d7942cbeed3c68f18f79393b9 Mon Sep 17 00:00:00 2001
From: Ilan Peer <ilan.peer@intel.com>
Date: Wed, 1 Apr 2015 12:47:02 +0300
Subject: [PATCH] P2P: Do not create a P2P Device interface is P2P is disabled

Do not add the dedicated P2P Device interface in case that
P2P is disabled in the configuration file or globally.

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
---
 wpa_supplicant/wpa_supplicant.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index f690b91..def6a16 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -4385,6 +4385,7 @@ struct wpa_supplicant * wpa_supplicant_add_iface(struct wpa_global *global,
 
 #ifdef CONFIG_P2P
 	if (wpa_s->global->p2p == NULL &&
+	    !wpa_s->global->p2p_disabled && !wpa_s->conf->p2p_disabled &&
 	    (wpa_s->drv_flags & WPA_DRIVER_FLAGS_DEDICATED_P2P_DEVICE) &&
 	    wpas_p2p_add_p2pdev_interface(wpa_s, iface->conf_p2p_dev) < 0) {
 		wpa_printf(MSG_INFO,
-- 
1.9.1

[PATCH v2 5/7] mac80211: Adjust chan_ctx when assigning reserved vif

[andrei.otc]

From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>

When a vif is assigned to a reserved channel context (during CSA, for example)
the width of this chanctx should be adjusted to be the maximum between the
reserved chandef and all the chandefs of other assigned vifs.
Not doing so would result in using chanctx with narrower width than actually
required. Fix this by calling ieee80211_change_chanctx with the widest common
chandef. This both changes the chanctx's width and recalcs min_def.

Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
Reviewed-by: Luciano Coelho <luciano.coelho@intel.com>
---
 net/mac80211/chan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 5bcd4e5..0fd9274 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -1008,6 +1008,8 @@ ieee80211_vif_use_reserved_reassign(struct ieee80211_sub_if_data *sdata)
 	if (WARN_ON(!chandef))
 		return -EINVAL;
 
+	ieee80211_change_chanctx(local, new_ctx, chandef);
+
 	vif_chsw[0].vif = &sdata->vif;
 	vif_chsw[0].old_ctx = &old_ctx->conf;
 	vif_chsw[0].new_ctx = &new_ctx->conf;
@@ -1079,6 +1081,8 @@ ieee80211_vif_use_reserved_assign(struct ieee80211_sub_if_data *sdata)
 	if (WARN_ON(!chandef))
 		return -EINVAL;
 
+	ieee80211_change_chanctx(local, new_ctx, chandef);
+
 	list_del(&sdata->reserved_chanctx_list);
 	sdata->reserved_chanctx = NULL;
 
-- 
1.8.3

Re: [PATCH v2 5/7] mac80211: Adjust chan_ctx when assigning reserved vif

[Johannes Berg]

On Tue, 2015-04-21 at 11:53 +0300, andrei.otc@gmail.com wrote:
> From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> 
> When a vif is assigned to a reserved channel context (during CSA, for example)

This .. doesn't make much sense. vifs aren't assigned to channel
contexts, it's the other way around. I guess I'll rewrite that to "When
a vif starts using a reserved  channel context (...)"

> the width of this chanctx should be adjusted to be the maximum between the
> reserved chandef and all the chandefs of other assigned vifs.

This is not what you do - you don't take anything in the chanctx into
account. ieee80211_chanctx_non_reserved_chandef() just recalculates the
required chanctx, and you're fixing the code to actually apply the
recalculated value.

> Not doing so would result in using chanctx with narrower width than actually
> required. Fix this by calling ieee80211_change_chanctx with the widest common
> chandef. This both changes the chanctx's width and recalcs min_def.

This seems possible, yeah.


>         chandef = ieee80211_chanctx_non_reserved_chandef(local, new_ctx,
>                                 &sdata->reserved_chandef);

>  	if (WARN_ON(!chandef))
>  		return -EINVAL;
>  
> +	ieee80211_change_chanctx(local, new_ctx, chandef);
> +
>  	vif_chsw[0].vif = &sdata->vif;
>  	vif_chsw[0].old_ctx = &old_ctx->conf;
>  	vif_chsw[0].new_ctx = &new_ctx->conf;
[...]
> 	ieee80211_vif_update_chandef(sdata, &sdata->reserved_chandef);

Hmm. (added more context)

The code here seems to be wrong though. It shouldn't overwrite
reserved_chandef, or it shouldn't call ieee80211_vif_update_chandef()
with it, as the vif's bss_conf.chandef should represent what *this* vif
wants/needs, while you're now putting there what the *combination*
requires in the chandef.

That's clearly wrong - please submit a new patch that fixes all the
issues in these functions.

johannes

Re: [PATCH v2 5/7] mac80211: Adjust chan_ctx when assigning reserved vif

[Andrei Otcheretianski]

On Fri, Apr 24, 2015 at 1:41 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
> On Tue, 2015-04-21 at 11:53 +0300, andrei.otc@gmail.com wrote:
> > From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> >
> > When a vif is assigned to a reserved channel context (during CSA, for example)
>
> This .. doesn't make much sense. vifs aren't assigned to channel
> contexts, it's the other way around. I guess I'll rewrite that to "When
> a vif starts using a reserved  channel context (...)"
>

OK, thanks for clarification.

> > the width of this chanctx should be adjusted to be the maximum between the
> > reserved chandef and all the chandefs of other assigned vifs.
>
> This is not what you do - you don't take anything in the chanctx into
> account. ieee80211_chanctx_non_reserved_chandef() just recalculates the
> required chanctx, and you're fixing the code to actually apply the
> recalculated value.
>

sdata->reserved_chandef  is passed to
ieee80211_chanctx_non_reserved_chandef, so it takes the reservation
into account when the required chandef is recalculated.
This is what I tried to say here. I'll rephrase to make it more clear.

> > Not doing so would result in using chanctx with narrower width than actually
> > required. Fix this by calling ieee80211_change_chanctx with the widest common
> > chandef. This both changes the chanctx's width and recalcs min_def.
>
> This seems possible, yeah.
>
>
> >         chandef = ieee80211_chanctx_non_reserved_chandef(local, new_ctx,
> >                                 &sdata->reserved_chandef);
>
> >       if (WARN_ON(!chandef))
> >               return -EINVAL;
> >
> > +     ieee80211_change_chanctx(local, new_ctx, chandef);
> > +
> >       vif_chsw[0].vif = &sdata->vif;
> >       vif_chsw[0].old_ctx = &old_ctx->conf;
> >       vif_chsw[0].new_ctx = &new_ctx->conf;
> [...]
> >       ieee80211_vif_update_chandef(sdata, &sdata->reserved_chandef);
>
> Hmm. (added more context)
>
> The code here seems to be wrong though. It shouldn't overwrite
> reserved_chandef, or it shouldn't call ieee80211_vif_update_chandef()
> with it, as the vif's bss_conf.chandef should represent what *this* vif
> wants/needs, while you're now putting there what the *combination*
> requires in the chandef.
>
> That's clearly wrong - please submit a new patch that fixes all the
> issues in these functions.

I don't see any issue here.  sdata->reserved_chandef isn't overwritten
and it is what sdata really needs.

>
> johannes
>

Re: [PATCH v2 5/7] mac80211: Adjust chan_ctx when assigning reserved vif

[Johannes Berg]

On Sun, 2015-04-26 at 11:13 +0300, Andrei Otcheretianski wrote:

> sdata->reserved_chandef  is passed to
> ieee80211_chanctx_non_reserved_chandef, so it takes the reservation
> into account when the required chandef is recalculated.
> This is what I tried to say here. I'll rephrase to make it more clear.

Ok, thanks.

> > > Not doing so would result in using chanctx with narrower width than actually
> > > required. Fix this by calling ieee80211_change_chanctx with the widest common
> > > chandef. This both changes the chanctx's width and recalcs min_def.
> >
> > This seems possible, yeah.
> >
> >
> > >         chandef = ieee80211_chanctx_non_reserved_chandef(local, new_ctx,
> > >                                 &sdata->reserved_chandef);
> >
> > >       if (WARN_ON(!chandef))
> > >               return -EINVAL;
> > >
> > > +     ieee80211_change_chanctx(local, new_ctx, chandef);
> > > +
> > >       vif_chsw[0].vif = &sdata->vif;
> > >       vif_chsw[0].old_ctx = &old_ctx->conf;
> > >       vif_chsw[0].new_ctx = &new_ctx->conf;
> > [...]
> > >       ieee80211_vif_update_chandef(sdata, &sdata->reserved_chandef);
> >
> > Hmm. (added more context)
> >
> > The code here seems to be wrong though. It shouldn't overwrite
> > reserved_chandef, or it shouldn't call ieee80211_vif_update_chandef()
> > with it, as the vif's bss_conf.chandef should represent what *this* vif
> > wants/needs, while you're now putting there what the *combination*
> > requires in the chandef.
> >
> > That's clearly wrong - please submit a new patch that fixes all the
> > issues in these functions.
> 
> I don't see any issue here.  sdata->reserved_chandef isn't overwritten
> and it is what sdata really needs.

Yes, good point, sorry. I confused the pointer assignments with struct
assignments.

johannes

[PATCH v3 5/7] mac80211: Adjust reserved chan_ctx when assigned to vif

[andrei.otc]

From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>

When a vif starts using a reserved channel context (during CSA, for example)
the required chandef was recalculated, however it was never applied.
This could result in using chanctx with narrower width than actually
required. Fix this by calling ieee80211_change_chanctx with the recalculated
chandef. This both changes the chanctx's width and recalcs min_def.

Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
Reviewed-by: Luciano Coelho <luciano.coelho@intel.com>
---
 net/mac80211/chan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 5bcd4e5..0fd9274 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -1008,6 +1008,8 @@ ieee80211_vif_use_reserved_reassign(struct ieee80211_sub_if_data *sdata)
 	if (WARN_ON(!chandef))
 		return -EINVAL;
 
+	ieee80211_change_chanctx(local, new_ctx, chandef);
+
 	vif_chsw[0].vif = &sdata->vif;
 	vif_chsw[0].old_ctx = &old_ctx->conf;
 	vif_chsw[0].new_ctx = &new_ctx->conf;
@@ -1079,6 +1081,8 @@ ieee80211_vif_use_reserved_assign(struct ieee80211_sub_if_data *sdata)
 	if (WARN_ON(!chandef))
 		return -EINVAL;
 
+	ieee80211_change_chanctx(local, new_ctx, chandef);
+
 	list_del(&sdata->reserved_chanctx_list);
 	sdata->reserved_chanctx = NULL;
 
-- 
1.8.3

Re: [PATCH v3 5/7] mac80211: Adjust reserved chan_ctx when assigned to vif

[Johannes Berg]

On Wed, 2015-05-06 at 18:30 +0300, andrei.otc@gmail.com wrote:
> From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> 
> When a vif starts using a reserved channel context (during CSA, for example)
> the required chandef was recalculated, however it was never applied.
> This could result in using chanctx with narrower width than actually
> required. Fix this by calling ieee80211_change_chanctx with the recalculated
> chandef. This both changes the chanctx's width and recalcs min_def.

Applied.

johannes