4:45 a.m.
Work priority was never explicitly defined anywhere, and a module
using wiphy_radio_work APIs needed to ensure it was not inserting
at a priority that would interfere with other work.
Now all the types of work have been defined with their own priority
and future priorities can easily be added before, after, or in
between existing priorities.
---
src/wiphy.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/wiphy.h b/src/wiphy.h
index 592eba70..fee5d41f 100644
--- a/src/wiphy.h
+++ b/src/wiphy.h
@@ -45,6 +45,12 @@ struct wiphy_radio_work_item {
const struct wiphy_radio_work_item_ops *ops;
};
+#define WIPHY_WORK_PRIORITY_FRAME 0
+#define WIPHY_WORK_PRIORITY_OFFCHANNEL 0
+#define WIPHY_WORK_PRIORITY_CONNECT 1
+#define WIPHY_WORK_PRIORITY_SCAN 2
+#define WIPHY_WORK_PRIORITY_PERIODIC_SCAN 3
+
enum wiphy_state_watch_event {
WIPHY_STATE_WATCH_EVENT_POWERED,
WIPHY_STATE_WATCH_EVENT_RFKILLED,
--
2.31.1
4:45 a.m.
New subject: [PATCH v2 2/6] scan: set scan priority via scan_common
This adds a priority argument to scan_common rather than hard
coding it when inserting the work item and uses the newly
defined wiphy priority for scanning.
---
src/scan.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/scan.c b/src/scan.c
index 54159dfd..e6ee3007 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -607,6 +607,7 @@ static struct scan_request *scan_request_new(struct scan_context *sc,
static uint32_t scan_common(uint64_t wdev_id, bool passive,
const struct scan_parameters *params,
+ int priority,
scan_trigger_func_t trigger,
scan_notify_func_t notify, void *userdata,
scan_destroy_func_t destroy)
@@ -625,7 +626,8 @@ static uint32_t scan_common(uint64_t wdev_id, bool passive,
l_queue_push_tail(sc->requests, sr);
- return wiphy_radio_work_insert(sc->wiphy, &sr->work, 2, &work_ops);
+ return wiphy_radio_work_insert(sc->wiphy, &sr->work,
+ priority, &work_ops);
}
uint32_t scan_passive(uint64_t wdev_id, struct scan_freq_set *freqs,
@@ -634,8 +636,8 @@ uint32_t scan_passive(uint64_t wdev_id, struct scan_freq_set *freqs,
{
struct scan_parameters params = { .freqs = freqs };
- return scan_common(wdev_id, true, ¶ms, trigger, notify,
- userdata, destroy);
+ return scan_common(wdev_id, true, ¶ms, WIPHY_WORK_PRIORITY_SCAN,
+ trigger, notify, userdata, destroy);
}
uint32_t scan_passive_full(uint64_t wdev_id,
@@ -644,8 +646,8 @@ uint32_t scan_passive_full(uint64_t wdev_id,
scan_notify_func_t notify, void *userdata,
scan_destroy_func_t destroy)
{
- return scan_common(wdev_id, true, params, trigger,
- notify, userdata, destroy);
+ return scan_common(wdev_id, true, params, WIPHY_WORK_PRIORITY_SCAN,
+ trigger, notify, userdata, destroy);
}
uint32_t scan_active(uint64_t wdev_id, uint8_t *extra_ie, size_t extra_ie_size,
@@ -658,7 +660,7 @@ uint32_t scan_active(uint64_t wdev_id, uint8_t *extra_ie, size_t
extra_ie_size,
params.extra_ie = extra_ie;
params.extra_ie_size = extra_ie_size;
- return scan_common(wdev_id, false, ¶ms,
+ return scan_common(wdev_id, false, ¶ms, WIPHY_WORK_PRIORITY_SCAN,
trigger, notify, userdata, destroy);
}
@@ -667,7 +669,7 @@ uint32_t scan_active_full(uint64_t wdev_id,
scan_trigger_func_t trigger, scan_notify_func_t notify,
void *userdata, scan_destroy_func_t destroy)
{
- return scan_common(wdev_id, false, params,
+ return scan_common(wdev_id, false, params, WIPHY_WORK_PRIORITY_SCAN,
trigger, notify, userdata, destroy);
}
@@ -795,7 +797,8 @@ uint32_t scan_owe_hidden(uint64_t wdev_id, struct l_queue *list,
done:
l_queue_push_tail(sc->requests, sr);
- return wiphy_radio_work_insert(sc->wiphy, &sr->work, 2, &work_ops);
+ return wiphy_radio_work_insert(sc->wiphy, &sr->work,
+ WIPHY_WORK_PRIORITY_SCAN, &work_ops);
}
bool scan_cancel(uint64_t wdev_id, uint32_t id)
--
2.31.1
4:45 a.m.
New subject: [PATCH v2 3/6] scan: don't special case periodic scan work
Periodic scans were handled specially where they were only
started if no other requests were pending in the scan queue.
This is fine, and what we want, but this can actually be
handled automatically by nature of the wiphy work queue rather
than needing to check the request queue explicitly.
Instead we can insert periodic scans at a lower priority than
other scans. This puts them at the end of the work queue, as
well as allows future requests to jump ahead if a periodic scan
has not yet started.
Eventually, once all pending scans are done, the peridoic scan
may begin. This is no different than the preivous behavior and
avoids the need for any special checks once scan requests
complete.
One check was added to address the problem of the periodic scan
timer firing before the scan could even start. Currently this
happened to be handled fine in scan_periodic_queue, as it checks
the queue length. Since this check was removed we must see check
for this condition inside scan_periodic_timeout.
---
src/scan.c | 50 +++++++++++++++++++++++++++++---------------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/src/scan.c b/src/scan.c
index e6ee3007..a82e6257 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -69,7 +69,6 @@ struct scan_periodic {
scan_trigger_func_t trigger;
scan_notify_func_t callback;
void *userdata;
- bool retry:1;
uint32_t id;
bool needs_active_scan:1;
};
@@ -881,27 +880,32 @@ static bool scan_periodic_notify(int err, struct l_queue *bss_list,
return false;
}
+static void scan_periodic_destroy(void *user_data)
+{
+ struct scan_context *sc = user_data;
+
+ sc->sp.id = 0;
+}
+
static bool scan_periodic_queue(struct scan_context *sc)
{
- if (!l_queue_isempty(sc->requests)) {
- sc->sp.retry = true;
- return false;
- }
+ struct scan_parameters params = {};
if (sc->sp.needs_active_scan && known_networks_has_hidden()) {
- struct scan_parameters params = {
- .randomize_mac_addr_hint = true
- };
+ params.randomize_mac_addr_hint = true;
sc->sp.needs_active_scan = false;
-
- sc->sp.id = scan_active_full(sc->wdev_id, ¶ms,
- scan_periodic_triggered,
- scan_periodic_notify, sc, NULL);
+ sc->sp.id = scan_common(sc->wdev_id, false, ¶ms,
+ WIPHY_WORK_PRIORITY_PERIODIC_SCAN,
+ scan_periodic_triggered,
+ scan_periodic_notify, sc,
+ scan_periodic_destroy);
} else
- sc->sp.id = scan_passive(sc->wdev_id, NULL,
- scan_periodic_triggered,
- scan_periodic_notify, sc, NULL);
+ sc->sp.id = scan_common(sc->wdev_id, true, ¶ms,
+ WIPHY_WORK_PRIORITY_PERIODIC_SCAN,
+ scan_periodic_triggered,
+ scan_periodic_notify, sc,
+ scan_periodic_destroy);
return sc->sp.id != 0;
}
@@ -973,7 +977,6 @@ bool scan_periodic_stop(uint64_t wdev_id)
sc->sp.trigger = NULL;
sc->sp.callback = NULL;
sc->sp.userdata = NULL;
- sc->sp.retry = false;
sc->sp.needs_active_scan = false;
return true;
@@ -1004,6 +1007,16 @@ static void scan_periodic_timeout(struct l_timeout *timeout, void
*user_data)
l_debug("scan_periodic_timeout: %" PRIx64, sc->wdev_id);
+ /*
+ * Timeout triggered before periodic scan could even start, just rearm
+ * with the same interval.
+ */
+ if (sc->sp.id) {
+ l_debug("Periodic scan timer called before scan could start!");
+ scan_periodic_rearm(sc);
+ return;
+ }
+
sc->sp.interval *= 2;
if (sc->sp.interval > SCAN_MAX_INTERVAL)
sc->sp.interval = SCAN_MAX_INTERVAL;
@@ -1048,11 +1061,6 @@ static bool start_next_scan_request(struct wiphy_radio_work_item
*item)
scan_request_failed(sc, sr, -EIO);
- if (sc->sp.retry) {
- sc->sp.retry = false;
- scan_periodic_queue(sc);
- }
-
return true;
}
--
2.31.1
4:45 a.m.
New subject: [PATCH v2 4/6] netdev: use work priorities rather than hard coding
---
src/netdev.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/netdev.c b/src/netdev.c
index 78467d3e..2d49b1f8 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -4040,8 +4040,8 @@ build_cmd_connect:
NL80211_EXT_FEATURE_CAN_REPLACE_PTK0))
handshake_state_set_no_rekey(hs, true);
- wiphy_radio_work_insert(netdev->wiphy, &netdev->work, 1,
- &connect_work_ops);
+ wiphy_radio_work_insert(netdev->wiphy, &netdev->work,
+ WIPHY_WORK_PRIORITY_CONNECT, &connect_work_ops);
}
int netdev_connect(struct netdev *netdev, struct scan_bss *bss,
@@ -4620,8 +4620,8 @@ int netdev_fast_transition(struct netdev *netdev,
netdev_get_oci, netdev);
memcpy(netdev->ap->prev_bssid, orig_bss->addr, ETH_ALEN);
- wiphy_radio_work_insert(netdev->wiphy, &netdev->work, 1,
- &ft_work_ops);
+ wiphy_radio_work_insert(netdev->wiphy, &netdev->work,
+ WIPHY_WORK_PRIORITY_CONNECT, &ft_work_ops);
return 0;
}
@@ -4661,8 +4661,8 @@ int netdev_fast_transition_over_ds(struct netdev *netdev,
netdev);
memcpy(netdev->ap->prev_bssid, orig_bss->addr, ETH_ALEN);
- wiphy_radio_work_insert(netdev->wiphy, &netdev->work, 1,
- &ft_work_ops);
+ wiphy_radio_work_insert(netdev->wiphy, &netdev->work,
+ WIPHY_WORK_PRIORITY_CONNECT, &ft_work_ops);
return 0;
}
--
2.31.1
4:45 a.m.
New subject: [PATCH v2 5/6] offchannel: use work priorities rather than hard coding
The offchannel priority was also changed to zero, which matches the
priority of frames. Currently there should be no interaction between
offchannel and connect (previous offchannel priority).
---
src/offchannel.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/offchannel.c b/src/offchannel.c
index e01f76c7..c6fed569 100644
--- a/src/offchannel.c
+++ b/src/offchannel.c
@@ -168,8 +168,9 @@ uint32_t offchannel_start(uint64_t wdev_id, uint32_t freq, uint32_t
duration,
*/
info->error = -ECANCELED;
- return wiphy_radio_work_insert(wiphy_find_by_wdev(wdev_id),
- &info->work, 1, &offchannel_work_ops);
+ return wiphy_radio_work_insert(wiphy_find_by_wdev(wdev_id), &info->work,
+ WIPHY_WORK_PRIORITY_OFFCHANNEL,
+ &offchannel_work_ops);
}
void offchannel_cancel(uint64_t wdev_id, uint32_t id)
--
2.31.1
4:45 a.m.
New subject: [PATCH v2 6/6] frame-xchg: use work priorities rather than hard coding
---
src/frame-xchg.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/frame-xchg.c b/src/frame-xchg.c
index 4ecfc45d..2c06269c 100644
--- a/src/frame-xchg.c
+++ b/src/frame-xchg.c
@@ -1213,7 +1213,8 @@ uint32_t frame_xchg_startv(uint64_t wdev_id, struct iovec *frame,
uint32_t freq,
* wait on channel) are introduced.
*/
return wiphy_radio_work_insert(wiphy_find_by_wdev(wdev_id),
- &fx->work, 0, &work_ops);
+ &fx->work, WIPHY_WORK_PRIORITY_FRAME,
+ &work_ops);
}
static bool frame_xchg_cancel_by_wdev(void *data, void *user_data)
--
2.31.1
5:38 a.m.
Hi James,
On 1/11/22 11:45, James Prestwood wrote:
Work priority was never explicitly defined anywhere, and a module
using wiphy_radio_work APIs needed to ensure it was not inserting
at a priority that would interfere with other work.
Now all the types of work have been defined with their own priority
and future priorities can easily be added before, after, or in
between existing priorities.
---
src/wiphy.h | 6 ++++++
1 file changed, 6 insertions(+)
All applied, thanks.
Regards,
-Denis
250
days inactive
250
days old
6 comments
2 participants
participants (2)
-
Denis Kenzior -
James Prestwood