2015-10-19 18:49:43

by Marty Faltesek

[permalink] [raw]
Subject: [PATCH] mwifiex: toggle carrier state in start_ap/stop_ap.

In uap mode the carrier is not enabled until after the first STA joins.
The carrier triggers the bridge to start its state machine, and if STP
is enabled, it takes 4 seconds as it transitions from disabled to
forwarding. During this time the bridge drops all traffic, and the EAPOL
handshake times out after 3 seconds, preventing stations from joining.

Follow the logic used in mac80211 and start the carrier in start_ap
and disable it in stop_ap. This has a nice benefit of allowing the
first station connection time to be reduced by up to 75% when STP is
in use.

Signed-off-by: Martin Faltesek <[email protected]>
---
drivers/net/wireless/mwifiex/cfg80211.c | 12 ++++++++++++
drivers/net/wireless/mwifiex/uap_event.c | 11 -----------
2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
index 495621f..275ce3c 100644
--- a/drivers/net/wireless/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/mwifiex/cfg80211.c
@@ -1765,6 +1765,13 @@ static int mwifiex_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *dev)
return -1;
}

+ priv->media_connected = false;
+ if (netif_carrier_ok(priv->netdev))
+ netif_carrier_off(priv->netdev);
+ mwifiex_stop_net_dev_queue(priv->netdev, priv->adapter);
+ mwifiex_clean_txrx(priv);
+ mwifiex_del_all_sta_list(priv);
+
return 0;
}

@@ -1863,6 +1870,11 @@ static int mwifiex_cfg80211_start_ap(struct wiphy *wiphy,
if (mwifiex_set_mgmt_ies(priv, &params->beacon))
return -1;

+ priv->media_connected = true;
+ if (!netif_carrier_ok(priv->netdev))
+ netif_carrier_on(priv->netdev);
+ mwifiex_wake_up_net_dev_queue(priv->netdev, priv->adapter);
+
memcpy(&priv->bss_cfg, bss_cfg, sizeof(priv->bss_cfg));
kfree(bss_cfg);
return 0;
diff --git a/drivers/net/wireless/mwifiex/uap_event.c b/drivers/net/wireless/mwifiex/uap_event.c
index f4794cd..c5bb6ee 100644
--- a/drivers/net/wireless/mwifiex/uap_event.c
+++ b/drivers/net/wireless/mwifiex/uap_event.c
@@ -113,19 +113,8 @@ int mwifiex_process_uap_event(struct mwifiex_private *priv)
mwifiex_del_sta_entry(priv, deauth_mac);
break;
case EVENT_UAP_BSS_IDLE:
- priv->media_connected = false;
- if (netif_carrier_ok(priv->netdev))
- netif_carrier_off(priv->netdev);
- mwifiex_stop_net_dev_queue(priv->netdev, adapter);
-
- mwifiex_clean_txrx(priv);
- mwifiex_del_all_sta_list(priv);
break;
case EVENT_UAP_BSS_ACTIVE:
- priv->media_connected = true;
- if (!netif_carrier_ok(priv->netdev))
- netif_carrier_on(priv->netdev);
- mwifiex_wake_up_net_dev_queue(priv->netdev, adapter);
break;
case EVENT_UAP_BSS_START:
dev_dbg(adapter->dev, "AP EVENT: event id: %#x\n", eventcause);
--
2.6.0.rc2.230.g3dd15c0



2015-10-20 20:20:01

by Avinash Patil

[permalink] [raw]
Subject: Re: [PATCH] mwifiex: toggle carrier state in start_ap/stop_ap.

Hello Marty,


On Mon, Oct 19, 2015 at 11:49 AM, Marty Faltesek <[email protected]> wrote:
>
> In uap mode the carrier is not enabled until after the first STA joins.
> The carrier triggers the bridge to start its state machine, and if STP
> is enabled, it takes 4 seconds as it transitions from disabled to
> forwarding. During this time the bridge drops all traffic, and the EAPOL
> handshake times out after 3 seconds, preventing stations from joining.
>
> Follow the logic used in mac80211 and start the carrier in start_ap
> and disable it in stop_ap. This has a nice benefit of allowing the
> first station connection time to be reduced by up to 75% when STP is
> in use.
>
> Signed-off-by: Martin Faltesek <[email protected]>
> ---
> drivers/net/wireless/mwifiex/cfg80211.c | 12 ++++++++++++
> drivers/net/wireless/mwifiex/uap_event.c | 11 -----------
> 2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
> index 495621f..275ce3c 100644
> --- a/drivers/net/wireless/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/mwifiex/cfg80211.c
> @@ -1765,6 +1765,13 @@ static int mwifiex_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *dev)
> return -1;
> }
>
> + priv->media_connected = false;
> + if (netif_carrier_ok(priv->netdev))
> + netif_carrier_off(priv->netdev);
> + mwifiex_stop_net_dev_queue(priv->netdev, priv->adapter);
> + mwifiex_clean_txrx(priv);
> + mwifiex_del_all_sta_list(priv);
> +
> return 0;
> }
>
> @@ -1863,6 +1870,11 @@ static int mwifiex_cfg80211_start_ap(struct wiphy *wiphy,
> if (mwifiex_set_mgmt_ies(priv, &params->beacon))
> return -1;
>
> + priv->media_connected = true;
> + if (!netif_carrier_ok(priv->netdev))
> + netif_carrier_on(priv->netdev);
> + mwifiex_wake_up_net_dev_queue(priv->netdev, priv->adapter);
> +
> memcpy(&priv->bss_cfg, bss_cfg, sizeof(priv->bss_cfg));
> kfree(bss_cfg);
> return 0;
> diff --git a/drivers/net/wireless/mwifiex/uap_event.c b/drivers/net/wireless/mwifiex/uap_event.c
> index f4794cd..c5bb6ee 100644
> --- a/drivers/net/wireless/mwifiex/uap_event.c
> +++ b/drivers/net/wireless/mwifiex/uap_event.c
> @@ -113,19 +113,8 @@ int mwifiex_process_uap_event(struct mwifiex_private *priv)
> mwifiex_del_sta_entry(priv, deauth_mac);
> break;
> case EVENT_UAP_BSS_IDLE:
> - priv->media_connected = false;
> - if (netif_carrier_ok(priv->netdev))
> - netif_carrier_off(priv->netdev);
> - mwifiex_stop_net_dev_queue(priv->netdev, adapter);
> -
> - mwifiex_clean_txrx(priv);
> - mwifiex_del_all_sta_list(priv);
> break;
> case EVENT_UAP_BSS_ACTIVE:
> - priv->media_connected = true;

I remember we have encountered an issue if we download packets to
interface/FW before bss has become active. I think media_connected
cannot be set to true in start_ap; this may cause issues.

Amit,
I would suggest running some tests to see that FW does not crash if we
set media_connected before BSS_ACTIVE event.

Also, this patch has side-effect of cleaning TXRX and station list
data structures only in stop_ap. This is not correct. There is no
point in holding these structures/memory if no stations are connected.

> - if (!netif_carrier_ok(priv->netdev))
> - netif_carrier_on(priv->netdev);
> - mwifiex_wake_up_net_dev_queue(priv->netdev, adapter);
> break;
> case EVENT_UAP_BSS_START:
> dev_dbg(adapter->dev, "AP EVENT: event id: %#x\n", eventcause);
> --
> 2.6.0.rc2.230.g3dd15c0
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

Thanks,
Avinash

2015-10-21 03:41:46

by Amitkumar Karwar

[permalink] [raw]
Subject: RE: [PATCH] mwifiex: toggle carrier state in start_ap/stop_ap.

SGkgQXZpbmFzaA0KDQo+IEZyb206IGxpbnV4LXdpcmVsZXNzLW93bmVyQHZnZXIua2VybmVsLm9y
ZyBbbWFpbHRvOmxpbnV4LXdpcmVsZXNzLQ0KPiBvd25lckB2Z2VyLmtlcm5lbC5vcmddIE9uIEJl
aGFsZiBPZiBBdmluYXNoIFBhdGlsDQo+IFNlbnQ6IFdlZG5lc2RheSwgT2N0b2JlciAyMSwgMjAx
NSAxOjUwIEFNDQo+IFRvOiBNYXJ0eSBGYWx0ZXNlaw0KPiBDYzogbGludXgtd2lyZWxlc3NAdmdl
ci5rZXJuZWwub3JnOyBBbWl0a3VtYXIgS2Fyd2FyOyBOaXNoYW50DQo+IFNhcm11a2FkYW07IERl
bnRvbiBHZW50cnk7IEF2ZXJ5IFBlbm5hcnVuDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0hdIG13aWZp
ZXg6IHRvZ2dsZSBjYXJyaWVyIHN0YXRlIGluIHN0YXJ0X2FwL3N0b3BfYXAuDQo+IA0KPiBIZWxs
byBNYXJ0eSwNCj4gDQo+IA0KPiBPbiBNb24sIE9jdCAxOSwgMjAxNSBhdCAxMTo0OSBBTSwgTWFy
dHkgRmFsdGVzZWsgPG1mYWx0ZXNla0Bnb29nbGUuY29tPg0KPiB3cm90ZToNCj4gPg0KPiA+IElu
IHVhcCBtb2RlIHRoZSBjYXJyaWVyIGlzIG5vdCBlbmFibGVkIHVudGlsIGFmdGVyIHRoZSBmaXJz
dCBTVEENCj4gam9pbnMuDQo+ID4gVGhlIGNhcnJpZXIgdHJpZ2dlcnMgdGhlIGJyaWRnZSB0byBz
dGFydCBpdHMgc3RhdGUgbWFjaGluZSwgYW5kIGlmIFNUUA0KPiA+IGlzIGVuYWJsZWQsIGl0IHRh
a2VzIDQgc2Vjb25kcyBhcyBpdCB0cmFuc2l0aW9ucyBmcm9tIGRpc2FibGVkIHRvDQo+ID4gZm9y
d2FyZGluZy4gRHVyaW5nIHRoaXMgdGltZSB0aGUgYnJpZGdlIGRyb3BzIGFsbCB0cmFmZmljLCBh
bmQgdGhlDQo+ID4gRUFQT0wgaGFuZHNoYWtlIHRpbWVzIG91dCBhZnRlciAzIHNlY29uZHMsIHBy
ZXZlbnRpbmcgc3RhdGlvbnMgZnJvbQ0KPiBqb2luaW5nLg0KPiA+DQo+ID4gRm9sbG93IHRoZSBs
b2dpYyB1c2VkIGluIG1hYzgwMjExIGFuZCBzdGFydCB0aGUgY2FycmllciBpbiBzdGFydF9hcA0K
PiA+IGFuZCBkaXNhYmxlIGl0IGluIHN0b3BfYXAuIFRoaXMgaGFzIGEgbmljZSBiZW5lZml0IG9m
IGFsbG93aW5nIHRoZQ0KPiA+IGZpcnN0IHN0YXRpb24gY29ubmVjdGlvbiB0aW1lIHRvIGJlIHJl
ZHVjZWQgYnkgdXAgdG8gNzUlIHdoZW4gU1RQIGlzDQo+ID4gaW4gdXNlLg0KPiA+DQo+ID4gU2ln
bmVkLW9mZi1ieTogTWFydGluIEZhbHRlc2VrIDxtZmFsdGVzZWtAZ29vZ2xlLmNvbT4NCj4gPiAt
LS0NCj4gPiAgZHJpdmVycy9uZXQvd2lyZWxlc3MvbXdpZmlleC9jZmc4MDIxMS5jICB8IDEyICsr
KysrKysrKysrKw0KPiA+IGRyaXZlcnMvbmV0L3dpcmVsZXNzL213aWZpZXgvdWFwX2V2ZW50LmMg
fCAxMSAtLS0tLS0tLS0tLQ0KPiA+ICAyIGZpbGVzIGNoYW5nZWQsIDEyIGluc2VydGlvbnMoKyks
IDExIGRlbGV0aW9ucygtKQ0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvbmV0L3dpcmVs
ZXNzL213aWZpZXgvY2ZnODAyMTEuYw0KPiA+IGIvZHJpdmVycy9uZXQvd2lyZWxlc3MvbXdpZmll
eC9jZmc4MDIxMS5jDQo+ID4gaW5kZXggNDk1NjIxZi4uMjc1Y2UzYyAxMDA2NDQNCj4gPiAtLS0g
YS9kcml2ZXJzL25ldC93aXJlbGVzcy9td2lmaWV4L2NmZzgwMjExLmMNCj4gPiArKysgYi9kcml2
ZXJzL25ldC93aXJlbGVzcy9td2lmaWV4L2NmZzgwMjExLmMNCj4gPiBAQCAtMTc2NSw2ICsxNzY1
LDEzIEBAIHN0YXRpYyBpbnQgbXdpZmlleF9jZmc4MDIxMV9zdG9wX2FwKHN0cnVjdA0KPiB3aXBo
eSAqd2lwaHksIHN0cnVjdCBuZXRfZGV2aWNlICpkZXYpDQo+ID4gICAgICAgICAgICAgICAgIHJl
dHVybiAtMTsNCj4gPiAgICAgICAgIH0NCj4gPg0KPiA+ICsgICAgICAgcHJpdi0+bWVkaWFfY29u
bmVjdGVkID0gZmFsc2U7DQo+ID4gKyAgICAgICBpZiAobmV0aWZfY2Fycmllcl9vayhwcml2LT5u
ZXRkZXYpKQ0KPiA+ICsgICAgICAgICAgICAgICBuZXRpZl9jYXJyaWVyX29mZihwcml2LT5uZXRk
ZXYpOw0KPiA+ICsgICAgICAgbXdpZmlleF9zdG9wX25ldF9kZXZfcXVldWUocHJpdi0+bmV0ZGV2
LCBwcml2LT5hZGFwdGVyKTsNCj4gPiArICAgICAgIG13aWZpZXhfY2xlYW5fdHhyeChwcml2KTsN
Cj4gPiArICAgICAgIG13aWZpZXhfZGVsX2FsbF9zdGFfbGlzdChwcml2KTsNCj4gPiArDQo+ID4g
ICAgICAgICByZXR1cm4gMDsNCj4gPiAgfQ0KPiA+DQo+ID4gQEAgLTE4NjMsNiArMTg3MCwxMSBA
QCBzdGF0aWMgaW50IG13aWZpZXhfY2ZnODAyMTFfc3RhcnRfYXAoc3RydWN0DQo+IHdpcGh5ICp3
aXBoeSwNCj4gPiAgICAgICAgIGlmIChtd2lmaWV4X3NldF9tZ210X2llcyhwcml2LCAmcGFyYW1z
LT5iZWFjb24pKQ0KPiA+ICAgICAgICAgICAgICAgICByZXR1cm4gLTE7DQo+ID4NCj4gPiArICAg
ICAgIHByaXYtPm1lZGlhX2Nvbm5lY3RlZCA9IHRydWU7DQo+ID4gKyAgICAgICBpZiAoIW5ldGlm
X2NhcnJpZXJfb2socHJpdi0+bmV0ZGV2KSkNCj4gPiArICAgICAgICAgICAgICAgbmV0aWZfY2Fy
cmllcl9vbihwcml2LT5uZXRkZXYpOw0KPiA+ICsgICAgICAgbXdpZmlleF93YWtlX3VwX25ldF9k
ZXZfcXVldWUocHJpdi0+bmV0ZGV2LCBwcml2LT5hZGFwdGVyKTsNCj4gPiArDQo+ID4gICAgICAg
ICBtZW1jcHkoJnByaXYtPmJzc19jZmcsIGJzc19jZmcsIHNpemVvZihwcml2LT5ic3NfY2ZnKSk7
DQo+ID4gICAgICAgICBrZnJlZShic3NfY2ZnKTsNCj4gPiAgICAgICAgIHJldHVybiAwOw0KPiA+
IGRpZmYgLS1naXQgYS9kcml2ZXJzL25ldC93aXJlbGVzcy9td2lmaWV4L3VhcF9ldmVudC5jDQo+
ID4gYi9kcml2ZXJzL25ldC93aXJlbGVzcy9td2lmaWV4L3VhcF9ldmVudC5jDQo+ID4gaW5kZXgg
ZjQ3OTRjZC4uYzViYjZlZSAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL25ldC93aXJlbGVzcy9t
d2lmaWV4L3VhcF9ldmVudC5jDQo+ID4gKysrIGIvZHJpdmVycy9uZXQvd2lyZWxlc3MvbXdpZmll
eC91YXBfZXZlbnQuYw0KPiA+IEBAIC0xMTMsMTkgKzExMyw4IEBAIGludCBtd2lmaWV4X3Byb2Nl
c3NfdWFwX2V2ZW50KHN0cnVjdA0KPiBtd2lmaWV4X3ByaXZhdGUgKnByaXYpDQo+ID4gICAgICAg
ICAgICAgICAgIG13aWZpZXhfZGVsX3N0YV9lbnRyeShwcml2LCBkZWF1dGhfbWFjKTsNCj4gPiAg
ICAgICAgICAgICAgICAgYnJlYWs7DQo+ID4gICAgICAgICBjYXNlIEVWRU5UX1VBUF9CU1NfSURM
RToNCj4gPiAtICAgICAgICAgICAgICAgcHJpdi0+bWVkaWFfY29ubmVjdGVkID0gZmFsc2U7DQo+
ID4gLSAgICAgICAgICAgICAgIGlmIChuZXRpZl9jYXJyaWVyX29rKHByaXYtPm5ldGRldikpDQo+
ID4gLSAgICAgICAgICAgICAgICAgICAgICAgbmV0aWZfY2Fycmllcl9vZmYocHJpdi0+bmV0ZGV2
KTsNCj4gPiAtICAgICAgICAgICAgICAgbXdpZmlleF9zdG9wX25ldF9kZXZfcXVldWUocHJpdi0+
bmV0ZGV2LCBhZGFwdGVyKTsNCj4gPiAtDQo+ID4gLSAgICAgICAgICAgICAgIG13aWZpZXhfY2xl
YW5fdHhyeChwcml2KTsNCj4gPiAtICAgICAgICAgICAgICAgbXdpZmlleF9kZWxfYWxsX3N0YV9s
aXN0KHByaXYpOw0KPiA+ICAgICAgICAgICAgICAgICBicmVhazsNCj4gPiAgICAgICAgIGNhc2Ug
RVZFTlRfVUFQX0JTU19BQ1RJVkU6DQo+ID4gLSAgICAgICAgICAgICAgIHByaXYtPm1lZGlhX2Nv
bm5lY3RlZCA9IHRydWU7DQo+IA0KPiBJIHJlbWVtYmVyIHdlIGhhdmUgZW5jb3VudGVyZWQgYW4g
aXNzdWUgaWYgd2UgZG93bmxvYWQgcGFja2V0cyB0bw0KPiBpbnRlcmZhY2UvRlcgYmVmb3JlIGJz
cyBoYXMgYmVjb21lIGFjdGl2ZS4gSSB0aGluayBtZWRpYV9jb25uZWN0ZWQNCj4gY2Fubm90IGJl
IHNldCB0byB0cnVlIGluIHN0YXJ0X2FwOyB0aGlzIG1heSBjYXVzZSBpc3N1ZXMuDQo+IA0KPiBB
bWl0LA0KPiBJIHdvdWxkIHN1Z2dlc3QgcnVubmluZyBzb21lIHRlc3RzIHRvIHNlZSB0aGF0IEZX
IGRvZXMgbm90IGNyYXNoIGlmIHdlDQo+IHNldCBtZWRpYV9jb25uZWN0ZWQgYmVmb3JlIEJTU19B
Q1RJVkUgZXZlbnQuDQo+IA0KPiBBbHNvLCB0aGlzIHBhdGNoIGhhcyBzaWRlLWVmZmVjdCBvZiBj
bGVhbmluZyBUWFJYIGFuZCBzdGF0aW9uIGxpc3QgZGF0YQ0KPiBzdHJ1Y3R1cmVzIG9ubHkgaW4g
c3RvcF9hcC4gVGhpcyBpcyBub3QgY29ycmVjdC4gVGhlcmUgaXMgbm8gcG9pbnQgaW4NCj4gaG9s
ZGluZyB0aGVzZSBzdHJ1Y3R1cmVzL21lbW9yeSBpZiBubyBzdGF0aW9ucyBhcmUgY29ubmVjdGVk
Lg0KPiANCg0KVGhhbmtzIGZvciB5b3VyIHJldmlldy4NCg0KSSBzdXBwb3NlIG9ubHkgY2Fycmll
ciBzdGF0ZSB0b2dnbGUgbG9naWMgKG5ldGlmX2NhcnJpZXJfb2ZmL29uIGxpbmVzKSBuZWVkIHRv
IGJlIG1vdmVkIHRvIHN0YXJ0X2FwL3N0b3BfYXAgdG8gZml4IHRoZSBwcm9ibGVtLg0KSSB3aWxs
IHJ1biBzb21lIHRlc3RzIGFuZCBzdWJtaXQgdXBkYXRlZCB2ZXJzaW9uLg0KDQpSZWdhcmRzLA0K
QW1pdGt1bWFyDQo=