9:03 a.m.
---
src/scan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/scan.c b/src/scan.c
index f2332545..4bc95afe 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -297,8 +297,8 @@ static struct l_genl_msg *scan_build_cmd(struct scan_context *sc,
ext_capa = wiphy_get_extended_capabilities(sc->wiphy,
NL80211_IFTYPE_STATION);
/*
- * XXX: If adding IE's here ensure that ordering is not broken for
- * probe requests (IEEE-2016 Table 9-33).
+ * If adding IE's here ensure that ordering is not broken for
+ * probe requests (IEEE Std 802.11-2016 Table 9-33).
*/
/* Order 9 - Extended Capabilities */
iov[iov_elems].iov_base = (void *) ext_capa;
--
2.21.0
9:04 a.m.
New subject: [PATCH 2/4] scan: Fix bit checking for interworking
The checker function will later be changed to match the bit setter.
---
src/scan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/scan.c b/src/scan.c
index 4bc95afe..f1317686 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -305,7 +305,7 @@ static struct l_genl_msg *scan_build_cmd(struct scan_context *sc,
iov[iov_elems].iov_len = ext_capa[1] + 2;
iov_elems++;
- if (util_is_bit_set(ext_capa[3], 7)) {
+ if (util_is_bit_set(ext_capa[2 + 3], 7)) {
/* Order 12 - Interworking */
interworking[0] = IE_TYPE_INTERWORKING;
interworking[1] = 1;
--
2.21.0
9:04 a.m.
New subject: [PATCH 3/4] scan: Separate IE attr creation into logical block
This also introduces the max IE lenght check and exludes the addition
of IEs for the drivers that don't support it.
---
src/scan.c | 51 ++++++++++++++++++++++++++++++---------------------
1 file changed, 30 insertions(+), 21 deletions(-)
diff --git a/src/scan.c b/src/scan.c
index f1317686..a3d6e5d8 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -266,34 +266,15 @@ static void scan_build_attr_scan_frequencies(struct l_genl_msg
*msg,
l_genl_msg_leave_nested(msg);
}
-static bool scan_mac_address_randomization_is_disabled(void)
-{
- const struct l_settings *config = iwd_get_config();
- bool disabled;
-
- if (!l_settings_get_bool(config, "Scan",
- "DisableMacAddressRandomization",
- &disabled))
- return false;
-
- return disabled;
-}
-
-static struct l_genl_msg *scan_build_cmd(struct scan_context *sc,
- bool ignore_flush_flag, bool is_passive,
+static void scan_build_attr_ie(struct l_genl_msg *msg,
+ struct scan_context *sc,
const struct scan_parameters *params)
{
- struct l_genl_msg *msg;
- uint32_t flags = 0;
struct iovec iov[3];
unsigned int iov_elems = 0;
const uint8_t *ext_capa;
uint8_t interworking[3];
- msg = l_genl_msg_new(NL80211_CMD_TRIGGER_SCAN);
-
- l_genl_msg_append_attr(msg, NL80211_ATTR_WDEV, 8, &sc->wdev_id);
-
ext_capa = wiphy_get_extended_capabilities(sc->wiphy,
NL80211_IFTYPE_STATION);
/*
@@ -325,6 +306,34 @@ static struct l_genl_msg *scan_build_cmd(struct scan_context *sc,
}
l_genl_msg_append_attrv(msg, NL80211_ATTR_IE, iov, iov_elems);
+}
+
+static bool scan_mac_address_randomization_is_disabled(void)
+{
+ const struct l_settings *config = iwd_get_config();
+ bool disabled;
+
+ if (!l_settings_get_bool(config, "Scan",
+ "DisableMacAddressRandomization",
+ &disabled))
+ return false;
+
+ return disabled;
+}
+
+static struct l_genl_msg *scan_build_cmd(struct scan_context *sc,
+ bool ignore_flush_flag, bool is_passive,
+ const struct scan_parameters *params)
+{
+ struct l_genl_msg *msg;
+ uint32_t flags = 0;
+
+ msg = l_genl_msg_new(NL80211_CMD_TRIGGER_SCAN);
+
+ l_genl_msg_append_attr(msg, NL80211_ATTR_WDEV, 8, &sc->wdev_id);
+
+ if (wiphy_get_max_scan_ie_len(sc->wiphy))
+ scan_build_attr_ie(msg, sc, params);
if (params->freqs)
scan_build_attr_scan_frequencies(msg, params->freqs);
--
2.21.0
9:04 a.m.
New subject: [PATCH 4/4] scan: Separate cck rates attr creation into logical block
---
src/scan.c | 82 ++++++++++++++++++++++++++++--------------------------
1 file changed, 42 insertions(+), 40 deletions(-)
diff --git a/src/scan.c b/src/scan.c
index a3d6e5d8..f99ad1e7 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -308,6 +308,45 @@ static void scan_build_attr_ie(struct l_genl_msg *msg,
l_genl_msg_append_attrv(msg, NL80211_ATTR_IE, iov, iov_elems);
}
+static void scan_build_attr_tx_no_cck_rates(struct l_genl_msg *msg,
+ struct scan_context *sc)
+{
+ static const uint8_t b_rates[] = { 2, 4, 11, 22 };
+ uint8_t *scan_rates;
+ const uint8_t *supported;
+ unsigned int num_supported;
+ unsigned int count;
+ unsigned int i;
+
+ l_genl_msg_append_attr(msg, NL80211_ATTR_TX_NO_CCK_RATE, 0, NULL);
+
+ /*
+ * Assume if we're sending the probe requests at OFDM bit
+ * rates we don't want to advertise support for 802.11b rates.
+ */
+ if (L_WARN_ON(!(supported = wiphy_get_supported_rates(sc->wiphy,
+ NL80211_BAND_2GHZ,
+ &num_supported))))
+ return;
+
+ scan_rates = l_malloc(num_supported);
+
+ for (count = 0, i = 0; i < num_supported; i++)
+ if (!memchr(b_rates, supported[i], L_ARRAY_SIZE(b_rates)))
+ scan_rates[count++] = supported[i];
+
+ if (L_WARN_ON(!count)) {
+ l_free(scan_rates);
+ return;
+ }
+
+ l_genl_msg_enter_nested(msg, NL80211_ATTR_SCAN_SUPP_RATES);
+ l_genl_msg_append_attr(msg, NL80211_BAND_2GHZ, count, scan_rates);
+ l_genl_msg_leave_nested(msg);
+
+ l_free(scan_rates);
+}
+
static bool scan_mac_address_randomization_is_disabled(void)
{
const struct l_settings *config = iwd_get_config();
@@ -338,6 +377,9 @@ static struct l_genl_msg *scan_build_cmd(struct scan_context *sc,
if (params->freqs)
scan_build_attr_scan_frequencies(msg, params->freqs);
+ if (params->no_cck_rates)
+ scan_build_attr_tx_no_cck_rates(msg, sc);
+
if (params->flush && !ignore_flush_flag)
flags |= NL80211_SCAN_FLAG_FLUSH;
@@ -357,46 +399,6 @@ static struct l_genl_msg *scan_build_cmd(struct scan_context *sc,
if (flags)
l_genl_msg_append_attr(msg, NL80211_ATTR_SCAN_FLAGS, 4, &flags);
- if (params->no_cck_rates) {
- static const uint8_t b_rates[] = { 2, 4, 11, 22 };
- uint8_t *scan_rates;
- const uint8_t *supported;
- unsigned int num_supported;
- unsigned int count;
- unsigned int i;
-
- l_genl_msg_append_attr(msg, NL80211_ATTR_TX_NO_CCK_RATE, 0,
- NULL);
-
- /*
- * Assume if we're sending the probe requests at OFDM bit
- * rates we don't want to advertise support for 802.11b rates.
- */
- if (L_WARN_ON(!(supported = wiphy_get_supported_rates(sc->wiphy,
- NL80211_BAND_2GHZ,
- &num_supported))))
- goto done;
-
- scan_rates = l_malloc(num_supported);
-
- for (count = 0, i = 0; i < num_supported; i++)
- if (!memchr(b_rates, supported[i],
- L_ARRAY_SIZE(b_rates)))
- scan_rates[count++] = supported[i];
-
- if (L_WARN_ON(!count)) {
- l_free(scan_rates);
- goto done;
- }
-
- l_genl_msg_enter_nested(msg, NL80211_ATTR_SCAN_SUPP_RATES);
- l_genl_msg_append_attr(msg, NL80211_BAND_2GHZ,
- count, scan_rates);
- l_genl_msg_leave_nested(msg);
- l_free(scan_rates);
- }
-
-done:
return msg;
}
--
2.21.0
1:06 p.m.
New subject: [PATCH 4/4] scan: Separate cck rates attr creation into logical
block
Hi Tim,
---
src/scan.c | 82 ++++++++++++++++++++++++++++--------------------------
1 file changed, 42 insertions(+), 40 deletions(-)
diff --git a/src/scan.c b/src/scan.c
index a3d6e5d8..f99ad1e7 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -308,6 +308,45 @@ static void scan_build_attr_ie(struct l_genl_msg *msg,
l_genl_msg_append_attrv(msg, NL80211_ATTR_IE, iov, iov_elems);
}
+static void scan_build_attr_tx_no_cck_rates(struct l_genl_msg *msg,
+ struct scan_context *sc)
+{
+ static const uint8_t b_rates[] = { 2, 4, 11, 22 };
+ uint8_t *scan_rates;
+ const uint8_t *supported;
+ unsigned int num_supported;
+ unsigned int count;
+ unsigned int i;
+
+ l_genl_msg_append_attr(msg, NL80211_ATTR_TX_NO_CCK_RATE, 0, NULL);
+
+ /*
+ * Assume if we're sending the probe requests at OFDM bit
+ * rates we don't want to advertise support for 802.11b rates.
+ */
+ if (L_WARN_ON(!(supported = wiphy_get_supported_rates(sc->wiphy,
+ NL80211_BAND_2GHZ,
+ &num_supported))))
+ return;
these kind of “nested” statements become really hard to read and check if they are
correct.
supported = wiphy_get_supported_rates(sc->wiphy, ..);
if (L_WARN_ON(!supported))
return;
I realize that this code is the same as below, but lets improve readability as well. It is
fine to submit a cleanup patch to improve readability if we find such places.
Regards
Marcel
1:40 p.m.
New subject: [PATCH 4/4] scan: Separate cck rates attr creation into logical
block
Hi Marcel,
> + if (L_WARN_ON(!(supported =
wiphy_get_supported_rates(sc->wiphy,
> + NL80211_BAND_2GHZ,
> + &num_supported))))
> + return;
these kind of “nested” statements become really hard to read and check if they are
correct.
supported = wiphy_get_supported_rates(sc->wiphy, ..);
if (L_WARN_ON(!supported))
return;
I realize that this code is the same as below, but lets improve readability as well. It
is fine to submit a cleanup patch to improve readability if we find such places.
Actually, I explicitly requested that this be the way it is. For
L_WARN_ON, I want the resultant warning to carry as much info as
possible. Having a message like "Warning: !supported" show up is pretty
pointless. Yes it carries the line number and filename, but it still
requires an extra step. And things can change between different
versions of iwd, etc.
So while the readability is a little worse, I think the hit is worth it
for cases where L_WARN_ON is used.
Regards,
-Denis
2:21 p.m.
New subject: [PATCH 4/4] scan: Separate cck rates attr creation into logical
block
Hi Denis,
>> + if (L_WARN_ON(!(supported =
wiphy_get_supported_rates(sc->wiphy,
>> + NL80211_BAND_2GHZ,
>> + &num_supported))))
>> + return;
> these kind of “nested” statements become really hard to read and check if they are
correct.
> supported = wiphy_get_supported_rates(sc->wiphy, ..);
> if (L_WARN_ON(!supported))
> return;
> I realize that this code is the same as below, but lets improve readability as well.
It is fine to submit a cleanup patch to improve readability if we find such places.
Actually, I explicitly requested that this be the way it is. For L_WARN_ON, I want the
resultant warning to carry as much info as possible. Having a message like "Warning:
!supported" show up is pretty pointless. Yes it carries the line number and
filename, but it still requires an extra step. And things can change between different
versions of iwd, etc.
So while the readability is a little worse, I think the hit is worth it for cases where
L_WARN_ON is used.
ok then, fair point. Can we somehow improve these cases with a comment to ease reading and
understanding the code.
Regards
Marcel
2:33 p.m.
New subject: [PATCH 4/4] scan: Separate cck rates attr creation into logical
block
Hi Marcel,
ok then, fair point. Can we somehow improve these cases with a comment to ease reading
and understanding the code.
So something like:
+ *
+ * wiphy should have parsed the supported rates from the
+ * kernel / driver. If not, warn and give up
Or do you have something else in mind?
Regards,
-Denis
2:42 p.m.
New subject: [PATCH 4/4] scan: Separate cck rates attr creation into logical
block
On Nov 9, 2019, at 04:33, Denis Kenzior <denkenz(a)gmail.com>
wrote:
Hi Marcel,
> ok then, fair point. Can we somehow improve these cases with a comment to ease
reading and understanding the code.
So something like:
+ *
+ * wiphy should have parsed the supported rates from the
+ * kernel / driver. If not, warn and give up
Or do you have something else in mind?
In addition I would also do
*
* Keep full statement in L_WARN_ON to include more context when
* the warning is triggered.
Just as a reminder for later days on why this is done. I am pretty sure otherwise I am
going to stumble over it again and ask the same question.
Regards
Marcel
2:46 p.m.
New subject: [PATCH 4/4] scan: Separate cck rates attr creation into logical
block
Hi Marcel,
On 11/8/19 9:42 PM, Marcel Holtmann wrote:
> On Nov 9, 2019, at 04:33, Denis Kenzior <denkenz(a)gmail.com> wrote:
>
> Hi Marcel,
>
>> ok then, fair point. Can we somehow improve these cases with a comment to ease
reading and understanding the code.
>
> So something like:
>
> + *
> + * wiphy should have parsed the supported rates from the
> + * kernel / driver. If not, warn and give up
>
> Or do you have something else in mind?
In addition I would also do
*
* Keep full statement in L_WARN_ON to include more context when
* the warning is triggered.
Just as a reminder for later days on why this is done. I am pretty sure otherwise I am
going to stumble over it again and ask the same question.
Right. We do have a dozen or so of these now and I think the use of
L_WARN will only keep growing. Perhaps we should add another section to
doc/coding-style.txt instead? Formalize / encourage this as an optional
best practice going forward.
Regards,
-Denis
2:08 p.m.
New subject: [PATCH 4/4] scan: Separate cck rates attr creation into logical
block
Hi Tim,
On 11/8/19 4:04 PM, Tim Kourt wrote:
---
src/scan.c | 82 ++++++++++++++++++++++++++++--------------------------
1 file changed, 42 insertions(+), 40 deletions(-)
<snip>
+static void scan_build_attr_tx_no_cck_rates(struct l_genl_msg *msg,
+ struct scan_context *sc)
Given that this appends 2 attributes, I think the naming is misleading.
Regards,
-Denis
1044
days inactive
1045
days old
11 comments
3 participants
participants (3)
-
Denis Kenzior -
Marcel Holtmann -
Tim Kourt