* [PATCHv4 3/3] Bluetooth: check L2CAP length in first ACL fragment
From: Emeltchenko Andrei @ 2010-09-15 11:28 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1284550124-31201-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Current Bluetooth code assembles fragments of big L2CAP packets
in l2cap_recv_acldata and then checks allowed L2CAP size in
assemled L2CAP packet (pi->imtu < skb->len).
The patch moves allowed L2CAP size check to the early stage when
we receive the first fragment of L2CAP packet. We do not need to
reserve and keep L2CAP fragments for bad packets.
Updated version after comments from Mat Martineau <mathewm@codeaurora.org>
and Gustavo Padovan <padovan@profusion.mobi>.
Trace below is received when using stress tools sending big
fragmented L2CAP packets.
...
[ 1712.798492] swapper: page allocation failure. order:4, mode:0x4020
[ 1712.804809] [<c0031870>] (unwind_backtrace+0x0/0xdc) from [<c00a1f70>]
(__alloc_pages_nodemask+0x4)
[ 1712.814666] [<c00a1f70>] (__alloc_pages_nodemask+0x47c/0x4d4) from
[<c00a1fd8>] (__get_free_pages+)
[ 1712.824645] [<c00a1fd8>] (__get_free_pages+0x10/0x3c) from [<c026eb5c>]
(__alloc_skb+0x4c/0xfc)
[ 1712.833465] [<c026eb5c>] (__alloc_skb+0x4c/0xfc) from [<bf28c738>]
(l2cap_recv_acldata+0xf0/0x1f8 )
[ 1712.843322] [<bf28c738>] (l2cap_recv_acldata+0xf0/0x1f8 [l2cap]) from
[<bf0094ac>] (hci_rx_task+0x)
...
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
net/bluetooth/l2cap.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index ce8f5e4..3aa2e9c 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4654,6 +4654,8 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl
if (flags & ACL_START) {
struct l2cap_hdr *hdr;
+ struct sock *sk;
+ u16 cid;
int len;
if (conn->rx_len) {
@@ -4673,6 +4675,7 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl
hdr = (struct l2cap_hdr *) skb->data;
len = __le16_to_cpu(hdr->len) + L2CAP_HDR_SIZE;
+ cid = __le16_to_cpu(hdr->cid);
if (len == skb->len) {
/* Complete frame received */
@@ -4689,6 +4692,19 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl
goto drop;
}
+ sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
+
+ if (sk && l2cap_pi(sk)->imtu < len - L2CAP_HDR_SIZE) {
+ BT_ERR("Frame exceeding recv MTU (len %d, MTU %d)",
+ len, l2cap_pi(sk)->imtu);
+ bh_unlock_sock(sk);
+ l2cap_conn_unreliable(conn, ECOMM);
+ goto drop;
+ }
+
+ if (sk)
+ bh_unlock_sock(sk);
+
/* Allocate skb for the complete frame (with header) */
conn->rx_skb = bt_skb_alloc(len, GFP_ATOMIC);
if (!conn->rx_skb)
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] Add handling of TITLE contact's field in vCard
From: Johan Hedberg @ 2010-09-15 12:49 UTC (permalink / raw)
To: Rafał Michalski; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikjKRY_ZBjNeyJrsSObb4qVNfNgdpdUzghmYrD8@mail.gmail.com>
Hi Rafal,
On Wed, Sep 15, 2010, Rafał Michalski wrote:
> From: Rafal Michalski <michalski.raf@gmail.com>
> Date: Wed, 15 Sep 2010 09:16:11 +0200
> Subject: [PATCH] Add handling of TITLE contact's field in vCard
>
> After pulling contacts this field wasn't presented in downloaded vCard
> structure in spite of that it existed (not empty). Now it shows downloaded
> TITLE field (using database query).
> ---
> plugins/phonebook-tracker.c | 23 ++++++++++++-----------
> plugins/vcard.c | 4 ++++
> plugins/vcard.h | 1 +
> 3 files changed, 17 insertions(+), 11 deletions(-)
Thanks for the patch. It's now upstream.
Johan
^ permalink raw reply
* Re: [RFC] D-Bus API for out of band association model
From: Claudio Takahasi @ 2010-09-15 13:30 UTC (permalink / raw)
To: Andrzej Kaczmarek, linux-bluetooth, par-gunnar.p.hjalmdahl
In-Reply-To: <20100915081047.GA15065@jh-x301>
On Wed, Sep 15, 2010 at 5:10 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Andrzej,
>
> On Wed, Sep 15, 2010, Andrzej Kaczmarek wrote:
>> On behalf of ST-Ericsson SA, below is our proposal of API for out of band
>> association model. To give you a bit more overview how this will be integrated
>> into BlueZ:
>>
>> .RequestRemoteOobData should be called from hcid_dbus_request_io_cap to
>> check if any data was exchange/retrieved from remote device. Returned data
>> will be stored (if any) and used to reply with proper IO capabilities as well
>> as to to return retrieved data. This will involve some changes in flow from
>> io_capa_request onwards.
>>
>> Please let me know if you have any comments. Implementation will follow.
>
> I'd prefer to do this completely internally in bluetoothd through a
> plugin. Do you see any reasons why not to do it that way? We still need
> some UI to call CreatePairedDevice but a plugin that knows about local
> platform specific OOB mechanisms (e.g. NFC) could provide the core
> daemon with enough info to set the IO capabilities correctly and provide
> the necessary OOB values.
>
> Johan
> --
Hi Andrzej,
I did some testing with OOB last year.
See my branch git://git.infradead.org/users/cktakahasi/bluez.git
oob-displayyesno
About the API changes. MAYBE, Hash and Randomizer could be only
adapter properties.
For the remote Hash and Randomizer still not clear to me what is the
best way to allow external apps to set those values, but in my opinion
BlueZ should not know the OOB mechanism, only the capability is
enough.
Regards,
Claudio
^ permalink raw reply
* Re: [PATCH 00/10] Media API
From: Johan Hedberg @ 2010-09-15 13:35 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1284387337-18822-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Mon, Sep 13, 2010, Luiz Augusto von Dentz wrote:
> Luiz Augusto von Dentz (10):
> Add media API documentation
> Add rule to enabling talking to org.bluez.MediaEndpoint
> Add option to enable/disable unix ipc via audio.conf
> Add support for media transport in gstreamer plugin
> Add simple-endpoint test script
> Add initial implementation of org.bluez.Media spec
> Introduce headset_get_inband
> Update a2dp transport delay when it changes
> Remove local cache for nrec and inband
> Add proper checks for MediaTransport.SetProperty
>
> Makefile.am | 6 +-
> audio/a2dp-codecs.h | 116 ++++++++
> audio/a2dp.c | 749 +++++++++++++++++++++++++++++++++++++++++--------
> audio/a2dp.h | 15 +
> audio/avdtp.c | 88 +++----
> audio/avdtp.h | 5 +-
> audio/gsta2dpsink.c | 33 ++-
> audio/gsta2dpsink.h | 1 +
> audio/gstavdtpsink.c | 609 +++++++++++++++++++++++++++++++++++++++-
> audio/gstavdtpsink.h | 6 +
> audio/headset.c | 10 +
> audio/headset.h | 1 +
> audio/main.c | 9 -
> audio/manager.c | 66 +++++
> audio/manager.h | 2 +
> audio/media.c | 689 +++++++++++++++++++++++++++++++++++++++++++++
> audio/media.h | 54 ++++
> audio/sink.c | 179 ++-----------
> audio/source.c | 174 ++----------
> audio/transport.c | 758 ++++++++++++++++++++++++++++++++++++++++++++++++++
> audio/transport.h | 36 +++
> audio/unix.c | 1 +
> doc/media-api.txt | 169 +++++++++++
> src/bluetooth.conf | 1 +
> test/simple-endpoint | 126 +++++++++
> 25 files changed, 3395 insertions(+), 508 deletions(-)
> create mode 100644 audio/a2dp-codecs.h
> create mode 100644 audio/media.c
> create mode 100644 audio/media.h
> create mode 100644 audio/transport.c
> create mode 100644 audio/transport.h
> create mode 100644 doc/media-api.txt
> create mode 100755 test/simple-endpoint
All patches have been pushed upstream now (with the few minor changes
that we discussed). Next step: talk to PulseAudio guys and get this
merged there too.
Johan
^ permalink raw reply
* Re: [RFC] D-Bus API for out of band association model
From: jaikumar Ganesh @ 2010-09-15 17:13 UTC (permalink / raw)
To: Claudio Takahasi
Cc: Andrzej Kaczmarek, linux-bluetooth, par-gunnar.p.hjalmdahl
In-Reply-To: <AANLkTimm++uSFQmkKHF+vHzp=Kc0tYpzb+xqq=eG6jnW@mail.gmail.com>
Hi,
On Wed, Sep 15, 2010 at 6:30 AM, Claudio Takahasi
<claudio.takahasi@openbossa.org> wrote:
> On Wed, Sep 15, 2010 at 5:10 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> Hi Andrzej,
>>
>> On Wed, Sep 15, 2010, Andrzej Kaczmarek wrote:
>>> On behalf of ST-Ericsson SA, below is our proposal of API for out of band
>>> association model. To give you a bit more overview how this will be integrated
>>> into BlueZ:
>>>
>>> .RequestRemoteOobData should be called from hcid_dbus_request_io_cap to
>>> check if any data was exchange/retrieved from remote device. Returned data
>>> will be stored (if any) and used to reply with proper IO capabilities as well
>>> as to to return retrieved data. This will involve some changes in flow from
>>> io_capa_request onwards.
>>>
>>> Please let me know if you have any comments. Implementation will follow.
>>
>> I'd prefer to do this completely internally in bluetoothd through a
>> plugin. Do you see any reasons why not to do it that way? We still need
>> some UI to call CreatePairedDevice but a plugin that knows about local
>> platform specific OOB mechanisms (e.g. NFC) could provide the core
>> daemon with enough info to set the IO capabilities correctly and provide
>> the necessary OOB values.
>>
>> Johan
>> --
>
> Hi Andrzej,
>
> I did some testing with OOB last year.
> See my branch git://git.infradead.org/users/cktakahasi/bluez.git
> oob-displayyesno
>
> About the API changes. MAYBE, Hash and Randomizer could be only
> adapter properties.
> For the remote Hash and Randomizer still not clear to me what is the
> best way to allow external apps to set those values, but in my opinion
> BlueZ should not know the OOB mechanism, only the capability is
> enough.
[Resending, because the mail to linux-bluetooth bounced. Sorry if some
folks got it twice]
We started to work on this API too last week. Not complete yet to post
to the mailing list.
The approach we took was this:
a) Since OOB authentication is just another means of authentication
compared to Passkey, Pin entry,
we added an agent API (similar to the lines of other
authentication mechanisms) that will ask for the Out Of Band data.
b) We added an API to the adapter to get the local Out of Band Data
c) Currently, RegisterAgent is called to register with the
capabilities. Since OOB is part of the capabilities data structure, we
added OOB to agent structure which already has the capabilities.
d) We added an API variant of createPairedDevice which will be used
for OutOfBand Pairing. When the device agent is created, the OOB field
in agent is updated.
e) When a capability request comes from the remote side the agent OOB
field is checked. It will be set if the pairing is initiated locally
through d).
For incoming pairing requests if the Agent has OOB field set but
there is no device specific agent, we need to make an Agent API call
asking if
there is OOB data for this remote device.
We left it to the users of the API - regarding the OOB channel to use.
It can be NFC or any other trusted channel.
Comments ?
Regards,
Jaikumar
>
> Regards,
> Claudio
> --
> 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
>
^ permalink raw reply
* Re: Possible regression with skb_clone() in 2.6.36
From: Gustavo F. Padovan @ 2010-09-15 18:42 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, netdev, linux-kernel, marcel, davem
In-Reply-To: <20100910194509.GC19693@vigoh>
Hi Mat,
* Gustavo F. Padovan <padovan@profusion.mobi> [2010-09-10 16:45:09 -0300]:
> Hi Mat,
>
> * Mat Martineau <mathewm@codeaurora.org> [2010-09-10 09:53:31 -0700]:
>
> >
> > Gustavo -
> >
> > I'm not sure why the streaming code used to work, but this does not
> > look like an skb_clone() problem. Your patch to remove the
> > skb_clone() call in l2cap_streaming_send() addresses the root cause of
> > this crash.
> >
> > On Wed, 8 Sep 2010, Gustavo F. Padovan wrote:
> >
> > > I've been experiencing some problems when running the L2CAP Streaming mode in
> > > 2.6.36. The system quickly runs in an Out Of Memory condition and crash. That
> > > wasn't happening before, so I think we may have a regression here (I didn't
> > > find where yet). The crash log is below.
> > >
> > > The following patch does not fix the regression, but shows that removing the
> > > skb_clone() call from l2cap_streaming_send() we workaround the problem. The
> > > patch is good anyway because it saves memory and time.
> > >
> > > By now I have no idea on how to fix this.
> > >
> > > <snip>
> >
> > This has to do with the sk->sk_wmem_alloc accounting that controls the
> > amount of write buffer space used on the socket.
> >
> > When the L2CAP streaming mode socket segments its data, it allocates
> > memory using sock_alloc_send_skb() (via bt_skb_send_alloc()). Before
> > that allocation call returns, skb_set_owner_w() is called on the new
> > skb. This adds to sk->sk_wmem_alloc and sets skb->destructor so that
> > sk->sk_wmem_alloc is correctly updated when the skb is freed.
> >
> > When that skb is cloned, the clone is not "owned" by the write buffer.
> > The clone's destructor is set to NULL in __skb_clone(). The version
> > of l2cap_streaming_send() that runs out of memory is passing the
> > non-owned skb clone down to the HCI layer. The original skb (the one
> > that's "owned by w") is immediately freed, which adjusts
> > sk->sk_wmem_alloc back down - the socket thinks it has unlimited write
> > buffer space. As a result, bt_skb_send_alloc() never blocks waiting
> > for buffer space (or returns EAGAIN for nonblocking writes) and the
> > HCI send queue keeps growing.
>
> If the problem is what you are saying, add a skb_set_owner_w(skb, sk) on
> the cloned skb should solve the problem, but it doesn't. That's exactly
> what tcp_transmit_skb() does. Also that just appeared in 2.6.36, is was
> working fine before, i.e, we have a regression here.
I've run some other tests and what you said also fixes the problem for
Streaming Mode. If I use skb_set_owner_w() on the cloned skb, everything
works fine. But we still have the problem for ERTM as I described.
send() blocks wainting for memory. The regression is there yet.
:(
>
> >
> > This isn't a problem for the ERTM sends, because the original skbs are
> > kept in the ERTM tx queue until they are acked. Once they're acked,
> > the write buffer space is freed and additional skbs can be allocated.
>
> It affects ERTM as well, but in that case the kernel doesn't crash
> because ERTM block on sending trying to allocate memory. Then we are not
> able to receive any ack (everything stays queued in sk_backlog_queue as
> the sk is owned by the user) and ERTM stalls.
>
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [RFC] D-Bus API for out of band association model
From: Johan Hedberg @ 2010-09-15 18:53 UTC (permalink / raw)
To: jaikumar Ganesh
Cc: Claudio Takahasi, Andrzej Kaczmarek, linux-bluetooth,
par-gunnar.p.hjalmdahl
In-Reply-To: <AANLkTikryj2AeaCWpVKQhkMznfAF-d15QpJF6=BgyGsY@mail.gmail.com>
Hi Jaikumar,
On Wed, Sep 15, 2010, jaikumar Ganesh wrote:
> We started to work on this API too last week. Not complete yet to post
> to the mailing list.
> The approach we took was this:
>
> a) Since OOB authentication is just another means of authentication
> compared to Passkey, Pin entry,
> we added an agent API (similar to the lines of other
> authentication mechanisms) that will ask for the Out Of Band data.
> b) We added an API to the adapter to get the local Out of Band Data
> c) Currently, RegisterAgent is called to register with the
> capabilities. Since OOB is part of the capabilities data structure, we
> added OOB to agent structure which already has the capabilities.
> d) We added an API variant of createPairedDevice which will be used
> for OutOfBand Pairing. When the device agent is created, the OOB field
> in agent is updated.
> e) When a capability request comes from the remote side the agent OOB
> field is checked. It will be set if the pairing is initiated locally
> through d).
> For incoming pairing requests if the Agent has OOB field set but
> there is no device specific agent, we need to make an Agent API call
> asking if
> there is OOB data for this remote device.
>
> We left it to the users of the API - regarding the OOB channel to use.
> It can be NFC or any other trusted channel.
>
> Comments ?
I'm still not convinced that the Agent is the right place for this. The
agent is typically representing the UI whereas in most cases (like NFC)
the OOB data will be coming from below bluetoothd in the software stack
and not from above it (which is where the agent resides). Also, the
presence of OOB data isn't really agent specific. It's something that
can come and go during the lifetime of an agent.
My idea has been to add a a plugin interface through which a hardware
specific plugin could notify the core daemon about the existence of OOB
data. bluetoothd would then take care of setting the right flag when it
gets a IO capability request. This doesn't of course rule out the
possibility of having part of the work done in another process since the
plugin could simply export a service over D-Bus or talk to another D-Bus
service.
Johan
^ permalink raw reply
* Re: [RFC] D-Bus API for out of band association model
From: jaikumar Ganesh @ 2010-09-15 19:13 UTC (permalink / raw)
To: jaikumar Ganesh, Claudio Takahasi, Andrzej Kaczmarek,
linux-bluetooth, par-gunnar.p.hjalmdahl
In-Reply-To: <20100915185340.GA2804@jh-x301>
Hi Johan:
On Wed, Sep 15, 2010 at 11:53 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Jaikumar,
>
> On Wed, Sep 15, 2010, jaikumar Ganesh wrote:
>> We started to work on this API too last week. Not complete yet to post
>> to the mailing list.
>> The approach we took was this:
>>
>> a) Since OOB authentication is just another means of authentication
>> compared to Passkey, Pin entry,
>> we added an agent API (similar to the lines of other
>> authentication mechanisms) that will ask for the Out Of Band data.
>> b) We added an API to the adapter to get the local Out of Band Data
>> c) Currently, RegisterAgent is called to register with the
>> capabilities. Since OOB is part of the capabilities data structure, we
>> added OOB to agent structure which already has the capabilities.
>> d) We added an API variant of createPairedDevice which will be used
>> for OutOfBand Pairing. When the device agent is created, the OOB field
>> in agent is updated.
>> e) When a capability request comes from the remote side the agent OOB
>> field is checked. It will be set if the pairing is initiated locally
>> through d).
>> For incoming pairing requests if the Agent has OOB field set but
>> there is no device specific agent, we need to make an Agent API call
>> asking if
>> there is OOB data for this remote device.
>>
>> We left it to the users of the API - regarding the OOB channel to use.
>> It can be NFC or any other trusted channel.
>>
>> Comments ?
>
> I'm still not convinced that the Agent is the right place for this. The
> agent is typically representing the UI whereas in most cases (like NFC)
> the OOB data will be coming from below bluetoothd in the software stack
> and not from above it (which is where the agent resides). Also, the
> presence of OOB data isn't really agent specific. It's something that
> can come and go during the lifetime of an agent.
Consider a case where there is a third party app residing on the device.
This wants to initiate pairing and has OOB data. (here OOB data comes
from above bluetoothd)
When the remote request OOB data comes, bluetoothd asks the app for
the OOB data.
Depending on the implementation either the app can show a UI to the user
asking permission to pair when the OOB data Agent API is called. This is again
similar to what we have for the passkey mechanism.
This need of the UI and the fact that OOB data can come from above
bluetoothd (similar to pin entry)
is what made us go with the agent approach.
The device agent is removed when the device is unpaired. So next time
when pairing
happens depending upon the API used we can again have OOB pairing or
non OOB pairing.
Yes OOB data can come and go during the life of an agent. Hence we put
this burden on the users
of bluetoothd to supply bluetoothd if the OOB data. We not storing any
OOB data of the remote device
in bluetoothd
> My idea has been to add a a plugin interface through which a hardware
> specific plugin could notify the core daemon about the existence of OOB
> data. bluetoothd would then take care of setting the right flag when it
> gets a IO capability request. This doesn't of course rule out the
> possibility of having part of the work done in another process since the
> plugin could simply export a service over D-Bus or talk to another D-Bus
> service.
If I understand correctly, hardware specific plugin works well for
things like NFC.
What about cases where the trusted channel is say something like an
app running on
both the computer and the device. That channel is trusted using some
other authentication mechanism
The apps communicate to exchange OOB data.
For example, you can use the browser to device messaging mechanism
(Cloud to device messaging using Chrome).
>
> Johan
>
^ permalink raw reply
* Re: Request for input regarding new driver in MFD for GPS_Bluetooth_FM controller CG2900
From: Pavan Savoy @ 2010-09-15 20:57 UTC (permalink / raw)
To: pghatwork; +Cc: Alan Cox, linus.walleij, linux-kernel, linux-bluetooth
In-Reply-To: <AANLkTi=CNdDYVGNmx17miVBXfPgnWY0X59Ao1pRQDN=S@mail.gmail.com>
Hi,
On Wed, Sep 15, 2010 at 5:14 AM, Par-Gunnar Hjalmdahl
<par-gunnar.p.hjalmdahl@stericsson.com> wrote:
> Hi Pavan,
>
> Thanks for your comments and sorry for me taking time to answer your mail.
> See below for answers.
>
>>
>> ok bit more of information ....
>> We don;t use the hciattach, instead we have our own daemon which opens
>> up the UART and installs the line discipline (not N_HCI, but similar
>> one called N_SHARED) when the hciconfig hci0 up happens or even when
>> /dev/radio0 (FM V4L2 device) happens or when generic GPS character
>> device (/dev/tigps) happens...
>
> :-) We also have our own user space application for opening the UART
> and setting the line disc since we don't want to depend completely on
> hciattach, which contains more than we need, and also misses stuff
> that we want to do towards our specific controller. I just used it as
> an example since that is the common BlueZ way to open the Bluetooth
> transport.
Ok, yes currently we communicate with user-space daemon over rfkill,
we can probably use UDEV events to communicate opening/installation of
ldisc or closing of UART.
>>
>> There is non-mailine driver which gets modified to get into mainline @
>> http://git.omapzoom.org/?p=kernel/omap.git;a=tree;f=drivers/misc/ti-
>> st;h=028ff4a739d7b59b94d0c613b5ef510ff338b65d;hb=refs/heads/p-android-
>> omap-2.6.32
>>
>> feel free to have a look at it...
>> Yes our solution too works with BlueZ and non-exactly a MFD driver but
>> it is a simple platform device driver .. by looks of things the driver
>> can run as is for your chip too .. (except for the firmware search and
>> download part .. may be...).
>
> Your code is in a lot of ways similar to ours (which is not so strange
> since you address the same market with your chip as we do with ours),
> but there are several differences that would at least for now make it
> impossible for us to use your driver. When we post our first patches
> (which will hopefully be in a few days) you can see for yourself. But
> I don't think there is a simple way for us to re-use your driver.
Humn, I kind of thought may be we can have a same driver for 2
different types of chips.
>>
>> and note when we would want to support SPI transport for the same, we
>> plan a SPI-TTY driver ('ala usb-serial) where-in we can install this
>> N_TI_WL line discipline on that /dev/ttySPI0 device, and the SPI
>> related stuff to be handled by the spi-tty.c which registers itself as
>> a tty_device and a tty_driver ....
>
> Our SPI usage will be a bit different, where we don't use TTY for our
> driver. We will use the SPI directly instead.
>
Ok, I was intending towards the SPI usage of our driver when we move to SPI.
>> regards,
>> Pavan
>>
>> On Fri, Sep 10, 2010 at 2:07 PM, Pavan Savoy <pavan_savoy@sify.com>
>> wrote:
>> > Can you directly make use of the ti-st driver which is currently
>> staged?
>> > It has _EXACTLY_ the same thing.... which is REALLY REALLY surprising
>> !!!
>
> :-) As I said earlier this is quite natural since both TI and
> ST-Ericsson address the same market segments.
yes...
> /P-G
>
^ permalink raw reply
* Re: CSP implementation for MCAP
From: Elvis Pfützenreuter @ 2010-09-15 22:45 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <4A11989E-5448-43B7-887F-217C54977E22@epx.com.br>
On 02/09/2010, at 15:58, Elvis Pfützenreuter wrote:
> This is the repository for the CSP implementation, rebased over the recently accepted MCAP, for your appreciation:
>
> git://gitorious.org/bluez-epx/bluez-epx.git csp
>
> or
>
> http://www.gitorious.org/bluez-epx/bluez-epx/commits/csp--
And I am happy to announce that this CSP implementation passed on PTS :) Fixes are on topmost patch of the repository.
^ permalink raw reply
* Re: Possible regression with skb_clone() in 2.6.36
From: Gustavo F. Padovan @ 2010-09-16 0:10 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, netdev, linux-kernel, marcel, davem
In-Reply-To: <20100910194509.GC19693@vigoh>
* Gustavo F. Padovan <padovan@profusion.mobi> [2010-09-10 16:45:09 -0300]:
> Hi Mat,
>
> * Mat Martineau <mathewm@codeaurora.org> [2010-09-10 09:53:31 -0700]:
>
> >
> > Gustavo -
> >
> > I'm not sure why the streaming code used to work, but this does not
> > look like an skb_clone() problem. Your patch to remove the
> > skb_clone() call in l2cap_streaming_send() addresses the root cause of
> > this crash.
> >
> > On Wed, 8 Sep 2010, Gustavo F. Padovan wrote:
> >
> > > I've been experiencing some problems when running the L2CAP Streaming mode in
> > > 2.6.36. The system quickly runs in an Out Of Memory condition and crash. That
> > > wasn't happening before, so I think we may have a regression here (I didn't
> > > find where yet). The crash log is below.
> > >
> > > The following patch does not fix the regression, but shows that removing the
> > > skb_clone() call from l2cap_streaming_send() we workaround the problem. The
> > > patch is good anyway because it saves memory and time.
> > >
> > > By now I have no idea on how to fix this.
> > >
> > > <snip>
> >
> > This has to do with the sk->sk_wmem_alloc accounting that controls the
> > amount of write buffer space used on the socket.
> >
> > When the L2CAP streaming mode socket segments its data, it allocates
> > memory using sock_alloc_send_skb() (via bt_skb_send_alloc()). Before
> > that allocation call returns, skb_set_owner_w() is called on the new
> > skb. This adds to sk->sk_wmem_alloc and sets skb->destructor so that
> > sk->sk_wmem_alloc is correctly updated when the skb is freed.
> >
> > When that skb is cloned, the clone is not "owned" by the write buffer.
> > The clone's destructor is set to NULL in __skb_clone(). The version
> > of l2cap_streaming_send() that runs out of memory is passing the
> > non-owned skb clone down to the HCI layer. The original skb (the one
> > that's "owned by w") is immediately freed, which adjusts
> > sk->sk_wmem_alloc back down - the socket thinks it has unlimited write
> > buffer space. As a result, bt_skb_send_alloc() never blocks waiting
> > for buffer space (or returns EAGAIN for nonblocking writes) and the
> > HCI send queue keeps growing.
>
> If the problem is what you are saying, add a skb_set_owner_w(skb, sk) on
> the cloned skb should solve the problem, but it doesn't. That's exactly
> what tcp_transmit_skb() does. Also that just appeared in 2.6.36, is was
> working fine before, i.e, we have a regression here.
>
> >
> > This isn't a problem for the ERTM sends, because the original skbs are
> > kept in the ERTM tx queue until they are acked. Once they're acked,
> > the write buffer space is freed and additional skbs can be allocated.
>
> It affects ERTM as well, but in that case the kernel doesn't crash
> because ERTM block on sending trying to allocate memory. Then we are not
> able to receive any ack (everything stays queued in sk_backlog_queue as
> the sk is owned by the user) and ERTM stalls.
By reverting
commit 218bb9dfd21472128f86b38ad2eab123205c2991
Author: Gustavo F. Padovan <padovan@profusion.mobi>
Date: Mon Jun 21 18:53:22 2010 -0300
Bluetooth: Add backlog queue to ERTM code
backlog queue is the canonical mechanism to avoid race conditions due
interrupts in bottom half context. After the socket lock is released the
net core take care of push all skb in its backlog queue.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
we can workaround the bug. It's not the real cause of the bug because the
backlog queue was working before 2.6.36, but removing the backlog queue scheme
make L2CAP works again and also give us more time to find the real cause of the
problem.
Before implement the backlog queue L2CAP had his own lock scheme to avoid race
conditions. The point is that the backlog queue adds too much serialization to
L2CAP, that was the cause of the ERTM bug. The old scheme (the one we are going
to use after revert this commit) just serialize access to some places.
By not using the backlog queue, we can receive the acknowledgement frames more
quickly and then free the acked frames quickly. In fact the old scheme is
looks better than backlog queue.
I think we should proceed with the revert with this commit in mainline, and at the
same time try to find the root cause of the problem.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* [PATCH] Bluetooth: Replace hard code of fixed channels bit mask
From: haijun liu @ 2010-09-16 7:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mat Martineau, dantian.ip
This patch add fixed channels bit mask definition for
L2CAP_FIXCHAN_NULLID
L2CAP_FIXCHAN_SIGNAL
L2CAP_FIXCHAN_CONNLESS
L2CAP_FIXCHAN_A2MP
And replace hard code in source file with the macro.
Signed-off-by: Haijun.Liu <Haijun.Liu@Atheros.com>
---
include/net/bluetooth/l2cap.h | 6 ++++++
net/bluetooth/l2cap.c | 2 +-
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 6c24144..e4fe2c7 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -97,6 +97,12 @@ struct l2cap_conninfo {
#define L2CAP_FEAT_FCS 0x00000020
#define L2CAP_FEAT_FIXED_CHAN 0x00000080
+/* L2CAP fixed channel bitmask */
+#define L2CAP_FIXCHAN_NULLID 0x00
+#define L2CAP_FIXCHAN_SIGNAL 0x02
+#define L2CAP_FIXCHAN_CONNLESS 0x04
+#define L2CAP_FIXCHAN_A2MP 0x08
+
/* L2CAP checksum option */
#define L2CAP_FCS_NONE 0x00
#define L2CAP_FCS_CRC16 0x01
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index c784703..23e487e 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -60,7 +60,7 @@
static int disable_ertm = 0;
static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
-static u8 l2cap_fixed_chan[8] = { 0x02, };
+static u8 l2cap_fixed_chan[8] = { L2CAP_FIXCHAN_SIGNAL, };
static const struct proto_ops l2cap_sock_ops;
--
1.6.3.3
^ permalink raw reply related
* Re: [RFC] BlueZ D-Bus Sim Access Profile Server API description
From: Suraj Sumangala @ 2010-09-16 7:50 UTC (permalink / raw)
To: Waldemar.Rymarkiewicz@tieto.com
Cc: marcel@holtmann.org, Suraj Sumangala,
linux-bluetooth@vger.kernel.org, Jothikumar Mothilal
In-Reply-To: <99B09243E1A5DA4898CDD8B700111448097968EDE3@EXMB04.eu.tieto.com>
Hi,
> I agree with this too. I prefere to have a sap implementation as bluez plugin, but we need to define api for sim operations which could be implemented in ofono or other proprietary stacks. I guess dbus was intended to be kind of hw abstraction api.
> Some time ago Claudio sent proposal implementation of sap server where he defined api for a driver to sim.
The main question will be
"where should be SAP profile packet handling be implemented?"
1. If it has to be implemented as part of bluetoothd, there is no other
option other than using dbus so that we have a consistent interface for
all modules who would be interested in using it.
2. If it has to be done in the "modem/SIM reader", then it will be part
of that module, not an independent module.
will all SIM card reader implementation be ready to implement a
Bluetooth SAP parser as part of it just to support Bluetooth as transport?
>
> Regards,
> /Waldek
Regards
Suraj
^ permalink raw reply
* [PATCH] Bluetooth: Fix bug in l2cap_parse_conf_rsp()
From: haijun liu @ 2010-09-16 8:26 UTC (permalink / raw)
To: linux-bluetooth, dantian.ip, Mat Martineau
Acording bluetooth spec [Host Volume] chapter, section
of "7.1.2 Response path", in l2cap_parse_conf_rsp(), MTU should be
incoming mtu, not outgoing mtu.
Signed-off-by: Haijun.Liu <Haijun.Liu@Atheros.com>
---
net/bluetooth/l2cap.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 23e487e..1797277 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2771,10 +2771,10 @@ static int l2cap_parse_conf_rsp(struct sock
*sk, void *rsp, int len, void *data,
case L2CAP_CONF_MTU:
if (val < L2CAP_DEFAULT_MIN_MTU) {
*result = L2CAP_CONF_UNACCEPT;
- pi->omtu = L2CAP_DEFAULT_MIN_MTU;
+ pi->imtu = L2CAP_DEFAULT_MIN_MTU;
} else
- pi->omtu = val;
- l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->omtu);
+ pi->imtu = val;
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
break;
case L2CAP_CONF_FLUSH_TO:
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Fix bug in l2cap_parse_conf_rsp()
From: Andrei Emeltchenko @ 2010-09-16 9:00 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth, dantian.ip, Mat Martineau
In-Reply-To: <AANLkTi=JEhr6nO7WqBm2=bGrpCxuc6daG_hMGMd_-hxY@mail.gmail.com>
Hi,
Duplicated:
http://www.spinics.net/lists/linux-bluetooth/msg07366.html
BTW: How long will it tale to get the patch in?
On Thu, Sep 16, 2010 at 11:26 AM, haijun liu <liuhaijun.er@gmail.com> wrote:
> Acording bluetooth spec [Host Volume] chapter, section
> of "7.1.2 Response path", in l2cap_parse_conf_rsp(), MTU should be
> incoming mtu, not outgoing mtu.
>
> Signed-off-by: Haijun.Liu <Haijun.Liu@Atheros.com>
> ---
> net/bluetooth/l2cap.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 23e487e..1797277 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -2771,10 +2771,10 @@ static int l2cap_parse_conf_rsp(struct sock
> *sk, void *rsp, int len, void *data,
> case L2CAP_CONF_MTU:
> if (val < L2CAP_DEFAULT_MIN_MTU) {
> *result = L2CAP_CONF_UNACCEPT;
> - pi->omtu = L2CAP_DEFAULT_MIN_MTU;
> + pi->imtu = L2CAP_DEFAULT_MIN_MTU;
> } else
> - pi->omtu = val;
> - l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->omtu);
> + pi->imtu = val;
> + l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
> break;
>
> case L2CAP_CONF_FLUSH_TO:
> --
> 1.6.3.3
> --
> 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
>
^ permalink raw reply
* [PATCH] Bluetooth: Replace hard code of configuration continuation flag.
From: haijun liu @ 2010-09-16 9:21 UTC (permalink / raw)
To: linux-bluetooth, Mat Martineau, dantian.ip
Replace hard code of configuration continuation flag with self-comment macro.
Signed-off-by: Haijun.Liu <Haijun.Liu@Atheros.com>
---
include/net/bluetooth/l2cap.h | 2 ++
net/bluetooth/l2cap.c | 4 ++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index e4fe2c7..a37b266 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -199,6 +199,8 @@ struct l2cap_conf_rsp {
#define L2CAP_CONF_REJECT 0x0002
#define L2CAP_CONF_UNKNOWN 0x0003
+#define L2CAP_CONF_FLAG_CONT 0x0001
+
struct l2cap_conf_opt {
__u8 type;
__u8 len;
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 1797277..725744b 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3121,7 +3121,7 @@ static inline int l2cap_config_req(struct
l2cap_conn *conn, struct l2cap_cmd_hdr
memcpy(l2cap_pi(sk)->conf_req + l2cap_pi(sk)->conf_len, req->data, len);
l2cap_pi(sk)->conf_len += len;
- if (flags & 0x0001) {
+ if (flags & L2CAP_CONF_FLAG_CONT) {
/* Incomplete config. Send empty response. */
l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP,
l2cap_build_conf_rsp(sk, rsp,
@@ -3228,7 +3228,7 @@ static inline int l2cap_config_rsp(struct
l2cap_conn *conn, struct l2cap_cmd_hdr
goto done;
}
- if (flags & 0x01)
+ if (flags & L2CAP_CONF_FLAG_CONT)
goto done;
l2cap_pi(sk)->conf_state |= L2CAP_CONF_INPUT_DONE;
--
1.6.3.3
--
Haijun Liu
^ permalink raw reply related
* Re: CSP implementation for MCAP
From: Johan Hedberg @ 2010-09-16 12:39 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <1FA1AF09-53BA-4187-81B3-78E2E5C82852@signove.com>
Hi Elvis,
On Wed, Sep 15, 2010, Elvis Pfützenreuter wrote:
> > This is the repository for the CSP implementation, rebased over the
> > recently accepted MCAP, for your appreciation:
> >
> > git://gitorious.org/bluez-epx/bluez-epx.git csp
> >
> > or
> >
> > http://www.gitorious.org/bluez-epx/bluez-epx/commits/csp--
>
> And I am happy to announce that this CSP implementation passed on PTS
> :) Fixes are on topmost patch of the repository.
Good to hear about the progress with the PTS :)
Unfortunately you'll need to rebase again since I pushed some cleanups
to the MCAP code. Could you also get rid of the unnecessary
double-pointers and type casts which aren't needed anymore now that
mcap_send_data accepts void *.
Johan
^ permalink raw reply
* Re: [RFC] D-Bus API for out of band association model
From: Andrzej Kaczmarek @ 2010-09-16 12:51 UTC (permalink / raw)
To: jaikumar Ganesh, Claudio Takahasi, linux-bluetooth,
par-gunnar.p.hjalmdahl
In-Reply-To: <20100915185340.GA2804@jh-x301>
Hi,
On 15.09.2010 20:53, Johan Hedberg wrote:
> I'm still not convinced that the Agent is the right place for this. The
> agent is typically representing the UI whereas in most cases (like NFC)
> the OOB data will be coming from below bluetoothd in the software stack
> and not from above it (which is where the agent resides).
In case of NFC I guess data will be coming from some application or
perhaps a daemon which can understand data coming from lower layers,
which I guess will rather process raw data. So these data will come from
the same level as bluetoothd resides or above. This is something that
agent can communicate freely with. Also from our point of view OOB data
is used in the same way as i.e. passkey and this is what agent provides.
We don't care how it will receive such data.
> Also, the
> presence of OOB data isn't really agent specific. It's something that
> can come and go during the lifetime of an agent
That's not a problem, agent should make use of interfaces published by
applications which can provide secure channel. NFC application is one of
examples.
Also Jakiumar gave quite good example where NFC application receives
some data which is identified as BT OOB data so it initiates pairing
with device specific agent which will provide these data.
> My idea has been to add a a plugin interface through which a hardware
> specific plugin could notify the core daemon about the existence of OOB
> data. bluetoothd would then take care of setting the right flag when it
> gets a IO capability request.
Do I understand correctly that also plugin or daemon should manage
received data? Won't it make bluetoothd store unnecessary data, i.e. for
devices we won't use anyway?
Also what in case we want to use several different secure channels?
Multiple plugins? In case of agent, we make single call and no need to
worry where the data come from. Agent is platform dependent anyway so
platform vendor can put required logic there.
> This doesn't of course rule out the
> possibility of having part of the work done in another process since the
> plugin could simply export a service over D-Bus or talk to another D-Bus
> service.
I agree, such plugin can communicate with some other daemon/application
to get data. So do agent. And in case of agent we're consistent that all
bonding related data (pin, passkey, OOB...) are handled in one place
which is an agent application.
BR,
Andrzej
^ permalink raw reply
* Re: CSP implementation for MCAP
From: José Antonio Santos Cadenas @ 2010-09-16 12:55 UTC (permalink / raw)
To: Johan Hedberg; +Cc: Elvis Pfützenreuter, linux-bluetooth
In-Reply-To: <20100916123900.GA20869@jh-x301>
El Thursday 16 September 2010 14:39:00 Johan Hedberg escribió:
> Hi Elvis,
>
> On Wed, Sep 15, 2010, Elvis Pfützenreuter wrote:
> > > This is the repository for the CSP implementation, rebased over the
> > > recently accepted MCAP, for your appreciation:
> > >
> > > git://gitorious.org/bluez-epx/bluez-epx.git csp
> > >
> > > or
> > >
> > > http://www.gitorious.org/bluez-epx/bluez-epx/commits/csp--
> >
> > And I am happy to announce that this CSP implementation passed on PTS
> >
> > :) Fixes are on topmost patch of the repository.
>
> Good to hear about the progress with the PTS :)
>
> Unfortunately you'll need to rebase again since I pushed some cleanups
> to the MCAP code. Could you also get rid of the unnecessary
> double-pointers and type casts which aren't needed anymore now that
> mcap_send_data accepts void *.
There are also a lot of (! condition) with an extra space between the ! and
the condition variable.
Also some use of g_malloc0 instead of g_new0. About this issue, remember that
g_new0 initializes all the structure with 0 so the initialization don't need
to do for some fields in mcap_sync_init function.
Regards.
>
> Johan
> --
> 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
^ permalink raw reply
* Re: [PATCH] Bluetooth: Fix bug in l2cap_parse_conf_rsp()
From: Gustavo F. Padovan @ 2010-09-16 14:13 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: haijun liu, linux-bluetooth, dantian.ip, Mat Martineau
In-Reply-To: <AANLkTingGbnYQuyQvjPcK-=zSvo-RCpq2tGioysEMYNk@mail.gmail.com>
* Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> [2010-09-16 12:00:50 +0300]:
> Hi,
>
> Duplicated:
> http://www.spinics.net/lists/linux-bluetooth/msg07366.html
>
> BTW: How long will it tale to get the patch in?
We are planning to put it in 2.6.36. I'm tracking it.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: CSP implementation for MCAP
From: Elvis Pfützenreuter @ 2010-09-16 14:14 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100916123900.GA20869@jh-x301>
On 16/09/2010, at 09:39, Johan Hedberg wrote:
> Hi Elvis,
>
> On Wed, Sep 15, 2010, Elvis Pfützenreuter wrote:
>>> This is the repository for the CSP implementation, rebased over the
>>> recently accepted MCAP, for your appreciation:
>>>
>>> git://gitorious.org/bluez-epx/bluez-epx.git csp
>>>
>>> or
>>>
>>> http://www.gitorious.org/bluez-epx/bluez-epx/commits/csp--
>>
>> And I am happy to announce that this CSP implementation passed on PTS
>> :) Fixes are on topmost patch of the repository.
>
> Good to hear about the progress with the PTS :)
>
> Unfortunately you'll need to rebase again since I pushed some cleanups
> to the MCAP code. Could you also get rid of the unnecessary
> double-pointers and type casts which aren't needed anymore now that
> mcap_send_data accepts void *.
It's done, and tested.
^ permalink raw reply
* Re: [PATCH] Bluetooth: Replace hard code of fixed channels bit mask
From: Gustavo F. Padovan @ 2010-09-16 14:16 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth, Mat Martineau, dantian.ip
In-Reply-To: <AANLkTi=86t=osr8YUfUfU3QLHiHXgp8SCuwqOE_GoPgp@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-09-16 15:15:18 +0800]:
> This patch add fixed channels bit mask definition for
> L2CAP_FIXCHAN_NULLID
> L2CAP_FIXCHAN_SIGNAL
> L2CAP_FIXCHAN_CONNLESS
> L2CAP_FIXCHAN_A2MP
> And replace hard code in source file with the macro.
>
> Signed-off-by: Haijun.Liu <Haijun.Liu@Atheros.com>
> ---
> include/net/bluetooth/l2cap.h | 6 ++++++
> net/bluetooth/l2cap.c | 2 +-
> 2 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 6c24144..e4fe2c7 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -97,6 +97,12 @@ struct l2cap_conninfo {
> #define L2CAP_FEAT_FCS 0x00000020
> #define L2CAP_FEAT_FIXED_CHAN 0x00000080
>
> +/* L2CAP fixed channel bitmask */
> +#define L2CAP_FIXCHAN_NULLID 0x00
> +#define L2CAP_FIXCHAN_SIGNAL 0x02
> +#define L2CAP_FIXCHAN_CONNLESS 0x04
> +#define L2CAP_FIXCHAN_A2MP 0x08
That's wrong, signaling channel is 0x01, connless is 0x02 and A2MP is
0x03. And if you haven't noted we already have macros for the signaling
and connectionless channels:
#define L2CAP_CID_SIGNALING 0x0001
#define L2CAP_CID_CONN_LESS 0x0002
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: CSP implementation for MCAP
From: Johan Hedberg @ 2010-09-16 15:03 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <F9784FEC-442D-4448-9BA1-234AA9963ECE@signove.com>
Hi Elvis,
On Thu, Sep 16, 2010, Elvis Pfützenreuter wrote:
> > On Wed, Sep 15, 2010, Elvis Pfützenreuter wrote:
> >>> This is the repository for the CSP implementation, rebased over the
> >>> recently accepted MCAP, for your appreciation:
> >>>
> >>> git://gitorious.org/bluez-epx/bluez-epx.git csp
> >>>
> >>> or
> >>>
> >>> http://www.gitorious.org/bluez-epx/bluez-epx/commits/csp--
> >>
> >> And I am happy to announce that this CSP implementation passed on PTS
> >> :) Fixes are on topmost patch of the repository.
> >
> > Good to hear about the progress with the PTS :)
> >
> > Unfortunately you'll need to rebase again since I pushed some cleanups
> > to the MCAP code. Could you also get rid of the unnecessary
> > double-pointers and type casts which aren't needed anymore now that
> > mcap_send_data accepts void *.
>
> It's done, and tested.
Thanks. I still had to do quite a lot of coding style fixes as well as
plug a memory leak, but the patches are now pushed upstream.
Next steps:
- Get rid of the forward declarations of static functions
wherever possible. It looked like you could accomplish that
with a simple reordering of the functions.
- Get rid of raw HCI access (hci_open_dev). bluetoothd shouldn't use
this type of sockets in the future at all. You'll need to add a proper
callback to the adapter_opts and a function to plugins/hciopts.c and
use that instead. Then export a function like btd_adapter_read_clock
for the plugin to use.
Johan
^ permalink raw reply
* Re: Possible regression with skb_clone() in 2.6.36
From: Mat Martineau @ 2010-09-16 16:40 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth, marcel
In-Reply-To: <20100916001012.GA5656@vigoh>
Gustavo -
Since this is now a only a discussion of blocking behavior with ERTM
(and not a regression with skb_clone()), I've removed linux-kernel and
netdev from the cc list. If you think they still need to see this
thread, please add them back in.
On Wed, 15 Sep 2010, Gustavo F. Padovan wrote:
> * Gustavo F. Padovan <padovan@profusion.mobi> [2010-09-10 16:45:09 -0300]:
>
>> Hi Mat,
>>
>> * Mat Martineau <mathewm@codeaurora.org> [2010-09-10 09:53:31 -0700]:
>>
>>>
>>> Gustavo -
>>>
>>> I'm not sure why the streaming code used to work, but this does not
>>> look like an skb_clone() problem. Your patch to remove the
>>> skb_clone() call in l2cap_streaming_send() addresses the root cause of
>>> this crash.
>>>
>>> On Wed, 8 Sep 2010, Gustavo F. Padovan wrote:
>>>
>>>> I've been experiencing some problems when running the L2CAP Streaming mode in
>>>> 2.6.36. The system quickly runs in an Out Of Memory condition and crash. That
>>>> wasn't happening before, so I think we may have a regression here (I didn't
>>>> find where yet). The crash log is below.
>>>>
>>>> The following patch does not fix the regression, but shows that removing the
>>>> skb_clone() call from l2cap_streaming_send() we workaround the problem. The
>>>> patch is good anyway because it saves memory and time.
>>>>
>>>> By now I have no idea on how to fix this.
>>>>
>>>> <snip>
>>>
>>> This has to do with the sk->sk_wmem_alloc accounting that controls the
>>> amount of write buffer space used on the socket.
>>>
>>> When the L2CAP streaming mode socket segments its data, it allocates
>>> memory using sock_alloc_send_skb() (via bt_skb_send_alloc()). Before
>>> that allocation call returns, skb_set_owner_w() is called on the new
>>> skb. This adds to sk->sk_wmem_alloc and sets skb->destructor so that
>>> sk->sk_wmem_alloc is correctly updated when the skb is freed.
>>>
>>> When that skb is cloned, the clone is not "owned" by the write buffer.
>>> The clone's destructor is set to NULL in __skb_clone(). The version
>>> of l2cap_streaming_send() that runs out of memory is passing the
>>> non-owned skb clone down to the HCI layer. The original skb (the one
>>> that's "owned by w") is immediately freed, which adjusts
>>> sk->sk_wmem_alloc back down - the socket thinks it has unlimited write
>>> buffer space. As a result, bt_skb_send_alloc() never blocks waiting
>>> for buffer space (or returns EAGAIN for nonblocking writes) and the
>>> HCI send queue keeps growing.
>>
>> If the problem is what you are saying, add a skb_set_owner_w(skb, sk) on
>> the cloned skb should solve the problem, but it doesn't. That's exactly
>> what tcp_transmit_skb() does. Also that just appeared in 2.6.36, is was
>> working fine before, i.e, we have a regression here.
>>
>>>
>>> This isn't a problem for the ERTM sends, because the original skbs are
>>> kept in the ERTM tx queue until they are acked. Once they're acked,
>>> the write buffer space is freed and additional skbs can be allocated.
>>
>> It affects ERTM as well, but in that case the kernel doesn't crash
>> because ERTM block on sending trying to allocate memory. Then we are not
>> able to receive any ack (everything stays queued in sk_backlog_queue as
>> the sk is owned by the user) and ERTM stalls.
>
> By reverting
>
> commit 218bb9dfd21472128f86b38ad2eab123205c2991
> Author: Gustavo F. Padovan <padovan@profusion.mobi>
> Date: Mon Jun 21 18:53:22 2010 -0300
>
> Bluetooth: Add backlog queue to ERTM code
>
> backlog queue is the canonical mechanism to avoid race conditions due
> interrupts in bottom half context. After the socket lock is released the
> net core take care of push all skb in its backlog queue.
>
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
>
>
> we can workaround the bug. It's not the real cause of the bug because the
> backlog queue was working before 2.6.36, but removing the backlog queue scheme
> make L2CAP works again and also give us more time to find the real cause of the
> problem.
>
> Before implement the backlog queue L2CAP had his own lock scheme to avoid race
> conditions. The point is that the backlog queue adds too much serialization to
> L2CAP, that was the cause of the ERTM bug. The old scheme (the one we are going
> to use after revert this commit) just serialize access to some places.
>
> By not using the backlog queue, we can receive the acknowledgement frames more
> quickly and then free the acked frames quickly. In fact the old scheme is
> looks better than backlog queue.
>
> I think we should proceed with the revert with this commit in mainline, and at the
> same time try to find the root cause of the problem.
I don't think the backlog patch should be reverted. Although the old
code would appear to get things working again, it's only because locks
were being ignored in some important cases. Maybe it seems ok most of
the time, but the ERTM socket state was not being properly protected.
The backlog is the right way to do it, but also depends on correct
lock handling in the rest of L2CAP.
I think this is one of the locking issues from this message to
linux-bluetooth ("ERTM known bugs/regressions (was Re: [PATCH 0/8]
...)") on August 2:
http://www.spinics.net/lists/linux-bluetooth/msg06734.html
We've both arrived at the root cause already: incoming frames
(including acks) get stuck on the backlog queue while the socket is
locked waiting for memory -- and no memory gets freed until the acks
are processed.
The only time the socket lock is held for a long time is when waiting
for memory, so I think the best solution is to not hold the lock while
allocating. This should be safe, since the only socket state that's
changed during bt_skb_send_alloc() is sk->sk_wmem_alloc, which is an
atomic_t.
Then some changes are needed to these functions:
l2cap_sar_segment_sdu() and l2cap_create_iframe_pdu(): Take some args
on the stack (like remote_mps) instead of accessing l2cap_pinfo.
Don't add to TX_QUEUE(sk) within these functions (let the caller do
that).
l2cap_sock_sendmsg(): Copy necessary data to local variables from
l2cap_pinfo while the lock is held. release_sock(sk), then call
l2cap_sar_segment_sdu() (which will work for the "one PDU" case too)
with the new args for mps, fcs, etc. After l2cap_sar_segment_sdu()
returns, check the socket state again (like the code at the beginning
of l2cap_sock_sendmsg()) and then lock_sock(sk), queue the data, and
send it.
(I know it would be better and clearer to send a patch for this, but
I have some other things to fix right now!)
The other locking problem (with ERTM timers) also deserves some
attention, but maybe in a new email thread!
Regards,
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* [PATCH 1/3] bluetooth: Take a runtime pm reference on hid connections
From: Matthew Garrett @ 2010-09-16 17:58 UTC (permalink / raw)
To: linux-bluetooth; +Cc: linux-usb, marcel, Matthew Garrett
Bluetooth runtime PM interacts badly with input devices - the connection
will be dropped if the device becomes idle, resulting in noticable lag when
the user interacts with the input device again. Bump the pm runtime count
when the device is associated and release it when it's disassociated in
order to avoid this.
Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
net/bluetooth/hidp/core.c | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index bfe641b..a4489a7 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -36,6 +36,7 @@
#include <linux/file.h>
#include <linux/init.h>
#include <linux/wait.h>
+#include <linux/pm_runtime.h>
#include <net/sock.h>
#include <linux/input.h>
@@ -622,6 +623,14 @@ static int hidp_session(void *arg)
return 0;
}
+static struct hci_dev *hidp_get_hci(struct hidp_session *session)
+{
+ bdaddr_t *src = &bt_sk(session->ctrl_sock->sk)->src;
+ bdaddr_t *dst = &bt_sk(session->ctrl_sock->sk)->dst;
+
+ return hci_get_route(dst, src);
+}
+
static struct device *hidp_get_device(struct hidp_session *session)
{
bdaddr_t *src = &bt_sk(session->ctrl_sock->sk)->src;
@@ -819,6 +828,7 @@ fault:
int hidp_add_connection(struct hidp_connadd_req *req, struct socket *ctrl_sock, struct socket *intr_sock)
{
struct hidp_session *session, *s;
+ struct hci_dev *hdev;
int err;
BT_DBG("");
@@ -889,6 +899,10 @@ int hidp_add_connection(struct hidp_connadd_req *req, struct socket *ctrl_sock,
hidp_input_event(session->input, EV_LED, 0, 0);
}
+ hdev = hidp_get_hci(session);
+ pm_runtime_get(hdev->parent);
+ hci_dev_put(hdev);
+
up_write(&hidp_session_sem);
return 0;
@@ -925,6 +939,7 @@ failed:
int hidp_del_connection(struct hidp_conndel_req *req)
{
struct hidp_session *session;
+ struct hci_dev *hdev;
int err = 0;
BT_DBG("");
@@ -952,6 +967,9 @@ int hidp_del_connection(struct hidp_conndel_req *req)
} else
err = -ENOENT;
+ hdev = hidp_get_hci(session);
+ pm_runtime_put(hdev->parent);
+ hci_dev_put(hdev);
up_read(&hidp_session_sem);
return err;
}
--
1.7.2.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