* [RFC 3/6] Bluetooth: Factor out common L2CAP connection code
From: Andrei Emeltchenko @ 2012-10-11 14:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: mathewm
In-Reply-To: <1349966905-32336-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Mat Martineau <mathewm@codeaurora.org>
L2CAP connect requests and create channel requests share a significant
amount of code. This change moves common code to a new function.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
net/bluetooth/l2cap_core.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 632f291..d56cc57 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3399,7 +3399,8 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn, struct l2cap_cmd_hd
return 0;
}
-static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, u8 *data)
+static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
+ u8 *data, u8 rsp_code, u8 amp_id)
{
struct l2cap_conn_req *req = (struct l2cap_conn_req *) data;
struct l2cap_conn_rsp rsp;
@@ -3493,7 +3494,7 @@ sendresp:
rsp.dcid = cpu_to_le16(dcid);
rsp.result = cpu_to_le16(result);
rsp.status = cpu_to_le16(status);
- l2cap_send_cmd(conn, cmd->ident, L2CAP_CONN_RSP, sizeof(rsp), &rsp);
+ l2cap_send_cmd(conn, cmd->ident, rsp_code, sizeof(rsp), &rsp);
if (result == L2CAP_CR_PEND && status == L2CAP_CS_NO_INFO) {
struct l2cap_info_req info;
@@ -3516,7 +3517,12 @@ sendresp:
l2cap_build_conf_req(chan, buf), buf);
chan->num_conf_req++;
}
+}
+static int l2cap_connect_req(struct l2cap_conn *conn,
+ struct l2cap_cmd_hdr *cmd, u8 *data)
+{
+ __l2cap_connect(conn, cmd, data, L2CAP_CONN_RSP, 0);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [RFC 2/6] Bluetooth: Process create response and connect response identically
From: Andrei Emeltchenko @ 2012-10-11 14:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: mathewm
In-Reply-To: <1349966905-32336-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Mat Martineau <mathewm@codeaurora.org>
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
net/bluetooth/l2cap_core.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 1560108..632f291 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4038,14 +4038,6 @@ static inline int l2cap_create_channel_req(struct l2cap_conn *conn,
return 0;
}
-static inline int l2cap_create_channel_rsp(struct l2cap_conn *conn,
- struct l2cap_cmd_hdr *cmd, void *data)
-{
- BT_DBG("conn %p", conn);
-
- return l2cap_connect_rsp(conn, cmd, data);
-}
-
static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
u16 icid, u16 result)
{
@@ -4249,6 +4241,7 @@ static inline int l2cap_bredr_sig_cmd(struct l2cap_conn *conn,
break;
case L2CAP_CONN_RSP:
+ case L2CAP_CREATE_CHAN_RSP:
err = l2cap_connect_rsp(conn, cmd, data);
break;
@@ -4287,10 +4280,6 @@ static inline int l2cap_bredr_sig_cmd(struct l2cap_conn *conn,
err = l2cap_create_channel_req(conn, cmd, cmd_len, data);
break;
- case L2CAP_CREATE_CHAN_RSP:
- err = l2cap_create_channel_rsp(conn, cmd, data);
- break;
-
case L2CAP_MOVE_CHAN_REQ:
err = l2cap_move_channel_req(conn, cmd, cmd_len, data);
break;
--
1.7.9.5
^ permalink raw reply related
* [RFC 1/6] Bluetooth: Add new l2cap_chan struct members for high speed channels
From: Andrei Emeltchenko @ 2012-10-11 14:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: mathewm
From: Mat Martineau <mathewm@codeaurora.org>
An L2CAP channel using high speed continues to be associated with a
BR/EDR l2cap_conn, while also tracking an additional hci_conn
(representing a physical link on a high speed controller) and hci_chan
(representing a logical link). There may only be one physical link
between two high speed controllers. Each physical link may contain
several logical links, with each logical link representing a channel
with specific quality of service.
During a channel move, the destination channel id, current move state,
and role (initiator vs. responder) are tracked and used by the channel
move state machine. The ident value associated with a move request
must also be stored in order to use it in later move responses.
The active channel is stored in local_amp_id.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
include/net/bluetooth/l2cap.h | 29 +++++++++++++++++++++++++++++
net/bluetooth/l2cap_core.c | 5 +++++
2 files changed, 34 insertions(+)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index caab98c..ca40d12 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -434,6 +434,8 @@ struct l2cap_chan {
struct sock *sk;
struct l2cap_conn *conn;
+ struct hci_conn *hs_hcon;
+ struct hci_chan *hs_hchan;
struct kref kref;
__u8 state;
@@ -477,6 +479,11 @@ struct l2cap_chan {
unsigned long conn_state;
unsigned long flags;
+ __u8 local_amp_id;
+ __u8 move_id;
+ __u8 move_state;
+ __u8 move_role;
+
__u16 next_tx_seq;
__u16 expected_ack_seq;
__u16 expected_tx_seq;
@@ -643,6 +650,9 @@ enum {
enum {
L2CAP_RX_STATE_RECV,
L2CAP_RX_STATE_SREJ_SENT,
+ L2CAP_RX_STATE_MOVE,
+ L2CAP_RX_STATE_WAIT_P,
+ L2CAP_RX_STATE_WAIT_F,
};
enum {
@@ -673,6 +683,25 @@ enum {
L2CAP_EV_RECV_FRAME,
};
+enum {
+ L2CAP_MOVE_ROLE_NONE,
+ L2CAP_MOVE_ROLE_INITIATOR,
+ L2CAP_MOVE_ROLE_RESPONDER,
+};
+
+enum {
+ L2CAP_MOVE_STABLE,
+ L2CAP_MOVE_WAIT_REQ,
+ L2CAP_MOVE_WAIT_RSP,
+ L2CAP_MOVE_WAIT_RSP_SUCCESS,
+ L2CAP_MOVE_WAIT_CONFIRM,
+ L2CAP_MOVE_WAIT_CONFIRM_RSP,
+ L2CAP_MOVE_WAIT_LOGICAL_COMP,
+ L2CAP_MOVE_WAIT_LOGICAL_CFM,
+ L2CAP_MOVE_WAIT_LOCAL_BUSY,
+ L2CAP_MOVE_WAIT_PREPARE,
+};
+
void l2cap_chan_hold(struct l2cap_chan *c);
void l2cap_chan_put(struct l2cap_chan *c);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d42cdb1..1560108 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2784,6 +2784,11 @@ int l2cap_ertm_init(struct l2cap_chan *chan)
skb_queue_head_init(&chan->tx_q);
+ chan->local_amp_id = 0;
+ chan->move_id = 0;
+ chan->move_state = L2CAP_MOVE_STABLE;
+ chan->move_role = L2CAP_MOVE_ROLE_NONE;
+
if (chan->mode != L2CAP_MODE_ERTM)
return 0;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v3 01/10] doc: Add settings storage documentation
From: Frederic Danis @ 2012-10-11 14:38 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1349890310.27233.140.camel@aeonflux>
Hello Marcel,
On 10/10/2012 19:31, Marcel Holtmann wrote:
> Hi Fred,
>
>> doc/settings-storage.txt | 102 ++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 102 insertions(+)
>> create mode 100644 doc/settings-storage.txt
>>
>> diff --git a/doc/settings-storage.txt b/doc/settings-storage.txt
>> new file mode 100644
>> index 0000000..51d2220
>> --- /dev/null
>> +++ b/doc/settings-storage.txt
>> @@ -0,0 +1,102 @@
>> +Information related to local adapters and remote devices are stored in storage
>> +directory (default: /var/lib/bluetooth).
>> +Files are in ini-file format.
>> +
>> +There is one directory per adapter, named by its bluetooth address, which
>> +contains:
>> + - a settings file for the local adapter
>> + - an attribute_db file containing attributes of supported LE services
>> + - names file containing names of all devices already seen
>> + - one directory per remote device, named by remote device address, which
>> + contains:
>> + - a settings file
>> + - a key file accessible only by root
>> + - an attribute_db file containing attributes of remote LE services
>> +
>> +So the directory structure should be:
>> + /var/lib/bluetooth/<adapter address>/
>> + ./settings
>> + ./attribute_db
>
> not sure this is a good name.
Are you OK with "attributes" name for this file?
>> + ./names
>
> Lets call this one "cache". Since we could throw it away and it will be
> just rediscovered. And we can also stuff other cacheable details in
> there.
OK, I will replace it with cache directory, containing one file per
remote device.
>> + ./<remote device address>/
>> + ./settings
>
> Besides the Alias, are they really settings. You can not configure much
> anyway.
Rename it "info"?
>> + ./keys
>
> Not sure that I want the keys separated. That seems like a waste.
OK, I will merge it with previous file "info".
>> + ./attribute_db
>> + ./<remote device address>/
>> + ./settings
>> + ./keys
>> + ./attribute_db
>> + ...
>
> Why did we want directories for remote devices again? Can we not just
> put all in one big file with the name of the remote address?
Attribute_db (or whatever name we chose) stores the list of attributes
advertised by the remote device.
In this file I use the handle value as group name (primary key) as we
can get multiple entry with same UUID.
I think this should remain separated from "info" file, in order to parse
it more easily (just need to parse all groups in it to retrieve complete
attribute list).
That's why I think we should use one directory per remote device.
>> +
>> +Adapter directory files
>> +=======================
>> +
>> +Settings file contains 1 [General] group with adapter info:
>> + [General]
>> + Name=
>> + Class=0x000000
>> + Pairable=<true|false>
>> + PairableTimeout=0
>> + DiscoverableTimeout=0
>> + Mode=<off|connectable|discoverable>
>> + OnMode=<connectable|discoverable>
>
> Actually with management interface now being used, we can be a bit
> smarter here. The mode can be programmed even if the controller is not
> powered.
>
> So this might be better as Discoverable, Connectable, Pairable and
> Powered boolean options.
OK, I will replace Mode and OnMode keys by Discoverable, Connectable,
Pairable and Powered keys.
> Also Pairable has no timeout in the kernel API anymore. Do we still need
> this?
>
>> +The attribute_db file is a list of handles (group name) with UUID and Value as
>> +keys, for example:
>> + [0x0001]
>> + UUID=00002800-0000-1000-8000-00805f9b34fb
>> + Value=0018
>> +
>> + [0x0004]
>> + UUID=00002803-0000-1000-8000-00805f9b34fb
>> + Value=020600002A
>> +
>> + [0x0006]
>> + UUID=00002a00-0000-1000-8000-00805f9b34fb
>> + Value=4578616D706C6520446576696365
>
> Is this our primary key, the handle?
>
>> +
>> +Names file contains 1 [Names] group with device address as key:
>> + [Names]
>> + <nn:nn:nn:nn:nn:nn>=device name a
>> + <mm:mm:mm:mm:mm:mm>=device name b
>
> I am not sure this is the best idea this way. Maybe just creating files
> for each address we ever see is a better idea. Details like LastSeen
> could be useful for unpaired devices.
OK
Regarding LastSeen key, is it interesting to keep it in device "info"
file too?
So, we will open the corresponding cache/<device addr> file each time we
need to resolve remote device name, right?
>
>> +
>> +Device directory files
>> +======================
>> +
>> +Remote device settings file will include a [General] group with device infos
>> +and a [DeviceID] group with related infos:
>> + [General]
>> + Alias=
>> + Class=0x000000
>> + Manufacturer=0
>> + LmpVersion=
>> + LmpSubversion=
>
> Take these 3 our. We were just storing them for statistic purposes.
Should I remove them?
>> + Features=0000000000000000
>> + AddressType=<BR/EDR|LE>
>> + LEAddressType=<public|random>
>
> That is not needed. It is encoded in the address itself.
>
>> + LastSeen=yyyy-mm-dd hh:mm:ss GMT
>> + LastUsed=yyyy-mm-dd hh:mm:ss GMT
>> + Trusted=<true|false>
>> + Profiles=<128 bits UUID of profile 1>;<128 bits UUID of profile 2>;...
>> +
>> + [DeviceID]
>> + Assigner=0
>
> This is called Source.
OK, I will change this.
>> + Vendor=0
>> + Product=0
>> + Version=0
>> +
>> +Keys file includes informations related to link key or long term link key:
>> + [LinkKey]
>> + Key=
>> + Type=0
>> + PINLength=0
>> +
>> + [LongTermKey]
>> + Key=
>> + Authenticated=<true|false>
>> + EncSize=0
>> + EDiv=0
>> + Rand=0
>
> Can be both in the same file about the device information.
OK, I will merge them.
Regards
Fred
--
Frederic Danis Open Source Technology Center
frederic.danis@intel.com Intel Corporation
^ permalink raw reply
* Re: [PATCH] Bluetooth: SMP: Fix setting unknown auth_req bits
From: Andrei Emeltchenko @ 2012-10-11 14:34 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1349965566-29482-1-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
On Thu, Oct 11, 2012 at 04:26:06PM +0200, Johan Hedberg wrote:
> From: Johan Hedberg <johan.hedberg@intel.com>
>
> When sending a pairing request or response we should not just blindly
> copy the value that the remote device sent. Instead we should at least
> make sure to mask out any unknown bits. This is particularly critical
> from the upcoming LE Secure Connections feature perspective as
> incorrectly indicating support for it (by copying the remote value)
> would cause a failure to pair with devices that support it.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> Cc: stable@kernel.org
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> ---
> net/bluetooth/smp.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index 9b54fea..9176bc1 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -32,6 +32,8 @@
>
> #define SMP_TIMEOUT msecs_to_jiffies(30000)
>
> +#define AUTH_REQ_MASK 0x07
> +
> static inline void swap128(u8 src[16], u8 dst[16])
> {
> int i;
> @@ -230,7 +232,7 @@ static void build_pairing_cmd(struct l2cap_conn *conn,
> req->max_key_size = SMP_MAX_ENC_KEY_SIZE;
> req->init_key_dist = 0;
> req->resp_key_dist = dist_keys;
> - req->auth_req = authreq;
> + req->auth_req = (authreq & AUTH_REQ_MASK);
> return;
> }
>
> @@ -239,7 +241,7 @@ static void build_pairing_cmd(struct l2cap_conn *conn,
> rsp->max_key_size = SMP_MAX_ENC_KEY_SIZE;
> rsp->init_key_dist = 0;
> rsp->resp_key_dist = req->resp_key_dist & dist_keys;
> - rsp->auth_req = authreq;
> + rsp->auth_req = (authreq & AUTH_REQ_MASK);
Would it be better to omit braces like in the line above? For both cases.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [PATCH 1/4] Bluetooth: Fix L2CAP PSM bind issue
From: Luiz Augusto von Dentz @ 2012-10-11 14:29 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Syam Sidhardhan, linux-bluetooth
In-Reply-To: <1349963810.27233.178.camel@aeonflux>
Hi Marcel,
On Thu, Oct 11, 2012 at 3:56 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Syam,
>
>> >> Problem: If we bind a particular PSM with source address as BDADDR_ANY,
>> >> then we are able to bind the same PSM with adapter address(or any other
>> >> address), which is incorrect.
>> >
>> > why is this incorrect? Explain that to me.
>> >
>>
>> Here there is a correction required in the commit message.
>>
>> As per my understanding the a particular PSM can be binded only once
>> for a single adapter address. Kindly correct me if I'm wrong here.
>
> a PSM can be bound once per adapter address. And of course once per
> BDADDR_ANY.
>
> So you can bind PSM 23 to BDADDR_ANY and 11:22:33:44:55:66. An incoming
> connection to 11:22:33:44:55:66 will arrive to that specific socket, but
> an incoming to all others will be handled by the BDADDR_ANY.
>
> The BDADDR_ANY is special. Consider it a wildcard bound with lower
> priority. If overwritten with a specific address bound, that will be
> considered first of course.
>
> If we are not doing it that way, than that is a bug.
>
> Outgoing connections should behave the same btw. If a specific address
> is bound, than that will be used. If not available, then you can not
> connect. If BDADDR_ANY is used, then the next available adapter will be
> used.
The real problem is by giving psm 0 the kernel should return the first
available psm in the non-reserved space, but since we reuse the same
code to do the matching it end up given both obexd and hdp the same
psm, IMO in this case we should consider using Syam code and not
__l2cap_global_chan_by_addr as the userspace is probably asking for a
psm not in use which should exclude those in use by BDADDR_ANY.
--
Luiz Augusto von Dentz
^ permalink raw reply
* [PATCH] Bluetooth: SMP: Fix setting unknown auth_req bits
From: Johan Hedberg @ 2012-10-11 14:26 UTC (permalink / raw)
To: linux-bluetooth
From: Johan Hedberg <johan.hedberg@intel.com>
When sending a pairing request or response we should not just blindly
copy the value that the remote device sent. Instead we should at least
make sure to mask out any unknown bits. This is particularly critical
from the upcoming LE Secure Connections feature perspective as
incorrectly indicating support for it (by copying the remote value)
would cause a failure to pair with devices that support it.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Cc: stable@kernel.org
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
net/bluetooth/smp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 9b54fea..9176bc1 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -32,6 +32,8 @@
#define SMP_TIMEOUT msecs_to_jiffies(30000)
+#define AUTH_REQ_MASK 0x07
+
static inline void swap128(u8 src[16], u8 dst[16])
{
int i;
@@ -230,7 +232,7 @@ static void build_pairing_cmd(struct l2cap_conn *conn,
req->max_key_size = SMP_MAX_ENC_KEY_SIZE;
req->init_key_dist = 0;
req->resp_key_dist = dist_keys;
- req->auth_req = authreq;
+ req->auth_req = (authreq & AUTH_REQ_MASK);
return;
}
@@ -239,7 +241,7 @@ static void build_pairing_cmd(struct l2cap_conn *conn,
rsp->max_key_size = SMP_MAX_ENC_KEY_SIZE;
rsp->init_key_dist = 0;
rsp->resp_key_dist = req->resp_key_dist & dist_keys;
- rsp->auth_req = authreq;
+ rsp->auth_req = (authreq & AUTH_REQ_MASK);
}
static u8 check_enc_key_size(struct l2cap_conn *conn, __u8 max_key_size)
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v3 01/10] doc: Add settings storage documentation
From: Johan Hedberg @ 2012-10-11 14:24 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Frédéric Danis, linux-bluetooth
In-Reply-To: <1349962908.27233.164.camel@aeonflux>
Hi Marcel,
On Thu, Oct 11, 2012, Marcel Holtmann wrote:
> > > > + Features=0000000000000000
> > > > + AddressType=<BR/EDR|LE>
> > > > + LEAddressType=<public|random>
> > >
> > > That is not needed. It is encoded in the address itself.
> >
> > Not quite true. You cannot distinguish between public and random. But
> > once you do know that it's random you *can* figure out what kind of
> > random it is from the two most significant bits of the address.
> >
> > What we discussed on #bluez was that this could simply be a reference to
> > what address is indicated by the directory (or filename, if that's what
> > we go with) for the device storage, e.g. AddressType=<static|public> and
> > for private resolvable addresses (for which we'd have the public address
> > in storage) we'd need to check if we have any IRK's to know if we need
> > to look for a private address to connect to them. Additionally we could
> > have a SupportedTechnologies list like "BR/EDR,LE", "LE", etc (feel free
> > to suggest a better name if you want).
>
> Since we will not store private random addresses, and we turn static
> random addresses into a cache. So why do we still need this?
We will need to store exactly the same information for static random
addresses as for public ones, and not just the "core" data but also
profile-specific data. So why would you want to do it in two different
ways? Sounds like that just complicates the implementation for no
particular gain.
Johan
^ permalink raw reply
* Re: [PATCH 1/4] Bluetooth: Fix L2CAP PSM bind issue
From: Marcel Holtmann @ 2012-10-11 13:56 UTC (permalink / raw)
To: Syam Sidhardhan; +Cc: linux-bluetooth
In-Reply-To: <E3660838E8D64186924BA81A32F30682@sisodomain.com>
Hi Syam,
> >> Problem: If we bind a particular PSM with source address as BDADDR_ANY,
> >> then we are able to bind the same PSM with adapter address(or any other
> >> address), which is incorrect.
> >
> > why is this incorrect? Explain that to me.
> >
>
> Here there is a correction required in the commit message.
>
> As per my understanding the a particular PSM can be binded only once
> for a single adapter address. Kindly correct me if I'm wrong here.
a PSM can be bound once per adapter address. And of course once per
BDADDR_ANY.
So you can bind PSM 23 to BDADDR_ANY and 11:22:33:44:55:66. An incoming
connection to 11:22:33:44:55:66 will arrive to that specific socket, but
an incoming to all others will be handled by the BDADDR_ANY.
The BDADDR_ANY is special. Consider it a wildcard bound with lower
priority. If overwritten with a specific address bound, that will be
considered first of course.
If we are not doing it that way, than that is a bug.
Outgoing connections should behave the same btw. If a specific address
is bound, than that will be used. If not available, then you can not
connect. If BDADDR_ANY is used, then the next available adapter will be
used.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH -v2 8/8] Bluetooth: Call ops->teardown() without checking for NULL
From: Marcel Holtmann @ 2012-10-11 13:52 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan
In-Reply-To: <1349938311-25101-8-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> Users that don't implement teardown() should use l2cap_chan_no_teardown()
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
> net/bluetooth/l2cap_core.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH -v2 7/8] Bluetooth: Use locked l2cap_state_change()
From: Marcel Holtmann @ 2012-10-11 13:52 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan
In-Reply-To: <1349938311-25101-7-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> No one was protecting the state set in l2cap_send_disconn_req()
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
> net/bluetooth/l2cap_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH -v2 6/8] Bluetooth: use l2cap_chan_set_err()
From: Marcel Holtmann @ 2012-10-11 13:51 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan
In-Reply-To: <1349938311-25101-6-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> l2cap_conn_unreliable() doesn't take the sk lock, so we need to take it
> using l2cap_chan_set_err().
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
> net/bluetooth/l2cap_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH -v2 5/8] Bluetooth: Remove GFP_ATOMIC usage from l2cap_core.c
From: Marcel Holtmann @ 2012-10-11 13:50 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan
In-Reply-To: <1349938311-25101-5-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> Since we change the Bluetooth core to run in process context we don't need
> to use GFP_ATOMIC in many of places we were using it. The we just replace
> by GFP_KERNEL.
I assume you double-checked that we nowhere hold a lock or the caller
holds a lock.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
> net/bluetooth/l2cap_core.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH -v2 4/8] Bluetooth: Add chan->ops->defer()
From: Marcel Holtmann @ 2012-10-11 13:49 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan
In-Reply-To: <1349938311-25101-4-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> When DEFER_SETUP is set defer() will trigger an authorization
> request to the userspace.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
> include/net/bluetooth/l2cap.h | 5 +++++
> net/bluetooth/a2mp.c | 1 +
> net/bluetooth/l2cap_core.c | 10 +++-------
> net/bluetooth/l2cap_sock.c | 13 +++++++++++++
> 4 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index caab98c..6e23afd 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -541,6 +541,7 @@ struct l2cap_ops {
> void (*state_change) (struct l2cap_chan *chan,
> int state);
> void (*ready) (struct l2cap_chan *chan);
> + void (*defer) (struct l2cap_chan *chan);
> struct sk_buff *(*alloc_skb) (struct l2cap_chan *chan,
> unsigned long len, int nb);
> };
> @@ -748,6 +749,10 @@ static inline void l2cap_chan_no_ready(struct l2cap_chan *chan)
> {
> }
>
> +static inline void l2cap_chan_no_defer(struct l2cap_chan *chan)
> +{
> +}
> +
> extern bool disable_ertm;
>
> int l2cap_init_sockets(void);
> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
> index 3ff4dc9..7bf9a10 100644
> --- a/net/bluetooth/a2mp.c
> +++ b/net/bluetooth/a2mp.c
> @@ -699,6 +699,7 @@ static struct l2cap_ops a2mp_chan_ops = {
> .new_connection = l2cap_chan_no_new_connection,
> .teardown = l2cap_chan_no_teardown,
> .ready = l2cap_chan_no_ready,
> + .defer = l2cap_chan_no_defer,
> };
>
> static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn, bool locked)
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 91d312b..a2f945d 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1123,11 +1123,9 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
> lock_sock(sk);
> if (test_bit(BT_SK_DEFER_SETUP,
> &bt_sk(sk)->flags)) {
> - struct sock *parent = bt_sk(sk)->parent;
> rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
> rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
> - if (parent)
> - parent->sk_data_ready(parent, 0);
> + chan->ops->defer(chan);
I would prefer you explain this in detail. Sine the no_defer is then no
longer calling sk_data_ready. Is this safe?
>
> } else {
> __l2cap_state_change(chan, BT_CONFIG);
> @@ -3459,7 +3457,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn,
> __l2cap_state_change(chan, BT_CONNECT2);
> result = L2CAP_CR_PEND;
> status = L2CAP_CS_AUTHOR_PEND;
> - parent->sk_data_ready(parent, 0);
> + chan->ops->defer(chan);
> } else {
> __l2cap_state_change(chan, BT_CONFIG);
> result = L2CAP_CR_SUCCESS;
> @@ -5520,11 +5518,9 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> if (!status) {
> if (test_bit(BT_SK_DEFER_SETUP,
> &bt_sk(sk)->flags)) {
> - struct sock *parent = bt_sk(sk)->parent;
> res = L2CAP_CR_PEND;
> stat = L2CAP_CS_AUTHOR_PEND;
> - if (parent)
> - parent->sk_data_ready(parent, 0);
> + chan->ops->defer(chan);
> } else {
> __l2cap_state_change(chan, BT_CONFIG);
> res = L2CAP_CR_SUCCESS;
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 45c81e1..efcb914 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -956,6 +956,7 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
>
> bt_accept_enqueue(parent, sk);
>
> +
No pointless empty lines please.
> release_sock(parent);
>
> return l2cap_pi(sk)->chan;
> @@ -1088,6 +1089,17 @@ static void l2cap_sock_ready_cb(struct l2cap_chan *chan)
> release_sock(sk);
> }
>
> +static void l2cap_sock_defer_cb(struct l2cap_chan *chan)
> +{
> + struct sock *sk = chan->data;
> + struct sock *parent;
> +
> + parent = bt_sk(sk)->parent;
You can do struct sock *parent = bt_sk(sk)-parent here.
> +
> + if (parent)
> + parent->sk_data_ready(parent, 0);
> +}
> +
> static struct l2cap_ops l2cap_chan_ops = {
> .name = "L2CAP Socket Interface",
> .new_connection = l2cap_sock_new_connection_cb,
> @@ -1096,6 +1108,7 @@ static struct l2cap_ops l2cap_chan_ops = {
> .teardown = l2cap_sock_teardown_cb,
> .state_change = l2cap_sock_state_change_cb,
> .ready = l2cap_sock_ready_cb,
> + .defer = l2cap_sock_defer_cb,
> .alloc_skb = l2cap_sock_alloc_skb_cb,
> };
>
I am bit unease with accidentally having no_defer and then DEFER_SETUP
is set. Are we not changing behavior here?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH -v2 3/8] Bluetooth: Remove parent socket usage from l2cap_core.c
From: Marcel Holtmann @ 2012-10-11 13:45 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan
In-Reply-To: <1349938311-25101-3-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> We can lock the parent lock only inside the new_connection() call,
> then we just use the l2cap_chan_lock() in core code.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
> net/bluetooth/l2cap_core.c | 16 ++++++----------
> net/bluetooth/l2cap_sock.c | 9 ++++++++-
> 2 files changed, 14 insertions(+), 11 deletions(-)
I am fine with this patch under the premise that 2/8 is properly
explained. And here you need to also get into a bit more detail. With
any kind of locking changes, I expect a proper commit message explaining
why this is safe.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH -v2 2/8] Bluetooth: Move bt_accept_enqueue() to l2cap_sock.c
From: Marcel Holtmann @ 2012-10-11 13:44 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan
In-Reply-To: <1349938311-25101-2-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> This is move the parent socket usage to l2cap_sock.c
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
> net/bluetooth/l2cap_core.c | 4 ----
> net/bluetooth/l2cap_sock.c | 2 ++
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index b382631..e10a1a3 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1233,8 +1233,6 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
> bacpy(&bt_sk(sk)->src, conn->src);
> bacpy(&bt_sk(sk)->dst, conn->dst);
>
> - bt_accept_enqueue(parent, sk);
> -
> l2cap_chan_add(conn, chan);
>
> l2cap_chan_ready(chan);
> @@ -3451,8 +3449,6 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn,
> chan->psm = psm;
> chan->dcid = scid;
>
> - bt_accept_enqueue(parent, sk);
> -
> __l2cap_chan_add(conn, chan);
>
> dcid = chan->scid;
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index f95fc7e..d5093b8 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -949,6 +949,8 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
>
> l2cap_sock_init(sk, parent);
>
> + bt_accept_enqueue(parent, sk);
> +
> return l2cap_pi(sk)->chan;
> }
>
and this has no side effect with adding during this window. Your commit
message is not really explaining why this is fine. Please fix that.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH -v2 1/8] Bluetooth: Fix L2CAP coding style
From: Marcel Holtmann @ 2012-10-11 13:43 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan
In-Reply-To: <1349938311-25101-1-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> Follow the net subsystem coding style
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
> net/bluetooth/l2cap_core.c | 313 ++++++++++++++++++++++++---------------------
> net/bluetooth/l2cap_sock.c | 62 +++++----
> 2 files changed, 202 insertions(+), 173 deletions(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v3 01/10] doc: Add settings storage documentation
From: Marcel Holtmann @ 2012-10-11 13:41 UTC (permalink / raw)
To: Johan Hedberg; +Cc: Frédéric Danis, linux-bluetooth
In-Reply-To: <20121010184103.GA23441@x220.ger.corp.intel.com>
Hi Johan,
> > > + ./attribute_db
> > > + ./<remote device address>/
> > > + ./settings
> > > + ./keys
> > > + ./attribute_db
> > > + ...
> >
> > Why did we want directories for remote devices again? Can we not just
> > put all in one big file with the name of the remote address?
>
> For profile-specific storage this could be convenient (when removing the
> device we just remove the entire directory and the core daemon doesn't
> need to care about profiles details). OTOH the core needs to either way
> provide some API so the plugins know to write to the right location
> (be it a single file for the entire device or a single directory for the
> device).
fair enough. And a generic function to return the path for the device
specific storage location is a good idea anyway. So the profile does
need to care.
However we could also force the profile implementation to use a specific
file with such a function. And we could derive the name from the driver
name entry.
So we could return <device-bdaddr>-magic-profile for storage.
> > > +Adapter directory files
> > > +=======================
> > > +
> > > +Settings file contains 1 [General] group with adapter info:
> > > + [General]
> > > + Name=
> > > + Class=0x000000
> > > + Pairable=<true|false>
> > > + PairableTimeout=0
> > > + DiscoverableTimeout=0
> > > + Mode=<off|connectable|discoverable>
> > > + OnMode=<connectable|discoverable>
> >
> > Actually with management interface now being used, we can be a bit
> > smarter here. The mode can be programmed even if the controller is not
> > powered.
> >
> > So this might be better as Discoverable, Connectable, Pairable and
> > Powered boolean options.
> >
> > Also Pairable has no timeout in the kernel API anymore. Do we still need
> > this?
>
> We do have it currently as a documented adapter D-Bus property so that'd
> need to be removed too. I don't personally have a great need for this
> but I could imagine some device manufacturers deciding to have something
> like this (especially those with very simple UI's with a single button
> or so).
Fair enough. I was just asking if we still want the pairable timeout. I
have no problem with it.
> > > +The attribute_db file is a list of handles (group name) with UUID and Value as
> > > +keys, for example:
> > > + [0x0001]
> > > + UUID=00002800-0000-1000-8000-00805f9b34fb
> > > + Value=0018
> > > +
> > > + [0x0004]
> > > + UUID=00002803-0000-1000-8000-00805f9b34fb
> > > + Value=020600002A
> > > +
> > > + [0x0006]
> > > + UUID=00002a00-0000-1000-8000-00805f9b34fb
> > > + Value=4578616D706C6520446576696365
> >
> > Is this our primary key, the handle?
>
> Yep, the reasoning was that you can easily have multiple
> profiles/services with the same UUID with GATT, so the UUID is no good
> as a unique identifier (not that GLib even supports multiple groups with
> the same name - it merges their content together to a single logical
> group).
>
> > > +Names file contains 1 [Names] group with device address as key:
> > > + [Names]
> > > + <nn:nn:nn:nn:nn:nn>=device name a
> > > + <mm:mm:mm:mm:mm:mm>=device name b
> >
> > I am not sure this is the best idea this way. Maybe just creating files
> > for each address we ever see is a better idea. Details like LastSeen
> > could be useful for unpaired devices.
>
> So instead of a cache file we'd have a cache directory? I'd be fine with
> that.
Something like that. Details need to be figured out, but I like to make
it bluntly clear that this data could be purged.
>
> > > + Features=0000000000000000
> > > + AddressType=<BR/EDR|LE>
> > > + LEAddressType=<public|random>
> >
> > That is not needed. It is encoded in the address itself.
>
> Not quite true. You cannot distinguish between public and random. But
> once you do know that it's random you *can* figure out what kind of
> random it is from the two most significant bits of the address.
>
> What we discussed on #bluez was that this could simply be a reference to
> what address is indicated by the directory (or filename, if that's what
> we go with) for the device storage, e.g. AddressType=<static|public> and
> for private resolvable addresses (for which we'd have the public address
> in storage) we'd need to check if we have any IRK's to know if we need
> to look for a private address to connect to them. Additionally we could
> have a SupportedTechnologies list like "BR/EDR,LE", "LE", etc (feel free
> to suggest a better name if you want).
Since we will not store private random addresses, and we turn static
random addresses into a cache. So why do we still need this?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/4] Bluetooth: Fix L2CAP PSM bind issue
From: Syam Sidhardhan @ 2012-10-11 11:19 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1349891245.27233.153.camel@aeonflux>
Hi Marcel,
----- Original Message -----
From: "Marcel Holtmann" <marcel@holtmann.org>
To: "Syam Sidhardhan" <s.syam@samsung.com>
Cc: <linux-bluetooth@vger.kernel.org>
Sent: Wednesday, October 10, 2012 11:17 PM
Subject: Re: [PATCH 1/4] Bluetooth: Fix L2CAP PSM bind issue
> Hi Syam,
>
>> Problem: If we bind a particular PSM with source address as BDADDR_ANY,
>> then we are able to bind the same PSM with adapter address(or any other
>> address), which is incorrect.
>
> why is this incorrect? Explain that to me.
>
Here there is a correction required in the commit message.
As per my understanding the a particular PSM can be binded only once
for a single adapter address. Kindly correct me if I'm wrong here.
As I mentioned in the additional comment part of the patch, there is
a PSM conflict happens between the Obexd and HDP, because of the
source address differences. With this patch, it's getting resolved and
I'm able to run Obexd and HDP simultaniously.
The otherway to fix it, is in userspace. Always provide the source adapter
address during the PSM binding, rather than providing BDADDR_ANY.
If this patch code changes are valid, then I can send a seperate version
patch with
the following corrected commit message description.
"Problem: If we bind a particular PSM with source address as BDADDR_ANY,
then we are able to bind the same PSM with adapter address." ...
Kindly let me know.
Thanks,
Syam.
^ permalink raw reply
* Re: [PATCH BlueZ 1/2] hog: Fix characteristic descriptor discovery
From: Johan Hedberg @ 2012-10-11 7:04 UTC (permalink / raw)
To: João Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <1349933483-10436-1-git-send-email-jprvita@openbossa.org>
João Paulo,
On Thu, Oct 11, 2012, João Paulo Rechi Vita wrote:
> The discover descriptors sub-procedure is complete when the error
> response is received and the error code is set to "Attribute Not Found"
> or the find information response has an attribute handle that is equal
> to the last handle in the request. This commit fixes the stop condition
> for characteristic descriptor discovery.
> ---
> profiles/input/hog_device.c | 46 ++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 37 insertions(+), 9 deletions(-)
Both patches have been applied. Thanks.
I had to do some manual fix-up on the first patch due to conflicts with
patches from Vinicius so please check that I didn't make any mistakes.
Johan
^ permalink raw reply
* Re: [PATCH BlueZ] core: Fix connecting to an already connected device on pairing
From: Johan Hedberg @ 2012-10-11 6:58 UTC (permalink / raw)
To: João Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <1349933446-10391-1-git-send-email-jprvita@openbossa.org>
João Paulo,
On Thu, Oct 11, 2012, João Paulo Rechi Vita wrote:
> ---
> src/device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH BlueZ] test: Add support for passing address type to btiotest
From: Johan Hedberg @ 2012-10-11 6:57 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1349913403-25606-1-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
On Wed, Oct 10, 2012, Vinicius Costa Gomes wrote:
> ---
> test/btiotest.c | 51 +++++++++++++++++++++++++++++----------------------
> 1 file changed, 29 insertions(+), 22 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH BlueZ 1/5] gattrib: Add support for listening for events for specific handles
From: Johan Hedberg @ 2012-10-11 6:57 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1349913357-25547-1-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
On Wed, Oct 10, 2012, Vinicius Costa Gomes wrote:
> We want only the profile that implements a service to be notified of
> changes on that service. Before this patch, all the registered event
> notifiers are being called.
> ---
> Hi,
>
> I am re-sending this series because, if it makes sense, it would be awesome
> if it gets the "UPF treatment".
>
> Cheers,
>
>
> attrib/client.c | 4 ++--
> attrib/gattrib.c | 33 ++++++++++++++++++++++++++++-----
> attrib/gattrib.h | 7 ++++---
> attrib/gatttool.c | 8 ++++----
> attrib/interactive.c | 8 ++++----
> profiles/gatt/gas.c | 1 +
> profiles/heartrate/heartrate.c | 2 +-
> profiles/input/hog_device.c | 5 +++--
> profiles/scanparam/scan.c | 4 ++--
> profiles/thermometer/thermometer.c | 2 ++
> src/attrib-server.c | 2 +-
> 11 files changed, 52 insertions(+), 24 deletions(-)
I went and applied this set too as it looks good. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH BlueZ 1/6] gattrib: Fix ignoring the error message when write fails
From: Johan Hedberg @ 2012-10-11 6:57 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1349912103-663-1-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
On Wed, Oct 10, 2012, Vinicius Costa Gomes wrote:
> If an error happens during writing to the socket, we should complain
> that it failed.
> ---
> attrib/gattrib.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
All patches in this set have been applied. Thanks.
Johan
^ permalink raw reply
* [PATCH -v2 8/8] Bluetooth: Call ops->teardown() without checking for NULL
From: Gustavo Padovan @ 2012-10-11 6:51 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Gustavo Padovan
In-Reply-To: <1349938311-25101-1-git-send-email-gustavo@padovan.org>
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Users that don't implement teardown() should use l2cap_chan_no_teardown()
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
net/bluetooth/l2cap_core.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 4c83157..c3a2c1c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -542,8 +542,7 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
hci_conn_put(conn->hcon);
}
- if (chan->ops->teardown)
- chan->ops->teardown(chan, err);
+ chan->ops->teardown(chan, err);
if (test_bit(CONF_NOT_COMPLETE, &chan->conf_state))
return;
@@ -582,8 +581,7 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
switch (chan->state) {
case BT_LISTEN:
- if (chan->ops->teardown)
- chan->ops->teardown(chan, 0);
+ chan->ops->teardown(chan, 0);
break;
case BT_CONNECTED:
@@ -625,8 +623,7 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
break;
default:
- if (chan->ops->teardown)
- chan->ops->teardown(chan, 0);
+ chan->ops->teardown(chan, 0);
break;
}
}
--
1.7.11.4
^ 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