Device type is input to decide how to establish the L2CAP connection.
For LE devices fixed channel ID 4 is set, otherwise PSM should be set
for basic rate connections. LE public and random constants are misused
for discovery, however I don't see advantage of defining basic rate and
LE values only.
Based on jhe/master:
git://git.kernel.org/pub/scm/linux/kernel/git/jh/linux-2.6.git
Opinions?
---
include/net/bluetooth/bluetooth.h | 6 ++++++
include/net/bluetooth/hci_core.h | 4 ++--
include/net/bluetooth/mgmt.h | 1 +
net/bluetooth/hci_event.c | 16 ++++++++--------
net/bluetooth/mgmt.c | 5 +++--
5 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 4375043..b91eec9 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -94,6 +94,12 @@ typedef struct {
#define BDADDR_ANY (&(bdaddr_t) {{0, 0, 0, 0, 0, 0}})
#define BDADDR_LOCAL (&(bdaddr_t) {{0, 0, 0, 0xff, 0xff, 0xff}})
+enum {
+ BDADDR_TYPE_BR = 0,
+ BDADDR_TYPE_LE_PUBLIC,
+ BRADDR_TYPE_LE_RANDOM
+};
+
/* Copy, swap, convert BD Address */
static inline int bacmp(bdaddr_t *ba1, bdaddr_t *ba2)
{
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8200704..b357346 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -808,8 +808,8 @@ int mgmt_auth_failed(u16 index, bdaddr_t *bdaddr, u8 status);
int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status);
int mgmt_read_local_oob_data_reply_complete(u16 index, u8 *hash, u8 *randomizer,
u8 status);
-int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 *dev_class, s8 rssi,
- u8 *eir);
+int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 bdaddr_type,
+ u8 *dev_class, s8 rssi, u8 *eir);
int mgmt_remote_name(u16 index, bdaddr_t *bdaddr, u8 *name);
int mgmt_periodic_inq_complete(u16 index, u8 status);
int mgmt_exit_periodic_inq_complete(u16 index, u8 status);
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 2a70541..c299f6a 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -281,6 +281,7 @@ struct mgmt_ev_local_name_changed {
#define MGMT_EV_DEVICE_FOUND 0x0012
struct mgmt_ev_device_found {
bdaddr_t bdaddr;
+ __u8 bdaddr_type;
__u8 dev_class[3];
__s8 rssi;
__u8 eir[HCI_MAX_EIR_LENGTH];
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ded8e0c..997d8f6 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1285,8 +1285,8 @@ static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *
if (!test_bit(HCI_MGMT, &hdev->flags))
continue;
- mgmt_device_found(hdev->id, &info->bdaddr, info->dev_class, 0,
- NULL);
+ mgmt_device_found(hdev->id, &info->bdaddr, BDADDR_TYPE_BR,
+ info->dev_class, 0, NULL);
if (test_bit(HCI_DISCOVERY, &hdev->flags))
hci_add_found_device(hdev, &info->bdaddr, 0x00);
@@ -2257,8 +2257,8 @@ static inline void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev, struct
continue;
mgmt_device_found(hdev->id, &info->bdaddr,
- info->dev_class, info->rssi,
- NULL);
+ BDADDR_TYPE_BR, info->dev_class,
+ info->rssi, NULL);
if (!test_bit(HCI_DISCOVERY, &hdev->flags))
continue;
@@ -2283,8 +2283,8 @@ static inline void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev, struct
continue;
mgmt_device_found(hdev->id, &info->bdaddr,
- info->dev_class, info->rssi,
- NULL);
+ BDADDR_TYPE_BR, info->dev_class,
+ info->rssi, NULL);
if (!test_bit(HCI_DISCOVERY, &hdev->flags))
continue;
@@ -2465,8 +2465,8 @@ static inline void hci_extended_inquiry_result_evt(struct hci_dev *hdev, struct
if (!test_bit(HCI_MGMT, &hdev->flags))
continue;
- mgmt_device_found(hdev->id, &info->bdaddr, info->dev_class,
- info->rssi, info->data);
+ mgmt_device_found(hdev->id, &info->bdaddr, BDADDR_TYPE_BR,
+ info->dev_class, info->rssi, info->data);
if (!test_bit(HCI_DISCOVERY, &hdev->flags))
continue;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 2b20ee9..ad673f2 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2157,14 +2157,15 @@ int mgmt_read_local_oob_data_reply_complete(u16 index, u8 *hash, u8 *randomizer,
return err;
}
-int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 *dev_class, s8 rssi,
- u8 *eir)
+int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 bdaddr_type,
+ u8 *dev_class, s8 rssi, u8 *eir)
{
struct mgmt_ev_device_found ev;
memset(&ev, 0, sizeof(ev));
bacpy(&ev.bdaddr, bdaddr);
+ ev.bdaddr_type = bdaddr_type;
memcpy(ev.dev_class, dev_class, sizeof(ev.dev_class));
ev.rssi = rssi;
--
1.7.4.1
Hi Marcel/Johan/Gustavo,
On Sat, Apr 16, 2011 at 5:04 PM, Claudio Takahasi
<[email protected]> wrote:
> Hi Marcel,
>
> On Sat, Apr 16, 2011 at 7:00 AM, Johan Hedberg <[email protected]> wrote:
>> Hi Marcel,
>>
>> On Fri, Apr 15, 2011, Marcel Holtmann wrote:
>>> > > + BDADDR_TYPE_LE_PUBLIC,
>>> > > + BRADDR_TYPE_LE_RANDOM
>>> > > +};
>>>
>>> I am also not sure the we should have this BR/EDR differentiation since
>>> the specification only talks about public and random addresses. And we
>>> should follow the specification type value here. I am against
>>> introducing our enum here.
>>
>> The HCI specification only has values for public and random because
>> everywhere they are used it is already clear from the context (the HCI
>> command or event in question) if we're talking about LE or BR/EDR. We on
>> the other hand don't have this contextual information with the
>> mgmt_pair_device command. Saying "public" there could mean both BR/EDR
>> public or LE public, i.e. an enum with just two possible values is not
>> going to be of much use to us. Because of this difference between our
>> API and that of HCI I don't think it's fair to apply the HCI
>> convention/restriction to us.
>>
>> Johan
>>
>
> I added 3 values because it gives more flexibility. Possible use cases:
> - Whitelist needs the address type
> - SMP
> - As input to decide to store or not information about the device
> since private address can change every 15minutes
>
> At the moment we only need to know if the address is basic rate or LE
> to select the discovery type: SDP or LE Discovery primary. For
> pairing, Vinicius is using the kernel advertising cache to discover
> the address type, passing the address type could avoid wrong fallback
> to basic rate if the entry is not found in the cache.
>
> Claudio
>
Any objection to add the address type in the MGMT_EV_DEVICE_CONNECTED event?
Inside event.c there are a lot of get_adapter_and_device calls, for
some contexts it creates a new device object if it doesn't exist(eg:
incoming pairing). But the type is required to create a new device,
otherwise it will not be possible to trigger the reverse service
search. Obtain the type later based on the link key type will not
work, unless we create a device with unknown type to be able to call
the agent methods.
BTW, is there a reason why it is necessary to "force" device
creation(get_adapter_and_device option)? In my opinion we could create
the device(if it doesn't exist) it inside btd_event_conn_complete
only. There is a potential race condition: other application calling
RemoveDevice, but for this case reference counting should work.
Currently, controllers doesn't support simultaneous BR/EDR/LE, this is
another argument to export the address type or connection type through
management interface avoiding future changes on the API.
Regards,
Claudio.
Hi Marcel,
On Sat, Apr 16, 2011 at 7:00 AM, Johan Hedberg <[email protected]> wrote:
> Hi Marcel,
>
> On Fri, Apr 15, 2011, Marcel Holtmann wrote:
>> > > + BDADDR_TYPE_LE_PUBLIC,
>> > > + BRADDR_TYPE_LE_RANDOM
>> > > +};
>>
>> I am also not sure the we should have this BR/EDR differentiation since
>> the specification only talks about public and random addresses. And we
>> should follow the specification type value here. I am against
>> introducing our enum here.
>
> The HCI specification only has values for public and random because
> everywhere they are used it is already clear from the context (the HCI
> command or event in question) if we're talking about LE or BR/EDR. We on
> the other hand don't have this contextual information with the
> mgmt_pair_device command. Saying "public" there could mean both BR/EDR
> public or LE public, i.e. an enum with just two possible values is not
> going to be of much use to us. Because of this difference between our
> API and that of HCI I don't think it's fair to apply the HCI
> convention/restriction to us.
>
> Johan
>
I added 3 values because it gives more flexibility. Possible use cases:
- Whitelist needs the address type
- SMP
- As input to decide to store or not information about the device
since private address can change every 15minutes
At the moment we only need to know if the address is basic rate or LE
to select the discovery type: SDP or LE Discovery primary. For
pairing, Vinicius is using the kernel advertising cache to discover
the address type, passing the address type could avoid wrong fallback
to basic rate if the entry is not found in the cache.
Claudio
Hi Marcel,
On Fri, Apr 15, 2011, Marcel Holtmann wrote:
> > > + BDADDR_TYPE_LE_PUBLIC,
> > > + BRADDR_TYPE_LE_RANDOM
> > > +};
>
> I am also not sure the we should have this BR/EDR differentiation since
> the specification only talks about public and random addresses. And we
> should follow the specification type value here. I am against
> introducing our enum here.
The HCI specification only has values for public and random because
everywhere they are used it is already clear from the context (the HCI
command or event in question) if we're talking about LE or BR/EDR. We on
the other hand don't have this contextual information with the
mgmt_pair_device command. Saying "public" there could mean both BR/EDR
public or LE public, i.e. an enum with just two possible values is not
going to be of much use to us. Because of this difference between our
API and that of HCI I don't think it's fair to apply the HCI
convention/restriction to us.
Johan
Hi Claudio,
> > Device type is input to decide how to establish the L2CAP connection.
> > For LE devices fixed channel ID 4 is set, otherwise PSM should be set
> > for basic rate connections. LE public and random constants are misused
> > for discovery, however I don't see advantage of defining basic rate and
> > LE values only.
> >
> > Based on jhe/master:
> > git://git.kernel.org/pub/scm/linux/kernel/git/jh/linux-2.6.git
> >
> > Opinions?
> > ---
> > include/net/bluetooth/bluetooth.h | 6 ++++++
> > include/net/bluetooth/hci_core.h | 4 ++--
> > include/net/bluetooth/mgmt.h | 1 +
> > net/bluetooth/hci_event.c | 16 ++++++++--------
> > net/bluetooth/mgmt.c | 5 +++--
> > 5 files changed, 20 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > index 4375043..b91eec9 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -94,6 +94,12 @@ typedef struct {
> > #define BDADDR_ANY (&(bdaddr_t) {{0, 0, 0, 0, 0, 0}})
> > #define BDADDR_LOCAL (&(bdaddr_t) {{0, 0, 0, 0xff, 0xff, 0xff}})
> >
> > +enum {
> > + BDADDR_TYPE_BR = 0,
>
> As we going to differentiate this, can't we separate BR and BR/EDR?
> Or maybe not once most of the device today should be BR/EDR.
what is the difference between BR and BR/EDR exactly? I don't see one
and in general the specification calls this BR/EDR. So using BREDR seems
more correct.
> > + BDADDR_TYPE_LE_PUBLIC,
> > + BRADDR_TYPE_LE_RANDOM
> > +};
I am also not sure the we should have this BR/EDR differentiation since
the specification only talks about public and random addresses. And we
should follow the specification type value here. I am against
introducing our enum here.
Regards
Marcel
Hi Claudio,
* Claudio Takahasi <[email protected]> [2011-04-15 15:33:42 -0300]:
> Device type is input to decide how to establish the L2CAP connection.
> For LE devices fixed channel ID 4 is set, otherwise PSM should be set
> for basic rate connections. LE public and random constants are misused
> for discovery, however I don't see advantage of defining basic rate and
> LE values only.
>
> Based on jhe/master:
> git://git.kernel.org/pub/scm/linux/kernel/git/jh/linux-2.6.git
>
> Opinions?
> ---
> include/net/bluetooth/bluetooth.h | 6 ++++++
> include/net/bluetooth/hci_core.h | 4 ++--
> include/net/bluetooth/mgmt.h | 1 +
> net/bluetooth/hci_event.c | 16 ++++++++--------
> net/bluetooth/mgmt.c | 5 +++--
> 5 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 4375043..b91eec9 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -94,6 +94,12 @@ typedef struct {
> #define BDADDR_ANY (&(bdaddr_t) {{0, 0, 0, 0, 0, 0}})
> #define BDADDR_LOCAL (&(bdaddr_t) {{0, 0, 0, 0xff, 0xff, 0xff}})
>
> +enum {
> + BDADDR_TYPE_BR = 0,
As we going to differentiate this, can't we separate BR and BR/EDR?
Or maybe not once most of the device today should be BR/EDR.
> + BDADDR_TYPE_LE_PUBLIC,
> + BRADDR_TYPE_LE_RANDOM
> +};
> +
> /* Copy, swap, convert BD Address */
> static inline int bacmp(bdaddr_t *ba1, bdaddr_t *ba2)
> {
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 8200704..b357346 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -808,8 +808,8 @@ int mgmt_auth_failed(u16 index, bdaddr_t *bdaddr, u8 status);
> int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status);
> int mgmt_read_local_oob_data_reply_complete(u16 index, u8 *hash, u8 *randomizer,
> u8 status);
> -int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 *dev_class, s8 rssi,
> - u8 *eir);
> +int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 bdaddr_type,
> + u8 *dev_class, s8 rssi, u8 *eir);
> int mgmt_remote_name(u16 index, bdaddr_t *bdaddr, u8 *name);
> int mgmt_periodic_inq_complete(u16 index, u8 status);
> int mgmt_exit_periodic_inq_complete(u16 index, u8 status);
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 2a70541..c299f6a 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -281,6 +281,7 @@ struct mgmt_ev_local_name_changed {
> #define MGMT_EV_DEVICE_FOUND 0x0012
> struct mgmt_ev_device_found {
> bdaddr_t bdaddr;
> + __u8 bdaddr_type;
> __u8 dev_class[3];
> __s8 rssi;
> __u8 eir[HCI_MAX_EIR_LENGTH];
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index ded8e0c..997d8f6 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1285,8 +1285,8 @@ static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *
> if (!test_bit(HCI_MGMT, &hdev->flags))
> continue;
>
> - mgmt_device_found(hdev->id, &info->bdaddr, info->dev_class, 0,
> - NULL);
> + mgmt_device_found(hdev->id, &info->bdaddr, BDADDR_TYPE_BR,
> + info->dev_class, 0, NULL);
>
> if (test_bit(HCI_DISCOVERY, &hdev->flags))
> hci_add_found_device(hdev, &info->bdaddr, 0x00);
> @@ -2257,8 +2257,8 @@ static inline void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev, struct
> continue;
>
> mgmt_device_found(hdev->id, &info->bdaddr,
> - info->dev_class, info->rssi,
> - NULL);
> + BDADDR_TYPE_BR, info->dev_class,
> + info->rssi, NULL);
>
> if (!test_bit(HCI_DISCOVERY, &hdev->flags))
> continue;
> @@ -2283,8 +2283,8 @@ static inline void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev, struct
> continue;
>
> mgmt_device_found(hdev->id, &info->bdaddr,
> - info->dev_class, info->rssi,
> - NULL);
> + BDADDR_TYPE_BR, info->dev_class,
> + info->rssi, NULL);
>
> if (!test_bit(HCI_DISCOVERY, &hdev->flags))
> continue;
> @@ -2465,8 +2465,8 @@ static inline void hci_extended_inquiry_result_evt(struct hci_dev *hdev, struct
> if (!test_bit(HCI_MGMT, &hdev->flags))
> continue;
>
> - mgmt_device_found(hdev->id, &info->bdaddr, info->dev_class,
> - info->rssi, info->data);
> + mgmt_device_found(hdev->id, &info->bdaddr, BDADDR_TYPE_BR,
> + info->dev_class, info->rssi, info->data);
>
> if (!test_bit(HCI_DISCOVERY, &hdev->flags))
> continue;
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 2b20ee9..ad673f2 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -2157,14 +2157,15 @@ int mgmt_read_local_oob_data_reply_complete(u16 index, u8 *hash, u8 *randomizer,
> return err;
> }
>
> -int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 *dev_class, s8 rssi,
> - u8 *eir)
> +int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 bdaddr_type,
> + u8 *dev_class, s8 rssi, u8 *eir)
> {
> struct mgmt_ev_device_found ev;
>
> memset(&ev, 0, sizeof(ev));
>
> bacpy(&ev.bdaddr, bdaddr);
> + ev.bdaddr_type = bdaddr_type;
> memcpy(ev.dev_class, dev_class, sizeof(ev.dev_class));
> ev.rssi = rssi;
Otherwise looks good.
--
Gustavo F. Padovan
http://profusion.mobi
Hi,
On Mon, Apr 18, 2011 at 9:20 PM, Claudio Takahasi
<[email protected]> wrote:
> Hi Marcel/Johan/Gustavo,
>
> On Sat, Apr 16, 2011 at 5:04 PM, Claudio Takahasi
> <[email protected]> wrote:
>> Hi Marcel,
>>
>> On Sat, Apr 16, 2011 at 7:00 AM, Johan Hedberg <[email protected]>=
wrote:
>>> Hi Marcel,
>>>
>>> On Fri, Apr 15, 2011, Marcel Holtmann wrote:
>>>> > > + BDADDR_TYPE_LE_PUBLIC,
>>>> > > + BRADDR_TYPE_LE_RANDOM
>>>> > > +};
>>>>
>>>> I am also not sure the we should have this BR/EDR differentiation sinc=
e
>>>> the specification only talks about public and random addresses. And we
>>>> should follow the specification type value here. I am against
>>>> introducing our enum here.
>>>
>>> The HCI specification only has values for public and random because
>>> everywhere they are used it is already clear from the context (the HCI
>>> command or event in question) if we're talking about LE or BR/EDR. We o=
n
>>> the other hand don't have this contextual information with the
>>> mgmt_pair_device command. Saying "public" there could mean both BR/EDR
>>> public or LE public, i.e. an enum with just two possible values is not
>>> going to be of much use to us. Because of this difference between our
>>> API and that of HCI I don't think it's fair to apply the HCI
>>> convention/restriction to us.
>>>
>>> Johan
>>>
>>
>> I added 3 values because it gives more flexibility. Possible use cases:
>> - Whitelist needs the address type
>> - SMP
>> - As input to decide to store or not information about the device
>> since private address can change every 15minutes
>>
>> =A0At the moment we only need to know if the address is basic rate or LE
>> to select the discovery type: SDP or LE Discovery primary. For
>> pairing, Vinicius is using the kernel advertising cache to discover
>> the address type, passing the address type could avoid wrong fallback
>> to basic rate if the entry is not found in the cache.
>>
>> Claudio
>>
>
> Any objection to add the address type in the MGMT_EV_DEVICE_CONNECTED eve=
nt?
> Inside event.c there are a lot of get_adapter_and_device calls, for
> some contexts it creates a new device object if it doesn't exist(eg:
> incoming pairing). But the type is required to create a new device,
> otherwise it will not be possible to trigger the reverse service
> search. Obtain the type later based on the link key type will not
> work, unless we create a device with unknown type to be able to call
> the agent methods.
>
> BTW, is there a reason why it is necessary to "force" device
> creation(get_adapter_and_device option)? In my opinion we could create
> the device(if it doesn't exist) it inside btd_event_conn_complete
> only. There is a potential race condition: other application calling
> RemoveDevice, but for this case reference counting should work.
>
> Currently, controllers doesn't support simultaneous BR/EDR/LE, this is
> another argument to export the address type or connection type through
> management interface avoiding future changes on the API.
What about having a different socket family for le e.g.
AF_BLUETOOTH_LE? With that we could have more direct mapping with the
spec, with proper 49 bit addresses and things like that so we don't
have to break existing code.
--=20
Luiz Augusto von Dentz
Computer Engineer
SGksDQoNCj4gZG9uJ3QgdGhpbmsgd2UgbmVlZCB0byBzcGxpdCBiZXR3ZWVuIGluY29taW5nIGFu
ZCBvdXRnb2luZyBNVFUgYW55d2F5DQo+IHNpbmNlIHRoYXQgd2FzIGEgcHJldHR5IGJhZCBpZGVh
IGZyb20gZWFybHkgTDJDQVAgdGhhdCBpcyBqdXN0IHN0dXBpZC4NCj4NCj4gUmVnYXJkcw0KPg0K
PiBNYXJjZWwNCg0KTm8sIGl0IHdhc24ndCBhIHN0dXBpZCBpZGVhLg0KDQpXaGVuIHRyYWZmaWMg
ZmxvdyAocHJpbnRpbmcsIGF1ZGlvIHN0cmVhbWluZykgaXMgYXN5bW1ldHJpYyB0aGVuIGhhdmlu
ZyBhc3ltbWV0cmljIE1UVXMgaXMgYSBnb29kIGlkZWEuICBJIGFsc28gdXJnZSB5b3UgdG8gY29u
c2lkZXIgYXMgYW4gZXhlcmNpc2UgaG93IFJGQ09NTSdzIHN5bW1ldHJpYyBNVFVzIG1hZGUgZGVw
bG95bWVudCBvZiBFRFIgcHJvYmxlbWF0aWMuDQoNClRpbQ0KDQoNCg0KVGhpcyBtZXNzYWdlIGlz
IGZvciB0aGUgZGVzaWduYXRlZCByZWNpcGllbnQgb25seSBhbmQgbWF5IGNvbnRhaW4gcHJpdmls
ZWdlZCwgcHJvcHJpZXRhcnksIG9yIG90aGVyd2lzZSBwcml2YXRlIGluZm9ybWF0aW9uLiAgSWYg
eW91IGhhdmUgcmVjZWl2ZWQgaXQgaW4gZXJyb3IsIHBsZWFzZSBub3RpZnkgdGhlIHNlbmRlciBp
bW1lZGlhdGVseSBhbmQgZGVsZXRlIHRoZSBvcmlnaW5hbC4gIEFueSBvdGhlciB1c2Ugb2YgdGhl
IGVtYWlsIGJ5IHlvdSBpcyBwcm9oaWJpdGVkLg0K
Hi Anderson,
> >> >>> On Fri, Apr 15, 2011, Marcel Holtmann wrote:
> >> >>>> > > + BDADDR_TYPE_LE_PUBLIC,
> >> >>>> > > + BRADDR_TYPE_LE_RANDOM
> >> >>>> > > +};
> >> >>>>
> >> >>>> I am also not sure the we should have this BR/EDR differentiation since
> >> >>>> the specification only talks about public and random addresses. And we
> >> >>>> should follow the specification type value here. I am against
> >> >>>> introducing our enum here.
> >> >>>
> >> >>> The HCI specification only has values for public and random because
> >> >>> everywhere they are used it is already clear from the context (the HCI
> >> >>> command or event in question) if we're talking about LE or BR/EDR. We on
> >> >>> the other hand don't have this contextual information with the
> >> >>> mgmt_pair_device command. Saying "public" there could mean both BR/EDR
> >> >>> public or LE public, i.e. an enum with just two possible values is not
> >> >>> going to be of much use to us. Because of this difference between our
> >> >>> API and that of HCI I don't think it's fair to apply the HCI
> >> >>> convention/restriction to us.
> >> >>>
> >> >>> Johan
> >> >>>
> >> >>
> >> >> I added 3 values because it gives more flexibility. Possible use cases:
> >> >> - Whitelist needs the address type
> >> >> - SMP
> >> >> - As input to decide to store or not information about the device
> >> >> since private address can change every 15minutes
> >> >>
> >> >> At the moment we only need to know if the address is basic rate or LE
> >> >> to select the discovery type: SDP or LE Discovery primary. For
> >> >> pairing, Vinicius is using the kernel advertising cache to discover
> >> >> the address type, passing the address type could avoid wrong fallback
> >> >> to basic rate if the entry is not found in the cache.
> >> >>
> >> >> Claudio
> >> >>
> >> >
> >> > Any objection to add the address type in the MGMT_EV_DEVICE_CONNECTED event?
> >> > Inside event.c there are a lot of get_adapter_and_device calls, for
> >> > some contexts it creates a new device object if it doesn't exist(eg:
> >> > incoming pairing). But the type is required to create a new device,
> >> > otherwise it will not be possible to trigger the reverse service
> >> > search. Obtain the type later based on the link key type will not
> >> > work, unless we create a device with unknown type to be able to call
> >> > the agent methods.
> >> >
> >> > BTW, is there a reason why it is necessary to "force" device
> >> > creation(get_adapter_and_device option)? In my opinion we could create
> >> > the device(if it doesn't exist) it inside btd_event_conn_complete
> >> > only. There is a potential race condition: other application calling
> >> > RemoveDevice, but for this case reference counting should work.
> >> >
> >> > Currently, controllers doesn't support simultaneous BR/EDR/LE, this is
> >> > another argument to export the address type or connection type through
> >> > management interface avoiding future changes on the API.
> >>
> >> What about having a different socket family for le e.g.
> >> AF_BLUETOOTH_LE? With that we could have more direct mapping with the
> >> spec, with proper 49 bit addresses and things like that so we don't
> >> have to break existing code.
> >
> > A new socket family may be too much, we can do this through a new sockopt
> > item. I think this is possible, especially if we plan to export some other LE
> > specific info to the userspace. Do you have any idea of which things will we
> > put on a l2cap_options_le struct?
>
> Yes. We could add an MTU value for MTU configuration on LE connections
> [1]. What do you think?
>
> [1] http://marc.info/?l=linux-bluetooth&m=130373816101608&w=2
please stop going crazy here. First of all, we are not introducing a new
socket family just for LE. That is crazy talk ;)
And as I explained before, being able to change the MTU at runtime is
fine as a generic socket option. That it will fail on BR/EDR sockets is
also fine since that is a clear defined invalid behavior.
Just try adding BT_MTU socket option and see where this leads us. I
don't think we need to split between incoming and outgoing MTU anyway
since that was a pretty bad idea from early L2CAP that is just stupid.
Regards
Marcel
Hi,
On Wed, Jun 1, 2011 at 3:03 PM, Gustavo F. Padovan
<[email protected]> wrote:
> * Luiz Augusto von Dentz <[email protected]> [2011-05-30 20:27:22 +030=
0]:
>
>> Hi,
>>
>> On Mon, Apr 18, 2011 at 9:20 PM, Claudio Takahasi
>> <[email protected]> wrote:
>> > Hi Marcel/Johan/Gustavo,
>> >
>> > On Sat, Apr 16, 2011 at 5:04 PM, Claudio Takahasi
>> > <[email protected]> wrote:
>> >> Hi Marcel,
>> >>
>> >> On Sat, Apr 16, 2011 at 7:00 AM, Johan Hedberg <[email protected]=
om> wrote:
>> >>> Hi Marcel,
>> >>>
>> >>> On Fri, Apr 15, 2011, Marcel Holtmann wrote:
>> >>>> > > + BDADDR_TYPE_LE_PUBLIC,
>> >>>> > > + BRADDR_TYPE_LE_RANDOM
>> >>>> > > +};
>> >>>>
>> >>>> I am also not sure the we should have this BR/EDR differentiation s=
ince
>> >>>> the specification only talks about public and random addresses. And=
we
>> >>>> should follow the specification type value here. I am against
>> >>>> introducing our enum here.
>> >>>
>> >>> The HCI specification only has values for public and random because
>> >>> everywhere they are used it is already clear from the context (the H=
CI
>> >>> command or event in question) if we're talking about LE or BR/EDR. W=
e on
>> >>> the other hand don't have this contextual information with the
>> >>> mgmt_pair_device command. Saying "public" there could mean both BR/E=
DR
>> >>> public or LE public, i.e. an enum with just two possible values is n=
ot
>> >>> going to be of much use to us. Because of this difference between ou=
r
>> >>> API and that of HCI I don't think it's fair to apply the HCI
>> >>> convention/restriction to us.
>> >>>
>> >>> Johan
>> >>>
>> >>
>> >> I added 3 values because it gives more flexibility. Possible use case=
s:
>> >> - Whitelist needs the address type
>> >> - SMP
>> >> - As input to decide to store or not information about the device
>> >> since private address can change every 15minutes
>> >>
>> >> =A0At the moment we only need to know if the address is basic rate or=
LE
>> >> to select the discovery type: SDP or LE Discovery primary. For
>> >> pairing, Vinicius is using the kernel advertising cache to discover
>> >> the address type, passing the address type could avoid wrong fallback
>> >> to basic rate if the entry is not found in the cache.
>> >>
>> >> Claudio
>> >>
>> >
>> > Any objection to add the address type in the MGMT_EV_DEVICE_CONNECTED =
event?
>> > Inside event.c there are a lot of get_adapter_and_device calls, for
>> > some contexts it creates a new device object if it doesn't exist(eg:
>> > incoming pairing). But the type is required to create a new device,
>> > otherwise it will not be possible to trigger the reverse service
>> > search. Obtain the type later based on the link key type will not
>> > work, unless we create a device with unknown type to be able to call
>> > the agent methods.
>> >
>> > BTW, is there a reason why it is necessary to "force" device
>> > creation(get_adapter_and_device option)? In my opinion we could create
>> > the device(if it doesn't exist) it inside btd_event_conn_complete
>> > only. There is a potential race condition: other application calling
>> > RemoveDevice, but for this case reference counting should work.
>> >
>> > Currently, controllers doesn't support simultaneous BR/EDR/LE, this is
>> > another argument to export the address type or connection type through
>> > management interface avoiding future changes on the API.
>>
>> What about having a different socket family for le e.g.
>> AF_BLUETOOTH_LE? With that we could have more direct mapping with the
>> spec, with proper 49 bit addresses and things like that so we don't
>> have to break existing code.
>
> A new socket family may be too much, we can do this through a new sockopt
> item. I think this is possible, especially if we plan to export some othe=
r LE
> specific info to the userspace. Do you have any idea of which things will=
we
> put on a l2cap_options_le struct?
Yes. We could add an MTU value for MTU configuration on LE connections
[1]. What do you think?
[1] http://marc.info/?l=3Dlinux-bluetooth&m=3D130373816101608&w=3D2
BR,
Anderson Briglia
>
> --
> Gustavo F. Padovan
> http://profusion.mobi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to [email protected]
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>
--=20
INdT - Instituto Nokia de tecnologia
+55 2126 1122
http://techblog.briglia.net
* Luiz Augusto von Dentz <[email protected]> [2011-05-30 20:27:22 +0300]:
> Hi,
>=20
> On Mon, Apr 18, 2011 at 9:20 PM, Claudio Takahasi
> <[email protected]> wrote:
> > Hi Marcel/Johan/Gustavo,
> >
> > On Sat, Apr 16, 2011 at 5:04 PM, Claudio Takahasi
> > <[email protected]> wrote:
> >> Hi Marcel,
> >>
> >> On Sat, Apr 16, 2011 at 7:00 AM, Johan Hedberg <[email protected]=
m> wrote:
> >>> Hi Marcel,
> >>>
> >>> On Fri, Apr 15, 2011, Marcel Holtmann wrote:
> >>>> > > + BDADDR_TYPE_LE_PUBLIC,
> >>>> > > + BRADDR_TYPE_LE_RANDOM
> >>>> > > +};
> >>>>
> >>>> I am also not sure the we should have this BR/EDR differentiation si=
nce
> >>>> the specification only talks about public and random addresses. And =
we
> >>>> should follow the specification type value here. I am against
> >>>> introducing our enum here.
> >>>
> >>> The HCI specification only has values for public and random because
> >>> everywhere they are used it is already clear from the context (the HCI
> >>> command or event in question) if we're talking about LE or BR/EDR. We=
on
> >>> the other hand don't have this contextual information with the
> >>> mgmt_pair_device command. Saying "public" there could mean both BR/EDR
> >>> public or LE public, i.e. an enum with just two possible values is not
> >>> going to be of much use to us. Because of this difference between our
> >>> API and that of HCI I don't think it's fair to apply the HCI
> >>> convention/restriction to us.
> >>>
> >>> Johan
> >>>
> >>
> >> I added 3 values because it gives more flexibility. Possible use cases:
> >> - Whitelist needs the address type
> >> - SMP
> >> - As input to decide to store or not information about the device
> >> since private address can change every 15minutes
> >>
> >> =A0At the moment we only need to know if the address is basic rate or =
LE
> >> to select the discovery type: SDP or LE Discovery primary. For
> >> pairing, Vinicius is using the kernel advertising cache to discover
> >> the address type, passing the address type could avoid wrong fallback
> >> to basic rate if the entry is not found in the cache.
> >>
> >> Claudio
> >>
> >
> > Any objection to add the address type in the MGMT_EV_DEVICE_CONNECTED e=
vent?
> > Inside event.c there are a lot of get_adapter_and_device calls, for
> > some contexts it creates a new device object if it doesn't exist(eg:
> > incoming pairing). But the type is required to create a new device,
> > otherwise it will not be possible to trigger the reverse service
> > search. Obtain the type later based on the link key type will not
> > work, unless we create a device with unknown type to be able to call
> > the agent methods.
> >
> > BTW, is there a reason why it is necessary to "force" device
> > creation(get_adapter_and_device option)? In my opinion we could create
> > the device(if it doesn't exist) it inside btd_event_conn_complete
> > only. There is a potential race condition: other application calling
> > RemoveDevice, but for this case reference counting should work.
> >
> > Currently, controllers doesn't support simultaneous BR/EDR/LE, this is
> > another argument to export the address type or connection type through
> > management interface avoiding future changes on the API.
>=20
> What about having a different socket family for le e.g.
> AF_BLUETOOTH_LE? With that we could have more direct mapping with the
> spec, with proper 49 bit addresses and things like that so we don't
> have to break existing code.
A new socket family may be too much, we can do this through a new sockopt
item. I think this is possible, especially if we plan to export some other =
LE
specific info to the userspace. Do you have any idea of which things will we
put on a l2cap_options_le struct?=20
--=20
Gustavo F. Padovan
http://profusion.mobi