* Re: Wireshark
From: Andrei Emeltchenko @ 2012-09-28 13:57 UTC (permalink / raw)
To: Michal.Labedzki; +Cc: linux-bluetooth
In-Reply-To: <E50901D4F2CF69428D43141B7C8586792CFACCF505@EXMB03.eu.tieto.com>
Hi Michał,
On Fri, Sep 28, 2012 at 04:07:44PM +0300, Michal.Labedzki@tieto.com wrote:
> Hello All,
>
> I am trying to add full Bluetooth support to a Wireshark. Wireshark can be
> used as replacement for "hcidump" or other GUI applications for displaying
> Bluetooth frames/packets.
>
> Currently Wireshark support:
> 1. Sniffing via Bluetooth dongle
> 2. Protocols/Profiles:
> HCI (without BLE)
HCI also does not have full support for AMP HCI commands. I can send that
log.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [PATCH v8 00/13] Heart Rate Profile plugin
From: Johan Hedberg @ 2012-09-28 13:59 UTC (permalink / raw)
To: Christian Cier-Zniewski, linux-bluetooth
In-Reply-To: <20120928131647.GA4026@x220>
Hi,
On Fri, Sep 28, 2012, Johan Hedberg wrote:
> On Fri, Sep 28, 2012, Christian Cier-Zniewski wrote:
> > Hello Johan,
> >
> > On 28.09.2012 12:25, Johan Hedberg wrote:
> > > Heart Rate Profile (HRP) client API
> > >
> > > Makefile.am | 9 +-
> > >
> > >All patches in this set have been applied. Thanks.
> > >
> > >
> >
> >
> > I had to apply the following patch to get the heart rate profile working.
> > It is currently missing in HEAD although it was present in the
> > patchset provided by Andrzej.
> >
> > Christian
> >
> > ---
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 3b37198..c27eb01 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -212,7 +212,7 @@ endif
> >
> > if GATTMODULES
> > builtin_modules += thermometer alert time gatt_example proximity
> > deviceinfo \
> > - gatt scanparam
> > + gatt scanparam heartrate
> > builtin_sources += profiles/thermometer/main.c \
> > profiles/thermometer/manager.h \
> > profiles/thermometer/manager.c \
> >
>
> The Makefile.am changes had conflicts because of other GATT profiles I
> applied earlier, so I had to completely manually fix this and probably
> made some mistake. Could you please send a proper git patch for this and
> I'll apply it?
Nevermind, I went ahead and fixed it myself.
Johan
^ permalink raw reply
* Re: [RFC] Convert storage to use per-remote device directories
From: Anderson Lizardo @ 2012-09-28 14:04 UTC (permalink / raw)
To: Frederic Danis; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <5065A248.7020009@linux.intel.com>
Hi Frederic,
On Fri, Sep 28, 2012 at 9:12 AM, Frederic Danis
<frederic.danis@linux.intel.com> wrote:
> Following glib documentation:
> "groups in key files may contain the same key multiple times; the last entry
> wins. Key files may also contain multiple groups with the same name; they
> are merged together."
>
> So, I do not think this meet our needs.
I agree.
>> In LE we can have services with same UUID (not sure if this
>> is valid for BR/EDR also), and they are differentiated by other means
>> (e.g. a "Description" descriptor with different value, besides the
>> different handle).
>
> For BDEDR, it is easy to retrieve info related to a profile as profile's
> UUIDs are unique.
>
> Are "Description" descriptors for a profile well-know or implementation
> dependent ?
>
> Do you think we can use [<UUID>,<Description>] as group name ?
The Description descriptor is profile specific (some profiles don't
have description, but on the other hand are unique in the database).
Unfortunately, I think it is not possible to use them on the group
name.
>> Regarding LE, we have two kinds of storage needs: server profiles, and
>> client profiles.
>>
>> For server profiles, we need to store the attribute database.
>
>
> Is it possible to save it as we did for SDP records (record entry in profile
> group) ?
It seems the SDP records have the UUID as group on the INI file. This
is not feasible for GATT/ATT because the UUID is not a unique key, but
the attribute handle is.
Are you sure that for SDP each record has its own UUID (i.e. no SDP
record shares same UUID)?
>> Currently we don't do this, but we need to implement it, or have
>> ServiceChanged characteristic notifying that the service ranges have
>> changed every time BlueZ restarts (which adds overhead as service
>> discovery is triggered on the client side). I suggest we store the
>> attribute database into a "attribute_db.conf" file containing the
>> attribute name/value/uuid triples.
>
>
> Isn't it possible to save it in [<uuid>] group of device.conf file ?
The attribute database needs to be stored on the adapter side (the
services exposed to remote devices over GATT are group of attributes).
> My first thought for LE data in device.conf is:
> [<Primary UUID>#<Description>]
> Handle=
> Characteristic=
> Attribute=
> CCC=
As mentioned above, the "#Description" part is profile specific and
thus not generic enough for the group name (IMHO). It is actually a
high level information (a Characteristic Descriptor, which is just a
special attribute with a specific UUID.
If that helps, this is how I could imagine the attribute database (see
Core Spec page 1950 for an example database where I based the info
below from):
/var/lib/bluetooth/<adapter address>/attribute_db.conf
[0x0001]
UUID=00002800-0000-1000-8000-00805f9b34fb
Value=0018
[0x0004]
UUID=00002803-0000-1000-8000-00805f9b34fb
Value=020600002A
[0x0006]
UUID=00002a00-0000-1000-8000-00805f9b34fb
Value=4578616D706C6520446576696365
[0x0010]
UUID=00002800-0000-1000-8000-00805f9b34fb
Value=0118
...
With this format, it is easy to "serialize" and "de-serialize" the
attribute database for the adapter.
I still haven't thought for the client side stuff, but they definitely
be on the device directory.
Hope that helps,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH v8 00/13] Heart Rate Profile plugin
From: Christian Cier-Zniewski @ 2012-09-28 14:07 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <20120928131647.GA4026@x220>
Hi,
Am 28.09.2012 15:16, schrieb Johan Hedberg:
> The Makefile.am changes had conflicts because of other GATT profiles I
> applied earlier, so I had to completely manually fix this and probably
> made some mistake. Could you please send a proper git patch for this
> and I'll apply it? Johan
Enable heart rate profile in Makefile.am.
---
Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile.am b/Makefile.am
index 3b37198..c27eb01 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -212,7 +212,7 @@ endif
if GATTMODULES
builtin_modules += thermometer alert time gatt_example proximity
deviceinfo \
- gatt scanparam
+ gatt scanparam heartrate
builtin_sources += profiles/thermometer/main.c \
profiles/thermometer/manager.h \
profiles/thermometer/manager.c \
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH BlueZ v7 2/9] mgmt: Add LE scanning callback
From: Claudio Takahasi @ 2012-09-28 14:27 UTC (permalink / raw)
To: João Paulo Rechi Vita, linux-bluetooth, Claudio Takahasi
In-Reply-To: <20120927074338.GC9610@x220>
Hi Johan:
On Thu, Sep 27, 2012 at 4:43 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi,
>
> On Tue, Sep 25, 2012, João Paulo Rechi Vita wrote:
>> +int mgmt_start_scanning(int index)
>
> Firstly I don't think the name is good since BR/EDR also has the concept
> of scanning (page scan & inquiry scan). Secondly, maybe it'd be simpler
> to reuse mgmt_start_discovery and simply add a new parameter which
> provides the value for info->discov_type?
>
> Johan
My suggestions are:
1. rename to mgmt_start_le_scanning
2. add a new parameter to mgmt_start_discovery: gboolean le_only
3. add a new parameter to mgmt_start_discovery: informing the adapter
operation mode(BREDR/LE/BREDR_LE)
Option 3 will require to expose controller info/settings to adapter.c.
It is necessary to know the controller features to send the right
discover type. btd_adapter_start function could be extended to receive
the supported features or a parameter specifying the operation mode.
IMO, using this approach we are duplicating information.
Which approach do you prefer?
Regards,
Claudio.
^ permalink raw reply
* Re: Wireshark
From: Marcel Holtmann @ 2012-09-28 14:30 UTC (permalink / raw)
To: Johan Hedberg; +Cc: Michal.Labedzki, linux-bluetooth
In-Reply-To: <20120928133944.GC5075@x220>
Hi Michal,
> Have you looked into supporting the new HCI_CHANNEL_MONITOR available in
> recent kernels? There's a very simple "btmon" command-line tool for it
> in bluez.git (see e.g. monitor/control.c) but it can't do any kind of
> high-level decoding (e.g. profiles). It'd be interesting to know if it
> could be easily supported in wireshark since right now there doesn't
> seem to be a viable way of porting decoders from hcidump to btmon due to
> their very different ways of handling buffers etc.
>
> One of the big benefits of the monitor channel compared traditional HCI
> sockets is that you get dynamic notifications of added and removed
> adapters including the very early HCI traffic that occurs. In the long
> run I think the idea is to add also "replay" connection events for
> existing ACL and L2CAP links so that you'd get the right protocol
> decoded even if you start logging when the connection already exists.
I can only second this. Wireshark should only be using the new monitor
socket that we introduced. It could be compared to the pseudo interface
for network interfaces with extra information about added and removed
adapters. It will also tell you if you see a BR/EDR/LE controller vs AMP
controller HCI device.
Especially if you want to do Bluetooth High-Speed debugging, you need to
be able to trace a BR/EDR/LE controller + AMP controller at the same
time.
In addition it will tell you the BD_ADDR of the controller even if you
missed the initial setup HCI packets. That allows for an unique matching
of traffic from different HCI devices even after unplug-replug and their
device names and indexes differ.
The protocol for it is documented in include/net/bluetooth/hci_mon.h in
the kernel code. It is really simple and does support SCM_TIMESTAMP.
Regards
Marcel
^ permalink raw reply
* Re: [RFC] Convert storage to use per-remote device directories
From: Marcel Holtmann @ 2012-09-28 14:33 UTC (permalink / raw)
To: Frederic Danis; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <50642389.40107@intel.com>
Hi Fred,
> Here is my proposal for new storage directory structure using ini-file
> format.
>
> Each adapter directory (/var/lib/bluetooth/<adapter address>/) will
> contain a config file for the local adapter and one directory per remote
> device.
> The adapter config file just need to be converted to ini-file format
> with only 1 group called [adapter].
>
> Each of remote device directories' name will be based on remote device
> address and address type (address#type).
> This directory will contain a config file with remote device infos and a
> linkkey file.
> Remote device config file will include a [device] group with general
> device infos (name, alias, profiles or services list, ...), and groups
> named by profile uuid (or service uuid) with related infos.
>
> So the directory structure should be:
> /var/lib/bluetooth/<adapter address>/
> ./config
> ./<remote device address#type>/
> ./config
> ./linkkey
> ./<remote device address#type>/
> ./config
> ./linkkey
> ...
why do we care about the address type here? Can we actually have a
different link key for BR/EDR and for LE? Right now it is really only
one choice on how to use that remote device. If it is dual-mode, then we
use BR/EDR and if it is single-mode we use LE.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/5] Add support for mSBC frame header
From: Marcel Holtmann @ 2012-09-28 14:45 UTC (permalink / raw)
To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1348757068-31048-3-git-send-email-frederic.dalleau@linux.intel.com>
Hi Fred,
> mSBC modifies header so that it contains: OxAD 0x00 0x00.
> The first bytes allows to distinguish mSBC packets from standard SBC
> packets used in A2DP. The two zero bytes are reserved for future definition.
> ---
> sbc/sbc.c | 104 +++++++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 63 insertions(+), 41 deletions(-)
>
> diff --git a/sbc/sbc.c b/sbc/sbc.c
> index 7e4faa0..131755b 100644
> --- a/sbc/sbc.c
> +++ b/sbc/sbc.c
> @@ -407,55 +407,71 @@ static int sbc_unpack_frame(sbc_t *sbc, const uint8_t *data,
> if (len < 4)
> return -1;
>
> - if (data[0] != SBC_SYNCWORD)
> - return -2;
> -
> - frame->frequency = (data[1] >> 6) & 0x03;
> + if (sbc->flags & SBC_MSBC) {
> + if (data[0] != MSBC_SYNCWORD)
> + return -2;
> + if (data[1] != 0)
> + return -5;
> + if (data[2] != 0)
> + return -6;
I am getting the felling that just splitting this into separate
unpack_sbc_frame and unpack_msbc_frame would make this a lot cleaner
instead of keeping to check if mSBC flag or not is set.
We might even include a function callback in sbc_t where we on sbc_init
just select one or the other. No need to keep repeating that check for
every single frame. As far as I can tell, you never can switch from SBC
to mSBC or vice-versa anyway.
With that you also would not need to change the function prototype to
include sbc_t at all.
Regards
Marcel
^ permalink raw reply
* [PATCH BlueZ v0 0/4] Fix losing Service Changed indication
From: Claudio Takahasi @ 2012-09-28 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
Apply after:
"gatt: Remove reading Service Changed characteristic after connected"
sent from Andrzej Kaczmarek
This patch series adds Service Changed characteristic handle storage
allowing GATT service to manage Service Changed indication when the
connection is established.
The patch to remove the entry from "gatt" file will be sent in the next
patch series. It is unclear to me if we need to extend the device or
profile drive to indicate that the device is being removed/purged from
the system or if the expected implementation is to move to per-device
storage(TODO file). At the moment, it is not possible to detect in the
drivers remove callback if the device is being removed from the system
or if it bluetoothd is exiting.
Claudio Takahasi (4):
gatt: Remove unneeded handle range check
gatt: Ignore Service Changed if not bonded
gatt: Add storing Service Changed handle
gatt: Add reading stored Service Changed handle
profiles/gatt/gas.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 83 insertions(+), 7 deletions(-)
--
1.7.12
^ permalink raw reply
* [PATCH BlueZ v0 1/4] gatt: Remove unneeded handle range check
From: Claudio Takahasi @ 2012-09-28 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1348843674-10878-1-git-send-email-claudio.takahasi@openbossa.org>
Since the Service Changed is a read-only control point, it not necessary
to compare the previous value read. This logic was added to compare the
result of the read operation with the received indications. Repeated
indications containing the same range is a valid procedure.
---
profiles/gatt/gas.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index d189221..a828fcd 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -44,7 +44,6 @@ struct gas {
struct btd_device *device;
struct att_range gap; /* GAP Primary service range */
struct att_range gatt; /* GATT Primary service range */
- struct att_range changed; /* Affected handle range */
GAttrib *attrib;
guint attioid;
guint changed_ind;
@@ -152,12 +151,6 @@ static void indication_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
olen = enc_confirmation(opdu, plen);
g_attrib_send(gas->attrib, 0, opdu[0], opdu, olen, NULL, NULL, NULL);
- if (gas->changed.start == start && gas->changed.end == end)
- return;
-
- gas->changed.start = start;
- gas->changed.end = end;
-
btd_device_gatt_set_service_changed(gas->device, start, end);
}
--
1.7.12
^ permalink raw reply related
* [PATCH BlueZ v0 2/4] gatt: Ignore Service Changed if not bonded
From: Claudio Takahasi @ 2012-09-28 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1348843674-10878-1-git-send-email-claudio.takahasi@openbossa.org>
This patch fix replying for Service Changed indication when the device
is not bonded. From Erratum 3951: Indications caused by changes to the
Service Changed Characteristic Value shall be considered lost if the
client has not enabled indications the Client Configuration
Characteristic Descriptor.
---
profiles/gatt/gas.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index a828fcd..b42aa64 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -146,6 +146,11 @@ static void indication_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
DBG("Service Changed start: 0x%04X end: 0x%04X", start, end);
+ if (device_is_bonded(gas->device) == FALSE) {
+ DBG("Ignoring Service Changed: device is not bonded");
+ return;
+ }
+
/* Confirming indication received */
opdu = g_attrib_get_buffer(gas->attrib, &plen);
olen = enc_confirmation(opdu, plen);
--
1.7.12
^ permalink raw reply related
* [PATCH BlueZ v0 3/4] gatt: Add storing Service Changed handle
From: Claudio Takahasi @ 2012-09-28 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1348843674-10878-1-git-send-email-claudio.takahasi@openbossa.org>
This patch stores the Service Changed control-point handle in order to
be able to listen for Service Changed indications when the connection
is established.
---
profiles/gatt/gas.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index b42aa64..8b6a0bd 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -25,6 +25,9 @@
#endif
#include <stdbool.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
#include <glib.h>
#include <bluetooth/uuid.h>
@@ -37,6 +40,7 @@
#include "btio.h"
#include "gatt.h"
#include "log.h"
+#include "textfile.h"
#include "gas.h"
/* Generic Attribute/Access Service */
@@ -70,6 +74,34 @@ static gint cmp_device(gconstpointer a, gconstpointer b)
return (gas->device == device ? 0 : -1);
}
+static inline int create_filename(char *buf, size_t size,
+ const bdaddr_t *bdaddr, const char *name)
+{
+ char addr[18];
+
+ ba2str(bdaddr, addr);
+
+ return create_name(buf, size, STORAGEDIR, addr, name);
+}
+
+static int write_ctp_handle(bdaddr_t *sba, bdaddr_t *dba, uint8_t bdaddr_type,
+ uint16_t uuid, uint16_t handle)
+{
+ char filename[PATH_MAX + 1], addr[18], key[27], value[7];
+
+ create_filename(filename, PATH_MAX, sba, "gatt");
+
+ create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+ ba2str(dba, addr);
+
+ snprintf(key, sizeof(key), "%17s#%hhu#0x%4.4x", addr, bdaddr_type,
+ uuid);
+ snprintf(value, sizeof(value), "0x%4.4x", handle);
+
+ return textfile_put(filename, key, value);
+}
+
static void gap_appearance_cb(guint8 status, const guint8 *pdu, guint16 plen,
gpointer user_data)
{
@@ -107,6 +139,10 @@ done:
static void ccc_written_cb(guint8 status, const guint8 *pdu, guint16 plen,
gpointer user_data)
{
+ struct gas *gas = user_data;
+ bdaddr_t sba, dba;
+ uint8_t bdaddr_type;
+
if (status) {
error("Write Service Changed CCC failed: %s",
att_ecode2str(status));
@@ -114,6 +150,12 @@ static void ccc_written_cb(guint8 status, const guint8 *pdu, guint16 plen,
}
DBG("Service Changed indications enabled");
+
+ adapter_get_address(device_get_adapter(gas->device), &sba);
+ device_get_address(gas->device, &dba, &bdaddr_type);
+
+ write_ctp_handle(&sba, &dba, bdaddr_type, GATT_CHARAC_SERVICE_CHANGED,
+ gas->changed_handle);
}
static void write_ccc(GAttrib *attrib, uint16_t handle, gpointer user_data)
--
1.7.12
^ permalink raw reply related
* [PATCH BlueZ v0 4/4] gatt: Add reading stored Service Changed handle
From: Claudio Takahasi @ 2012-09-28 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1348843674-10878-1-git-send-email-claudio.takahasi@openbossa.org>
This patch avoids discovering Service Changed handle when the ATT
connection is established and there is a previous stored handle.
---
profiles/gatt/gas.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index 8b6a0bd..da6d555 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -25,9 +25,11 @@
#endif
#include <stdbool.h>
+#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
+#include <errno.h>
#include <glib.h>
#include <bluetooth/uuid.h>
@@ -102,6 +104,32 @@ static int write_ctp_handle(bdaddr_t *sba, bdaddr_t *dba, uint8_t bdaddr_type,
return textfile_put(filename, key, value);
}
+static int read_ctp_handle(bdaddr_t *sba, bdaddr_t *dba, uint8_t bdaddr_type,
+ uint16_t uuid, uint16_t *value)
+{
+ char filename[PATH_MAX + 1], addr[18], key[27];
+ char *str;
+
+ create_filename(filename, PATH_MAX, sba, "gatt");
+
+ ba2str(dba, addr);
+ snprintf(key, sizeof(key), "%17s#%hhu#0x%04x", addr, bdaddr_type,
+ uuid);
+
+ str = textfile_get(filename, key);
+ if (str == NULL)
+ return -errno;
+
+ if (sscanf(str, "%hx", value) != 1) {
+ free(str);
+ return -ENOENT;
+ }
+
+ free(str);
+
+ return 0;
+}
+
static void gap_appearance_cb(guint8 status, const guint8 *pdu, guint16 plen,
gpointer user_data)
{
@@ -349,6 +377,8 @@ int gas_register(struct btd_device *device, struct att_range *gap,
struct att_range *gatt)
{
struct gas *gas;
+ bdaddr_t sba, dba;
+ uint8_t bdaddr_type;
gas = g_new0(struct gas, 1);
gas->gap.start = gap->start;
@@ -364,6 +394,12 @@ int gas_register(struct btd_device *device, struct att_range *gap,
attio_connected_cb,
attio_disconnected_cb, gas);
+ adapter_get_address(device_get_adapter(gas->device), &sba);
+ device_get_address(gas->device, &dba, &bdaddr_type);
+
+ read_ctp_handle(&sba, &dba, bdaddr_type, GATT_CHARAC_SERVICE_CHANGED,
+ &gas->changed_handle);
+
return 0;
}
--
1.7.12
^ permalink raw reply related
* Re: [PATCH 1/5] Add msbc encoding and decoding flag
From: Marcel Holtmann @ 2012-09-28 14:53 UTC (permalink / raw)
To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1348757068-31048-2-git-send-email-frederic.dalleau@linux.intel.com>
Hi Fred,
> Also enable 15 blocks support using stdc sbc primitives
> ---
> Makefile.am | 1 +
> sbc/sbc.c | 123 +++++++++--------
> sbc/sbc.h | 3 +
> sbc/sbc_primitives.c | 8 +-
> sbc/sbc_primitives.h | 7 +-
> sbc/sbc_primitives_stdc.c | 321 +++++++++++++++++++++++++++++++++++++++++++++
> sbc/sbc_primitives_stdc.h | 36 +++++
> 7 files changed, 443 insertions(+), 56 deletions(-)
> create mode 100644 sbc/sbc_primitives_stdc.c
> create mode 100644 sbc/sbc_primitives_stdc.h
can you split this patch into at least two?
> diff --git a/Makefile.am b/Makefile.am
> index cad6a3b..75e3a4a 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -14,6 +14,7 @@ sbc_headers = sbc/sbc.h
>
> sbc_sources = sbc/sbc.c sbc/sbc_private.h sbc/sbc_math.h sbc/sbc_tables.h \
> sbc/sbc_primitives.h sbc/sbc_primitives.c \
> + sbc/sbc_primitives_stdc.h sbc/sbc_primitives_stdc.c \
> sbc/sbc_primitives_mmx.h sbc/sbc_primitives_mmx.c \
> sbc/sbc_primitives_iwmmxt.h sbc/sbc_primitives_iwmmxt.c \
> sbc/sbc_primitives_neon.h sbc/sbc_primitives_neon.c \
> diff --git a/sbc/sbc.c b/sbc/sbc.c
> index f0c77c7..7e4faa0 100644
> --- a/sbc/sbc.c
> +++ b/sbc/sbc.c
> @@ -6,6 +6,7 @@
> * Copyright (C) 2004-2010 Marcel Holtmann <marcel@holtmann.org>
> * Copyright (C) 2004-2005 Henryk Ploetz <henryk@ploetzli.ch>
> * Copyright (C) 2005-2008 Brad Midgley <bmidgley@xmission.com>
> + * Copyright (C) 2012 Intel Corporation
> *
> *
> * This library is free software; you can redistribute it and/or
> @@ -51,6 +52,17 @@
> #include "sbc_primitives.h"
>
> #define SBC_SYNCWORD 0x9C
> +#define MSBC_SYNCWORD 0xAD
> +#define SBC_BLOCKS(sbc, blocks) (((sbc)->flags & SBC_MSBC) \
> + ? MSBC_BLOCKS : (blocks))
Not sure about this macro. Why do we need this? Wouldn't it be better to
make it explicit in the code.
> +#define MSBC_BLOCKMODE SBC_BLK_16
> +#define MSBC_BLOCKS 15
> +#define MSBC_BITPOOL 26
> +#define MSBC_SUBBAND_MODE 1
> +#define MSBC_SUBBANDS 8
> +#define MSBC_CHANNEL 1
> +#define MSBC_ALLOCATION SBC_AM_LOUDNESS
And as a nitpick, keep an empty line between SBC_ and MSBC_ constants.
> /* This structure contains an unpacked SBC frame.
> Yes, there is probably quite some unused space herein */
> @@ -373,9 +385,11 @@ static void sbc_calculate_bits(const struct sbc_frame *frame, int (*bits)[8])
> * -2 Sync byte incorrect
> * -3 CRC8 incorrect
> * -4 Bitpool value out of bounds
> + * -5 msbc reserved byte 1 not 0
> + * -6 msbc reserved byte 2 not 0
> */
What are you doing with these return values. If nothing, then just
return sync byte incorrect.
> -static int sbc_unpack_frame(const uint8_t *data, struct sbc_frame *frame,
> - size_t len)
> +static int sbc_unpack_frame(sbc_t *sbc, const uint8_t *data,
> + struct sbc_frame *frame, size_t len)
> {
As in the comment from the other patch, instead of changing the function
prototype, just provide two separate function, that we choose from on
sbc_init.
> unsigned int consumed;
> /* Will copy the parts of the header that are relevant to crc
> @@ -413,6 +427,8 @@ static int sbc_unpack_frame(const uint8_t *data, struct sbc_frame *frame,
> frame->blocks = 16;
> break;
> }
> + if (sbc->flags & SBC_MSBC)
> + frame->blocks = MSBC_BLOCKS;
>
> frame->mode = (data[1] >> 2) & 0x03;
> switch (frame->mode) {
> @@ -690,13 +706,13 @@ static int sbc_analyze_audio(struct sbc_encoder_state *state,
> for (ch = 0; ch < frame->channels; ch++) {
> x = &state->X[ch][state->position - 16 +
> frame->blocks * 4];
> - for (blk = 0; blk < frame->blocks; blk += 4) {
> + for (blk = 0; blk < frame->blocks; blk += state->inc) {
> state->sbc_analyze_4b_4s(
> x,
> frame->sb_sample_f[blk][ch],
> frame->sb_sample_f[blk + 1][ch] -
> frame->sb_sample_f[blk][ch]);
> - x -= 16;
> + x -= 4 * state->inc;
> }
> }
> return frame->blocks * 4;
> @@ -705,13 +721,13 @@ static int sbc_analyze_audio(struct sbc_encoder_state *state,
> for (ch = 0; ch < frame->channels; ch++) {
> x = &state->X[ch][state->position - 32 +
> frame->blocks * 8];
> - for (blk = 0; blk < frame->blocks; blk += 4) {
> + for (blk = 0; blk < frame->blocks; blk += state->inc) {
> state->sbc_analyze_4b_8s(
> x,
> frame->sb_sample_f[blk][ch],
> frame->sb_sample_f[blk + 1][ch] -
> frame->sb_sample_f[blk][ch]);
> - x -= 32;
> + x -= 8 * state->inc;
> }
> }
> return frame->blocks * 8;
> @@ -764,10 +780,9 @@ static int sbc_analyze_audio(struct sbc_encoder_state *state,
> * -99 not implemented
> */
>
> -static SBC_ALWAYS_INLINE ssize_t sbc_pack_frame_internal(uint8_t *data,
> - struct sbc_frame *frame, size_t len,
> - int frame_subbands, int frame_channels,
> - int joint)
> +static SBC_ALWAYS_INLINE ssize_t sbc_pack_frame_internal(sbc_t *sbc,
> + uint8_t *data, struct sbc_frame *frame, size_t len,
> + int frame_subbands, int frame_channels, int joint)
> {
> /* Bitstream writer starts from the fourth byte */
> uint8_t *data_ptr = data + 4;
> @@ -785,37 +800,37 @@ static SBC_ALWAYS_INLINE ssize_t sbc_pack_frame_internal(uint8_t *data,
> uint32_t levels[2][8]; /* levels are derived from that */
> uint32_t sb_sample_delta[2][8];
>
> - data[0] = SBC_SYNCWORD;
> + data[0] = SBC_SYNCWORD;
>
> - data[1] = (frame->frequency & 0x03) << 6;
> + data[1] = (frame->frequency & 0x03) << 6;
>
> - data[1] |= (frame->block_mode & 0x03) << 4;
> + data[1] |= (frame->block_mode & 0x03) << 4;
>
> - data[1] |= (frame->mode & 0x03) << 2;
> + data[1] |= (frame->mode & 0x03) << 2;
>
> - data[1] |= (frame->allocation & 0x01) << 1;
> + data[1] |= (frame->allocation & 0x01) << 1;
>
> - switch (frame_subbands) {
> - case 4:
> - /* Nothing to do */
> - break;
> - case 8:
> - data[1] |= 0x01;
> - break;
> - default:
> - return -4;
> - break;
> - }
> + switch (frame_subbands) {
> + case 4:
> + /* Nothing to do */
> + break;
> + case 8:
> + data[1] |= 0x01;
> + break;
> + default:
> + return -4;
> + break;
> + }
>
> - data[2] = frame->bitpool;
> + data[2] = frame->bitpool;
>
> - if ((frame->mode == MONO || frame->mode == DUAL_CHANNEL) &&
> - frame->bitpool > frame_subbands << 4)
> - return -5;
> + if ((frame->mode == MONO || frame->mode == DUAL_CHANNEL) &&
> + frame->bitpool > frame_subbands << 4)
> + return -5;
>
> - if ((frame->mode == STEREO || frame->mode == JOINT_STEREO) &&
> - frame->bitpool > frame_subbands << 5)
> - return -5;
> + if ((frame->mode == STEREO || frame->mode == JOINT_STEREO) &&
> + frame->bitpool > frame_subbands << 5)
> + return -5;
>
> /* Can't fill in crc yet */
>
> @@ -881,33 +896,33 @@ static SBC_ALWAYS_INLINE ssize_t sbc_pack_frame_internal(uint8_t *data,
> return data_ptr - data;
> }
>
> -static ssize_t sbc_pack_frame(uint8_t *data, struct sbc_frame *frame, size_t len,
> - int joint)
> +static ssize_t sbc_pack_frame(sbc_t *sbc, uint8_t *data,
> + struct sbc_frame *frame, size_t len, int joint)
> {
> if (frame->subbands == 4) {
> if (frame->channels == 1)
> return sbc_pack_frame_internal(
> - data, frame, len, 4, 1, joint);
> + sbc, data, frame, len, 4, 1, joint);
> else
> return sbc_pack_frame_internal(
> - data, frame, len, 4, 2, joint);
> + sbc, data, frame, len, 4, 2, joint);
> } else {
> if (frame->channels == 1)
> return sbc_pack_frame_internal(
> - data, frame, len, 8, 1, joint);
> + sbc, data, frame, len, 8, 1, joint);
> else
> return sbc_pack_frame_internal(
> - data, frame, len, 8, 2, joint);
> + sbc, data, frame, len, 8, 2, joint);
> }
> }
>
> -static void sbc_encoder_init(struct sbc_encoder_state *state,
> +static void sbc_encoder_init(int msbc, struct sbc_encoder_state *state,
> const struct sbc_frame *frame)
> {
> memset(&state->X, 0, sizeof(state->X));
> state->position = (SBC_X_BUFFER_SIZE - frame->subbands * 9) & ~7;
>
> - sbc_init_primitives(state);
> + sbc_init_primitives(msbc, state);
> }
I personally prefer having mSBC capable primitives. If this would
actually lead to some sort of issue, then we might need a different
option that allows us to fallback to non-optimized primitives. And then
we might not just bind this for mSBC, then we might just make that an
option for the library users.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 0/5] mSBC tests
From: Marcel Holtmann @ 2012-09-28 15:05 UTC (permalink / raw)
To: Siarhei Siamashka; +Cc: Frédéric Dalleau, linux-bluetooth
In-Reply-To: <20120927233420.3441f14e@i7>
Hi Siarhei,
> > I wanted to know more about mSBC and I look into the spec and saw it
> > was just a set of settings for SBC. So I tried to exercise them using
> > sbcenc and sbcdec and found it wasn't possible, I ended modifying SBC
> > library and now it should be possible to use mSBC settings.
> > Using 15 block doesn't work using SIMD processing because of data
> > reordering so I recalled some legacy C code from an older release
> > (sbc_analyse_eight) and put in a separate primitives file.
>
>
> I wonder why they decided to go with the odd sided blocks? Anyway,
> I guess there is no other way but to adapt to this extended spec.
> Thanks for taking a look at this stuff.
>
> Have you tried to tweak the SIMD code to make it process one
> block at a time instead of batches of 4?
>
> I suspect that we might only need to track the state and flip between
> "even" and "odd" constant tables for every other processed block in
> "sbc_analyze_4b_4s_simd" and "sbc_analyze_4b_8s_simd" replacement
> functions. The functions "sbc_encoder_process_input_s4_internal"
> and "sbc_encoder_process_input_s8_internal" also need to be
> updated to handle arbitrary number of blocks and not just
> multiples of 4. Though I haven't been working with sbc code lately
> and may be mistaken.
>
> Using the old tables "_sbc_proto_fixed8" and "_sbc_proto_fixed4" has
> a disadvantage of not having the tweaks from:
> http://git.kernel.org/?p=bluetooth/sbc.git;a=commit;h=bec3d3989ba2
> They haven't been in use for ages and just remained in the sources by.
>
> If we just could make SIMD code work, that would be the best option in
> my opinion.
>
> > Work done from the spec, not tested with any reference encoder.
>
> Does a reference mSBC encoder exist somewhere? Or any ideas how to test
> the new code with something other than bluez just to be sure that it's
> correct?
I looked at the Bluetooth SIG website and couldn't find a reference
encoder or sample files. The old SBC reference encoder and decoder don't
seem to support other block sizes.
SBC Encoder LIB Version 1.5
Copyright (c) 2002 Philips Consumer Electronics, ASA Labs
Usage:
sbc_encoder [-jsv] [-lblk_len] [-nsubbands] [-p] [-rrate] [-ooutputfile] inputfile
[-s] use the stereo mode for stereo signals
[-v] verbose mode
[-j] allow the use of joint coding for stereo signals
[-lblk_len] blk_len specifies the APCM block length, out of [4,8,12,16]
[-nsubbands] subbands specifies the number of subbands, out of [4,8]
[-p] a simple psycho acoustic model is used
[-rrate] specifies the bit rate in bps
[-ooutputfile] specifies the name of the bitstream output file
inputfile specifies the audio input file, the major audio formats are supported
SBC Decoder LIB Version 1.5
Copyright (c) 2002 Philips Consumer Electronics, ASA Labs
Usage:
sbc_decoder [-v] [-ooutputfile] [-pstartpos] inputfile
[-v] verbose mode
[-pstartpos] startpos specifies the byte offset to start with decoding
[-ooutputfile] specifies the name of the audio output file
inputfile specifies the name of the bitstream input file
And the sample files are all from 2001. We might have to create our own
ones and our own naive and stupid reference encoder/decoder.
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv2] Bluetooth: A2MP: Correct assoc_len size
From: Gustavo Padovan @ 2012-09-28 15:53 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1348840500-16336-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-09-28 16:55:00 +0300]:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Correct assoc_len and fix warning for x86-64 by using %zu specifier
> reported by Fengguang Wu <fengguang.wu@intel.com>.
>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
> v2: make assoc_len type of size_t
> net/bluetooth/a2mp.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
Patch has been applied to bluetooth-next, Thanks.
Gustavo
^ permalink raw reply
* Re: [PATCH] Bluetooth: Use %zu print specifier for size_t type
From: Gustavo Padovan @ 2012-09-28 15:54 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1348832663-13569-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-09-28 14:44:23 +0300]:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Correct warnings reported by Fengguang Wu <fengguang.wu@intel.com>.
>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
> net/bluetooth/hci_event.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Patch has been applied to bluetooth-next. Thanks.
Gustavo
^ permalink raw reply
* [PATCH v4 0/5] Audio profile authorization
From: Mikel Astiz @ 2012-09-28 16:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
v4 considers the review from Luiz and Johan, including:
- guint used for authorization ids, instead of int
- static local variable used to generate new ids
- service_auth_id renamed to auth_id
>From original cover-letter:
The agent-based profile authorization makes a special consideration for audio profiles: they are processed as if they all belonged to one single profile. This includes several internal policies that are inconvenient for IVI use-cases.
This patchset removes this exception by making use of the conventional authorization mechanism also for audio profiles.
The new approach is not straightforward since devices might send several connection requets in parallel (i.e. HFP, A2DP, AVRCP). This cannot be neither automatically rejected (EBUSY) nor forwarded in parallel to the agent, so a queue was added to store the pending authorization requests. These will be sent to the agent sequentially.
Mikel Astiz (5):
build: Update glib dependency to 2.32
adapter: Replace device authorizing flag
adapter: Use authorization id for cancelling
adapter: Queue parallel authorization requests
audio: Drop audio-specific authorization mechanism
acinclude.m4 | 4 +-
audio/avctp.c | 27 +++---
audio/avdtp.c | 23 ++++--
audio/device.c | 144 --------------------------------
audio/device.h | 12 +--
audio/manager.c | 28 ++++---
plugins/service.c | 18 ++--
profiles/input/server.c | 7 +-
profiles/network/server.c | 9 +-
profiles/sap/server.c | 8 +-
src/adapter.c | 204 +++++++++++++++++++++++++++++-----------------
src/adapter.h | 4 +-
src/device.c | 11 ---
src/device.h | 2 -
src/profile.c | 25 ++----
15 files changed, 207 insertions(+), 319 deletions(-)
--
1.7.11.4
^ permalink raw reply
* [PATCH v4 1/5] build: Update glib dependency to 2.32
From: Mikel Astiz @ 2012-09-28 16:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1348849945-5762-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
This version of the library adds several convenient features such as
g_queue_free_full.
---
acinclude.m4 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/acinclude.m4 b/acinclude.m4
index ed2d011..9f4b11f 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -99,8 +99,8 @@ AC_DEFUN([AC_PATH_DBUS], [
])
AC_DEFUN([AC_PATH_GLIB], [
- PKG_CHECK_MODULES(GLIB, glib-2.0 >= 2.28, dummy=yes,
- AC_MSG_ERROR(GLib >= 2.28 is required))
+ PKG_CHECK_MODULES(GLIB, glib-2.0 >= 2.32, dummy=yes,
+ AC_MSG_ERROR(GLib >= 2.32 is required))
AC_SUBST(GLIB_CFLAGS)
AC_SUBST(GLIB_LIBS)
])
--
1.7.11.4
^ permalink raw reply related
* [PATCH v4 2/5] adapter: Replace device authorizing flag
From: Mikel Astiz @ 2012-09-28 16:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1348849945-5762-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Refactor code to drop the device authorizing flag by replacing it with a
private authorization pointer in btd_adapter. After all, no more than
one authorization can be ongoing, so the code is easier to follow if
this is made explicit.
---
src/adapter.c | 56 +++++++++++++++++++++++++++++++++++++-------------------
src/device.c | 11 -----------
src/device.h | 2 --
3 files changed, 37 insertions(+), 32 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 421cd43..524885c 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -124,7 +124,8 @@ struct btd_adapter {
GSList *found_devices;
GSList *oor_devices; /* out of range device list */
struct agent *agent; /* For the new API */
- guint auth_idle_id; /* Ongoing authorization */
+ guint auth_idle_id; /* Ongoing authorization (trusted) */
+ struct service_auth *auth; /* Ongoing authorization */
GSList *connections; /* Connected devices */
GSList *devices; /* Devices structure pointers */
GSList *mode_sessions; /* Request Mode sessions */
@@ -964,8 +965,11 @@ void adapter_remove_device(struct btd_adapter *adapter,
agent = device_get_agent(device);
- if (agent && device_is_authorizing(device))
+ if (agent && adapter->auth && adapter->auth->device == device) {
+ g_free(adapter->auth);
+ adapter->auth = NULL;
agent_cancel(agent);
+ }
device_remove(device, remove_storage);
}
@@ -2433,6 +2437,9 @@ static void adapter_free(gpointer user_data)
if (adapter->auth_idle_id)
g_source_remove(adapter->auth_idle_id);
+ if (adapter->auth)
+ g_free(adapter->auth);
+
if (adapter->off_timer)
off_timer_remove(adapter);
@@ -3167,22 +3174,28 @@ void btd_unregister_adapter_driver(struct btd_adapter_driver *driver)
static void agent_auth_cb(struct agent *agent, DBusError *derr,
void *user_data)
{
- struct service_auth *auth = user_data;
+ struct btd_adapter *adapter = user_data;
+ struct service_auth *auth = adapter->auth;
- device_set_authorizing(auth->device, FALSE);
+ adapter->auth = NULL;
auth->cb(derr, auth->user_data);
+
+ g_free(auth);
}
static gboolean auth_idle_cb(gpointer user_data)
{
- struct service_auth *auth = user_data;
- struct btd_adapter *adapter = auth->adapter;
+ struct btd_adapter *adapter = user_data;
+ struct service_auth *auth = adapter->auth;
+ adapter->auth = NULL;
adapter->auth_idle_id = 0;
auth->cb(NULL, auth->user_data);
+ g_free(auth);
+
return FALSE;
}
@@ -3206,7 +3219,7 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
if (!g_slist_find(adapter->connections, device))
error("Authorization request for non-connected device!?");
- if (adapter->auth_idle_id)
+ if (adapter->auth != NULL)
return -EBUSY;
auth = g_try_new0(struct service_auth, 1);
@@ -3219,10 +3232,8 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
auth->adapter = adapter;
if (device_is_trusted(device) == TRUE) {
- adapter->auth_idle_id = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE,
- auth_idle_cb, auth,
- g_free);
- return 0;
+ adapter->auth_idle_id = g_idle_add(auth_idle_cb, adapter);
+ goto done;
}
agent = device_get_agent(device);
@@ -3234,14 +3245,16 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
dev_path = device_get_path(device);
- err = agent_authorize(agent, dev_path, uuid, agent_auth_cb, auth,
- g_free);
- if (err < 0)
+ err = agent_authorize(agent, dev_path, uuid, agent_auth_cb, adapter,
+ NULL);
+ if (err < 0) {
g_free(auth);
- else
- device_set_authorizing(device, TRUE);
+ return err;
+ }
- return err;
+done:
+ adapter->auth = auth;
+ return 0;
}
int btd_request_authorization(const bdaddr_t *src, const bdaddr_t *dst,
@@ -3294,6 +3307,9 @@ int btd_cancel_authorization(const bdaddr_t *src, const bdaddr_t *dst)
return 0;
}
+ if (!adapter->auth || adapter->auth->device != device)
+ return -EPERM;
+
/*
* FIXME: Cancel fails if authorization is requested to adapter's
* agent and in the meanwhile CreatePairedDevice is called.
@@ -3305,8 +3321,10 @@ int btd_cancel_authorization(const bdaddr_t *src, const bdaddr_t *dst)
err = agent_cancel(agent);
- if (err == 0)
- device_set_authorizing(device, FALSE);
+ if (err == 0) {
+ g_free(adapter->auth);
+ adapter->auth = NULL;
+ }
return err;
}
diff --git a/src/device.c b/src/device.c
index efa3dbe..85b1d0c 100644
--- a/src/device.c
+++ b/src/device.c
@@ -163,7 +163,6 @@ struct btd_device {
gboolean bonded;
gboolean auto_connect;
- gboolean authorizing;
gint ref;
GIOChannel *att_io;
@@ -3040,16 +3039,6 @@ gboolean device_is_authenticating(struct btd_device *device)
return (device->authr != NULL);
}
-gboolean device_is_authorizing(struct btd_device *device)
-{
- return device->authorizing;
-}
-
-void device_set_authorizing(struct btd_device *device, gboolean auth)
-{
- device->authorizing = auth;
-}
-
void device_register_services(struct btd_device *device,
GSList *prim_list, int psm)
{
diff --git a/src/device.h b/src/device.h
index eb45244..e82fd0e 100644
--- a/src/device.h
+++ b/src/device.h
@@ -100,8 +100,6 @@ int device_notify_pincode(struct btd_device *device, gboolean secure,
const char *pincode, void *cb);
void device_cancel_authentication(struct btd_device *device, gboolean aborted);
gboolean device_is_authenticating(struct btd_device *device);
-gboolean device_is_authorizing(struct btd_device *device);
-void device_set_authorizing(struct btd_device *device, gboolean auth);
void device_add_connection(struct btd_device *device);
void device_remove_connection(struct btd_device *device);
void device_request_disconnect(struct btd_device *device, DBusMessage *msg);
--
1.7.11.4
^ permalink raw reply related
* [PATCH v4 3/5] adapter: Use authorization id for cancelling
From: Mikel Astiz @ 2012-09-28 16:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1348849945-5762-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Return a request id in btd_request_authorization() in order to be used
when the request needs to be cancelled. This id alone will be enough to
use btd_cancel_authorization().
---
audio/device.c | 25 +++++++++++-------
plugins/service.c | 18 ++++++++-----
profiles/input/server.c | 7 +++--
profiles/network/server.c | 9 +++----
profiles/sap/server.c | 8 +++---
src/adapter.c | 67 ++++++++++++++++++++++++++++-------------------
src/adapter.h | 4 +--
src/profile.c | 25 ++++++------------
8 files changed, 87 insertions(+), 76 deletions(-)
diff --git a/audio/device.c b/audio/device.c
index 99d6512..c6ae96a 100644
--- a/audio/device.c
+++ b/audio/device.c
@@ -83,6 +83,7 @@ struct dev_priv {
sink_state_t sink_state;
avctp_state_t avctp_state;
GSList *auths;
+ guint auth_id;
DBusMessage *conn_req;
DBusMessage *dc_req;
@@ -737,6 +738,8 @@ static void auth_cb(DBusError *derr, void *user_data)
struct audio_device *dev = user_data;
struct dev_priv *priv = dev->priv;
+ priv->auth_id = 0;
+
if (derr == NULL)
priv->authorized = TRUE;
@@ -797,7 +800,6 @@ int audio_device_request_authorization(struct audio_device *dev,
{
struct dev_priv *priv = dev->priv;
struct service_auth *auth;
- int err;
auth = g_try_new0(struct service_auth, 1);
if (!auth)
@@ -815,14 +817,15 @@ int audio_device_request_authorization(struct audio_device *dev,
return 0;
}
- err = btd_request_authorization(&dev->src, &dev->dst, uuid, auth_cb,
- dev);
- if (err < 0) {
- priv->auths = g_slist_remove(priv->auths, auth);
- g_free(auth);
- }
+ priv->auth_id = btd_request_authorization(&dev->src, &dev->dst, uuid,
+ auth_cb, dev);
+ if (priv->auth_id != 0)
+ return 0;
+
+ priv->auths = g_slist_remove(priv->auths, auth);
+ g_free(auth);
- return err;
+ return -EPERM;
}
int audio_device_cancel_authorization(struct audio_device *dev,
@@ -850,8 +853,10 @@ int audio_device_cancel_authorization(struct audio_device *dev,
if (priv->auth_idle_id > 0) {
g_source_remove(priv->auth_idle_id);
priv->auth_idle_id = 0;
- } else
- btd_cancel_authorization(&dev->src, &dev->dst);
+ } else {
+ btd_cancel_authorization(priv->auth_id);
+ priv->auth_id = 0;
+ }
}
return 0;
diff --git a/plugins/service.c b/plugins/service.c
index e02a673..45886ac 100644
--- a/plugins/service.c
+++ b/plugins/service.c
@@ -65,6 +65,7 @@ struct pending_auth {
char *sender;
bdaddr_t dst;
char uuid[MAX_LEN_UUID_STR];
+ guint id;
};
struct service_adapter {
@@ -557,8 +558,9 @@ done:
else
bacpy(&src, BDADDR_ANY);
- btd_request_authorization(&src, &auth->dst,
- auth->uuid, auth_cb, serv_adapter);
+ auth->id = btd_request_authorization(&src, &auth->dst,
+ auth->uuid, auth_cb,
+ serv_adapter);
}
static DBusMessage *request_authorization(DBusConnection *conn,
@@ -633,8 +635,9 @@ static DBusMessage *request_authorization(DBusConnection *conn,
else
bacpy(&src, BDADDR_ANY);
- if (btd_request_authorization(&src, &auth->dst, auth->uuid, auth_cb,
- serv_adapter) < 0) {
+ auth->id = btd_request_authorization(&src, &auth->dst, auth->uuid,
+ auth_cb, serv_adapter);
+ if (auth->id == 0) {
serv_adapter->pending_list = g_slist_remove(serv_adapter->pending_list,
auth);
g_free(auth);
@@ -664,7 +667,7 @@ static DBusMessage *cancel_authorization(DBusConnection *conn,
else
bacpy(&src, BDADDR_ANY);
- btd_cancel_authorization(&src, &auth->dst);
+ btd_cancel_authorization(auth->id);
reply = btd_error_not_authorized(auth->msg);
dbus_message_unref(auth->msg);
@@ -683,8 +686,9 @@ static DBusMessage *cancel_authorization(DBusConnection *conn,
else
bacpy(&src, BDADDR_ANY);
- btd_request_authorization(&src, &auth->dst,
- auth->uuid, auth_cb, serv_adapter);
+ auth->id = btd_request_authorization(&src, &auth->dst,
+ auth->uuid, auth_cb,
+ serv_adapter);
done:
return dbus_message_new_method_return(msg);
diff --git a/profiles/input/server.c b/profiles/input/server.c
index f71fdc0..eaf3b6a 100644
--- a/profiles/input/server.c
+++ b/profiles/input/server.c
@@ -154,7 +154,7 @@ static void confirm_event_cb(GIOChannel *chan, gpointer user_data)
bdaddr_t src, dst;
GError *err = NULL;
char addr[18];
- int ret;
+ guint ret;
bt_io_get(chan, &err,
BT_IO_OPT_SOURCE_BDADDR, &src,
@@ -179,12 +179,11 @@ static void confirm_event_cb(GIOChannel *chan, gpointer user_data)
ret = btd_request_authorization(&src, &dst, HID_UUID,
auth_callback, server);
- if (ret == 0)
+ if (ret != 0)
return;
ba2str(&src, addr);
- error("input: authorization for %s failed: %s (%d)",
- addr, strerror(-ret), -ret);
+ error("input: authorization for %s failed", addr);
g_io_channel_unref(server->confirm);
server->confirm = NULL;
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 43ce9d9..6ee4770 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -500,10 +500,10 @@ static void confirm_event(GIOChannel *chan, gpointer user_data)
{
struct network_adapter *na = user_data;
struct network_server *ns;
- int perr;
bdaddr_t src, dst;
char address[18];
GError *err = NULL;
+ guint ret;
bt_io_get(chan, &err,
BT_IO_OPT_SOURCE_BDADDR, &src,
@@ -537,11 +537,10 @@ static void confirm_event(GIOChannel *chan, gpointer user_data)
bacpy(&na->setup->dst, &dst);
na->setup->io = g_io_channel_ref(chan);
- perr = btd_request_authorization(&src, &dst, BNEP_SVC_UUID,
+ ret = btd_request_authorization(&src, &dst, BNEP_SVC_UUID,
auth_cb, na);
- if (perr < 0) {
- error("Refusing connect from %s: %s (%d)", address,
- strerror(-perr), -perr);
+ if (ret == 0) {
+ error("Refusing connect from %s", address);
setup_destroy(na);
goto drop;
}
diff --git a/profiles/sap/server.c b/profiles/sap/server.c
index 6c5aa21..6072432 100644
--- a/profiles/sap/server.c
+++ b/profiles/sap/server.c
@@ -1213,7 +1213,7 @@ static void connect_confirm_cb(GIOChannel *io, gpointer data)
GError *gerr = NULL;
bdaddr_t src, dst;
char dstaddr[18];
- int err;
+ guint ret;
DBG("conn %p io %p", conn, io);
@@ -1253,10 +1253,10 @@ static void connect_confirm_cb(GIOChannel *io, gpointer data)
ba2str(&dst, dstaddr);
- err = btd_request_authorization(&src, &dst, SAP_UUID, connect_auth_cb,
+ ret = btd_request_authorization(&src, &dst, SAP_UUID, connect_auth_cb,
server);
- if (err < 0) {
- error("Authorization failure (err %d)", err);
+ if (ret == 0) {
+ error("Authorization failure");
sap_server_remove_conn(server);
return;
}
diff --git a/src/adapter.c b/src/adapter.c
index 524885c..982fd8d 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -97,6 +97,7 @@ struct session_req {
};
struct service_auth {
+ guint id;
service_auth_cb cb;
void *user_data;
struct btd_device *device;
@@ -3209,27 +3210,29 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
char address[18];
const gchar *dev_path;
int err;
+ static guint id = 0;
ba2str(dst, address);
device = adapter_find_device(adapter, address);
if (!device)
- return -EPERM;
+ return 0;
/* Device connected? */
if (!g_slist_find(adapter->connections, device))
error("Authorization request for non-connected device!?");
if (adapter->auth != NULL)
- return -EBUSY;
+ return 0;
auth = g_try_new0(struct service_auth, 1);
if (!auth)
- return -ENOMEM;
+ return 0;
auth->cb = cb;
auth->user_data = user_data;
auth->device = device;
auth->adapter = adapter;
+ auth->id = ++id;
if (device_is_trusted(device) == TRUE) {
adapter->auth_idle_id = g_idle_add(auth_idle_cb, adapter);
@@ -3240,7 +3243,7 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
if (!agent) {
warn("Can't find device agent");
g_free(auth);
- return -EPERM;
+ return 0;
}
dev_path = device_get_path(device);
@@ -3249,15 +3252,15 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
NULL);
if (err < 0) {
g_free(auth);
- return err;
+ return 0;
}
done:
adapter->auth = auth;
- return 0;
+ return auth->id;
}
-int btd_request_authorization(const bdaddr_t *src, const bdaddr_t *dst,
+guint btd_request_authorization(const bdaddr_t *src, const bdaddr_t *dst,
const char *uuid, service_auth_cb cb,
void *user_data)
{
@@ -3267,55 +3270,65 @@ int btd_request_authorization(const bdaddr_t *src, const bdaddr_t *dst,
if (bacmp(src, BDADDR_ANY) != 0) {
adapter = manager_find_adapter(src);
if (!adapter)
- return -EPERM;
+ return 0;
return adapter_authorize(adapter, dst, uuid, cb, user_data);
}
for (l = manager_get_adapters(); l != NULL; l = g_slist_next(l)) {
- int err;
+ guint id;
adapter = l->data;
- err = adapter_authorize(adapter, dst, uuid, cb, user_data);
- if (err == 0)
- return 0;
+ id = adapter_authorize(adapter, dst, uuid, cb, user_data);
+ if (id != 0)
+ return id;
}
- return -EPERM;
+ return 0;
}
-int btd_cancel_authorization(const bdaddr_t *src, const bdaddr_t *dst)
+static struct btd_adapter *find_authorization(guint id)
{
- struct btd_adapter *adapter = manager_find_adapter(src);
- struct btd_device *device;
+ GSList *l;
+
+ for (l = manager_get_adapters(); l != NULL; l = g_slist_next(l)) {
+ struct btd_adapter *adapter = l->data;
+
+ if (adapter->auth == NULL)
+ continue;
+
+ if (adapter->auth->id == id)
+ return adapter;
+ }
+
+ return NULL;
+}
+
+int btd_cancel_authorization(guint id)
+{
+ struct btd_adapter *adapter;
struct agent *agent;
- char address[18];
int err;
- if (!adapter)
- return -EPERM;
-
- ba2str(dst, address);
- device = adapter_find_device(adapter, address);
- if (!device)
+ adapter = find_authorization(id);
+ if (adapter == NULL)
return -EPERM;
if (adapter->auth_idle_id) {
g_source_remove(adapter->auth_idle_id);
adapter->auth_idle_id = 0;
+ g_free(adapter->auth);
+ adapter->auth = NULL;
return 0;
}
- if (!adapter->auth || adapter->auth->device != device)
- return -EPERM;
-
/*
* FIXME: Cancel fails if authorization is requested to adapter's
* agent and in the meanwhile CreatePairedDevice is called.
*/
- agent = device_get_agent(device);
+ agent = device_get_agent(adapter->auth->device);
if (!agent)
return -EPERM;
diff --git a/src/adapter.h b/src/adapter.h
index f34763f..436f167 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -149,9 +149,9 @@ void adapter_add_profile(struct btd_adapter *adapter, gpointer p);
void adapter_remove_profile(struct btd_adapter *adapter, gpointer p);
int btd_register_adapter_driver(struct btd_adapter_driver *driver);
void btd_unregister_adapter_driver(struct btd_adapter_driver *driver);
-int btd_request_authorization(const bdaddr_t *src, const bdaddr_t *dst,
+guint btd_request_authorization(const bdaddr_t *src, const bdaddr_t *dst,
const char *uuid, service_auth_cb cb, void *user_data);
-int btd_cancel_authorization(const bdaddr_t *src, const bdaddr_t *dst);
+int btd_cancel_authorization(guint id);
const char *adapter_any_get_path(void);
diff --git a/src/profile.c b/src/profile.c
index 24f7b28..a051cf7 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -72,7 +72,7 @@ struct ext_io {
GIOChannel *io;
guint io_id;
- bool authorizing;
+ guint auth_id;
DBusPendingCall *new_conn;
};
@@ -151,14 +151,8 @@ static void ext_io_destroy(gpointer p)
g_io_channel_shutdown(ext_io->io, FALSE, NULL);
g_io_channel_unref(ext_io->io);
- if (ext_io->authorizing) {
- bdaddr_t src, dst;
-
- if (bt_io_get(ext_io->io, NULL, BT_IO_OPT_SOURCE_BDADDR, &src,
- BT_IO_OPT_DEST_BDADDR, &dst,
- BT_IO_OPT_INVALID))
- btd_cancel_authorization(&src, &dst);
- }
+ if (ext_io->auth_id != 0)
+ btd_cancel_authorization(ext_io->auth_id);
if (ext_io->new_conn) {
dbus_pending_call_cancel(ext_io->new_conn);
@@ -318,7 +312,7 @@ static void ext_auth(DBusError *err, void *user_data)
GError *gerr = NULL;
char addr[18];
- conn->authorizing = false;
+ conn->auth_id = 0;
bt_io_get(conn->io, &gerr, BT_IO_OPT_DEST, addr, BT_IO_OPT_INVALID);
if (gerr != NULL) {
@@ -372,7 +366,6 @@ static void ext_confirm(GIOChannel *io, gpointer user_data)
GError *gerr = NULL;
bdaddr_t src, dst;
char addr[18];
- int err;
bt_io_get(io, &gerr,
BT_IO_OPT_SOURCE_BDADDR, &src,
@@ -390,16 +383,14 @@ static void ext_confirm(GIOChannel *io, gpointer user_data)
conn = create_conn(server, io);
- err = btd_request_authorization(&src, &dst, ext->uuid, ext_auth, conn);
- if (err < 0) {
- error("%s authorization failure: %s", ext->name,
- strerror(-err));
+ conn->auth_id = btd_request_authorization(&src, &dst, ext->uuid,
+ ext_auth, conn);
+ if (conn->auth_id == 0) {
+ error("%s authorization failure", ext->name);
ext_io_destroy(conn);
return;
}
- conn->authorizing = true;
-
ext->conns = g_slist_append(ext->conns, conn);
DBG("%s authorizing connection from %s", ext->name, addr);
--
1.7.11.4
^ permalink raw reply related
* [PATCH v4 4/5] adapter: Queue parallel authorization requests
From: Mikel Astiz @ 2012-09-28 16:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1348849945-5762-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Remote device could try to connect several profiles in parallel, or
several devices could be trying to connect services. Instead of
returning EBUSY, this patch adds a queue to each adapter such that the
authorization requests will be queued and processed sequentially.
---
src/adapter.c | 181 +++++++++++++++++++++++++++++++++-------------------------
1 file changed, 103 insertions(+), 78 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 982fd8d..f11be70 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -100,8 +100,10 @@ struct service_auth {
guint id;
service_auth_cb cb;
void *user_data;
+ const char *uuid;
struct btd_device *device;
struct btd_adapter *adapter;
+ struct agent *agent; /* NULL for queued auths */
};
struct btd_adapter {
@@ -125,8 +127,8 @@ struct btd_adapter {
GSList *found_devices;
GSList *oor_devices; /* out of range device list */
struct agent *agent; /* For the new API */
- guint auth_idle_id; /* Ongoing authorization (trusted) */
- struct service_auth *auth; /* Ongoing authorization */
+ guint auth_idle_id; /* Pending authorization dequeue */
+ GQueue *auths; /* Ongoing and pending auths */
GSList *connections; /* Connected devices */
GSList *devices; /* Devices structure pointers */
GSList *mode_sessions; /* Request Mode sessions */
@@ -154,6 +156,8 @@ struct btd_adapter {
GSList *profiles;
};
+static gboolean process_auth_queue(gpointer user_data);
+
static void dev_info_free(void *data)
{
struct remote_dev_info *dev = data;
@@ -947,16 +951,49 @@ static struct btd_device *adapter_create_device(struct btd_adapter *adapter,
return device;
}
+static void service_auth_cancel(struct service_auth *auth)
+{
+ DBusError derr;
+
+ dbus_error_init(&derr);
+ dbus_set_error_const(&derr, "org.bluez.Error.Canceled", NULL);
+
+ auth->cb(&derr, auth->user_data);
+
+ dbus_error_free(&derr);
+
+ if (auth->agent != NULL)
+ agent_cancel(auth->agent);
+
+ g_free(auth);
+}
+
void adapter_remove_device(struct btd_adapter *adapter,
struct btd_device *device,
gboolean remove_storage)
{
const gchar *dev_path = device_get_path(device);
- struct agent *agent;
+ GList *l;
adapter->devices = g_slist_remove(adapter->devices, device);
adapter->connections = g_slist_remove(adapter->connections, device);
+ l = adapter->auths->head;
+ while (l != NULL) {
+ struct service_auth *auth = l->data;
+ GList *next = g_list_next(l);
+
+ if (auth->device != device) {
+ l = next;
+ continue;
+ }
+
+ g_queue_delete_link(adapter->auths, l);
+ l = next;
+
+ service_auth_cancel(auth);
+ }
+
adapter_update_devices(adapter);
g_dbus_emit_signal(btd_get_dbus_connection(), adapter->path,
@@ -964,14 +1001,6 @@ void adapter_remove_device(struct btd_adapter *adapter,
DBUS_TYPE_OBJECT_PATH, &dev_path,
DBUS_TYPE_INVALID);
- agent = device_get_agent(device);
-
- if (agent && adapter->auth && adapter->auth->device == device) {
- g_free(adapter->auth);
- adapter->auth = NULL;
- agent_cancel(agent);
- }
-
device_remove(device, remove_storage);
}
@@ -2438,8 +2467,7 @@ static void adapter_free(gpointer user_data)
if (adapter->auth_idle_id)
g_source_remove(adapter->auth_idle_id);
- if (adapter->auth)
- g_free(adapter->auth);
+ g_queue_free_full(adapter->auths, g_free);
if (adapter->off_timer)
off_timer_remove(adapter);
@@ -2533,6 +2561,7 @@ struct btd_adapter *adapter_create(int id)
}
adapter->dev_id = id;
+ adapter->auths = g_queue_new();
snprintf(path, sizeof(path), "%s/hci%d", base_path, id);
adapter->path = g_strdup(path);
@@ -3176,26 +3205,60 @@ static void agent_auth_cb(struct agent *agent, DBusError *derr,
void *user_data)
{
struct btd_adapter *adapter = user_data;
- struct service_auth *auth = adapter->auth;
+ struct service_auth *auth = adapter->auths->head->data;
- adapter->auth = NULL;
+ g_queue_pop_head(adapter->auths);
auth->cb(derr, auth->user_data);
g_free(auth);
+
+ adapter->auth_idle_id = g_idle_add(process_auth_queue, adapter);
}
-static gboolean auth_idle_cb(gpointer user_data)
+static gboolean process_auth_queue(gpointer user_data)
{
struct btd_adapter *adapter = user_data;
- struct service_auth *auth = adapter->auth;
+ DBusError err;
- adapter->auth = NULL;
adapter->auth_idle_id = 0;
- auth->cb(NULL, auth->user_data);
+ dbus_error_init(&err);
+ dbus_set_error_const(&err, "org.bluez.Error.Rejected", NULL);
- g_free(auth);
+ while (!g_queue_is_empty(adapter->auths)) {
+ struct service_auth *auth = adapter->auths->head->data;
+ struct btd_device *device = auth->device;
+ const gchar *dev_path;
+
+ if (device_is_trusted(device) == TRUE) {
+ auth->cb(NULL, auth->user_data);
+ goto next;
+ }
+
+ auth->agent = device_get_agent(device);
+ if (auth->agent == NULL) {
+ warn("Can't find device agent");
+ auth->cb(&err, auth->user_data);
+ goto next;
+ }
+
+ dev_path = device_get_path(device);
+
+ if (agent_authorize(auth->agent, dev_path, auth->uuid,
+ agent_auth_cb, adapter, NULL) < 0) {
+ auth->cb(&err, auth->user_data);
+ goto next;
+ }
+
+ break;
+
+next:
+ g_free(auth);
+ g_queue_pop_head(adapter->auths);
+ }
+
+ dbus_error_free(&err);
return FALSE;
}
@@ -3206,10 +3269,7 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
{
struct service_auth *auth;
struct btd_device *device;
- struct agent *agent;
char address[18];
- const gchar *dev_path;
- int err;
static guint id = 0;
ba2str(dst, address);
@@ -3221,42 +3281,27 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
if (!g_slist_find(adapter->connections, device))
error("Authorization request for non-connected device!?");
- if (adapter->auth != NULL)
- return 0;
-
auth = g_try_new0(struct service_auth, 1);
if (!auth)
return 0;
auth->cb = cb;
auth->user_data = user_data;
+ auth->uuid = uuid;
auth->device = device;
auth->adapter = adapter;
auth->id = ++id;
- if (device_is_trusted(device) == TRUE) {
- adapter->auth_idle_id = g_idle_add(auth_idle_cb, adapter);
- goto done;
- }
+ g_queue_push_tail(adapter->auths, auth);
- agent = device_get_agent(device);
- if (!agent) {
- warn("Can't find device agent");
- g_free(auth);
- return 0;
- }
+ if (adapter->auths->length != 1)
+ return auth->id;
- dev_path = device_get_path(device);
+ if (adapter->auth_idle_id != 0)
+ return auth->id;
- err = agent_authorize(agent, dev_path, uuid, agent_auth_cb, adapter,
- NULL);
- if (err < 0) {
- g_free(auth);
- return 0;
- }
+ adapter->auth_idle_id = g_idle_add(process_auth_queue, adapter);
-done:
- adapter->auth = auth;
return auth->id;
}
@@ -3288,18 +3333,20 @@ guint btd_request_authorization(const bdaddr_t *src, const bdaddr_t *dst,
return 0;
}
-static struct btd_adapter *find_authorization(guint id)
+static struct service_auth *find_authorization(guint id)
{
GSList *l;
+ GList *l2;
for (l = manager_get_adapters(); l != NULL; l = g_slist_next(l)) {
struct btd_adapter *adapter = l->data;
- if (adapter->auth == NULL)
- continue;
+ for (l2 = adapter->auths->head; l2 != NULL; l2 = l2->next) {
+ struct service_auth *auth = l2->data;
- if (adapter->auth->id == id)
- return adapter;
+ if (auth->id == id)
+ return auth;
+ }
}
return NULL;
@@ -3307,39 +3354,17 @@ static struct btd_adapter *find_authorization(guint id)
int btd_cancel_authorization(guint id)
{
- struct btd_adapter *adapter;
- struct agent *agent;
- int err;
-
- adapter = find_authorization(id);
- if (adapter == NULL)
- return -EPERM;
-
- if (adapter->auth_idle_id) {
- g_source_remove(adapter->auth_idle_id);
- adapter->auth_idle_id = 0;
- g_free(adapter->auth);
- adapter->auth = NULL;
- return 0;
- }
-
- /*
- * FIXME: Cancel fails if authorization is requested to adapter's
- * agent and in the meanwhile CreatePairedDevice is called.
- */
+ struct service_auth *auth;
- agent = device_get_agent(adapter->auth->device);
- if (!agent)
+ auth = find_authorization(id);
+ if (auth == NULL)
return -EPERM;
- err = agent_cancel(agent);
+ g_queue_remove(auth->adapter->auths, auth);
- if (err == 0) {
- g_free(adapter->auth);
- adapter->auth = NULL;
- }
+ service_auth_cancel(auth);
- return err;
+ return 0;
}
static gchar *adapter_any_path = NULL;
--
1.7.11.4
^ permalink raw reply related
* [PATCH v4 5/5] audio: Drop audio-specific authorization mechanism
From: Mikel Astiz @ 2012-09-28 16:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1348849945-5762-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Remove the audio-specific service authorization mechanism in favor of
using the conventional one.
The main difference is that audio profiles will be authorized
independently. Therefore a single connection might result in several
profile authorization requests to the agent (i.e. HFP, A2DP and AVRCP).
This removes any internal policy that would skip the authorization
procedure, making it simpler and more convenient for IVI use-cases.
Agents interested in simulating the old behavior are encouraged to
either set the device as trusted or just reply to the additional
authorization requests automatically without user intervention.
---
audio/avctp.c | 27 ++++------
audio/avdtp.c | 23 ++++++---
audio/device.c | 149 --------------------------------------------------------
audio/device.h | 12 +----
audio/manager.c | 28 ++++++-----
5 files changed, 43 insertions(+), 196 deletions(-)
diff --git a/audio/avctp.c b/audio/avctp.c
index 6fd5491..b7b3083 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -143,6 +143,7 @@ struct avctp {
GIOChannel *browsing_io;
guint control_io_id;
guint browsing_io_id;
+ guint auth_id;
uint16_t control_mtu;
uint16_t browsing_mtu;
@@ -360,13 +361,9 @@ static void avctp_disconnected(struct avctp *session)
g_source_remove(session->control_io_id);
session->control_io_id = 0;
- if (session->state == AVCTP_STATE_CONNECTING) {
- struct audio_device *dev;
-
- dev = manager_get_device(&session->server->src,
- &session->dst, FALSE);
- audio_device_cancel_authorization(dev, auth_cb,
- session);
+ if (session->auth_id != 0) {
+ btd_cancel_authorization(session->auth_id);
+ session->auth_id = 0;
}
}
@@ -409,15 +406,7 @@ static void avctp_set_state(struct avctp *session, avctp_state_t new_state)
switch (new_state) {
case AVCTP_STATE_DISCONNECTED:
DBG("AVCTP Disconnected");
-
avctp_disconnected(session);
-
- if (old_state != AVCTP_STATE_CONNECTED)
- break;
-
- if (!audio_device_is_active(dev, NULL))
- audio_device_set_authorized(dev, FALSE);
-
break;
case AVCTP_STATE_CONNECTING:
DBG("AVCTP Connecting");
@@ -769,6 +758,8 @@ static void auth_cb(DBusError *derr, void *user_data)
struct avctp *session = user_data;
GError *err = NULL;
+ session->auth_id = 0;
+
if (session->control_io_id) {
g_source_remove(session->control_io_id);
session->control_io_id = 0;
@@ -854,8 +845,10 @@ static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
avctp_set_state(session, AVCTP_STATE_CONNECTING);
session->control_io = g_io_channel_ref(chan);
- if (audio_device_request_authorization(dev, AVRCP_TARGET_UUID,
- auth_cb, session) < 0)
+ session->auth_id = btd_request_authorization(&dev->src, &dev->dst,
+ AVRCP_TARGET_UUID,
+ auth_cb, session);
+ if (session->auth_id == 0)
goto drop;
session->control_io_id = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP |
diff --git a/audio/avdtp.c b/audio/avdtp.c
index d44c504..8836ca1 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -400,6 +400,8 @@ struct avdtp {
/* True if the entire device is being disconnected */
gboolean device_disconnect;
+ guint auth_id;
+
GIOChannel *io;
guint io_id;
@@ -1129,16 +1131,21 @@ static void release_stream(struct avdtp_stream *stream, struct avdtp *session)
static int avdtp_cancel_authorization(struct avdtp *session)
{
- struct audio_device *dev;
+ int err;
if (session->state != AVDTP_SESSION_STATE_CONNECTING)
return 0;
- dev = manager_get_device(&session->server->src, &session->dst, FALSE);
- if (dev == NULL)
- return -ENODEV;
+ if (session->auth_id == 0)
+ return 0;
- return audio_device_cancel_authorization(dev, auth_cb, session);
+ err = btd_cancel_authorization(session->auth_id);
+ if (err < 0)
+ return err;
+
+ session->auth_id = 0;
+
+ return 0;
}
static void connection_lost(struct avdtp *session, int err)
@@ -2507,7 +2514,6 @@ static void avdtp_confirm_cb(GIOChannel *chan, gpointer data)
struct audio_device *dev;
char address[18];
bdaddr_t src, dst;
- int perr;
GError *err = NULL;
bt_io_get(chan, &err,
@@ -2566,9 +2572,10 @@ static void avdtp_confirm_cb(GIOChannel *chan, gpointer data)
session->io_id = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
(GIOFunc) session_cb, session);
- perr = audio_device_request_authorization(dev, ADVANCED_AUDIO_UUID,
+ session->auth_id = btd_request_authorization(&dev->src, &dev->dst,
+ ADVANCED_AUDIO_UUID,
auth_cb, session);
- if (perr < 0) {
+ if (session->auth_id == 0) {
avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
avdtp_unref(session);
goto drop;
diff --git a/audio/device.c b/audio/device.c
index c6ae96a..175e3c0 100644
--- a/audio/device.c
+++ b/audio/device.c
@@ -71,19 +71,12 @@ typedef enum {
AUDIO_STATE_CONNECTED,
} audio_state_t;
-struct service_auth {
- service_auth_cb cb;
- void *user_data;
-};
-
struct dev_priv {
audio_state_t state;
headset_state_t hs_state;
sink_state_t sink_state;
avctp_state_t avctp_state;
- GSList *auths;
- guint auth_id;
DBusMessage *conn_req;
DBusMessage *dc_req;
@@ -94,8 +87,6 @@ struct dev_priv {
guint dc_id;
gboolean disconnecting;
- gboolean authorized;
- guint auth_idle_id;
};
static unsigned int sink_callback_id = 0;
@@ -110,8 +101,6 @@ static void device_free(struct audio_device *dev)
btd_device_unref(dev->btd_dev);
if (priv) {
- if (priv->auths)
- audio_device_cancel_authorization(dev, NULL, NULL);
if (priv->control_timer)
g_source_remove(priv->control_timer);
if (priv->avdtp_timer)
@@ -238,8 +227,6 @@ static void device_set_state(struct audio_device *dev, audio_state_t new_state)
return;
if (new_state == AUDIO_STATE_DISCONNECTED) {
- priv->authorized = FALSE;
-
if (priv->dc_id) {
device_remove_disconnect_watch(dev->btd_dev,
priv->dc_id);
@@ -732,139 +719,3 @@ void audio_device_unregister(struct audio_device *device)
device_free(device);
}
-
-static void auth_cb(DBusError *derr, void *user_data)
-{
- struct audio_device *dev = user_data;
- struct dev_priv *priv = dev->priv;
-
- priv->auth_id = 0;
-
- if (derr == NULL)
- priv->authorized = TRUE;
-
- while (priv->auths) {
- struct service_auth *auth = priv->auths->data;
-
- auth->cb(derr, auth->user_data);
- priv->auths = g_slist_remove(priv->auths, auth);
- g_free(auth);
- }
-}
-
-static gboolean auth_idle_cb(gpointer user_data)
-{
- struct audio_device *dev = user_data;
- struct dev_priv *priv = dev->priv;
-
- priv->auth_idle_id = 0;
-
- auth_cb(NULL, dev);
-
- return FALSE;
-}
-
-static gboolean audio_device_is_connected(struct audio_device *dev)
-{
- if (dev->headset) {
- headset_state_t state = headset_get_state(dev);
-
- if (state == HEADSET_STATE_CONNECTED ||
- state == HEADSET_STATE_PLAY_IN_PROGRESS ||
- state == HEADSET_STATE_PLAYING)
- return TRUE;
- }
-
- if (dev->sink) {
- sink_state_t state = sink_get_state(dev);
-
- if (state == SINK_STATE_CONNECTED ||
- state == SINK_STATE_PLAYING)
- return TRUE;
- }
-
- if (dev->source) {
- source_state_t state = source_get_state(dev);
-
- if (state == SOURCE_STATE_CONNECTED ||
- state == SOURCE_STATE_PLAYING)
- return TRUE;
- }
-
- return FALSE;
-}
-
-int audio_device_request_authorization(struct audio_device *dev,
- const char *uuid, service_auth_cb cb,
- void *user_data)
-{
- struct dev_priv *priv = dev->priv;
- struct service_auth *auth;
-
- auth = g_try_new0(struct service_auth, 1);
- if (!auth)
- return -ENOMEM;
-
- auth->cb = cb;
- auth->user_data = user_data;
-
- priv->auths = g_slist_append(priv->auths, auth);
- if (g_slist_length(priv->auths) > 1)
- return 0;
-
- if (priv->authorized || audio_device_is_connected(dev)) {
- priv->auth_idle_id = g_idle_add(auth_idle_cb, dev);
- return 0;
- }
-
- priv->auth_id = btd_request_authorization(&dev->src, &dev->dst, uuid,
- auth_cb, dev);
- if (priv->auth_id != 0)
- return 0;
-
- priv->auths = g_slist_remove(priv->auths, auth);
- g_free(auth);
-
- return -EPERM;
-}
-
-int audio_device_cancel_authorization(struct audio_device *dev,
- authorization_cb cb, void *user_data)
-{
- struct dev_priv *priv = dev->priv;
- GSList *l, *next;
-
- for (l = priv->auths; l != NULL; l = next) {
- struct service_auth *auth = l->data;
-
- next = g_slist_next(l);
-
- if (cb && auth->cb != cb)
- continue;
-
- if (user_data && auth->user_data != user_data)
- continue;
-
- priv->auths = g_slist_remove(priv->auths, auth);
- g_free(auth);
- }
-
- if (g_slist_length(priv->auths) == 0) {
- if (priv->auth_idle_id > 0) {
- g_source_remove(priv->auth_idle_id);
- priv->auth_idle_id = 0;
- } else {
- btd_cancel_authorization(priv->auth_id);
- priv->auth_id = 0;
- }
- }
-
- return 0;
-}
-
-void audio_device_set_authorized(struct audio_device *dev, gboolean auth)
-{
- struct dev_priv *priv = dev->priv;
-
- priv->authorized = auth;
-}
diff --git a/audio/device.h b/audio/device.h
index f9b868f..2b92528 100644
--- a/audio/device.h
+++ b/audio/device.h
@@ -47,6 +47,7 @@ struct audio_device {
struct target *target;
guint hs_preauth_id;
+ guint hs_auth_id;
struct dev_priv *priv;
};
@@ -59,14 +60,3 @@ void audio_device_unregister(struct audio_device *device);
gboolean audio_device_is_active(struct audio_device *dev,
const char *interface);
-
-typedef void (*authorization_cb) (DBusError *derr, void *user_data);
-
-int audio_device_cancel_authorization(struct audio_device *dev,
- authorization_cb cb, void *user_data);
-
-int audio_device_request_authorization(struct audio_device *dev,
- const char *uuid, authorization_cb cb,
- void *user_data);
-
-void audio_device_set_authorized(struct audio_device *dev, gboolean auth);
diff --git a/audio/manager.c b/audio/manager.c
index 70be01b..dbc98d5 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -300,6 +300,8 @@ static void headset_auth_cb(DBusError *derr, void *user_data)
GError *err = NULL;
GIOChannel *io;
+ device->hs_auth_id = 0;
+
if (device->hs_preauth_id) {
g_source_remove(device->hs_preauth_id);
device->hs_preauth_id = 0;
@@ -328,7 +330,8 @@ static gboolean hs_preauth_cb(GIOChannel *chan, GIOCondition cond,
DBG("Headset disconnected during authorization");
- audio_device_cancel_authorization(device, headset_auth_cb, device);
+ btd_cancel_authorization(device->hs_auth_id);
+ device->hs_auth_id = 0;
headset_set_state(device, HEADSET_STATE_DISCONNECTED);
@@ -343,7 +346,6 @@ static void ag_confirm(GIOChannel *chan, gpointer data)
struct audio_device *device;
gboolean hfp_active;
bdaddr_t src, dst;
- int perr;
GError *err = NULL;
uint8_t ch;
@@ -398,10 +400,13 @@ static void ag_confirm(GIOChannel *chan, gpointer data)
headset_set_state(device, HEADSET_STATE_CONNECTING);
- perr = audio_device_request_authorization(device, server_uuid,
- headset_auth_cb, device);
- if (perr < 0) {
- DBG("Authorization denied: %s", strerror(-perr));
+ device->hs_auth_id = btd_request_authorization(&device->src,
+ &device->dst,
+ server_uuid,
+ headset_auth_cb,
+ device);
+ if (device->hs_auth_id == 0) {
+ DBG("Authorization denied");
headset_set_state(device, HEADSET_STATE_DISCONNECTED);
return;
}
@@ -443,7 +448,7 @@ static void hf_io_cb(GIOChannel *chan, gpointer data)
uint8_t ch;
const char *server_uuid, *remote_uuid;
struct audio_device *device;
- int perr;
+ guint auth_id;
bt_io_get(chan, &err,
BT_IO_OPT_SOURCE_BDADDR, &src,
@@ -480,10 +485,11 @@ static void hf_io_cb(GIOChannel *chan, gpointer data)
goto drop;
}
- perr = audio_device_request_authorization(device, server_uuid,
- gateway_auth_cb, device);
- if (perr < 0) {
- DBG("Authorization denied: %s", strerror(-perr));
+ auth_id = btd_request_authorization(&device->src, &device->dst,
+ server_uuid, gateway_auth_cb,
+ device);
+ if (auth_id == 0) {
+ DBG("Authorization denied");
gateway_set_state(device, GATEWAY_STATE_DISCONNECTED);
}
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ] build: Fix out-of-tree builds
From: Anderson Lizardo @ 2012-09-28 19:43 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
This commit makes a few changes that mostly affect "out-of-tree" builds:
* Fix symlink creation for profiles/sap/sap.c (should use build
directory, not source tree).
* Remove useless -I$(builddir)/{tools,health} directives (these
build directories do not contain any headers).
* Add -I$(srcdir)/profiles/sap (necessary because sap.c is a symlink
into the build directory, and refers to headers in the source tree).
Tested with:
./bootstrap
mkdir build
cd build
../configure [...]
---
Makefile.am | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index c27eb01..257c18a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -405,10 +405,10 @@ AM_CFLAGS += @DBUS_CFLAGS@ @GLIB_CFLAGS@
INCLUDES = -I$(builddir)/lib -I$(builddir)/src -I$(srcdir)/src \
-I$(srcdir)/audio -I$(srcdir)/sbc -I$(srcdir)/gdbus \
-I$(srcdir)/attrib -I$(srcdir)/btio -I$(srcdir)/tools \
- -I$(builddir)/tools -I$(srcdir)/monitor
+ -I$(srcdir)/monitor
-if MCAP
-INCLUDES += -I$(builddir)/health
+if SAPPLUGIN
+INCLUDES += -I$(srcdir)/profiles/sap
endif
unit_objects =
@@ -447,7 +447,7 @@ audio/telephony.c: audio/@TELEPHONY_DRIVER@
$(AM_V_GEN)$(LN_S) $(abs_top_builddir)/$< $@
profiles/sap/sap.c: profiles/sap/@SAP_DRIVER@
- $(AM_V_GEN)$(LN_S) $(abs_top_srcdir)/$< $@
+ $(AM_V_GEN)$(LN_S) $(abs_top_builddir)/$< $@
profiles/input/suspend.c: profiles/input/@HOG_SUSPEND_DRIVER@
$(AM_V_GEN)$(LN_S) $(abs_top_srcdir)/$< $@
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 3/3] Bluetooth: btmrvl: Fix skb buffer overflow
From: Gustavo Padovan @ 2012-09-28 19:52 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth, bzhao
In-Reply-To: <1348832170-13250-3-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-09-28 14:36:10 +0300]:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Add extra check to avoid skb buffer overflow. Fixes crash below:
>
> [ 101.030427] ------------[ cut here ]------------
> [ 101.030459] kernel BUG at net/core/skbuff.c:127!
> [ 101.030486] invalid opcode: 0000 [#1] SMP
> ...
> [ 101.030806] Pid: 2010, comm: btmrvl_main_ser Not tainted 3.5.0+ #80 Laptop
> [ 101.030859] EIP: 0060:[<c14f2ba9>] EFLAGS: 00010282 CPU: 0
> [ 101.030894] EIP is at skb_put+0x99/0xa0
> [ 101.030919] EAX: 00000080 EBX: f129380b ECX: ef923540 EDX: 00000001
> [ 101.030956] ESI: f00a4000 EDI: 00001003 EBP: ed4a5efc ESP: ed4a5ecc
> [ 101.030992] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> [ 101.031024] CR0: 8005003b CR2: 08fca014 CR3: 30960000 CR4: 000407f0
> [ 101.031062] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> [ 101.031100] DR6: ffff0ff0 DR7: 00000400
> [ 101.031125] Process btmrvl_main_ser (pid: 2010, ti=ed4a4000 task=ef923540 task.ti=ed4a4000)
> [ 101.031174] Stack:
> [ 101.031188] c18126f8 c1651938 f853f8d2 00001003 00001003 f1292800 f1292808 f129380b
> [ 101.031250] f1292940 f00a4000 eddb1280 efc0f9c0 ed4a5f44 f853f8d2 00000040 00000000
> [ 101.031312] ef923540 c15ee096 ef923540 eddb12d4 00000004 f00a4000 00000040 00000000
> [ 101.031376] Call Trace:
> [ 101.031396] [<f853f8d2>] ? btmrvl_sdio_process_int_status+0x272/0x3d0 [btmrvl_sdio]
> [ 101.031444] [<f853f8d2>] btmrvl_sdio_process_int_status+0x272/0x3d0 [btmrvl_sdio]
> [ 101.031488] [<c15ee096>] ? _raw_spin_unlock_irqrestore+0x36/0x70
> [ 101.031526] [<f85a46e4>] btmrvl_service_main_thread+0x244/0x300 [btmrvl]
> [ 101.031568] [<f853fb50>] ? btmrvl_sdio_poll_card_status.isra.6.constprop.7+0x90/0x90 [btmrvl_sdio]
> [ 101.031619] [<c107eda0>] ? try_to_wake_up+0x270/0x270
> [ 101.031648] [<f85a44a0>] ? btmrvl_process_event+0x3b0/0x3b0 [btmrvl]
> [ 101.031686] [<c106d19d>] kthread+0x7d/0x90
> [ 101.031713] [<c106d120>] ? flush_kthread_work+0x150/0x150
> [ 101.031745] [<c15f5a82>] kernel_thread_helper+0x6/0x10
> ...
> [ 101.032008] EIP: [<c14f2ba9>] skb_put+0x99/0xa0 SS:ESP 0068:ed4a5ecc
> [ 101.056125] ---[ end trace a0bd01d1a9a796c8 ]---
>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
> drivers/bluetooth/btmrvl_sdio.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
All 3 patches were applied. Thanks.
Gustavo
^ 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