Hi Alain,
On Fri, May 29, 2020 at 10:05 AM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> Hi Alain,
>
> On Fri, May 29, 2020 at 8:41 AM Alain Michaud <alainm@xxxxxxxxxxxx> wrote:
> >
> > This change loads any specified system configuration if provided and
> > supported by the kernel.
> >
> > ---
> >
> > Changes in v3: None
> > Changes in v2: None
> >
> > src/adapter.c | 251 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > src/hcid.h | 39 ++++++++
> > 2 files changed, 290 insertions(+)
> >
> > diff --git a/src/adapter.c b/src/adapter.c
> > index 972d88772..da3777cba 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -120,6 +120,8 @@ static bool kernel_conn_control = false;
> >
> > static bool kernel_blocked_keys_supported = false;
> >
> > +static bool kernel_set_system_params = false;
> > +
> > static GList *adapter_list = NULL;
> > static unsigned int adapter_remaining = 0;
> > static bool powering_down = false;
> > @@ -4157,6 +4159,250 @@ static void probe_devices(void *user_data)
> > device_probe_profiles(device, btd_device_get_uuids(device));
> > }
> >
> > +static void load_default_system_params(struct btd_adapter *adapter)
> > +{
> > + /* note: for now all params are 2 bytes, if varying size params are
> > + * added, calculating the right size of buffer will be necessary first.
> > + */
> > + struct controller_params_tlv {
> > + uint16_t parameter_type;
> > + uint8_t length;
> > + uint16_t value;
> > + } __packed;
>
> Don't we have a definition of the tlv entry on mgmt.h? Id use that
> instead of redefining it here.
Ive actually end up cleanup up this a bit:
https://gist.github.com/Vudentz/4426a1032e21b73229b9439aba6f0105
Though this code would probably need some rework if we had to support
different sizes for the value, so we probably need the value to be a
union with different types it can assume, but if we were to support
variable length it would be better to use something like an iovec and
make changes to mgmt_send (or add mgmt_send_iov).
>
> > + struct controller_params_tlv *params = NULL;
> > + uint16_t i = 0;
> > + size_t cp_size;
> > +
> > + if (!main_opts.default_params.num_set_params ||
> > + !kernel_set_system_params)
> > + return;
> > +
> > + cp_size = sizeof(struct controller_params_tlv) *
> > + main_opts.default_params.num_set_params;
> > + params = g_try_malloc0(cp_size);
>
> For new code we prefer to use new0 instead of glib allocation.
>
> > + if (!params)
> > + return;
> > +
> > + if (main_opts.default_params.br_page_scan_type != 0xFFFF) {
> > + params[i].parameter_type = 0x0000;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value = main_opts.default_params.br_page_scan_type;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.br_page_scan_interval) {
> > + params[i].parameter_type = 0x0001;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.br_page_scan_interval;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.br_page_scan_window) {
> > + params[i].parameter_type = 0x0002;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value = main_opts.default_params.br_page_scan_window;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.br_inquiry_scan_type != 0xFFFF) {
> > + params[i].parameter_type = 0x0003;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value = main_opts.default_params.br_inquiry_scan_type;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.br_inquiry_scan_interval) {
> > + params[i].parameter_type = 0x0004;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.br_inquiry_scan_interval;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.br_inquiry_scan_window) {
> > + params[i].parameter_type = 0x0005;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.br_inquiry_scan_window;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.br_link_supervision_timeout) {
> > + params[i].parameter_type = 0x0006;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.br_link_supervision_timeout;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.br_page_timeout) {
> > + params[i].parameter_type = 0x0007;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value = main_opts.default_params.br_page_timeout;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.br_min_sniff_interval) {
> > + params[i].parameter_type = 0x0008;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.br_min_sniff_interval;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.br_max_sniff_interval) {
> > + params[i].parameter_type = 0x0009;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.br_max_sniff_interval;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_min_adv_interval) {
> > + params[i].parameter_type = 0x000a;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value = main_opts.default_params.le_min_adv_interval;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_max_adv_interval) {
> > + params[i].parameter_type = 0x000b;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value = main_opts.default_params.le_max_adv_interval;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_multi_adv_rotation_interval) {
> > + params[i].parameter_type = 0x000c;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_multi_adv_rotation_interval;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_scan_interval_autoconnect) {
> > + params[i].parameter_type = 0x000d;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_scan_interval_autoconnect;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_scan_window_autoconnect) {
> > + params[i].parameter_type = 0x000e;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_scan_window_autoconnect;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_scan_interval_suspend) {
> > + params[i].parameter_type = 0x000f;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_scan_interval_suspend;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_scan_window_suspend) {
> > + params[i].parameter_type = 0x0010;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_scan_window_suspend;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_scan_interval_discovery) {
> > + params[i].parameter_type = 0x0011;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_scan_interval_discovery;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_scan_window_discovery) {
> > + params[i].parameter_type = 0x0012;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_scan_window_discovery;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_scan_interval_adv_monitor) {
> > + params[i].parameter_type = 0x0013;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_scan_interval_adv_monitor;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_scan_window_adv_monitor) {
> > + params[i].parameter_type = 0x0014;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_scan_window_adv_monitor;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_scan_interval_connect) {
> > + params[i].parameter_type = 0x0015;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_scan_interval_connect;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_scan_window_connect) {
> > + params[i].parameter_type = 0x0016;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_scan_window_connect;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_min_connection_interval) {
> > + params[i].parameter_type = 0x0017;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_min_connection_interval;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_max_connection_interval) {
> > + params[i].parameter_type = 0x0018;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_max_connection_interval;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_connection_latency) {
> > + params[i].parameter_type = 0x0019;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value =
> > + main_opts.default_params.le_connection_latency;
> > + ++i;
> > + }
> > +
> > + if (main_opts.default_params.le_connection_lsto) {
> > + params[i].parameter_type = 0x001a;
> > + params[i].length = sizeof(params[i].value);
> > + params[i].value = main_opts.default_params.le_connection_lsto;
> > + ++i;
> > + }
> > +
> > + mgmt_send(adapter->mgmt, MGMT_OP_SET_DEFAULT_SYSTEM_PARAMETERS,
> > + adapter->dev_id, cp_size, params, NULL, NULL, NULL);
> > +
> > + btd_error(adapter->dev_id,
> > + "Failed to set default system params for hci%u",
> > + adapter->dev_id);
>
> I guess we are missing a if statement above otherwise this will always
> be printing an error.
>
> > +
> > + g_free(params);
> > +}
> > +
> > static void load_devices(struct btd_adapter *adapter)
> > {
> > char dirname[PATH_MAX];
> > @@ -8265,6 +8511,7 @@ load:
> > load_drivers(adapter);
> > btd_profile_foreach(probe_profile, adapter);
> > clear_blocked(adapter);
> > + load_default_system_params(adapter);
> > load_devices(adapter);
> >
> > /* restore Service Changed CCC value for bonded devices */
> > @@ -9158,6 +9405,10 @@ static void read_commands_complete(uint8_t status, uint16_t length,
> > DBG("kernel supports the set_blocked_keys op");
> > kernel_blocked_keys_supported = true;
> > break;
> > + case MGMT_OP_SET_DEFAULT_SYSTEM_PARAMETERS:
> > + DBG("kernel supports set system params");
> > + kernel_set_system_params = true;
> > + break;
> > default:
> > break;
> > }
> > diff --git a/src/hcid.h b/src/hcid.h
> > index 1bf93784d..e6d966c58 100644
> > --- a/src/hcid.h
> > +++ b/src/hcid.h
> > @@ -49,6 +49,45 @@ struct main_opts {
> > uint32_t discovto;
> > uint8_t privacy;
> >
> > + struct {
> > + uint16_t num_set_params;
> > +
> > + uint16_t br_page_scan_type;
> > + uint16_t br_page_scan_interval;
> > + uint16_t br_page_scan_window;
> > +
> > + uint16_t br_inquiry_scan_type;
> > + uint16_t br_inquiry_scan_interval;
> > + uint16_t br_inquiry_scan_window;
> > +
> > + uint16_t br_link_supervision_timeout;
> > + uint16_t br_page_timeout;
> > +
> > + uint16_t br_min_sniff_interval;
> > + uint16_t br_max_sniff_interval;
> > +
> > + uint16_t le_min_adv_interval;
> > + uint16_t le_max_adv_interval;
> > + uint16_t le_multi_adv_rotation_interval;
> > +
> > + uint16_t le_scan_interval_autoconnect;
> > + uint16_t le_scan_window_autoconnect;
> > + uint16_t le_scan_interval_suspend;
> > + uint16_t le_scan_window_suspend;
> > + uint16_t le_scan_interval_discovery;
> > + uint16_t le_scan_window_discovery;
> > + uint16_t le_scan_interval_adv_monitor;
> > + uint16_t le_scan_window_adv_monitor;
> > + uint16_t le_scan_interval_connect;
> > + uint16_t le_scan_window_connect;
> > +
> > + uint16_t le_min_connection_interval;
> > + uint16_t le_max_connection_interval;
> > + uint16_t le_connection_latency;
> > + uint16_t le_connection_lsto;
> > + } default_params;
> > +
> > +
> > gboolean reverse_discovery;
> > gboolean name_resolv;
> > gboolean debug_keys;
> > --
> > 2.27.0.rc0.183.gde8f92d652-goog
> >
>
>
> --
> Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
