Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [PATCH 1/1] net: wireless: intersil: fix improper return value
From: Kalle Valo @ 2016-12-07 12:00 UTC (permalink / raw)
  To: Pan Bian; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <1480760572-4444-1-git-send-email-bianpan2016@163.com>

Pan Bian <bianpan2016@163.com> writes:

> Function orinoco_ioctl_commit() returns 0 (indicates success) when the
> call to orinoco_lock() fails. Thus, the return value is inconsistent with
> the execution status. It may be better to return "-EBUSY" when the call 
> to orinoco_lock() fails.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188671
>
> Signed-off-by: Pan Bian <bianpan2016@163.com>
> ---
>  drivers/net/wireless/intersil/orinoco/wext.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Please use prefix "orinoco:", more info:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#subject_name

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH] iwlegacy: make il3945_mac_ops __ro_after_init
From: Stanislaw Gruszka @ 2016-12-07 10:53 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg
In-Reply-To: <20161207063646.30969-1-johannes@sipsolutions.net>

On Wed, Dec 07, 2016 at 07:36:46AM +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> There's no need for this to be only __read_mostly, since
> it's only written in a single way depending on the module
> parameter, so that can be moved into the module's __init
> function, and the ops can be __ro_after_init.
> 
> This is a little bit safer since it means the ops can't
> be overwritten (accidentally or otherwise), which would
> otherwise cause an arbitrary function or bad pointer to
> be called.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>

^ permalink raw reply

* Re: [PATCH 1/4] cfg80211: Add support to enable or disable btcoex
From: Tamizh chelvam @ 2016-12-07 11:04 UTC (permalink / raw)
  To: Johannes Berg; +Cc: c_traja, linux-wireless, ath10k
In-Reply-To: <1480949161.31788.23.camel@sipsolutions.net>

Hi Johannes,

Thanks for the comments.

On 2016-12-05 20:16, Johannes Berg wrote:
> On Tue, 2016-11-08 at 18:45 +0530, c_traja@qti.qualcomm.com wrote:
>> From: Tamizh chelvam <c_traja@qti.qualcomm.com>
>> 
>> This patch adds support to enable or disable btcoex by
>> adding NL80211_ATTR_WIPHY_BTCOEX_ENABLE attribute in
>> NL80211_CMD_SET_WIPHY command. By default BTCOEX disabled in driver.
> 
> I think overloading SET_WIPHY even more is a mistake - why not group
> this with the new command you're introducing in the second patch?
> 
Sure.

> Also, can have a flag attribute to enable/disable, or better even
> disable when you're not setting any other parameters.

BTCOEX will be disabled by default. I will use 
NL80211_ATTR_WIPHY_BTCOEX_ENABLE
attribute to enable/disable BTCOEX.

^ permalink raw reply

* RE: [1/2] mwifiex: code rearrangement in pcie.c and sdio.c
From: Amitkumar Karwar @ 2016-12-07 10:52 UTC (permalink / raw)
  To: Kalle Valo
  Cc: linux-wireless@vger.kernel.org, Cathy Luo, Nishant Sarmukadam,
	rajatja@google.com, briannorris@google.com,
	dmitry.torokhov@gmail.com, Xinming Hu
In-Reply-To: <20161205110906.28C706034F@smtp.codeaurora.org>

SGkgS2FsbGUsDQoNCj4gRnJvbTogbGludXgtd2lyZWxlc3Mtb3duZXJAdmdlci5rZXJuZWwub3Jn
IFttYWlsdG86bGludXgtd2lyZWxlc3MtDQo+IG93bmVyQHZnZXIua2VybmVsLm9yZ10gT24gQmVo
YWxmIE9mIEthbGxlIFZhbG8NCj4gU2VudDogTW9uZGF5LCBEZWNlbWJlciAwNSwgMjAxNiA0OjM5
IFBNDQo+IFRvOiBBbWl0a3VtYXIgS2Fyd2FyDQo+IENjOiBsaW51eC13aXJlbGVzc0B2Z2VyLmtl
cm5lbC5vcmc7IENhdGh5IEx1bzsgTmlzaGFudCBTYXJtdWthZGFtOw0KPiByYWphdGphQGdvb2ds
ZS5jb207IGJyaWFubm9ycmlzQGdvb2dsZS5jb207IGRtaXRyeS50b3Jva2hvdkBnbWFpbC5jb207
DQo+IFhpbm1pbmcgSHU7IEFtaXRrdW1hciBLYXJ3YXINCj4gU3ViamVjdDogUmU6IFsxLzJdIG13
aWZpZXg6IGNvZGUgcmVhcnJhbmdlbWVudCBpbiBwY2llLmMgYW5kIHNkaW8uYw0KPiANCj4gQW1p
dGt1bWFyIEthcndhciA8YWthcndhckBtYXJ2ZWxsLmNvbT4gd3JvdGU6DQo+ID4gRnJvbTogWGlu
bWluZyBIdSA8aHV4bUBtYXJ2ZWxsLmNvbT4NCj4gPg0KPiA+IE5leHQgcGF0Y2ggaW4gdGhpcyBz
ZXJpZXMgaXMgZ29pbmcgdG8gdXNlIG13aWZpZXhfcmVhZF9yZWcoKSBpbg0KPiByZW1vdmUNCj4g
PiBoYW5kbGVycy4gVGhlIGNoYW5nZXMgaGVyZSBhcmUgcHJlcmVxdWlzaXRlcyB0byBhdm9pZCBm
b3J3YXJkDQo+ID4gZGVjbGFyYXRpb25zLg0KPiA+DQo+ID4gU2lnbmVkLW9mZi1ieTogWGlubWlu
ZyBIdSA8aHV4bUBtYXJ2ZWxsLmNvbT4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBBbWl0a3VtYXIgS2Fy
d2FyIDxha2Fyd2FyQG1hcnZlbGwuY29tPg0KPiANCj4gRmFpbGVkIHRvIGFwcGx5Og0KPiANCj4g
ZmF0YWw6IHNoYTEgaW5mb3JtYXRpb24gaXMgbGFja2luZyBvciB1c2VsZXNzDQo+IChkcml2ZXJz
L25ldC93aXJlbGVzcy9tYXJ2ZWxsL213aWZpZXgvcGNpZS5jKS4NCj4gQXBwbHlpbmc6IG13aWZp
ZXg6IGdldCByaWQgb2YgZ2xvYmFsIHVzZXJfcm1tb2QgZmxhZyBSZXBvc2l0b3J5IGxhY2tzDQo+
IG5lY2Vzc2FyeSBibG9icyB0byBmYWxsIGJhY2sgb24gMy13YXkgbWVyZ2UuDQo+IENhbm5vdCBm
YWxsIGJhY2sgdG8gdGhyZWUtd2F5IG1lcmdlLg0KPiBQYXRjaCBmYWlsZWQgYXQgMDAwMSBtd2lm
aWV4OiBnZXQgcmlkIG9mIGdsb2JhbCB1c2VyX3JtbW9kIGZsYWcNCj4gDQo+IDIgcGF0Y2hlcyBz
ZXQgdG8gQ2hhbmdlcyBSZXF1ZXN0ZWQuDQo+IA0KPiA5NDU0NDkxIFsxLzJdIG13aWZpZXg6IGNv
ZGUgcmVhcnJhbmdlbWVudCBpbiBwY2llLmMgYW5kIHNkaW8uYw0KPiA5NDU0NDkzIFsyLzJdIG13
aWZpZXg6IGdldCByaWQgb2YgZ2xvYmFsIHVzZXJfcm1tb2QgZmxhZw0KPiANCj4gLS0NCj4gaHR0
cHM6Ly9wYXRjaHdvcmsua2VybmVsLm9yZy9wYXRjaC85NDU0NDkxLw0KPiANCj4gRG9jdW1lbnRh
dGlvbiBhYm91dCBzdWJtaXR0aW5nIHdpcmVsZXNzIHBhdGNoZXMgYW5kIGNoZWNraW5nIHN0YXR1
cw0KPiBmcm9tIHBhdGNod29yazoNCj4gDQo+IGh0dHBzOi8vd2lyZWxlc3Mud2lraS5rZXJuZWwu
b3JnL2VuL2RldmVsb3BlcnMvZG9jdW1lbnRhdGlvbi9zdWJtaXR0aW5nDQo+IHBhdGNoZXMNCg0K
DQpUaGVzZSB0d28gcGF0Y2hlcyBoYXZlIGRlcGVuZGVuY3kgd2l0aCBvdGhlciBwYXRjaCBzZXJp
ZXMuIEkgd2FudCB5b3UgdG8gY29uc2lkZXIgcGF0Y2hlcyBpbiBmb2xsb3dpbmcgb3JkZXIoZmly
c3QgYmVpbmcgcmVjZW50KS4NCg0KbXdpZmlleDogc2RpbzogZml4IHVzZSBhZnRlciBmcmVlIGlz
c3VlIGZvciBzYXZlX2FkYXB0ZXINCm13aWZpZXg6IHVzZSBtb2R1bGVfKl9kcml2ZXIgaGVscGVy
IG1hY3Jvcw0KDQpbMi8yXSBtd2lmaWV4OiBnZXQgcmlkIG9mIGdsb2JhbCB1c2VyX3JtbW9kIGZs
YWcNClsxLzJdIG13aWZpZXg6IGNvZGUgcmVhcnJhbmdlbWVudCBpbiBwY2llLmMgYW5kIHNkaW8u
Yw0KDQpbdjMsNS81XSBtd2lmaWV4OiBtb3ZlIHBjaWVfd29yayBhbmQgcmVsYXRlZCB2YXJpYWJs
ZXMgaW5zaWRlIGNhcmQgLS0tLS0tLS0gVGhpcyBzZXJpZXMgY2FuIGJlIGFjY2VwdGVkIGlmIHRo
ZXJlIGFyZSBubyBmdXJ0aGVyIGNvbmNlcm5zL2NvbW1lbnRzIGZyb20gQnJpYW4vRG1pdHJ5LiAN
Clt2Myw0LzVdIG13aWZpZXg6IHdhaXQgZmlybXdhcmUgZHVtcCBjb21wbGV0ZSBkdXJpbmcgY2Fy
ZCByZW1vdmUgcHJvY2Vzcw0KW3YzLDMvNV0gbXdpZmlleDogZ2V0IHJpZCBvZiBkcnZfaW5mbyog
YWRhcHRlciB2YXJpYWJsZXMNClt2MywyLzVdIG13aWZpZXg6IGRvIG5vdCBmcmVlIGZpcm13YXJl
IGR1bXAgbWVtb3J5IGluIHNodXRkb3duX2Rydg0KW3YzLDEvNV0gbXdpZmlleDogZG9uJ3Qgd2Fp
dCBmb3IgbWFpbl9wcm9jZXNzIGluIHNodXRkb3duX2Rydg0KDQpSZWdhcmRzLA0KQW1pdGt1bWFy
DQo=

^ permalink raw reply

* RE: [PATCH] mac80211: Ensure enough headroom when forwarding mesh pkt
From: Cedric Izoard @ 2016-12-07  9:57 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless@vger.kernel.org; +Cc: Laurent Trarieux
In-Reply-To: <1481103603.4092.38.camel@sipsolutions.net>

PiA+IC0JZndkX3NrYiA9IHNrYl9jb3B5KHNrYiwgR0ZQX0FUT01JQyk7DQo+ID4gKwlpZiAoc2ti
X2hlYWRyb29tKHNrYikgPj0gbG9jYWwtPnR4X2hlYWRyb29tKQ0KPiA+ICsJCWZ3ZF9za2IgPSBz
a2JfY29weShza2IsIEdGUF9BVE9NSUMpOw0KPiA+ICsJZWxzZQ0KPiA+ICsJCWZ3ZF9za2IgPSBz
a2JfY29weV9leHBhbmQoc2tiLCBsb2NhbC0+dHhfaGVhZHJvb20sDQo+ID4gKwkJCQkJwqDCoDAs
IEdGUF9BVE9NSUMpOw0KPiANCj4gV2h5IGJvdGhlciBtYWtpbmcgdGhpcyBjb25kaXRpb25hbD8g
SXQgc2VlbXMgdGhhdCBhbHdheXMgdXNpbmcNCj4gc2tiX2NvcHlfZXhwYW5kKCkgc2hvdWxkIGJl
IHN1ZmZpY2llbnQ/IFRoZSBjb2RlIGJldHdlZW4gdGhlIHR3byAoc2tiX2NvcHksDQo+IHNrYl9j
b3B5X2V4cGFuZCkgaXMgYWxtb3N0IGlkZW50aWNhbCBhbnl3YXksIGFwYXJ0IGZyb20gdGhlIGxh
dHRlciBzZXR0aW5nDQo+IHRoZSBoZWFkcm9vbSAoYW5kIHRhaWxyb29tKS4NCg0KSW5kZWVkLCBj
YWxsaW5nIHNrYl9jb3B5X2V4cGFuZCgpIHdvdWxkIHdvcmsgaW4gYW55IGNhc2UuDQpJIHdpbGwg
c2VuZCBuZXcgcGF0Y2gNCg0KY2VkcmljDQo=

^ permalink raw reply

* [PATCH v2] mac80211: Ensure enough headroom when forwarding mesh pkt
From: Cedric Izoard @ 2016-12-07  9:59 UTC (permalink / raw)
  To: linux-wireless@vger.kernel.org

When a buffer is duplicated during MESH packet forwarding,
this patch ensures that the new buffer has enough headroom.

Signed-off-by: Cedric Izoard <cedric.izoard@ceva-dsp.com>
---
 net/mac80211/rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index d2a00f2..bfa5f4d 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2468,7 +2468,7 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx)
 	if (!ifmsh->mshcfg.dot11MeshForwarding)
 		goto out;
 
-	fwd_skb = skb_copy(skb, GFP_ATOMIC);
+	fwd_skb = skb_copy_expand(skb, local->tx_headroom, 0, GFP_ATOMIC);
 	if (!fwd_skb) {
 		net_info_ratelimited("%s: failed to clone mesh frame\n",
 				    sdata->name);
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Andrew Zaborowski @ 2016-12-07  9:49 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Denis Kenzior, linux-wireless
In-Reply-To: <1481091554.4092.7.camel@sipsolutions.net>

On 7 December 2016 at 07:19, Johannes Berg <johannes@sipsolutions.net> wrote:
>> Possibly Johanness refers to the fact that if you use
>> CMD_AUTHENTICATE, or if you use CMD_CONNECT but the driver implements
>> the SME -- doesn't use the cfg80211 software SME -- then
>> cfg80211_disconnect won't do anything if we're not associated, only
>> authenticated.  Currently cfg80211 doesn't have knowledge of whether
>> it is authenticated and where to.
>
> We used to track it, but it was a nightmare and just always buggy :)
>
>> With the software SME current_bss would be set from the moment the
>> authentication attempt starts,
>
> I'm almost certain this isn't true, what makes you think so?

Ok, I think I misread the code, indeed it looks consistent between the
driver SME and cfg80211 SME.

I'm fine only adding the socket owner flag to CMD_ASSOCIATE and
CMD_CONNECT.  I'll need to store the bssid of the association in
progress so we can send the Deauthenticate if the socket is closed
before association finishes.

>
>> so there seems to be an inconsistency
>> which would affect for example the NL80211_BSS_STATUS_ASSOCIATED
>> flags in the result of CMD_GET_SCAN.
>
> Thus this can't be the case.
>
>> Perhaps this can be fixed by always
>> setting current_bss on auth attempt start, with flags to indicate
>> whether authentication has happened and whether association happened.
>
> No! That would be wrong!
>
>> At the very least with this patch if you set the socket owner during
>> CMD_AUTHENTICATE and then separately associate, you'd get the
>> expected deauthentication.
>
> That would *NOT* be expected. There's no need to even authenticate
> through CMD_AUTHENTICATE at all to connect to (another) AP!
>
> You need to think beyond the 1996 version of 802.11 ;-)

Best regards

^ permalink raw reply

* Re: [PATCH] mac80211: Ensure enough headroom when forwarding mesh pkt
From: Johannes Berg @ 2016-12-07  9:40 UTC (permalink / raw)
  To: Cedric Izoard, linux-wireless@vger.kernel.org; +Cc: Laurent Trarieux
In-Reply-To: <356db1fcf788467f9145abc10e218d66@ceva-dsp.com>


> -	fwd_skb = skb_copy(skb, GFP_ATOMIC);
> +	if (skb_headroom(skb) >= local->tx_headroom)
> +		fwd_skb = skb_copy(skb, GFP_ATOMIC);
> +	else
> +		fwd_skb = skb_copy_expand(skb, local->tx_headroom,
> +					  0, GFP_ATOMIC);

Why bother making this conditional? It seems that always using
skb_copy_expand() should be sufficient? The code between the two
(skb_copy, skb_copy_expand) is almost identical anyway, apart from the
latter setting the headroom (and tailroom).

johannes

^ permalink raw reply

* Re: [PATCH v2 2/2] mac80211:  Show pending txqlen in debugfs.
From: Johannes Berg @ 2016-12-07  9:36 UTC (permalink / raw)
  To: greearb, linux-wireless
In-Reply-To: <1480964310-16698-2-git-send-email-greearb@candelatech.com>

On Mon, 2016-12-05 at 10:58 -0800, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> Could be useful for debugging memory consumption issues,
> and perhaps power-save as well.

Applied this one, but I tend to agree with Felix about the ethtool
stuff, so I've dropped that.

FWIW, we have cfg80211_get_station() now (maybe we could extend that
and provide an iterator as well), so instead of carrying patches you
could also have a small additional module that uses the existing APIs
to get the data, since nl80211 seems to somehow be so problematic for
you.

johannes

^ permalink raw reply

* RE: [PATCH v2 2/2] cfg80211: Add support to sched scan to report better BSSs
From: Vamsi, Krishna @ 2016-12-07  9:33 UTC (permalink / raw)
  To: Johannes Berg, Malinen, Jouni; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1481102727.4092.34.camel@sipsolutions.net>

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBKb2hhbm5lcyBCZXJnIFttYWls
dG86am9oYW5uZXNAc2lwc29sdXRpb25zLm5ldF0NCg0KPiBXaGF0IGFib3V0IEFyZW5kJ3MgY29t
bWVudCByZWdhcmRpbmcgdGhpcyBmdW5jdGlvbmFsaXR5IG92ZXJsYXBwaW5nIHdpdGggdGhlDQo+
IEJTUyBzZWxlY3Rpb24gb2ZmbG9hZCBjb25maWd1cmF0aW9uPw0KPiANCj4gRG8geW91IHRoaW5r
IHRoZXJlJ3MgYW55IGFiaWxpdHkgdG8gc2hhcmUgYXR0cmlidXRlcy9mdW5jdGlvbmFsaXR5Pw0K
DQpIaSBKb2hhbm5lcywNCg0KSSB0aGluayB0aGUgbGF0ZXIgY29tbWVudCBmcm9tIEx1Y2Egb24g
dGhpcyB3aGljaCBJIHBhc3RlZCBiZWxvdyBpcyBtb3JlIGFncmVlYWJsZS4NCg0KWWVzLCBzaW1p
bGFyIGJ1dCBub3QgcXVpdGUgdGhlIHNhbWUuICBUaGUgZXhpc3RpbmcgY2FzZSBpcyBmb3IgZmlu
ZGluZyBCU1NzIHRoYXQgYXJlIHdvcnRoIHdha2luZyB0aGUgaG9zdCB1cCBmb3IgKHdoaWxlIGRp
c2Nvbm5lY3RlZCksIHNvIGl0IG5lZWRzIHRvIGJlIGJldHRlciB0aGFuIHRoZSBSU1NJIHBhc3Nl
ZCAoYWJzb2x1dGUgbnVtYmVyKS4gIE5vdyB0aGlzIGlzIGFib3V0IHJlbGF0aXZlIFJTU0kgYXMg
Y29tcGFyZWQgdG8gdGhlIGN1cnJlbnQgY29ubmVjdGlvbiwgc28gUkVMQVRJVkVfUlNTSSBpcyBk
aWZmZXJlbnQgdGhhbiBSU1NJIGFuZCBJIHRoaW5rIHRoZSBzYW1lIGF0dHJpYnV0ZSBzaG91bGQg
bm90IGJlIHVzZWQsIHRvIGF2b2lkIGNvbmZ1c2lvbi4NCg0KVGhhbmtzLA0KVmFtc2kNCg==

^ permalink raw reply

* Re: [PATCH v2 2/2] cfg80211: Add support to sched scan to report better BSSs
From: Johannes Berg @ 2016-12-07  9:25 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: linux-wireless, vamsi krishna
In-Reply-To: <1480715949-20169-2-git-send-email-jouni@qca.qualcomm.com>


> v2: address comments from Luca, Arend, and Johannes

What about Arend's comment regarding this functionality overlapping
with the BSS selection offload configuration?

Do you think there's any ability to share attributes/functionality?

johannes

^ permalink raw reply

* Re: [PATCH] mac80211: Remove invalid flag operations in mesh TSF synchronization
From: Johannes Berg @ 2016-12-07  9:24 UTC (permalink / raw)
  To: Thomas Pedersen, Masashi Honma; +Cc: Bob Copeland, linux-wireless
In-Reply-To: <CADjYELw=-LEqDra0pM+3FjtJDve+NiNYEMz5=F0i5Ge-5k3LaA@mail.gmail.com>

Hi,

I'm not sure what to do with this now :)

> > There are two functionalities.
> > 
> > 1) 13.13.2.2 Neighbor offset synchronization method
> > 2) 13.13.4.4 TBTT adjustment

Yes, this much is obvious.

> > The flag is updated by 2).

Yes, the definition of the flag says:

"The TBTT Adjusting subfield is set to 1 while the TBTT adjustment
procedure is ongoing, and is set to 0 otherwise. (See 13.13.4.4.3.)"

(802.11-2012 8.4.2.100.8 Mesh Capability)

> > 13.13.4.4.3 TBTT scanning and adjustment procedures:
> > The mesh STA shall set the TBTT Adjusting field in the Mesh
> > Configuration element to 1 in order to announce that the TBTT
> > adjustment procedure is ongoing.

That's saying the same thing again, I guess :)

> > And the flag is refered by 1) as you said.
> > 
> > 
> > The purpose of the flag is to prevent 1) while 2) is ongoing.
> > 
> > In other words, 1) has only read access authority to the flag.
> > However,
> > previous code updated the flag in 1). In addition, there is no code
> > for
> > 2). So I just remove the invalid accessing codes.
> 
> I don't think 1) has read only access to that flag. A TSF adjust will
> by definition move the TBTT as well.

It seems that the wording in the spec disagrees with that - it says
(twice) to set the bit only while the TBTT adjustment procedure is
ongoing, which isn't the case here?

Then again, what exactly *is* this code doing?

It's called mesh_sync_offset_adjust_tbtt() which matches more closely
"TBTT adjustment" than "neighbor offset synchronization"? The code
looks more like offset synchronization though. Perhaps there's some
confusing and it's kinda doing both?

johannes

^ permalink raw reply

* Re: [PATCH] mac80211:  fix rx-rate report when rate is invalid.
From: Johannes Berg @ 2016-12-07  9:07 UTC (permalink / raw)
  To: greearb, linux-wireless
In-Reply-To: <1480975935-12760-1-git-send-email-greearb@candelatech.com>

On Mon, 2016-12-05 at 14:12 -0800, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> This fixes obtaining the rate info via sta_set_sinfo
> when the rx rate is invalid (for instance, on IBSS
> interface that has received no frames from one of its
> peers).

Please fix this completely while at it, i.e.
make sta_set_rate_info_rx() have a return value and don't set
the BIT(NL80211_STA_INFO_RX_BITRATE) when there's no known rate.

johannes

^ permalink raw reply

* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Johannes Berg @ 2016-12-07  7:38 UTC (permalink / raw)
  To: Denis Kenzior, Andrew Zaborowski, linux-wireless
In-Reply-To: <5847B51A.6090303@gmail.com>


> > > Fair enough.  I think we can live with that since we're not using
> > > CMD_AUTHENTICATE unless we need to roam using FT.
> > 
> > That seems like a very strange decision, but it's obviously up to
> > you
> > :)
> > 
> 
> Care to elaborate ?

Well, for example, you won't be able to do SAE, the upcoming FILS or
generally have any control over what's going on in the auth/assoc
handshake, since the kernel's version thereof is really really simple.

johannes

^ permalink raw reply

* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Denis Kenzior @ 2016-12-07  7:07 UTC (permalink / raw)
  To: Johannes Berg, Andrew Zaborowski, linux-wireless
In-Reply-To: <1481093287.4092.20.camel@sipsolutions.net>

Hi Johannes,

On 12/07/2016 12:48 AM, Johannes Berg wrote:
> On Wed, 2016-12-07 at 00:40 -0600, Denis Kenzior wrote:
>>
>>> No, this is the part you didn't understand. Simply authenticating
>>> doesn't actually create anything like a "link" to the AP. The only
>>
>> Okay, but it is then a bit misleading that iw link reports us being
>> 'connected' during this time for example.
>
> It doesn't!
>
> # iw wlan0 auth <ssid> <bssid> open 2462
>
> -> iw event reporting
> wlan0 (phy #0): auth <localMAC> -> <bssid> status: 0: Successful
>
> # iw wlan0 link
> Not connected.
>

Yes, you're right.  It is iw info that shows us 'connected'

< Request: Get Interface (0x05) len 8 [ack]                 35235.316448
     Interface Index: 3 (0x00000003)
 > Result: New Interface (0x07) len 108                      35235.316455
     Interface Index: 3 (0x00000003)
     Interface Name: wlp2s0
     Wiphy: 0 (0x00000000)
     Interface Type: 2 (0x00000002)
     Wireless Device: 1 (0x0000000000000001)
     Generation: 5 (0x00000005)
     Wiphy Frequency: 2417 (0x00000971)
     Wiphy Channel Type: 1 (0x00000001)
     Channel Width: 1 (0x00000001)
     Center Frequency 1: 2417 (0x00000971)
     Wiphy TX Power Level: 2200 (0x00000898)
 > Response: Get Interface (0x05) len 4                      35235.316457
     Status: Success (0)

>
>> Fair enough.  I think we can live with that since we're not using
>> CMD_AUTHENTICATE unless we need to roam using FT.
>
> That seems like a very strange decision, but it's obviously up to you
> :)
>

Care to elaborate ?

Regards,
-Denis

^ permalink raw reply

* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Johannes Berg @ 2016-12-07  6:48 UTC (permalink / raw)
  To: Denis Kenzior, Andrew Zaborowski, linux-wireless
In-Reply-To: <5847AEF9.90409@gmail.com>

On Wed, 2016-12-07 at 00:40 -0600, Denis Kenzior wrote:
> 
> > No, this is the part you didn't understand. Simply authenticating
> > doesn't actually create anything like a "link" to the AP. The only
> 
> Okay, but it is then a bit misleading that iw link reports us being 
> 'connected' during this time for example.

It doesn't!

# iw wlan0 auth <ssid> <bssid> open 2462

-> iw event reporting
wlan0 (phy #0): auth <localMAC> -> <bssid> status: 0: Successful

# iw wlan0 link
Not connected.


> Fair enough.  I think we can live with that since we're not using 
> CMD_AUTHENTICATE unless we need to roam using FT.

That seems like a very strange decision, but it's obviously up to you
:)

johannes

^ permalink raw reply

* Re: [PATCH] RFC: Universal scan proposal
From: Johannes Berg @ 2016-12-07  6:44 UTC (permalink / raw)
  To: Dmitry Shmidt; +Cc: linux-wireless
In-Reply-To: <CAH7ZN-wcvoJLTr_zYMwpbjuvMBGwmNhuVYx-MuNU1pOPTf9HEA@mail.gmail.com>


> Indeed, results are results. I just want to take care of two things:
> 1) Memory consumption - we can clear stale scan results for
> connection, but not for location if we are using history scan.

Well eventually we also have to clear for location if we run out of
memory, that usually means dumping them out to the host, no?

> 2) Use of insufficient results for connection - in case we had
> history or hotlist scan only for very limited amount of channels,
> then we may not have enough APs in our result for "sterling"
> connection decision.

I'm not entirely sure about this case - surely noticing "we can do
better now" is still better than waiting for being able to make the
perfect decision?

> > > Report: none / batch / immediate
> > 
> > Not sure I see much point in "none"??
> > 
> > Can you define these more clearly? Do you think "batch" reporting
> > should be like the gscan buckets? Or actually have full
> > information?
> 
> None - means that there is not need to report. It can be useful
> in case of roaming scan, scheduling or hotlist scan - you didn't find
> anything suitable - don't report that there is no scan results.

But that seems more of a filtering thing, combined with "immediate" for
anything passing the filter?

> > >    Request may have priority and can be inserted into
> > > the head of the queue.
> > >    Types of scans:
> > > - Normal scan
> > > - Scheduled scan
> > > - Hotlist (BSSID scan)
> > > - Roaming
> > > - AutoJoin
> > 
> > I think somebody else said this but I didn't find it now - I think
> > this would make more sense to define in terms of expected behaviour
> > than use cases for each type of scan.
> 
> I think Luca made this statement. 

Yeah - I just couldn't find it again on re-reading the thread :)

> It is totally ok from SW point of
> view - especially due to the fact that scan is scan. However,
> I suspect it will be harder to handle from user experience. I mean
> at the end wireless framework / driver / FW will convert special
> scan type to usual scan with special params and response, but why
> to put this burden on user?

I just think it's more flexible and open-ended. The actual definition
of the resulting parameters needs to be somewhere anyway - putting it
into driver/firmware (vs. wifi framework or so) seems to duplicate it
and certainly makes it harder to modify/extend in the future, no?

johannes

^ permalink raw reply

* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Denis Kenzior @ 2016-12-07  6:40 UTC (permalink / raw)
  To: Johannes Berg, Andrew Zaborowski, linux-wireless
In-Reply-To: <1481091301.4092.5.camel@sipsolutions.net>

Hi Johannes,

On 12/07/2016 12:15 AM, Johannes Berg wrote:
>
>> I'm afraid to consider what you're like when you _are_ 'super'
>> against something :)
>
> Why, that's easy, there wouldn't be a long discussion like that ;-)

lol

>
> No, this is the part you didn't understand. Simply authenticating
> doesn't actually create anything like a "link" to the AP. The only

Okay, but it is then a bit misleading that iw link reports us being 
'connected' during this time for example.

> reason we keep the station entry around for a few seconds is that it
> *probably* will be used next to associate. But if you don't do that, or
> authenticate to some other AP, or do whatever else - nothing stops you
> from doing that. There's no connection, nothing really stays active
> except for this 5 second grace period to associate.
>
> So even if you crash here like in the example, there's nothing to clean
> up, a subsequent authentication attempt to the same or another AP will
> go through anyway.
>
> Therefore, there's nothing to "own" with an authentication attempt,
> since it doesn't actually keep any (permanent) state in the kernel, and
> keeping the station entry around is just an optimisation.
>
> I don't think it's worth trying to clean that up.

Fair enough.  I think we can live with that since we're not using 
CMD_AUTHENTICATE unless we need to roam using FT.

Regards,
-Denis

^ permalink raw reply

* [PATCH] iwlegacy: make il3945_mac_ops __ro_after_init
From: Johannes Berg @ 2016-12-07  6:36 UTC (permalink / raw)
  To: linux-wireless; +Cc: Stanislaw Gruszka, Johannes Berg

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

There's no need for this to be only __read_mostly, since
it's only written in a single way depending on the module
parameter, so that can be moved into the module's __init
function, and the ops can be __ro_after_init.

This is a little bit safer since it means the ops can't
be overwritten (accidentally or otherwise), which would
otherwise cause an arbitrary function or bad pointer to
be called.

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

diff --git a/drivers/net/wireless/intel/iwlegacy/3945-mac.c b/drivers/net/wireless/intel/iwlegacy/3945-mac.c
index 466912eb2d87..e8e65115feba 100644
--- a/drivers/net/wireless/intel/iwlegacy/3945-mac.c
+++ b/drivers/net/wireless/intel/iwlegacy/3945-mac.c
@@ -3469,7 +3469,7 @@ static struct attribute_group il3945_attribute_group = {
 	.attrs = il3945_sysfs_entries,
 };
 
-static struct ieee80211_ops il3945_mac_ops __read_mostly = {
+static struct ieee80211_ops il3945_mac_ops __ro_after_init = {
 	.tx = il3945_mac_tx,
 	.start = il3945_mac_start,
 	.stop = il3945_mac_stop,
@@ -3627,15 +3627,6 @@ il3945_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	il->cmd_queue = IL39_CMD_QUEUE_NUM;
 
-	/*
-	 * Disabling hardware scan means that mac80211 will perform scans
-	 * "the hard way", rather than using device's scan.
-	 */
-	if (il3945_mod_params.disable_hw_scan) {
-		D_INFO("Disabling hw_scan\n");
-		il3945_mac_ops.hw_scan = NULL;
-	}
-
 	D_INFO("*** LOAD DRIVER ***\n");
 	il->cfg = cfg;
 	il->ops = &il3945_ops;
@@ -3913,6 +3904,15 @@ il3945_init(void)
 	pr_info(DRV_DESCRIPTION ", " DRV_VERSION "\n");
 	pr_info(DRV_COPYRIGHT "\n");
 
+	/*
+	 * Disabling hardware scan means that mac80211 will perform scans
+	 * "the hard way", rather than using device's scan.
+	 */
+	if (il3945_mod_params.disable_hw_scan) {
+		pr_info("hw_scan is disabled\n");
+		il3945_mac_ops.hw_scan = NULL;
+	}
+
 	ret = il3945_rate_control_register();
 	if (ret) {
 		pr_err("Unable to register rate control algorithm: %d\n", ret);
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH 1/2] ath10k: add accounting for the extended peer statistics
From: Mohammed Shafi Shajakhan @ 2016-12-07  6:28 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: linux-wireless, ath10k, Kalle Valo
In-Reply-To: <992a4e2676037a06f482cdbe2d3d39e287530be5.1480974623.git.chunkeey@googlemail.com>

Hi Christian,

On Mon, Dec 05, 2016 at 10:52:45PM +0100, Christian Lamparter wrote:
> The 10.4 firmware adds extended peer information to the
> firmware's statistics payload. This additional info is
> stored as a separate data field and the elements are
> stored in their own "peers_extd" list.
> 
> These elements can pile up in the same way as the peer
> information elements. This is because the
> ath10k_wmi_10_4_op_pull_fw_stats() function tries to
> pull the same amount (num_peer_stats) for every statistic
> data unit.
> 
> Fixes: 4a49ae94a448faa ("ath10k: fix 10.4 extended peer stats update")
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> ---
>  drivers/net/wireless/ath/ath10k/debug.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
> index 82a4c67f3672..4acd9eb65910 100644
> --- a/drivers/net/wireless/ath/ath10k/debug.c
> +++ b/drivers/net/wireless/ath/ath10k/debug.c
> @@ -399,6 +399,7 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
>  			 * prevent firmware from DoS-ing the host.
>  			 */
>  			ath10k_fw_stats_peers_free(&ar->debug.fw_stats.peers);
> +			ath10k_fw_extd_stats_peers_free(&ar->debug.fw_stats.peers_extd);

[shafi] thanks for fixing this !

>  			ath10k_warn(ar, "dropping fw peer stats\n");
>  			goto free;
>  		}
> @@ -409,10 +410,12 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
>  			goto free;
>  		}
>  
> +		if (!list_empty(&stats.peers))

[shafi] sorry please correct me if i am wrong, for 'extended peer stats' we are checking
for normal 'peer stats' ? Is this the fix intended, i had started a build to
check your change and we will keep you posted, does this fix displaying
'rx_duration' in ath10k fw_stats.

> +			list_splice_tail_init(&stats.peers_extd,
> +					      &ar->debug.fw_stats.peers_extd);
> +
>  		list_splice_tail_init(&stats.peers, &ar->debug.fw_stats.peers);
>  		list_splice_tail_init(&stats.vdevs, &ar->debug.fw_stats.vdevs);
> -		list_splice_tail_init(&stats.peers_extd,
> -				      &ar->debug.fw_stats.peers_extd);
>  	}
>  
>  	complete(&ar->debug.fw_stats_complete);

thanks,
shafi

^ permalink raw reply

* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Johannes Berg @ 2016-12-07  6:19 UTC (permalink / raw)
  To: Andrew Zaborowski, Denis Kenzior; +Cc: linux-wireless
In-Reply-To: <CAOq732LOk-GQiSfs9Js-qEgvmbNMD_xf6j_1NR88Z1SDL=J8WA@mail.gmail.com>


> Possibly Johanness refers to the fact that if you use
> CMD_AUTHENTICATE, or if you use CMD_CONNECT but the driver implements
> the SME -- doesn't use the cfg80211 software SME -- then
> cfg80211_disconnect won't do anything if we're not associated, only
> authenticated.  Currently cfg80211 doesn't have knowledge of whether
> it is authenticated and where to.

We used to track it, but it was a nightmare and just always buggy :)

> With the software SME current_bss would be set from the moment the
> authentication attempt starts, 

I'm almost certain this isn't true, what makes you think so?

> so there seems to be an inconsistency
> which would affect for example the NL80211_BSS_STATUS_ASSOCIATED
> flags in the result of CMD_GET_SCAN.

Thus this can't be the case.

> Perhaps this can be fixed by always
> setting current_bss on auth attempt start, with flags to indicate
> whether authentication has happened and whether association happened.

No! That would be wrong!

> At the very least with this patch if you set the socket owner during
> CMD_AUTHENTICATE and then separately associate, you'd get the
> expected deauthentication.

That would *NOT* be expected. There's no need to even authenticate
through CMD_AUTHENTICATE at all to connect to (another) AP!

You need to think beyond the 1996 version of 802.11 ;-)

johannes

^ permalink raw reply

* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Johannes Berg @ 2016-12-07  6:15 UTC (permalink / raw)
  To: Denis Kenzior, Andrew Zaborowski, linux-wireless
In-Reply-To: <584730D7.2020708@gmail.com>


> I'm afraid to consider what you're like when you _are_ 'super'
> against something :)

Why, that's easy, there wouldn't be a long discussion like that ;-)

> So here's a quick test, with the client triggering authenticate, then
> crashing:

> < Request: Authenticate (0x25) len 52
> [ack]                  362.339712

> [...]

>  > Event: Del Station (0x14) len
> 1144                         367.442024

> Pay attention to the time stamps.  The del station event comes in 5 
> seconds or so after our client has aborted.  So for 5 seconds we have
> an unmanaged link to some AP.

No, this is the part you didn't understand. Simply authenticating
doesn't actually create anything like a "link" to the AP. The only
reason we keep the station entry around for a few seconds is that it
*probably* will be used next to associate. But if you don't do that, or
authenticate to some other AP, or do whatever else - nothing stops you
from doing that. There's no connection, nothing really stays active
except for this 5 second grace period to associate.

So even if you crash here like in the example, there's nothing to clean
up, a subsequent authentication attempt to the same or another AP will
go through anyway.

Therefore, there's nothing to "own" with an authentication attempt,
since it doesn't actually keep any (permanent) state in the kernel, and
keeping the station entry around is just an optimisation.

I don't think it's worth trying to clean that up.

Also, consider that authentication doesn't block anything, so another
socket might immediately do another authentication/association, and you
don't want to kill that even when the first one dies. Corner case,
sure, but at least with association the second one would get "-EBUSY"
or so, whereas authentication keeps no state in the kernel.

johannes

^ permalink raw reply

* Re: [PATCH] net: wireless: realtek: constify rate_control_ops structures
From: Jes Sorensen @ 2016-12-07  5:07 UTC (permalink / raw)
  To: Larry Finger
  Cc: Bhumika Goyal, julia.lawall, chaoming_li, kvalo, linux-wireless,
	netdev, linux-kernel
In-Reply-To: <0f223291-734f-7658-57b7-18e962d15823@lwfinger.net>

Larry Finger <Larry.Finger@lwfinger.net> writes:
> On 12/02/2016 03:50 AM, Bhumika Goyal wrote:
>> The structures rate_control_ops are only passed as an argument to the
>> functions ieee80211_rate_control_{register/unregister}. This argument is
>> of type const, so rate_control_ops having this property can also be
>> declared as const.
>> Done using Coccinelle:
>>
>> @r1 disable optional_qualifier @
>> identifier i;
>> position p;
>> @@
>> static struct rate_control_ops i@p = {...};
>>
>> @ok1@
>> identifier r1.i;
>> position p;
>> @@
>> ieee80211_rate_control_register(&i@p)
>>
>> @ok2@
>> identifier r1.i;
>> position p;
>> @@
>> ieee80211_rate_control_unregister(&i@p)
>>
>> @bad@
>> position p!={r1.p,ok1.p,ok2.p};
>> identifier r1.i;
>> @@
>> i@p
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> static
>> +const
>> struct rate_control_ops i={...};
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> +const
>> struct rate_control_ops i;
>>
>> File size before:
>>    text	   data	    bss	    dec	    hex	filename
>>    1991	    104	      0	   2095	    82f wireless/realtek/rtlwifi/rc.o
>>
>> File size after:
>>    text	   data	    bss	    dec	    hex	filename
>>    2095	      0	      0	   2095	    wireless/realtek/rtlwifi/rc.o
>>
[snip]
> The content of your patch is OK; however, your subject is not. By
> convention, "net: wireless: realtek:" is assumed. We do, however,
> include "rtlwifi:" to indicate which part of
> drivers/net/wireless/realtek/ is referenced.

In addition, the first part of the description is useful and the file
size information is reasonable too, but ~20 lines of coccinelle scripts
in the commit message is rather pointless.

Jes

^ permalink raw reply

* [PATCH] ath10k: Avoid potential page alloc BUG_ON in tx free path
From: Mohammed Shafi Shajakhan @ 2016-12-07  5:00 UTC (permalink / raw)
  To: ath10k; +Cc: mohammed, linux-wireless, Mohammed Shafi Shajakhan

From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>

'ath10k_htt_tx_free_cont_txbuf' and 'ath10k_htt_tx_free_cont_frag_desc'
have NULL pointer checks to avoid crash if they are called twice
but this is as of now not sufficient as these pointers are not assigned
to NULL once the contiguous DMA memory allocation is freed, fix this.
Though this may not be hit with the explicity check of state variable
'tx_mem_allocated' check, good to have this addressed as well.

Below BUG_ON is hit when the above scenario is simulated
with kernel debugging enabled

 page:f6d09a00 count:0 mapcount:-127 mapping:  (null)
index:0x0
 flags: 0x40000000()
 page dumped because: VM_BUG_ON_PAGE(page_ref_count(page)
== 0)
 ------------[ cut here ]------------
 kernel BUG at ./include/linux/mm.h:445!
 invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
 EIP is at put_page_testzero.part.88+0xd/0xf
 Call Trace:
  [<c118a2cc>] __free_pages+0x3c/0x40
  [<c118a30e>] free_pages+0x3e/0x50
  [<c10222b4>] dma_generic_free_coherent+0x24/0x30
  [<f8c1d9a8>] ath10k_htt_tx_free_cont_txbuf+0xf8/0x140

  [<f8c1e2a9>] ath10k_htt_tx_destroy+0x29/0xa0

  [<f8c143e0>] ath10k_core_destroy+0x60/0x80 [ath10k_core]
  [<f8acd7e9>] ath10k_pci_remove+0x79/0xa0 [ath10k_pci]
  [<c13ed7a8>] pci_device_remove+0x38/0xb0
  [<c14d3492>] __device_release_driver+0x72/0x100
  [<c14d36b7>] driver_detach+0x97/0xa0
  [<c14d29c0>] bus_remove_driver+0x40/0x80
  [<c14d427a>] driver_unregister+0x2a/0x60
  [<c13ec768>] pci_unregister_driver+0x18/0x70
  [<f8aced4f>] ath10k_pci_exit+0xd/0x2be [ath10k_pci]
  [<c1101e78>] SyS_delete_module+0x158/0x210
  [<c11b34f1>] ? __might_fault+0x41/0xa0
  [<c11b353b>] ? __might_fault+0x8b/0xa0
  [<c1001a4b>] do_fast_syscall_32+0x9b/0x1c0
  [<c178da34>] sysenter_past_esp+0x45/0x74

Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/htt_tx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 27e49db..86b427f 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -239,6 +239,7 @@ static void ath10k_htt_tx_free_cont_txbuf(struct ath10k_htt *htt)
 
 	size = htt->max_num_pending_tx * sizeof(struct ath10k_htt_txbuf);
 	dma_free_coherent(ar->dev, size, htt->txbuf.vaddr, htt->txbuf.paddr);
+	htt->txbuf.vaddr = NULL;
 }
 
 static int ath10k_htt_tx_alloc_cont_txbuf(struct ath10k_htt *htt)
@@ -268,6 +269,7 @@ static void ath10k_htt_tx_free_cont_frag_desc(struct ath10k_htt *htt)
 			  size,
 			  htt->frag_desc.vaddr,
 			  htt->frag_desc.paddr);
+	htt->frag_desc.vaddr = NULL;
 }
 
 static int ath10k_htt_tx_alloc_cont_frag_desc(struct ath10k_htt *htt)
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Andrew Zaborowski @ 2016-12-07  1:40 UTC (permalink / raw)
  To: Denis Kenzior; +Cc: Johannes Berg, linux-wireless
In-Reply-To: <584730D7.2020708@gmail.com>

Hi,

On 6 December 2016 at 22:42, Denis Kenzior <denkenz@gmail.com> wrote:
> On 12/06/2016 01:16 AM, Johannes Berg wrote:
>> Anyway ... I'm not even super against this patch, but you need to fix
>> things:
>>   * this is actually wrong for authenticate - I'll let you figure out
>>     why by yourself
>
>
> So here's a quick test, with the client triggering authenticate, then
> crashing:
>
> < Request: Authenticate (0x25) len 52 [ack]                  362.339712
>     Interface Index: 3 (0x00000003)
>     Wiphy Frequency: 2417 (0x00000971)
>     MAC Address <snip>
>     SSID: len 9
>         <snip>
>     Auth Type: 0 (0x00000000)
>> Event: New Station (0x13) len 32                           362.341337
>     Interface Index: 3 (0x00000003)
>> Response: Authenticate (0x25) len 4                        362.341860
>     Status: Success (0)
>> Event: Authenticate (0x25) len 64                          362.343640
>     Wiphy: 0 (0x00000000)
>     Interface Index: 3 (0x00000003)
>> Event: Del Station (0x14) len 1144                         367.442024
>     Interface Index: 3 (0x00000003)
>     MAC Address <snip>
>
> Pay attention to the time stamps.  The del station event comes in 5 seconds
> or so after our client has aborted.  So for 5 seconds we have an unmanaged
> link to some AP.  It would be nice if the authentication was aborted as soon
> as we crash.
>
> So again, why do you say that SOCKET_OWNER is 'wrong' for authenticate?  Or
> do you mean something else?

Possibly Johanness refers to the fact that if you use
CMD_AUTHENTICATE, or if you use CMD_CONNECT but the driver implements
the SME -- doesn't use the cfg80211 software SME -- then
cfg80211_disconnect won't do anything if we're not associated, only
authenticated.  Currently cfg80211 doesn't have knowledge of whether
it is authenticated and where to.

With the software SME current_bss would be set from the moment the
authentication attempt starts, so there seems to be an inconsistency
which would affect for example the NL80211_BSS_STATUS_ASSOCIATED flags
in the result of CMD_GET_SCAN.  Perhaps this can be fixed by always
setting current_bss on auth attempt start, with flags to indicate
whether authentication has happened and whether association happened.

At the very least with this patch if you set the socket owner during
CMD_AUTHENTICATE and then separately associate, you'd get the expected
deauthentication.

Best regards

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox