| Message ID | 20170727230654.30850-10-sergey.matyukevich.os@quantenna.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c7ead2abd26ab536a2e479af605a6d9529e3a694 |
| Delegated to: | Kalle Valo |
| Headers | show |
Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes: > Userspace tools may hang on scan in the case when scan completion event > is not returned by firmware. This patch implements the scan timeout > to avoid such situation. > > Signed-off-by: Igor Mitsyanko <igor.mitsyanko.os@quantenna.com> > Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> > Signed-off-by: Avinash Patil <avinashp@quantenna.com> [...] > --- a/drivers/net/wireless/quantenna/qtnfmac/event.c > +++ b/drivers/net/wireless/quantenna/qtnfmac/event.c > @@ -345,6 +345,8 @@ qtnf_event_handle_scan_complete(struct qtnf_wmac *mac, > return -EINVAL; > } > > + if (timer_pending(&mac->scan_timeout)) > + del_timer_sync(&mac->scan_timeout); What if the device is removed while the timer is pending, is that handled? No need to resend because of this, a followup patch is fine. Just started to wonder this while reviewing the patches.
> > + if (timer_pending(&mac->scan_timeout)) > > + del_timer_sync(&mac->scan_timeout); > > What if the device is removed while the timer is pending, is that > handled? Good point. I took another look at this kind of corner cases. Timer is not disabled explicitely. But ongoing scan request is explicitely aborted in relevant cfg80211 ops, e.g. on virtual interface change or removal. Though it looks like some of AP usecases are not handled: e.g. when AP is stopped while scan is in progress. I will queue the fix into the next cleanup/bugfix patch series if it is needed to abort scan in such a case. Thanks, Sergey
Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes: >> > + if (timer_pending(&mac->scan_timeout)) >> > + del_timer_sync(&mac->scan_timeout); >> >> What if the device is removed while the timer is pending, is that >> handled? > > Good point. I took another look at this kind of corner cases. Timer is not disabled > explicitely. But ongoing scan request is explicitely aborted in relevant > cfg80211 ops, e.g. on virtual interface change or removal. Though it looks like > some of AP usecases are not handled: e.g. when AP is stopped while scan is > in progress. I will queue the fix into the next cleanup/bugfix patch series > if it is needed to abort scan in such a case. Good, thanks for checking.
diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c index ac8fdc1db482..856fa6e8327e 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c +++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c @@ -579,19 +579,33 @@ qtnf_del_station(struct wiphy *wiphy, struct net_device *dev, return ret; } +static void qtnf_scan_timeout(unsigned long data) +{ + struct qtnf_wmac *mac = (struct qtnf_wmac *)data; + + pr_warn("mac%d scan timed out\n", mac->macid); + qtnf_scan_done(mac, true); +} + static int qtnf_scan(struct wiphy *wiphy, struct cfg80211_scan_request *request) { struct qtnf_wmac *mac = wiphy_priv(wiphy); - int ret; mac->scan_req = request; - ret = qtnf_cmd_send_scan(mac); - if (ret) + if (qtnf_cmd_send_scan(mac)) { pr_err("MAC%u: failed to start scan\n", mac->macid); + mac->scan_req = NULL; + return -EFAULT; + } - return ret; + mac->scan_timeout.data = (unsigned long)mac; + mac->scan_timeout.function = qtnf_scan_timeout; + mod_timer(&mac->scan_timeout, + jiffies + QTNF_SCAN_TIMEOUT_SEC * HZ); + + return 0; } static int diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h index 5bd33124a7c8..6a4af52522b8 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h +++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h @@ -34,10 +34,14 @@ static inline void qtnf_scan_done(struct qtnf_wmac *mac, bool aborted) .aborted = aborted, }; + mutex_lock(&mac->mac_lock); + if (mac->scan_req) { cfg80211_scan_done(mac->scan_req, &info); mac->scan_req = NULL; } + + mutex_unlock(&mac->mac_lock); } #endif /* _QTN_FMAC_CFG80211_H_ */ diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.c b/drivers/net/wireless/quantenna/qtnfmac/core.c index 17d17e332a8b..5e60180482d1 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/core.c +++ b/drivers/net/wireless/quantenna/qtnfmac/core.c @@ -288,6 +288,8 @@ static struct qtnf_wmac *qtnf_core_mac_alloc(struct qtnf_bus *bus, mac->iflist[i].mac = mac; mac->iflist[i].vifid = i; qtnf_sta_list_init(&mac->iflist[i].sta_list); + mutex_init(&mac->mac_lock); + init_timer(&mac->scan_timeout); } qtnf_mac_init_primary_intf(mac); diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.h b/drivers/net/wireless/quantenna/qtnfmac/core.h index 099aad76afeb..066fcd1095a0 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/core.h +++ b/drivers/net/wireless/quantenna/qtnfmac/core.h @@ -46,6 +46,7 @@ #define QTNF_MAX_EVENT_QUEUE_LEN 255 #define QTNF_DEFAULT_BG_SCAN_PERIOD 300 #define QTNF_MAX_BG_SCAN_PERIOD 0xffff +#define QTNF_SCAN_TIMEOUT_SEC 15 #define QTNF_DEF_BSS_PRIORITY 0 #define QTNF_DEF_WDOG_TIMEOUT 5 @@ -147,6 +148,8 @@ struct qtnf_wmac { struct cfg80211_scan_request *scan_req; struct cfg80211_chan_def chandef; struct cfg80211_chan_def csa_chandef; + struct mutex mac_lock; /* lock during wmac speicific ops */ + struct timer_list scan_timeout; }; struct qtnf_hw_info { diff --git a/drivers/net/wireless/quantenna/qtnfmac/event.c b/drivers/net/wireless/quantenna/qtnfmac/event.c index 43d2e7fd6e02..0fc2814eafad 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/event.c +++ b/drivers/net/wireless/quantenna/qtnfmac/event.c @@ -345,6 +345,8 @@ qtnf_event_handle_scan_complete(struct qtnf_wmac *mac, return -EINVAL; } + if (timer_pending(&mac->scan_timeout)) + del_timer_sync(&mac->scan_timeout); qtnf_scan_done(mac, le32_to_cpu(status->flags) & QLINK_SCAN_ABORTED); return 0;