By default for instance 0, local name will be added for scan rsp
data, but currently the property is set as non scannable and hence
Set Adv parameter fails with Invalid parameter.
< HCI Command: LE Set Extended Advertising Parameters (0x08|0x0036) plen 25
Handle: 0x00
Properties: 0x0010
Use legacy advertising PDUs: ADV_NONCONN_IND
Min advertising interval: 1280.000 msec (0x0800)
Max advertising interval: 1280.000 msec (0x0800)
Channel map: 37, 38, 39 (0x07)
Own address type: Random (0x01)
Peer address type: Public (0x00)
Peer address: 00:00:00:00:00:00 (OUI 00-00-00)
Filter policy: Allow Scan Request from Any, Allow Connect Request from Any (0x00)
TX power: 127 dbm (0x7f)
Primary PHY: LE 1M (0x01)
Secondary max skip: 0x00
Secondary PHY: LE 1M (0x01)
SID: 0x00
Scan request notifications: Disabled (0x00)
> HCI Event: Command Complete (0x0e) plen 5
LE Set Extended Advertising Parameters (0x08|0x0036) ncmd 1
Status: Success (0x00)
TX power (selected): 7 dbm (0x07)
< HCI Command: LE Set Extended Scan Response Data (0x08|0x0038) plen 35
Handle: 0x00
Operation: Complete scan response data (0x03)
Fragment preference: Minimize fragmentation (0x01)
Data length: 0x0d
Name (short): localhost.
> HCI Event: Command Complete (0x0e) plen 4
LE Set Extended Scan Response Data (0x08|0x0038) ncmd 1
Status: Invalid HCI Command Parameters (0x12)
This patch makes the instance 0 scannable by default.
< HCI Command: LE Set Extended Advertising Parameters (0x08|0x0036) plen 25
Handle: 0x00
Properties: 0x0012
Scannable
Use legacy advertising PDUs: ADV_SCAN_IND
Min advertising interval: 1280.000 msec (0x0800)
Max advertising interval: 1280.000 msec (0x0800)
Channel map: 37, 38, 39 (0x07)
Own address type: Random (0x01)
Peer address type: Public (0x00)
Peer address: 00:00:00:00:00:00 (OUI 00-00-00)
Filter policy: Allow Scan Request from Any, Allow Connect Request from Any (0x00)
TX power: 127 dbm (0x7f)
Primary PHY: LE 1M (0x01)
Secondary max skip: 0x00
Secondary PHY: LE 1M (0x01)
SID: 0x00
Scan request notifications: Disabled (0x00)
> HCI Event: Command Complete (0x0e) plen 5
LE Set Extended Advertising Parameters (0x08|0x0036) ncmd 1
Status: Success (0x00)
TX power (selected): 7 dbm (0x07)
< HCI Command: LE Set Extended Scan Response Data (0x08|0x0038) plen 35
Handle: 0x00
Operation: Complete scan response data (0x03)
Fragment preference: Minimize fragmentation (0x01)
Data length: 0x0d
Name (short): localhost.
> HCI Event: Command Complete (0x0e) plen 4
LE Set Extended Scan Response Data (0x08|0x0038) ncmd 1
Status: Success (0x00)
Signed-off-by: Jaganath Kanakkassery <[email protected]>
---
net/bluetooth/hci_request.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 7f6a581..362b1ca 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1601,7 +1601,12 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)
cp.evt_properties = cpu_to_le16(LE_EXT_ADV_CONN_IND);
else
cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_IND);
- } else if (get_adv_instance_scan_rsp_len(hdev, instance)) {
+ } else if (!instance || get_adv_instance_scan_rsp_len(hdev, instance)) {
+ /* Always set scannable for instance 0, as scan rsp data will
+ * be set by default with local name. For other instances set
+ * scannable based on whether scan rsp data is there for the
+ * respective instance
+ */
if (secondary_adv)
cp.evt_properties = cpu_to_le16(LE_EXT_ADV_SCAN_IND);
else
--
2.7.4
Hi Jaganath,
> By default for instance 0, local name will be added for scan rsp
> data, but currently the property is set as non scannable and hence
> Set Adv parameter fails with Invalid parameter.
can you be a bit more specific why this is the correct behavior. We must have documented in mgmt-api.txt somewhere, right? Or is this something that used to be correct, but we broke it with adding extended advertising?
> < HCI Command: LE Set Extended Advertising Parameters (0x08|0x0036) plen 25
> Handle: 0x00
> Properties: 0x0010
> Use legacy advertising PDUs: ADV_NONCONN_IND
> Min advertising interval: 1280.000 msec (0x0800)
> Max advertising interval: 1280.000 msec (0x0800)
> Channel map: 37, 38, 39 (0x07)
> Own address type: Random (0x01)
> Peer address type: Public (0x00)
> Peer address: 00:00:00:00:00:00 (OUI 00-00-00)
> Filter policy: Allow Scan Request from Any, Allow Connect Request from Any (0x00)
> TX power: 127 dbm (0x7f)
> Primary PHY: LE 1M (0x01)
> Secondary max skip: 0x00
> Secondary PHY: LE 1M (0x01)
> SID: 0x00
> Scan request notifications: Disabled (0x00)
>> HCI Event: Command Complete (0x0e) plen 5
> LE Set Extended Advertising Parameters (0x08|0x0036) ncmd 1
> Status: Success (0x00)
> TX power (selected): 7 dbm (0x07)
>
> < HCI Command: LE Set Extended Scan Response Data (0x08|0x0038) plen 35
> Handle: 0x00
> Operation: Complete scan response data (0x03)
> Fragment preference: Minimize fragmentation (0x01)
> Data length: 0x0d
> Name (short): localhost.
>> HCI Event: Command Complete (0x0e) plen 4
> LE Set Extended Scan Response Data (0x08|0x0038) ncmd 1
> Status: Invalid HCI Command Parameters (0x12)
>
> This patch makes the instance 0 scannable by default.
>
> < HCI Command: LE Set Extended Advertising Parameters (0x08|0x0036) plen 25
> Handle: 0x00
> Properties: 0x0012
> Scannable
> Use legacy advertising PDUs: ADV_SCAN_IND
> Min advertising interval: 1280.000 msec (0x0800)
> Max advertising interval: 1280.000 msec (0x0800)
> Channel map: 37, 38, 39 (0x07)
> Own address type: Random (0x01)
> Peer address type: Public (0x00)
> Peer address: 00:00:00:00:00:00 (OUI 00-00-00)
> Filter policy: Allow Scan Request from Any, Allow Connect Request from Any (0x00)
> TX power: 127 dbm (0x7f)
> Primary PHY: LE 1M (0x01)
> Secondary max skip: 0x00
> Secondary PHY: LE 1M (0x01)
> SID: 0x00
> Scan request notifications: Disabled (0x00)
>> HCI Event: Command Complete (0x0e) plen 5
> LE Set Extended Advertising Parameters (0x08|0x0036) ncmd 1
> Status: Success (0x00)
> TX power (selected): 7 dbm (0x07)
>
> < HCI Command: LE Set Extended Scan Response Data (0x08|0x0038) plen 35
> Handle: 0x00
> Operation: Complete scan response data (0x03)
> Fragment preference: Minimize fragmentation (0x01)
> Data length: 0x0d
> Name (short): localhost.
>> HCI Event: Command Complete (0x0e) plen 4
> LE Set Extended Scan Response Data (0x08|0x0038) ncmd 1
> Status: Success (0x00)
>
> Signed-off-by: Jaganath Kanakkassery <[email protected]>
> ---
> net/bluetooth/hci_request.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index 7f6a581..362b1ca 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -1601,7 +1601,12 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)
> cp.evt_properties = cpu_to_le16(LE_EXT_ADV_CONN_IND);
> else
> cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_IND);
> - } else if (get_adv_instance_scan_rsp_len(hdev, instance)) {
> + } else if (!instance || get_adv_instance_scan_rsp_len(hdev, instance)) {
> + /* Always set scannable for instance 0, as scan rsp data will
> + * be set by default with local name. For other instances set
> + * scannable based on whether scan rsp data is there for the
> + * respective instance
> + */
> if (secondary_adv)
> cp.evt_properties = cpu_to_le16(LE_EXT_ADV_SCAN_IND);
So this comment is at a wrong location. And I am not convinced that we would actually depend on if scan_rsp data is present. We might want to set scannable all the time if that is what we decided for instance 0.
Regards
Marcel
Hi Marcel
On Thu, Oct 24, 2019 at 6:31 PM Marcel Holtmann <[email protected]> wrote:
>
> Hi Jaganath,
>
> > By default for instance 0, local name will be added for scan rsp
> > data, but currently the property is set as non scannable and hence
> > Set Adv parameter fails with Invalid parameter.
>
> can you be a bit more specific why this is the correct behavior. We must have documented in mgmt-api.txt somewhere, right? Or is this something that used to be correct, but we broke it with adding extended advertising?
It seems to be functionally broken in legacy adv as well as we set scan rsp
for instance 0 always but Adv Properties is set to non connectable (if
connectable
is not set). Even we need to fix mgmt-api.txt as well as you pointed out.
But in legacy case commands do not fail as scan rsp can be set even before
Set Adv Param where as in extended case Adv param for an Adv set has to be set
first before sending Scan rsp and other commands for that particular Adv set.
>
> > < HCI Command: LE Set Extended Advertising Parameters (0x08|0x0036) plen 25
> > Handle: 0x00
> > Properties: 0x0010
> > Use legacy advertising PDUs: ADV_NONCONN_IND
> > Min advertising interval: 1280.000 msec (0x0800)
> > Max advertising interval: 1280.000 msec (0x0800)
> > Channel map: 37, 38, 39 (0x07)
> > Own address type: Random (0x01)
> > Peer address type: Public (0x00)
> > Peer address: 00:00:00:00:00:00 (OUI 00-00-00)
> > Filter policy: Allow Scan Request from Any, Allow Connect Request from Any (0x00)
> > TX power: 127 dbm (0x7f)
> > Primary PHY: LE 1M (0x01)
> > Secondary max skip: 0x00
> > Secondary PHY: LE 1M (0x01)
> > SID: 0x00
> > Scan request notifications: Disabled (0x00)
> >> HCI Event: Command Complete (0x0e) plen 5
> > LE Set Extended Advertising Parameters (0x08|0x0036) ncmd 1
> > Status: Success (0x00)
> > TX power (selected): 7 dbm (0x07)
> >
> > < HCI Command: LE Set Extended Scan Response Data (0x08|0x0038) plen 35
> > Handle: 0x00
> > Operation: Complete scan response data (0x03)
> > Fragment preference: Minimize fragmentation (0x01)
> > Data length: 0x0d
> > Name (short): localhost.
> >> HCI Event: Command Complete (0x0e) plen 4
> > LE Set Extended Scan Response Data (0x08|0x0038) ncmd 1
> > Status: Invalid HCI Command Parameters (0x12)
> >
> > This patch makes the instance 0 scannable by default.
> >
> > < HCI Command: LE Set Extended Advertising Parameters (0x08|0x0036) plen 25
> > Handle: 0x00
> > Properties: 0x0012
> > Scannable
> > Use legacy advertising PDUs: ADV_SCAN_IND
> > Min advertising interval: 1280.000 msec (0x0800)
> > Max advertising interval: 1280.000 msec (0x0800)
> > Channel map: 37, 38, 39 (0x07)
> > Own address type: Random (0x01)
> > Peer address type: Public (0x00)
> > Peer address: 00:00:00:00:00:00 (OUI 00-00-00)
> > Filter policy: Allow Scan Request from Any, Allow Connect Request from Any (0x00)
> > TX power: 127 dbm (0x7f)
> > Primary PHY: LE 1M (0x01)
> > Secondary max skip: 0x00
> > Secondary PHY: LE 1M (0x01)
> > SID: 0x00
> > Scan request notifications: Disabled (0x00)
> >> HCI Event: Command Complete (0x0e) plen 5
> > LE Set Extended Advertising Parameters (0x08|0x0036) ncmd 1
> > Status: Success (0x00)
> > TX power (selected): 7 dbm (0x07)
> >
> > < HCI Command: LE Set Extended Scan Response Data (0x08|0x0038) plen 35
> > Handle: 0x00
> > Operation: Complete scan response data (0x03)
> > Fragment preference: Minimize fragmentation (0x01)
> > Data length: 0x0d
> > Name (short): localhost.
> >> HCI Event: Command Complete (0x0e) plen 4
> > LE Set Extended Scan Response Data (0x08|0x0038) ncmd 1
> > Status: Success (0x00)
> >
> > Signed-off-by: Jaganath Kanakkassery <[email protected]>
> > ---
> > net/bluetooth/hci_request.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> > index 7f6a581..362b1ca 100644
> > --- a/net/bluetooth/hci_request.c
> > +++ b/net/bluetooth/hci_request.c
> > @@ -1601,7 +1601,12 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)
> > cp.evt_properties = cpu_to_le16(LE_EXT_ADV_CONN_IND);
> > else
> > cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_IND);
> > - } else if (get_adv_instance_scan_rsp_len(hdev, instance)) {
> > + } else if (!instance || get_adv_instance_scan_rsp_len(hdev, instance)) {
> > + /* Always set scannable for instance 0, as scan rsp data will
> > + * be set by default with local name. For other instances set
> > + * scannable based on whether scan rsp data is there for the
> > + * respective instance
> > + */
> > if (secondary_adv)
> > cp.evt_properties = cpu_to_le16(LE_EXT_ADV_SCAN_IND);
>
> So this comment is at a wrong location. And I am not convinced that we would actually depend on if scan_rsp data is present. We might want to set scannable all the time if that is what we decided for instance 0.
>
For other instances Scan rsp data will be present based on if user is sets
the same (Using Add Advertising).
So Do you want to set scannable always for all instances (even if user does
not set scan rsp data) and hence in that case we will not use non connectable
property at all?
Thanks,
Jaganath
Hi Jaganath,
>>> By default for instance 0, local name will be added for scan rsp
>>> data, but currently the property is set as non scannable and hence
>>> Set Adv parameter fails with Invalid parameter.
>>
>> can you be a bit more specific why this is the correct behavior. We must have documented in mgmt-api.txt somewhere, right? Or is this something that used to be correct, but we broke it with adding extended advertising?
>
> It seems to be functionally broken in legacy adv as well as we set scan rsp
> for instance 0 always but Adv Properties is set to non connectable (if
> connectable
> is not set). Even we need to fix mgmt-api.txt as well as you pointed out.
>
> But in legacy case commands do not fail as scan rsp can be set even before
> Set Adv Param where as in extended case Adv param for an Adv set has to be set
> first before sending Scan rsp and other commands for that particular Adv set.
how does this intersect with the patches that Luiz was sending earlier?
Regards
Marcel
Hi Marcel,
On Sat, Oct 26, 2019 at 11:02 AM Marcel Holtmann <[email protected]> wrote:
>
> Hi Jaganath,
>
> >>> By default for instance 0, local name will be added for scan rsp
> >>> data, but currently the property is set as non scannable and hence
> >>> Set Adv parameter fails with Invalid parameter.
> >>
> >> can you be a bit more specific why this is the correct behavior. We must have documented in mgmt-api.txt somewhere, right? Or is this something that used to be correct, but we broke it with adding extended advertising?
> >
> > It seems to be functionally broken in legacy adv as well as we set scan rsp
> > for instance 0 always but Adv Properties is set to non connectable (if
> > connectable
> > is not set). Even we need to fix mgmt-api.txt as well as you pointed out.
> >
> > But in legacy case commands do not fail as scan rsp can be set even before
> > Set Adv Param where as in extended case Adv param for an Adv set has to be set
> > first before sending Scan rsp and other commands for that particular Adv set.
>
> how does this intersect with the patches that Luiz was sending earlier?
>
I think Luiz patches covers both the issues.
Thanks,
Jaganath
Hi Jaganath,
On Mon, Oct 28, 2019 at 6:50 PM Jaganath K <[email protected]> wrote:
>
> Hi Marcel,
>
> On Sat, Oct 26, 2019 at 11:02 AM Marcel Holtmann <[email protected]> wrote:
> >
> > Hi Jaganath,
> >
> > >>> By default for instance 0, local name will be added for scan rsp
> > >>> data, but currently the property is set as non scannable and hence
> > >>> Set Adv parameter fails with Invalid parameter.
> > >>
> > >> can you be a bit more specific why this is the correct behavior. We must have documented in mgmt-api.txt somewhere, right? Or is this something that used to be correct, but we broke it with adding extended advertising?
> > >
> > > It seems to be functionally broken in legacy adv as well as we set scan rsp
> > > for instance 0 always but Adv Properties is set to non connectable (if
> > > connectable
> > > is not set). Even we need to fix mgmt-api.txt as well as you pointed out.
> > >
> > > But in legacy case commands do not fail as scan rsp can be set even before
> > > Set Adv Param where as in extended case Adv param for an Adv set has to be set
> > > first before sending Scan rsp and other commands for that particular Adv set.
> >
> > how does this intersect with the patches that Luiz was sending earlier?
> >
> I think Luiz patches covers both the issues.
Yep, my patches were actually targeting the same issues, though you
had a better description, anyway it is a fix nevertheless.
--
Luiz Augusto von Dentz