2015-03-12 06:53:33

by Grumbach, Emmanuel

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

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

Signed-off-by: Emmanuel Grumbach <[email protected]>
---
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



2015-03-12 06:53:46

by Grumbach, Emmanuel

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

From: Johannes Berg <[email protected]>

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 <[email protected]>
Reviewed-by: Luciano Coelho <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
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


2015-03-12 06:53:45

by Grumbach, Emmanuel

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

From: Andrei Otcheretianski <[email protected]>

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 <[email protected]>
Reviewed-by: Luciano Coelho <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
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


2015-03-16 08:33:06

by Johannes Berg

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

On Thu, 2015-03-12 at 08:53 +0200, Emmanuel Grumbach wrote:
> From: Andrei Otcheretianski <[email protected]>
>
> 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


2015-03-12 06:53:36

by Grumbach, Emmanuel

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

From: Johannes Berg <[email protected]>

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

Signed-off-by: Johannes Berg <[email protected]>
---
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


2015-03-12 06:53:49

by Grumbach, Emmanuel

[permalink] [raw]
Subject: [PATCH 7/7] mac80211: Count correctly interface types

From: Andrei Otcheretianski <[email protected]>

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 <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
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


2015-03-12 06:53:38

by Grumbach, Emmanuel

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

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
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


2015-03-12 06:53:41

by Grumbach, Emmanuel

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

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
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


2015-04-01 07:12:07

by Johannes Berg

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

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


2015-04-01 09:51:03

by Peer, Ilan

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

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:[email protected]]
> Sent: Wednesday, April 01, 2015 12:16
> To: Johannes Berg
> Cc: Janusz Dziedzic; Peer, Ilan; Grumbach, Emmanuel; linux-
> [email protected]; 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


Attachments:
0001-P2P-Do-not-create-a-P2P-Device-interface-is-P2P-is-d.patch (1.02 kB)
0001-P2P-Do-not-create-a-P2P-Device-interface-is-P2P-is-d.patch

2015-04-24 10:42:02

by Johannes Berg

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

On Tue, 2015-04-21 at 11:53 +0300, [email protected] wrote:
> From: Andrei Otcheretianski <[email protected]>
>
> 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


2015-04-01 08:44:15

by Johannes Berg

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

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


2015-04-01 09:15:47

by Arend van Spriel

[permalink] [raw]
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

2015-04-21 08:54:06

by Andrei Otcheretianski

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

From: Andrei Otcheretianski <[email protected]>

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 <[email protected]>
Reviewed-by: Luciano Coelho <[email protected]>
---
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


2015-04-26 08:13:36

by Andrei Otcheretianski

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

On Fri, Apr 24, 2015 at 1:41 PM, Johannes Berg
<[email protected]> wrote:
>
> On Tue, 2015-04-21 at 11:53 +0300, [email protected] wrote:
> > From: Andrei Otcheretianski <[email protected]>
> >
> > 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
>

2015-04-01 08:36:11

by Arend van Spriel

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

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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2015-04-01 06:06:06

by Janusz Dziedzic

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

On 12 March 2015 at 07:53, Emmanuel Grumbach
<[email protected]> wrote:
>
> From: Andrei Otcheretianski <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Emmanuel Grumbach <[email protected]>
> ---
> 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

2015-05-06 12:50:32

by Johannes Berg

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

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


2015-05-07 12:25:47

by Johannes Berg

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

On Wed, 2015-05-06 at 18:30 +0300, [email protected] wrote:
> From: Andrei Otcheretianski <[email protected]>
>
> 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


2015-05-06 15:31:27

by Andrei Otcheretianski

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

From: Andrei Otcheretianski <[email protected]>

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 <[email protected]>
Reviewed-by: Luciano Coelho <[email protected]>
---
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