2013-11-11 19:32:53

by Chauhan, Rajesh

[permalink] [raw]
Subject: [PATCHv3] cfg80211: add support for frequency interference event

Add support for wireless driver to report frequency range(s) to
be avoided because of interference. Wireless driver specifies
source of interference for a given frequency range. For now,
applicable source of interference is "cellular" only and other
interference source types can be added later. If SoftAP/P2P-GO
is operating on interfering frequency then user space should stop
and restart them avoiding interfering frequency range(s). User
space may decide to continue operation on interfering frequency,
but in such case, there might be impact on performance.

Signed-off-by: Rajesh Chauhan <[email protected]>
---
include/net/cfg80211.h | 36 ++++++++++++++++++++++
include/uapi/linux/nl80211.h | 48 +++++++++++++++++++++++++++++
net/wireless/nl80211.c | 73 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 157 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 419202c..b49bc47 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1933,6 +1933,30 @@ struct cfg80211_update_ft_ies_params {
};

/**
+ * struct cfg80211_avoid_frequency_range - frequency range(s) to avoid
+ *
+ * This structure provides frequency range(s) that should be avoided
+ * because of interference.
+ *
+ * @interference_source: enum nl80211_freq_interference_source_type
+ * is used to specify source of interference.
+ * @start_freq: start of frequency range. Frequency is specified in
+ * KHz and is center frequency in 20 MHz channel bandwidth
+ * @end_freq: end of frequency range. Frequency is specified in KHz
+ * and is center frequency in 20 MHz channel bandwidth
+ */
+struct cfg80211_avoid_frequency_range {
+ enum nl80211_freq_interference_source_type interference_source;
+ unsigned int start_freq;
+ unsigned int end_freq;
+};
+
+struct cfg80211_avoid_frequency_list {
+ unsigned int n_freq_ranges;
+ struct cfg80211_avoid_frequency_range *freq_range[0];
+};
+
+/**
* struct cfg80211_ops - backend description for wireless configuration
*
* This struct is registered by fullmac card drivers and/or wireless stacks
@@ -4349,6 +4373,18 @@ void cfg80211_ft_event(struct net_device *netdev,
struct cfg80211_ft_event_params *ft_event);

/**
+ * cfg80211_avoid_frequency_event - frequency interference event
+ * @netdev: network device
+ * @freq_list: frequency range(s) information
+ * @gfp: allocation flags
+ *
+ * Wireless driver calls this function to notify userspace about frequency
+ * range(s) that should be avoided because of interference.
+ */
+void cfg80211_avoid_frequency_event(struct net_device *netdev,
+ struct cfg80211_avoid_frequency_list *freq_list, gfp_t gfp);
+
+/**
* cfg80211_get_p2p_attr - find and copy a P2P attribute from IE buffer
* @ies: the input IE buffer
* @len: the input length
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index fde2c02..32b7841 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -686,6 +686,14 @@
* other station that transmission must be blocked until the channel
* switch is complete.
*
+ * @NL80211_CMD_AVOID_FREQUENCIES_EVENT: Send range(s) of interfering
+ * frequencies that should be avoided, from the wireless driver to the
+ * user space. If SoftAP/P2P-GO is operating on interfering frequency
+ * then user space should stop and resart them avoiding interfering
+ * frequency range(s). User space may decide to continue operation on
+ * interfering frequency, but in such case, there might be impact on
+ * performance.
+ *
* @NL80211_CMD_MAX: highest used command number
* @__NL80211_CMD_AFTER_LAST: internal use
*/
@@ -853,6 +861,8 @@ enum nl80211_commands {

NL80211_CMD_CHANNEL_SWITCH,

+ NL80211_CMD_AVOID_FREQUENCIES_EVENT,
+
/* add new commands above here */

/* used to define NL80211_CMD_MAX below */
@@ -1496,6 +1506,8 @@ enum nl80211_commands {
* @NL80211_ATTR_RXMGMT_FLAGS: flags for nl80211_send_mgmt(), u32.
* As specified in the &enum nl80211_rxmgmt_flags.
*
+ * @NL80211_ATTR_AVOID_FREQUENCIES: Avoid frequencies container information
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1806,6 +1818,8 @@ enum nl80211_attrs {

NL80211_ATTR_RXMGMT_FLAGS,

+ NL80211_ATTR_AVOID_FREQUENCIES,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
@@ -2255,6 +2269,40 @@ enum nl80211_frequency_attr {
#define NL80211_FREQUENCY_ATTR_MAX_TX_POWER NL80211_FREQUENCY_ATTR_MAX_TX_POWER

/**
+ * enum nl80211_freq_interference_source_type - interference source type
+ * @NL80211_FREQUENCY_INTERFERENCE_SOURCE_CELLULAR: interference source
+ * is cellular
+ */
+enum nl80211_freq_interference_source_type {
+ NL80211_FREQUENCY_INTERFERENCE_SOURCE_CELLULAR,
+};
+
+/**,
+ * enum nl80211_avoid_frequency_attr - avoid frequency attributes
+ * @__NL80211_FREQUENCY_ATTR_INVALID: attribute number 0 is reserved
+ * @NL80211_AVOID_FREQUENCY_ATTR_INTERFERENCE_SOURCE_TYPE: interference
+ * source
+ * @NL80211_AVOID_FREQUENCY_ATTR_START_FREQ: Start of frequency range.
+ * Frequency is specified in KHz and is center frequency in 20 MHz
+ * channel bandwidth.
+ * @NL80211_AVOID_FREQUENCY_ATTR_END_FREQ: End of frequency range.
+ * Frequency is specified in KHz and is center frequency in 20 MHz
+ * channel bandwidth.
+ * @__NL80211_FREQUENCY_ATTR_AFTER_LAST: internal use
+ */
+enum nl80211_avoid_frequency_attr {
+ __NL80211_AVOID_FREQUENCY_ATTR_INVALID,
+ NL80211_AVOID_FREQUENCY_ATTR_INTERFERENCE_SOURCE_TYPE,
+ NL80211_AVOID_FREQUENCY_ATTR_START_FREQ,
+ NL80211_AVOID_FREQUENCY_ATTR_END_FREQ,
+
+ /* keep last */
+ __NL80211_AVOID_FREQUENCY_ATTR_AFTER_LAST,
+ NL80211_AVOID_FREQUENCY_ATTR_MAX =
+ __NL80211_AVOID_FREQUENCY_ATTR_AFTER_LAST - 1
+};
+
+/**
* enum nl80211_bitrate_attr - bitrate attributes
* @__NL80211_BITRATE_ATTR_INVALID: attribute number 0 is reserved
* @NL80211_BITRATE_ATTR_RATE: Bitrate in units of 100 kbps
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index cbbef88..469116f 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -354,6 +354,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_CSA_IES] = { .type = NLA_NESTED },
[NL80211_ATTR_CSA_C_OFF_BEACON] = { .type = NLA_U16 },
[NL80211_ATTR_CSA_C_OFF_PRESP] = { .type = NLA_U16 },
+ [NL80211_ATTR_AVOID_FREQUENCIES] = { .type = NLA_NESTED },
};

/* policy for the key attributes */
@@ -11234,6 +11235,78 @@ void cfg80211_ft_event(struct net_device *netdev,
}
EXPORT_SYMBOL(cfg80211_ft_event);

+void cfg80211_avoid_frequency_event(struct net_device *netdev,
+ struct cfg80211_avoid_frequency_list *freq_list, gfp_t gfp)
+{
+ struct wiphy *wiphy;
+ struct cfg80211_registered_device *rdev;
+ struct sk_buff *msg;
+ void *hdr;
+ struct nlattr *nl_ranges, *nl_range;
+ int err = -EINVAL;
+ unsigned int i;
+
+ if (!netdev || !netdev->ieee80211_ptr ||
+ !netdev->ieee80211_ptr->wiphy || !freq_list)
+ return;
+
+ wiphy = netdev->ieee80211_ptr->wiphy;
+ rdev = wiphy_to_dev(wiphy);
+
+ if (!rdev)
+ return;
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, gfp);
+ if (!msg)
+ return;
+
+ hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_AVOID_FREQUENCIES_EVENT);
+ if (!hdr) {
+ nlmsg_free(msg);
+ return;
+ }
+
+ if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
+ nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex))
+ goto nla_put_failure;
+
+ nl_ranges = nla_nest_start(msg, NL80211_ATTR_AVOID_FREQUENCIES);
+ if (!nl_ranges)
+ goto nla_put_failure;
+
+ for (i = 0; i < freq_list->n_freq_ranges; i++) {
+ nl_range = nla_nest_start(msg, i);
+ if (!nl_range)
+ goto nla_put_failure;
+
+ if (nla_put_u32(msg,
+ NL80211_AVOID_FREQUENCY_ATTR_INTERFERENCE_SOURCE_TYPE,
+ freq_list->freq_range[i]->interference_source) ||
+ nla_put_u32(msg,
+ NL80211_AVOID_FREQUENCY_ATTR_START_FREQ,
+ freq_list->freq_range[i]->start_freq) ||
+ nla_put_u32(msg,
+ NL80211_AVOID_FREQUENCY_ATTR_END_FREQ,
+ freq_list->freq_range[i]->end_freq))
+ goto nla_put_failure;
+ nla_nest_end(msg, nl_range);
+ }
+ nla_nest_end(msg, nl_ranges);
+
+ err = genlmsg_end(msg, hdr);
+ if (err < 0)
+ goto nla_put_failure;
+
+ genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
+ nl80211_mlme_mcgrp.id, gfp);
+ return;
+
+nla_put_failure:
+ genlmsg_cancel(msg, hdr);
+ nlmsg_free(msg);
+}
+EXPORT_SYMBOL(cfg80211_avoid_frequency_event);
+
void cfg80211_crit_proto_stopped(struct wireless_dev *wdev, gfp_t gfp)
{
struct cfg80211_registered_device *rdev;
--
1.8.4



2013-11-15 18:53:56

by Chauhan, Rajesh

[permalink] [raw]
Subject: RE: [PATCHv3] cfg80211: add support for frequency interference event

SGkgSm9oYW5uZXMsDQoNCkNvdWxkIHlvdSBwbC4gbGV0IHVzIGtub3cgb2YgYW55IGZ1cnRoZXIg
Y29tbWVudHMvZmVlZGJhY2sgb24gcGF0Y2ggc2V0IHYzPw0KDQpSZWdhcmRzLA0KUmFqZXNoIENo
YXVoYW4NCg0KLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZyb206IG1jZ3JvZkBnbWFpbC5j
b20gW21haWx0bzptY2dyb2ZAZ21haWwuY29tXSBPbiBCZWhhbGYgT2YgTHVpcyBSLiBSb2RyaWd1
ZXoNClNlbnQ6IFRodXJzZGF5LCBOb3ZlbWJlciAxNCwgMjAxMyAxOjU0IEFNDQpUbzogQ2hhdWhh
biwgUmFqZXNoDQpDYzogSm9oYW5uZXMgQmVyZzsgbGludXgtd2lyZWxlc3M7IE1hbGluZW4sIEpv
dW5pOyBCQUhJTkksIEhlbnJpOyBKb2huc29uLCBKZWZmOyBDaGFuZywgTGVvOyBMdW8sIFh1bjsg
VGhhbGFwcGlsLCBTYW1lZXI7IEh1c3NhaW4sIEFyaWYNClN1YmplY3Q6IFJlOiBbUEFUQ0h2M10g
Y2ZnODAyMTE6IGFkZCBzdXBwb3J0IGZvciBmcmVxdWVuY3kgaW50ZXJmZXJlbmNlIGV2ZW50DQoN
Ck9uIE1vbiwgTm92IDExLCAyMDEzIGF0IDg6MzIgUE0sIFJhamVzaCBDaGF1aGFuIDxyYWplc2hj
QHFjYS5xdWFsY29tbS5jb20+IHdyb3RlOg0KPiBBZGQgc3VwcG9ydCBmb3Igd2lyZWxlc3MgZHJp
dmVyIHRvIHJlcG9ydCBmcmVxdWVuY3kgcmFuZ2UocykgdG8gYmUgDQo+IGF2b2lkZWQgYmVjYXVz
ZSBvZiBpbnRlcmZlcmVuY2UuIFdpcmVsZXNzIGRyaXZlciBzcGVjaWZpZXMgc291cmNlIG9mIA0K
PiBpbnRlcmZlcmVuY2UgZm9yIGEgZ2l2ZW4gZnJlcXVlbmN5IHJhbmdlLiBGb3Igbm93LCBhcHBs
aWNhYmxlIHNvdXJjZSANCj4gb2YgaW50ZXJmZXJlbmNlIGlzICJjZWxsdWxhciIgb25seSBhbmQg
b3RoZXIgaW50ZXJmZXJlbmNlIHNvdXJjZSB0eXBlcyANCj4gY2FuIGJlIGFkZGVkIGxhdGVyLiBJ
ZiBTb2Z0QVAvUDJQLUdPIGlzIG9wZXJhdGluZyBvbiBpbnRlcmZlcmluZyANCj4gZnJlcXVlbmN5
IHRoZW4gdXNlciBzcGFjZSBzaG91bGQgc3RvcCBhbmQgcmVzdGFydCB0aGVtIGF2b2lkaW5nIA0K
PiBpbnRlcmZlcmluZyBmcmVxdWVuY3kgcmFuZ2UocykuIFVzZXIgc3BhY2UgbWF5IGRlY2lkZSB0
byBjb250aW51ZSANCj4gb3BlcmF0aW9uIG9uIGludGVyZmVyaW5nIGZyZXF1ZW5jeSwgYnV0IGlu
IHN1Y2ggY2FzZSwgdGhlcmUgbWlnaHQgYmUgDQo+IGltcGFjdCBvbiBwZXJmb3JtYW5jZS4NCj4N
Cj4gU2lnbmVkLW9mZi1ieTogUmFqZXNoIENoYXVoYW4gPHJhamVzaGNAcWNhLnF1YWxjb21tLmNv
bT4NCg0KUmV2aWV3ZWQtYnk6IEx1aXMgUi4gUm9kcmlndWV6IDxtY2dyb2ZAZG8tbm90LXBhbmlj
LmNvbT4NCg0KICBMdWlzDQo=

2013-11-14 09:54:38

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCHv3] cfg80211: add support for frequency interference event

On Mon, Nov 11, 2013 at 8:32 PM, Rajesh Chauhan
<[email protected]> wrote:
> Add support for wireless driver to report frequency range(s) to
> be avoided because of interference. Wireless driver specifies
> source of interference for a given frequency range. For now,
> applicable source of interference is "cellular" only and other
> interference source types can be added later. If SoftAP/P2P-GO
> is operating on interfering frequency then user space should stop
> and restart them avoiding interfering frequency range(s). User
> space may decide to continue operation on interfering frequency,
> but in such case, there might be impact on performance.
>
> Signed-off-by: Rajesh Chauhan <[email protected]>

Reviewed-by: Luis R. Rodriguez <[email protected]>

Luis

2013-11-17 09:17:23

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv3] cfg80211: add support for frequency interference event

On Fri, 2013-11-15 at 18:53 +0000, Chauhan, Rajesh wrote:
> Hi Johannes,
>
> Could you pl. let us know of any further comments/feedback on patch set v3?

Trying to push me will make it likely that you get to drop to the bottom
of the pile. I have more than one thing to look at, as you may have
noticed.

johannes


2013-12-20 08:37:36

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCHv3] cfg80211: add support for frequency interference event

On Fri, Dec 20, 2013 at 12:30 AM, BAHINI, Henri
<[email protected]> wrote:
> Please do educate us a bit!
> We were not aware of the requirement that all patches contributions to the cfg/nl have to be for driver
> that are already up streamed.

Its not and this will depend on the driver / approach / and intent but
it does help.

> As for our effort for upstreaming our driver, we should take this out of this context since there
> are factors driving this that are completely orthogonal to this discussion.

That's true but it makes it harder for provide examples of the patches
you are introducing. Typically we just remove code that's unused. If
you have no intent on upstreaming then what's the point of doing tons
of review and consideration on API that we deeply care of for
userspace? The patch is debatable as-is already, I've provided my
feedback on it before and if you don't get good feedback from others
and also if some of this is because you are not upstreaming then I'm
trying to highlight for you that perhaps that should be of a little
more importance to you.

Luis

2013-12-20 07:44:10

by Chauhan, Rajesh

[permalink] [raw]
Subject: RE: [PATCHv3] cfg80211: add support for frequency interference event

SGVsbG8gSm9oYW5uZXMsDQoNClBsZWFzZSBhbGxvdyBtZSB0byBjbGFyaWZ5IGhlcmUuIA0KDQpT
byB0aGUgZmllbGQgImludGVyZmVyZW5jZV9zb3VyY2UiICBpbiB0aGUgc3RydWN0ICJjZmc4MDIx
MV9hdm9pZF9mcmVxdWVuY3lfcmFuZ2UiIGlzIG5vdCBiZWluZyBlZmZlY3RpdmVseSB1c2VkIGN1
cnJlbnRseS4gVGhhdCBmaWVsZCB3YXMgYWRkZWQgcGVyIHN1Z2dlc3Rpb24gZnJvbSBMdWlzIGZv
ciBmdXR1cmUgdXNlLiBCdXQgaWYgeW91IGFyZSBhZHZpc2luZyB0byByZW1vdmUgaXQgdGhlbiB5
ZXMgd2UgY2FuIHJlbW92ZSB0aGF0IGZpZWxkLg0KDQpUaGUgQVBJIGNmZzgwMjExX2F2b2lkX2Zy
ZXF1ZW5jeV9ldmVudCgpIHdoaWNoIGlzIGludHJvZHVjZWQgaW4gdGhpcyBwYXRjaCBzaGFsbCBi
ZSB1c2VkIGJ5IG91ciBXTEFOIGRyaXZlciB3aGljaCBpcyBhbiBvcGVuIHNvdXJjZSBkcml2ZXIu
IFdMQU4gZHJpdmVyIHNoYWxsIGludm9rZSB0aGlzIGV2ZW50IEFQSSB0byBwYXNzIGZyZXF1ZW5j
eSBpbmZvcm1hdGlvbiB0byBXUEEgU3VwcGxpY2FudC4gV2UgYWxzbyBoYXZlIGEgcGF0Y2ggcmVh
ZHkgZm9yIFdQQSBzdXBwbGljYW50IHdoaWNoIEpvdW5pIHdvdWxkIHVwc3RyZWFtIG9uY2Uga2Vy
bmVsIHBhdGNoIGlzIGJsZXNzZWQgYW5kIHVwIHN0cmVhbWVkLg0KDQpJIHdpbGwgcmVtb3ZlIGZp
ZWxkICJpbnRlcmZlcmVuY2Vfc291cmNlIiBhbmQgc2VuZCByZXZpc2VkIHBhdGNoIGFuZCBhbHNv
IGluY29ycG9yYXRlIHlvdXIgb3RoZXIgY29tbWVudHMuIFBsLiBkbyBsZXQgdXMga25vdyBpZiB5
b3Ugc3RpbGwgc2VlIGFueSBpc3N1ZS4NCg0KUmVnYXJkcywNClJhamVzaCBDaGF1aGFuDQoNCg0K
LS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZyb206IEpvaGFubmVzIEJlcmcgW21haWx0bzpq
b2hhbm5lc0BzaXBzb2x1dGlvbnMubmV0XSANClNlbnQ6IFdlZG5lc2RheSwgRGVjZW1iZXIgMDQs
IDIwMTMgMTI6MjEgQU0NClRvOiBMdWlzIFIuIFJvZHJpZ3Vleg0KQ2M6IENoYXVoYW4sIFJhamVz
aDsgbGludXgtd2lyZWxlc3M7IE1hbGluZW4sIEpvdW5pOyBCQUhJTkksIEhlbnJpOyBKb2huc29u
LCBKZWZmOyBDaGFuZywgTGVvOyBMdW8sIFh1bjsgVGhhbGFwcGlsLCBTYW1lZXI7IEh1c3NhaW4s
IEFyaWYNClN1YmplY3Q6IFJlOiBbUEFUQ0h2M10gY2ZnODAyMTE6IGFkZCBzdXBwb3J0IGZvciBm
cmVxdWVuY3kgaW50ZXJmZXJlbmNlIGV2ZW50DQoNCk9uIFdlZCwgMjAxMy0xMi0wNCBhdCAwOToy
MCArMDEwMCwgSm9oYW5uZXMgQmVyZyB3cm90ZToNCj4gT24gVHVlLCAyMDEzLTEyLTAzIGF0IDE3
OjM1ICswMTAwLCBMdWlzIFIuIFJvZHJpZ3VleiB3cm90ZToNCj4gPiBPbiBUdWUsIERlYyAzLCAy
MDEzIGF0IDI6MjUgUE0sIEpvaGFubmVzIEJlcmcgPGpvaGFubmVzQHNpcHNvbHV0aW9ucy5uZXQ+
IHdyb3RlOg0KPiA+ID4+ICsgKiBAaW50ZXJmZXJlbmNlX3NvdXJjZTogZW51bSBubDgwMjExX2Zy
ZXFfaW50ZXJmZXJlbmNlX3NvdXJjZV90eXBlDQo+ID4gPj4gKyAqICAgaXMgdXNlZCB0byBzcGVj
aWZ5IHNvdXJjZSBvZiBpbnRlcmZlcmVuY2UuDQo+ID4gPg0KPiA+ID4gRG8gd2UgZXhwZWN0IHRo
YXQgZGlmZmVyZW50IHNvdXJjZXMgd291bGQgYmUgdHJlYXRlZCBkaWZmZXJlbnRseT8gDQo+ID4g
PiBJZiBub3QsIHdoYXQgdXNlIGlzIHRoaXM/DQo+ID4gDQo+ID4gVGhhdCB3YXMgYmVjYXVzZSBv
ZiBteSBuYWdnaW5nLCB3aXRob3V0IHN1Y2ggYSBmaWVsZCB0aGUgdHlwZSBvZiANCj4gPiBpbnRl
cmZlcmVuY2Ugd291bGQgYmUgY29tcGxldGVseSBhbWJpZ3VvdXMgYW5kIHdlJ2QgaGF2ZSBubyB1
c2VycyBvbiANCj4gPiB0aGUgTGludXgga2VybmVsIG9mIHRoaXMgQVBJLA0KPiANCj4gV2FpdCwg
d2UgaGF2ZSBubyB1c2VycyBmb3IgdGhpcyBBUEk/IEZvcmdldCBpdCB0aGVuIC0gSSB0aG91Z2h0
IHlvdSANCj4gd2VyZSBnb2luZyB0byBwb3N0IGEgcGF0Y2ggc29vbi4gSSBndWVzcyBJIHNob3Vs
ZCBoYXZlIGxlYXJuZWQgYnkgbm93IA0KPiBub3QgdG8gdHJ1c3QgUUNBIHdpdGggdGhpcy4NCj4g
DQo+IEknbGwgZHJvcCB0aGlzIHVudGlsIEkgc2VlIGEgcGF0Y2ggdXNpbmcgaXQuDQoNCllvdSBj
YW4gc2VlIHRoaXMgYXMgYSBjYXJyb3QgKG9yIHN0aWNrLCBkZXBlbmRpbmcgb24gaG93IHlvdSBs
b29rIGF0IGl0KSBmb3IgdXBzdHJlYW1pbmcgdGhlIGRyaXZlciB0aGF0IHdhbnRzIGl0Lg0KDQpq
b2hhbm5lcw0KDQo=

2013-12-20 08:30:50

by BAHINI, Henri

[permalink] [raw]
Subject: RE: [PATCHv3] cfg80211: add support for frequency interference event

UGxlYXNlIGRvIGVkdWNhdGUgdXMgYSBiaXQhDQpXZSB3ZXJlIG5vdCBhd2FyZSBvZiB0aGUgcmVx
dWlyZW1lbnQgdGhhdCBhbGwgcGF0Y2hlcyBjb250cmlidXRpb25zIHRvIHRoZSBjZmcvbmwgaGF2
ZSB0byBiZSBmb3IgZHJpdmVyIHRoYXQgYXJlIGFscmVhZHkgdXAgc3RyZWFtZWQuDQoNCkFzIGZv
ciBvdXIgZWZmb3J0IGZvciB1cHN0cmVhbWluZyBvdXIgZHJpdmVyLCB3ZSBzaG91bGQgdGFrZSB0
aGlzIG91dCBvZiB0aGlzIGNvbnRleHQgc2luY2UgdGhlcmUgYXJlIGZhY3RvcnMgZHJpdmluZyB0
aGlzIHRoYXQgYXJlIGNvbXBsZXRlbHkgb3J0aG9nb25hbCB0byB0aGlzIGRpc2N1c3Npb24uDQoN
ClRoYW5rcywNCi1IZW5yaQ0KLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZyb206IG1jZ3Jv
ZkBnbWFpbC5jb20gW21haWx0bzptY2dyb2ZAZ21haWwuY29tXSBPbiBCZWhhbGYgT2YgTHVpcyBS
LiBSb2RyaWd1ZXoNClNlbnQ6IEZyaWRheSwgRGVjZW1iZXIgMjAsIDIwMTMgMTI6MjYgQU0NClRv
OiBCQUhJTkksIEhlbnJpDQpDYzogQ2hhdWhhbiwgUmFqZXNoOyBKb2hhbm5lcyBCZXJnOyBsaW51
eC13aXJlbGVzczsgTWFsaW5lbiwgSm91bmk7IEpvaG5zb24sIEplZmY7IENoYW5nLCBMZW87IEx1
bywgWHVuOyBUaGFsYXBwaWwsIFNhbWVlcjsgSHVzc2FpbiwgQXJpZg0KU3ViamVjdDogUmU6IFtQ
QVRDSHYzXSBjZmc4MDIxMTogYWRkIHN1cHBvcnQgZm9yIGZyZXF1ZW5jeSBpbnRlcmZlcmVuY2Ug
ZXZlbnQNCg0KT24gRnJpLCBEZWMgMjAsIDIwMTMgYXQgMTI6MTYgQU0sIEJBSElOSSwgSGVucmkg
PGhiYWhpbmlAcWNhLnF1YWxjb21tLmNvbT4gd3JvdGU6DQo+IEx1aXMtDQo+ICAgQ2FuIHdlIGJl
IGEgYml0IHByb2Zlc3Npb25hbCBoZXJlIGFuZCBsaW1pdCB0aGUgZGlzY3Vzc2lvbiB0byB0aGUg
cGF0Y2ggdGhhdCB3ZSdyZSBpbnRyb2R1Y2luZy4NCj4gICBSYWplc2ggZG9lcyBub3QgZGVjaWRl
IHdoZW4gdGhlIGRyaXZlciBnZXRzIHVwIHN0cmVhbWVkLg0KDQpMZXQncyBiZSBwcm9mZXNzaW9u
YWwuIFdobyB0aGUgZnVjayBkZWNpZGVzIHRvIHRyeSB0byBpbnRyb2R1Y2UgQVBJcyBmb3Igc2hp
dCBmb3IgZHJpdmVycyB0aGF0IGFyZSBub3QgdXBzdHJlYW0gPw0KDQo+ICBDYW4gd2UgZ2V0IHRo
aXMgZGlzY3Vzc2lvbiBvdXRzaWRlIG9mIHRoZSBjb250ZXh0IG9mIHRoaXMgcGF0Y2g/DQoNCldo
YXQgdHlwZSBvZiBzZXJpb3VzIGVmZm9ydCBhcmUgeW91IHB1dHRpbmcgaW50byBnZXR0aW5nIHlv
dXIgZHJpdmVyIHVwc3RyZWFtPyBPdGhlcndpc2UgaXQgc2VlbXMgdG8gbWUgbGlrZSBhIHdhc3Rl
IG9mIG15IGFuZCBvdGhlcidzIG93biB0aW1lIGZvciByZXZpZXcuDQoNCiAgTHVpcw0K

2013-12-20 08:45:20

by BAHINI, Henri

[permalink] [raw]
Subject: RE: [PATCHv3] cfg80211: add support for frequency interference event

SSBkbyBhcHByZWNpYXRlIHlvdXIgZmVlZGJhY2sgYW5kIHVuZGVyc3RhbmRpbmcsIEx1aXMuDQoN
ClBsZWFzZSBjb25zaWRlciB0aGF0IHRoZSB3Y24zNnh4IGRyaXZlciB0aGF0IGlzIGFscmVhZHkg
dXAgc3RyZWFtZWQgd2lsbCBtYWtlIHVzZWQgb2YgdGhpcyBBUEkgaW4gZHVlIGNvdXJzZTogaXQg
aXMgYWxyZWFkeSBpbiB0aGUgcGxhbm5pbmcgaW4gdGhlIGNvdXJzZSBvZiBmaXJzdCBoYWxmIG9m
IG5leHQgeWVhciBhcyB3ZSBpbnRlbnNpZnkgdGhlIHB1c2ggb2Ygc29tZSBvZiBvdXIgZmVhdHVy
ZXMgdG8gdGhpcyBkcml2ZXIuDQpQbGVhc2UgYWxzbyBjb25zaWRlciB0aGF0IHRoZSBjb3JyZXNw
b25kaW5nIGNoYW5nZXMgaW4gdGhlIHN1cHBsaWNhbnQgd2lsbCBiZSBwaWNrZWQgdXAgYnkgSm91
bmkncyB0cmVlIGFzIHNvb24gYXMgdGhpcyBjZmc4MDIxMSBBUEkgaXMgdXAgc3RyZWFtZWQuIEpv
dW5pIGNhbiB2b3VjaCBmb3IgdGhpcy4NCg0KV2l0aCBhYm92ZSwgSSBhcHByZWNpYXRlIHlvdSBn
dXlzIGNvbnNpZGVyaW5nIHRoaXMgQVBJLg0KDQpUaGFua3MsDQotSGVucmkgDQo=

2013-12-04 08:20:09

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv3] cfg80211: add support for frequency interference event

On Tue, 2013-12-03 at 17:35 +0100, Luis R. Rodriguez wrote:
> On Tue, Dec 3, 2013 at 2:25 PM, Johannes Berg <[email protected]> wrote:
> >> + * @interference_source: enum nl80211_freq_interference_source_type
> >> + * is used to specify source of interference.
> >
> > Do we expect that different sources would be treated differently? If
> > not, what use is this?
>
> That was because of my nagging, without such a field the type of
> interference would be completely ambiguous and we'd have no users on
> the Linux kernel of this API,

Wait, we have no users for this API? Forget it then - I thought you were
going to post a patch soon. I guess I should have learned by now not to
trust QCA with this.

I'll drop this until I see a patch using it.

> making anyone wonder WTF this is used
> for, except for those poking on some random non upstream driver.
> Additionally from a technical perspective having this information is
> purely informative at this point given that hostapd would be expected
> to be treating the cellular based source of interference as avoidance
> hints. It does leave open, for example, drivers with other types of
> future sources of advisories to simply piggy on top of this when the
> source of interference is coming from the 802.11 devices somehow.

I think we could also just document the kind of things it should be used
for, rather than having a useless (and potentially fake, since nobody
cares) value.

johannes


2013-12-03 13:25:33

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv3] cfg80211: add support for frequency interference event

On Mon, 2013-11-11 at 11:32 -0800, Rajesh Chauhan wrote:

> + * This structure provides frequency range(s) that should be avoided
> + * because of interference.

extra tab that should be a space only

> + * @interference_source: enum nl80211_freq_interference_source_type
> + * is used to specify source of interference.

Do we expect that different sources would be treated differently? If
not, what use is this?

> + * @start_freq: start of frequency range. Frequency is specified in
> + * KHz and is center frequency in 20 MHz channel bandwidth
> + * @end_freq: end of frequency range. Frequency is specified in KHz
> + * and is center frequency in 20 MHz channel bandwidth

Why would those be required to be center frequencies of some wireless
channels? That doesn't make much sense, e.g. if you have raw
interference data from a modem, no?

> /**
> + * cfg80211_avoid_frequency_event - frequency interference event
> + * @netdev: network device
> + * @freq_list: frequency range(s) information
> + * @gfp: allocation flags
> + *
> + * Wireless driver calls this function to notify userspace about frequency
> + * range(s) that should be avoided because of interference.
> + */
> +void cfg80211_avoid_frequency_event(struct net_device *netdev,
> + struct cfg80211_avoid_frequency_list *freq_list, gfp_t gfp);

Indentation. Also, that freq_list parameter should probably be const?

> + * @NL80211_CMD_AVOID_FREQUENCIES_EVENT: Send range(s) of interfering
> + * frequencies that should be avoided, from the wireless driver to the
> + * user space. If SoftAP/P2P-GO is operating on interfering frequency
> + * then user space should stop and resart them avoiding interfering

typo: restart

> + * frequency range(s). User space may decide to continue operation on
> + * interfering frequency, but in such case, there might be impact on
> + * performance.

Also, I think describing policy here ("userspace should stop and
restart") is wrong. This should be up to userspace. It could
alternatively do a real CSA for example.

> + * @NL80211_ATTR_AVOID_FREQUENCIES: Avoid frequencies container
> information

That should probably describe in more detail how the information in it
is formatted.

> +/**,
> + * enum nl80211_avoid_frequency_attr - avoid frequency attributes
> + * @__NL80211_FREQUENCY_ATTR_INVALID: attribute number 0 is reserved
> + * @NL80211_AVOID_FREQUENCY_ATTR_INTERFERENCE_SOURCE_TYPE: interference
> + * source
> + * @NL80211_AVOID_FREQUENCY_ATTR_START_FREQ: Start of frequency range.
> + * Frequency is specified in KHz and is center frequency in 20 MHz
> + * channel bandwidth.
> + * @NL80211_AVOID_FREQUENCY_ATTR_END_FREQ: End of frequency range.
> + * Frequency is specified in KHz and is center frequency in 20 MHz
> + * channel bandwidth.
> + * @__NL80211_FREQUENCY_ATTR_AFTER_LAST: internal use

missing docs for the other remaining enum members


> +void cfg80211_avoid_frequency_event(struct net_device *netdev,
> + struct cfg80211_avoid_frequency_list *freq_list, gfp_t gfp)

indentation

> + struct wiphy *wiphy;
> + struct cfg80211_registered_device *rdev;
> + struct sk_buff *msg;
> + void *hdr;
> + struct nlattr *nl_ranges, *nl_range;
> + int err = -EINVAL;

wtf? that value is never used - don't randomly initialize variables. Is
the variable itself even used?

> + hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_AVOID_FREQUENCIES_EVENT);
> + if (!hdr) {
> + nlmsg_free(msg);
> + return;
> + }
> +
> + if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
> + nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex))
> + goto nla_put_failure;

wdev index is missing, however, see above.

> + for (i = 0; i < freq_list->n_freq_ranges; i++) {
> + nl_range = nla_nest_start(msg, i);

0 is invalid, I'm pretty sure.

> + genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
> + nl80211_mlme_mcgrp.id, gfp);

This is no longer the right API

johannes


2013-12-03 16:35:50

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCHv3] cfg80211: add support for frequency interference event

On Tue, Dec 3, 2013 at 2:25 PM, Johannes Berg <[email protected]> wrote:
>> + * @interference_source: enum nl80211_freq_interference_source_type
>> + * is used to specify source of interference.
>
> Do we expect that different sources would be treated differently? If
> not, what use is this?

That was because of my nagging, without such a field the type of
interference would be completely ambiguous and we'd have no users on
the Linux kernel of this API, making anyone wonder WTF this is used
for, except for those poking on some random non upstream driver.
Additionally from a technical perspective having this information is
purely informative at this point given that hostapd would be expected
to be treating the cellular based source of interference as avoidance
hints. It does leave open, for example, drivers with other types of
future sources of advisories to simply piggy on top of this when the
source of interference is coming from the 802.11 devices somehow.

Luis

2013-12-03 15:06:47

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv3] cfg80211: add support for frequency interference event

On Tue, 2013-12-03 at 14:25 +0100, Johannes Berg wrote:

> > + if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
> > + nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex))
> > + goto nla_put_failure;
>
> wdev index is missing, however, see above.

Forgot add the "see above" part, so I'll just put it here:

I don't think this API makes sense per netdev.

johannes


2013-12-20 08:16:03

by BAHINI, Henri

[permalink] [raw]
Subject: RE: [PATCHv3] cfg80211: add support for frequency interference event

THVpcy0NCiAgQ2FuIHdlIGJlIGEgYml0IHByb2Zlc3Npb25hbCBoZXJlIGFuZCBsaW1pdCB0aGUg
ZGlzY3Vzc2lvbiB0byB0aGUgcGF0Y2ggdGhhdCB3ZSdyZSBpbnRyb2R1Y2luZy4NCiAgUmFqZXNo
IGRvZXMgbm90IGRlY2lkZSB3aGVuIHRoZSBkcml2ZXIgZ2V0cyB1cCBzdHJlYW1lZC4gQ2FuIHdl
IGdldCB0aGlzIGRpc2N1c3Npb24gb3V0c2lkZSBvZiB0aGUgY29udGV4dCBvZiB0aGlzIHBhdGNo
Pw0KDQpUaGFua3MsDQotSGVucmkgDQotLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KRnJvbTog
bWNncm9mQGdtYWlsLmNvbSBbbWFpbHRvOm1jZ3JvZkBnbWFpbC5jb21dIE9uIEJlaGFsZiBPZiBM
dWlzIFIuIFJvZHJpZ3Vleg0KU2VudDogRnJpZGF5LCBEZWNlbWJlciAyMCwgMjAxMyAxMjoxMiBB
TQ0KVG86IENoYXVoYW4sIFJhamVzaA0KQ2M6IEpvaGFubmVzIEJlcmc7IGxpbnV4LXdpcmVsZXNz
OyBNYWxpbmVuLCBKb3VuaTsgQkFISU5JLCBIZW5yaTsgSm9obnNvbiwgSmVmZjsgQ2hhbmcsIExl
bzsgTHVvLCBYdW47IFRoYWxhcHBpbCwgU2FtZWVyOyBIdXNzYWluLCBBcmlmDQpTdWJqZWN0OiBS
ZTogW1BBVENIdjNdIGNmZzgwMjExOiBhZGQgc3VwcG9ydCBmb3IgZnJlcXVlbmN5IGludGVyZmVy
ZW5jZSBldmVudA0KDQpPbiBUaHUsIERlYyAxOSwgMjAxMyBhdCAxMTo0NCBQTSwgQ2hhdWhhbiwg
UmFqZXNoIDxyYWplc2hjQHFjYS5xdWFsY29tbS5jb20+IHdyb3RlOg0KPiBUaGUgQVBJIGNmZzgw
MjExX2F2b2lkX2ZyZXF1ZW5jeV9ldmVudCgpIHdoaWNoIGlzIGludHJvZHVjZWQgaW4gdGhpcyBw
YXRjaCBzaGFsbCBiZSB1c2VkIGJ5IG91ciBXTEFOIGRyaXZlciB3aGljaCBpcyBhbiBvcGVuIHNv
dXJjZSBkcml2ZXIuDQoNCkdldCB5b3VyIHNoaXQgdXBzdHJlYW0uIEZvciBmdWNrJ3Mgc2FrZS4N
Cg0KICBMdWlzDQo=

2013-12-20 08:26:27

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCHv3] cfg80211: add support for frequency interference event

On Fri, Dec 20, 2013 at 12:16 AM, BAHINI, Henri
<[email protected]> wrote:
> Luis-
> Can we be a bit professional here and limit the discussion to the patch that we're introducing.
> Rajesh does not decide when the driver gets up streamed.

Let's be professional. Who the fuck decides to try to introduce APIs
for shit for drivers that are not upstream ?

> Can we get this discussion outside of the context of this patch?

What type of serious effort are you putting into getting your driver
upstream? Otherwise it seems to me like a waste of my and other's own
time for review.

Luis

2013-12-20 08:11:56

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCHv3] cfg80211: add support for frequency interference event

On Thu, Dec 19, 2013 at 11:44 PM, Chauhan, Rajesh
<[email protected]> wrote:
> The API cfg80211_avoid_frequency_event() which is introduced in this patch shall be used by our WLAN driver which is an open source driver.

Get your shit upstream. For fuck's sake.

Luis

2013-12-04 08:20:44

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv3] cfg80211: add support for frequency interference event

On Wed, 2013-12-04 at 09:20 +0100, Johannes Berg wrote:
> On Tue, 2013-12-03 at 17:35 +0100, Luis R. Rodriguez wrote:
> > On Tue, Dec 3, 2013 at 2:25 PM, Johannes Berg <[email protected]> wrote:
> > >> + * @interference_source: enum nl80211_freq_interference_source_type
> > >> + * is used to specify source of interference.
> > >
> > > Do we expect that different sources would be treated differently? If
> > > not, what use is this?
> >
> > That was because of my nagging, without such a field the type of
> > interference would be completely ambiguous and we'd have no users on
> > the Linux kernel of this API,
>
> Wait, we have no users for this API? Forget it then - I thought you were
> going to post a patch soon. I guess I should have learned by now not to
> trust QCA with this.
>
> I'll drop this until I see a patch using it.

You can see this as a carrot (or stick, depending on how you look at it)
for upstreaming the driver that wants it.

johannes


2014-01-06 16:38:14

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv3] cfg80211: add support for frequency interference event

On Fri, 2013-12-20 at 08:30 +0000, BAHINI, Henri wrote:
> Please do educate us a bit!
> We were not aware of the requirement that all patches contributions to
> the cfg/nl have to be for driver that are already up streamed.

You're adding code that needs to be maintained, thought about when
changing other things, etc. I don't see why we as the community should
shoulder that burden if you're not even willing to upstream the code
using it. It feels like you're just dumping code that you happen to need
today over the wall.

We've never really had to even consider this situation because you're
the first to actively work on cfg80211/nl80211 while apparently not even
thinking about upstreaming your driver!

johannes