* Re: [PATCH 5/7] Fix in logic to write EIR when SDP records are changed.
From: Johan Hedberg @ 2010-08-04 14:09 UTC (permalink / raw)
To: Inga Stotland; +Cc: linux-bluetooth, rshaffer, marcel
In-Reply-To: <1280880237-2296-6-git-send-email-ingas@codeaurora.org>
Hi Inga,
On Tue, Aug 03, 2010, Inga Stotland wrote:
> +static void update_ext_inquiry_response(struct btd_adapter *adapter);
Is this forward declaration really necessary? Would it be possible to
get rid of it by rearranging the static functions in this file (i.e.
move update_ext_inquiry_response higher up in the file)?
Johan
^ permalink raw reply
* Reg: L2cap Security And Role Switch
From: Prabhakaran M.C @ 2010-08-04 14:08 UTC (permalink / raw)
To: linux-bluetooth
Hello All,
Whenever L2cap security is HIGH and remote device does role switch,
Bluez accepts the Role switch and L2cap disconnects the channel
because of HIGH security.
For PAN profile, I would like to keep the L2cap security to HIGH
since it involves internet browsing but the Widcomm stack always does
a role switch in PAN connection and Bluez disconnects l2cap channel.
Can someone please point in specification about the l2cap security
level and Role switch relation. I tried to find out this but I could
not get this behavior described in specification. Please provide your
comments and inputs. Thanks in Advance.
Thanks,
Prabhakaran.
^ permalink raw reply
* Re: [PATCH 4/7] Support for adding UUID128 to extended inquiry response
From: Johan Hedberg @ 2010-08-04 14:08 UTC (permalink / raw)
To: Inga Stotland; +Cc: linux-bluetooth, rshaffer, marcel
In-Reply-To: <1280880237-2296-5-git-send-email-ingas@codeaurora.org>
Hi Inga,
On Tue, Aug 03, 2010, Inga Stotland wrote:
> - ptr[1] = (truncated) ? EIR_UUID16_SOME : EIR_UUID16_ALL;
> + ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
And here's the fix I mentioned in the previous email that shouldn't be
needed to begin with.
Johan
^ permalink raw reply
* Re: [PATCH 3/7] Clean up code that generates extended inquiry response.
From: Johan Hedberg @ 2010-08-04 14:07 UTC (permalink / raw)
To: Inga Stotland; +Cc: linux-bluetooth, rshaffer, marcel
In-Reply-To: <1280880237-2296-4-git-send-email-ingas@codeaurora.org>
Hi Inga,
The patches are starting to look pretty good. A couple more coding style
issues though (in this and a few consequtive emails):
On Tue, Aug 03, 2010, Inga Stotland wrote:
> + ptr[1] = (truncated) ? EIR_UUID16_SOME : EIR_UUID16_ALL;
Another patch in your patch set "fixes" this and removes the parenthesis
around truncated. Could you just fix it in this patch itself. In general
patches with fixes to other patches that aren't yet upstream should be
avoided by redoing the the first patch that caused the need for a fix to
begin with.
Johan
^ permalink raw reply
* Re: Changes in HDP API
From: José Antonio Santos Cadenas @ 2010-08-04 13:54 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTima=oJ4mPMkfPCNPjsqhxX0drGBDURXBMaLrJ1C@mail.gmail.com>
Hello,
El Wednesday 04 August 2010 15:09:49 Luiz Augusto von Dentz escribió:
> Hi,
>
> On Wed, Aug 4, 2010 at 10:49 AM, Jose Antonio Santos Cadenas
>
> <santoscadenas@gmail.com> wrote:
> > This patch makes some changes in the HDP API based in the conversation
> > that I had yesterday with Luiz and Elvis.
> >
> > I still have a doubt about the notification of devices in the agent. Luiz
> > commented that with this API the Agent and the Application could not be
> > in different processes, is this a problem?
> >
> > An other open issue is: if we notify when a device is discovered and
> > suitable for connect with an application and when a devices is removed
> > (or becomes not suitable for connect). I think that we already need
> > something like the patch that we start discussing yesterday, something
> > that can be called by the user in order to force a new service discovery
> > and notifies the driver that will notify the agent. Of course we can
> > avoid this not notifying the suitable devices and just letting to try
> > the connection against all the devices (if there is no matching service
> > it will fail). What do you think?
> >
> > Regards.
>
> Lets say you want the pairing agent to connect right after pairing or
> directly via applet, these applications has zero knowledge of what
> endpoints the devices has, that why I suggest something simpler, so we
> might only need e.g. HealthDevice which the applet can say, please
> connect to this device so it calls e.g. HealthDevice.Connect(), this
> basically will start a discovery of the health endpoints on
> bluetoothd, then it matches with local endpoints (HealthApplication),
> create the channels which are informed to the HealthAgent which can
> then acquire the file descriptor.
I like this approach more than the previous one. But I still have one
question. I write it further down.
>
> This has 2 main advantages:
>
> 1. Anyone can request to connect not only applications which holds
> local endpoints
How can you guess the remote end point to connect to if you don't have a local
end point?
> 2. There is no need to fetch the services, discovery will take place
> every time the application attempt to connect.
I agree with this one. Less discovery implies less traffic and less battery
consumption and bandwidth.
Regards.
^ permalink raw reply
* Re: Changes in HDP API
From: Luiz Augusto von Dentz @ 2010-08-04 13:09 UTC (permalink / raw)
To: Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <1280908152-3743-1-git-send-email-santoscadenas@gmail.com>
Hi,
On Wed, Aug 4, 2010 at 10:49 AM, Jose Antonio Santos Cadenas
<santoscadenas@gmail.com> wrote:
> This patch makes some changes in the HDP API based in the conversation that I
> had yesterday with Luiz and Elvis.
>
> I still have a doubt about the notification of devices in the agent. Luiz
> commented that with this API the Agent and the Application could not be in
> different processes, is this a problem?
>
> An other open issue is: if we notify when a device is discovered and suitable
> for connect with an application and when a devices is removed (or becomes not
> suitable for connect). I think that we already need something like the patch
> that we start discussing yesterday, something that can be called by the user
> in order to force a new service discovery and notifies the driver that will
> notify the agent. Of course we can avoid this not notifying the suitable
> devices and just letting to try the connection against all the devices (if
> there is no matching service it will fail). What do you think?
>
> Regards.
Lets say you want the pairing agent to connect right after pairing or
directly via applet, these applications has zero knowledge of what
endpoints the devices has, that why I suggest something simpler, so we
might only need e.g. HealthDevice which the applet can say, please
connect to this device so it calls e.g. HealthDevice.Connect(), this
basically will start a discovery of the health endpoints on
bluetoothd, then it matches with local endpoints (HealthApplication),
create the channels which are informed to the HealthAgent which can
then acquire the file descriptor.
This has 2 main advantages:
1. Anyone can request to connect not only applications which holds
local endpoints
2. There is no need to fetch the services, discovery will take place
every time the application attempt to connect.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: MCAP patches
From: José Antonio Santos Cadenas @ 2010-08-04 9:02 UTC (permalink / raw)
To: Santiago Carot-Nemesio; +Cc: linux-bluetooth
In-Reply-To: <1280304015-9230-1-git-send-email-sancane@gmail.com>
Hi all,
Wednesday 28 July 2010 10:00:09 Santiago Carot-Nemesio wrote:
> Hello,
> Next are the patches that we have prepared for MCAP.
> Git can be cloned from git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git
>
> Regards.
Any comments about this patches?
Regards.
^ permalink raw reply
* [PATCH] Changes in HDP API.
From: Jose Antonio Santos Cadenas @ 2010-08-04 7:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1280908152-3743-1-git-send-email-santoscadenas@gmail.com>
Now the connection is made directly to a device and bluetoothd daemon
guess the PSM connection and the MDEPID based on the data get from the
SDP record.
---
doc/health-api.txt | 33 +++++++++++++--------------------
1 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/doc/health-api.txt b/doc/health-api.txt
index f469df3..aa462bd 100644
--- a/doc/health-api.txt
+++ b/doc/health-api.txt
@@ -47,12 +47,12 @@ Methods:
--------------------------------------------------------------------------------
Service org.bluez
-Interface org.bluez.HealthApplication
-Object path [variable prefix]/health_app_ZZZZ
+Interface org.bluez.HealthDevice
+Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
Methods:
- void Echo(object service)
+ void Echo(object application)
Sends an echo petition to the remote service. Returns True if
response matches with the buffer sent. If some error is detected
@@ -61,7 +61,7 @@ Methods:
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.OutOfRange
- object CreateChannel(object service, string type)
+ object CreateChannel(object application, string type)
Creates a new data channel with the indicated config to the
remote Service.
@@ -74,18 +74,12 @@ Methods:
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.HealthError
- void DestroyChannel(object channel)
+ void DestroyChannel(object application)
Destroys the data channel object.
Possible errors: org.bluez.Error.InvalidArguments
- orb.bluez.Error.NotFound
-
---------------------------------------------------------------------------------
-
-Service org.bluez
-Interface org.bluez.HealthService
-Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
+ org.bluez.Error.NotFound
--------------------------------------------------------------------------------
@@ -142,18 +136,17 @@ Methods:
to unregister the agent, because when this method gets called it
has already been unregistered.
- void ServiceDiscovered(object service)
+ void DeviceDiscovered(object device)
This method is called when a device containing an HDP
- application is paired or when the method Update of the
- HealthManager is called and new HealthServices are discovered.
- The method will be called once for each HealthService.
+ application is paired and is suitable for connect with the
+ application that registered this agent.
- void ServiceRemoved(object service)
+ void DeviceRemoved(object device)
- This is called if during an Update some HealthServices
- have disappeared. The method is called once for each removed
- HealthService.
+ This method is called if a device that was suitable for
+ connecting with the application becomes not suitable for
+ connecting with the application.
void ChannelConnected(object channel)
--
1.7.0.4
^ permalink raw reply related
* Changes in HDP API
From: Jose Antonio Santos Cadenas @ 2010-08-04 7:49 UTC (permalink / raw)
To: linux-bluetooth
This patch makes some changes in the HDP API based in the conversation that I
had yesterday with Luiz and Elvis.
I still have a doubt about the notification of devices in the agent. Luiz
commented that with this API the Agent and the Application could not be in
different processes, is this a problem?
An other open issue is: if we notify when a device is discovered and suitable
for connect with an application and when a devices is removed (or becomes not
suitable for connect). I think that we already need something like the patch
that we start discussing yesterday, something that can be called by the user
in order to force a new service discovery and notifies the driver that will
notify the agent. Of course we can avoid this not notifying the suitable
devices and just letting to try the connection against all the devices (if
there is no matching service it will fail). What do you think?
Regards.
Jose.
^ permalink raw reply
* [PATCH] Bluetooth: Check result code for l2cap info rsp
From: Ville Tervo @ 2010-08-04 6:43 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, Ville Tervo
Check result code in l2cap information response. Othervise
the response code would read invalid feature mask and access
invalid memory.
Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
net/bluetooth/l2cap.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 0f34e12..3e3cd9d 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3348,6 +3348,15 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
del_timer(&conn->info_timer);
+ if (result != L2CAP_IR_SUCCESS) {
+ conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
+ conn->info_ident = 0;
+
+ l2cap_conn_start(conn);
+
+ return 0;
+ }
+
if (type == L2CAP_IT_FEAT_MASK) {
conn->feat_mask = get_unaligned_le32(rsp->data);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported
From: Ville Tervo @ 2010-08-04 6:18 UTC (permalink / raw)
To: ext Marcel Holtmann
Cc: Gustavo F. Padovan, linux-bluetooth@vger.kernel.org,
Gustavo F. Padovan
In-Reply-To: <1280886991.12579.98.camel@localhost.localdomain>
On Wed, Aug 04, 2010 at 03:56:31AM +0200, ext Marcel Holtmann wrote:
> Hi Ville,
>
> > > On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> > > >From: Gustavo F. Padovan<padovan@profusion.mobi>
> > > >
> > > >If the remote side doesn't support Enhanced Retransmission Mode neither
> > > >Streaming Mode, we shall not send the RFC option.
> > > >Some devices that only supports Basic Mode do not understanding the RFC
> > > >option. This patch fix the regression found with that devices.
> > >
> > >
> > > Yes this is better. After some research i found out that quite many
> > > old devices are not handling properly unknown options.
> >
> > Nice, we can put this upstream, do you agree?
> >
> > >
> > > However I found another regression. And this kind of patch is needed
> > > also. Otherwise the info rsp code is reading feat_mask from failed
> > > response (and does invalid memory access).
> > >
> > >
> > > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > > index 0f34e12..1e174a4 100644
> > > --- a/net/bluetooth/l2cap.c
> > > +++ b/net/bluetooth/l2cap.c
> > > @@ -3348,6 +3348,13 @@ static inline int
> > > l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
> > >
> > > del_timer(&conn->info_timer);
> > >
> > > + if (result != L2CAP_IR_SUCCESS) {
> > > + conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
> > > + conn->info_ident = 0;
> > > + l2cap_conn_start(conn);
> > > + return 0;
> > > + }
> > > +
> > > if (type == L2CAP_IT_FEAT_MASK) {
> > > conn->feat_mask = get_unaligned_le32(rsp->data);
> > >
> >
> > Ack. Send a proper GIT patch then we can push this fix.
>
> am I getting this proper patch or not?
Yes.
--
Ville
^ permalink raw reply
* [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported
From: Gustavo F. Padovan @ 2010-08-04 2:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, Gustavo F. Padovan
From: Gustavo F. Padovan <padovan@profusion.mobi>
If the remote side doesn't support Enhanced Retransmission Mode neither
Streaming Mode, we shall not send the RFC option.
Some devices that only supports Basic Mode do not understanding the RFC
option. This patch fix the regression found with that devices.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/l2cap.c | 15 ++++++++++++---
1 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9ba1e8e..0f34e12 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2527,6 +2527,10 @@ done:
if (pi->imtu != L2CAP_DEFAULT_MTU)
l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
+ if (!(pi->conn->feat_mask & L2CAP_FEAT_ERTM) &&
+ !(pi->conn->feat_mask & L2CAP_FEAT_STREAMING))
+ break;
+
rfc.mode = L2CAP_MODE_BASIC;
rfc.txwin_size = 0;
rfc.max_transmit = 0;
@@ -2534,6 +2538,8 @@ done:
rfc.monitor_timeout = 0;
rfc.max_pdu_size = 0;
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long) &rfc);
break;
case L2CAP_MODE_ERTM:
@@ -2546,6 +2552,9 @@ done:
if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long) &rfc);
+
if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
break;
@@ -2566,6 +2575,9 @@ done:
if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long) &rfc);
+
if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
break;
@@ -2577,9 +2589,6 @@ done:
break;
}
- l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
- (unsigned long) &rfc);
-
/* FIXME: Need actual value of the flush timeout */
//if (flush_to != L2CAP_DEFAULT_FLUSH_TO)
// l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, pi->flush_to);
--
1.7.1.1
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported
From: Marcel Holtmann @ 2010-08-04 1:56 UTC (permalink / raw)
To: Gustavo F. Padovan
Cc: Ville Tervo, linux-bluetooth@vger.kernel.org, Gustavo F. Padovan
In-Reply-To: <20100731224144.GA29039@vigoh>
Hi Ville,
> > On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> > >From: Gustavo F. Padovan<padovan@profusion.mobi>
> > >
> > >If the remote side doesn't support Enhanced Retransmission Mode neither
> > >Streaming Mode, we shall not send the RFC option.
> > >Some devices that only supports Basic Mode do not understanding the RFC
> > >option. This patch fix the regression found with that devices.
> >
> >
> > Yes this is better. After some research i found out that quite many
> > old devices are not handling properly unknown options.
>
> Nice, we can put this upstream, do you agree?
>
> >
> > However I found another regression. And this kind of patch is needed
> > also. Otherwise the info rsp code is reading feat_mask from failed
> > response (and does invalid memory access).
> >
> >
> > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > index 0f34e12..1e174a4 100644
> > --- a/net/bluetooth/l2cap.c
> > +++ b/net/bluetooth/l2cap.c
> > @@ -3348,6 +3348,13 @@ static inline int
> > l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
> >
> > del_timer(&conn->info_timer);
> >
> > + if (result != L2CAP_IR_SUCCESS) {
> > + conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
> > + conn->info_ident = 0;
> > + l2cap_conn_start(conn);
> > + return 0;
> > + }
> > +
> > if (type == L2CAP_IT_FEAT_MASK) {
> > conn->feat_mask = get_unaligned_le32(rsp->data);
> >
>
> Ack. Send a proper GIT patch then we can push this fix.
am I getting this proper patch or not?
Regards
Marcel
^ permalink raw reply
* [PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
---
src/adapter.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 103 insertions(+), 3 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index a2ef9aa..9603ba7 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2750,8 +2750,100 @@ static void emit_device_found(const char *path, const char *address,
g_dbus_send_message(connection, signal);
}
+static char **get_eir_uuids(uint8_t *eir_data, size_t *uuid_count)
+{
+ uint8_t len = 0;
+ char **uuid_buf = NULL;
+ char **uuids;
+ size_t total = 0;
+ size_t uuid16_count = 0;
+ size_t uuid32_count = 0;
+ size_t uuid128_count = 0;
+ uint8_t *uuid16;
+ uint8_t *uuid32;
+ uint8_t *uuid128;
+ uuid_t service;
+ int i;
+
+ while (len < EIR_DATA_LENGTH) {
+ uint8_t type = eir_data[1];
+ uint8_t field_len = eir_data[0];
+
+ switch(type) {
+ case EIR_UUID16_SOME:
+ case EIR_UUID16_ALL:
+ uuid16_count = field_len / 2;
+ uuid16 = &eir_data[2];
+ break;
+ case EIR_UUID32_SOME:
+ case EIR_UUID32_ALL:
+ uuid32_count = field_len / 4;
+ uuid32 = &eir_data[2];
+ break;
+ case EIR_UUID128_SOME:
+ case EIR_UUID128_ALL:
+ uuid128_count = field_len / 16;
+ uuid128 = &eir_data[2];
+ break;
+ }
+
+ len += field_len + 1;
+ eir_data += field_len + 1;
+ }
+
+ total = uuid16_count + uuid32_count + uuid128_count;
+ *uuid_count = total;
+
+ if (!total)
+ return NULL;
+
+ uuid_buf = g_new0(char *, total + 1);
+
+ if (!uuid_buf)
+ return NULL;
+
+ uuids = uuid_buf;
+
+ /* Generate uuids in SDP format (EIR data is Little Endian) */
+ service.type = SDP_UUID16;
+ for (i = 0; i < uuid16_count; i++) {
+ uint16_t val16 = uuid16[1];
+
+ val16 = (val16<<8) + uuid16[0];
+ service.value.uuid16 = val16;
+ *uuids++ = bt_uuid2string(&service);
+ uuid16 += 2;
+ }
+
+ service.type = SDP_UUID32;
+ for (i = 0; i < uuid32_count; i++) {
+ uint32_t val32 = uuid32[3];
+ int k;
+
+ for (k = 2; k >= 0; k--)
+ val32 = (val32 << 8) + uuid32[k];
+
+ service.value.uuid32 = val32;
+ *uuids++ = bt_uuid2string(&service);
+ uuid32 += 4;
+ }
+
+ service.type = SDP_UUID128;
+ for (i = 0; i < uuid128_count; i++) {
+ int k;
+
+ for (k = 0; k < 16; k++)
+ service.value.uuid128.data[k] = uuid128[16 - k - 1];
+
+ *uuids++ = bt_uuid2string(&service);
+ uuid128 += 16;
+ }
+
+ return uuid_buf;
+}
+
void adapter_emit_device_found(struct btd_adapter *adapter,
- struct remote_dev_info *dev)
+ struct remote_dev_info *dev, uint8_t *eir_data)
{
struct btd_device *device;
char peer_addr[18], local_addr[18];
@@ -2759,6 +2851,8 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
dbus_bool_t paired = FALSE;
dbus_int16_t rssi = dev->rssi;
char *alias;
+ char **uuids = NULL;
+ size_t uuid_count = 0;
ba2str(&dev->bdaddr, peer_addr);
ba2str(&adapter->bdaddr, local_addr);
@@ -2778,6 +2872,10 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
} else
alias = g_strdup(dev->alias);
+ /* Extract UUIDs from extended inquiry response if any*/
+ if (eir_data != NULL)
+ uuids = get_eir_uuids(eir_data, &uuid_count);
+
emit_device_found(adapter->path, paddr,
"Address", DBUS_TYPE_STRING, &paddr,
"Class", DBUS_TYPE_UINT32, &dev->class,
@@ -2787,15 +2885,17 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
"Alias", DBUS_TYPE_STRING, &alias,
"LegacyPairing", DBUS_TYPE_BOOLEAN, &dev->legacy,
"Paired", DBUS_TYPE_BOOLEAN, &paired,
+ "UUIDs", DBUS_TYPE_ARRAY, &uuids, uuid_count,
NULL);
g_free(alias);
+ g_strfreev(uuids);
}
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
int8_t rssi, uint32_t class, const char *name,
const char *alias, gboolean legacy,
- name_status_t name_status)
+ name_status_t name_status, uint8_t *eir_data)
{
struct remote_dev_info *dev, match;
@@ -2834,7 +2934,7 @@ done:
adapter->found_devices = g_slist_sort(adapter->found_devices,
(GCompareFunc) dev_rssi_cmp);
- adapter_emit_device_found(adapter, dev);
+ adapter_emit_device_found(adapter, dev, eir_data);
}
int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr)
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 6/7] Handle arrays in device properties dictionary.
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
---
src/adapter.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index e1f3051..a2ef9aa 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2702,6 +2702,7 @@ static void append_dict_valist(DBusMessageIter *iter,
DBusMessageIter dict;
const char *key;
int type;
+ int n_elements;
void *val;
dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
@@ -2713,7 +2714,12 @@ static void append_dict_valist(DBusMessageIter *iter,
while (key) {
type = va_arg(var_args, int);
val = va_arg(var_args, void *);
- dict_append_entry(&dict, key, type, val);
+ if (type == DBUS_TYPE_ARRAY) {
+ n_elements = va_arg(var_args, int);
+ if (n_elements > 0)
+ dict_append_array(&dict, key, DBUS_TYPE_STRING, val, n_elements);
+ } else
+ dict_append_entry(&dict, key, type, val);
key = va_arg(var_args, char *);
}
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 5/7] Fix in logic to write EIR when SDP records are changed.
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
Whenever SDP service record is added/deleted/modified check for whether
class of device needs to be updated as well. If the update is
needed, proceed as before: new EIR will be written subsequently.
If the class of device is already present, just update EIR and return.
---
src/adapter.c | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 4615326..e1f3051 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -137,6 +137,8 @@ struct btd_adapter {
gint ref;
};
+static void update_ext_inquiry_response(struct btd_adapter *adapter);
+
static void adapter_set_pairable_timeout(struct btd_adapter *adapter,
guint interval);
@@ -216,12 +218,17 @@ static int adapter_set_service_classes(struct btd_adapter *adapter,
adapter->wanted_cod &= 0x00ffff;
adapter->wanted_cod |= (value << 16);
- /* If we already have the CoD we want or the cache is enabled or an
- * existing CoD write is in progress just bail out */
- if (adapter->current_cod == adapter->wanted_cod ||
- adapter->cache_enable || adapter->pending_cod)
+ /* If the cache is enabled or an existing CoD write is in progress
+ * just bail out */
+ if (adapter->cache_enable || adapter->pending_cod)
return 0;
+ /* If we already have the CoD we want, update EIR and return */
+ if (adapter->current_cod == adapter->wanted_cod) {
+ update_ext_inquiry_response(adapter);
+ return 0;
+ }
+
DBG("Changing service classes to 0x%06x", adapter->wanted_cod);
err = adapter_ops->set_class(adapter->dev_id, adapter->wanted_cod);
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 4/7] Support for adding UUID128 to extended inquiry response
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
---
src/sdpd-service.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 64 insertions(+), 1 deletions(-)
diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index 50c0a97..651e24c 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -49,6 +49,8 @@
#include "manager.h"
#include "adapter.h"
+#define SIZEOF_UUID128 16
+
static sdp_record_t *server = NULL;
static uint16_t did_vendor = 0x0000;
@@ -174,6 +176,63 @@ static void update_svclass_list(const bdaddr_t *src)
}
+static void eir_generate_uuid128(sdp_list_t *list,
+ uint8_t *ptr, uint16_t *eir_len)
+{
+ int i, k, index = 0;
+ uint16_t len = *eir_len;
+ uint8_t *uuid128;
+ gboolean truncated = FALSE;
+
+ /* Store UUID128 in place, skip 2 bytes to insert type and length later */
+ uuid128 = ptr + 2;
+
+ for (; list; list = list->next) {
+ sdp_record_t *rec = (sdp_record_t *) list->data;
+ uint8_t *uuid128_data = rec->svclass.value.uuid128.data;
+
+ if (rec->svclass.type != SDP_UUID128)
+ continue;
+
+ /* Stop if not enough space to put next UUID128 */
+ if ((len + 2 + SIZEOF_UUID128) > EIR_DATA_LENGTH) {
+ truncated = TRUE;
+ break;
+ }
+
+ /* Check for duplicates, EIR data is Little Endian */
+ for (i = 0; i < index; i++) {
+ for (k = 0; k < SIZEOF_UUID128; k++) {
+ if (uuid128[i * SIZEOF_UUID128 + k] !=
+ uuid128_data[SIZEOF_UUID128 - k])
+ break;
+ }
+ if (k == SIZEOF_UUID128)
+ break;
+ }
+
+ if (i < index)
+ continue;
+
+ /* EIR data is Little Endian */
+ for (k = 0; k < SIZEOF_UUID128; k++)
+ uuid128[index * SIZEOF_UUID128 + k] =
+ uuid128_data[SIZEOF_UUID128 - 1 - k];
+
+ len += SIZEOF_UUID128;
+ index++;
+ }
+
+ if (index > 0 || truncated) {
+ /* EIR Data length */
+ ptr[0] = (index * SIZEOF_UUID128) + 1;
+ /* EIR Data type */
+ ptr[1] = truncated ? EIR_UUID128_SOME : EIR_UUID128_ALL;
+ len += 2;
+ *eir_len = len;
+ }
+}
+
void create_ext_inquiry_response(const char *name,
int8_t tx_power, sdp_list_t *services,
uint8_t *data)
@@ -261,7 +320,7 @@ void create_ext_inquiry_response(const char *name,
/* EIR Data length */
ptr[0] = (index * sizeof(uint16_t)) + 1;
/* EIR Data type */
- ptr[1] = (truncated) ? EIR_UUID16_SOME : EIR_UUID16_ALL;
+ ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
ptr += 2;
eir_len += 2;
@@ -271,6 +330,10 @@ void create_ext_inquiry_response(const char *name,
*ptr++ = (uuid16[i] & 0xff00) >> 8;
}
}
+
+ /* Group all UUID128 types */
+ if (eir_len <= EIR_DATA_LENGTH - 2)
+ eir_generate_uuid128(services, ptr, &eir_len);
}
void register_public_browse_group(void)
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 3/7] Clean up code that generates extended inquiry response.
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
---
src/sdpd-service.c | 44 +++++++++++++++++++++++++++++---------------
1 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index 35e333d..50c0a97 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -180,35 +180,41 @@ void create_ext_inquiry_response(const char *name,
{
sdp_list_t *list = services;
uint8_t *ptr = data;
- uint16_t uuid[24];
+ uint16_t eir_len = 0;
+ uint16_t uuid16[EIR_DATA_LENGTH / 2];
int i, index = 0;
+ gboolean truncated = FALSE;
if (name) {
int len = strlen(name);
+ /* EIR Data type */
if (len > 48) {
len = 48;
- ptr[1] = 0x08;
+ ptr[1] = EIR_NAME_SHORT;
} else
- ptr[1] = 0x09;
+ ptr[1] = EIR_NAME_COMPLETE;
+ /* EIR Data length */
ptr[0] = len + 1;
memcpy(ptr + 2, name, len);
- ptr += len + 2;
+ eir_len += (len + 2);
+ ptr += (len + 2);
}
if (tx_power != 0) {
*ptr++ = 2;
- *ptr++ = 0x0a;
+ *ptr++ = EIR_TX_POWER;
*ptr++ = (uint8_t) tx_power;
+ eir_len += 3;
}
if (did_vendor != 0x0000) {
uint16_t source = 0x0002;
*ptr++ = 9;
- *ptr++ = 0x10;
+ *ptr++ = EIR_DEVICE_ID;
*ptr++ = (source & 0x00ff);
*ptr++ = (source & 0xff00) >> 8;
*ptr++ = (did_vendor & 0x00ff);
@@ -217,10 +223,10 @@ void create_ext_inquiry_response(const char *name,
*ptr++ = (did_product & 0xff00) >> 8;
*ptr++ = (did_version & 0x00ff);
*ptr++ = (did_version & 0xff00) >> 8;
+ eir_len += 10;
}
- ptr[1] = 0x03;
-
+ /* Group all UUID16 types */
for (; list; list = list->next) {
sdp_record_t *rec = (sdp_record_t *) list->data;
@@ -233,28 +239,36 @@ void create_ext_inquiry_response(const char *name,
if (rec->svclass.value.uuid16 == PNP_INFO_SVCLASS_ID)
continue;
- if (index > 23) {
- ptr[1] = 0x02;
+ /* Stop if not enough space to put next UUID16 */
+ if ((eir_len + 2 + sizeof(uint16_t)) > EIR_DATA_LENGTH) {
+ truncated = TRUE;
break;
}
+ /* Check for duplicates */
for (i = 0; i < index; i++)
- if (uuid[i] == rec->svclass.value.uuid16)
+ if (uuid16[i] == rec->svclass.value.uuid16)
break;
if (i < index)
continue;
- uuid[index++] = rec->svclass.value.uuid16;
+ uuid16[index++] = rec->svclass.value.uuid16;
+ eir_len += sizeof(uint16_t);
}
if (index > 0) {
- ptr[0] = (index * 2) + 1;
+ /* EIR Data length */
+ ptr[0] = (index * sizeof(uint16_t)) + 1;
+ /* EIR Data type */
+ ptr[1] = (truncated) ? EIR_UUID16_SOME : EIR_UUID16_ALL;
+
ptr += 2;
+ eir_len += 2;
for (i = 0; i < index; i++) {
- *ptr++ = (uuid[i] & 0x00ff);
- *ptr++ = (uuid[i] & 0xff00) >> 8;
+ *ptr++ = (uuid16[i] & 0x00ff);
+ *ptr++ = (uuid16[i] & 0xff00) >> 8;
}
}
}
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 2/7] Minor fix when skipping duplicate UUID16 from EIR.
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
---
src/sdpd-service.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index cdbb4f4..35e333d 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -242,7 +242,7 @@ void create_ext_inquiry_response(const char *name,
if (uuid[i] == rec->svclass.value.uuid16)
break;
- if (i == index - 1)
+ if (i < index)
continue;
uuid[index++] = rec->svclass.value.uuid16;
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 1/7] Spec constants for Extended Inquiry Response field types
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
---
src/sdpd.h | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/src/sdpd.h b/src/sdpd.h
index e93b0b6..5bab869 100644
--- a/src/sdpd.h
+++ b/src/sdpd.h
@@ -34,6 +34,19 @@
#define SDPDBG(fmt...)
#endif
+#define EIR_DATA_LENGTH 240
+
+#define EIR_UUID16_SOME 0x02 /* 16-bit UUID, more available */
+#define EIR_UUID16_ALL 0x03 /* 16-bit UUID, all listed */
+#define EIR_UUID32_SOME 0x04 /* 32-bit UUID, more available */
+#define EIR_UUID32_ALL 0x05 /* 32-bit UUID, all listed */
+#define EIR_UUID128_SOME 0x06 /* 128-bit UUID, more available */
+#define EIR_UUID128_ALL 0x07 /* 128-bit UUID, all listed */
+#define EIR_NAME_SHORT 0x08 /* shortened local name */
+#define EIR_NAME_COMPLETE 0x09 /* complete local name */
+#define EIR_TX_POWER 0x0A /* transmit power level */
+#define EIR_DEVICE_ID 0x10 /* device ID */
+
typedef struct request {
bdaddr_t device;
bdaddr_t bdaddr;
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH v6 0/7] Enhanced support for extended inquiry response
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel
EIR needs to be updated whenever local SDP record database changes.
Previously, when an SDP record was added/removed/updated, EIR would
be written in callback fired off HCI "Command Complete" event set off by
"Write Class Of Device" command. However, if the class of device did not
need to be updated, the EIR write was not happening either. This
implementation has been augmented to write new EIR when modification
in SDP database does not lead to updating of class of device.
Added support for UUID128 service descriptors in local EIR.
Service UUIDs are written to "device properties" dictionary when emitting
"Device Found" signal. This allows peek at available services offered by
a remote device without establishing SDP connection.
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* Re: [PATCH][RFC] Fix SDP resolving segfault
From: Manuel Naranjo @ 2010-08-03 20:24 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Johan Hedberg, BlueZ
In-Reply-To: <AANLkTinYN=KDban-WsF4wZw-ZZDU7fQ0SFnAWdofbokd@mail.gmail.com>
Luiz,
> This looks like a different issue, at least it doesn't seems to crash
> in the same point, so I assume the patch does fix something but
> uncover another problem. But we have to find out where exactly is this
> other problem because btd_device_unref doesn't seems to be the place,
> can't you get any core dumps from the tests you did?
>
I agree it looks like it triggered another issue, I will try to get logs
tomorrow.
Manuel
^ permalink raw reply
* Re: [PATCH][RFC] Fix SDP resolving segfault
From: Luiz Augusto von Dentz @ 2010-08-03 20:17 UTC (permalink / raw)
To: Manuel Naranjo; +Cc: Johan Hedberg, BlueZ
In-Reply-To: <AANLkTi=i8MhyC80qis9TjZBmtNUJDBMNw5g2rjKPQvdj@mail.gmail.com>
Hi Manuel,
On Tue, Aug 3, 2010 at 4:21 PM, Manuel Naranjo <manuel@aircable.net> wrote:
>> I will give this a try and let you know. Maybe I can results before the
>> Tuesday, but I doubt it.
>
> As I said on IRC it keeps crashing. Here's the log, as you can see the
> crash happened in a different place now.
>
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x9411de0: ref=1
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x9411de0: ref=2
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x9409570: ref=2
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x9408b60: ref=2
> bluetoothd[1842]: src/adapter.c:create_device() C8:7E:75:D6:B3:B0
> bluetoothd[1842]: src/adapter.c:adapter_create_device() C8:7E:75:D6:B3:B0
> bluetoothd[1842]: src/device.c:device_create() Creating device
> /org/bluez/1842/hci0/dev_C8_7E_75_D6_B3_B0
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x93fd258: ref=1
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x93fd258: ref=2
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x9404dc0: ref=3
> bluetoothd[1842]: Discovery session 0x93fb658 with :1.58 activated
> bluetoothd[1842]: src/adapter.c:session_ref() 0x93fb658: ref=1
> bluetoothd[1842]: src/adapter.c:adapter_get_device() 00:25:47:2D:20:98
> bluetoothd[1842]: src/adapter.c:session_unref() 0x93fb658: ref=0
> bluetoothd[1842]: src/adapter.c:session_remove() Discovery session
> 0x93fb658 with :1.58 deactivated
> bluetoothd[1842]: src/adapter.c:session_remove() Stopping discovery
> bluetoothd[1842]: Stopping discovery
> bluetoothd[1842]: /org/bluez/1842/hci0/dev_C8_7E_75_D6_B3_B0: error
> updating services: Device or resource busy (16)
> bluetoothd[1842]: src/device.c:btd_device_unref() 0x93fd258: ref=1
> bluetoothd[1842]: Discovery session 0x93fd4b0 with :1.58 activated
> bluetoothd[1842]: src/adapter.c:session_ref() 0x93fd4b0: ref=1
> bluetoothd[1842]: /org/bluez/1842/hci0/dev_00_26_E2_3C_5E_05: error
> updating services: Host is down (112)
> bluetoothd[1842]: src/device.c:btd_device_unref() 0x9404dc0: ref=2
> bluetoothd[1842]: src/adapter.c:adapter_get_device() 00:26:E2:3C:5E:05
> bluetoothd[1842]: src/adapter.c:session_unref() 0x93fd4b0: ref=0
> bluetoothd[1842]: src/adapter.c:session_remove() Discovery session
> 0x93fd4b0 with :1.58 deactivated
> bluetoothd[1842]: src/adapter.c:session_remove() Stopping discovery
> bluetoothd[1842]: Stopping discovery
> bluetoothd[1842]: /org/bluez/1842/hci0/dev_00_1C_D6_B1_58_1B: error
> updating services: Device or resource busy (16)
> bluetoothd[1842]: src/device.c:btd_device_unref() 0x9409570: ref=1
> bluetoothd[1842]: Discovery session 0x93f9f80 with :1.58 activated
> bluetoothd[1842]: src/adapter.c:session_ref() 0x93f9f80: ref=1
> bluetoothd[1842]: src/adapter.c:adapter_remove_connection() Removing
> temporary device /org/bluez/1842/hci0/dev_00_25_47_2D_20_98
> bluetoothd[1842]: src/device.c:device_remove() Removing device
> /org/bluez/1842/hci0/dev_00_25_47_2D_20_98
> bluetoothd[1842]: src/device.c:btd_device_unref() 0x9411120: ref=1
> bluetoothd[1842]: src/device.c:btd_device_unref() 0x9411120: ref=0
> bluetoothd[1842]: src/device.c:device_free() 0x9411120
> bluetoothd[1842]: src/adapter.c:adapter_get_device() 00:17:D5:65:D0:3D
> bluetoothd[1842]: src/adapter.c:session_unref() 0x93f9f80: ref=0
> bluetoothd[1842]: src/adapter.c:session_remove() Discovery session
> 0x93f9f80 with :1.58 deactivated
> bluetoothd[1842]: src/adapter.c:session_remove() Stopping discovery
> bluetoothd[1842]: Stopping discovery
> bluetoothd[1842]: Discovery session 0x9410560 with :1.58 activated
> bluetoothd[1842]: src/adapter.c:session_ref() 0x9410560: ref=1
> bluetoothd[1842]: src/device.c:device_probe_drivers() Probe drivers
> for /org/bluez/1842/hci0/dev_00_17_D5_65_D0_3D
> bluetoothd[1842]: serial/manager.c:serial_probe() path
> /org/bluez/1842/hci0/dev_00_17_D5_65_D0_3D:
> 00001105-0000-1000-8000-00805f9b34fb
> bluetoothd[1842]: serial/port.c:create_serial_device() Registered
> interface org.bluez.Serial on path
> /org/bluez/1842/hci0/dev_00_17_D5_65_D0_3D
> bluetoothd[1842]: src/device.c:btd_device_unref() 0x9408b60: ref=1
> Segmentation fault
This looks like a different issue, at least it doesn't seems to crash
in the same point, so I assume the patch does fix something but
uncover another problem. But we have to find out where exactly is this
other problem because btd_device_unref doesn't seems to be the place,
can't you get any core dumps from the tests you did?
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Gustavo F. Padovan @ 2010-08-03 18:44 UTC (permalink / raw)
To: Mat Martineau; +Cc: Marcel Holtmann, linux-bluetooth, rshaffer
In-Reply-To: <alpine.DEB.2.00.1008030814570.19861@linux-sea-02>
Hi Mat,
* Mat Martineau <mathewm@codeaurora.org> [2010-08-03 08:58:57 -0700]:
>
> On Mon, 2 Aug 2010, Marcel Holtmann wrote:
>
> >Hi Mat,
> >
> >>>* Marcel Holtmann <marcel@holtmann.org> [2010-08-02 12:38:32 -0700]:
> >>>
> >>>>Hi Mat,
> >>>>
> >>>>>Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> >>>>>---
> >>>>> net/bluetooth/l2cap.c | 12 ++++++++----
> >>>>> 1 files changed, 8 insertions(+), 4 deletions(-)
> >>>>>
> >>>>>diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> >>>>>index 9ba1e8e..aed72f2 100644
> >>>>>--- a/net/bluetooth/l2cap.c
> >>>>>+++ b/net/bluetooth/l2cap.c
> >>>>>@@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
> >>>>> goto unlock;
> >>>>>
> >>>>> if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
> >>>>>- if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>>- l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
> >>>>>+ if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
> >>>>>+ l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
> >>>>>+ (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>>+ l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
> >>>>> l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
> >>>>
> >>>>this becomes unreadable and my brain starts to throw a core dump. So it
> >>>>clearly needs to be put into a helper inline function.
> >>>
> >>>Actually we don't need that, since the code that deals with Basic Mode
> >>>never check and use the l2cap_pi(sk)->fcs. So we don't care about FCS
> >>>value in the Basic Mode.
> >>
> >>There isn't currently any Basic Mode code that triggers this latent
> >>bug, but I have a patch coming up that does require this fix.
> >>
> >>As it stands, getsockopt() on a connected basic mode socket shows FCS
> >>enabled, so this bug is visible from userspace.
> >
> >can we just fail the setsockopt() when trying to set basic mode and FCS
> >off.
>
> It definitely makes sense to have more validation of L2CAP_OPTIONS
> passed to setsockopt().
>
> >And also in case fallback to basic mode happens, then FCS should be set
> >to be enabled. Since for FCS and basic mode we always have to use FCS.
> >So that seems just fine to me.
>
> The spec says "The FCS option shall only be used when the mode is
> being, or is already configured to Enhanced Retransmission mode or
> Streaming mode." FCS is never used in basic mode (fallback or not).
>
> (Maybe I've misunderstood your point)
>
> >Maybe you need to explain a bit more in detail what you are trying to
> >achieve in conjunction with userspace API.
>
> My goal is to only have l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 when
> the FCS option is actually in use. Otherwise, any logic checking
> for FCS also has to check the L2CAP mode. Might as well check the
> mode once and set fcs accordingly -- which is what my patch does.
>
> Gustavo is correct that l2cap_pi(sk)->fcs is currently only checked
> on code paths used with ERTM and streaming mode. However, future
> code (including a patch I'll be posting soon) will depend on the fcs
> value being accurate in all modes.
So queue this togheter with the one you are going to send soon. It will
be easy the real need for the checks you are doing here.
>
> I only mentioned getsockopt() to show that this issue is not
> completely invisible, and is worth patching.
You can also set l2cap_pi(sk)->fcs to NOFCS when the mode configured
is Basic Mode.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Marcel Holtmann @ 2010-08-03 16:50 UTC (permalink / raw)
To: Mat Martineau; +Cc: Gustavo F. Padovan, linux-bluetooth, rshaffer
In-Reply-To: <alpine.DEB.2.00.1008030918360.19861@linux-sea-02>
Hi Mat,
> >>>>>>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> >>>>>>> ---
> >>>>>>> net/bluetooth/l2cap.c | 12 ++++++++----
> >>>>>>> 1 files changed, 8 insertions(+), 4 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> >>>>>>> index 9ba1e8e..aed72f2 100644
> >>>>>>> --- a/net/bluetooth/l2cap.c
> >>>>>>> +++ b/net/bluetooth/l2cap.c
> >>>>>>> @@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
> >>>>>>> goto unlock;
> >>>>>>>
> >>>>>>> if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
> >>>>>>> - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>>>> - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
> >>>>>>> + if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
> >>>>>>> + l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
> >>>>>>> + (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>>>> + l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
> >>>>>>> l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
> >>>>>>
> >>>>>> this becomes unreadable and my brain starts to throw a core dump. So it
> >>>>>> clearly needs to be put into a helper inline function.
> >>>>>
> >>>>> Actually we don't need that, since the code that deals with Basic Mode
> >>>>> never check and use the l2cap_pi(sk)->fcs. So we don't care about FCS
> >>>>> value in the Basic Mode.
> >>>>
> >>>> There isn't currently any Basic Mode code that triggers this latent
> >>>> bug, but I have a patch coming up that does require this fix.
> >>>>
> >>>> As it stands, getsockopt() on a connected basic mode socket shows FCS
> >>>> enabled, so this bug is visible from userspace.
> >>>
> >>> can we just fail the setsockopt() when trying to set basic mode and FCS
> >>> off.
> >>
> >> It definitely makes sense to have more validation of L2CAP_OPTIONS
> >> passed to setsockopt().
> >>
> >>> And also in case fallback to basic mode happens, then FCS should be set
> >>> to be enabled. Since for FCS and basic mode we always have to use FCS.
> >>> So that seems just fine to me.
> >>
> >> The spec says "The FCS option shall only be used when the mode is
> >> being, or is already configured to Enhanced Retransmission mode or
> >> Streaming mode." FCS is never used in basic mode (fallback or not).
> >>
> >> (Maybe I've misunderstood your point)
> >
> > So basic mode uses CRC16 as FCS. And with basic that is not changeable.
> > If you try basic mode + no FCS then we should clearly fail the call to
> > setsockopt() for that.
>
> That's the opposite of what I'm trying to say! :)
>
> Basic mode B-frames have *no* FCS (L2CAP spec section 3.1). The FCS
> field is only found in I-frames and S-frames used in ERTM and
> streaming modes. If you try basic mode + FCS, that should fail.
I confused myself right now. You are fully correct. The default for
basic mode is no fcs.
And as soon as an application wants to use ERTM we just make it to
specific the FCS option. That being CRC16 or no FCS. And of course the
getsockopt() call will return whatever we ended up configuring.
And yes, basic mode + crc16 should fail.
> >>> Maybe you need to explain a bit more in detail what you are trying to
> >>> achieve in conjunction with userspace API.
> >>
> >> My goal is to only have l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 when the
> >> FCS option is actually in use. Otherwise, any logic checking for FCS
> >> also has to check the L2CAP mode. Might as well check the mode once
> >> and set fcs accordingly -- which is what my patch does.
> >>
> >> Gustavo is correct that l2cap_pi(sk)->fcs is currently only checked on
> >> code paths used with ERTM and streaming mode. However, future code
> >> (including a patch I'll be posting soon) will depend on the fcs value
> >> being accurate in all modes.
> >
> > As I said, I have no problem with returning basic mode + crc16. Since
> > that is actually what you are doing in that case.
> >
> > The only thing that we have to keep in mind to be backward compatible is
> > to allow setting of basic mode and fcs = 0x00 and then change that into
> > crc16.
>
> The fcs field in L2CAP_OPTIONS is new for ERTM/Streaming modes, so
> there should be no backward compatibility issues with basic mode.
There is none. I got confused. Too many emails at the same time.
Regards
Marcel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox