diff mbox

[v2] cfg80211: Allow differnt beacon interval if driver supports

Message ID 1468849985-6881-1-git-send-email-pkushwah@qti.qualcomm.com (mailing list archive)
State Superseded
Delegated to: Johannes Berg
Headers show

Commit Message

Purushottam Kushwaha July 18, 2016, 1:53 p.m. UTC
Driver may allow support for different beacon interval on virtual interfaces.
Allow if such support is advertised by driver. This adds new ext_feature as
NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL.

Signed-off-by: Purushottam Kushwaha <pkushwah@qti.qualcomm.com>
---
 include/uapi/linux/nl80211.h | 3 +++
 net/wireless/util.c          | 4 ++++
 2 files changed, 7 insertions(+)

Comments

Kalle Valo July 18, 2016, 4:25 p.m. UTC | #1
Purushottam Kushwaha <pkushwah@qti.qualcomm.com> writes:

> Driver may allow support for different beacon interval on virtual interfaces.
> Allow if such support is advertised by driver. This adds new ext_feature as
> NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL.
>
> Signed-off-by: Purushottam Kushwaha <pkushwah@qti.qualcomm.com>

A typo in the title: "differnt"
Johannes Berg July 18, 2016, 6:56 p.m. UTC | #2
On Mon, 2016-07-18 at 19:23 +0530, Purushottam Kushwaha wrote:
> Driver may allow support for different beacon interval on virtual
> interfaces.
> Allow if such support is advertised by driver. This adds new
> ext_feature as NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL.

We have NL80211_IFACE_COMB_STA_AP_BI_MATCH in interface combinations,
perhaps it would make sense to also put this flag there?

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
Ben Greear July 18, 2016, 7:04 p.m. UTC | #3
On 07/18/2016 11:56 AM, Johannes Berg wrote:
> On Mon, 2016-07-18 at 19:23 +0530, Purushottam Kushwaha wrote:
>> Driver may allow support for different beacon interval on virtual
>> interfaces.
>> Allow if such support is advertised by driver. This adds new
>> ext_feature as NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL.
>
> We have NL80211_IFACE_COMB_STA_AP_BI_MATCH in interface combinations,
> perhaps it would make sense to also put this flag there?

I was under impression that ath9k had supported this for years,
but I guess I haven't tested it lately....

Thanks,
Ben
Arend van Spriel July 18, 2016, 7:13 p.m. UTC | #4
On 18-7-2016 20:56, Johannes Berg wrote:
> On Mon, 2016-07-18 at 19:23 +0530, Purushottam Kushwaha wrote:
>> Driver may allow support for different beacon interval on virtual
>> interfaces.
>> Allow if such support is advertised by driver. This adds new
>> ext_feature as NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL.
> 
> We have NL80211_IFACE_COMB_STA_AP_BI_MATCH in interface combinations,
> perhaps it would make sense to also put this flag there?

Hi Johannes,

Was looking at the same thing. The description of that flag was a bit
unclear to me.

"""
 * @beacon_int_infra_match: In this combination, the beacon intervals
 *      between infrastructure and AP types must match. This is required
 *      only in special cases.
"""

It is not explicitly stated but it implies the STA vif is connected, right.

Probably going off-topic here, but I am also wondering about the
use-case of the above patch. I have been looking at M-BSS. Toward
user-space these are AP interfaces, but like described in hostapd.conf
example a number of AP configuration items are required to be equal. Now
we also have chipsets with two 802.11 cores and on each an AP could be
setup with independent beacon interval. So to make the distinction would
it make sense to introduce MBSS_AP iftype? Or does AP_VLAN cover the
MBSS use-case?

Regards,
Arend
--
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
Johannes Berg July 18, 2016, 7:14 p.m. UTC | #5
On Mon, 2016-07-18 at 12:04 -0700, Ben Greear wrote:
> 
> On 07/18/2016 11:56 AM, Johannes Berg wrote:
> > On Mon, 2016-07-18 at 19:23 +0530, Purushottam Kushwaha wrote:
> > > Driver may allow support for different beacon interval on virtual
> > > interfaces.
> > > Allow if such support is advertised by driver. This adds new
> > > ext_feature as NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL.
> > 
> > We have NL80211_IFACE_COMB_STA_AP_BI_MATCH in interface
> > combinations,
> > perhaps it would make sense to also put this flag there?
> 
> I was under impression that ath9k had supported this for years,
> but I guess I haven't tested it lately....
> 

It may very well have, but if userspace can't rely on it because other
drivers don't then it's pretty pointless.

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
Johannes Berg July 18, 2016, 7:15 p.m. UTC | #6
On Mon, 2016-07-18 at 21:13 +0200, Arend Van Spriel wrote:
> 
> On 18-7-2016 20:56, Johannes Berg wrote:
> > On Mon, 2016-07-18 at 19:23 +0530, Purushottam Kushwaha wrote:
> > > Driver may allow support for different beacon interval on virtual
> > > interfaces.
> > > Allow if such support is advertised by driver. This adds new
> > > ext_feature as NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL.
> > 
> > We have NL80211_IFACE_COMB_STA_AP_BI_MATCH in interface
> > combinations,
> > perhaps it would make sense to also put this flag there?
> 
> Hi Johannes,
> 
> Was looking at the same thing. The description of that flag was a bit
> unclear to me.
> 
> """
>  * @beacon_int_infra_match: In this combination, the beacon intervals
>  *      between infrastructure and AP types must match. This is
> required
>  *      only in special cases.
> """
> 
> It is not explicitly stated but it implies the STA vif is connected,
> right.

Yes.

Forget this flag. I don't think any driver sets it - it was a hack for
iwldvm. I also don't think any userspace cares about it, and it likely
never really worked. What if the STA vif reconnects anyway?

I was merely pointing this out wrt. the grouping and where to put
something new.

> Probably going off-topic here, but I am also wondering about the
> use-case of the above patch. I have been looking at M-BSS. Toward
> user-space these are AP interfaces, but like described in
> hostapd.conf
> example a number of AP configuration items are required to be equal.
> Now
> we also have chipsets with two 802.11 cores and on each an AP could
> be
> setup with independent beacon interval. So to make the distinction
> would
> it make sense to introduce MBSS_AP iftype? Or does AP_VLAN cover the
> MBSS use-case?
> 

I don't think AP_VLAN does, but isn't a mesh portal simply a mesh point
interface and an AP interface?

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
Arend van Spriel July 18, 2016, 7:21 p.m. UTC | #7
On 18-7-2016 21:15, Johannes Berg wrote:
> On Mon, 2016-07-18 at 21:13 +0200, Arend Van Spriel wrote:
>>
>> On 18-7-2016 20:56, Johannes Berg wrote:
>>> On Mon, 2016-07-18 at 19:23 +0530, Purushottam Kushwaha wrote:
>>>> Driver may allow support for different beacon interval on virtual
>>>> interfaces.
>>>> Allow if such support is advertised by driver. This adds new
>>>> ext_feature as NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL.
>>>
>>> We have NL80211_IFACE_COMB_STA_AP_BI_MATCH in interface
>>> combinations,
>>> perhaps it would make sense to also put this flag there?
>>
>> Hi Johannes,
>>
>> Was looking at the same thing. The description of that flag was a bit
>> unclear to me.
>>
>> """
>>  * @beacon_int_infra_match: In this combination, the beacon intervals
>>  *      between infrastructure and AP types must match. This is
>> required
>>  *      only in special cases.
>> """
>>
>> It is not explicitly stated but it implies the STA vif is connected,
>> right.
> 
> Yes.
> 
> Forget this flag. I don't think any driver sets it - it was a hack for
> iwldvm. I also don't think any userspace cares about it, and it likely
> never really worked. What if the STA vif reconnects anyway?
> 
> I was merely pointing this out wrt. the grouping and where to put
> something new.
> 
>> Probably going off-topic here, but I am also wondering about the
>> use-case of the above patch. I have been looking at M-BSS. Toward
>> user-space these are AP interfaces, but like described in
>> hostapd.conf
>> example a number of AP configuration items are required to be equal.
>> Now
>> we also have chipsets with two 802.11 cores and on each an AP could
>> be
>> setup with independent beacon interval. So to make the distinction
>> would
>> it make sense to introduce MBSS_AP iftype? Or does AP_VLAN cover the
>> MBSS use-case?
>>
> 
> I don't think AP_VLAN does, but isn't a mesh portal simply a mesh point
> interface and an AP interface?

Whoops. Should not use abbreviations like that. I meant the
Multiple-BSSID functionality (from hostapd):

##### Multiple BSSID support
##################################################
#
# Above configuration is using the default interface (wlan#, or
multi-SSID VLAN
# interfaces). Other BSSIDs can be added by using separator 'bss' with
# default interface name to be allocated for the data packets of the new
BSS.
#
# hostapd will generate BSSID mask based on the BSSIDs that are
# configured. hostapd will verify that dev_addr & MASK == dev_addr. If
this is
# not the case, the MAC address of the radio must be changed before starting
# hostapd (ifconfig wlan0 hw ether <MAC addr>). If a BSSID is configured for
# every secondary BSS, this limitation is not applied at hostapd and other
# masks may be used if the driver supports them (e.g., swap the locally
# administered bit)
#
# BSSIDs are assigned in order to each BSS, unless an explicit BSSID is
# specified using the 'bssid' parameter.
# If an explicit BSSID is specified, it must be chosen such that it:
# - results in a valid MASK that covers it and the dev_addr
# - is not the same as the MAC address of the radio
# - is not the same as any other explicitly specified BSSID
#
# Alternatively, the 'use_driver_iface_addr' parameter can be used to
request
# hostapd to use the driver auto-generated interface address (e.g., to
use the
# exact MAC addresses allocated to the device).
#
# Not all drivers support multiple BSSes. The exact mechanism for
determining
# the driver capabilities is driver specific. With the current (i.e., a
recent
# kernel) drivers using nl80211, this information can be checked with
"iw list"
# (search for "valid interface combinations").
#
# Please note that hostapd uses some of the values configured for the
first BSS
# as the defaults for the following BSSes. However, it is recommended
that all
# BSSes include explicit configuration of all relevant configuration items.
#

Regards,
Arend
--
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
Johannes Berg July 18, 2016, 7:22 p.m. UTC | #8
> Whoops. Should not use abbreviations like that. I meant the
> Multiple-BSSID functionality (from hostapd):

Ah, heh. Yes, I think this is exactly the use case they have in mind.
Each of the multiple BSSes is represented by its own AP type virtual
interface.

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
Ben Greear July 18, 2016, 7:31 p.m. UTC | #9
On 07/18/2016 12:14 PM, Johannes Berg wrote:
> On Mon, 2016-07-18 at 12:04 -0700, Ben Greear wrote:
>>
>> On 07/18/2016 11:56 AM, Johannes Berg wrote:
>>> On Mon, 2016-07-18 at 19:23 +0530, Purushottam Kushwaha wrote:
>>>> Driver may allow support for different beacon interval on virtual
>>>> interfaces.
>>>> Allow if such support is advertised by driver. This adds new
>>>> ext_feature as NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL.
>>>
>>> We have NL80211_IFACE_COMB_STA_AP_BI_MATCH in interface
>>> combinations,
>>> perhaps it would make sense to also put this flag there?
>>
>> I was under impression that ath9k had supported this for years,
>> but I guess I haven't tested it lately....
>>
>
> It may very well have, but if userspace can't rely on it because other
> drivers don't then it's pretty pointless.

A flag would be nice, but for backwards compat, it could be a negative
flag.  But, if mac80211 has to be patched to make this work, then maybe
the ath9k feature has been disabled/broken for some time and wouldn't matter
anyway.

Also, ath9k had some restrictions about having the timers be certain
multiples of each other so the hardware could properly service beacons
for multiple vdevs.  If chip-specific restrictions remain, then it may be almost impossible
to communicate this properly to the hostapd/userspace.  At best, the driver
could just fail to start the vdev in case of mismatch?

Thanks,
Ben
Arend van Spriel July 18, 2016, 7:35 p.m. UTC | #10
On 18-7-2016 21:21, Arend Van Spriel wrote:
> On 18-7-2016 21:15, Johannes Berg wrote:
>> On Mon, 2016-07-18 at 21:13 +0200, Arend Van Spriel wrote:
>>>
>>> On 18-7-2016 20:56, Johannes Berg wrote:
>>>> On Mon, 2016-07-18 at 19:23 +0530, Purushottam Kushwaha wrote:
>>>>> Driver may allow support for different beacon interval on virtual
>>>>> interfaces.
>>>>> Allow if such support is advertised by driver. This adds new
>>>>> ext_feature as NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL.
>>>>
>>>> We have NL80211_IFACE_COMB_STA_AP_BI_MATCH in interface
>>>> combinations,
>>>> perhaps it would make sense to also put this flag there?
>>>
>>> Hi Johannes,
>>>
>>> Was looking at the same thing. The description of that flag was a bit
>>> unclear to me.
>>>
>>> """
>>>  * @beacon_int_infra_match: In this combination, the beacon intervals
>>>  *      between infrastructure and AP types must match. This is
>>> required
>>>  *      only in special cases.
>>> """
>>>
>>> It is not explicitly stated but it implies the STA vif is connected,
>>> right.
>>
>> Yes.
>>
>> Forget this flag. I don't think any driver sets it - it was a hack for
>> iwldvm. I also don't think any userspace cares about it, and it likely
>> never really worked. What if the STA vif reconnects anyway?

Uhm. My memory tells me differently and LXR also [1] :-p

Regards,
Arend

[1]
http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c#L6182
--
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
Sunil Dutt Undekari July 20, 2016, 1:15 p.m. UTC | #11
PkFoLCBoZWguIFllcywgSSB0aGluayB0aGlzIGlzIGV4YWN0bHkgdGhlIHVzZSBjYXNlIHRoZXkg
aGF2ZSBpbiBtaW5kLg0KPkVhY2ggb2YgdGhlIG11bHRpcGxlIEJTU2VzIGlzIHJlcHJlc2VudGVk
IGJ5IGl0cyBvd24gQVAgdHlwZSB2aXJ0dWFsIGludGVyZmFjZS4NClllcyAuIFRoaXMgaXMgdGhl
IHJlcXVpcmVtZW50IHdlIGFyZSB0YXJnZXRpbmcgLiBUaGUgdXNlIGNhc2UgaXMgYWxzbyB0byBo
YXZlIGRpZmZlcmVudCBiZWFjb24gaW50ZXJ2YWwgZm9yIHRoZXNlIFZBUCdzIGFuZCBoZW5jZSB0
aGlzIHBhdGNoLiANClBsZWFzZSBsZXQgdXMga25vdyBpZiB0aGlzIHBhdGNoIHdpdGggdHlwbyBh
ZGRyZXNzZWQgdG8gImRpZmZlcmVudCIgaXMgYWNjZXB0YWJsZS4gU2hhbGwgcmFpc2UgYW5vdGhl
ciBwYXRjaCBzZXQgYWNjb3JkaW5nbHkuIA0KDQpSZWdhcmRzLA0KU3VuaWwNCg0K
--
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
diff mbox

Patch

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 2206941..a910d0e 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4551,6 +4551,8 @@  enum nl80211_feature_flags {
  *	(if available).
  * @NL80211_EXT_FEATURE_SET_SCAN_DWELL: This driver supports configuration of
  *	channel dwell time.
+ * @NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL: This driver supports different
+ *	beacon interval on virtual interfaces.
  *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -4562,6 +4564,7 @@  enum nl80211_ext_feature_index {
 	NL80211_EXT_FEATURE_SCAN_START_TIME,
 	NL80211_EXT_FEATURE_BSS_PARENT_TSF,
 	NL80211_EXT_FEATURE_SET_SCAN_DWELL,
+	NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 2443ee3..65eeb23 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1560,6 +1560,10 @@  int cfg80211_validate_beacon_int(struct cfg80211_registered_device *rdev,
 	if (!beacon_int)
 		return -EINVAL;
 
+	if (wiphy_ext_feature_isset(&rdev->wiphy,
+				    NL80211_EXT_FEATURE_DIFF_BEACON_INTERVAL))
+		return 0;
+
 	list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) {
 		if (!wdev->beacon_interval)
 			continue;