| Message ID | 20210303111817.Bluez.2.I45b896f4512038309cbeab7a01f51e503141edab@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Bluetooth: Fix scannable broadcast advertising on extended APIs | expand |
Hi Daniel, On Wed, Mar 3, 2021 at 11:20 AM Daniel Winkler <danielwinkler@google.com> wrote: > > In order for the advertising parameters hci request to indicate that an > advertising set uses a scannable PDU, we pass a scannable flag along > with the initial parameters MGMT request. > > Without this patch, a broadcast advertisement with a scan response will > either be rejected by the controller, or will ignore the requested scan > response. The patch is tested by performing the above and confirming > that the scan response is retrievable from a peer as expected. > > Reviewed-by: Alain Michaud <alainm@chromium.org> > Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org> > > --- > > lib/mgmt.h | 1 + > src/advertising.c | 4 ++++ > 2 files changed, 5 insertions(+) > > diff --git a/lib/mgmt.h b/lib/mgmt.h > index 76a03c9c2..7b1b9ab54 100644 > --- a/lib/mgmt.h > +++ b/lib/mgmt.h > @@ -507,6 +507,7 @@ struct mgmt_rp_add_advertising { > #define MGMT_ADV_PARAM_TIMEOUT (1 << 13) > #define MGMT_ADV_PARAM_INTERVALS (1 << 14) > #define MGMT_ADV_PARAM_TX_POWER (1 << 15) > +#define MGMT_ADV_PARAM_SCAN_RSP (1 << 16) > > #define MGMT_OP_REMOVE_ADVERTISING 0x003F > struct mgmt_cp_remove_advertising { > diff --git a/src/advertising.c b/src/advertising.c > index f3dc357a1..38cef565f 100644 > --- a/src/advertising.c > +++ b/src/advertising.c > @@ -945,6 +945,10 @@ static int refresh_extended_adv(struct btd_adv_client *client, > return -EINVAL; > } > > + /* Indicate that this instance will be configured as scannable */ > + if (client->scan_rsp_len) > + flags |= MGMT_ADV_PARAM_SCAN_RSP; > + Don't we need to check if the flag is actually supported by the kernel? > cp.flags = htobl(flags); For new code it is prefered to use the function from src/shared/util.h (cpu_to_*). > mgmt_ret = mgmt_send(client->manager->mgmt, MGMT_OP_ADD_EXT_ADV_PARAMS, > -- > 2.30.1.766.gb4fecdf3b7-goog >
Hello Luiz, Thank you for the catch and suggestion. I have just sent out a v2 to address your recommendations. Thanks! Daniel On Thu, Mar 4, 2021 at 10:59 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Daniel, > > On Wed, Mar 3, 2021 at 11:20 AM Daniel Winkler <danielwinkler@google.com> wrote: > > > > In order for the advertising parameters hci request to indicate that an > > advertising set uses a scannable PDU, we pass a scannable flag along > > with the initial parameters MGMT request. > > > > Without this patch, a broadcast advertisement with a scan response will > > either be rejected by the controller, or will ignore the requested scan > > response. The patch is tested by performing the above and confirming > > that the scan response is retrievable from a peer as expected. > > > > Reviewed-by: Alain Michaud <alainm@chromium.org> > > Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org> > > > > --- > > > > lib/mgmt.h | 1 + > > src/advertising.c | 4 ++++ > > 2 files changed, 5 insertions(+) > > > > diff --git a/lib/mgmt.h b/lib/mgmt.h > > index 76a03c9c2..7b1b9ab54 100644 > > --- a/lib/mgmt.h > > +++ b/lib/mgmt.h > > @@ -507,6 +507,7 @@ struct mgmt_rp_add_advertising { > > #define MGMT_ADV_PARAM_TIMEOUT (1 << 13) > > #define MGMT_ADV_PARAM_INTERVALS (1 << 14) > > #define MGMT_ADV_PARAM_TX_POWER (1 << 15) > > +#define MGMT_ADV_PARAM_SCAN_RSP (1 << 16) > > > > #define MGMT_OP_REMOVE_ADVERTISING 0x003F > > struct mgmt_cp_remove_advertising { > > diff --git a/src/advertising.c b/src/advertising.c > > index f3dc357a1..38cef565f 100644 > > --- a/src/advertising.c > > +++ b/src/advertising.c > > @@ -945,6 +945,10 @@ static int refresh_extended_adv(struct btd_adv_client *client, > > return -EINVAL; > > } > > > > + /* Indicate that this instance will be configured as scannable */ > > + if (client->scan_rsp_len) > > + flags |= MGMT_ADV_PARAM_SCAN_RSP; > > + > > Don't we need to check if the flag is actually supported by the kernel? > > > cp.flags = htobl(flags); > > For new code it is prefered to use the function from src/shared/util.h > (cpu_to_*). > > > mgmt_ret = mgmt_send(client->manager->mgmt, MGMT_OP_ADD_EXT_ADV_PARAMS, > > -- > > 2.30.1.766.gb4fecdf3b7-goog > > > > > -- > Luiz Augusto von Dentz
diff --git a/lib/mgmt.h b/lib/mgmt.h index 76a03c9c2..7b1b9ab54 100644 --- a/lib/mgmt.h +++ b/lib/mgmt.h @@ -507,6 +507,7 @@ struct mgmt_rp_add_advertising { #define MGMT_ADV_PARAM_TIMEOUT (1 << 13) #define MGMT_ADV_PARAM_INTERVALS (1 << 14) #define MGMT_ADV_PARAM_TX_POWER (1 << 15) +#define MGMT_ADV_PARAM_SCAN_RSP (1 << 16) #define MGMT_OP_REMOVE_ADVERTISING 0x003F struct mgmt_cp_remove_advertising { diff --git a/src/advertising.c b/src/advertising.c index f3dc357a1..38cef565f 100644 --- a/src/advertising.c +++ b/src/advertising.c @@ -945,6 +945,10 @@ static int refresh_extended_adv(struct btd_adv_client *client, return -EINVAL; } + /* Indicate that this instance will be configured as scannable */ + if (client->scan_rsp_len) + flags |= MGMT_ADV_PARAM_SCAN_RSP; + cp.flags = htobl(flags); mgmt_ret = mgmt_send(client->manager->mgmt, MGMT_OP_ADD_EXT_ADV_PARAMS,