2016-08-12 08:40:00

by Purushottam Kushwaha

[permalink] [raw]
Subject: [PATCH v6] cfg80211: Provision to allow the support for different beacon intervals

This commit provides a mechanism for the host drivers to advertise the
support for different beacon intervals among the respective interface
combinations in a group, through diff_beacon_int_gcd (u32).

The configured BI for a specific interface must be a multiple of this
value and also the active beaconing interfaces (along with the current
one) must match with the interface combinations in a group that advertise
the support for different beacon interval.

Signed-off-by: Purushottam Kushwaha <[email protected]>
---
include/net/cfg80211.h | 4 ++++
include/uapi/linux/nl80211.h | 8 ++++++--
net/wireless/core.h | 2 +-
net/wireless/nl80211.c | 13 ++++++++++---
net/wireless/util.c | 36 ++++++++++++++++++++++++++++++++++--
5 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9c23f4d3..a0c635a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2939,6 +2939,8 @@ struct ieee80211_iface_limit {
* only in special cases.
* @radar_detect_widths: bitmap of channel widths supported for radar detection
* @radar_detect_regions: bitmap of regions supported for radar detection
+ * @diff_beacon_int_gcd: This interface combination supports different beacon
+ * intervals in multiple of GCD value.
*
* With this structure the driver can describe which interface
* combinations it supports concurrently.
@@ -2970,6 +2972,7 @@ struct ieee80211_iface_limit {
* .n_limits = ARRAY_SIZE(limits2),
* .max_interfaces = 8,
* .num_different_channels = 1,
+ * .diff_beacon_int_gcd = 100,
* };
*
*
@@ -2997,6 +3000,7 @@ struct ieee80211_iface_combination {
bool beacon_int_infra_match;
u8 radar_detect_widths;
u8 radar_detect_regions;
+ u32 diff_beacon_int_gcd;
};

struct ieee80211_txrx_stypes {
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 2206941..369e403 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4203,6 +4203,9 @@ enum nl80211_iface_limit_attrs {
* of supported channel widths for radar detection.
* @NL80211_IFACE_COMB_RADAR_DETECT_REGIONS: u32 attribute containing the bitmap
* of supported regulatory regions for radar detection.
+ * @NL80211_IFACE_COMB_DIFF_BI_GCD: u32 attribute specifying the GCD of
+ * different beacon intervals supported by all the interface combinations
+ * in this group (not present if all beacon interval must match).
* @NUM_NL80211_IFACE_COMB: number of attributes
* @MAX_NL80211_IFACE_COMB: highest attribute number
*
@@ -4210,8 +4213,8 @@ enum nl80211_iface_limit_attrs {
* limits = [ #{STA} <= 1, #{AP} <= 1 ], matching BI, channels = 1, max = 2
* => allows an AP and a STA that must match BIs
*
- * numbers = [ #{AP, P2P-GO} <= 8 ], channels = 1, max = 8
- * => allows 8 of AP/GO
+ * numbers = [ #{AP, P2P-GO} <= 8 ], diff BI gcd, channels = 1, max = 8,
+ * => allows 8 of AP/GO that can beacon at multiple of gcd intervals
*
* numbers = [ #{STA} <= 2 ], channels = 2, max = 2
* => allows two STAs on different channels
@@ -4237,6 +4240,7 @@ enum nl80211_if_combination_attrs {
NL80211_IFACE_COMB_NUM_CHANNELS,
NL80211_IFACE_COMB_RADAR_DETECT_WIDTHS,
NL80211_IFACE_COMB_RADAR_DETECT_REGIONS,
+ NL80211_IFACE_COMB_DIFF_BI_GCD,

/* keep last */
NUM_NL80211_IFACE_COMB,
diff --git a/net/wireless/core.h b/net/wireless/core.h
index eee9144..5fffe58 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -475,7 +475,7 @@ int ieee80211_get_ratemask(struct ieee80211_supported_band *sband,
u32 *mask);

int cfg80211_validate_beacon_int(struct cfg80211_registered_device *rdev,
- u32 beacon_int);
+ enum nl80211_iftype iftype, u32 beacon_int);

void cfg80211_update_iface_num(struct cfg80211_registered_device *rdev,
enum nl80211_iftype iftype, int num);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 4997857..01945fb 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1020,6 +1020,10 @@ static int nl80211_put_iface_combinations(struct wiphy *wiphy,
nla_put_u32(msg, NL80211_IFACE_COMB_RADAR_DETECT_REGIONS,
c->radar_detect_regions)))
goto nla_put_failure;
+ if (c->diff_beacon_int_gcd &&
+ nla_put_u32(msg, NL80211_IFACE_COMB_DIFF_BI_GCD,
+ c->diff_beacon_int_gcd))
+ goto nla_put_failure;

nla_nest_end(msg, nl_combi);
}
@@ -3433,7 +3437,8 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
params.dtim_period =
nla_get_u32(info->attrs[NL80211_ATTR_DTIM_PERIOD]);

- err = cfg80211_validate_beacon_int(rdev, params.beacon_interval);
+ err = cfg80211_validate_beacon_int(rdev, dev->ieee80211_ptr->iftype,
+ params.beacon_interval);
if (err)
return err;

@@ -7768,7 +7773,8 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
ibss.beacon_interval =
nla_get_u32(info->attrs[NL80211_ATTR_BEACON_INTERVAL]);

- err = cfg80211_validate_beacon_int(rdev, ibss.beacon_interval);
+ err = cfg80211_validate_beacon_int(rdev, NL80211_IFTYPE_ADHOC,
+ ibss.beacon_interval);
if (err)
return err;

@@ -9245,7 +9251,8 @@ static int nl80211_join_mesh(struct sk_buff *skb, struct genl_info *info)
setup.beacon_interval =
nla_get_u32(info->attrs[NL80211_ATTR_BEACON_INTERVAL]);

- err = cfg80211_validate_beacon_int(rdev, setup.beacon_interval);
+ err = cfg80211_validate_beacon_int(rdev, NL80211_IFTYPE_MESH_POINT,
+ setup.beacon_interval);
if (err)
return err;
}
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 0675f51..a8f0171 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1553,20 +1553,52 @@ bool ieee80211_chandef_to_operating_class(struct cfg80211_chan_def *chandef,
}
EXPORT_SYMBOL(ieee80211_chandef_to_operating_class);

+struct diff_beacon_int {
+ u32 beacon_int;
+ bool valid;
+};
+
+static void
+cfg80211_validate_diff_beacon_int(const struct ieee80211_iface_combination *c,
+ void *data)
+{
+ struct diff_beacon_int *diff_bi = data;
+
+ if (c->diff_beacon_int_gcd &&
+ !(diff_bi->beacon_int % c->diff_beacon_int_gcd))
+ diff_bi->valid = true;
+}
+
int cfg80211_validate_beacon_int(struct cfg80211_registered_device *rdev,
- u32 beacon_int)
+ enum nl80211_iftype iftype, u32 beacon_int)
{
struct wireless_dev *wdev;
int res = 0;
+ int iftype_num[NUM_NL80211_IFTYPES];

if (beacon_int < 10 || beacon_int > 10000)
return -EINVAL;

+ memset(iftype_num, 0, sizeof(iftype_num));
+ list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) {
+ if (!wdev->beacon_interval)
+ continue;
+ iftype_num[wdev->iftype]++;
+ }
+ iftype_num[iftype]++;
+
list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) {
if (!wdev->beacon_interval)
continue;
if (wdev->beacon_interval != beacon_int) {
- res = -EINVAL;
+ struct diff_beacon_int diff_bi = { beacon_int, false };
+
+ res = cfg80211_iter_combinations(&rdev->wiphy, 0, 0, iftype_num,
+ cfg80211_validate_diff_beacon_int,
+ &diff_bi);
+ if (res)
+ return res;
+ res = (diff_bi.valid) ? 0 : -EINVAL;
break;
}
}
--
1.9.1



2016-08-12 12:32:40

by Sunil Dutt Undekari

[permalink] [raw]
Subject: RE: [PATCH v6] cfg80211: Provision to allow the support for different beacon intervals

PiBJIHdhcyBqdXN0IHRoaW5raW5nIG91dCBsb3VkIDopDQoNCmNmZzgwMjExX3ZhbGlkYXRlX2Jl
YWNvbl9pbnQgLT4gY2ZnODAyMTFfaXRlcl9jb21iaW5hdGlvbnMgc2hhbGwgYmUgaW52b2tlZCBm
b3IgdGhlIGludGVyZmFjZSBjb21iaW5hdGlvbnMgLCBjdXJyZW50bHkuIGRpZmZfYmVhY29uX2lu
dF9nY2RfbWluIGlzIGFwcGxpY2FibGUgZm9yIHRoZSBpbnRlcmZhY2UgY29tYmluYXRpb25zIGFu
ZCBhbSBub3Qgc3VyZSBob3cgY2FuIHdlIHZhbGlkYXRlIHRoaXMgZm9yIGEgc2luZ2xlIGludGVy
ZmFjZSAuIFRoaXMgc3BlY2lmaWMgaW50ZXJmYWNlIGNhbiBiZSBwYXJ0IG9mIHR3byBkaWZmZXJl
bnQgZ3JvdXBzICggaW50ZXJmYWNlIGNvbWJpbmF0aW9ucykgd2l0aCBkaWZmZXJlbnQgdmFsdWVz
IGZvciAiZGlmZl9iZWFjb25faW50X2djZF9taW4iLiANCkkgZG9uJ3QgdGhpbmsgd2UgY2FuIGdl
dCB0aGUgbWF0Y2ggZm9yIHRoZSByaWdodCBzZXQgb2YgY29tYmluYXRpb24gaGVyZSAsIGlzbid0
ID8gDQoNClRvIG1ha2UgdGhpbmdzIHNpbXBsZSAsIGNhbiB3ZSBpZ25vcmUgdGhlIGZvbGxvd2lu
ZyBydWxlIA0KIiBXaGVuID4wLCBhbnkgYmVhY29uIGludGVydmFsIG11c3QgYWxzbyBiZSBiaWdn
ZXIgdGhhbiB0aGlzIHZhbHVlLiIgDQphbmQgcmF0aGVyIGhhdmUgb25seSB0aGUgZm9sbG93aW5n
IG9uZSA/IA0KIiBXaGVuID4wLCBkaWZmZXJlbnQgYmVhY29uIGludGVydmFscyBtdXN0IGhhdmUg
YSBHQ0QgdGhhdCdzIGF0IGxlYXN0IGFzIGJpZyBhcyB0aGlzIHZhbHVlLiIgIChUbyBiZSBtb3Jl
IHByZWNpc2UgLCBhbnkgc2Vjb25kIGludGVyZmFjZSB3aGljaCBkb2VzIG5vdCBtZWV0IHRoaXMg
cnVsZSAsIHdpbGwgZmFpbCB0byBzdGFydCApIC4NCg0KUmVnYXJkcywNClN1bmlsDQoNCi0tLS0t
T3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBKb2hhbm5lcyBCZXJnIFttYWlsdG86am9oYW5u
ZXNAc2lwc29sdXRpb25zLm5ldF0gDQpTZW50OiBGcmlkYXksIEF1Z3VzdCAxMiwgMjAxNiA1OjEz
IFBNDQpUbzogVW5kZWthcmksIFN1bmlsIER1dHQgPHVzZHV0dEBxdGkucXVhbGNvbW0uY29tPjsg
S3VzaHdhaGEsIFB1cnVzaG90dGFtIDxwa3VzaHdhaEBxdGkucXVhbGNvbW0uY29tPg0KQ2M6IGxp
bnV4LXdpcmVsZXNzQHZnZXIua2VybmVsLm9yZzsgTWFsaW5lbiwgSm91bmkgPGpvdW5pQHFjYS5x
dWFsY29tbS5jb20+OyBIdWxsdXIgU3VicmFtYW55YW0sIEFtYXJuYXRoIDxhbWFybmF0aEBxY2Eu
cXVhbGNvbW0uY29tPjsgS3VtYXIsIERlZXBhayAoUUNBKSA8ZGppbmRhbEBxdGkucXVhbGNvbW0u
Y29tPg0KU3ViamVjdDogUmU6IFtQQVRDSCB2Nl0gY2ZnODAyMTE6IFByb3Zpc2lvbiB0byBhbGxv
dyB0aGUgc3VwcG9ydCBmb3IgZGlmZmVyZW50IGJlYWNvbiBpbnRlcnZhbHMNCg0KT24gRnJpLCAy
MDE2LTA4LTEyIGF0IDExOjM0ICswMDAwLCBVbmRla2FyaSwgU3VuaWwgRHV0dCB3cm90ZToNCj4g
PiANCj4gPiBUaGUgY2xhcmlmaWNhdGlvbiB0aGF0IGl0IGFsc28gcmVwcmVzZW50cyB0aGUgbWlu
aW11bSBmb3IgYSBzaW5nbGUgDQo+ID4gYmVhY29uIGludGVydmFsIHdvdWxkIG1ha2Ugc29tZSBz
ZW5zZSB0byBtZSwgYnV0IGF0IHRoZSBzYW1lIHRpbWUgaXQgDQo+ID4gY2FuJ3QgYmUgdXNlZCBv
bmx5IGZvciB0aGF0LCBzbyBwZXJoYXBzIHNlcGFyYXRpbmcgYSBtaW5pbXVtIG91dA0KPiA+ID4o
cmF0aGVyIHRoYW4gdXNpbmcgdGhlIGhhcmQtY29kZWQgbWluaW11bSBvZiAxMCkgd291bGQgbWFr
ZSBzZW5zZS4NCj4gU29ycnkgLiBDb3VsZCBub3QgZ2V0IHlvdXIgc3RhdGVtZW50IGFib3ZlIC4g
QXJlIHlvdSBzYXlpbmcgdG8gbm90IA0KPiBjaGVjayBpZiB0aGUgYmVhY29uIGludGVydmFsIGlz
IDwgMTAgaW4gY2ZnODAyMTFfdmFsaWRhdGVfYmVhY29uX2ludCANCj4gcmF0aGVyIG9ubHkgY29u
c2lkZXIgPiAxMDAwMCBhbmQgZG8gdGhlIHZhbGlkYXRpb24gaWYgdGhlIGNvbmZpZ3VyZWQgDQo+
IGJlYWNvbiBpbnRlcnZhbCBpcyBsZXNzIHRoYW4gZGlmZl9iZWFjb25faW50X2djZF9taW4gLCB3
aGVuIGNvbmZpZ3VyZWQgDQo+ID8NCj4gSWYgeWVzICwgaG93IGRvIHlvdSB3YW50IHRoZSB2YWxp
ZGF0aW9uIGZvciB0aGUgQkkgKCA8IDEwICkgZm9yIHRoZSANCj4gZmlyc3QgaW50ZXJmYWNlIHRv
IGhhcHBlbiA/DQoNCkkgd2FzIGp1c3QgdGhpbmtpbmcgb3V0IGxvdWQgOikNCg0KUmlnaHQgbm93
IHdlIHZlcmlmeSB0aGF0IGl0J3MgPj0xMCwgYnV0IGRvZXMgdGhhdCBtYWtlIHNlbnNlIGlmIHNh
eSBtaW5fZ2NkIGlzIDIwPyBNYXRoZW1hdGljYWxseSwgZGVmaW5pbmcgZ2NkKG4pPW4gd291bGQg
bWFrZSBzZW5zZSwgc28gaWYgeW91IGp1c3QgaGFkIGEgc2luZ2xlIGludGVyZmFjZSwgYXBwbHlp
bmcgdGhlIG1pbl9nY2Qgd291bGQgbWVhbiB0aGF0IHRoaXMgaXMgYWxzbyB0aGUgbWluaW11bSBi
ZWFjb24gaW50ZXJ2YWwuDQoNCldlIGNhbiBzdGlsbCBsZWF2ZSB0aGUgPDEwIGNoZWNrLCBidXQg
aWYgdGhlIG1pbl9nY2QgaXMgc2V0IHRyZWF0IGp1c3QgYSBzaW5nbGUgaW50ZXJmYWNlIHdpdGgg
YmVhY29uaW5nIHdpdGggdGhlIGFib3ZlIGdjZCgpIGRlZmluaXRpb24gYW5kIGNoZWNrIHRoYXQg
dGhlIGJlYWNvbiBpbnRlcnZhbCBpcyA+PSBtaW5fZ2NkPw0KDQpSZWFsbHkgdGhhdCBqdXN0IG1l
YW5zIGV4dGVuZGluZyB0aGUgZnVuY3Rpb24gdG8gY2FsY3VsYXRlIHRoZSBHQ0QgdG8gYmUgYWJs
ZSB0byByZXR1cm4gYSB2YWx1ZSBmb3IgYSBzaW5nbGUgbnVtYmVyLg0KDQpCdXQgbWF5YmUgSSdt
IG92ZXJkZXNpZ25pbmcgdGhpcyA6KQ0KDQpqb2hhbm5lcw0K

2016-08-12 11:37:43

by Sunil Dutt Undekari

[permalink] [raw]
Subject: RE: [PATCH v6] cfg80211: Provision to allow the support for different beacon intervals

PlRoZSBjbGFyaWZpY2F0aW9uIHRoYXQgaXQgYWxzbyByZXByZXNlbnRzIHRoZSBtaW5pbXVtIGZv
ciBhIHNpbmdsZSBiZWFjb24gaW50ZXJ2YWwgd291bGQgbWFrZSBzb21lIHNlbnNlIHRvIG1lLCBi
dXQgYXQgdGhlIHNhbWUgdGltZSBpdCBjYW4ndCBiZSB1c2VkIG9ubHkgZm9yIHRoYXQsIHNvIHBl
cmhhcHMgc2VwYXJhdGluZyBhIG1pbmltdW0gb3V0ID4ocmF0aGVyIHRoYW4gdXNpbmcgdGhlIGhh
cmQtY29kZWQgbWluaW11bSBvZiAxMCkgd291bGQgbWFrZSBzZW5zZS4NClNvcnJ5IC4gQ291bGQg
bm90IGdldCB5b3VyIHN0YXRlbWVudCBhYm92ZSAuIA0KQXJlIHlvdSBzYXlpbmcgdG8gbm90IGNo
ZWNrIGlmIHRoZSBiZWFjb24gaW50ZXJ2YWwgaXMgPCAxMCBpbiBjZmc4MDIxMV92YWxpZGF0ZV9i
ZWFjb25faW50IHJhdGhlciBvbmx5IGNvbnNpZGVyID4gMTAwMDAgYW5kIGRvIHRoZSB2YWxpZGF0
aW9uIGlmIHRoZSBjb25maWd1cmVkIGJlYWNvbiBpbnRlcnZhbCBpcyBsZXNzIHRoYW4gZGlmZl9i
ZWFjb25faW50X2djZF9taW4gLCB3aGVuIGNvbmZpZ3VyZWQgPyANCklmIHllcyAsIGhvdyBkbyB5
b3Ugd2FudCB0aGUgdmFsaWRhdGlvbiBmb3IgdGhlIEJJICggPCAxMCApIGZvciB0aGUgZmlyc3Qg
aW50ZXJmYWNlIHRvIGhhcHBlbiA/DQoNClJlZ2FyZHMsDQpTdW5pbA0KDQoNCg0KLS0tLS1Pcmln
aW5hbCBNZXNzYWdlLS0tLS0NCkZyb206IEpvaGFubmVzIEJlcmcgW21haWx0bzpqb2hhbm5lc0Bz
aXBzb2x1dGlvbnMubmV0XSANClNlbnQ6IEZyaWRheSwgQXVndXN0IDEyLCAyMDE2IDM6MjUgUE0N
ClRvOiBLdXNod2FoYSwgUHVydXNob3R0YW0gPHBrdXNod2FoQHF0aS5xdWFsY29tbS5jb20+DQpD
YzogbGludXgtd2lyZWxlc3NAdmdlci5rZXJuZWwub3JnOyBNYWxpbmVuLCBKb3VuaSA8am91bmlA
cWNhLnF1YWxjb21tLmNvbT47IFVuZGVrYXJpLCBTdW5pbCBEdXR0IDx1c2R1dHRAcXRpLnF1YWxj
b21tLmNvbT47IEh1bGx1ciBTdWJyYW1hbnlhbSwgQW1hcm5hdGggPGFtYXJuYXRoQHFjYS5xdWFs
Y29tbS5jb20+OyBLdW1hciwgRGVlcGFrIChRQ0EpIDxkamluZGFsQHF0aS5xdWFsY29tbS5jb20+
DQpTdWJqZWN0OiBSZTogW1BBVENIIHY2XSBjZmc4MDIxMTogUHJvdmlzaW9uIHRvIGFsbG93IHRo
ZSBzdXBwb3J0IGZvciBkaWZmZXJlbnQgYmVhY29uIGludGVydmFscw0KDQpPbiBGcmksIDIwMTYt
MDgtMTIgYXQgMTQ6MDkgKzA1MzAsIFB1cnVzaG90dGFtIEt1c2h3YWhhIHdyb3RlOg0KPsKgDQo+
IMKgICogQHJhZGFyX2RldGVjdF9yZWdpb25zOiBiaXRtYXAgb2YgcmVnaW9ucyBzdXBwb3J0ZWQg
Zm9yIHJhZGFyIA0KPiBkZXRlY3Rpb24NCj4gKyAqIEBkaWZmX2JlYWNvbl9pbnRfZ2NkOiBUaGlz
IGludGVyZmFjZSBjb21iaW5hdGlvbiBzdXBwb3J0cw0KPiBkaWZmZXJlbnQgYmVhY29uDQo+ICsg
KglpbnRlcnZhbHMgaW4gbXVsdGlwbGUgb2YgR0NEIHZhbHVlLg0KDQpJIHRoaW5rIHlvdSBzaG91
bGQgYWRkICJMZWF2ZSB0aGlzIHNldCB0byAwIHRvIHJlcXVpcmUgYWxsIGJlYWNvbiBpbnRlcnZh
bHMgdG8gbWF0Y2ggZXhhY3RseS4iDQoNCkFuZCBhbm90aGVyIHRoaW5nIEkganVzdCByZWFsaXpl
ZDogUGVyaGFwcyBzb21lYm9keSBtaWdodCBub3QganVzdCBoYXZlIGEgR0NEIHJlcXVpcmVtZW50
LCBidXQgYWxzbyByZXF1aXJlIHRoYXQgYWxsIGJlYWNvbiBpbnRlcnZhbHMgYXJlIGFuIGV4YWN0
IG11bHRpcGxlIG9mIHRoZSBzbWFsbGVzdCBvZiB0aGVtPyBJLmUuIHRoYXQgdGhlIEdDRChhbGwg
b2YgdGhlbSkgPT0gbWluKGFsbCBvZiB0aGVtKT8NCg0KQW55d2F5LCBpZiB5b3UgZG9uJ3QgaGF2
ZSBzdWNoIGEgcmVxdWlyZW1lbnQgbm93LCB0aGVyZSdzIG5vIHJlYXNvbiB0byBhZGQgaXQgbm93
IGVpdGhlci4NCg0KPiDCoCAqIFdpdGggdGhpcyBzdHJ1Y3R1cmUgdGhlIGRyaXZlciBjYW4gZGVz
Y3JpYmUgd2hpY2ggaW50ZXJmYWNlDQo+IMKgICogY29tYmluYXRpb25zIGl0IHN1cHBvcnRzIGNv
bmN1cnJlbnRseS4NCj4gQEAgLTI5NzAsNiArMjk3Miw3IEBAIHN0cnVjdCBpZWVlODAyMTFfaWZh
Y2VfbGltaXQgew0KPiDCoCAqCS5uX2xpbWl0cyA9IEFSUkFZX1NJWkUobGltaXRzMiksDQo+IMKg
ICoJLm1heF9pbnRlcmZhY2VzID0gOCwNCj4gwqAgKgkubnVtX2RpZmZlcmVudF9jaGFubmVscyA9
IDEsDQo+ICsgKgkuZGlmZl9iZWFjb25faW50X2djZCA9IDEwMCwNCj4gwqAgKsKgwqB9Ow0KDQpU
aGlzIGJlY29tZXMgc29tZXdoYXQgY3VyaW91cy4gRG9lcyBpdCBhbHNvIG1lYW4gdGhhdCB5b3Ug
Y2FuIG9ubHkgc3VwcG9ydCBiZWFjb24gaW50ZXJ2YWxzIHRoYXQgYXJlIG11bHRpcGxlcyBvZiAx
MDA/DQoNClRoaXMgYmVpbmcgeW91ciBleGFtcGxlIGtpbmRhIG1ha2VzIG1lIHRoaW5rIHRoYXQg
eW91ICpkbyogd2FudCB0byBoYXZlIGl0IG11bHRpcGxlcyBvZiB0aGUgc21hbGxlc3QsIGxpa2Ug
SSB3YXMgb3V0bGluaW5nIGJlbG93Pw0KDQpBc3N1bWluZyB5b3VyIGRyaXZlciB3b3VsZCBhZHZl
cnRpc2UgaXQgdGhpcyB3YXksIHdvdWxkIHlvdSBhY3R1YWxseSBiZSBhYmxlIHRvIGRvIEJJcyBv
ZiAyMDAgYW5kIDMwMCBvbiB0d28gaW50ZXJmYWNlcz8NCg0KUmVnYXJkbGVzcywgYWR2ZXJ0aXNp
bmcgMTAwIHdvdWxkIG1lYW4gbm90IHN1cHBvcnRpbmcgYSBiZWFjb24gaW50ZXJ2YWwgb2YgNTAg
YXQgYWxsLCB3aGljaCBzZWVtcyBvZGQuDQoNCg0KSSB0aGluayB0aGUgR0NEIHJlcXVpcmVtZW50
IHNob3VsZCBiZSByZXBocmFzZWQgKGFuZCBteSBvcmlnaW5hbCBtZW50aW9uIG9mIHRoaXMgd2Fz
bid0IHZlcnkgcHJlY2lzZSkgLSBJIHRoaW5rIGl0IHJlYWxseSBzaG91bGRuJ3QgYmUgYW4gZXhh
Y3QgR0NEIHJlcXVpcmVtZW50IGxpa2UgeW91IGRpZCBub3csIGJ1dCBhIHJlcXVpcmVtZW50ICpv
biogdGhlIEdDRCwgbGlrZQ0KDQrCoEBkaWZmX2JlYWNvbl9pbnRfZ2NkX21pbjogV2hlbiAwLCBh
bGwgYmVhY29uIGludGVydmFscyBtdXN0IG1hdGNoLg0KCVdoZW4gPjAsIGRpZmZlcmVudCBiZWFj
b24gaW50ZXJ2YWxzIG11c3QgaGF2ZSBhIEdDRCB0aGF0J3MgYXQNCglsZWFzdCBhcyBiaWcgYXMg
dGhpcyB2YWx1ZS4NCg0KbWF5YmUgYWxzbyBhZGQsIHNpbmNlIHRoZSBHQ0Qgb2YgYSBzaW5nbGUg
dmFsdWUgaXMgZmlzaHk6DQoNCglXaGVuID4wLCBhbnkgYmVhY29uIGludGVydmFsIG11c3QgYWxz
byBiZSBiaWdnZXIgdGhhbiB0aGlzDQoJdmFsdWUuDQoNCldpdGggYSBkaWZmX2JlYWNvbl9pbnRf
Z2NkX21pbj01MCwgdGhpcyB3b3VsZCBzdGlsbCBhbGxvdyBiZWFjb24gaW50ZXJ2YWxzIDEwMiwx
NTMgKEdDRCA1MSksIHdoaWNoIHNlZW1zIG11Y2ggbW9yZSBmbGV4aWJsZS4NCg0KVGhlIGNsYXJp
ZmljYXRpb24gdGhhdCBpdCBhbHNvIHJlcHJlc2VudHMgdGhlIG1pbmltdW0gZm9yIGEgc2luZ2xl
IGJlYWNvbiBpbnRlcnZhbCB3b3VsZCBtYWtlIHNvbWUgc2Vuc2UgdG8gbWUsIGJ1dCBhdCB0aGUg
c2FtZSB0aW1lIGl0IGNhbid0IGJlIHVzZWQgb25seSBmb3IgdGhhdCwgc28gcGVyaGFwcyBzZXBh
cmF0aW5nIGEgbWluaW11bSBvdXQgKHJhdGhlciB0aGFuIHVzaW5nIHRoZSBoYXJkLWNvZGVkIG1p
bmltdW0gb2YgMTApIHdvdWxkIG1ha2Ugc2Vuc2UuDQoNCj4gKyAqIEBOTDgwMjExX0lGQUNFX0NP
TUJfRElGRl9CSV9HQ0Q6IHUzMiBhdHRyaWJ1dGUgc3BlY2lmeWluZyB0aGUgR0NEDQo+IG9mDQo+
ICsgKglkaWZmZXJlbnQgYmVhY29uIGludGVydmFscyBzdXBwb3J0ZWQgYnkgYWxsIHRoZSBpbnRl
cmZhY2UNCj4gY29tYmluYXRpb25zDQo+ICsgKglpbiB0aGlzIGdyb3VwIChub3QgcHJlc2VudCBp
ZiBhbGwgYmVhY29uIGludGVydmFsIG11c3QNCj4gbWF0Y2gpLg0KDQpJJ2QgdHVybiB0aGF0IGFy
b3VuZDogIihpZiBub3QgcHJlc2VudCwgYWxsIGJlYWNvbiBpbnRlcnZhbHMgbXVzdCBtYXRjaCki
DQoNCj4gK3N0cnVjdCBkaWZmX2JlYWNvbl9pbnQgew0KPiA+ICsJdTMyIGJlYWNvbl9pbnQ7DQo+
ID4gKwlib29sIHZhbGlkOw0KPiArfTsNCj4gKw0KPiArc3RhdGljIHZvaWQNCj4gK2NmZzgwMjEx
X3ZhbGlkYXRlX2RpZmZfYmVhY29uX2ludChjb25zdCBzdHJ1Y3QgDQo+ICtpZWVlODAyMTFfaWZh
Y2VfY29tYmluYXRpb24gKmMsDQo+ID4gKwkJCQnCoMKgdm9pZCAqZGF0YSkNCj4gK3sNCj4gPiAr
CXN0cnVjdCBkaWZmX2JlYWNvbl9pbnQgKmRpZmZfYmkgPSBkYXRhOw0KPiArDQo+ID4gKwlpZiAo
Yy0+ZGlmZl9iZWFjb25faW50X2djZCAmJg0KPiA+ICsJwqDCoMKgwqAhKGRpZmZfYmktPmJlYWNv
bl9pbnQgJSBjLT5kaWZmX2JlYWNvbl9pbnRfZ2NkKSkNCj4gPiArCQlkaWZmX2JpLT52YWxpZCA9
IHRydWU7DQo+ICt9DQo+ID4gDQoNCk9idmlvdXNseSwgdGhpcyB2YWxpZGF0aW9uIGlzIGZhciBl
YXNpZXIgdGhhbiBjYWxjdWxhdGluZyB0aGUgR0NEIGZpcnN0LCBidXQgSSB0aGluayBpdCdzIHdv
cnRod2hpbGUgZG9pbmcgaXQgdGhlIG90aGVyIHdheS4NCg0Kam9oYW5uZXMNCg0K

2016-08-12 11:42:49

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v6] cfg80211: Provision to allow the support for different beacon intervals

On Fri, 2016-08-12 at 11:34 +0000, Undekari, Sunil Dutt wrote:
> >
> > The clarification that it also represents the minimum for a single
> > beacon interval would make some sense to me, but at the same time
> > it can't be used only for that, so perhaps separating a minimum out
> > >(rather than using the hard-coded minimum of 10) would make sense.
> Sorry . Could not get your statement above . 
> Are you saying to not check if the beacon interval is < 10 in
> cfg80211_validate_beacon_int rather only consider > 10000 and do the
> validation if the configured beacon interval is less than
> diff_beacon_int_gcd_min , when configured ? 
> If yes , how do you want the validation for the BI ( < 10 ) for the
> first interface to happen ?

I was just thinking out loud :)

Right now we verify that it's >=10, but does that make sense if say
min_gcd is 20? Mathematically, defining gcd(n)=n would make sense, so
if you just had a single interface, applying the min_gcd would mean
that this is also the minimum beacon interval.

We can still leave the <10 check, but if the min_gcd is set treat just
a single interface with beaconing with the above gcd() definition and
check that the beacon interval is >= min_gcd?

Really that just means extending the function to calculate the GCD to
be able to return a value for a single number.

But maybe I'm overdesigning this :)

johannes

2016-08-12 10:30:19

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v6] cfg80211: Provision to allow the support for different beacon intervals

On Fri, 2016-08-12 at 14:09 +0530, Purushottam Kushwaha wrote:

>   * @radar_detect_regions: bitmap of regions supported for radar
> detection
> + * @diff_beacon_int_gcd: This interface combination supports
> different beacon
> + * intervals in multiple of GCD value.

I think you should add "Leave this set to 0 to require all beacon
intervals to match exactly."

And another thing I just realized: Perhaps somebody might not just have
a GCD requirement, but also require that all beacon intervals are an
exact multiple of the smallest of them? I.e. that the GCD(all of them)
== min(all of them)?

Anyway, if you don't have such a requirement now, there's no reason to
add it now either.

>   * With this structure the driver can describe which interface
>   * combinations it supports concurrently.
> @@ -2970,6 +2972,7 @@ struct ieee80211_iface_limit {
>   * .n_limits = ARRAY_SIZE(limits2),
>   * .max_interfaces = 8,
>   * .num_different_channels = 1,
> + * .diff_beacon_int_gcd = 100,
>   *  };

This becomes somewhat curious. Does it also mean that you can only
support beacon intervals that are multiples of 100?

This being your example kinda makes me think that you *do* want to have
it multiples of the smallest, like I was outlining below?

Assuming your driver would advertise it this way, would you actually be
able to do BIs of 200 and 300 on two interfaces?

Regardless, advertising 100 would mean not supporting a beacon interval
of 50 at all, which seems odd.


I think the GCD requirement should be rephrased (and my original
mention of this wasn't very precise) - I think it really shouldn't be
an exact GCD requirement like you did now, but a requirement *on* the
GCD, like

 @diff_beacon_int_gcd_min: When 0, all beacon intervals must match.
When >0, different beacon intervals must have a GCD that's at
least as big as this value.

maybe also add, since the GCD of a single value is fishy:

When >0, any beacon interval must also be bigger than this
value.

With a diff_beacon_int_gcd_min=50, this would still allow beacon
intervals 102,153 (GCD 51), which seems much more flexible.

The clarification that it also represents the minimum for a single
beacon interval would make some sense to me, but at the same time it
can't be used only for that, so perhaps separating a minimum out
(rather than using the hard-coded minimum of 10) would make sense.

> + * @NL80211_IFACE_COMB_DIFF_BI_GCD: u32 attribute specifying the GCD
> of
> + * different beacon intervals supported by all the interface
> combinations
> + * in this group (not present if all beacon interval must
> match).

I'd turn that around: "(if not present, all beacon intervals must
match)"

> +struct diff_beacon_int {
> > + u32 beacon_int;
> > + bool valid;
> +};
> +
> +static void
> +cfg80211_validate_diff_beacon_int(const struct ieee80211_iface_combination *c,
> > +   void *data)
> +{
> > + struct diff_beacon_int *diff_bi = data;
> +
> > + if (c->diff_beacon_int_gcd &&
> > +     !(diff_bi->beacon_int % c->diff_beacon_int_gcd))
> > + diff_bi->valid = true;
> +}
> >

Obviously, this validation is far easier than calculating the GCD
first, but I think it's worthwhile doing it the other way.

johannes


2016-08-26 08:07:53

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v6] cfg80211: Provision to allow the support for different beacon intervals

Hi,

Sorry - missed that mail somehow.

> cfg80211_validate_beacon_int -> cfg80211_iter_combinations shall be
> invoked for the interface combinations , currently.
> diff_beacon_int_gcd_min is applicable for the interface combinations
> and am not sure how can we validate this for a single interface .
> This specific interface can be part of two different groups (
> interface combinations) with different values for
> "diff_beacon_int_gcd_min". 
> I don't think we can get the match for the right set of combination
> here , isn't ? 

Well if you have just a single interface, any combination that contains
it is valid, so you'd just continue?

I *think* the code works by checking if any combination applies to the
currently desired state, and rejects if no combination is possible,
that would still be perfectly reasonable here, no?

> To make things simple , can we ignore the following rule 
> " When >0, any beacon interval must also be bigger than this value." 
> and rather have only the following one ? 
> " When >0, different beacon intervals must have a GCD that's at least
> as big as this value."  (To be more precise , any second interface
> which does not meet this rule , will fail to start ) .

Yeah, I suppose we could, but does that really make sense? If you can
have a smaller BI, then you could as well have a smaller GCD, no?

johannes