* Re: BLE - multiple connections
From: Ajay KV @ 2012-08-22 16:07 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth
In-Reply-To: <CAJdJm_OBh4OzKJejongsaWm1mSx2GN5Yki3B4nYc6C2DbVS=nw@mail.gmail.com>
On 08/17/2012 12:27 PM, Anderson Lizardo wrote:
> Hi Ajay
>
> [your emails to BlueZ mailing list may be getting blocked because you
> are using HTML. Please fix that on your mail client. Also remember to
> not top-post]
>
> On Fri, Aug 17, 2012 at 7:34 PM, Ajay KV<ajay.kv@globaledgesoft.com> wrote:
>
>> Thanks for your reply. i have 2 dongles of different made (iogear and
>> chirago) . "Hciconfig hci0 lestates" gives this following result
>>
>> Supported link layer states:
>> [...]
>>
>> Here my issue is to know whether (connected mode + advertising
>> mode) is possible at a time . Once A is connected to B (initiator), i
>> could not able to enable advertise in A (getting command disallowed ) , so
>> that device C is not getting the connectable advertising packets on
>> scanning.
>>
> I could not see any reason to get "command disallowed" when enabling
> LE adv. Did you try changing dongle combinations to make sure it is
> not some hardware issue?
>
> Regards,
>
Hi,
i could not able to figure it out yet , why its showing this
error " command disallowed" . Here im using fedora 13 with kernel 2.6.39
. i tried with both dongles , iogear and cirago . But the result is
same. i dont think that its something wrong with kernel version, as
we uses hci commands (hciconfig hci0 leadv ) that directly talks with
controller . please clear this issue..
^ permalink raw reply
* PBAP + two-step download
From: Patrick Ohly @ 2012-08-22 16:04 UTC (permalink / raw)
To: Bluez; +Cc: Mikel Astiz, Jeremy Whiting
Hello!
For IVI use cases, Mikel and I were discussing how a phone's address
book could be cached intelligently by an IVI head unit. The rough idea
is that first all contacts get pulled without PHOTO data. This data is
used to match the current phone address book with some potentially
cached local data. Then in a second step, the PHOTO data of all contacts
is requested (*). This is all done in the same PBAP session, so the
numbering of contacts is the same in both steps.
The goal is to get the essential data (names, phone numbers) quickly and
then add pictures later on.
There's one problem with the obexd API regarding this: the result of
PullAll does not include any information about the number that each
entry has in the current PBAP session.
For example, in the first step I get:
BEGIN:VCARD
N:John;;Doe
END:VCARD
In the second step I get:
BEGIN:VCARD
PHOTO:...
END:VCARD
There is no guarantee that this entry is for the same contact, is it?
For example, "John Doe" might have been removed in the meantime, to be
replaced by someone else. The API description also makes no guarantees
about the ordering of entries.
One could request additional data in the second step (and perhaps phones
will send it anyway), but doing the matching based on that anew is
expensive.
Would it be acceptable to extend PullAll() semantics? If the caller
gives the name of an existing target directory, obexd could write each
contact into a separate file in that directory, using a <number>.vcf
naming scheme.
Alternatively, the content of each vcard could be extended with a
"X-OBEXD-PBAP-NUMBER:x" where x is the number of that vcard.
(*) If there was a way to request only contacts which have a photo, that
would be used here. Is there something like it? Without it, the entire
address book needs to be read again with a filter that includes PHOTO
data and nothing else.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
^ permalink raw reply
* Re: Re : BLE -Multiple connection
From: Anderson Lizardo @ 2012-08-22 16:01 UTC (permalink / raw)
To: Ajay KV; +Cc: linux-bluetooth
In-Reply-To: <503568C5.4040000@globaledgesoft.com>
Hi Ajay,
On Wed, Aug 22, 2012 at 7:18 PM, Ajay KV <ajay.kv@globaledgesoft.com> wrote:
> First of all, sorry for my ignorance . Following are the steps i did
> in my test case. i am also using dual mode dongles , so as you said spec
> is not allowing us to do this operation
> [snip]
I remembered that the Core spec mentions that it is not required for a
controller to support more than one connection. Maybe it is the case
for the dongles you have? In this case, the controller will not allow
to send connectable advertising because of the active connection.
(sorry, I could not find the section of the spec that mentions this)
If you close the connection, are you able to run "hciconfig hci0
leadv" again on "A"?
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: BLE - multiple connections
From: Vinicius Costa Gomes @ 2012-08-22 15:13 UTC (permalink / raw)
To: Ajay KV; +Cc: Anderson Lizardo, linux-bluetooth
In-Reply-To: <503503C1.6060805@globaledgesoft.com>
Hi,
On 12:07 Wed 22 Aug, Ajay KV wrote:
> On 08/17/2012 12:27 PM, Anderson Lizardo wrote:
> >Hi Ajay
> >
> >[your emails to BlueZ mailing list may be getting blocked because you
> >are using HTML. Please fix that on your mail client. Also remember to
> >not top-post]
> >
> >On Fri, Aug 17, 2012 at 7:34 PM, Ajay KV<ajay.kv@globaledgesoft.com> wrote:
> >>Thanks for your reply. i have 2 dongles of different made (iogear and
> >>chirago) . "Hciconfig hci0 lestates" gives this following result
> >>
> >> Supported link layer states:
> >>[...]
> >>
> >> Here my issue is to know whether (connected mode + advertising
> >>mode) is possible at a time . Once A is connected to B (initiator), i
> >>could not able to enable advertise in A (getting command disallowed ) , so
> >>that device C is not getting the connectable advertising packets on
> >>scanning.
> >I could not see any reason to get "command disallowed" when enabling
> >LE adv. Did you try changing dongle combinations to make sure it is
> >not some hardware issue?
> >
> >Regards,
>
> Hi,
> i could not able to figure it out yet , why its showing
> this error " command disallowed" . Here im using fedora 13 with
> kernel 2.6.39 . i tried with both dongles , iogear and cirago . But
> the result is same. i dont think that its something wrong with
> kernel version, as we uses hci commands (hciconfig hci0 leadv )
> that directly talks with controller . please clear this issue..
Most probably the controller is in the page scan or inquiry scan state.
Try doing 'hciconfig hci0 noscan', before enabling advertising.
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
--
Vinicius
^ permalink raw reply
* Re: [PATCH obexd v0] client-doc: Guarantee prefix in transfer paths
From: Luiz Augusto von Dentz @ 2012-08-22 12:52 UTC (permalink / raw)
To: Patrick Ohly; +Cc: Mikel Astiz, linux-bluetooth, Mikel Astiz
In-Reply-To: <1345637062.29868.313.camel@pohly-mobl1.fritz.box>
Hi Patrick,
On Wed, Aug 22, 2012 at 3:04 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Wed, 2012-08-22 at 14:39 +0300, Luiz Augusto von Dentz wrote:
>> Hi Patrick,
>>
>> On Tue, Aug 21, 2012 at 11:31 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
>> >> diff --git a/doc/client-api.txt b/doc/client-api.txt
>> >> index 839a78c..7ca65cc 100644
>> >> --- a/doc/client-api.txt
>> >> +++ b/doc/client-api.txt
>> >> @@ -466,7 +466,7 @@ Transfer hierarchy
>> >>
>> >> Service org.bluez.obex.client
>> >> Interface org.bluez.obex.Transfer
>> >> -Object path [variable prefix]/{transfer0,transfer1,...}
>> >> +Object path [variable prefix]/{session0,session1,...}/{transfer0,...}
>> >>
>> >> Methods dict GetProperties()
>> >
>> > It would be even better to explicitly mention that the "[variable
>> > prefix]" here is the same as the "[variable prefix]" in the Session.
>> >
>> > Or perhaps change it like this?
>> >
>> > -Object path [variable prefix]/{transfer0,transfer1,...}
>> > +Object path [session prefix]/{transfer0,...}
>>
>> Hmm, I prefer the original proposal since that imo looks more clear
>> how the path is formatted, anyway the point here is that the transfers
>> are tied to sessions.
>
> But as Marcel said, "variable prefix" means "no guarantee made about its
> content, none whatsoever". Without knowing Bluez conventions, that was
> also my understand when reading the API description. Assigning some
> other meaning to "variable prefix" would break with developer
> expectations.
But we are not changing that, that why we return the object path to
you, you will never have to guess it.
> Unless the meaning of "variable prefix" gets redefined, "[variable
> prefix]/{session0,session1,...}" still doesn't allow the developer to do
> path_namespace filtering.
Im not sure why this is important? Prefix or variable prefix it does
not matter, you will not be able to hardcode nor you should be
constructing a path based on the session prefix, so if you are
planning to do prefix matching I would strongly advise not to.
> How about this:
>
> -Object path [variable prefix]/{transfer0,transfer1,...}
> +Object path [prefix]/{session0,session1,...}/{transfer0,...}"
> + with "prefix" as in the corresponding session
>
>> > Using this knowledge efficiently is not always possible, however. D-Bus
>> > itself has a "path_namespace" filter [1], but many D-Bus bindings don't
>> > expose it (Python [2], GIO D-Bus [3]). Therefore a client using those
>> > bindings still has to receive all Transfer signals and do its own
>> > filtering.
>> >
>> > [1] http://dbus.freedesktop.org/doc/dbus-specification.html#message-bus-routing-match-rules
>> > [2] http://dbus.freedesktop.org/doc/dbus-python/api/dbus.connection.Connection-class.html#add_signal_receiver
>> > [3] http://developer.gnome.org/gio/stable/GDBusConnection.html#g-dbus-connection-signal-subscribe
>>
>> Apparently people are not doing a good job with bindings then, arg
>> matching is quite essential to things like NameOwnerChanged, anyway
>> for what is worth even properties have this race condition with values
>> changing in meantime while GetAll/Get/GetProperties is returning, so
>> it seems we need to solve this problem by making the transfer
>> properties unlikely to change so the application have enough time to
>> subscribe to signals.
>
> "unlikely" doesn't sit well with me. An unlikely failure is still a
> failure. Tell me if I'm too pedantic here ;-)
I said unlikely because we cannot guarantee that we dont get some
unexpected event e.g. remote device crash/disconnects.
> I can imagine three ways to address race conditions:
> 1. Clients subscribe to signals in advance, before any are sent.
Can be done already, but it means you have to listen to every transfer
which can cause you application to wakeup a little too much.
> 2. Clients subscribe to signals later *and* check the current state.
> 3. Clients provide a callback object with methods which get invoked by
> obexd.
>
> The second option increases traffic and depends on being able to check
> the current state. Currently this approach is not possible for Transfer
> objects because they can get removed before the client checked their
> state.
I don't thing this is true, we do return the properties of the
transfer so you don't need to check the current state just listen for
changes, btw if you mean a method call for checking the current state
it does happen to have the same race conditions.
> The third option has the drawback that there's no standard way of
> letting the client declare which of these methods it wants to have
> called (in contrast to D-Bus signals). The advantage is that it becomes
> possible to clean up temp files after completion of the transfer: add an
> explicit "here's your result" callback method and when that fails,
> delete the temporary file.
We normally used the term agent for this type of object that is
registered to act as callback mechanism, in fact this was used before
and was just replaced with the current design recently as it was
considered more complicated to the client application to implement and
did not allow any other application like a download manager to listen
for transfer progress.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH obexd v0] client-doc: Guarantee prefix in transfer paths
From: Mikel Astiz @ 2012-08-22 12:42 UTC (permalink / raw)
To: Patrick Ohly; +Cc: Luiz Augusto von Dentz, linux-bluetooth, Mikel Astiz
In-Reply-To: <1345637062.29868.313.camel@pohly-mobl1.fritz.box>
Hi Patrick,
On Wed, Aug 22, 2012 at 2:04 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Wed, 2012-08-22 at 14:39 +0300, Luiz Augusto von Dentz wrote:
>> Hi Patrick,
>>
>> On Tue, Aug 21, 2012 at 11:31 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
>> >> diff --git a/doc/client-api.txt b/doc/client-api.txt
>> >> index 839a78c..7ca65cc 100644
>> >> --- a/doc/client-api.txt
>> >> +++ b/doc/client-api.txt
>> >> @@ -466,7 +466,7 @@ Transfer hierarchy
>> >>
>> >> Service org.bluez.obex.client
>> >> Interface org.bluez.obex.Transfer
>> >> -Object path [variable prefix]/{transfer0,transfer1,...}
>> >> +Object path [variable prefix]/{session0,session1,...}/{transfer0,...}
>> >>
>> >> Methods dict GetProperties()
>> >
>> > It would be even better to explicitly mention that the "[variable
>> > prefix]" here is the same as the "[variable prefix]" in the Session.
>> >
>> > Or perhaps change it like this?
>> >
>> > -Object path [variable prefix]/{transfer0,transfer1,...}
>> > +Object path [session prefix]/{transfer0,...}
>>
>> Hmm, I prefer the original proposal since that imo looks more clear
>> how the path is formatted, anyway the point here is that the transfers
>> are tied to sessions.
>
> But as Marcel said, "variable prefix" means "no guarantee made about its
> content, none whatsoever". Without knowing Bluez conventions, that was
> also my understand when reading the API description. Assigning some
> other meaning to "variable prefix" would break with developer
> expectations.
>
> Unless the meaning of "variable prefix" gets redefined, "[variable
> prefix]/{session0,session1,...}" still doesn't allow the developer to do
> path_namespace filtering.
>
> How about this:
>
> -Object path [variable prefix]/{transfer0,transfer1,...}
> +Object path [prefix]/{session0,session1,...}/{transfer0,...}"
> + with "prefix" as in the corresponding session
>From my point of view, this is beyond the scope of this patch. The
modification proposed here is in general consistent with the existing
documentation in the mentioned projects, and in particular with the
file being modified (see org.bluez.obex.Message).
I agree that the exact interpretation of "variable prefix" is
undocumented, but fixing that has a broad scope affecting several
projects. See for example org.bluez.Device or org.ofono.VoiceCall.
Personally, I find it relatively obvious that both variable prefixes
match. Otherwise there would be no reference to the session (or
BlueZ-adapter, oFono-modem, etc., depending on the case).
Regarding the lack of support in some bindings, I would say it's not
critical. After all, we're doing an optimization here (reduce number
of context-switches). If someone is using a limited binding, then
there's a performance penalty.
Cheers,
Mikel
^ permalink raw reply
* Re: Re : BLE -Multiple connection
From: Anderson Lizardo @ 2012-08-22 12:24 UTC (permalink / raw)
To: Ajay KV; +Cc: linux-bluetooth
In-Reply-To: <503545C5.80607@globaledgesoft.com>
Hi Ajay,
On Wed, Aug 22, 2012 at 4:49 PM, Ajay KV <ajay.kv@globaledgesoft.com> wrote:
> I think we already discussed about this issue . following is
> our previous conversation. please go through it
Please, try to disable HTML on your email client and send plain text
e-mails. It is difficult to follow your discussion with all this HTML.
It is very likely people are not helping you because your emails are
unreadable for a lot of people here.
Much information is missing from your previous messages:
1) You did not provide the exact steps you followed (which commands
you run after plugging the bluetooth dongle). Therefore it is
difficult to reproduce your tests.
2) Your HCI dump output shows just the command giving error. This is
not enough, you should provide the entire dump since the adapter was
plugged, including the first "B to A" connection you described, and
the second LE advertising attempt, otherwise it is not possible to
identify at which state the LE controller is when the LE adv. enable
command is issued.
Also, do you realize that you are not allowed by the current Core spec
v4.0 to have one dual mode device connected to another dual mode over
the LE link? The spec says that a dual mode device in "connectable
mode" (like device "A" in you original scenario) shall not be issuing
connectable advertising over the LE channel. This is not enforced by
the controller (and we actually do this for testing BlueZ to BlueZ
connections over LE), but the spec explicitly forbids it.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH obexd v0] client-doc: Guarantee prefix in transfer paths
From: Patrick Ohly @ 2012-08-22 12:04 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Mikel Astiz, linux-bluetooth, Mikel Astiz
In-Reply-To: <CABBYNZLUS-BrZ=FN28gLgrA=1vS1JA-exECd3xgjj2EfPrAuyw@mail.gmail.com>
On Wed, 2012-08-22 at 14:39 +0300, Luiz Augusto von Dentz wrote:
> Hi Patrick,
>
> On Tue, Aug 21, 2012 at 11:31 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> >> diff --git a/doc/client-api.txt b/doc/client-api.txt
> >> index 839a78c..7ca65cc 100644
> >> --- a/doc/client-api.txt
> >> +++ b/doc/client-api.txt
> >> @@ -466,7 +466,7 @@ Transfer hierarchy
> >>
> >> Service org.bluez.obex.client
> >> Interface org.bluez.obex.Transfer
> >> -Object path [variable prefix]/{transfer0,transfer1,...}
> >> +Object path [variable prefix]/{session0,session1,...}/{transfer0,...}
> >>
> >> Methods dict GetProperties()
> >
> > It would be even better to explicitly mention that the "[variable
> > prefix]" here is the same as the "[variable prefix]" in the Session.
> >
> > Or perhaps change it like this?
> >
> > -Object path [variable prefix]/{transfer0,transfer1,...}
> > +Object path [session prefix]/{transfer0,...}
>
> Hmm, I prefer the original proposal since that imo looks more clear
> how the path is formatted, anyway the point here is that the transfers
> are tied to sessions.
But as Marcel said, "variable prefix" means "no guarantee made about its
content, none whatsoever". Without knowing Bluez conventions, that was
also my understand when reading the API description. Assigning some
other meaning to "variable prefix" would break with developer
expectations.
Unless the meaning of "variable prefix" gets redefined, "[variable
prefix]/{session0,session1,...}" still doesn't allow the developer to do
path_namespace filtering.
How about this:
-Object path [variable prefix]/{transfer0,transfer1,...}
+Object path [prefix]/{session0,session1,...}/{transfer0,...}"
+ with "prefix" as in the corresponding session
> > Using this knowledge efficiently is not always possible, however. D-Bus
> > itself has a "path_namespace" filter [1], but many D-Bus bindings don't
> > expose it (Python [2], GIO D-Bus [3]). Therefore a client using those
> > bindings still has to receive all Transfer signals and do its own
> > filtering.
> >
> > [1] http://dbus.freedesktop.org/doc/dbus-specification.html#message-bus-routing-match-rules
> > [2] http://dbus.freedesktop.org/doc/dbus-python/api/dbus.connection.Connection-class.html#add_signal_receiver
> > [3] http://developer.gnome.org/gio/stable/GDBusConnection.html#g-dbus-connection-signal-subscribe
>
> Apparently people are not doing a good job with bindings then, arg
> matching is quite essential to things like NameOwnerChanged, anyway
> for what is worth even properties have this race condition with values
> changing in meantime while GetAll/Get/GetProperties is returning, so
> it seems we need to solve this problem by making the transfer
> properties unlikely to change so the application have enough time to
> subscribe to signals.
"unlikely" doesn't sit well with me. An unlikely failure is still a
failure. Tell me if I'm too pedantic here ;-)
I can imagine three ways to address race conditions:
1. Clients subscribe to signals in advance, before any are sent.
2. Clients subscribe to signals later *and* check the current state.
3. Clients provide a callback object with methods which get invoked by
obexd.
The second option increases traffic and depends on being able to check
the current state. Currently this approach is not possible for Transfer
objects because they can get removed before the client checked their
state.
The third option has the drawback that there's no standard way of
letting the client declare which of these methods it wants to have
called (in contrast to D-Bus signals). The advantage is that it becomes
possible to clean up temp files after completion of the transfer: add an
explicit "here's your result" callback method and when that fails,
delete the temporary file.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
^ permalink raw reply
* Re: [PATCH obexd v0] client-doc: Guarantee prefix in transfer paths
From: Luiz Augusto von Dentz @ 2012-08-22 11:39 UTC (permalink / raw)
To: Patrick Ohly; +Cc: Mikel Astiz, linux-bluetooth, Mikel Astiz
In-Reply-To: <1345537878.29868.185.camel@pohly-mobl1.fritz.box>
Hi Patrick,
On Tue, Aug 21, 2012 at 11:31 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
>> diff --git a/doc/client-api.txt b/doc/client-api.txt
>> index 839a78c..7ca65cc 100644
>> --- a/doc/client-api.txt
>> +++ b/doc/client-api.txt
>> @@ -466,7 +466,7 @@ Transfer hierarchy
>>
>> Service org.bluez.obex.client
>> Interface org.bluez.obex.Transfer
>> -Object path [variable prefix]/{transfer0,transfer1,...}
>> +Object path [variable prefix]/{session0,session1,...}/{transfer0,...}
>>
>> Methods dict GetProperties()
>
> It would be even better to explicitly mention that the "[variable
> prefix]" here is the same as the "[variable prefix]" in the Session.
>
> Or perhaps change it like this?
>
> -Object path [variable prefix]/{transfer0,transfer1,...}
> +Object path [session prefix]/{transfer0,...}
Hmm, I prefer the original proposal since that imo looks more clear
how the path is formatted, anyway the point here is that the transfers
are tied to sessions.
> Using this knowledge efficiently is not always possible, however. D-Bus
> itself has a "path_namespace" filter [1], but many D-Bus bindings don't
> expose it (Python [2], GIO D-Bus [3]). Therefore a client using those
> bindings still has to receive all Transfer signals and do its own
> filtering.
>
> [1] http://dbus.freedesktop.org/doc/dbus-specification.html#message-bus-routing-match-rules
> [2] http://dbus.freedesktop.org/doc/dbus-python/api/dbus.connection.Connection-class.html#add_signal_receiver
> [3] http://developer.gnome.org/gio/stable/GDBusConnection.html#g-dbus-connection-signal-subscribe
Apparently people are not doing a good job with bindings then, arg
matching is quite essential to things like NameOwnerChanged, anyway
for what is worth even properties have this race condition with values
changing in meantime while GetAll/Get/GetProperties is returning, so
it seems we need to solve this problem by making the transfer
properties unlikely to change so the application have enough time to
subscribe to signals.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: BLE - multiple connections
From: Anderson Lizardo @ 2012-08-22 10:40 UTC (permalink / raw)
To: Ajay KV; +Cc: linux-bluetooth
In-Reply-To: <503503C1.6060805@globaledgesoft.com>
Hi Ajay,
On Wed, Aug 22, 2012 at 12:07 PM, Ajay KV <ajay.kv@globaledgesoft.com> wrote:
> i could not able to figure it out yet , why its showing this error
> " command disallowed" . Here im using fedora 13 with kernel 2.6.39 . i tried
This kernel is too old. For sure a lot of LE stuff will not work
correctly on it. Can you try compiling your own kernel (latest
version) and use tha latest BlueZ version with it?
> with both dongles , iogear and cirago . But the result is same. i dont
> think that its something wrong with kernel version, as we uses hci
> commands (hciconfig hci0 leadv ) that directly talks with controller .
> please clear this issue..
Sorry, but I ran out of ideas. Can you describe with more details
which exact steps are you taking to get the "command disallowed" HCI
error? I.e. right after you attach the dongle, which commands are you
issuing.
Also an hcidump showing the entire session (since the dongle is
attached) should help. Without more information we are not unable to
help further.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* [PATCH hcidump 2/2] core: set HCI channel explicitly when bind
From: chanyeol.park @ 2012-08-22 9:20 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1345627245-19333-1-git-send-email-chanyeol.park@samsung.com>
From: Chan-yeol Park <chanyeol.park@samsung.com>
---
src/hcidump.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/hcidump.c b/src/hcidump.c
index 089d444..7423cdd 100644
--- a/src/hcidump.c
+++ b/src/hcidump.c
@@ -590,8 +590,10 @@ static int open_socket(int dev, unsigned long flags)
memset(&addr, 0, sizeof(addr));
addr.hci_family = AF_BLUETOOTH;
addr.hci_dev = dev;
+ addr.hci_channel = HCI_CHANNEL_RAW;
+
if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
- printf("Can't attach to device hci%d. %s(%d)\n",
+ printf("Can't attach to device hci%d. %s(%d)\n",
dev, strerror(errno), errno);
return -1;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH hcidump 1/2] lib: update mgmt control channel id
From: chanyeol.park @ 2012-08-22 9:20 UTC (permalink / raw)
To: linux-bluetooth
From: Chan-yeol Park <chanyeol.park@samsung.com>
kernel side has updated its control channel id from 1 to 3.
---
lib/hci.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/hci.h b/lib/hci.h
index 3dbb221..2c573a4 100644
--- a/lib/hci.h
+++ b/lib/hci.h
@@ -2279,8 +2279,8 @@ struct sockaddr_hci {
#define HCI_DEV_NONE 0xffff
#define HCI_CHANNEL_RAW 0
-#define HCI_CHANNEL_CONTROL 1
#define HCI_CHANNEL_MONITOR 2
+#define HCI_CHANNEL_CONTROL 3
struct hci_filter {
uint32_t type_mask;
--
1.7.9.5
^ permalink raw reply related
* [PATCH 3/3] network: Fix bnep setup failure handle bug
From: chanyeol.park @ 2012-08-22 9:20 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1345627238-19290-1-git-send-email-chanyeol.park@samsung.com>
From: Chan-yeol Park <chanyeol.park@samsung.com>
This patch initializes rsp varailbe again for the remain setup failure
procedure.
---
profiles/network/server.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 9bf2a02..77a8844 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -340,7 +340,7 @@ static uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req,
return BNEP_CONN_INVALID_SVC;
}
- return 0;
+ return BNEP_SUCCESS;
}
static void session_free(void *data)
@@ -422,6 +422,8 @@ static gboolean bnep_setup(GIOChannel *chan,
if (rsp)
goto reply;
+ rsp = BNEP_CONN_NOT_ALLOWED;
+
ns = find_server(na->servers, dst_role);
if (!ns) {
error("Server unavailable: (0x%x)", dst_role);
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/3] core: replace NULL by dbus invalid arguments
From: chanyeol.park @ 2012-08-22 9:20 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1345627238-19290-1-git-send-email-chanyeol.park@samsung.com>
From: Chan-yeol Park <chanyeol.park@samsung.com>
This patch replaces NULL with dbus invalid arguments return
if dbus method call gets invalid arguments from the peer.
This will avoid dbus peer coommander's infinite wait.
---
audio/media.c | 4 ++--
audio/transport.c | 4 ++--
plugins/service.c | 8 ++++----
profiles/network/connection.c | 2 +-
profiles/network/server.c | 4 ++--
src/adapter.c | 4 ++--
src/manager.c | 2 +-
7 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/audio/media.c b/audio/media.c
index ea6d582..b0ea4e9 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -926,7 +926,7 @@ static DBusMessage *unregister_endpoint(DBusConnection *conn, DBusMessage *msg,
if (!dbus_message_get_args(msg, NULL,
DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID))
- return NULL;
+ return btd_error_invalid_args(msg);
sender = dbus_message_get_sender(msg);
@@ -1808,7 +1808,7 @@ static DBusMessage *unregister_player(DBusConnection *conn, DBusMessage *msg,
if (!dbus_message_get_args(msg, NULL,
DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID))
- return NULL;
+ return btd_error_invalid_args(msg);
sender = dbus_message_get_sender(msg);
diff --git a/audio/transport.c b/audio/transport.c
index 832ad2a..d40c92d 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -667,7 +667,7 @@ static DBusMessage *acquire(DBusConnection *conn, DBusMessage *msg,
if (!dbus_message_get_args(msg, NULL,
DBUS_TYPE_STRING, &accesstype,
DBUS_TYPE_INVALID))
- return NULL;
+ return btd_error_invalid_args(msg);
sender = dbus_message_get_sender(msg);
@@ -704,7 +704,7 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
if (!dbus_message_get_args(msg, NULL,
DBUS_TYPE_STRING, &accesstype,
DBUS_TYPE_INVALID))
- return NULL;
+ return btd_error_invalid_args(msg);
sender = dbus_message_get_sender(msg);
diff --git a/plugins/service.c b/plugins/service.c
index 9f7bdb7..f16abe7 100644
--- a/plugins/service.c
+++ b/plugins/service.c
@@ -422,7 +422,7 @@ static DBusMessage *update_xml_record(DBusConnection *conn,
DBUS_TYPE_UINT32, &handle,
DBUS_TYPE_STRING, &record,
DBUS_TYPE_INVALID) == FALSE)
- return NULL;
+ return btd_error_invalid_args(msg);
len = (record ? strlen(record) : 0);
if (len == 0)
@@ -475,7 +475,7 @@ static DBusMessage *add_service_record(DBusConnection *conn,
if (dbus_message_get_args(msg, NULL,
DBUS_TYPE_STRING, &record, DBUS_TYPE_INVALID) == FALSE)
- return NULL;
+ return btd_error_invalid_args(msg);
sender = dbus_message_get_sender(msg);
err = add_xml_record(conn, sender, serv_adapter, record, &handle);
@@ -509,7 +509,7 @@ static DBusMessage *remove_service_record(DBusConnection *conn,
if (dbus_message_get_args(msg, NULL, DBUS_TYPE_UINT32, &handle,
DBUS_TYPE_INVALID) == FALSE)
- return NULL;
+ return btd_error_invalid_args(msg);
sender = dbus_message_get_sender(msg);
@@ -582,7 +582,7 @@ static DBusMessage *request_authorization(DBusConnection *conn,
if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_UINT32, &handle,
DBUS_TYPE_INVALID) == FALSE)
- return NULL;
+ return btd_error_invalid_args(msg);
sender = dbus_message_get_sender(msg);
if (find_pending_by_sender(serv_adapter, sender))
diff --git a/profiles/network/connection.c b/profiles/network/connection.c
index 59423a9..321f567 100644
--- a/profiles/network/connection.c
+++ b/profiles/network/connection.c
@@ -400,7 +400,7 @@ static DBusMessage *connection_connect(DBusConnection *conn,
if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &svc,
DBUS_TYPE_INVALID) == FALSE)
- return NULL;
+ return btd_error_invalid_args(msg);
id = bnep_service_id(svc);
nc = find_connection(peer->connections, id);
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 8ae608c..9bf2a02 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -607,7 +607,7 @@ static DBusMessage *register_server(DBusConnection *conn,
if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &uuid,
DBUS_TYPE_STRING, &bridge, DBUS_TYPE_INVALID))
- return NULL;
+ return btd_error_invalid_args(msg);
if (g_strcmp0(uuid, "nap"))
return btd_error_failed(msg, "Invalid UUID");
@@ -642,7 +642,7 @@ static DBusMessage *unregister_server(DBusConnection *conn,
if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &uuid,
DBUS_TYPE_INVALID))
- return NULL;
+ return btd_error_invalid_args(msg);
if (g_strcmp0(uuid, "nap"))
return btd_error_failed(msg, "Invalid UUID");
diff --git a/src/adapter.c b/src/adapter.c
index b642e37..69ec821 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1574,7 +1574,7 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,
if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_STRING, &capability, DBUS_TYPE_INVALID))
- return NULL;
+ return btd_error_invalid_args(msg);
if (adapter->agent)
return btd_error_already_exists(msg);
@@ -1604,7 +1604,7 @@ static DBusMessage *unregister_agent(DBusConnection *conn, DBusMessage *msg,
if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID))
- return NULL;
+ return btd_error_invalid_args(msg);
name = dbus_message_get_sender(msg);
diff --git a/src/manager.c b/src/manager.c
index 4a39461..738bf0b 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -96,7 +96,7 @@ static DBusMessage *find_adapter(DBusConnection *conn,
if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &pattern,
DBUS_TYPE_INVALID))
- return NULL;
+ return btd_error_invalid_args(msg);
/* hci_devid() would make sense to use here, except it is
* restricted to devices which are up */
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/3] audio: Remove unused enum values
From: chanyeol.park @ 2012-08-22 9:20 UTC (permalink / raw)
To: linux-bluetooth
From: Chan-yeol Park <chanyeol.park@samsung.com>
---
audio/manager.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/audio/manager.c b/audio/manager.c
index 999124d..9db7479 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -69,23 +69,6 @@
#include "sdpd.h"
#include "telephony.h"
-typedef enum {
- HEADSET = 1 << 0,
- GATEWAY = 1 << 1,
- SINK = 1 << 2,
- SOURCE = 1 << 3,
- CONTROL = 1 << 4,
- TARGET = 1 << 5,
- INVALID = 1 << 6
-} audio_service_type;
-
-typedef enum {
- GENERIC_AUDIO = 0,
- ADVANCED_AUDIO,
- AV_REMOTE,
- GET_RECORDS
-} audio_sdp_state_t;
-
struct audio_adapter {
struct btd_adapter *btd_adapter;
gboolean powered;
--
1.7.9.5
^ permalink raw reply related
* Bluez/Obexd upstream test result_20120822(bluez-4.101.207+ obexd-0.47.22)
From: Li, XiaX @ 2012-08-22 3:12 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org; +Cc: zhouzhy@neusoft.com
[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]
Hi all,
QA finished upstream testing. The test is for bluez-4.101.207 and obexd-0.47.22. Against last, BlueZ is changed from 4.101.178 to 4.101.207, Obexd is changed from 0.47.7 to 0.47.22. Some dbus interface has been changed.
In this test, totally ran 100 case: 77 Pass, 5 Fail. Other 18 are blocked or unavailable (some cases validation method is still in investigating).
The pass rate is 94% (pass / <pass + fail>). Pulseaudio has not caught up with the BlueZ interface changing in HFP profile.
New bugs:
===============================================
No
Re-open bugs:
===============================================
No
Verified bugs:
===============================================
No
Testing Environment
==============================================
Hardware: netbook Eeepc 1005HA | Acer AspireOne NAV50
Image: netbook-ia32-pinetrail-tizen_20120424.2
Linux Kernel: v3.4-rc7
bluez-4.101.207.g46c5967-1.1.i586
obexd-0.47.22.g671e521-1.1.i586
obexd-server-0.47.22.g671e521-1.1.i586
connman-1.5.3.ga669c2d-1.1.i586
ofono-1.9.11.g9f7087a-1.1.i586
Pulseaudio: d3cd82d411f9b574678870d8ccd836032a238684 (July, 9th)
For detailed test results, please see attached file.
Thanks
Li Xia
[-- Attachment #2: Bluetooth_upstream_quality_20120822.txt --]
[-- Type: text/plain, Size: 7323 bytes --]
== Detail Result ==
[PASS]: case passed <br>
[FAIL]: case failed <br>
[Block]: case is blocked by one bug <br>
[N/A]: case not available to be tested
=== Audio-A2DP ===
[PASS] A2DP_001: audio sink connect/re-connect
[PASS] A2DP_002: stereo headset playback
[PASS] A2DP_003: stereo headset voice record
[PASS] A2DP_004: Codec support: SBC
[PASS] A2DP_005: Playback music on line
[PASS] A2DP_006: SINK role, as speaker for other device music playing
With some BT headset, we ever met issue [https://bugs.meego.com/show_bug.cgi?id=25480 Bug#25480 A2DP-source: pulseaudio fails to create bluez_sink and bluez_source]
=== Audio-AVRCP ===
[PASS] AVRCP_001: Volume Up/Down
[Block] AVRCP_002: Play/Pause (by unavailable media-player)
[Block] AVRCP_003: Next/Privious (by unavailable media-player)
[PASS] AVRCP_004: Connection Establish/Release
[N/A] AVRCP_005: Audio metadata get.
We still try to find avaliable media-player to support AVRCP-1.4
=== Audio-HFP ===
[PASS] HFP_001: RFCOMM connection on AG
[PASS] HFP_002: list (ofono) hfp modem
[PASS] HFP_003: HFP modem establishment (Ofono)
[FAIL] HFP_004: Audio SNK and SRC establishment
[FAIL] HFP_005: voicecall, audio creates BT SNK/SRC
[FAIL] HFP_006: redirect AG SNK/SRC to local SRC/SNK
BlueZ interface has some regression, bug is reported as [https://bugs.meego.com/show_bug.cgi?id=25473 Bug#25473 HFP: latest bluez cannot support pulseaudio to create bluez_sink and bluez_source]
Pulseaudio has new crash bug: [https://bugs.meego.com/show_bug.cgi?id=25593 Bug#25593 Pulseaudio crashes after connect BT sink to alsa source]
[PASS] HFP_007: Check AT-commands from HF part
[PASS] HFP_008: Connect BT Headset to HFP phone
=== Audio-HSP ===
[PASS] HSP_001: Use mono headset to play music
*** DUT is a Netbook, unable to take phone ***
[N/A] HSP_002: Take incoming call by button-press
[N/A] HSP_003: Audio transfer between AG and HS
[PASS] HSP_004: Adjust Volume Up/Down
=== OBEX-FTP ===
[PASS] FTP_001: pull/push files from/to server.
[PASS] FTP_002: browse server files
[PASS] FTP_003: Client "delete file", "rename file"
[PASS] FTP_004: Server enables FTP parameter
[PASS] FTP_005: Server sets sharing root path
[PASS] FTP_006: Server handles all requirements
[PASS] FTP_007: Server has ability to set permission for FTP
[PASS] FTP_008: FTP data-rate about 30k/s~230k/s
[PASS] FTP_009: Big size file transferred stable
=== OBEX-OPP ===
[PASS] OPP_001: pull un-patterned object from server.
[PASS] OPP_002: Server enables OPP parameter
[PASS] OPP_003: During object transferring, the progress is clear
=== OBEX-PBAP ===
[PASS] PBAP_001: Client gets phone book entries from server
[PASS] PBAP_002: Client gets ICH, OCH, MCH and CCH from server
[FAIL] PBAP_003: PSE can provide PBAP daemon by enabling corresponding parameter
Case failed due to [https://bugs.meego.com/show_bug.cgi?id=25189 Bug#25189, PBAP Server has segmentation fault during responding to client PullAll]
[PASS] PBAP_004: Both sides support vCard2.1/vCard3.0
=== OBEX-SYNC ===
[PASS] SYNC_001: Server enables SYNC daemon
[PASS] SYNC_002: During sync, Server can show "syncing..."
[PASS] SYNC_003: client can set PIM fetching from INT
[PASS] SYNC_004: Server get/put entire phonebook from/to client.
[PASS] SYNC_005: Client can support vCard2.1, vCard3.0
=== OBEX-MAP MCE ===
[FAIL] MAP_001: MCE can browse message/folder list on MSE
Regression happened on client side, bug is reported as [https://bugs.meego.com/show_bug.cgi?id=25595 Bug#25595 MAP client fail to get message list from Samsung GT-i9100]
[BLOCK] MAP_002: MCE can upload local message to MSE
[BLOCK] MAP_003: MCE can delete the message on MSE side
[BLOCK] MAP_004: MCE can take use of MSE to send message
=== Network-PAN ===
[PASS] PAN_001: PANU can init nap0 device connect to NAP
[PASS] PAN_002: PANU can get ip address or assigned static ip
[PASS] PAN_003: PANU can logon internet website
[N/A] PAN_004: NAP can init bridge bnep
[N/A] PAN_005: NAP can support one or multiple PANU connection
[N/A] PAN_006: NAP can have DHCP responding to each PANU
Current connman does not support DHCP functions.
=== Network-BNEP ===
[PASS] BNEP_001: Check BNEP support on DUT
=== Networl-DUN ===
[PASS] DUN_001: GW (DUT) can parse a series of AT commands from the data terminal.
[PASS] DUN_002: DT can build up rfcomm device by bluetooth and DUN modem by Ofono.
[PASS] DUN_003: DT can use Ofono to dial up special service number ("*99#").
[PASS] DUN_004: When network connected, DT can log on website in internet.
=== SIM-SAP ===
The SAP test method is still in investigation.
[N/A] SAP_001: Server can enable a module to register Client
[N/A] SAP_002: Server can power on/off SIM Card or reset it
[N/A] SAP_003: Server can disconnect Client
[N/A] SAP_004: Server can disconnect SIM
[N/A] SAP_005: Client can connect to Server SIM
[N/A] SAP_006: Client can power on Server SIM
[N/A] SAP_007: Client can disconnect Server
=== Generic-GAP ===
[PASS] GAP_001: PSCAN/ISCAN mode setting.
[PASS] GAP_002: Active pairing to another bluetooth device.
[PASS] GAP_003: Passive pairing, accepte pair master requirement
[PASS] GAP_004: SSP supports
[PASS] GAP_005: If lost link-key, it needs re-pair
[PASS] GAP_006: reboot DUT with link-key restored, no need to re-pair
[PASS] GAP_007: Pair can be released
=== Generic-SDP ===
[PASS] SDP_001: browse available service list in local
[PASS] SDP_002: browse available service list from remote
[PASS] SDP_003: browse service by RecHandle for detail info
[PASS] SDP_004: Server can answer SDP searching request
[PASS] SDP_005: Server can add/del/update service records
=== Others-HID ===
[PASS] HID_001: Host can search nearby HID device.
[PASS] HID_002: [BAT] Host can connect to HID device.
[PASS] HID_003: Host can handle multiple human input/output devices.
=== HCI ===
[PASS] HCI_001: Build hci connection between Dev_A and Dev_B.
[PASS] HCI_002: Receive ACL data with HCI.
[PASS] HCI_003: Send ACL data with HCI.
[PASS] HCI_004: Change name of remote Dev by HCI.
[PASS] HCI_005: Change class of remote Dev by HCI.
[PASS] HCI_006: Read local HCI controller information.
[PASS] HCI_007: Read remote HCI controller information.
=== L2CAP ===
[PASS] L2CAP_001: Build l2cap connection between Dev_A and Dev_B.
[PASS] L2CAP_002: Dev_A and Dev_B take l2cap protocol to do pingpong.
=== RFCOMM ===
[PASS] RFCOMM_001: Build rfcomm connection between Dev_A and Dev_B.
[PASS] RFCOMM_002: Dev_A and Dev_B take rfcomm protocol to do pingpong.
=== Settings ===
[PASS] SET_001: [BAT] No error output during bluetoothd startup process.
=== Bluetooth Utils ===
Some basic checking for hciconfig, hcitool, sdptool commands.
[PASS] BTCMD_001: hciconfig -a
[PASS] BTCMD_002: hciconfig <adapter> piscan
[PASS] BTCMD_003: hciconfig <adapter> up/down
[PASS] BTCMD_004: hcitool scan
[PASS] BTCMD_005: sdptool browse local
[PASS] BTCMD_006: sdptool browse <ermote MAC>
[PASS] BTCMD_007: sdptool add/del
^ permalink raw reply
* Re: [RFC] HFP support into oFono and BlueZ
From: Lucas De Marchi @ 2012-08-21 22:46 UTC (permalink / raw)
To: ofono; +Cc: Gustavo F. Padovan, linux-bluetooth, zhenhua.zhang
In-Reply-To: <ac8af0be1001202354i7f1a98dbw55c56fc65f878358@mail.gmail.com>
Hi Zhao,
On Thu, Jan 21, 2010 at 5:54 AM, Zhao Forrest <forrest.zhao@gmail.com> wrote:
> Your patch removes many DBus APIs provided by audio/gateway.c, which
> was originally designed to provide a set of DBus functions/signals for
> ease of application development. In other words, your patch is too
> oFono-specific, and is not a general support for HFP HF unit role in
> BlueZ IMO. The DBus interface provided by your patch is not easy to
> develop the application because the application developer has to parse
> the AT commands by himself. I know that oFono could parse AT commands.
> But I don't think it a common case for other application developers.
> Why not extend audio/gateway.c instead of removing its DBus APIs or
> create a new file to support HFP for oFono in blueZ?
>
> I think maybe Marcel would have more comments on this to give a clear direction.
First of all, please don't top-post to this list.
Applications developers don't have to parse the AT commands by
themselves but rather use a telephony stack that implements the DBus
APIs.
This is kind of an agent (as in the pairing process) that anyone
could implement. oFono implements it, so it's 1 option.
Lucas De Marchi
^ permalink raw reply
* Re: [PATCH] Bluetooth: mgmt: Fix returning a list with invalid indexes
From: Vinicius Costa Gomes @ 2012-08-21 19:12 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <20120821181029.GA14471@samus>
Hi,
On 15:10 Tue 21 Aug, Vinicius Costa Gomes wrote:
> Hi,
>
> On 20:59 Tue 14 Aug, Vinicius Costa Gomes wrote:
> > When a device is in the HCI_SETUP state, it was being counted as
> > a valid device, but it wasn't included in the list returned to
> > userspace.
> >
> > This can be reproduced when you add an adapter after doing
> > 'rfkill block all', for example.
> >
> > This commit also has the (intended) effect of making
> > 'btmgmt power on' work after the rfkill block has been lifted.
> >
> > Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
>
> bump :-)
Please ignore this patch.
As discussed on IRC, seems that the right solution would be to fix the
num_controllers parameters to not count devices in the HCI_SETUP state
and do a proper power on on the device when it leaves the rfkill blocked
state.
Cheers,
--
Vinicius
^ permalink raw reply
* Re: [PATCH 3/4] Bluetooth: Avoid rfcomm_session_timeout using freed pointer
From: Gustavo Padovan @ 2012-08-21 18:56 UTC (permalink / raw)
To: Dean Jenkins; +Cc: linux-bluetooth
In-Reply-To: <1344710830-10301-4-git-send-email-djenkins@mvista.com>
Hi Dean,
* Dean Jenkins <djenkins@mvista.com> [2012-08-11 19:47:09 +0100]:
> rfcomm_session_timeout() protects the scenario of the remote
> Bluetooth device failing to send a DISC on the rfcomm control
> channel after the last data DLC channel has been closed.
>
> There is a race condition between the timer expiring causing
> rfcomm_session_timeout() to run and the rfcomm session being
> deleted. If the rfcomm session is deleted then
> rfcomm_session_timeout() would use a freed rfcomm session
> pointer resulting in a potential kernel crash or memory corruption.
> Note the timer is cleared before the rfcomm session is deleted
> by del_timer() so the circumstances for a failure to occur are
> as follows:
>
> rfcomm_session_timeout() needs to be executing before the
> del_timer() is called to clear the timer but
> rfcomm_session_timeout() needs to be delayed from using the
> rfcomm session pointer until after the session has been deleted.
> Therefore, there is a very small window of opportunity for failure.
>
> The solution is to use del_timer_sync() instead of del_timer()
> as this ensures that rfcomm_session_timeout() is not running
> when del_timer_sync() returns. This means that it is not
> possible for rfcomm_session_timeout() to run after the rfcomm
> session has been deleted.
>
> Signed-off-by: Dean Jenkins <djenkins@mvista.com>
> ---
> net/bluetooth/rfcomm/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index 24d4d3c..c7921fd 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -264,8 +264,8 @@ static void rfcomm_session_clear_timer(struct rfcomm_session *s)
> {
> BT_DBG("session %p state %ld", s, s->state);
>
> - if (timer_pending(&s->timer))
> - del_timer(&s->timer);
> + /* ensure rfcomm_session_timeout() is not running past this point */
> + del_timer_sync(&s->timer);
I'm not happy of the idea of let the stack broken between patches 2 and 3
(this one). As you said if we use del_timer_sync() we don't need rfcnt here,
can you add this as a first patch, maybe? and after this continue to remove
the rest of the refcount code?
Gustavo
^ permalink raw reply
* Re: [PATCH 4/4] Bluetooth: On socket shutdown check rfcomm session and DLC exists
From: Gustavo Padovan @ 2012-08-21 18:50 UTC (permalink / raw)
To: Dean Jenkins; +Cc: linux-bluetooth
In-Reply-To: <1344710830-10301-5-git-send-email-djenkins@mvista.com>
Hi Dean,
* Dean Jenkins <djenkins@mvista.com> [2012-08-11 19:47:10 +0100]:
> A race condition exists between near simultaneous asynchronous
> DLC data channel disconnection requests from the host and remote device.
> This causes the socket layer to request a socket shutdown at the same time
> the rfcomm core is processing the disconnect request from the remote
> device.
>
> The socket layer retains a copy of a struct rfcomm_dlc d pointer.
> The d pointer refers to a copy of a struct rfcomm_session.
> When the socket layer thread performs a socket shutdown, the thread
> may wait on a rfcomm lock in rfcomm_dlc_close(). This means that
> whilst the thread waits, the rfcomm_session and/or rfcomm_dlc structures
> pointed to by d maybe freed due to rfcomm core handling. Consequently,
> when the rfcomm lock becomes available and the thread runs, a
> malfunction could occur as a freed rfcomm_session pointer and/or a
> freed d pointer will be erroneously reused.
>
> Therefore, after the rfcomm lock is acquired, check that the struct
> rfcomm_session is still valid by searching the rfcomm session list.
> If the session is valid then validate the d pointer by searching the
> rfcomm session list of active DLCs for the rfcomm_dlc structure
> pointed by d.
>
> If active DLCs exist when the rfcomm session is terminating,
> avoid a memory leak of rfcomm_dlc structures by ensuring that
> rfcomm_session_close() is used instead of rfcomm_session_del().
>
> Signed-off-by: Dean Jenkins <djenkins@mvista.com>
> ---
> net/bluetooth/rfcomm/core.c | 29 ++++++++++++++++++++++++++---
> 1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index c7921fd..1a7db34 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -496,11 +496,34 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
>
> int rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
> {
> - int r;
> + int r = 0;
> + struct rfcomm_dlc *d_list;
> + struct rfcomm_session *s, *s_list;
> + struct list_head *p, *n, *p2, *n2;
>
> rfcomm_lock();
>
> - r = __rfcomm_dlc_close(d, err);
> + s = d->session;
> + if (s) {
Please invert the check here, and add a goto unlock;. There too many
indentation levels here.
> + /* check the session still exists after waiting on the mutex */
> + list_for_each_safe(p, n, &session_list) {
> + s_list = list_entry(p, struct rfcomm_session, list);
please use list_for_each_entry(), the _safe version seems to not be needed.
> + if (s == s_list) {
> + /* check the dlc still exists */
> + /* after waiting on the mutex */
> + list_for_each_safe(p2, n2, &s->dlcs) {
> + d_list = list_entry(p2,
> + struct rfcomm_dlc,
> + list);
and here you can use rfcomm_dlc_get()
Gustavo
^ permalink raw reply
* Re: [PATCH] Bluetooth: mgmt: Fix returning a list with invalid indexes
From: Vinicius Costa Gomes @ 2012-08-21 18:10 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1344988788-16378-1-git-send-email-vinicius.gomes@openbossa.org>
Hi,
On 20:59 Tue 14 Aug, Vinicius Costa Gomes wrote:
> When a device is in the HCI_SETUP state, it was being counted as
> a valid device, but it wasn't included in the list returned to
> userspace.
>
> This can be reproduced when you add an adapter after doing
> 'rfkill block all', for example.
>
> This commit also has the (intended) effect of making
> 'btmgmt power on' work after the rfkill block has been lifted.
>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
bump :-)
Cheers,
--
Vinicius
^ permalink raw reply
* Re: [PATCHv1 4/4] Bluetooth: trivial: Use preferred method for NULL check
From: Gustavo Padovan @ 2012-08-21 17:55 UTC (permalink / raw)
To: Andrei Emeltchenko, linux-bluetooth
In-Reply-To: <20120815095721.GA5154@aemeltch-MOBL1>
* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-08-15 12:57:23 +0300]:
> Hi Gustavo,
>
> On Wed, Aug 15, 2012 at 01:16:55AM -0300, Gustavo Padovan wrote:
> > > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > >
> > > Use standard bluetooth way to check NULL pointer !var instead of
> > > var == NULL.
> > >
> > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > > ---
> > > net/bluetooth/af_bluetooth.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > I applied patches 2, 3 and 4 to bluetooth-next. Patch 1 needs rebase. Thanks.
>
> Patches 1 and 3 needs patch from linux-next and wireless-testing
I dropped them from the tree until this patch lands on bluetooth-next.
Gustavo
^ permalink raw reply
* Re: [PATCH] Bluetooth: Fix establishing ESCO links
From: Gustavo Padovan @ 2012-08-21 17:38 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1345250279-32158-1-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
* Vinicius Costa Gomes <vinicius.gomes@openbossa.org> [2012-08-17 21:37:59 -0300]:
> Commit 4cd2d98340b4f03d5532c30fdaeb451b035429cb "Bluetooth: Simplify
> the connection type handling" broke the creation of ESCO links.
>
> This patch adds a type parameter to hci_connect_sco() so it creates
> the connection of the right kind.
>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
Patch has been applied to bluetooth-next. Thanks.
Gustavo
^ permalink raw reply
* Re: [PATCH] Bluetooth: Use kref for l2cap channel reference counting
From: Gustavo Padovan @ 2012-08-21 17:34 UTC (permalink / raw)
To: Syam Sidhardhan; +Cc: linux-bluetooth, jaganath.k
In-Reply-To: <1343413282-8901-1-git-send-email-s.syam@samsung.com>
Hi Syam,
* Syam Sidhardhan <s.syam@samsung.com> [2012-07-27 23:51:21 +0530]:
> This patch changes the struct l2cap_chan and associated code to use
> kref api for object refcounting and freeing.
>
> Suggested-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
> Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
> ---
> include/net/bluetooth/l2cap.h | 3 +--
> net/bluetooth/l2cap_core.c | 15 ++++++++-------
> 2 files changed, 9 insertions(+), 9 deletions(-)
Patch has been applied to linux-bluetooth. Thanks.
Gustavo
^ permalink raw reply
* Re: [PATCH] Bluetooth: debug: Correct the PSM printing
From: Gustavo Padovan @ 2012-08-21 17:29 UTC (permalink / raw)
To: Syam Sidhardhan; +Cc: linux-bluetooth, jaganath.k
In-Reply-To: <1343413282-8901-2-git-send-email-s.syam@samsung.com>
Hi Syam,
* Syam Sidhardhan <s.syam@samsung.com> [2012-07-27 23:51:22 +0530]:
> Earlier we were printing chan->psm before assigning any value.
>
> Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
> ---
> net/bluetooth/l2cap_core.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Patch has been applied to bluetooth-next. 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