Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200929185522.BlueZ.v5.1.I1f083629d41c747a33a3937fbc199ea37dccde12@changeid>
 <20200929185522.BlueZ.v5.2.Iac5207375d22fb7b96ae4ef73b49ae4ab457219c@changeid>
In-Reply-To: <20200929185522.BlueZ.v5.2.Iac5207375d22fb7b96ae4ef73b49ae4ab457219c@changeid>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Tue, 29 Sep 2020 12:50:14 -0700
Message-ID: <CABBYNZ++XfOFCaAErMF6x+1mC+hT868h3b3GtaVNvnGDZcnkvQ@mail.gmail.com>
Subject: Re: [BlueZ PATCH v5 2/3] core: Add param to disable interleave scan
To:     Howard Chung <howardchung@google.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        Manish Mandlik <mmandlik@chromium.org>,
        Alain Michaud <alainm@chromium.org>,
        Miao-chen Chou <mcchou@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Howard,

On Tue, Sep 29, 2020 at 3:56 AM Howard Chung <howardchung@google.com> wrote:
>
> This patch adds parameter to enable/disable the interleave scan feature.
> ---
>
> Changes in v5:
> - Fix compile warning
>
> Changes in v4:
> - Set type of EnableAdvMonInterleaveScan to u8
> - Add support of different sizes of parameters
>
> Changes in v3:
> - Set EnableAdvMonInterleaveScan default to Disable
>
>  src/adapter.c | 10 ++++++++++
>  src/hcid.h    |  1 +
>  src/main.c    | 45 +++++++++++++++++++++++++++++++++++++++++++--
>  src/main.conf |  6 ++++++
>  4 files changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index f072541aad35..553db4aa8b1f 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -4169,6 +4169,7 @@ static void load_default_system_params(struct btd_adapter *adapter)
>                 struct mgmt_tlv entry;
>                 union {
>                         uint16_t u16;
> +                       uint8_t  u8;
>                 };
>         } __packed *params;
>         uint16_t i = 0;
> @@ -4436,6 +4437,15 @@ static void load_default_system_params(struct btd_adapter *adapter)
>                 len += sizeof(params[i].u16);
>         }
>
> +       if (main_opts.default_params.enable_advmon_interleave_scan != 0xFF) {
> +               params[i].entry.type = 0x001f;
> +               params[i].entry.length = sizeof(params[i].u8);
> +               params[i].u8 =
> +                       main_opts.default_params.enable_advmon_interleave_scan;
> +               ++i;
> +               len += sizeof(params[i].u8);
> +       }
> +

We could perhaps have a zero length here so that just the presence of
the type indicates it should be enabled, how about that?

>         err = mgmt_send(adapter->mgmt, MGMT_OP_SET_DEF_SYSTEM_CONFIG,
>                         adapter->dev_id, len, params, NULL, NULL, NULL);
>         if (!err)
> diff --git a/src/hcid.h b/src/hcid.h
> index 713eab8e03ae..34d22b3470f9 100644
> --- a/src/hcid.h
> +++ b/src/hcid.h
> @@ -83,6 +83,7 @@ struct main_opts {
>
>                 uint16_t        advmon_allowlist_scan_duration;
>                 uint16_t        advmon_no_filter_scan_duration;
> +               uint8_t         enable_advmon_interleave_scan;
>         } default_params;
>
>
> diff --git a/src/main.c b/src/main.c
> index 38f193a093df..274af6bdcc0f 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -112,6 +112,7 @@ static const char *controller_options[] = {
>         "LEAutoconnecttimeout",
>         "AdvMonAllowlistScanDuration",
>         "AdvMonNoFilterScanDuration",
> +       "EnableAdvMonInterleaveScan",
>         NULL
>  };
>
> @@ -307,130 +308,169 @@ static void parse_controller_config(GKeyFile *config)
>  {
>         static const struct {
>                 const char * const val_name;
> -               uint16_t * const val;
> +               void * const val;
> +               const size_t size;
>                 const uint16_t min;
>                 const uint16_t max;
>         } params[] = {
>                 { "BRPageScanType",
>                   &main_opts.default_params.br_page_scan_type,
> +                 sizeof(main_opts.default_params.br_page_scan_type),
>                   0,
>                   1},
>                 { "BRPageScanInterval",
>                   &main_opts.default_params.br_page_scan_interval,
> +                 sizeof(main_opts.default_params.br_page_scan_interval),
>                   0x0012,
>                   0x1000},
>                 { "BRPageScanWindow",
>                   &main_opts.default_params.br_page_scan_win,
> +                 sizeof(main_opts.default_params.br_page_scan_win),
>                   0x0011,
>                   0x1000},
>                 { "BRInquiryScanType",
>                   &main_opts.default_params.br_scan_type,
> +                 sizeof(main_opts.default_params.br_scan_type),
>                   0,
>                   1},
>                 { "BRInquiryScanInterval",
>                   &main_opts.default_params.br_scan_interval,
> +                 sizeof(main_opts.default_params.br_scan_interval),
>                   0x0012,
>                   0x1000},
>                 { "BRInquiryScanWindow",
>                   &main_opts.default_params.br_scan_win,
> +                 sizeof(main_opts.default_params.br_scan_win),
>                   0x0011,
>                   0x1000},
>                 { "BRLinkSupervisionTimeout",
>                   &main_opts.default_params.br_link_supervision_timeout,
> +                 sizeof(main_opts.default_params.br_link_supervision_timeout),
>                   0x0001,
>                   0xFFFF},
>                 { "BRPageTimeout",
>                   &main_opts.default_params.br_page_timeout,
> +                 sizeof(main_opts.default_params.br_page_scan_win),
>                   0x0001,
>                   0xFFFF},
>                 { "BRMinSniffInterval",
>                   &main_opts.default_params.br_min_sniff_interval,
> +                 sizeof(main_opts.default_params.br_min_sniff_interval),
>                   0x0001,
>                   0xFFFE},
>                 { "BRMaxSniffInterval",
>                   &main_opts.default_params.br_max_sniff_interval,
> +                 sizeof(main_opts.default_params.br_max_sniff_interval),
>                   0x0001,
>                   0xFFFE},
>                 { "LEMinAdvertisementInterval",
>                   &main_opts.default_params.le_min_adv_interval,
> +                 sizeof(main_opts.default_params.le_min_adv_interval),
>                   0x0020,
>                   0x4000},
>                 { "LEMaxAdvertisementInterval",
>                   &main_opts.default_params.le_max_adv_interval,
> +                 sizeof(main_opts.default_params.le_max_adv_interval),
>                   0x0020,
>                   0x4000},
>                 { "LEMultiAdvertisementRotationInterval",
>                   &main_opts.default_params.le_multi_adv_rotation_interval,
> +                 sizeof(main_opts.default_params.
> +                                               le_multi_adv_rotation_interval),
>                   0x0001,
>                   0xFFFF},
>                 { "LEScanIntervalAutoConnect",
>                   &main_opts.default_params.le_scan_interval_autoconnect,
> +                 sizeof(main_opts.default_params.le_scan_interval_autoconnect),
>                   0x0004,
>                   0x4000},
>                 { "LEScanWindowAutoConnect",
>                   &main_opts.default_params.le_scan_win_autoconnect,
> +                 sizeof(main_opts.default_params.le_scan_win_autoconnect),
>                   0x0004,
>                   0x4000},
>                 { "LEScanIntervalSuspend",
>                   &main_opts.default_params.le_scan_interval_suspend,
> +                 sizeof(main_opts.default_params.le_scan_interval_suspend),
>                   0x0004,
>                   0x4000},
>                 { "LEScanWindowSuspend",
>                   &main_opts.default_params.le_scan_win_suspend,
> +                 sizeof(main_opts.default_params.le_scan_win_suspend),
>                   0x0004,
>                   0x4000},
>                 { "LEScanIntervalDiscovery",
>                   &main_opts.default_params.le_scan_interval_discovery,
> +                 sizeof(main_opts.default_params.le_scan_interval_discovery),
>                   0x0004,
>                   0x4000},
>                 { "LEScanWindowDiscovery",
>                   &main_opts.default_params.le_scan_win_discovery,
> +                 sizeof(main_opts.default_params.le_scan_win_discovery),
>                   0x0004,
>                   0x4000},
>                 { "LEScanIntervalAdvMonitor",
>                   &main_opts.default_params.le_scan_interval_adv_monitor,
> +                 sizeof(main_opts.default_params.le_scan_interval_adv_monitor),
>                   0x0004,
>                   0x4000},
>                 { "LEScanWindowAdvMonitor",
>                   &main_opts.default_params.le_scan_win_adv_monitor,
> +                 sizeof(main_opts.default_params.le_scan_win_adv_monitor),
>                   0x0004,
>                   0x4000},
>                 { "LEScanIntervalConnect",
>                   &main_opts.default_params.le_scan_interval_connect,
> +                 sizeof(main_opts.default_params.le_scan_interval_connect),
>                   0x0004,
>                   0x4000},
>                 { "LEScanWindowConnect",
>                   &main_opts.default_params.le_scan_win_connect,
> +                 sizeof(main_opts.default_params.le_scan_win_connect),
>                   0x0004,
>                   0x4000},
>                 { "LEMinConnectionInterval",
>                   &main_opts.default_params.le_min_conn_interval,
> +                 sizeof(main_opts.default_params.le_min_conn_interval),
>                   0x0006,
>                   0x0C80},
>                 { "LEMaxConnectionInterval",
>                   &main_opts.default_params.le_max_conn_interval,
> +                 sizeof(main_opts.default_params.le_max_conn_interval),
>                   0x0006,
>                   0x0C80},
>                 { "LEConnectionLatency",
>                   &main_opts.default_params.le_conn_latency,
> +                 sizeof(main_opts.default_params.le_conn_latency),
>                   0x0000,
>                   0x01F3},
>                 { "LEConnectionSupervisionTimeout",
>                   &main_opts.default_params.le_conn_lsto,
> +                 sizeof(main_opts.default_params.le_conn_lsto),
>                   0x000A,
>                   0x0C80},
>                 { "LEAutoconnecttimeout",
>                   &main_opts.default_params.le_autoconnect_timeout,
> +                 sizeof(main_opts.default_params.le_autoconnect_timeout),
>                   0x0001,
>                   0x4000},
>                 { "AdvMonAllowlistScanDuration",
>                   &main_opts.default_params.advmon_allowlist_scan_duration,
> +                 sizeof(main_opts.default_params.
> +                                               advmon_allowlist_scan_duration),
>                   1,
>                   10000},
>                 { "AdvMonNoFilterScanDuration",
>                   &main_opts.default_params.advmon_no_filter_scan_duration,
> +                 sizeof(main_opts.default_params.br_page_scan_win),
>                   1,
>                   10000},
> +               { "EnableAdvMonInterleaveScan",
> +                 &main_opts.default_params.enable_advmon_interleave_scan,
> +                 sizeof(main_opts.default_params.
> +                                               enable_advmon_interleave_scan),
> +                 0,
> +                 1},
>         };
>         uint16_t i;
>
> @@ -449,7 +489,7 @@ static void parse_controller_config(GKeyFile *config)
>
>                         val = MAX(val, params[i].min);
>                         val = MIN(val, params[i].max);
> -                       *params[i].val = val;
> +                       memcpy(params[i].val, &val, params[i].size);
>                         ++main_opts.default_params.num_entries;
>                 }
>         }
> @@ -698,6 +738,7 @@ static void init_defaults(void)
>         main_opts.default_params.num_entries = 0;
>         main_opts.default_params.br_page_scan_type = 0xFFFF;
>         main_opts.default_params.br_scan_type = 0xFFFF;
> +       main_opts.default_params.enable_advmon_interleave_scan = 0xFF;
>
>         if (sscanf(VERSION, "%hhu.%hhu", &major, &minor) != 2)
>                 return;
> diff --git a/src/main.conf b/src/main.conf
> index 3b341f44c9cf..82ffc5813204 100644
> --- a/src/main.conf
> +++ b/src/main.conf
> @@ -159,6 +159,12 @@
>  # Default: 500
>  #AdvMonNoFilterScanDuration=
>
> +# Enable/Disable Advertisement Monitor interleave scan for power saving.
> +# 0: disable
> +# 1: enable
> +# Defaults to 0
> +#EnableAdvMonInterleaveScan=
> +
>  [GATT]
>  # GATT attribute cache.
>  # Possible values:
> --
> 2.28.0.709.gb0816b6eb0-goog
>


-- 
Luiz Augusto von Dentz