* RE: [PATCH 1/4] tools/csr_usb : Fix Resource leak: file
From: Gowtham Anandha Babu @ 2014-10-16 8:57 UTC (permalink / raw)
To: 'Luiz Augusto von Dentz'
Cc: linux-bluetooth, 'Dmitry Kasatkin',
'Bharat Panda', cpgs
In-Reply-To: <CABBYNZKe1=awjhj2XttF+7gyHaw8f5zWftL=p1wrf+HWMUefMQ@mail.gmail.com>
Hi Luiz,
> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
> Sent: Thursday, October 16, 2014 1:48 PM
> To: Gowtham Anandha Babu
> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat Panda;
> cpgs@samsung.com
> Subject: Re: [PATCH 1/4] tools/csr_usb : Fix Resource leak: file
>
> Hi,
>
> On Wed, Oct 15, 2014 at 5:08 PM, Gowtham Anandha Babu
> <gowtham.ab@samsung.com> wrote:
> > Handles resource leak.
> > ---
> > tools/csr_usb.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/csr_usb.c b/tools/csr_usb.c index 5fb6bdc..a1d7324
> > 100644
> > --- a/tools/csr_usb.c
> > +++ b/tools/csr_usb.c
> > @@ -80,9 +80,12 @@ static int read_value(const char *name, const char
> *attr, const char *format)
> > return -1;
> >
> > n = fscanf(file, format, &value);
> > - if (n != 1)
> > + if (n != 1) {
> > + fclose(file);
> > return -1;
> > + }
> >
> > + fclose(file);
> > return value;
> > }
> >
> > --
> > 1.9.1
>
> Applied 1-3, but note that I did some changes in the commit message, please
> be consistent and don't include things like space before ':'
> etc, it is always a good practice got look how previous commit have been
> formatted and follow the HACKING document.
>
>
> --
> Luiz Augusto von Dentz
Sorry for the formatting errors and 50/72 format. I will follow the HACKING documents here after.
Thanks !
Regards,
Gowtham Anandha Babu
^ permalink raw reply
* Re: [PATCH 1/4] tools/csr_usb : Fix Resource leak: file
From: Luiz Augusto von Dentz @ 2014-10-16 8:17 UTC (permalink / raw)
To: Gowtham Anandha Babu
Cc: linux-bluetooth@vger.kernel.org, Dmitry Kasatkin, Bharat Panda,
cpgs
In-Reply-To: <1413382102-4049-1-git-send-email-gowtham.ab@samsung.com>
Hi,
On Wed, Oct 15, 2014 at 5:08 PM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> Handles resource leak.
> ---
> tools/csr_usb.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/csr_usb.c b/tools/csr_usb.c
> index 5fb6bdc..a1d7324 100644
> --- a/tools/csr_usb.c
> +++ b/tools/csr_usb.c
> @@ -80,9 +80,12 @@ static int read_value(const char *name, const char *attr, const char *format)
> return -1;
>
> n = fscanf(file, format, &value);
> - if (n != 1)
> + if (n != 1) {
> + fclose(file);
> return -1;
> + }
>
> + fclose(file);
> return value;
> }
>
> --
> 1.9.1
Applied 1-3, but note that I did some changes in the commit message,
please be consistent and don't include things like space before ':'
etc, it is always a good practice got look how previous commit have
been formatted and follow the HACKING document.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH 4/4] Fix enum coding-style
From: Luiz Augusto von Dentz @ 2014-10-16 7:59 UTC (permalink / raw)
To: Gowtham Anandha Babu
Cc: linux-bluetooth@vger.kernel.org, Dmitry Kasatkin, Bharat Panda,
cpgs
In-Reply-To: <1413382102-4049-4-git-send-email-gowtham.ab@samsung.com>
Hi,
On Wed, Oct 15, 2014 at 5:08 PM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> Fix the enum coding-style as per the new coding style doc.
I guess the tabs confused you a little bit, the coding style is more
about having the values in case they come from a specification, the
alignment of the values don't need to be exactly as in the example in
the coding style.
> ---
> lib/uuid.h | 8 ++++----
> profiles/cups/cups.h | 16 ++++++++--------
> profiles/health/hdp_types.h | 4 ++--
> profiles/proximity/reporter.h | 6 +++---
> profiles/time/server.c | 20 ++++++++++----------
> 5 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/lib/uuid.h b/lib/uuid.h
> index 8303772..f05888a 100644
> --- a/lib/uuid.h
> +++ b/lib/uuid.h
> @@ -142,10 +142,10 @@ extern "C" {
>
> typedef struct {
> enum {
> - BT_UUID_UNSPEC = 0,
> - BT_UUID16 = 16,
> - BT_UUID32 = 32,
> - BT_UUID128 = 128,
> + BT_UUID_UNSPEC = 0,
> + BT_UUID16 = 16,
> + BT_UUID32 = 32,
> + BT_UUID128 = 128,
> } type;
> union {
> uint16_t u16;
> diff --git a/profiles/cups/cups.h b/profiles/cups/cups.h
> index f4e0c01..dbea1bf 100644
> --- a/profiles/cups/cups.h
> +++ b/profiles/cups/cups.h
> @@ -21,14 +21,14 @@
> *
> */
>
> -enum { /**** Backend exit codes ****/
> - CUPS_BACKEND_OK = 0, /* Job completed successfully */
> - CUPS_BACKEND_FAILED = 1, /* Job failed, use error-policy */
> - CUPS_BACKEND_AUTH_REQUIRED = 2, /* Job failed, authentication required */
> - CUPS_BACKEND_HOLD = 3, /* Job failed, hold job */
> - CUPS_BACKEND_STOP = 4, /* Job failed, stop queue */
> - CUPS_BACKEND_CANCEL = 5, /* Job failed, cancel job */
> - CUPS_BACKEND_RETRY = 6, /* Failure requires us to retry (BlueZ specific) */
> +enum { /**** Backend exit codes ****/
> + CUPS_BACKEND_OK = 0, /* Job completed successfully */
> + CUPS_BACKEND_FAILED = 1, /* Job failed, use error-policy */
> + CUPS_BACKEND_AUTH_REQUIRED = 2, /* Job failed, authentication required */
> + CUPS_BACKEND_HOLD = 3, /* Job failed, hold job */
> + CUPS_BACKEND_STOP = 4, /* Job failed, stop queue */
> + CUPS_BACKEND_CANCEL = 5, /* Job failed, cancel job */
> + CUPS_BACKEND_RETRY = 6, /* Failure requires us to retry (BlueZ specific) */
> };
>
> int sdp_search_spp(sdp_session_t *sdp, uint8_t *channel);
> diff --git a/profiles/health/hdp_types.h b/profiles/health/hdp_types.h
> index b34b5e0..92f461a 100644
> --- a/profiles/health/hdp_types.h
> +++ b/profiles/health/hdp_types.h
> @@ -49,8 +49,8 @@
> #define HDP_SOURCE_ROLE_AS_STRING "source"
>
> typedef enum {
> - HDP_SOURCE = 0x00,
> - HDP_SINK = 0x01
> + HDP_SOURCE = 0x00,
> + HDP_SINK = 0x01
> } HdpRole;
>
> typedef enum {
> diff --git a/profiles/proximity/reporter.h b/profiles/proximity/reporter.h
> index a8e1aac..243db26 100644
> --- a/profiles/proximity/reporter.h
> +++ b/profiles/proximity/reporter.h
> @@ -31,9 +31,9 @@
> #define POWER_LEVEL_CHR_UUID 0x2A07
>
> enum {
> - NO_ALERT = 0x00,
> - MILD_ALERT = 0x01,
> - HIGH_ALERT = 0x02,
> + NO_ALERT = 0x00,
> + MILD_ALERT = 0x01,
> + HIGH_ALERT = 0x02,
> };
>
> void reporter_device_remove(struct btd_service *service);
> diff --git a/profiles/time/server.c b/profiles/time/server.c
> index 1716a5e..0b6f70b 100644
> --- a/profiles/time/server.c
> +++ b/profiles/time/server.c
> @@ -55,22 +55,22 @@
> #define CT_TIME_CHR_UUID 0x2A2B
>
> enum {
> - UPDATE_RESULT_SUCCESSFUL = 0,
> - UPDATE_RESULT_CANCELED = 1,
> - UPDATE_RESULT_NO_CONN = 2,
> - UPDATE_RESULT_ERROR = 3,
> - UPDATE_RESULT_TIMEOUT = 4,
> - UPDATE_RESULT_NOT_ATTEMPTED = 5,
> + UPDATE_RESULT_SUCCESSFUL = 0,
> + UPDATE_RESULT_CANCELED = 1,
> + UPDATE_RESULT_NO_CONN = 2,
> + UPDATE_RESULT_ERROR = 3,
> + UPDATE_RESULT_TIMEOUT = 4,
> + UPDATE_RESULT_NOT_ATTEMPTED = 5,
> };
>
> enum {
> - UPDATE_STATE_IDLE = 0,
> - UPDATE_STATE_PENDING = 1,
> + UPDATE_STATE_IDLE = 0,
> + UPDATE_STATE_PENDING = 1,
> };
>
> enum {
> - GET_REFERENCE_UPDATE = 1,
> - CANCEL_REFERENCE_UPDATE = 2,
> + GET_REFERENCE_UPDATE = 1,
> + CANCEL_REFERENCE_UPDATE = 2,
> };
>
> static int encode_current_time(uint8_t value[10])
> --
> 1.9.1
>
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH] Bluetooth: Incorrect locking when sending data in softirq
From: Jukka Rissanen @ 2014-10-16 7:47 UTC (permalink / raw)
To: Peter Hurley; +Cc: linux-bluetooth
In-Reply-To: <543EDD20.8050508@hurleysoftware.com>
Hi Peter,
On ke, 2014-10-15 at 16:46 -0400, Peter Hurley wrote:
> That's happening because 6lowpan.c:send_mcast_pkt() is disabling
> interrupts with the read_lock_irqsave() before calling send_pkt().
>
> It's unclear browsing through the lowpan driver why the
> irqflags save/restore read_lock flavors are being used; is there a
> place where the bluetooth core is calling the driver in atomic
> context (ie., where interrupts are disabled)?
>
> The devices_lock/bt_6lowpan_devices list looks ideal for
> converting to RCU.
>
> Regards,
> Peter Hurley
>
> PS - list_for_each_entry_safe() is only required when the reference
> to the list entry may become invalid _while_ still iterating the list.
> So not necessary in peer_lookup_conn(), lookup_peer(), lookup_dev(),
> peer_lookup_chan(), peer_lookup_ba(), send_mcast_pkt(), maybe others.
>
Thanks for the explanations, these were very valuable. I will
investigate the RCU conversion and fixes to list handling.
Cheers,
Jukka
^ permalink raw reply
* Re: [PBAP 1/2] obexd/client/pbap: Add support for spd,fav in PBAP
From: Luiz Augusto von Dentz @ 2014-10-16 7:45 UTC (permalink / raw)
To: Gowtham Anandha Babu
Cc: linux-bluetooth@vger.kernel.org, Dmitry Kasatkin, Bharat Panda,
cpgs
In-Reply-To: <1413380382-3302-1-git-send-email-gowtham.ab@samsung.com>
Hi,
On Wed, Oct 15, 2014 at 4:39 PM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> Add support for the speed-dial and favorite entries in PBAP.
> ---
> doc/obex-api.txt | 2 ++
> obexd/client/pbap.c | 10 +++++++---
> obexd/plugins/phonebook.h | 4 ++++
> 3 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/doc/obex-api.txt b/doc/obex-api.txt
> index f80ef39..4439a41 100644
> --- a/doc/obex-api.txt
> +++ b/doc/obex-api.txt
> @@ -341,6 +341,8 @@ Methods void Select(string location, string phonebook)
> "och": outgoing call history
> "mch": missing call history
> "cch": combination of ich och mch
> + "spd": speed dials entry ( only for "internal" )
> + "fav": favorites entry ( only for "internal" )
>
> Possible errors: org.bluez.obex.Error.InvalidArguments
> org.bluez.obex.Error.Failed
> diff --git a/obexd/client/pbap.c b/obexd/client/pbap.c
> index 2089860..614616c 100644
> --- a/obexd/client/pbap.c
> +++ b/obexd/client/pbap.c
> @@ -183,11 +183,13 @@ static const GMarkupParser listing_parser = {
> static char *build_phonebook_path(const char *location, const char *item)
> {
> char *path = NULL, *tmp, *tmp1;
> + int int_telecom = 0;
>
> if (!g_ascii_strcasecmp(location, "int") ||
> - !g_ascii_strcasecmp(location, "internal"))
> + !g_ascii_strcasecmp(location, "internal")) {
> path = g_strdup("/telecom");
> - else if (!g_ascii_strncasecmp(location, "sim", 3)) {
> + int_telecom = 1;
> + } else if (!g_ascii_strncasecmp(location, "sim", 3)) {
> if (strlen(location) == 3)
> tmp = g_strdup("sim1");
> else
> @@ -202,7 +204,9 @@ static char *build_phonebook_path(const char *location, const char *item)
> !g_ascii_strcasecmp(item, "ich") ||
> !g_ascii_strcasecmp(item, "och") ||
> !g_ascii_strcasecmp(item, "mch") ||
> - !g_ascii_strcasecmp(item, "cch")) {
> + !g_ascii_strcasecmp(item, "cch") ||
> + (int_telecom && !g_ascii_strcasecmp(item, "spd")) ||
> + (int_telecom && !g_ascii_strcasecmp(item, "fav"))) {
> tmp = path;
> tmp1 = g_ascii_strdown(item, -1);
> path = g_build_filename(tmp, tmp1, NULL);
> diff --git a/obexd/plugins/phonebook.h b/obexd/plugins/phonebook.h
> index fff33c1..70a9cb7 100644
> --- a/obexd/plugins/phonebook.h
> +++ b/obexd/plugins/phonebook.h
> @@ -37,6 +37,8 @@
> #define PB_CALLS_INCOMING_FOLDER "/telecom/ich"
> #define PB_CALLS_MISSED_FOLDER "/telecom/mch"
> #define PB_CALLS_OUTGOING_FOLDER "/telecom/och"
> +#define PB_CALLS_SPEEDDIAL_FOLDER "/telecom/spd"
> +#define PB_CALLS_FAVORITE_FOLDER "/telecom/fav"
> #define PB_LUID_FOLDER "/telecom/pb/luid"
>
> #define PB_CONTACTS "/telecom/pb.vcf"
> @@ -44,6 +46,8 @@
> #define PB_CALLS_INCOMING "/telecom/ich.vcf"
> #define PB_CALLS_MISSED "/telecom/mch.vcf"
> #define PB_CALLS_OUTGOING "/telecom/och.vcf"
> +#define PB_CALLS_SPEEDDIAL "/telecom/spd.vcf"
> +#define PB_CALLS_FAVORITE "/telecom/fav.vcf"
> #define PB_DEVINFO "/telecom/devinfo.txt"
> #define PB_INFO_LOG "/telecom/pb/info.log"
> #define PB_CC_LOG "/telecom/pb/luid/cc.log"
> --
> 1.9.1
Applied, please make sure you following the 50/72 format for commits,
it is documented in the HACKING document and I would like that we stay
consistent with it.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH v2 1/5] shared/att: Drop the connection if a request is received while one is pending.
From: Luiz Augusto von Dentz @ 2014-10-16 7:31 UTC (permalink / raw)
To: Arman Uguray; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CAHrH25S-W2gVkeJfsn74VCcD9h2wS86zWDcUVKX=-994pwV76A@mail.gmail.com>
Hi Arman,
On Wed, Oct 15, 2014 at 8:36 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Luiz,
>
>
>>> + case ATT_OP_TYPE_REQ:
>>> + /* If a request is currently pending, then the sequential
>>> + * protocol was violated. Disconnect the bearer and notify the
>>> + * upper-layer.
>>> + */
>>
>> It seems this code is not following our coding style regarding multi
>> line comments, check doc/coding-style.txt(M2: Multiple line comment)
>> the first line should be empty.
>>
>
> I didn't realize that was the comment style. I've seen both styles in
> daemon code and I think I went with this one because of that. I'd
> rather have the code be consistent (since shared/att uses this style
> elsewhere) and maybe do a style correction pass in another patch?
Sure, I can probably fix this myself just made the comment to let you
know that we do in fact have a coding style for it.
>
>>> + if (att->in_req) {
>>> + util_debug(att->debug_callback, att->debug_data,
>>> + "Received request while another is "
>>> + "pending: 0x%02x", opcode);
>>> + disconnect_cb(att->io, att);
>>> + return false;
>>> + }
>>> +
>>> + att->in_req = true;
>>> +
>>> + /* Fall through to the next case */
>>
>> This might cause a problem with our own code when connecting to each
>> other if bt_att_cancel is used, apparently bt_att_cancel does not send
>> anything to the remote side which seems to enable sending a new
>> request without waiting any response for the first one.
>>
>
> I'm confused. Do you mean if bt_att is being used in the client role,
> it can send multiple requests this way? That is correct, though I
> don't see how that's related to this patch? Currently bt_att_cancel
> can be used to cancel waiting for a pending request and send the next
> one, as you said. In that regard, it doesn't actually "cancel"
> anything if the PDU has been sent over the air already. I don't know
> how big of a problem this is, since the remote end should normally
> detect the error and terminate the connection anyway. If this becomes
> a problem we can always prevent canceling a request that has already
> left the queue and is pending. Though, again, this isn't really
> related to this patch unless I'm missing something.
Sorry for the confusion, this patch is actually fine what we really
need to fix is bt_att_cancel should not really remove the pending
request otherwise the remote may disconnect if we proceed with another
request.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: btusb_intr_complete returns -EPIPE
From: Naveen Kumar Parna @ 2014-10-16 7:13 UTC (permalink / raw)
To: Alan Stern
Cc: Oliver Neukum, linux-bluetooth@vger.kernel.org, linux-usb, acho
In-Reply-To: <Pine.LNX.4.44L0.1410151207020.1210-100000@iolanthe.rowland.org>
[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]
> It's entirely possible that the stall packets are created by the hub.
> When a full-speed device is connected to a USB-2 hub, and the device
> fails to respond to a packet sent by the host, the hub reports this
> failure as a stall.
Here I don’t think device fails to respond to a packet sent by the
host. I verified this by connecting Ellisys USB analyser in between
host and devices.
For example Look at the attached(Sample_HciEvt.png) HCI event captured
by Ellisys USB analyser. It is a valid HCI event from device to Host.
IN transaction 96 1 ACK FS 16 bytes (FF 2F C2 01 00 17 00 DF 00 01 10
00 00 A9 EE 0F)
IN transaction 96 1 ACK FS 16 bytes (00 00 00 5A 06 9D 39 00 00 66 00
00 00 00 00 00)
IN transaction 96 1 ACK FS 16 bytes (00 00 00 00 00 00 00 00 00 00 00
8E 05 28 00 01)
IN transaction 96 1 ACK FS 1 byte (00)
Due to spurious stall packets , sometimes btusb driver is not
receiving this full event , instead it got STALL packet instead of
first 16 bytes plus rest of other 33 bytes.
> When the device is connected to an OHCI controller, such failures would
> be reported in a different way, such as a -EPROTO or -EILSEQ status.
>
I did not observed -EPROTO or -EILSEQ status in OHCI controller scenario.
Thanks,
Naveen
[-- Attachment #2: Sample_HciEvt.png --]
[-- Type: image/png, Size: 7343 bytes --]
^ permalink raw reply
* Re: help needed for bluez debugging. - suspend-resume
From: John Miller @ 2014-10-16 3:44 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <CAPJhuYJS1wSDc9GssTLUAbuLRPW7ud6VyG4C8k3X2ok4LLmWNw@mail.gmail.com>
Somebody, please help
On Wed, Oct 15, 2014 at 5:11 PM, John Miller <jmiller5611@gmail.com> wrote:
> Hello,
>
> I ran into a problem in which after resuming form suspend my bluetooth
> headset does not autoconnect.
>
> i have bluez-5.19 in my system with chromium OS on kernel 3.14
>
> btmon says that when my HCI host tries to connect after resuming,
> "authentication failure" error pops up.
>
> How can i further debug this issue?
>
> What could be the reason for failure?
>
> Please help.
^ permalink raw reply
* [PATCH] Bluetooth: 6lowpan: remove unnecessary codes in give_skb_to_upper
From: roy.qing.li @ 2014-10-16 2:21 UTC (permalink / raw)
To: linux-bluetooth, marcel, gustavo, johan.hedberg
From: Li RongQing <roy.qing.li@gmail.com>
netif_rx() only returns NET_RX_DROP and NET_RX_SUCCESS, not returns
negative value
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
net/bluetooth/6lowpan.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index c2e0d14..9b5c89b 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -249,19 +249,12 @@ static struct lowpan_dev *lookup_dev(struct l2cap_conn *conn)
static int give_skb_to_upper(struct sk_buff *skb, struct net_device *dev)
{
struct sk_buff *skb_cp;
- int ret;
skb_cp = skb_copy(skb, GFP_ATOMIC);
if (!skb_cp)
return -ENOMEM;
- ret = netif_rx(skb_cp);
- if (ret < 0) {
- BT_DBG("receive skb %d", ret);
- return NET_RX_DROP;
- }
-
- return ret;
+ return netif_rx(skb_cp);
}
static int process_data(struct sk_buff *skb, struct net_device *netdev,
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Incorrect locking when sending data in softirq
From: Peter Hurley @ 2014-10-16 1:20 UTC (permalink / raw)
To: Alexander Aring; +Cc: Jukka Rissanen, linux-bluetooth
In-Reply-To: <20141015222333.GC17138@omega>
On 10/15/2014 06:23 PM, Alexander Aring wrote:
> Hi Peter,
>
> On Wed, Oct 15, 2014 at 05:58:40PM -0400, Peter Hurley wrote:
>> On 10/15/2014 05:53 PM, Alexander Aring wrote:
>>> Hi Peter,
>>>
>>> On Wed, Oct 15, 2014 at 04:46:24PM -0400, Peter Hurley wrote:
>>> ...
>>>>
>>>> That's happening because 6lowpan.c:send_mcast_pkt() is disabling
>>>> interrupts with the read_lock_irqsave() before calling send_pkt().
>>>>
>>>> It's unclear browsing through the lowpan driver why the
>>>> irqflags save/restore read_lock flavors are being used; is there a
>>>> place where the bluetooth core is calling the driver in atomic
>>>> context (ie., where interrupts are disabled)?
>>>>
>>>
>>> In my opinion bt_xmit is called in atomic context. Make the stacktrace
>>> sense now?
>>
>> I don't think so.
>>
>> The network layer is very careful about keeping interrupts enabled
>> and calling network drivers from softirq, so that spin_lock_bh() is
>> typically all that's required.
>>
>>> It's the callback 'ndo_start_xmit' of 'struct net_device_ops' [0].
>>
>> The send_mcast_pkt() isn't showing in the stack trace because it's
>> being inlined; and send_mcast_pkt() is definitely disabling interrupts
>> and calling send_pkt().
>>
>
> Thanks for explanation, now I know a little more about different context
> and handling of them. I need to say I am not an expert into this. :-)
>
>
> Nevertheless, I found something in "Documentation/networking/netdevices.txt"
> about context information for 'ndo_start_xmit':
>
> <qoute>
> ...
> Context: Process with BHs disabled or BH (timer),
> will be called with interrupts disabled by netconsole.
> ...
> </qoute>
>
> Now I am not sure if this helps us and what exactly this means. But "...
> interrupts disabled..." is this atomic now?
Local interrupts disabled is only atomic for the same cpu. Which would mean
that spin_lock_bh() wouldn't be appropriate in the ndo_start_xmit() handler.
However, this driver doesn't support netconsole because it doesn't support
the netpoll api, so it won't be called from that context.
Regards,
Peter Hurley
^ permalink raw reply
* Fwd: help needed for bluez debugging. - suspend-resume
From: John Miller @ 2014-10-16 0:11 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <CAPJhuYJ9hTkoavCcg5O5jXJOONUQpns2S2iXgHQCza7v9u=zig@mail.gmail.com>
Hello,
I ran into a problem in which after resuming form suspend my bluetooth
headset does not autoconnect.
i have bluez-5.19 in my system with chromium OS on kernel 3.14
btmon says that when my HCI host tries to connect after resuming,
"authentication failure" error pops up.
How can i further debug this issue?
What could be the reason for failure?
Please help.
^ permalink raw reply
* Re: [PATCH] Bluetooth: Incorrect locking when sending data in softirq
From: Alexander Aring @ 2014-10-15 22:23 UTC (permalink / raw)
To: Peter Hurley; +Cc: Jukka Rissanen, linux-bluetooth
In-Reply-To: <543EEE10.2010000@hurleysoftware.com>
Hi Peter,
On Wed, Oct 15, 2014 at 05:58:40PM -0400, Peter Hurley wrote:
> On 10/15/2014 05:53 PM, Alexander Aring wrote:
> > Hi Peter,
> >
> > On Wed, Oct 15, 2014 at 04:46:24PM -0400, Peter Hurley wrote:
> > ...
> >>
> >> That's happening because 6lowpan.c:send_mcast_pkt() is disabling
> >> interrupts with the read_lock_irqsave() before calling send_pkt().
> >>
> >> It's unclear browsing through the lowpan driver why the
> >> irqflags save/restore read_lock flavors are being used; is there a
> >> place where the bluetooth core is calling the driver in atomic
> >> context (ie., where interrupts are disabled)?
> >>
> >
> > In my opinion bt_xmit is called in atomic context. Make the stacktrace
> > sense now?
>
> I don't think so.
>
> The network layer is very careful about keeping interrupts enabled
> and calling network drivers from softirq, so that spin_lock_bh() is
> typically all that's required.
>
> > It's the callback 'ndo_start_xmit' of 'struct net_device_ops' [0].
>
> The send_mcast_pkt() isn't showing in the stack trace because it's
> being inlined; and send_mcast_pkt() is definitely disabling interrupts
> and calling send_pkt().
>
Thanks for explanation, now I know a little more about different context
and handling of them. I need to say I am not an expert into this. :-)
Nevertheless, I found something in "Documentation/networking/netdevices.txt"
about context information for 'ndo_start_xmit':
<qoute>
...
Context: Process with BHs disabled or BH (timer),
will be called with interrupts disabled by netconsole.
...
</qoute>
Now I am not sure if this helps us and what exactly this means. But "...
interrupts disabled..." is this atomic now?
- Alex
^ permalink raw reply
* Re: [PATCH] Bluetooth: Incorrect locking when sending data in softirq
From: Peter Hurley @ 2014-10-15 21:58 UTC (permalink / raw)
To: Alexander Aring; +Cc: Jukka Rissanen, linux-bluetooth
In-Reply-To: <20141015215324.GB17138@omega>
On 10/15/2014 05:53 PM, Alexander Aring wrote:
> Hi Peter,
>
> On Wed, Oct 15, 2014 at 04:46:24PM -0400, Peter Hurley wrote:
> ...
>>
>> That's happening because 6lowpan.c:send_mcast_pkt() is disabling
>> interrupts with the read_lock_irqsave() before calling send_pkt().
>>
>> It's unclear browsing through the lowpan driver why the
>> irqflags save/restore read_lock flavors are being used; is there a
>> place where the bluetooth core is calling the driver in atomic
>> context (ie., where interrupts are disabled)?
>>
>
> In my opinion bt_xmit is called in atomic context. Make the stacktrace
> sense now?
I don't think so.
The network layer is very careful about keeping interrupts enabled
and calling network drivers from softirq, so that spin_lock_bh() is
typically all that's required.
> It's the callback 'ndo_start_xmit' of 'struct net_device_ops' [0].
The send_mcast_pkt() isn't showing in the stack trace because it's
being inlined; and send_mcast_pkt() is definitely disabling interrupts
and calling send_pkt().
Regards,
Peter Hurley
^ permalink raw reply
* Re: [PATCH] Bluetooth: Incorrect locking when sending data in softirq
From: Alexander Aring @ 2014-10-15 21:53 UTC (permalink / raw)
To: Peter Hurley; +Cc: Jukka Rissanen, linux-bluetooth
In-Reply-To: <543EDD20.8050508@hurleysoftware.com>
Hi Peter,
On Wed, Oct 15, 2014 at 04:46:24PM -0400, Peter Hurley wrote:
...
>
> That's happening because 6lowpan.c:send_mcast_pkt() is disabling
> interrupts with the read_lock_irqsave() before calling send_pkt().
>
> It's unclear browsing through the lowpan driver why the
> irqflags save/restore read_lock flavors are being used; is there a
> place where the bluetooth core is calling the driver in atomic
> context (ie., where interrupts are disabled)?
>
In my opinion bt_xmit is called in atomic context. Make the stacktrace
sense now?
It's the callback 'ndo_start_xmit' of 'struct net_device_ops' [0].
- Alex
[0] http://lxr.free-electrons.com/source/include/linux/netdevice.h#L1001
^ permalink raw reply
* Re: [PATCH] Bluetooth: Incorrect locking when sending data in softirq
From: Peter Hurley @ 2014-10-15 20:46 UTC (permalink / raw)
To: Jukka Rissanen; +Cc: linux-bluetooth
In-Reply-To: <1413379948.2705.120.camel@jrissane-mobl.ger.corp.intel.com>
On 10/15/2014 09:32 AM, Jukka Rissanen wrote:
> Hi Peter,
>
> On ke, 2014-10-15 at 09:18 -0400, Peter Hurley wrote:
>> Hi Jukka,
>>
>> On 10/15/2014 08:43 AM, Jukka Rissanen wrote:
>>> Use spin_lock_irqsave() when sending data to hci channel. Otherwise
>>> the lockdep gives inconsistent lock state warning when sending data
>>> to 6lowpan channel.
>>>
>>> [ INFO: inconsistent lock state ]
>>> 3.17.0-rc1-bt6lowpan #1 Not tainted
>>> ---------------------------------
>>> inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
>>> kworker/u3:0/321 [HC0[0]:SC0[0]:HE1:SE1] takes:
>>> (&(&list->lock)->rlock#6){+.?...}, at: [<f845fdac>] hci_send_acl+0xac/0x290 [bluetooth]
>>> {IN-SOFTIRQ-W} state was registered at:
>>> [<c10915a3>] __lock_acquire+0x6d3/0x1d20
>>> [<c109325d>] lock_acquire+0x9d/0x140
>>> [<c1889c25>] _raw_spin_lock+0x45/0x80
>>> [<f845fdac>] hci_send_acl+0xac/0x290 [bluetooth]
>>> [<f8480c60>] l2cap_do_send+0x60/0x100 [bluetooth]
>>> [<f8484830>] l2cap_chan_send+0x7f0/0x10e0 [bluetooth]
>>> [<f873191e>] send_pkt+0x4e/0xa0 [bluetooth_6lowpan]
>>> [<f8731d20>] bt_xmit+0x3b0/0x770 [bluetooth_6lowpan]
>>> [<c17742f4>] dev_hard_start_xmit+0x344/0x670
>>> [<c17749ad>] __dev_queue_xmit+0x38d/0x680
>>> [<c1774caf>] dev_queue_xmit+0xf/0x20
>>> [<c177b8b0>] neigh_connected_output+0x130/0x1a0
>>> [<c1812a63>] ip6_finish_output2+0x173/0x8c0
>>> [<c18182db>] ip6_finish_output+0x7b/0x1b0
>>> [<c18184a7>] ip6_output+0x97/0x2a0
>>> [<c183a46b>] mld_sendpack+0x5eb/0x650
>>> [<c183acc1>] mld_ifc_timer_expire+0x191/0x2f0
>>> [<c10ac385>] call_timer_fn+0x85/0x1c0
>>> [<c10acb72>] run_timer_softirq+0x192/0x280
>>> [<c104fd84>] __do_softirq+0xd4/0x300
>>> [<c10049fc>] do_softirq_own_stack+0x2c/0x40
>>> [<c1050136>] irq_exit+0x86/0xb0
>>> [<c188bd98>] smp_apic_timer_interrupt+0x38/0x50
>>> [<c188b6ce>] apic_timer_interrupt+0x32/0x38
>>>
>>> Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
>>> ---
>>> Hi,
>>>
>>> this patch fixes the locking issue when sending larger amount of
>>> data via 6lowpan link. After this patch the lockdep no longer warns
>>> about softirq issues.
>>>
>>> Cheers,
>>> Jukka
>>>
>>>
>>> net/bluetooth/hci_core.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>> index 8aea4be..3e295ff 100644
>>> --- a/net/bluetooth/hci_core.c
>>> +++ b/net/bluetooth/hci_core.c
>>> @@ -4642,6 +4642,7 @@ static void hci_queue_acl(struct hci_chan *chan, struct sk_buff_head *queue,
>>> struct hci_conn *conn = chan->conn;
>>> struct hci_dev *hdev = conn->hdev;
>>> struct sk_buff *list;
>>> + unsigned long irq_flags;
>>>
>>> skb->len = skb_headlen(skb);
>>> skb->data_len = 0;
>>> @@ -4673,7 +4674,7 @@ static void hci_queue_acl(struct hci_chan *chan, struct sk_buff_head *queue,
>>> skb_shinfo(skb)->frag_list = NULL;
>>>
>>> /* Queue all fragments atomically */
>>> - spin_lock(&queue->lock);
>>> + spin_lock_irqsave(&queue->lock, irq_flags);
>>
>> spin_lock_bh(&queue->lock) will suffice.
>
> I thought so too but when using spin_lock_bh() I saw this warning
>
> WARNING: CPU: 0 PID: 269 at .../linux/kernel/softirq.c:146
> __local_bh_enable_ip+0x98/0xf0()
> Modules linked in: bluetooth_6lowpan 6lowpan rfcomm ecb btusb bnep
> bluetooth nfc rfkill ohci_pci snd_intel8x0 snd_ac97_codec ac97_bus
> parport_pc parport
> CPU: 0 PID: 269 Comm: systemd-journal Not tainted 3.17.0-rc1-bt6lowpan
> #1
> Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox
> 12/01/2006
> 00000000 00000000 f600bb4c c18821c1 00000000 f600bb80 c104c472 c1ac7860
> 00000000 0000010d c1ac7c68 00000092 c104fc58 00000092 c104fc58 00000201
> f82d6e39 00000000 f600bb90 c104c532 00000009 00000000 f600bba0 c104fc58
> Call Trace:
> [<c18821c1>] dump_stack+0x4b/0x75
> [<c104c472>] warn_slowpath_common+0x82/0xa0
> [<c104fc58>] ? __local_bh_enable_ip+0x98/0xf0
> [<c104fc58>] ? __local_bh_enable_ip+0x98/0xf0
> [<f82d6e39>] ? hci_send_acl+0x199/0x290 [bluetooth]
> [<c104c532>] warn_slowpath_null+0x22/0x30
> [<c104fc58>] __local_bh_enable_ip+0x98/0xf0
> [<c1889fff>] _raw_spin_unlock_bh+0x2f/0x40
> [<f82d6e39>] hci_send_acl+0x199/0x290 [bluetooth]
> [<c108c956>] ? trace_hardirqs_off_caller+0x66/0x160
> [<f82f7bf0>] l2cap_do_send+0x60/0x100 [bluetooth]
> [<c108ca5b>] ? trace_hardirqs_off+0xb/0x10
> [<c188a051>] ? _raw_spin_unlock_irqrestore+0x41/0x70
> [<c1763125>] ? skb_dequeue+0x45/0x60
> [<f82fb7c0>] l2cap_chan_send+0x7f0/0x10e0 [bluetooth]
> [<c108ca5b>] ? trace_hardirqs_off+0xb/0x10
> [<c188a6d1>] ? _raw_write_unlock_irqrestore+0x41/0x70
> [<c187f10c>] ? kmemleak_alloc+0x3c/0xb0
> [<f84f591e>] send_pkt+0x4e/0xa0 [bluetooth_6lowpan]
> [<f84f5d20>] bt_xmit+0x3b0/0x770 [bluetooth_6lowpan]
> [<c176f2a0>] ? netif_napi_del+0x50/0x50
> [<c17742f4>] dev_hard_start_xmit+0x344/0x670
> [<c1889c4b>] ? _raw_spin_lock+0x6b/0x80
> [<c17749ad>] __dev_queue_xmit+0x38d/0x680
> [<c177465b>] ? __dev_queue_xmit+0x3b/0x680
> [<c108f1d4>] ? trace_hardirqs_on_caller+0xf4/0x240
> [<f84f5820>] ? lookup_peer+0xb0/0xb0 [bluetooth_6lowpan]
> [<c1774caf>] dev_queue_xmit+0xf/0x20
> [<c177b8b0>] neigh_connected_output+0x130/0x1a0
> [<c1812a63>] ? ip6_finish_output2+0x173/0x8c0
> [<c1812a63>] ip6_finish_output2+0x173/0x8c0
> [<c181293c>] ? ip6_finish_output2+0x4c/0x8c0
> [<c18182db>] ip6_finish_output+0x7b/0x1b0
> [<c18184a7>] ip6_output+0x97/0x2a0
> [<c1825cb0>] ? ip6_blackhole_route+0x250/0x250
> [<c183a46b>] mld_sendpack+0x5eb/0x650
> [<c183acc1>] ? mld_ifc_timer_expire+0x191/0x2f0
> [<c183acc1>] mld_ifc_timer_expire+0x191/0x2f0
> [<c10ac385>] call_timer_fn+0x85/0x1c0
> [<c10ac300>] ? process_timeout+0x10/0x10
> [<c108f1d4>] ? trace_hardirqs_on_caller+0xf4/0x240
> [<c183ab30>] ? mld_send_initial_cr.part.31+0xa0/0xa0
> [<c10acb72>] run_timer_softirq+0x192/0x280
> [<c104fcb0>] ? __local_bh_enable_ip+0xf0/0xf0
> [<c13d09ff>] ? __this_cpu_preempt_check+0xf/0x20
> [<c183ab30>] ? mld_send_initial_cr.part.31+0xa0/0xa0
> [<c104fd84>] __do_softirq+0xd4/0x300
> [<c104fcb0>] ? __local_bh_enable_ip+0xf0/0xf0
> [<c10049fc>] do_softirq_own_stack+0x2c/0x40
> <IRQ> [<c1050136>] irq_exit+0x86/0xb0
> [<c188bd98>] smp_apic_timer_interrupt+0x38/0x50
> [<c188b6ce>] apic_timer_interrupt+0x32/0x38
> [<c106e240>] ? __sched_fork.isra.74+0x140/0x140
> [<c10c8dbb>] ? is_module_text_address+0x2b/0x50
> [<c1065772>] __kernel_text_address+0x32/0x80
> [<c1005c2f>] print_context_stack+0x3f/0xe0
> [<c1004b65>] dump_trace+0xc5/0x1f0
> [<c100fcf0>] save_stack_trace+0x30/0x50
> [<c116aa8a>] create_object+0x10a/0x280
> [<c187f10c>] kmemleak_alloc+0x3c/0xb0
> [<c10718bb>] ? preempt_count_add+0x4b/0xa0
> [<c115e693>] kmem_cache_alloc+0x1a3/0x290
> [<c116fe47>] ? get_empty_filp+0x57/0x1d0
> [<c116fe47>] get_empty_filp+0x57/0x1d0
> [<c10761ef>] ? sched_clock_cpu+0x10f/0x160
> [<c117bcd8>] path_openat+0x28/0x5c0
> [<c13d09e2>] ? debug_smp_processor_id+0x12/0x20
> [<c117d331>] do_filp_open+0x31/0x90
> [<c118a2f8>] ? __alloc_fd+0x88/0x100
> [<c1889fac>] ? _raw_spin_unlock+0x2c/0x50
> [<c118a2f8>] ? __alloc_fd+0x88/0x100
> [<c116d887>] do_sys_open+0x117/0x210
> [<c188aeef>] ? restore_all+0xf/0xf
> [<c13d09ff>] ? __this_cpu_preempt_check+0xf/0x20
> [<c1080000>] ? dequeue_rt_stack+0x1a0/0x2e0
> [<c116d9a2>] SyS_open+0x22/0x30
> [<c188aeb6>] syscall_call+0x7/0x7
> [<c1880000>] ? hpet_reserve_platform_timers+0x6e/0x111
> ---[ end trace 81f9756f84a67848 ]---
That's happening because 6lowpan.c:send_mcast_pkt() is disabling
interrupts with the read_lock_irqsave() before calling send_pkt().
It's unclear browsing through the lowpan driver why the
irqflags save/restore read_lock flavors are being used; is there a
place where the bluetooth core is calling the driver in atomic
context (ie., where interrupts are disabled)?
The devices_lock/bt_6lowpan_devices list looks ideal for
converting to RCU.
Regards,
Peter Hurley
PS - list_for_each_entry_safe() is only required when the reference
to the list entry may become invalid _while_ still iterating the list.
So not necessary in peer_lookup_conn(), lookup_peer(), lookup_dev(),
peer_lookup_chan(), peer_lookup_ba(), send_mcast_pkt(), maybe others.
^ permalink raw reply
* Re: [PATCH v2 1/5] shared/att: Drop the connection if a request is received while one is pending.
From: Arman Uguray @ 2014-10-15 17:36 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZLYLDmjM0h2C9-sYOAxsRt4usdgr323bVmj=rF8byvjEw@mail.gmail.com>
Hi Luiz,
>> + case ATT_OP_TYPE_REQ:
>> + /* If a request is currently pending, then the sequential
>> + * protocol was violated. Disconnect the bearer and notify the
>> + * upper-layer.
>> + */
>
> It seems this code is not following our coding style regarding multi
> line comments, check doc/coding-style.txt(M2: Multiple line comment)
> the first line should be empty.
>
I didn't realize that was the comment style. I've seen both styles in
daemon code and I think I went with this one because of that. I'd
rather have the code be consistent (since shared/att uses this style
elsewhere) and maybe do a style correction pass in another patch?
>> + if (att->in_req) {
>> + util_debug(att->debug_callback, att->debug_data,
>> + "Received request while another is "
>> + "pending: 0x%02x", opcode);
>> + disconnect_cb(att->io, att);
>> + return false;
>> + }
>> +
>> + att->in_req = true;
>> +
>> + /* Fall through to the next case */
>
> This might cause a problem with our own code when connecting to each
> other if bt_att_cancel is used, apparently bt_att_cancel does not send
> anything to the remote side which seems to enable sending a new
> request without waiting any response for the first one.
>
I'm confused. Do you mean if bt_att is being used in the client role,
it can send multiple requests this way? That is correct, though I
don't see how that's related to this patch? Currently bt_att_cancel
can be used to cancel waiting for a pending request and send the next
one, as you said. In that regard, it doesn't actually "cancel"
anything if the PDU has been sent over the air already. I don't know
how big of a problem this is, since the remote end should normally
detect the error and terminate the connection anyway. If this becomes
a problem we can always prevent canceling a request that has already
left the queue and is pending. Though, again, this isn't really
related to this patch unless I'm missing something.
Thanks,
Arman
^ permalink raw reply
* Re: btusb_intr_complete returns -EPIPE
From: Alan Stern @ 2014-10-15 16:11 UTC (permalink / raw)
To: Naveen Kumar Parna
Cc: Oliver Neukum, linux-bluetooth@vger.kernel.org, linux-usb, acho
In-Reply-To: <CAG0bkvKo0FQh+8_qTUMbttq_ggBnfr8dgeJAopxyozNK6u4=Fw@mail.gmail.com>
On Wed, 15 Oct 2014, Naveen Kumar Parna wrote:
> Hi Oliver,
>
> I tried this test in two different set of hardware configurations.
>
>
>
> i) I tried in multiple test systems which has
> EHCI-USB host controller on PCI card and internal USB 2.0
> hub("rate-matching" hub). All the test systems with this configuration
> gives spurious stall packets.
>
> [lowerlayers@banunxcas29 ~]$ lspci | grep -i usb
>
> 00:1a.0 USB Controller: Intel Corporation 5 Series/3400 Series Chipset
> USB2 Enhanced Host Controller (rev 05)
>
> 00:1d.0 USB Controller: Intel Corporation 5 Series/3400 Series Chipset
> USB2 Enhanced Host Controller (rev 05)
>
>
> lsusb:
>
> Bus 001 Device 002: ID 8087:0020 Intel Corp. Integrated Rate Matching Hub
>
> Bus 002 Device 002: ID 8087:0020 Intel Corp. Integrated Rate Matching Hub
>
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>
> Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>
>
>
>
>
> ii) I found different test systems which has
> OHCI-USB host controller on PCI card and internal USB 1.1 hub. All the
> test systems with this configuration are not producing stall packets.
>
> [lowerlayers@camunxcas11 ~]$ lspci | grep -i usb
>
> 00:02.0 USB Controller: nVidia Corporation CK804 USB Controller (rev a2)
>
> 00:02.1 USB Controller: nVidia Corporation CK804 USB Controller (rev a3)
>
>
> lsusb:
>
> Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
>
> Bus 002 Device 002: ID 0451:2077 Texas Instruments, Inc. TUSB2077 Hub
>
>
>
>
> My device is a full-speed device. So , stall packets are due to buggy
> USB 2.0 hub?
It's entirely possible that the stall packets are created by the hub.
When a full-speed device is connected to a USB-2 hub, and the device
fails to respond to a packet sent by the host, the hub reports this
failure as a stall.
When the device is connected to an OHCI controller, such failures would
be reported in a different way, such as a -EPROTO or -EILSEQ status.
> Is there a chance of getting stall packets “If the device runs at low
> speed or full speed and is connected through a USB-2.0 hub”? If so it
> looks like hub driver issue right?
If the problem is that the device fails to respond to a packet then it
is an issue with the device.
> If the hub is the problem… what will be the better solution? Is it
> possible to change internal hub?
No, it is not possible.
Alan Stern
^ permalink raw reply
* Re: GATT Notification not sending
From: Johan Hedberg @ 2014-10-15 14:55 UTC (permalink / raw)
To: Nathan Jozwiak; +Cc: linux-bluetooth
In-Reply-To: <543D5645.7090403@mlsw.biz>
Hi Nathan,
On Tue, Oct 14, 2014, Nathan Jozwiak wrote:
> Update: I downloaded the LightBlue app from the Apple Store. The app allows
> you to connect to a GATT server and select available services to interact
> with. My notification characteristic showed up properly with an initial
> value of 0x00 and a CCC value of 0. But when attempted to register with the
> notification I saw this error from my bluetoothd output:
>
> bluetoothd[8754]: Refusing storage path for private addressed device
> /org/bluez/hci0/dev_59_F2_63_47_1D_90
> bluetoothd[8754]: Unable to get ccc storage path for device
> bluetoothd[8754]: attrib/gattrib.c:g_attrib_ref() 0xa073bd8: ref=3
> bluetoothd[8754]: attrib/gattrib.c:g_attrib_unref() 0xa073bd8: ref=2
>
> Unfortunately, I do not know the command LightBlue was sending to set the
> CCC, but I figured this might be a clue for someone.
Unfortunately this isn't related to your main issue. It just means that
you're running a kernel version that doesn't support LE Privacy. Because
of this the kernel has not been able to notify user space of the
Identity Address of the remote device so that information about it could
be stored persistently (iOS is one of the few implementations out there
actively using Random Private Addresses).
Johan
^ permalink raw reply
* Re: BT-4.1 features (e.g. concurrent GAP operations) support
From: Johan Hedberg @ 2014-10-15 14:50 UTC (permalink / raw)
To: Min Jun,Xi; +Cc: linux-bluetooth
In-Reply-To: <CAB0Wyv=3HBi6wnStgjGFwg7HUODoYj3mZBVTnZnC+tb+Mipyxw@mail.gmail.com>
Hi,
On Wed, Oct 15, 2014, Min Jun,Xi wrote:
> Where can I find the latest BT-4.1 features support for
> Bluetooth-subsystem and BlueZ?
I don't think the detailed feature status is documented anywhere but the
code, so the most reliable way would be to get the latest user space and
kernel trees and experiment with them yourself.
> In the 4.1 features, I know connection-oriented channels has been supported;
>
> I have one question about this new feature (Bluetooth Specification
> Version 4.1[Vol 3] page 293 of 668, ): "2.2.2.5 Concurrent Operation
> in Multiple GAP Roles"; this feature is very useful to support
> mesh-like topology,
>
> From the description in chapter 2.2.2.5, the controller should support
> operations in multiple GAP roles concurrently, and the host should
> read the supported Link Layer States and State combinations from the
> Controller before any procedures or modes are used.
>
> Can anyone tell me if the bluetooth subsystem has supported the Link
> Layer states and the States combinations? I am also looking for the
> Controller support operations in multiple GAP roles concurrently, can
> anyone know if CSR/Nordic/Broadcom has got the USB dongles which
> supports concurrent operation and can used under Linux?
We don't specifically look at the controller supported states at the
moment but simply stick to what we know existing controllers to be
capable of. This means that you will be able to both scan and advertise
at the same time (i.e. act as central and peripheral). What we do limit
(since we don't know if HW that supports it) is new LE connection
creation when there already is an existing slave-role connection.
Johan
^ permalink raw reply
* Re: [PATCH] android/pts: PTS tests update for RFCOMM
From: Szymon Janc @ 2014-10-15 14:43 UTC (permalink / raw)
To: Sebastian Chlad; +Cc: linux-bluetooth
In-Reply-To: <1413199783-19452-1-git-send-email-sebastian.chlad@tieto.com>
Hi Sebastian,
On Monday 13 of October 2014 13:29:43 Sebastian Chlad wrote:
> Test TC_RFC_BV_25_C depends on kernel patches for Non-Supported Command,
> "Bluetooth: Fix RFCOMM NSC response" and "Bluetooth: Improve
> RFCOMM__test_pf macro robustness"
> ---
> android/pts-rfcomm.txt | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/android/pts-rfcomm.txt b/android/pts-rfcomm.txt
> index 5d51905..98a691b 100644
> --- a/android/pts-rfcomm.txt
> +++ b/android/pts-rfcomm.txt
> @@ -1,8 +1,9 @@
> PTS test results for RFCOMM
>
> PTS version: 5.3
> -Tested: 08-October-2014
> +Tested: 13-October-2014
> Android version: 4.4.4
> +Kernel version: 3.18
>
> Results:
> PASS test passed
> @@ -18,6 +19,7 @@ TC_RFC_BV_01_C PASS rctest -n -P 1 <btaddr>
> TC_RFC_BV_02_C PASS rctest -r -P 1
> TC_RFC_BV_03_C PASS rctest -r -P 1
> TC_RFC_BV_04_C PASS Note: use ETS provided in PTS issue #12414
> + rctest -r -P 1
> TC_RFC_BV_05_C PASS rctest -n -P 4 <btaddr>
> Note: test requires IUT to connect on the given
> channel. sdptool browse <btaddr> to check the
> @@ -35,5 +37,5 @@ TC_RFC_BV_17_C PASS rctest -d -P 1
> TC_RFC_BV_19_C PASS
> TC_RFC_BV_21_C INC PTS issue #12421
> TC_RFC_BV_22_C INC PTS issue #12421
> -TC_RFC_BV_25_C INC PTS issue #12621
> +TC_RFC_BV_25_C PASS rctest -r -P 1
> ---------------------------------------------------------------------------
> ----
I've change kernel version to 3.19 since fix mentioned was applied to
bluetooth-next.git tree.
Applied. Thanks.
--
BR
Szymon Janc
^ permalink raw reply
* [PATCH v3 5/5] android/ipc-tester: Add cases for MAP client msg size
From: Grzegorz Kolodziejczyk @ 2014-10-15 14:28 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413383281-17292-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
Add cases testing message size verification for MAP client opcodes.
---
android/ipc-tester.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/android/ipc-tester.c b/android/ipc-tester.c
index 7dd25e8..357dda9 100644
--- a/android/ipc-tester.c
+++ b/android/ipc-tester.c
@@ -1486,5 +1486,19 @@ int main(int argc, char *argv[])
HAL_SERVICE_ID_BLUETOOTH,
HAL_SERVICE_ID_HANDSFREE_CLIENT);
+ /* check for valid data size for MAP CLIENT */
+ test_datasize_valid("MAP CLIENT Get instances+",
+ HAL_SERVICE_ID_MAP_CLIENT,
+ HAL_OP_MAP_CLIENT_GET_INSTANCES,
+ sizeof(struct hal_cmd_map_client_get_instances),
+ 1, HAL_SERVICE_ID_BLUETOOTH,
+ HAL_SERVICE_ID_MAP_CLIENT);
+ test_datasize_valid("MAP CLIENT Get instances-",
+ HAL_SERVICE_ID_MAP_CLIENT,
+ HAL_OP_MAP_CLIENT_GET_INSTANCES,
+ sizeof(struct hal_cmd_map_client_get_instances),
+ -1, HAL_SERVICE_ID_BLUETOOTH,
+ HAL_SERVICE_ID_MAP_CLIENT);
+
return tester_run();
}
--
1.9.3
^ permalink raw reply related
* [PATCH v3 4/5] android/ipc-tester: Add case for MAP client service opcode boundries
From: Grzegorz Kolodziejczyk @ 2014-10-15 14:28 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413383281-17292-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
This patch adds test sending out of range opcode for service.
---
android/ipc-tester.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/android/ipc-tester.c b/android/ipc-tester.c
index 161777d..7dd25e8 100644
--- a/android/ipc-tester.c
+++ b/android/ipc-tester.c
@@ -940,6 +940,10 @@ int main(int argc, char *argv[])
HAL_SERVICE_ID_BLUETOOTH,
HAL_SERVICE_ID_HANDSFREE_CLIENT);
+ test_opcode_valid("MAP_CLIENT", HAL_SERVICE_ID_MAP_CLIENT, 0x01, 0,
+ HAL_SERVICE_ID_BLUETOOTH,
+ HAL_SERVICE_ID_MAP_CLIENT);
+
/* check for valid data size */
test_datasize_valid("CORE Register+", HAL_SERVICE_ID_CORE,
HAL_OP_REGISTER_MODULE,
--
1.9.3
^ permalink raw reply related
* [PATCH v3 3/5] android/map-client: Add support for get remote MAS instances
From: Grzegorz Kolodziejczyk @ 2014-10-15 14:27 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413383281-17292-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
This allows to get remote mas instances. In case of wrong sdp record
fail status will be returned in notification.
---
android/map-client.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 130 insertions(+), 1 deletion(-)
diff --git a/android/map-client.c b/android/map-client.c
index 142c680..12f600b 100644
--- a/android/map-client.c
+++ b/android/map-client.c
@@ -30,21 +30,150 @@
#include <stdint.h>
#include <glib.h>
+#include "lib/sdp.h"
+#include "lib/sdp_lib.h"
+#include "src/sdp-client.h"
+
#include "ipc.h"
#include "lib/bluetooth.h"
#include "map-client.h"
#include "src/log.h"
#include "hal-msg.h"
+#include "ipc-common.h"
+#include "utils.h"
+#include "src/shared/util.h"
static struct ipc *hal_ipc = NULL;
static bdaddr_t adapter_addr;
+static int fill_mce_inst(void *buf, int32_t id, int32_t scn, int32_t msg_type,
+ const void *name, uint8_t name_len)
+{
+ struct hal_map_client_mas_instance *inst = buf;
+
+ inst->id = id;
+ inst->scn = scn;
+ inst->msg_types = msg_type;
+ inst->name_len = name_len;
+
+ if (name_len)
+ memcpy(inst->name, name, name_len);
+
+ return sizeof(*inst) + name_len;
+}
+
+static void map_client_sdp_search_cb(sdp_list_t *recs, int err, gpointer data)
+{
+ uint8_t buf[IPC_MTU];
+ struct hal_ev_map_client_remote_mas_instances *ev = (void *) buf;
+ bdaddr_t *dst = data;
+ sdp_list_t *list, *protos;
+ uint8_t status;
+ int32_t id, scn, msg_type, name_len, num_instances = 0;
+ char *name;
+ size_t size;
+
+ size = sizeof(*ev);
+ bdaddr2android(dst, &ev->bdaddr);
+
+ if (err < 0) {
+ error("mce: Unable to get SDP record: %s", strerror(-err));
+ status = HAL_STATUS_FAILED;
+ goto fail;
+ }
+
+ for (list = recs; list != NULL; list = list->next) {
+ sdp_record_t *rec = list->data;
+ sdp_data_t *data;
+
+ data = sdp_data_get(rec, SDP_ATTR_MAS_INSTANCE_ID);
+ if (!data) {
+ error("mce: cannot get mas instance id");
+ continue;
+ }
+
+ id = data->val.uint8;
+
+ data = sdp_data_get(rec, SDP_ATTR_SVCNAME_PRIMARY);
+ if (!data) {
+ error("mce: cannot get mas instance name");
+ continue;
+ }
+
+ name = data->val.str;
+ name_len = data->unitSize;
+
+ data = sdp_data_get(rec, SDP_ATTR_SUPPORTED_MESSAGE_TYPES);
+ if (!data) {
+ error("mce: cannot get mas instance msg type");
+ continue;
+ }
+
+ msg_type = data->val.uint8;
+
+ if (sdp_get_access_protos(rec, &protos) < 0) {
+ error("mce: cannot get mas instance sdp protocol list");
+ continue;
+ }
+
+ scn = sdp_get_proto_port(protos, RFCOMM_UUID);
+
+ sdp_list_foreach(protos, (sdp_list_func_t) sdp_list_free, NULL);
+ sdp_list_free(protos, NULL);
+
+ if (!scn) {
+ error("mce: cannot get mas instance rfcomm channel");
+ continue;
+ }
+
+ size += fill_mce_inst(buf + size, id, scn, msg_type, name,
+ name_len);
+ num_instances++;
+ }
+
+ status = HAL_STATUS_SUCCESS;
+
+fail:
+ ev->num_instances = num_instances;
+ ev->status = status;
+
+ ipc_send_notif(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT,
+ HAL_EV_MAP_CLIENT_REMOTE_MAS_INSTANCES, size, buf);
+
+ free(dst);
+}
+
static void handle_get_instances(const void *buf, uint16_t len)
{
+ const struct hal_cmd_map_client_get_instances *cmd = buf;
+ uint8_t status;
+ bdaddr_t *dst;
+ uuid_t uuid;
+
DBG("");
+ dst = new0(bdaddr_t, 1);
+ if (!dst) {
+ error("mce: Fail to allocate cb data");
+ status = HAL_STATUS_FAILED;
+ goto failed;
+ }
+
+ android2bdaddr(&cmd->bdaddr, dst);
+ sdp_uuid16_create(&uuid, MAP_MSE_SVCLASS_ID);
+
+ if (bt_search_service(&adapter_addr, dst, &uuid,
+ map_client_sdp_search_cb, dst, free, 0)) {
+ error("mce: Failed to search SDP details");
+ status = HAL_STATUS_FAILED;
+ goto failed;
+ }
+
+ status = HAL_STATUS_SUCCESS;
+
+failed:
ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT,
- HAL_OP_MAP_CLIENT_GET_INSTANCES, HAL_STATUS_FAILED);
+ HAL_OP_MAP_CLIENT_GET_INSTANCES, status);
}
static const struct ipc_handler cmd_handlers[] = {
--
1.9.3
^ permalink raw reply related
* [PATCH v3 2/5] android/map-client: Add stubs for MAP client commands handlers
From: Grzegorz Kolodziejczyk @ 2014-10-15 14:27 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413383281-17292-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
Add empty handlers for MAP client IPC commands.
---
android/map-client.c | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/android/map-client.c b/android/map-client.c
index 4556461..142c680 100644
--- a/android/map-client.c
+++ b/android/map-client.c
@@ -28,17 +28,49 @@
#include <stdbool.h>
#include <stdlib.h>
#include <stdint.h>
+#include <glib.h>
#include "ipc.h"
#include "lib/bluetooth.h"
#include "map-client.h"
+#include "src/log.h"
+#include "hal-msg.h"
+
+static struct ipc *hal_ipc = NULL;
+static bdaddr_t adapter_addr;
+
+static void handle_get_instances(const void *buf, uint16_t len)
+{
+ DBG("");
+
+ ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT,
+ HAL_OP_MAP_CLIENT_GET_INSTANCES, HAL_STATUS_FAILED);
+}
+
+static const struct ipc_handler cmd_handlers[] = {
+ /* HAL_OP_MAP_CLIENT_GET_INSTANCES */
+ { handle_get_instances, false,
+ sizeof(struct hal_cmd_map_client_get_instances) },
+};
bool bt_map_client_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode)
{
- return false;
+ DBG("");
+
+ bacpy(&adapter_addr, addr);
+
+ hal_ipc = ipc;
+
+ ipc_register(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT, cmd_handlers,
+ G_N_ELEMENTS(cmd_handlers));
+
+ return true;
}
void bt_map_client_unregister(void)
{
+ DBG("");
+ ipc_unregister(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT);
+ hal_ipc = NULL;
}
--
1.9.3
^ permalink raw reply related
* [PATCH v3 1/5] android/map-client: Add initial files
From: Grzegorz Kolodziejczyk @ 2014-10-15 14:27 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413383281-17292-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
This adds initial daemon code for MAP client profile.
---
android/Android.mk | 1 +
android/Makefile.am | 1 +
android/main.c | 12 ++++++++++++
android/map-client.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
android/map-client.h | 26 ++++++++++++++++++++++++++
5 files changed, 84 insertions(+)
create mode 100644 android/map-client.c
create mode 100644 android/map-client.h
diff --git a/android/Android.mk b/android/Android.mk
index 2b59fb8..aefe41c 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -57,6 +57,7 @@ LOCAL_SRC_FILES := \
bluez/android/gatt.c \
bluez/android/health.c \
bluez/profiles/health/mcap.c \
+ bluez/android/map-client.c \
bluez/src/log.c \
bluez/src/shared/mgmt.c \
bluez/src/shared/util.c \
diff --git a/android/Makefile.am b/android/Makefile.am
index b013095..b3eb1c5 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -46,6 +46,7 @@ android_bluetoothd_SOURCES = android/main.c \
android/gatt.h android/gatt.c \
android/health.h android/health.c \
profiles/health/mcap.h profiles/health/mcap.c \
+ android/map-client.h android/map-client.c \
attrib/att.c attrib/att.h \
attrib/gatt.c attrib/gatt.h \
attrib/gattrib.c attrib/gattrib.h \
diff --git a/android/main.c b/android/main.c
index 703b3b6..b5f6937 100644
--- a/android/main.c
+++ b/android/main.c
@@ -62,6 +62,7 @@
#include "gatt.h"
#include "health.h"
#include "handsfree-client.h"
+#include "map-client.h"
#include "utils.h"
#define DEFAULT_VENDOR "BlueZ"
@@ -235,6 +236,14 @@ static void service_register(const void *buf, uint16_t len)
}
break;
+ case HAL_SERVICE_ID_MAP_CLIENT:
+ if (!bt_map_client_register(hal_ipc, &adapter_bdaddr,
+ m->mode)) {
+ status = HAL_STATUS_FAILED;
+ goto failed;
+ }
+
+ break;
default:
DBG("service %u not supported", m->service_id);
status = HAL_STATUS_FAILED;
@@ -288,6 +297,9 @@ static bool unregister_service(uint8_t id)
case HAL_SERVICE_ID_HANDSFREE_CLIENT:
bt_hf_client_unregister();
break;
+ case HAL_SERVICE_ID_MAP_CLIENT:
+ bt_map_client_unregister();
+ break;
default:
DBG("service %u not supported", id);
return false;
diff --git a/android/map-client.c b/android/map-client.c
new file mode 100644
index 0000000..4556461
--- /dev/null
+++ b/android/map-client.c
@@ -0,0 +1,44 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Intel Corporation. All rights reserved.
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdint.h>
+
+#include "ipc.h"
+#include "lib/bluetooth.h"
+#include "map-client.h"
+
+bool bt_map_client_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode)
+{
+ return false;
+}
+
+void bt_map_client_unregister(void)
+{
+
+}
diff --git a/android/map-client.h b/android/map-client.h
new file mode 100644
index 0000000..0e63072
--- /dev/null
+++ b/android/map-client.h
@@ -0,0 +1,26 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Intel Corporation. All rights reserved.
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+bool bt_map_client_register(struct ipc *ipc, const bdaddr_t *addr,
+ uint8_t mode);
+void bt_map_client_unregister(void);
--
1.9.3
^ permalink raw reply related
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