On Sat, Jan 07, 2023 at 07:29:17AM +0000, David Ruth wrote:
> Allows collecting and exposing more information about the station's
> current connection from the kernel to the connection manager.
>
> * Add an enum to represent guard interval settings to driver.h.
> * Add fields for storing guard interval and dual carrier modulation
> information into the hostap_sta_driver_data struct.
> * Add bitmask values indicating the presence of fields.
> * STA_DRV_DATA_TX_HE_DCM
> * STA_DRV_DATA_RX_HE_DCM
> * STA_DRV_DATA_TX_HE_GI
> * STA_DRV_DATA_RX_HE_GI
> * Retrieve NL80211_RATE_INFO_HE_GI and NL80211_RATE_INFO_HE_DCM in
> get_sta_handler, and set appropriate flags.
Thanks, I applied the nl80211 related parts now, but as far as the D-Bus
interface is concerned:
> * Propagate the values over D-Bus.
I have some open questions on this below:
> diff --git a/src/drivers/driver.h b/src/drivers/driver.h
> +enum guard_interval {
> + GUARD_INTERVAL_0_4 = 1,
> + GUARD_INTERVAL_0_8 = 2,
> + GUARD_INTERVAL_1_6 = 3,
> + GUARD_INTERVAL_3_2 = 4,
> +};
These feels like an implementation internal definition where those
values might change.. At minimum, this would need a comment if there is
requirement for the values to be maintained as-is.
> diff --git a/wpa_supplicant/dbus/dbus_new_helpers.c b/wpa_supplicant/dbus/dbus_new_helpers.c
> @@ -1146,6 +1146,18 @@ int wpas_dbus_new_from_signal_information(DBusMessageIter *iter,
> + !wpa_dbus_dict_append_uint32(&iter_dict, "rx-guard-interval",
> + si->data.rx_guard_interval)) ||
> + (si->data.tx_guard_interval &&
> + !wpa_dbus_dict_append_uint32(&iter_dict, "tx-guard-interval",
> + si->data.tx_guard_interval)) ||
This would expose those enum guard_interval values as-is over the D-Bus
interface. Is that really appropriate? How would the application at the
other end know what these values 1-4 mean? The actual values 0.4, 0.8,
1.6, 3.2 might make more sense there or at least this mapping would need
to be clearly documented somewhere.
> + (si->data.rx_dcm &&
> + !wpa_dbus_dict_append_uint32(&iter_dict, "rx-dcm",
> + si->data.rx_dcm)) ||
> + (si->data.tx_dcm &&
> + !wpa_dbus_dict_append_uint32(&iter_dict, "tx-dcm",
> + si->data.tx_dcm)) ||
These might be fine as-is, but I'll note that the values from the kernel
are 0 or 1, so there might be cleaner encoding options for these than
uint32.
--
Jouni Malinen PGP id EFC895FA
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
