[PATCHv2 00/19] L2CAP signaling for AMP channel create/move

[PATCHv2 00/19] L2CAP signaling for AMP channel create/move

[Mat Martineau]

Here are the changes that process commands on the L2CAP signaling
channel for setting up AMP channels.  There's still a lot of
integration to do as other AMP functionality is implemented.  I've
marked places that require this integration with "Placeholder"
comments (look for that string).

Changes:
  * PATCHv2 - Fixed formatting and style issues, added acks
  * PATCHv1 - Rebased after AMP physical link patch set, incorporated
              mailing list feedback.
  * RFCv1 - Finished commit messages, fixed formatting/style issues
  * RFCv0 - Initial post

Mat Martineau (19):
  Bluetooth: Add new l2cap_chan struct members for high speed channels
  Bluetooth: Add L2CAP create channel request handling
  Bluetooth: Remove unnecessary intermediate function
  Bluetooth: Lookup channel structure based on DCID
  Bluetooth: Channel move request handling
  Bluetooth: Add new ERTM receive states for channel move
  Bluetooth: Add move channel confirm handling
  Bluetooth: Add state to hci_chan
  Bluetooth: Move channel response
  Bluetooth: Add logical link confirm
  Bluetooth: Add move confirm response handling
  Bluetooth: Handle physical link completion
  Bluetooth: Flag ACL frames as complete for AMP controllers
  Bluetooth: Do not send data during channel move
  Bluetooth: Configure appropriate timeouts for AMP controllers
  Bluetooth: Ignore BR/EDR packet size constraints when fragmenting for
    AMP
  Bluetooth: Send create channel request instead of connect for AMP
  Bluetooth: Do not retransmit data during a channel move
  Bluetooth: Start channel move when socket option is changed

 include/net/bluetooth/hci_core.h |   1 +
 include/net/bluetooth/l2cap.h    |  32 ++
 net/bluetooth/hci_conn.c         |   1 +
 net/bluetooth/l2cap_core.c       | 973 +++++++++++++++++++++++++++++++++++++--
 net/bluetooth/l2cap_sock.c       |   5 +
 5 files changed, 972 insertions(+), 40 deletions(-)

-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 01/19] Bluetooth: Add new l2cap_chan struct members for high speed channels

[Mat Martineau]

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>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 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 6e23afd..6d3615e 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;
@@ -644,6 +651,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 {
@@ -674,6 +684,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 f873619..f8d78f5 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2788,6 +2788,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.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 02/19] Bluetooth: Add L2CAP create channel request handling

[Mat Martineau]

The L2CAP create channel request is very similar to an L2CAP connect
request, but it has an additional parameter for the controller ID.  If
the controller id is 0, the channel is set up on the BR/EDR controller
(just like a connect request).  Using a valid high speed controller ID
will cause the channel to be initially created on that high speed
controller.  While the L2CAP data will be initially routed over the
AMP controller, the L2CAP fixed signaling channel only uses BR/EDR.

When a create channel request is received for a high speed controller,
a pending response is always sent first.  After the high speed
physical and logical links are complete a success response will be
sent.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap_core.c | 62 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 47 insertions(+), 15 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index f8d78f5..73ce337 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3400,8 +3400,9 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn,
 	return 0;
 }
 
-static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
-			  u8 *data, u8 rsp_code, u8 amp_id)
+static struct l2cap_chan *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;
@@ -3452,6 +3453,7 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
 	bacpy(&bt_sk(sk)->dst, conn->dst);
 	chan->psm  = psm;
 	chan->dcid = scid;
+	chan->local_amp_id = amp_id;
 
 	__l2cap_chan_add(conn, chan);
 
@@ -3469,8 +3471,16 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
 				status = L2CAP_CS_AUTHOR_PEND;
 				chan->ops->defer(chan);
 			} else {
-				__l2cap_state_change(chan, BT_CONFIG);
-				result = L2CAP_CR_SUCCESS;
+				/* Force pending result for AMP controllers.
+				 * The connection will succeed after the
+				 * physical link is up. */
+				if (amp_id) {
+					__l2cap_state_change(chan, BT_CONNECT2);
+					result = L2CAP_CR_PEND;
+				} else {
+					__l2cap_state_change(chan, BT_CONFIG);
+					result = L2CAP_CR_SUCCESS;
+				}
 				status = L2CAP_CS_NO_INFO;
 			}
 		} else {
@@ -3516,6 +3526,8 @@ sendresp:
 			       l2cap_build_conf_req(chan, buf), buf);
 		chan->num_conf_req++;
 	}
+
+	return chan;
 }
 
 static int l2cap_connect_req(struct l2cap_conn *conn,
@@ -4028,12 +4040,12 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn,
 	return 0;
 }
 
-static inline int l2cap_create_channel_req(struct l2cap_conn *conn,
-					   struct l2cap_cmd_hdr *cmd,
-					   u16 cmd_len, void *data)
+static int l2cap_create_channel_req(struct l2cap_conn *conn,
+				    struct l2cap_cmd_hdr *cmd,
+				    u16 cmd_len, void *data)
 {
 	struct l2cap_create_chan_req *req = data;
-	struct l2cap_create_chan_rsp rsp;
+	struct l2cap_chan *chan;
 	u16 psm, scid;
 
 	if (cmd_len != sizeof(*req))
@@ -4047,14 +4059,34 @@ static inline int l2cap_create_channel_req(struct l2cap_conn *conn,
 
 	BT_DBG("psm 0x%2.2x, scid 0x%4.4x, amp_id %d", psm, scid, req->amp_id);
 
-	/* Placeholder: Always reject */
-	rsp.dcid = 0;
-	rsp.scid = cpu_to_le16(scid);
-	rsp.result = __constant_cpu_to_le16(L2CAP_CR_NO_MEM);
-	rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
+	if (req->amp_id) {
+		struct hci_dev *hdev;
 
-	l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
-		       sizeof(rsp), &rsp);
+		/* Validate AMP controller id */
+		hdev = hci_dev_get(req->amp_id);
+		if (!hdev || hdev->dev_type != HCI_AMP ||
+		    !test_bit(HCI_UP, &hdev->flags)) {
+			struct l2cap_create_chan_rsp rsp;
+
+			rsp.dcid = 0;
+			rsp.scid = cpu_to_le16(scid);
+			rsp.result = __constant_cpu_to_le16(L2CAP_CR_BAD_AMP);
+			rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
+
+			l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
+				       sizeof(rsp), &rsp);
+
+			if (hdev)
+				hci_dev_put(hdev);
+
+			return 0;
+		}
+
+		hci_dev_put(hdev);
+	}
+
+	chan = l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
+			     req->amp_id);
 
 	return 0;
 }
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 03/19] Bluetooth: Remove unnecessary intermediate function

[Mat Martineau]

Resolves a conflict resolution issue in "Bluetooth: Fix L2CAP coding
style".

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap_core.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 73ce337..ec2b4d9 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3537,7 +3537,7 @@ static int l2cap_connect_req(struct l2cap_conn *conn,
 	return 0;
 }
 
-static inline int l2cap_connect_rsp(struct l2cap_conn *conn,
+static int l2cap_connect_create_rsp(struct l2cap_conn *conn,
 				    struct l2cap_cmd_hdr *cmd, u8 *data)
 {
 	struct l2cap_conn_rsp *rsp = (struct l2cap_conn_rsp *) data;
@@ -4091,15 +4091,6 @@ static 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)
 {
@@ -4306,7 +4297,7 @@ static inline int l2cap_bredr_sig_cmd(struct l2cap_conn *conn,
 
 	case L2CAP_CONN_RSP:
 	case L2CAP_CREATE_CHAN_RSP:
-		err = l2cap_connect_rsp(conn, cmd, data);
+		err = l2cap_connect_create_rsp(conn, cmd, data);
 		break;
 
 	case L2CAP_CONF_REQ:
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 04/19] Bluetooth: Lookup channel structure based on DCID

[Mat Martineau]

Processing a move channel request involves getting the channel
structure using the destination channel ID.  Previous code could only
look up using the source channel ID.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ec2b4d9..b5b849b 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -100,6 +100,22 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn,
 	return c;
 }
 
+/* Find channel with given DCID.
+ * Returns locked channel. */
+static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
+						 u16 cid)
+{
+	struct l2cap_chan *c;
+
+	mutex_lock(&conn->chan_lock);
+	c = __l2cap_get_chan_by_dcid(conn, cid);
+	if (c)
+		l2cap_chan_lock(c);
+	mutex_unlock(&conn->chan_lock);
+
+	return c;
+}
+
 static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn,
 						    u8 ident)
 {
@@ -4139,6 +4155,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
 					 u16 cmd_len, void *data)
 {
 	struct l2cap_move_chan_req *req = data;
+	struct l2cap_chan *chan;
 	u16 icid = 0;
 	u16 result = L2CAP_MR_NOT_ALLOWED;
 
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 04/19] Bluetooth: Lookup channel structure based on DCID

[Marcel Holtmann]

Hi Mat,

> Processing a move channel request involves getting the channel
> structure using the destination channel ID.  Previous code could only
> look up using the source channel ID.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCHv2 04/19] Bluetooth: Lookup channel structure based on DCID

[Andrei Emeltchenko]

Hi Mat,

On Tue, Oct 16, 2012 at 04:36:18PM -0700, Mat Martineau wrote:
> Processing a move channel request involves getting the channel
> structure using the destination channel ID.  Previous code could only
> look up using the source channel ID.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index ec2b4d9..b5b849b 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -100,6 +100,22 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn,
>  	return c;
>  }
>  
> +/* Find channel with given DCID.
> + * Returns locked channel. */
> +static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
> +						 u16 cid)
> +{
> +	struct l2cap_chan *c;
> +
> +	mutex_lock(&conn->chan_lock);
> +	c = __l2cap_get_chan_by_dcid(conn, cid);
> +	if (c)
> +		l2cap_chan_lock(c);
> +	mutex_unlock(&conn->chan_lock);
> +
> +	return c;
> +}
> +
>  static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn,
>  						    u8 ident)
>  {
> @@ -4139,6 +4155,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>  					 u16 cmd_len, void *data)
>  {
>  	struct l2cap_move_chan_req *req = data;
> +	struct l2cap_chan *chan;

This line seems do not belong to the patch.

Best regards 
Andrei Emeltchenko 

>  	u16 icid = 0;
>  	u16 result = L2CAP_MR_NOT_ALLOWED;

Re: [PATCHv2 04/19] Bluetooth: Lookup channel structure based on DCID

[Mat Martineau]

Andrei -

On Thu, 18 Oct 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Tue, Oct 16, 2012 at 04:36:18PM -0700, Mat Martineau wrote:
>> Processing a move channel request involves getting the channel
>> structure using the destination channel ID.  Previous code could only
>> look up using the source channel ID.
>>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  net/bluetooth/l2cap_core.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index ec2b4d9..b5b849b 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -100,6 +100,22 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn,
>>  	return c;
>>  }
>>
>> +/* Find channel with given DCID.
>> + * Returns locked channel. */
>> +static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
>> +						 u16 cid)
>> +{
>> +	struct l2cap_chan *c;
>> +
>> +	mutex_lock(&conn->chan_lock);
>> +	c = __l2cap_get_chan_by_dcid(conn, cid);
>> +	if (c)
>> +		l2cap_chan_lock(c);
>> +	mutex_unlock(&conn->chan_lock);
>> +
>> +	return c;
>> +}
>> +
>>  static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn,
>>  						    u8 ident)
>>  {
>> @@ -4139,6 +4155,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>>  					 u16 cmd_len, void *data)
>>  {
>>  	struct l2cap_move_chan_req *req = data;
>> +	struct l2cap_chan *chan;
>
> This line seems do not belong to the patch.

Yes, this line should be moved to patch 5 with the other changes to 
this function.

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 05/19] Bluetooth: Channel move request handling

[Mat Martineau]

On receipt of a channel move request, the request must be validated
based on the L2CAP mode, connection state, and controller
capabilities.  ERTM channels must have their state machines cleared
and transmission paused while the channel move takes place.

If the channel is being moved to an AMP controller then
an AMP physical link must be prepared.  Moving the channel back to
BR/EDR proceeds immediately.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap_core.c | 108 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index b5b849b..2fb0567 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -734,6 +734,12 @@ static void l2cap_send_cmd(struct l2cap_conn *conn, u8 ident, u8 code, u16 len,
 	hci_send_acl(conn->hchan, skb, flags);
 }
 
+static bool __chan_is_moving(struct l2cap_chan *chan)
+{
+	return chan->move_state != L2CAP_MOVE_STABLE &&
+	       chan->move_state != L2CAP_MOVE_WAIT_PREPARE;
+}
+
 static void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
 {
 	struct hci_conn *hcon = chan->conn->hcon;
@@ -995,6 +1001,41 @@ void l2cap_send_conn_req(struct l2cap_chan *chan)
 	l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_REQ, sizeof(req), &req);
 }
 
+static void l2cap_move_setup(struct l2cap_chan *chan)
+{
+	struct sk_buff *skb;
+
+	BT_DBG("chan %p", chan);
+
+	if (chan->mode != L2CAP_MODE_ERTM)
+		return;
+
+	__clear_retrans_timer(chan);
+	__clear_monitor_timer(chan);
+	__clear_ack_timer(chan);
+
+	chan->retry_count = 0;
+	skb_queue_walk(&chan->tx_q, skb) {
+		if (bt_cb(skb)->control.retries)
+			bt_cb(skb)->control.retries = 1;
+		else
+			break;
+	}
+
+	chan->expected_tx_seq = chan->buffer_seq;
+
+	clear_bit(CONN_REJ_ACT, &chan->conn_state);
+	clear_bit(CONN_SREJ_ACT, &chan->conn_state);
+	l2cap_seq_list_clear(&chan->retrans_list);
+	l2cap_seq_list_clear(&chan->srej_list);
+	skb_queue_purge(&chan->srej_q);
+
+	chan->tx_state = L2CAP_TX_STATE_XMIT;
+	chan->rx_state = L2CAP_RX_STATE_MOVE;
+
+	set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
+}
+
 static void l2cap_chan_ready(struct l2cap_chan *chan)
 {
 	/* This clears all conf flags, including CONF_NOT_COMPLETE */
@@ -4169,9 +4210,74 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
 	if (!enable_hs)
 		return -EINVAL;
 
-	/* Placeholder: Always refuse */
+	chan = l2cap_get_chan_by_dcid(conn, icid);
+
+	if (!chan || chan->scid < L2CAP_CID_DYN_START ||
+	    chan->chan_policy == BT_CHANNEL_POLICY_BREDR_ONLY ||
+	    (chan->mode != L2CAP_MODE_ERTM &&
+	     chan->mode != L2CAP_MODE_STREAMING)) {
+		result = L2CAP_MR_NOT_ALLOWED;
+		goto send_move_response;
+	}
+
+	if (chan->local_amp_id == req->dest_amp_id) {
+		result = L2CAP_MR_SAME_ID;
+		goto send_move_response;
+	}
+
+	if (req->dest_amp_id) {
+		struct hci_dev *hdev;
+		hdev = hci_dev_get(req->dest_amp_id);
+		if (!hdev || hdev->dev_type != HCI_AMP ||
+		    !test_bit(HCI_UP, &hdev->flags)) {
+			if (hdev)
+				hci_dev_put(hdev);
+
+			result = L2CAP_MR_BAD_ID;
+			goto send_move_response;
+		}
+		hci_dev_put(hdev);
+	}
+
+	/* Detect a move collision.  Only send a collision response
+	 * if this side has "lost", otherwise proceed with the move.
+	 * The winner has the larger bd_addr.
+	 */
+	if ((__chan_is_moving(chan) ||
+	     chan->move_role != L2CAP_MOVE_ROLE_NONE) &&
+	    bacmp(conn->src, conn->dst) > 0) {
+		result = L2CAP_MR_COLLISION;
+		goto send_move_response;
+	}
+
+	chan->ident = cmd->ident;
+	chan->move_role = L2CAP_MOVE_ROLE_RESPONDER;
+	l2cap_move_setup(chan);
+	chan->move_id = req->dest_amp_id;
+	icid = chan->dcid;
+
+	if (req->dest_amp_id == 0) {
+		/* Moving to BR/EDR */
+		if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
+			chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
+			result = L2CAP_MR_PEND;
+		} else {
+			chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
+			result = L2CAP_MR_SUCCESS;
+		}
+	} else {
+		chan->move_state = L2CAP_MOVE_WAIT_PREPARE;
+		/* Placeholder - uncomment when amp functions are available */
+		/*amp_accept_physical(chan, req->dest_amp_id);*/
+		result = L2CAP_MR_PEND;
+	}
+
+send_move_response:
 	l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
 
+	if (chan)
+		l2cap_chan_unlock(chan);
+
 	return 0;
 }
 
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 05/19] Bluetooth: Channel move request handling

[Andrei Emeltchenko]

Hi Mat,

On Tue, Oct 16, 2012 at 04:36:19PM -0700, Mat Martineau wrote:
> On receipt of a channel move request, the request must be validated
> based on the L2CAP mode, connection state, and controller
> capabilities.  ERTM channels must have their state machines cleared
> and transmission paused while the channel move takes place.
> 
> If the channel is being moved to an AMP controller then
> an AMP physical link must be prepared.  Moving the channel back to
> BR/EDR proceeds immediately.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  net/bluetooth/l2cap_core.c | 108 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index b5b849b..2fb0567 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -734,6 +734,12 @@ static void l2cap_send_cmd(struct l2cap_conn *conn, u8 ident, u8 code, u16 len,
>  	hci_send_acl(conn->hchan, skb, flags);
>  }
>  
> +static bool __chan_is_moving(struct l2cap_chan *chan)
> +{
> +	return chan->move_state != L2CAP_MOVE_STABLE &&
> +	       chan->move_state != L2CAP_MOVE_WAIT_PREPARE;
> +}

this one looks good now

> +
>  static void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
>  {
>  	struct hci_conn *hcon = chan->conn->hcon;
> @@ -995,6 +1001,41 @@ void l2cap_send_conn_req(struct l2cap_chan *chan)
>  	l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_REQ, sizeof(req), &req);
>  }
>  
> +static void l2cap_move_setup(struct l2cap_chan *chan)
> +{
> +	struct sk_buff *skb;
> +
> +	BT_DBG("chan %p", chan);
> +
> +	if (chan->mode != L2CAP_MODE_ERTM)
> +		return;
> +
> +	__clear_retrans_timer(chan);
> +	__clear_monitor_timer(chan);
> +	__clear_ack_timer(chan);
> +
> +	chan->retry_count = 0;
> +	skb_queue_walk(&chan->tx_q, skb) {
> +		if (bt_cb(skb)->control.retries)
> +			bt_cb(skb)->control.retries = 1;
> +		else
> +			break;
> +	}
> +
> +	chan->expected_tx_seq = chan->buffer_seq;
> +
> +	clear_bit(CONN_REJ_ACT, &chan->conn_state);
> +	clear_bit(CONN_SREJ_ACT, &chan->conn_state);
> +	l2cap_seq_list_clear(&chan->retrans_list);
> +	l2cap_seq_list_clear(&chan->srej_list);
> +	skb_queue_purge(&chan->srej_q);
> +
> +	chan->tx_state = L2CAP_TX_STATE_XMIT;
> +	chan->rx_state = L2CAP_RX_STATE_MOVE;
> +
> +	set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
> +}
> +
>  static void l2cap_chan_ready(struct l2cap_chan *chan)
>  {
>  	/* This clears all conf flags, including CONF_NOT_COMPLETE */
> @@ -4169,9 +4210,74 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>  	if (!enable_hs)
>  		return -EINVAL;
>  
> -	/* Placeholder: Always refuse */
> +	chan = l2cap_get_chan_by_dcid(conn, icid);
> +

extra line here

> +	if (!chan || chan->scid < L2CAP_CID_DYN_START ||
> +	    chan->chan_policy == BT_CHANNEL_POLICY_BREDR_ONLY ||
> +	    (chan->mode != L2CAP_MODE_ERTM &&
> +	     chan->mode != L2CAP_MODE_STREAMING)) {
> +		result = L2CAP_MR_NOT_ALLOWED;
> +		goto send_move_response;
> +	}
> +
> +	if (chan->local_amp_id == req->dest_amp_id) {
> +		result = L2CAP_MR_SAME_ID;
> +		goto send_move_response;
> +	}
> +
> +	if (req->dest_amp_id) {

here you compare "(val)"

> +		struct hci_dev *hdev;
> +		hdev = hci_dev_get(req->dest_amp_id);
> +		if (!hdev || hdev->dev_type != HCI_AMP ||
> +		    !test_bit(HCI_UP, &hdev->flags)) {
> +			if (hdev)
> +				hci_dev_put(hdev);
> +
> +			result = L2CAP_MR_BAD_ID;
> +			goto send_move_response;
> +		}
> +		hci_dev_put(hdev);
> +	}
> +
> +	/* Detect a move collision.  Only send a collision response
> +	 * if this side has "lost", otherwise proceed with the move.
> +	 * The winner has the larger bd_addr.
> +	 */
> +	if ((__chan_is_moving(chan) ||
> +	     chan->move_role != L2CAP_MOVE_ROLE_NONE) &&
> +	    bacmp(conn->src, conn->dst) > 0) {
> +		result = L2CAP_MR_COLLISION;
> +		goto send_move_response;
> +	}
> +
> +	chan->ident = cmd->ident;
> +	chan->move_role = L2CAP_MOVE_ROLE_RESPONDER;
> +	l2cap_move_setup(chan);
> +	chan->move_id = req->dest_amp_id;
> +	icid = chan->dcid;
> +
> +	if (req->dest_amp_id == 0) {

and here "(val == 0)"

I think it is better to use the single style "(val)"

> +		/* Moving to BR/EDR */
> +		if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
> +			chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
> +			result = L2CAP_MR_PEND;
> +		} else {
> +			chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
> +			result = L2CAP_MR_SUCCESS;
> +		}
> +	} else {
> +		chan->move_state = L2CAP_MOVE_WAIT_PREPARE;
> +		/* Placeholder - uncomment when amp functions are available */
> +		/*amp_accept_physical(chan, req->dest_amp_id);*/
> +		result = L2CAP_MR_PEND;
> +	}
> +
> +send_move_response:
>  	l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
>  
> +	if (chan)
> +		l2cap_chan_unlock(chan);
> +
>  	return 0;
>  }

Best regards 
Andrei Emeltchenko 

[PATCHv2 06/19] Bluetooth: Add new ERTM receive states for channel move

[Mat Martineau]

Two new states are required to implement channel moves with the ERTM
receive state machine.

The "WAIT_P" state is used by a move responder to wait for a "poll"
flag after a move is completed (success or failure).  "WAIT_F" is
similarly used by a move initiator to wait for a "final" flag when the
move is completing.  In either state, the reqseq value in the
poll/final frame tells the state machine exactly which frame should be
expected next.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 104 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 2fb0567..9663292 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4707,6 +4707,12 @@ static int l2cap_reassemble_sdu(struct l2cap_chan *chan, struct sk_buff *skb,
 	return err;
 }
 
+static int l2cap_resegment(struct l2cap_chan *chan)
+{
+	/* Placeholder */
+	return 0;
+}
+
 void l2cap_chan_busy(struct l2cap_chan *chan, int busy)
 {
 	u8 event;
@@ -5212,6 +5218,98 @@ static int l2cap_rx_state_srej_sent(struct l2cap_chan *chan,
 	return err;
 }
 
+static int l2cap_finish_move(struct l2cap_chan *chan)
+{
+	BT_DBG("chan %p", chan);
+
+	chan->move_role = L2CAP_MOVE_ROLE_NONE;
+	chan->rx_state = L2CAP_RX_STATE_RECV;
+
+	if (chan->hs_hcon)
+		chan->conn->mtu = chan->hs_hcon->hdev->block_mtu;
+	else
+		chan->conn->mtu = chan->conn->hcon->hdev->acl_mtu;
+
+	return l2cap_resegment(chan);
+}
+
+static int l2cap_rx_state_wait_p(struct l2cap_chan *chan,
+				 struct l2cap_ctrl *control,
+				 struct sk_buff *skb, u8 event)
+{
+	int err;
+
+	BT_DBG("chan %p, control %p, skb %p, event %d", chan, control, skb,
+	       event);
+
+	if (!control->poll)
+		return -EPROTO;
+
+	l2cap_process_reqseq(chan, control->reqseq);
+
+	if (!skb_queue_empty(&chan->tx_q))
+		chan->tx_send_head = skb_peek(&chan->tx_q);
+	else
+		chan->tx_send_head = NULL;
+
+	/* Rewind next_tx_seq to the point expected
+	 * by the receiver.
+	 */
+	chan->next_tx_seq = control->reqseq;
+	chan->unacked_frames = 0;
+
+	err = l2cap_finish_move(chan);
+	if (err)
+		return err;
+
+	set_bit(CONN_SEND_FBIT, &chan->conn_state);
+	l2cap_send_i_or_rr_or_rnr(chan);
+
+	if (event == L2CAP_EV_RECV_IFRAME)
+		return -EPROTO;
+
+	return l2cap_rx_state_recv(chan, control, NULL, event);
+}
+
+static int l2cap_rx_state_wait_f(struct l2cap_chan *chan,
+				 struct l2cap_ctrl *control,
+				 struct sk_buff *skb, u8 event)
+{
+	int err;
+
+	if (!control->final)
+		return -EPROTO;
+
+	clear_bit(CONN_REMOTE_BUSY, &chan->conn_state);
+	chan->move_role = L2CAP_MOVE_ROLE_NONE;
+
+	chan->rx_state = L2CAP_RX_STATE_RECV;
+	l2cap_process_reqseq(chan, control->reqseq);
+
+	if (!skb_queue_empty(&chan->tx_q))
+		chan->tx_send_head = skb_peek(&chan->tx_q);
+	else
+		chan->tx_send_head = NULL;
+
+	/* Rewind next_tx_seq to the point expected
+	 * by the receiver.
+	 */
+	chan->next_tx_seq = control->reqseq;
+	chan->unacked_frames = 0;
+
+	if (chan->hs_hcon)
+		chan->conn->mtu = chan->hs_hcon->hdev->block_mtu;
+	else
+		chan->conn->mtu = chan->conn->hcon->hdev->acl_mtu;
+
+	err = l2cap_resegment(chan);
+
+	if (!err)
+		err = l2cap_rx_state_recv(chan, control, skb, event);
+
+	return err;
+}
+
 static bool __valid_reqseq(struct l2cap_chan *chan, u16 reqseq)
 {
 	/* Make sure reqseq is for a packet that has been sent but not acked */
@@ -5238,6 +5336,12 @@ static int l2cap_rx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
 			err = l2cap_rx_state_srej_sent(chan, control, skb,
 						       event);
 			break;
+		case L2CAP_RX_STATE_WAIT_P:
+			err = l2cap_rx_state_wait_p(chan, control, skb, event);
+			break;
+		case L2CAP_RX_STATE_WAIT_F:
+			err = l2cap_rx_state_wait_f(chan, control, skb, event);
+			break;
 		default:
 			/* shut it down */
 			break;
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 06/19] Bluetooth: Add new ERTM receive states for channel move

[Marcel Holtmann]

Hi Mat,

> Two new states are required to implement channel moves with the ERTM
> receive state machine.
> 
> The "WAIT_P" state is used by a move responder to wait for a "poll"
> flag after a move is completed (success or failure).  "WAIT_F" is
> similarly used by a move initiator to wait for a "final" flag when the
> move is completing.  In either state, the reqseq value in the
> poll/final frame tells the state machine exactly which frame should be
> expected next.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 104 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 104 insertions(+)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

[PATCHv2 07/19] Bluetooth: Add move channel confirm handling

[Mat Martineau]

After sending a move channel response, a move responder waits for a
move channel confirm command.  If the received command has a
"confirmed" result the move is proceeding, and "unconfirmed" means the
move has failed and the channel will not change controllers.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 70 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 67 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 9663292..ed2c23f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1036,6 +1036,42 @@ static void l2cap_move_setup(struct l2cap_chan *chan)
 	set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
 }
 
+static void l2cap_move_success(struct l2cap_chan *chan)
+{
+	BT_DBG("chan %p", chan);
+
+	if (chan->mode != L2CAP_MODE_ERTM)
+		return;
+
+	switch (chan->move_role) {
+	case L2CAP_MOVE_ROLE_INITIATOR:
+		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
+		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
+		break;
+	case L2CAP_MOVE_ROLE_RESPONDER:
+		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
+		break;
+	}
+}
+
+static void l2cap_move_revert(struct l2cap_chan *chan)
+{
+	BT_DBG("chan %p", chan);
+
+	if (chan->mode != L2CAP_MODE_ERTM)
+		return;
+
+	switch (chan->move_role) {
+	case L2CAP_MOVE_ROLE_INITIATOR:
+		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
+		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
+		break;
+	case L2CAP_MOVE_ROLE_RESPONDER:
+		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
+		break;
+	}
+}
+
 static void l2cap_chan_ready(struct l2cap_chan *chan)
 {
 	/* This clears all conf flags, including CONF_NOT_COMPLETE */
@@ -4302,11 +4338,12 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
 	return 0;
 }
 
-static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
-					     struct l2cap_cmd_hdr *cmd,
-					     u16 cmd_len, void *data)
+static int l2cap_move_channel_confirm(struct l2cap_conn *conn,
+				      struct l2cap_cmd_hdr *cmd,
+				      u16 cmd_len, void *data)
 {
 	struct l2cap_move_chan_cfm *cfm = data;
+	struct l2cap_chan *chan;
 	u16 icid, result;
 
 	if (cmd_len != sizeof(*cfm))
@@ -4317,8 +4354,35 @@ static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
 
 	BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
 
+	chan = l2cap_get_chan_by_dcid(conn, icid);
+	if (!chan)
+		goto send_move_confirm_response;
+
+	if (chan->move_state == L2CAP_MOVE_WAIT_CONFIRM) {
+		chan->move_state = L2CAP_MOVE_STABLE;
+		if (result == L2CAP_MC_CONFIRMED) {
+			chan->local_amp_id = chan->move_id;
+			if (!chan->local_amp_id) {
+				/* Have moved off of AMP, free the channel */
+				chan->hs_hchan = NULL;
+				chan->hs_hcon = NULL;
+
+				/* Placeholder - free the logical link */
+			}
+			l2cap_move_success(chan);
+		} else {
+			chan->move_id = chan->local_amp_id;
+			l2cap_move_revert(chan);
+		}
+		chan->move_role = L2CAP_MOVE_ROLE_NONE;
+	}
+
+send_move_confirm_response:
 	l2cap_send_move_chan_cfm_rsp(conn, cmd->ident, icid);
 
+	if (chan)
+		l2cap_chan_unlock(chan);
+
 	return 0;
 }
 
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 07/19] Bluetooth: Add move channel confirm handling

[Marcel Holtmann]

Hi Mat,

> After sending a move channel response, a move responder waits for a
> move channel confirm command.  If the received command has a
> "confirmed" result the move is proceeding, and "unconfirmed" means the
> move has failed and the channel will not change controllers.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 70 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 67 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 9663292..ed2c23f 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1036,6 +1036,42 @@ static void l2cap_move_setup(struct l2cap_chan *chan)
>  	set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
>  }
>  
> +static void l2cap_move_success(struct l2cap_chan *chan)
> +{
> +	BT_DBG("chan %p", chan);
> +
> +	if (chan->mode != L2CAP_MODE_ERTM)
> +		return;
> +
> +	switch (chan->move_role) {
> +	case L2CAP_MOVE_ROLE_INITIATOR:
> +		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
> +		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
> +		break;
> +	case L2CAP_MOVE_ROLE_RESPONDER:
> +		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
> +		break;
> +	}
> +}
> +
> +static void l2cap_move_revert(struct l2cap_chan *chan)
> +{
> +	BT_DBG("chan %p", chan);
> +
> +	if (chan->mode != L2CAP_MODE_ERTM)
> +		return;
> +
> +	switch (chan->move_role) {
> +	case L2CAP_MOVE_ROLE_INITIATOR:
> +		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
> +		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
> +		break;
> +	case L2CAP_MOVE_ROLE_RESPONDER:
> +		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
> +		break;
> +	}
> +}
> +
>  static void l2cap_chan_ready(struct l2cap_chan *chan)
>  {
>  	/* This clears all conf flags, including CONF_NOT_COMPLETE */
> @@ -4302,11 +4338,12 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
>  	return 0;
>  }
>  
> -static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
> -					     struct l2cap_cmd_hdr *cmd,
> -					     u16 cmd_len, void *data)
> +static int l2cap_move_channel_confirm(struct l2cap_conn *conn,
> +				      struct l2cap_cmd_hdr *cmd,
> +				      u16 cmd_len, void *data)
>  {
>  	struct l2cap_move_chan_cfm *cfm = data;
> +	struct l2cap_chan *chan;
>  	u16 icid, result;
>  
>  	if (cmd_len != sizeof(*cfm))
> @@ -4317,8 +4354,35 @@ static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
>  
>  	BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
>  
> +	chan = l2cap_get_chan_by_dcid(conn, icid);
> +	if (!chan)
> +		goto send_move_confirm_response;
> +
> +	if (chan->move_state == L2CAP_MOVE_WAIT_CONFIRM) {
> +		chan->move_state = L2CAP_MOVE_STABLE;
> +		if (result == L2CAP_MC_CONFIRMED) {
> +			chan->local_amp_id = chan->move_id;
> +			if (!chan->local_amp_id) {
> +				/* Have moved off of AMP, free the channel */
> +				chan->hs_hchan = NULL;
> +				chan->hs_hcon = NULL;
> +
> +				/* Placeholder - free the logical link */
> +			}
> +			l2cap_move_success(chan);
> +		} else {
> +			chan->move_id = chan->local_amp_id;
> +			l2cap_move_revert(chan);
> +		}
> +		chan->move_role = L2CAP_MOVE_ROLE_NONE;
> +	}
> +
> +send_move_confirm_response:
>  	l2cap_send_move_chan_cfm_rsp(conn, cmd->ident, icid);
>  
> +	if (chan)
> +		l2cap_chan_unlock(chan);
> +

still not a big fan of the if (chan) check before the unlock. This way
of dealing with locks makes my brain hurt ;)

Regards

Marcel

Re: [PATCHv2 07/19] Bluetooth: Add move channel confirm handling

[Andrei Emeltchenko]

Hi Marcel,

On Wed, Oct 17, 2012 at 10:16:27AM -0700, Marcel Holtmann wrote:
> Hi Mat,
> 
> > After sending a move channel response, a move responder waits for a
> > move channel confirm command.  If the received command has a
> > "confirmed" result the move is proceeding, and "unconfirmed" means the
> > move has failed and the channel will not change controllers.
> > 
> > Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> > ---
> >  net/bluetooth/l2cap_core.c | 70 ++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 67 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 9663292..ed2c23f 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -1036,6 +1036,42 @@ static void l2cap_move_setup(struct l2cap_chan *chan)
> >  	set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
> >  }
> >  
> > +static void l2cap_move_success(struct l2cap_chan *chan)
> > +{
> > +	BT_DBG("chan %p", chan);
> > +
> > +	if (chan->mode != L2CAP_MODE_ERTM)
> > +		return;
> > +
> > +	switch (chan->move_role) {
> > +	case L2CAP_MOVE_ROLE_INITIATOR:
> > +		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
> > +		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
> > +		break;
> > +	case L2CAP_MOVE_ROLE_RESPONDER:
> > +		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
> > +		break;
> > +	}
> > +}
> > +
> > +static void l2cap_move_revert(struct l2cap_chan *chan)
> > +{
> > +	BT_DBG("chan %p", chan);
> > +
> > +	if (chan->mode != L2CAP_MODE_ERTM)
> > +		return;
> > +
> > +	switch (chan->move_role) {
> > +	case L2CAP_MOVE_ROLE_INITIATOR:
> > +		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
> > +		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
> > +		break;
> > +	case L2CAP_MOVE_ROLE_RESPONDER:
> > +		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
> > +		break;
> > +	}
> > +}
> > +
> >  static void l2cap_chan_ready(struct l2cap_chan *chan)
> >  {
> >  	/* This clears all conf flags, including CONF_NOT_COMPLETE */
> > @@ -4302,11 +4338,12 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
> >  	return 0;
> >  }
> >  
> > -static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
> > -					     struct l2cap_cmd_hdr *cmd,
> > -					     u16 cmd_len, void *data)
> > +static int l2cap_move_channel_confirm(struct l2cap_conn *conn,
> > +				      struct l2cap_cmd_hdr *cmd,
> > +				      u16 cmd_len, void *data)
> >  {
> >  	struct l2cap_move_chan_cfm *cfm = data;
> > +	struct l2cap_chan *chan;
> >  	u16 icid, result;
> >  
> >  	if (cmd_len != sizeof(*cfm))
> > @@ -4317,8 +4354,35 @@ static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
> >  
> >  	BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
> >  
> > +	chan = l2cap_get_chan_by_dcid(conn, icid);
> > +	if (!chan)
> > +		goto send_move_confirm_response;
> > +
> > +	if (chan->move_state == L2CAP_MOVE_WAIT_CONFIRM) {
> > +		chan->move_state = L2CAP_MOVE_STABLE;
> > +		if (result == L2CAP_MC_CONFIRMED) {
> > +			chan->local_amp_id = chan->move_id;
> > +			if (!chan->local_amp_id) {
> > +				/* Have moved off of AMP, free the channel */
> > +				chan->hs_hchan = NULL;
> > +				chan->hs_hcon = NULL;
> > +
> > +				/* Placeholder - free the logical link */
> > +			}
> > +			l2cap_move_success(chan);
> > +		} else {
> > +			chan->move_id = chan->local_amp_id;
> > +			l2cap_move_revert(chan);
> > +		}
> > +		chan->move_role = L2CAP_MOVE_ROLE_NONE;
> > +	}
> > +
> > +send_move_confirm_response:
> >  	l2cap_send_move_chan_cfm_rsp(conn, cmd->ident, icid);
> >  
> > +	if (chan)
> > +		l2cap_chan_unlock(chan);
> > +
> 
> still not a big fan of the if (chan) check before the unlock. This way
> of dealing with locks makes my brain hurt ;)

I think in this case the solution would be to invoke
l2cap_send_move_chan_cfm_rsp 2 times: 1st time atfer checking (!chan) and
second time here. Since this message does not need any error code this
looks OK to me.

Best regards 
Andrei Emeltchenko 

[PATCHv2 08/19] Bluetooth: Add state to hci_chan

[Mat Martineau]

On an AMP controller, hci_chan maps to a logical link.  When a channel
is being moved, the logical link may or may not be connected already.
The hci_chan->state is used to determine the existance of a useable
logical link so the link can be either used or requested.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/hci_core.h | 1 +
 net/bluetooth/hci_conn.c         | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 9fe8e2d..00abc52 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -355,6 +355,7 @@ struct hci_chan {
 	struct hci_conn *conn;
 	struct sk_buff_head data_q;
 	unsigned int	sent;
+	__u8		state;
 };
 
 extern struct list_head hci_dev_list;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index fe64621..6dcf452 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -959,6 +959,7 @@ struct hci_chan *hci_chan_create(struct hci_conn *conn)
 
 	chan->conn = conn;
 	skb_queue_head_init(&chan->data_q);
+	chan->state = BT_CONNECTED;
 
 	list_add_rcu(&chan->list, &conn->chan_list);
 
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 09/19] Bluetooth: Move channel response

[Mat Martineau]

The move response command includes a result code indicationg
"pending", "success", or "failure" status.  A pending result is
received when the remote address is still setting up a physical link,
and will be followed by success or failure.  On success, logical link
setup will proceed.  On failure, the move is stopped.  The receiver of
a move channel response must always follow up by sending a move
channel confirm command.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 include/net/bluetooth/l2cap.h |   2 +
 net/bluetooth/l2cap_core.c    | 161 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 158 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 6d3615e..b4c3c65 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -52,6 +52,8 @@
 #define L2CAP_ENC_TIMEOUT		msecs_to_jiffies(5000)
 #define L2CAP_CONN_TIMEOUT		msecs_to_jiffies(40000)
 #define L2CAP_INFO_TIMEOUT		msecs_to_jiffies(4000)
+#define L2CAP_MOVE_TIMEOUT		msecs_to_jiffies(4000)
+#define L2CAP_MOVE_ERTX_TIMEOUT		msecs_to_jiffies(60000)
 
 #define L2CAP_A2MP_DEFAULT_MTU		670
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ed2c23f..5e4796c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -128,6 +128,20 @@ static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn,
 	return NULL;
 }
 
+static struct l2cap_chan *l2cap_get_chan_by_ident(struct l2cap_conn *conn,
+						  u8 ident)
+{
+	struct l2cap_chan *c;
+
+	mutex_lock(&conn->chan_lock);
+	c = __l2cap_get_chan_by_ident(conn, ident);
+	if (c)
+		l2cap_chan_lock(c);
+	mutex_unlock(&conn->chan_lock);
+
+	return c;
+}
+
 static struct l2cap_chan *__l2cap_global_chan_by_addr(__le16 psm, bdaddr_t *src)
 {
 	struct l2cap_chan *c;
@@ -4227,6 +4241,13 @@ static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
 	l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
 }
 
+static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
+			      u8 status)
+{
+	/* Placeholder */
+	return;
+}
+
 static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
 					 struct l2cap_cmd_hdr *cmd,
 					 u16 cmd_len, void *data)
@@ -4317,9 +4338,137 @@ send_move_response:
 	return 0;
 }
 
-static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
-					 struct l2cap_cmd_hdr *cmd,
-					 u16 cmd_len, void *data)
+static void l2cap_move_continue(struct l2cap_conn *conn, u16 icid, u16 result)
+{
+	struct l2cap_chan *chan;
+
+	chan = l2cap_get_chan_by_scid(conn, icid);
+	if (!chan) {
+		l2cap_send_move_chan_cfm(conn, NULL, icid,
+					 L2CAP_MC_UNCONFIRMED);
+		return;
+	}
+
+	__clear_chan_timer(chan);
+	if (result == L2CAP_MR_PEND)
+		__set_chan_timer(chan, L2CAP_MOVE_ERTX_TIMEOUT);
+
+	if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_COMP) {
+		/* Move confirm will be sent when logical link
+		 * is complete.
+		 */
+		chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_CFM;
+	} else if (chan->move_state == L2CAP_MOVE_WAIT_RSP_SUCCESS) {
+		if (result == L2CAP_MR_PEND) {
+			goto done;
+		} else if (test_bit(CONN_LOCAL_BUSY,
+				    &chan->conn_state)) {
+			chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
+		} else {
+			/* Logical link is up or moving to BR/EDR,
+			 * proceed with move */
+			chan->move_state = L2CAP_MOVE_WAIT_CONFIRM_RSP;
+			l2cap_send_move_chan_cfm(conn, chan, chan->scid,
+						 L2CAP_MC_CONFIRMED);
+			__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+		}
+	} else if (chan->move_state == L2CAP_MOVE_WAIT_RSP) {
+		struct hci_chan *hchan = NULL;
+		/* Moving to AMP */
+		if (result == L2CAP_MR_SUCCESS) {
+			/* Remote is ready, send confirm immediately
+			 * after logical link is ready
+			 */
+			chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_CFM;
+		} else {
+			/* Both logical link and move success
+			 * are required to confirm
+			 */
+			chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_COMP;
+		}
+
+		/* Placeholder - get hci_chan for logical link */
+		if (!hchan) {
+			/* Logical link not available */
+			l2cap_send_move_chan_cfm(conn, chan, chan->scid,
+						 L2CAP_MC_UNCONFIRMED);
+			goto done;
+		}
+
+		/* If the logical link is not yet connected, do not
+		 * send confirmation.
+		 */
+		if (hchan->state != BT_CONNECTED)
+			goto done;
+
+		/* Logical link is already ready to go */
+
+		chan->hs_hcon = hchan->conn;
+		chan->hs_hcon->l2cap_data = chan->conn;
+
+		if (result == L2CAP_MR_SUCCESS) {
+			/* Can confirm now */
+			l2cap_send_move_chan_cfm(conn, chan, chan->scid,
+						 L2CAP_MC_CONFIRMED);
+		} else {
+			/* Now only need move success
+			 * to confirm
+			 */
+			chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
+		}
+
+		l2cap_logical_cfm(chan, hchan, L2CAP_MR_SUCCESS);
+	} else {
+		/* Any other amp move state means the move failed. */
+		chan->move_id = chan->local_amp_id;
+		chan->move_state = L2CAP_MOVE_STABLE;
+		l2cap_move_revert(chan);
+		chan->move_role = L2CAP_MOVE_ROLE_NONE;
+		l2cap_send_move_chan_cfm(conn, chan, chan->scid,
+					 L2CAP_MC_UNCONFIRMED);
+		__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+	}
+
+done:
+	l2cap_chan_unlock(chan);
+}
+
+static void l2cap_move_fail(struct l2cap_conn *conn, u8 ident, u16 icid,
+			    u16 result)
+{
+	struct l2cap_chan *chan;
+
+	chan = l2cap_get_chan_by_ident(conn, ident);
+	if (!chan) {
+		/* Could not locate channel, icid is best guess */
+		l2cap_send_move_chan_cfm(conn, NULL, icid,
+					 L2CAP_MC_UNCONFIRMED);
+		return;
+	}
+
+	__clear_chan_timer(chan);
+
+	if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
+		if (result == L2CAP_MR_COLLISION) {
+			chan->move_role = L2CAP_MOVE_ROLE_RESPONDER;
+		} else {
+			/* Cleanup - cancel move */
+			chan->move_id = chan->local_amp_id;
+			chan->move_state = L2CAP_MOVE_STABLE;
+			l2cap_move_revert(chan);
+			chan->move_role = L2CAP_MOVE_ROLE_NONE;
+		}
+	}
+
+	l2cap_send_move_chan_cfm(conn, chan, chan->scid, L2CAP_MC_UNCONFIRMED);
+	__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+
+	l2cap_chan_unlock(chan);
+}
+
+static int l2cap_move_channel_rsp(struct l2cap_conn *conn,
+				  struct l2cap_cmd_hdr *cmd,
+				  u16 cmd_len, void *data)
 {
 	struct l2cap_move_chan_rsp *rsp = data;
 	u16 icid, result;
@@ -4332,8 +4481,10 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
 
 	BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
 
-	/* Placeholder: Always unconfirmed */
-	l2cap_send_move_chan_cfm(conn, NULL, icid, L2CAP_MC_UNCONFIRMED);
+	if (result == L2CAP_MR_SUCCESS || result == L2CAP_MR_PEND)
+		l2cap_move_continue(conn, icid, result);
+	else
+		l2cap_move_fail(conn, cmd->ident, icid, result);
 
 	return 0;
 }
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 09/19] Bluetooth: Move channel response

[Marcel Holtmann]

Hi Mat,

> The move response command includes a result code indicationg
> "pending", "success", or "failure" status.  A pending result is
> received when the remote address is still setting up a physical link,
> and will be followed by success or failure.  On success, logical link
> setup will proceed.  On failure, the move is stopped.  The receiver of
> a move channel response must always follow up by sending a move
> channel confirm command.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  include/net/bluetooth/l2cap.h |   2 +
>  net/bluetooth/l2cap_core.c    | 161 ++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 158 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 6d3615e..b4c3c65 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -52,6 +52,8 @@
>  #define L2CAP_ENC_TIMEOUT		msecs_to_jiffies(5000)
>  #define L2CAP_CONN_TIMEOUT		msecs_to_jiffies(40000)
>  #define L2CAP_INFO_TIMEOUT		msecs_to_jiffies(4000)
> +#define L2CAP_MOVE_TIMEOUT		msecs_to_jiffies(4000)
> +#define L2CAP_MOVE_ERTX_TIMEOUT		msecs_to_jiffies(60000)
>  
>  #define L2CAP_A2MP_DEFAULT_MTU		670
>  
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index ed2c23f..5e4796c 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -128,6 +128,20 @@ static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn,
>  	return NULL;
>  }
>  
> +static struct l2cap_chan *l2cap_get_chan_by_ident(struct l2cap_conn *conn,
> +						  u8 ident)
> +{
> +	struct l2cap_chan *c;
> +
> +	mutex_lock(&conn->chan_lock);
> +	c = __l2cap_get_chan_by_ident(conn, ident);
> +	if (c)
> +		l2cap_chan_lock(c);
> +	mutex_unlock(&conn->chan_lock);
> +
> +	return c;
> +}
> +
>  static struct l2cap_chan *__l2cap_global_chan_by_addr(__le16 psm, bdaddr_t *src)
>  {
>  	struct l2cap_chan *c;
> @@ -4227,6 +4241,13 @@ static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
>  	l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
>  }
>  
> +static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
> +			      u8 status)
> +{
> +	/* Placeholder */
> +	return;
> +}
> +
>  static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>  					 struct l2cap_cmd_hdr *cmd,
>  					 u16 cmd_len, void *data)
> @@ -4317,9 +4338,137 @@ send_move_response:
>  	return 0;
>  }
>  
> -static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
> -					 struct l2cap_cmd_hdr *cmd,
> -					 u16 cmd_len, void *data)
> +static void l2cap_move_continue(struct l2cap_conn *conn, u16 icid, u16 result)
> +{
> +	struct l2cap_chan *chan;
> +
> +	chan = l2cap_get_chan_by_scid(conn, icid);
> +	if (!chan) {
> +		l2cap_send_move_chan_cfm(conn, NULL, icid,
> +					 L2CAP_MC_UNCONFIRMED);
> +		return;
> +	}
> +
> +	__clear_chan_timer(chan);
> +	if (result == L2CAP_MR_PEND)
> +		__set_chan_timer(chan, L2CAP_MOVE_ERTX_TIMEOUT);
> +
> +	if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_COMP) {
> +		/* Move confirm will be sent when logical link
> +		 * is complete.
> +		 */
> +		chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_CFM;
> +	} else if (chan->move_state == L2CAP_MOVE_WAIT_RSP_SUCCESS) {
> +		if (result == L2CAP_MR_PEND) {
> +			goto done;
> +		} else if (test_bit(CONN_LOCAL_BUSY,
> +				    &chan->conn_state)) {
> +			chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
> +		} else {
> +			/* Logical link is up or moving to BR/EDR,
> +			 * proceed with move */
> +			chan->move_state = L2CAP_MOVE_WAIT_CONFIRM_RSP;
> +			l2cap_send_move_chan_cfm(conn, chan, chan->scid,
> +						 L2CAP_MC_CONFIRMED);
> +			__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
> +		}
> +	} else if (chan->move_state == L2CAP_MOVE_WAIT_RSP) {
> +		struct hci_chan *hchan = NULL;
> +		/* Moving to AMP */
> +		if (result == L2CAP_MR_SUCCESS) {
> +			/* Remote is ready, send confirm immediately
> +			 * after logical link is ready
> +			 */
> +			chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_CFM;
> +		} else {
> +			/* Both logical link and move success
> +			 * are required to confirm
> +			 */
> +			chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_COMP;
> +		}
> +
> +		/* Placeholder - get hci_chan for logical link */
> +		if (!hchan) {
> +			/* Logical link not available */
> +			l2cap_send_move_chan_cfm(conn, chan, chan->scid,
> +						 L2CAP_MC_UNCONFIRMED);
> +			goto done;
> +		}
> +
> +		/* If the logical link is not yet connected, do not
> +		 * send confirmation.
> +		 */
> +		if (hchan->state != BT_CONNECTED)
> +			goto done;
> +
> +		/* Logical link is already ready to go */
> +
> +		chan->hs_hcon = hchan->conn;
> +		chan->hs_hcon->l2cap_data = chan->conn;
> +
> +		if (result == L2CAP_MR_SUCCESS) {
> +			/* Can confirm now */
> +			l2cap_send_move_chan_cfm(conn, chan, chan->scid,
> +						 L2CAP_MC_CONFIRMED);
> +		} else {
> +			/* Now only need move success
> +			 * to confirm
> +			 */
> +			chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
> +		}
> +
> +		l2cap_logical_cfm(chan, hchan, L2CAP_MR_SUCCESS);
> +	} else {
> +		/* Any other amp move state means the move failed. */
> +		chan->move_id = chan->local_amp_id;
> +		chan->move_state = L2CAP_MOVE_STABLE;
> +		l2cap_move_revert(chan);
> +		chan->move_role = L2CAP_MOVE_ROLE_NONE;
> +		l2cap_send_move_chan_cfm(conn, chan, chan->scid,
> +					 L2CAP_MC_UNCONFIRMED);
> +		__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
> +	}

can you just use a switch statement here.

> +
> +done:
> +	l2cap_chan_unlock(chan);
> +}
> +
> +static void l2cap_move_fail(struct l2cap_conn *conn, u8 ident, u16 icid,
> +			    u16 result)
> +{
> +	struct l2cap_chan *chan;
> +
> +	chan = l2cap_get_chan_by_ident(conn, ident);
> +	if (!chan) {
> +		/* Could not locate channel, icid is best guess */
> +		l2cap_send_move_chan_cfm(conn, NULL, icid,
> +					 L2CAP_MC_UNCONFIRMED);
> +		return;
> +	}
> +
> +	__clear_chan_timer(chan);
> +
> +	if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
> +		if (result == L2CAP_MR_COLLISION) {
> +			chan->move_role = L2CAP_MOVE_ROLE_RESPONDER;
> +		} else {
> +			/* Cleanup - cancel move */
> +			chan->move_id = chan->local_amp_id;
> +			chan->move_state = L2CAP_MOVE_STABLE;
> +			l2cap_move_revert(chan);
> +			chan->move_role = L2CAP_MOVE_ROLE_NONE;
> +		}
> +	}
> +
> +	l2cap_send_move_chan_cfm(conn, chan, chan->scid, L2CAP_MC_UNCONFIRMED);
> +	__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
> +
> +	l2cap_chan_unlock(chan);
> +}
> +
> +static int l2cap_move_channel_rsp(struct l2cap_conn *conn,
> +				  struct l2cap_cmd_hdr *cmd,
> +				  u16 cmd_len, void *data)
>  {
>  	struct l2cap_move_chan_rsp *rsp = data;
>  	u16 icid, result;
> @@ -4332,8 +4481,10 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
>  
>  	BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
>  
> -	/* Placeholder: Always unconfirmed */
> -	l2cap_send_move_chan_cfm(conn, NULL, icid, L2CAP_MC_UNCONFIRMED);
> +	if (result == L2CAP_MR_SUCCESS || result == L2CAP_MR_PEND)
> +		l2cap_move_continue(conn, icid, result);
> +	else
> +		l2cap_move_fail(conn, cmd->ident, icid, result);
>  
>  	return 0;
>  }

Regards

Marcel

[PATCHv2 10/19] Bluetooth: Add logical link confirm

[Mat Martineau]

The logical link confirm callback is executed when the AMP controller
completes its logical link setup.  During a channel move, a newly
formed logical link allows a move responder to send a move channel
response.  A move initiator will send a move channel confirm.  A
failed logical link will end the channel move and send an appropriate
response or confirm command indicating a failure.

If the channel is being created on an AMP controller, L2CAP
configuration is started after the logical link is set up.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 114 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 112 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 5e4796c..ddd8c72 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3799,6 +3799,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
 		goto unlock;
 	}
 
+	chan->ident = cmd->ident;
 	l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP, len, rsp);
 	chan->num_conf_rsp++;
 
@@ -4241,11 +4242,120 @@ static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
 	l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
 }
 
+/* Call with chan locked */
 static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
 			      u8 status)
 {
-	/* Placeholder */
-	return;
+	BT_DBG("chan %p, hchan %p, status %d", chan, hchan, status);
+
+	if (status) {
+		/* Logical link setup failed */
+		if (chan->state != BT_CONNECTED) {
+			/* Create channel failure, disconnect */
+			l2cap_send_disconn_req(chan->conn, chan, ECONNRESET);
+		} else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
+			l2cap_move_revert(chan);
+			chan->move_role = L2CAP_MOVE_ROLE_NONE;
+			chan->move_state = L2CAP_MOVE_STABLE;
+			l2cap_send_move_chan_rsp(chan->conn, chan->ident,
+						 chan->dcid, L2CAP_MR_NOT_SUPP);
+		} else if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
+			if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_COMP ||
+			    chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_CFM) {
+				/* Remote has only sent pending or
+				 * success responses, clean up
+				 */
+				l2cap_move_revert(chan);
+				chan->move_role = L2CAP_MOVE_ROLE_NONE;
+				chan->move_state = L2CAP_MOVE_STABLE;
+			}
+
+			/* Other amp move states imply that the move
+			 * has already aborted
+			 */
+			l2cap_send_move_chan_cfm(chan->conn, chan, chan->scid,
+						 L2CAP_MC_UNCONFIRMED);
+			__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+		}
+
+		chan->hs_hchan = NULL;
+		chan->hs_hcon = NULL;
+
+		/* Placeholder - free logical link */
+
+	} else if (chan->state != BT_CONNECTED) {
+		struct l2cap_conf_rsp rsp;
+		u8 code;
+
+		/* Create channel complete */
+
+		/* Ignore logical link if channel is on BR/EDR */
+		if (!chan->local_amp_id)
+			return;
+
+		chan->hs_hcon = hchan->conn;
+		chan->hs_hcon->l2cap_data = chan->conn;
+
+		code = l2cap_build_conf_rsp(chan, &rsp,
+					    L2CAP_CONF_SUCCESS, 0);
+		l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CONF_RSP, code,
+			       &rsp);
+		set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
+
+		if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
+			int err = 0;
+
+			set_default_fcs(chan);
+
+			err = l2cap_ertm_init(chan);
+			if (err < 0)
+				l2cap_send_disconn_req(chan->conn, chan, -err);
+			else
+				l2cap_chan_ready(chan);
+		}
+	} else {
+		/* Channel move */
+		chan->hs_hcon = hchan->conn;
+		chan->hs_hcon->l2cap_data = chan->conn;
+
+		BT_DBG("move_state %d", chan->move_state);
+
+		switch (chan->move_state) {
+		case L2CAP_MOVE_WAIT_LOGICAL_COMP:
+			/* Move confirm will be sent after a success
+			 * response is received
+			 */
+			chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
+			break;
+		case L2CAP_MOVE_WAIT_LOGICAL_CFM:
+			if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
+				chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
+			} else if (chan->move_role ==
+				   L2CAP_MOVE_ROLE_INITIATOR) {
+				chan->move_state = L2CAP_MOVE_WAIT_CONFIRM_RSP;
+				l2cap_send_move_chan_cfm(chan->conn, chan,
+							 chan->scid,
+							 L2CAP_MR_SUCCESS);
+				__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+			} else if (chan->move_role ==
+				   L2CAP_MOVE_ROLE_RESPONDER) {
+				chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
+				l2cap_send_move_chan_rsp(chan->conn,
+							 chan->ident,
+							 chan->dcid,
+							 L2CAP_MR_SUCCESS);
+			}
+			break;
+		default:
+			/* Move was not in expected state, free the channel */
+			chan->hs_hchan = NULL;
+			chan->hs_hcon = NULL;
+
+			/* Placeholder - free the logical link */
+
+			chan->move_state = L2CAP_MOVE_STABLE;
+		}
+	}
 }
 
 static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 10/19] Bluetooth: Add logical link confirm

[Marcel Holtmann]

Hi Mat,

> The logical link confirm callback is executed when the AMP controller
> completes its logical link setup.  During a channel move, a newly
> formed logical link allows a move responder to send a move channel
> response.  A move initiator will send a move channel confirm.  A
> failed logical link will end the channel move and send an appropriate
> response or confirm command indicating a failure.
> 
> If the channel is being created on an AMP controller, L2CAP
> configuration is started after the logical link is set up.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 114 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 112 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 5e4796c..ddd8c72 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3799,6 +3799,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
>  		goto unlock;
>  	}
>  
> +	chan->ident = cmd->ident;
>  	l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP, len, rsp);
>  	chan->num_conf_rsp++;
>  
> @@ -4241,11 +4242,120 @@ static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
>  	l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
>  }
>  
> +/* Call with chan locked */
>  static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
>  			      u8 status)
>  {
> -	/* Placeholder */
> -	return;
> +	BT_DBG("chan %p, hchan %p, status %d", chan, hchan, status);
> +
> +	if (status) {
> +		/* Logical link setup failed */
> +		if (chan->state != BT_CONNECTED) {
> +			/* Create channel failure, disconnect */
> +			l2cap_send_disconn_req(chan->conn, chan, ECONNRESET);
> +		} else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
> +			l2cap_move_revert(chan);
> +			chan->move_role = L2CAP_MOVE_ROLE_NONE;
> +			chan->move_state = L2CAP_MOVE_STABLE;
> +			l2cap_send_move_chan_rsp(chan->conn, chan->ident,
> +						 chan->dcid, L2CAP_MR_NOT_SUPP);
> +		} else if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
> +			if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_COMP ||
> +			    chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_CFM) {
> +				/* Remote has only sent pending or
> +				 * success responses, clean up
> +				 */
> +				l2cap_move_revert(chan);
> +				chan->move_role = L2CAP_MOVE_ROLE_NONE;
> +				chan->move_state = L2CAP_MOVE_STABLE;
> +			}
> +
> +			/* Other amp move states imply that the move
> +			 * has already aborted
> +			 */
> +			l2cap_send_move_chan_cfm(chan->conn, chan, chan->scid,
> +						 L2CAP_MC_UNCONFIRMED);
> +			__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
> +		}
> +
> +		chan->hs_hchan = NULL;
> +		chan->hs_hcon = NULL;
> +
> +		/* Placeholder - free logical link */
> +
> +	} else if (chan->state != BT_CONNECTED) {
> +		struct l2cap_conf_rsp rsp;
> +		u8 code;
> +
> +		/* Create channel complete */
> +
> +		/* Ignore logical link if channel is on BR/EDR */
> +		if (!chan->local_amp_id)
> +			return;
> +
> +		chan->hs_hcon = hchan->conn;
> +		chan->hs_hcon->l2cap_data = chan->conn;
> +
> +		code = l2cap_build_conf_rsp(chan, &rsp,
> +					    L2CAP_CONF_SUCCESS, 0);
> +		l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CONF_RSP, code,
> +			       &rsp);
> +		set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
> +
> +		if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
> +			int err = 0;
> +
> +			set_default_fcs(chan);
> +
> +			err = l2cap_ertm_init(chan);
> +			if (err < 0)
> +				l2cap_send_disconn_req(chan->conn, chan, -err);
> +			else
> +				l2cap_chan_ready(chan);
> +		}
> +	} else {
> +		/* Channel move */
> +		chan->hs_hcon = hchan->conn;
> +		chan->hs_hcon->l2cap_data = chan->conn;
> +
> +		BT_DBG("move_state %d", chan->move_state);
> +
> +		switch (chan->move_state) {
> +		case L2CAP_MOVE_WAIT_LOGICAL_COMP:
> +			/* Move confirm will be sent after a success
> +			 * response is received
> +			 */
> +			chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
> +			break;
> +		case L2CAP_MOVE_WAIT_LOGICAL_CFM:
> +			if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
> +				chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
> +			} else if (chan->move_role ==
> +				   L2CAP_MOVE_ROLE_INITIATOR) {
> +				chan->move_state = L2CAP_MOVE_WAIT_CONFIRM_RSP;
> +				l2cap_send_move_chan_cfm(chan->conn, chan,
> +							 chan->scid,
> +							 L2CAP_MR_SUCCESS);
> +				__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
> +			} else if (chan->move_role ==
> +				   L2CAP_MOVE_ROLE_RESPONDER) {
> +				chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
> +				l2cap_send_move_chan_rsp(chan->conn,
> +							 chan->ident,
> +							 chan->dcid,
> +							 L2CAP_MR_SUCCESS);
> +			}
> +			break;
> +		default:
> +			/* Move was not in expected state, free the channel */
> +			chan->hs_hchan = NULL;
> +			chan->hs_hcon = NULL;
> +
> +			/* Placeholder - free the logical link */
> +
> +			chan->move_state = L2CAP_MOVE_STABLE;
> +		}
> +	}
>  }
>  

I find this this function still a little bit too complex. Any chance we
can split into more logical pieces. It is fine if not, but we should at
least give it another try.

Regards

Marcel

Re: [PATCHv2 10/19] Bluetooth: Add logical link confirm

[Andrei Emeltchenko]

Hi,

On Wed, Oct 17, 2012 at 10:22:09AM -0700, Marcel Holtmann wrote:
> Hi Mat,
> 
> > The logical link confirm callback is executed when the AMP controller
> > completes its logical link setup.  During a channel move, a newly
> > formed logical link allows a move responder to send a move channel
> > response.  A move initiator will send a move channel confirm.  A
> > failed logical link will end the channel move and send an appropriate
> > response or confirm command indicating a failure.
> > 
> > If the channel is being created on an AMP controller, L2CAP
> > configuration is started after the logical link is set up.
> > 
> > Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> > ---
> >  net/bluetooth/l2cap_core.c | 114 ++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 112 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 5e4796c..ddd8c72 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -3799,6 +3799,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
> >  		goto unlock;
> >  	}
> >  
> > +	chan->ident = cmd->ident;
> >  	l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP, len, rsp);
> >  	chan->num_conf_rsp++;
> >  
> > @@ -4241,11 +4242,120 @@ static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
> >  	l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
> >  }
> >  
> > +/* Call with chan locked */
> >  static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
> >  			      u8 status)
> >  {
> > -	/* Placeholder */
> > -	return;
> > +	BT_DBG("chan %p, hchan %p, status %d", chan, hchan, status);
> > +
> > +	if (status) {
> > +		/* Logical link setup failed */
> > +		if (chan->state != BT_CONNECTED) {
> > +			/* Create channel failure, disconnect */
> > +			l2cap_send_disconn_req(chan->conn, chan, ECONNRESET);
> > +		} else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
> > +			l2cap_move_revert(chan);
> > +			chan->move_role = L2CAP_MOVE_ROLE_NONE;
> > +			chan->move_state = L2CAP_MOVE_STABLE;
> > +			l2cap_send_move_chan_rsp(chan->conn, chan->ident,
> > +						 chan->dcid, L2CAP_MR_NOT_SUPP);
> > +		} else if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
> > +			if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_COMP ||
> > +			    chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_CFM) {
> > +				/* Remote has only sent pending or
> > +				 * success responses, clean up
> > +				 */
> > +				l2cap_move_revert(chan);
> > +				chan->move_role = L2CAP_MOVE_ROLE_NONE;
> > +				chan->move_state = L2CAP_MOVE_STABLE;
> > +			}
> > +
> > +			/* Other amp move states imply that the move
> > +			 * has already aborted
> > +			 */
> > +			l2cap_send_move_chan_cfm(chan->conn, chan, chan->scid,
> > +						 L2CAP_MC_UNCONFIRMED);
> > +			__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
> > +		}
> > +
> > +		chan->hs_hchan = NULL;
> > +		chan->hs_hcon = NULL;
> > +
> > +		/* Placeholder - free logical link */
> > +
> > +	} else if (chan->state != BT_CONNECTED) {
> > +		struct l2cap_conf_rsp rsp;
> > +		u8 code;
> > +
> > +		/* Create channel complete */
> > +
> > +		/* Ignore logical link if channel is on BR/EDR */
> > +		if (!chan->local_amp_id)
> > +			return;
> > +
> > +		chan->hs_hcon = hchan->conn;

I was thinking that hs_hcon might be assigned when physical link is
completed. Isn't this assignment a bit late?

> > +		chan->hs_hcon->l2cap_data = chan->conn;
> > +
> > +		code = l2cap_build_conf_rsp(chan, &rsp,
> > +					    L2CAP_CONF_SUCCESS, 0);
> > +		l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CONF_RSP, code,
> > +			       &rsp);
> > +		set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
> > +
> > +		if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
> > +			int err = 0;
> > +
> > +			set_default_fcs(chan);
> > +
> > +			err = l2cap_ertm_init(chan);
> > +			if (err < 0)
> > +				l2cap_send_disconn_req(chan->conn, chan, -err);
> > +			else
> > +				l2cap_chan_ready(chan);
> > +		}
> > +	} else {
> > +		/* Channel move */
> > +		chan->hs_hcon = hchan->conn;
> > +		chan->hs_hcon->l2cap_data = chan->conn;
> > +
> > +		BT_DBG("move_state %d", chan->move_state);
> > +
> > +		switch (chan->move_state) {
> > +		case L2CAP_MOVE_WAIT_LOGICAL_COMP:
> > +			/* Move confirm will be sent after a success
> > +			 * response is received
> > +			 */
> > +			chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
> > +			break;
> > +		case L2CAP_MOVE_WAIT_LOGICAL_CFM:
> > +			if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
> > +				chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
> > +			} else if (chan->move_role ==
> > +				   L2CAP_MOVE_ROLE_INITIATOR) {
> > +				chan->move_state = L2CAP_MOVE_WAIT_CONFIRM_RSP;
> > +				l2cap_send_move_chan_cfm(chan->conn, chan,
> > +							 chan->scid,
> > +							 L2CAP_MR_SUCCESS);
> > +				__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
> > +			} else if (chan->move_role ==
> > +				   L2CAP_MOVE_ROLE_RESPONDER) {
> > +				chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
> > +				l2cap_send_move_chan_rsp(chan->conn,
> > +							 chan->ident,
> > +							 chan->dcid,
> > +							 L2CAP_MR_SUCCESS);
> > +			}
> > +			break;
> > +		default:
> > +			/* Move was not in expected state, free the channel */
> > +			chan->hs_hchan = NULL;
> > +			chan->hs_hcon = NULL;
> > +
> > +			/* Placeholder - free the logical link */
> > +
> > +			chan->move_state = L2CAP_MOVE_STABLE;
> > +		}
> > +	}
> >  }
> >  
> 
> I find this this function still a little bit too complex. Any chance we
> can split into more logical pieces. It is fine if not, but we should at
> least give it another try.

IMO this would be much cleaner if you would handle (status) case first
then channel create and then channel move without "if else", maybe
splitting to functions would be better.

Best regards 
Andrei Emeltchenko 

[PATCHv2 11/19] Bluetooth: Add move confirm response handling

[Mat Martineau]

The move confirm response concludes the channel move command sequence.
Receipt of this command indicates that data may begin to flow again.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap_core.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ddd8c72..2aaa9da 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4652,6 +4652,7 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
 						 u16 cmd_len, void *data)
 {
 	struct l2cap_move_chan_cfm_rsp *rsp = data;
+	struct l2cap_chan *chan;
 	u16 icid;
 
 	if (cmd_len != sizeof(*rsp))
@@ -4661,6 +4662,31 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
 
 	BT_DBG("icid 0x%4.4x", icid);
 
+	chan = l2cap_get_chan_by_scid(conn, icid);
+	if (!chan)
+		return 0;
+
+	__clear_chan_timer(chan);
+
+	if (chan->move_state == L2CAP_MOVE_WAIT_CONFIRM_RSP) {
+		chan->move_state = L2CAP_MOVE_STABLE;
+		chan->local_amp_id = chan->move_id;
+
+		if (!chan->local_amp_id && chan->hs_hchan) {
+			/* Have moved off of AMP, free the channel */
+			chan->hs_hchan = NULL;
+			chan->hs_hcon = NULL;
+
+			/* Placeholder - free the logical link */
+		}
+
+		l2cap_move_success(chan);
+
+		chan->move_role = L2CAP_MOVE_ROLE_NONE;
+	}
+
+	l2cap_chan_unlock(chan);
+
 	return 0;
 }
 
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 12/19] Bluetooth: Handle physical link completion

[Mat Martineau]

Several different actions may be taken when an AMP physical link
becomes available.  A channel being created on an AMP controller must
continue the connection process.  A channel being moved needs to
either send a move request or a move response.  A failed physical link
will revert to using a BR/EDR controller if possible.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap_core.c | 166 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 166 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 2aaa9da..a6cf657 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1015,6 +1015,19 @@ void l2cap_send_conn_req(struct l2cap_chan *chan)
 	l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_REQ, sizeof(req), &req);
 }
 
+static void l2cap_send_create_chan_req(struct l2cap_chan *chan, u8 amp_id)
+{
+	struct l2cap_create_chan_req req;
+	req.scid = cpu_to_le16(chan->scid);
+	req.psm  = chan->psm;
+	req.amp_id = amp_id;
+
+	chan->ident = l2cap_get_ident(chan->conn);
+
+	l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CREATE_CHAN_REQ,
+		       sizeof(req), &req);
+}
+
 static void l2cap_move_setup(struct l2cap_chan *chan)
 {
 	struct sk_buff *skb;
@@ -4199,6 +4212,23 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
 	return 0;
 }
 
+static void l2cap_send_move_chan_req(struct l2cap_chan *chan, u8 dest_amp_id)
+{
+	struct l2cap_move_chan_req req;
+	u8 ident;
+
+	BT_DBG("chan %p, dest_amp_id %d", chan, dest_amp_id);
+
+	ident = l2cap_get_ident(chan->conn);
+	chan->ident = ident;
+
+	req.icid = cpu_to_le16(chan->scid);
+	req.dest_amp_id = dest_amp_id;
+
+	l2cap_send_cmd(chan->conn, ident, L2CAP_MOVE_CHAN_REQ, sizeof(req),
+		       &req);
+}
+
 static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
 				     u16 icid, u16 result)
 {
@@ -4358,6 +4388,142 @@ static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
 	}
 }
 
+static void l2cap_do_create(struct l2cap_chan *chan, int result,
+			    u8 local_amp_id, u8 remote_amp_id)
+{
+	if (!test_bit(CONF_CONNECT_PEND, &chan->conf_state)) {
+		struct l2cap_conn_rsp rsp;
+		char buf[128];
+		rsp.scid = cpu_to_le16(chan->dcid);
+		rsp.dcid = cpu_to_le16(chan->scid);
+
+		/* Incoming channel on AMP */
+		if (result == L2CAP_CR_SUCCESS) {
+			/* Send successful response */
+			rsp.result = cpu_to_le16(L2CAP_CR_SUCCESS);
+			rsp.status = cpu_to_le16(L2CAP_CS_NO_INFO);
+		} else {
+			/* Send negative response */
+			rsp.result = cpu_to_le16(L2CAP_CR_NO_MEM);
+			rsp.status = cpu_to_le16(L2CAP_CS_NO_INFO);
+		}
+
+		l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CREATE_CHAN_RSP,
+			       sizeof(rsp), &rsp);
+
+		if (result == L2CAP_CR_SUCCESS) {
+			__l2cap_state_change(chan, BT_CONFIG);
+			set_bit(CONF_REQ_SENT, &chan->conf_state);
+			l2cap_send_cmd(chan->conn, l2cap_get_ident(chan->conn),
+				       L2CAP_CONF_REQ,
+				       l2cap_build_conf_req(chan, buf), buf);
+			chan->num_conf_req++;
+		}
+	} else {
+		/* Outgoing channel on AMP */
+		if (result == L2CAP_CR_SUCCESS) {
+			chan->local_amp_id = local_amp_id;
+			l2cap_send_create_chan_req(chan, remote_amp_id);
+		} else {
+			/* Revert to BR/EDR connect */
+			l2cap_send_conn_req(chan);
+		}
+	}
+}
+
+static void l2cap_do_move_initiate(struct l2cap_chan *chan, u8 local_amp_id,
+				   u8 remote_amp_id)
+{
+	l2cap_move_setup(chan);
+	chan->move_id = local_amp_id;
+	chan->move_state = L2CAP_MOVE_WAIT_RSP;
+
+	l2cap_send_move_chan_req(chan, remote_amp_id);
+	__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+}
+
+static void l2cap_do_move_respond(struct l2cap_chan *chan, int result)
+{
+	struct hci_chan *hchan = NULL;
+
+	/* Placeholder - get hci_chan for logical link */
+
+	if (hchan) {
+		if (hchan->state == BT_CONNECTED) {
+			/* Logical link is ready to go */
+			chan->hs_hcon = hchan->conn;
+			chan->hs_hcon->l2cap_data = chan->conn;
+			chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
+			l2cap_send_move_chan_rsp(chan->conn, chan->ident,
+						 chan->dcid, L2CAP_MR_SUCCESS);
+
+			l2cap_logical_cfm(chan, hchan, L2CAP_MR_SUCCESS);
+		} else {
+			/* Wait for logical link to be ready */
+			chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_CFM;
+		}
+	} else {
+		/* Logical link not available */
+		l2cap_send_move_chan_rsp(chan->conn, chan->ident, chan->dcid,
+					 L2CAP_MR_NOT_ALLOWED);
+	}
+}
+
+static void l2cap_do_move_cancel(struct l2cap_chan *chan, int result)
+{
+	if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
+		u8 rsp_result;
+		if (result == -EINVAL)
+			rsp_result = L2CAP_MR_BAD_ID;
+		else
+			rsp_result = L2CAP_MR_NOT_ALLOWED;
+
+		l2cap_send_move_chan_rsp(chan->conn, chan->ident, chan->dcid,
+					 rsp_result);
+	}
+
+	chan->move_role = L2CAP_MOVE_ROLE_NONE;
+	chan->move_state = L2CAP_MOVE_STABLE;
+
+	/* Restart data transmission */
+	l2cap_ertm_send(chan);
+}
+
+void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_amp_id,
+			u8 remote_amp_id)
+{
+	BT_DBG("chan %p, result %d, local_amp_id %d, remote_amp_id %d",
+	       chan, result, local_amp_id, remote_amp_id);
+
+	l2cap_chan_lock(chan);
+
+	if (chan->state == BT_DISCONN || chan->state == BT_CLOSED) {
+		l2cap_chan_unlock(chan);
+		return;
+	}
+
+	if (chan->state != BT_CONNECTED) {
+		l2cap_do_create(chan, result, local_amp_id, remote_amp_id);
+	} else if (result != L2CAP_MR_SUCCESS) {
+		l2cap_do_move_cancel(chan, result);
+	} else {
+		switch (chan->move_role) {
+		case L2CAP_MOVE_ROLE_INITIATOR:
+			l2cap_do_move_initiate(chan, local_amp_id,
+					       remote_amp_id);
+			break;
+		case L2CAP_MOVE_ROLE_RESPONDER:
+			l2cap_do_move_respond(chan, result);
+			break;
+		default:
+			l2cap_do_move_cancel(chan, result);
+			break;
+		}
+	}
+
+	l2cap_chan_unlock(chan);
+}
+
 static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
 					 struct l2cap_cmd_hdr *cmd,
 					 u16 cmd_len, void *data)
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 13/19] Bluetooth: Flag ACL frames as complete for AMP controllers

[Mat Martineau]

AMP controllers expect to transmit only "complete" ACL frames.  These
frames have both the "start" and "cont" bits set.  AMP does not allow
fragmented ACLs.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap_core.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index a6cf657..cb85bdd 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -762,6 +762,15 @@ static void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
 	BT_DBG("chan %p, skb %p len %d priority %u", chan, skb, skb->len,
 	       skb->priority);
 
+	if (chan->hs_hcon && !__chan_is_moving(chan)) {
+		if (chan->hs_hchan)
+			hci_send_acl(chan->hs_hchan, skb, ACL_COMPLETE);
+		else
+			kfree_skb(skb);
+
+		return;
+	}
+
 	if (!test_bit(FLAG_FLUSHABLE, &chan->flags) &&
 	    lmp_no_flush_capable(hcon->hdev))
 		flags = ACL_START_NO_FLUSH;
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 14/19] Bluetooth: Do not send data during channel move

[Mat Martineau]

Outgoing ERTM data is queued during a channel move.  The ERTM state
machine is partially reset at the start of a move, and must be
resynchronized with the remote state machine at the end of the move.
Data is not sent so that there are no state transitions between the
partial reset and the resync.

Streaming mode frames are dropped during a move.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap_core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index cb85bdd..5c7681d 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -946,6 +946,9 @@ static void l2cap_send_sframe(struct l2cap_chan *chan,
 	if (!control->sframe)
 		return;
 
+	if (__chan_is_moving(chan))
+		return;
+
 	if (test_and_clear_bit(CONN_SEND_FBIT, &chan->conn_state) &&
 	    !control->poll)
 		control->final = 1;
@@ -1824,6 +1827,9 @@ static void l2cap_streaming_send(struct l2cap_chan *chan,
 
 	BT_DBG("chan %p, skbs %p", chan, skbs);
 
+	if (__chan_is_moving(chan))
+		return;
+
 	skb_queue_splice_tail_init(skbs, &chan->tx_q);
 
 	while (!skb_queue_empty(&chan->tx_q)) {
@@ -1866,6 +1872,9 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
 	if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state))
 		return 0;
 
+	if (__chan_is_moving(chan))
+		return 0;
+
 	while (chan->tx_send_head &&
 	       chan->unacked_frames < chan->remote_tx_win &&
 	       chan->tx_state == L2CAP_TX_STATE_XMIT) {
@@ -1931,6 +1940,9 @@ static void l2cap_ertm_resend(struct l2cap_chan *chan)
 	if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state))
 		return;
 
+	if (__chan_is_moving(chan))
+		return;
+
 	while (chan->retrans_list.head != L2CAP_SEQ_LIST_CLEAR) {
 		seq = l2cap_seq_list_pop(&chan->retrans_list);
 
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 15/19] Bluetooth: Configure appropriate timeouts for AMP controllers

[Mat Martineau]

The L2CAP spec recommends specific retransmit and monitor timeouts for
ERTM channels that are on AMP controllers.  These timeouts are
calculated from the AMP controller's best effort flush timeout.

BR/EDR controllers use the default retransmit and monitor timeouts.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap_core.c | 47 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 5c7681d..14991d8 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2980,6 +2980,44 @@ static inline bool __l2cap_efs_supported(struct l2cap_chan *chan)
 	return enable_hs && chan->conn->feat_mask & L2CAP_FEAT_EXT_FLOW;
 }
 
+static void __l2cap_set_ertm_timeouts(struct l2cap_chan *chan,
+				      struct l2cap_conf_rfc *rfc)
+{
+	if (chan->local_amp_id && chan->hs_hcon) {
+		u64 ertm_to = chan->hs_hcon->hdev->amp_be_flush_to;
+
+		/* Class 1 devices have must have ERTM timeouts
+		 * exceeding the Link Supervision Timeout.  The
+		 * default Link Supervision Timeout for AMP
+		 * controllers is 10 seconds.
+		 *
+		 * Class 1 devices use 0xffffffff for their
+		 * best-effort flush timeout, so the clamping logic
+		 * will result in a timeout that meets the above
+		 * requirement.  ERTM timeouts are 16-bit values, so
+		 * the maximum timeout is 65.535 seconds.
+		 */
+
+		/* Convert timeout to milliseconds and round */
+		ertm_to = DIV_ROUND_UP_ULL(ertm_to, 1000);
+
+		/* This is the recommended formula for class 2 devices
+		 * that start ERTM timers when packets are sent to the
+		 * controller.
+		 */
+		ertm_to = 3 * ertm_to + 500;
+
+		if (ertm_to > 0xffff)
+			ertm_to = 0xffff;
+
+		rfc->retrans_timeout = cpu_to_le16((u16) ertm_to);
+		rfc->monitor_timeout = rfc->retrans_timeout;
+	} else {
+		rfc->retrans_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_RETRANS_TO);
+		rfc->monitor_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
+	}
+}
+
 static inline void l2cap_txwin_setup(struct l2cap_chan *chan)
 {
 	if (chan->tx_win > L2CAP_DEFAULT_TX_WINDOW &&
@@ -3046,8 +3084,8 @@ done:
 	case L2CAP_MODE_ERTM:
 		rfc.mode            = L2CAP_MODE_ERTM;
 		rfc.max_transmit    = chan->max_tx;
-		rfc.retrans_timeout = 0;
-		rfc.monitor_timeout = 0;
+
+		__l2cap_set_ertm_timeouts(chan, &rfc);
 
 		size = min_t(u16, L2CAP_DEFAULT_MAX_PDU_SIZE, chan->conn->mtu -
 			     L2CAP_EXT_HDR_SIZE - L2CAP_SDULEN_SIZE -
@@ -3275,10 +3313,7 @@ done:
 			rfc.max_pdu_size = cpu_to_le16(size);
 			chan->remote_mps = size;
 
-			rfc.retrans_timeout =
-				__constant_cpu_to_le16(L2CAP_DEFAULT_RETRANS_TO);
-			rfc.monitor_timeout =
-				__constant_cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
+			__l2cap_set_ertm_timeouts(chan, &rfc);
 
 			set_bit(CONF_MODE_DONE, &chan->conf_state);
 
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 16/19] Bluetooth: Ignore BR/EDR packet size constraints when fragmenting for AMP

[Mat Martineau]

When operating over BR/EDR, ERTM accounts for the maximum over-the-air
packet size when setting the PDU size.  AMP controllers do not use the
same over-the-air packets, so the PDU size should only be based on the
HCI MTU of the AMP controller.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap_core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 14991d8..7e8fe84 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2285,7 +2285,9 @@ static int l2cap_segment_sdu(struct l2cap_chan *chan,
 	/* PDU size is derived from the HCI MTU */
 	pdu_len = chan->conn->mtu;
 
-	pdu_len = min_t(size_t, pdu_len, L2CAP_BREDR_MAX_PAYLOAD);
+	/* Constrain PDU size for BR/EDR connections */
+	if (!chan->hs_hcon)
+		pdu_len = min_t(size_t, pdu_len, L2CAP_BREDR_MAX_PAYLOAD);
 
 	/* Adjust for largest possible L2CAP overhead. */
 	if (chan->fcs)
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 17/19] Bluetooth: Send create channel request instead of connect for AMP

[Mat Martineau]

When the channel policy is set to prefer AMP, then an L2CAP channel is
set up using the "create channel" command rather than the "connect"
command.  A physical link is also set up before sending "create
channel".

Behavior is unchanged if enable_hs is false.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 7e8fe84..17d917b 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -58,6 +58,9 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn,
 static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
 		     struct sk_buff_head *skbs, u8 event);
 
+static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
+			      u8 status);
+
 /* ---- L2CAP channels ---- */
 
 static struct l2cap_chan *__l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
@@ -576,6 +579,13 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
 			mgr->bredr_chan = NULL;
 	}
 
+	if (chan->hs_hchan) {
+		chan->hs_hchan = NULL;
+		chan->hs_hcon = NULL;
+
+		/* Placeholder - free logical link */
+	}
+
 	chan->ops->teardown(chan, err);
 
 	if (test_bit(CONF_NOT_COMPLETE, &chan->conf_state))
@@ -1126,6 +1136,7 @@ static void l2cap_start_connection(struct l2cap_chan *chan)
 {
 	if (__amp_capable(chan)) {
 		BT_DBG("chan %p AMP capable: discover AMPs", chan);
+		set_bit(CONF_CONNECT_PEND, &chan->conf_state);
 		a2mp_discover_amp(chan);
 	} else {
 		l2cap_send_conn_req(chan);
@@ -1271,6 +1282,16 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 				rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHEN_PEND);
 			}
 
+			if (rsp.result == __constant_cpu_to_le16(L2CAP_CR_SUCCESS) &&
+			    chan->local_amp_id) {
+				/* Placeholder - uncomment when amp functions
+				 * are available
+				amp_accept_physical(chan, chan->local_amp_id);
+				 */
+				l2cap_chan_unlock(chan);
+				continue;
+			}
+
 			l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP,
 				       sizeof(rsp), &rsp);
 
@@ -3359,6 +3380,18 @@ done:
 			rfc.mode = chan->mode;
 		}
 
+		if (test_bit(CONF_LOC_CONF_PEND, &chan->conf_state) &&
+		    chan->local_amp_id) {
+			struct hci_chan *hchan = NULL;
+
+			/* Placeholder - get hci_chan for logical link */
+
+			if (hchan && hchan->state == BT_CONNECTED) {
+				l2cap_logical_cfm(chan, hchan,
+						  L2CAP_MR_SUCCESS);
+			}
+		}
+
 		if (result == L2CAP_CONF_SUCCESS)
 			set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
 	}
@@ -6367,6 +6400,15 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 					stat = L2CAP_CS_AUTHOR_PEND;
 					chan->ops->defer(chan);
 				} else {
+					if (chan->local_amp_id) {
+						/* Placeholder - accept physical
+						 * link
+						amp_accept_physical(chan,
+							chan->local_amp_id);
+						*/
+						continue;
+					}
+
 					__l2cap_state_change(chan, BT_CONFIG);
 					res = L2CAP_CR_SUCCESS;
 					stat = L2CAP_CS_NO_INFO;
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 17/19] Bluetooth: Send create channel request instead of connect for AMP

[Marcel Holtmann]

Hi Mat,

> When the channel policy is set to prefer AMP, then an L2CAP channel is
> set up using the "create channel" command rather than the "connect"
> command.  A physical link is also set up before sending "create
> channel".
> 
> Behavior is unchanged if enable_hs is false.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 7e8fe84..17d917b 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -58,6 +58,9 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn,
>  static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
>  		     struct sk_buff_head *skbs, u8 event);
>  
> +static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
> +			      u8 status);
> +

any chance to avoid this forward declaration by moving some functions
around?

>  /* ---- L2CAP channels ---- */
>  
>  static struct l2cap_chan *__l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
> @@ -576,6 +579,13 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
>  			mgr->bredr_chan = NULL;
>  	}
>  
> +	if (chan->hs_hchan) {
> +		chan->hs_hchan = NULL;
> +		chan->hs_hcon = NULL;
> +
> +		/* Placeholder - free logical link */
> +	}
> +
>  	chan->ops->teardown(chan, err);
>  
>  	if (test_bit(CONF_NOT_COMPLETE, &chan->conf_state))
> @@ -1126,6 +1136,7 @@ static void l2cap_start_connection(struct l2cap_chan *chan)
>  {
>  	if (__amp_capable(chan)) {
>  		BT_DBG("chan %p AMP capable: discover AMPs", chan);
> +		set_bit(CONF_CONNECT_PEND, &chan->conf_state);
>  		a2mp_discover_amp(chan);
>  	} else {
>  		l2cap_send_conn_req(chan);
> @@ -1271,6 +1282,16 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
>  				rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHEN_PEND);
>  			}
>  
> +			if (rsp.result == __constant_cpu_to_le16(L2CAP_CR_SUCCESS) &&
> +			    chan->local_amp_id) {
> +				/* Placeholder - uncomment when amp functions
> +				 * are available
> +				amp_accept_physical(chan, chan->local_amp_id);
> +				 */
> +				l2cap_chan_unlock(chan);
> +				continue;
> +			}
> +
>  			l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP,
>  				       sizeof(rsp), &rsp);
>  
> @@ -3359,6 +3380,18 @@ done:
>  			rfc.mode = chan->mode;
>  		}
>  
> +		if (test_bit(CONF_LOC_CONF_PEND, &chan->conf_state) &&
> +		    chan->local_amp_id) {
> +			struct hci_chan *hchan = NULL;
> +
> +			/* Placeholder - get hci_chan for logical link */
> +
> +			if (hchan && hchan->state == BT_CONNECTED) {
> +				l2cap_logical_cfm(chan, hchan,
> +						  L2CAP_MR_SUCCESS);
> +			}
> +		}
> +
>  		if (result == L2CAP_CONF_SUCCESS)
>  			set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
>  	}
> @@ -6367,6 +6400,15 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
>  					stat = L2CAP_CS_AUTHOR_PEND;
>  					chan->ops->defer(chan);
>  				} else {
> +					if (chan->local_amp_id) {
> +						/* Placeholder - accept physical
> +						 * link
> +						amp_accept_physical(chan,
> +							chan->local_amp_id);
> +						*/
> +						continue;
> +					}
> +
>  					__l2cap_state_change(chan, BT_CONFIG);
>  					res = L2CAP_CR_SUCCESS;
>  					stat = L2CAP_CS_NO_INFO;

Check the forward declaration and fix it if possible. Otherwise this is
fine.

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCHv2 17/19] Bluetooth: Send create channel request instead of connect for AMP

[Andrei Emeltchenko]

On Tue, Oct 16, 2012 at 04:36:31PM -0700, Mat Martineau wrote:
> When the channel policy is set to prefer AMP, then an L2CAP channel is
> set up using the "create channel" command rather than the "connect"
> command.  A physical link is also set up before sending "create
> channel".

I feel that this patch doing something else that commit message implies.
So it is better to skip it for now.

> 
> Behavior is unchanged if enable_hs is false.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 7e8fe84..17d917b 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -58,6 +58,9 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn,
>  static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
>  		     struct sk_buff_head *skbs, u8 event);
>  
> +static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
> +			      u8 status);
> +
>  /* ---- L2CAP channels ---- */
>  
>  static struct l2cap_chan *__l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
> @@ -576,6 +579,13 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
>  			mgr->bredr_chan = NULL;
>  	}
>  
> +	if (chan->hs_hchan) {
> +		chan->hs_hchan = NULL;
> +		chan->hs_hcon = NULL;
> +
> +		/* Placeholder - free logical link */
> +	}
> +

Here we free logical link

>  	chan->ops->teardown(chan, err);
>  
>  	if (test_bit(CONF_NOT_COMPLETE, &chan->conf_state))
> @@ -1126,6 +1136,7 @@ static void l2cap_start_connection(struct l2cap_chan *chan)
>  {
>  	if (__amp_capable(chan)) {
>  		BT_DBG("chan %p AMP capable: discover AMPs", chan);
> +		set_bit(CONF_CONNECT_PEND, &chan->conf_state);
>  		a2mp_discover_amp(chan);
>  	} else {
>  		l2cap_send_conn_req(chan);
> @@ -1271,6 +1282,16 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
>  				rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHEN_PEND);
>  			}
>  
> +			if (rsp.result == __constant_cpu_to_le16(L2CAP_CR_SUCCESS) &&
> +			    chan->local_amp_id) {
> +				/* Placeholder - uncomment when amp functions
> +				 * are available
> +				amp_accept_physical(chan, chan->local_amp_id);
> +				 */
> +				l2cap_chan_unlock(chan);
> +				continue;
> +			}
> +
>  			l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP,
>  				       sizeof(rsp), &rsp);
>  
> @@ -3359,6 +3380,18 @@ done:
>  			rfc.mode = chan->mode;
>  		}
>  
> +		if (test_bit(CONF_LOC_CONF_PEND, &chan->conf_state) &&
> +		    chan->local_amp_id) {
> +			struct hci_chan *hchan = NULL;
> +
> +			/* Placeholder - get hci_chan for logical link */
> +
> +			if (hchan && hchan->state == BT_CONNECTED) {
> +				l2cap_logical_cfm(chan, hchan,
> +						  L2CAP_MR_SUCCESS);
> +			}
> +		}
> +

this is configuration phase which is happening after channel is created


>  		if (result == L2CAP_CONF_SUCCESS)
>  			set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
>  	}
> @@ -6367,6 +6400,15 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
>  					stat = L2CAP_CS_AUTHOR_PEND;
>  					chan->ops->defer(chan);
>  				} else {
> +					if (chan->local_amp_id) {
> +						/* Placeholder - accept physical
> +						 * link
> +						amp_accept_physical(chan,
> +							chan->local_amp_id);
> +						*/
> +						continue;
> +					}
> +

here is accept physical link

Best regards 
Andrei Emeltchenko 

Re: [PATCHv2 17/19] Bluetooth: Send create channel request instead of connect for AMP

[Mat Martineau]

Andrei -

On Thu, 18 Oct 2012, Andrei Emeltchenko wrote:

> On Tue, Oct 16, 2012 at 04:36:31PM -0700, Mat Martineau wrote:
>> When the channel policy is set to prefer AMP, then an L2CAP channel is
>> set up using the "create channel" command rather than the "connect"
>> command.  A physical link is also set up before sending "create
>> channel".
>
> I feel that this patch doing something else that commit message implies.
> So it is better to skip it for now.

I think you're right and we should skip this patch in this series. 
The logical and physical link handling has changed a bit, so some of 
these changes might not fit any more.

>>
>> Behavior is unchanged if enable_hs is false.
>>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  net/bluetooth/l2cap_core.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 42 insertions(+)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 7e8fe84..17d917b 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -58,6 +58,9 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn,
>>  static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
>>  		     struct sk_buff_head *skbs, u8 event);
>>
>> +static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
>> +			      u8 status);
>> +
>>  /* ---- L2CAP channels ---- */
>>
>>  static struct l2cap_chan *__l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
>> @@ -576,6 +579,13 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
>>  			mgr->bredr_chan = NULL;
>>  	}
>>
>> +	if (chan->hs_hchan) {
>> +		chan->hs_hchan = NULL;
>> +		chan->hs_hcon = NULL;
>> +
>> +		/* Placeholder - free logical link */
>> +	}
>> +
>
> Here we free logical link
>
>>  	chan->ops->teardown(chan, err);
>>
>>  	if (test_bit(CONF_NOT_COMPLETE, &chan->conf_state))
>> @@ -1126,6 +1136,7 @@ static void l2cap_start_connection(struct l2cap_chan *chan)
>>  {
>>  	if (__amp_capable(chan)) {
>>  		BT_DBG("chan %p AMP capable: discover AMPs", chan);
>> +		set_bit(CONF_CONNECT_PEND, &chan->conf_state);
>>  		a2mp_discover_amp(chan);
>>  	} else {
>>  		l2cap_send_conn_req(chan);
>> @@ -1271,6 +1282,16 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
>>  				rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHEN_PEND);
>>  			}
>>
>> +			if (rsp.result == __constant_cpu_to_le16(L2CAP_CR_SUCCESS) &&
>> +			    chan->local_amp_id) {
>> +				/* Placeholder - uncomment when amp functions
>> +				 * are available
>> +				amp_accept_physical(chan, chan->local_amp_id);
>> +				 */
>> +				l2cap_chan_unlock(chan);
>> +				continue;
>> +			}
>> +
>>  			l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP,
>>  				       sizeof(rsp), &rsp);
>>
>> @@ -3359,6 +3380,18 @@ done:
>>  			rfc.mode = chan->mode;
>>  		}
>>
>> +		if (test_bit(CONF_LOC_CONF_PEND, &chan->conf_state) &&
>> +		    chan->local_amp_id) {
>> +			struct hci_chan *hchan = NULL;
>> +
>> +			/* Placeholder - get hci_chan for logical link */
>> +
>> +			if (hchan && hchan->state == BT_CONNECTED) {
>> +				l2cap_logical_cfm(chan, hchan,
>> +						  L2CAP_MR_SUCCESS);
>> +			}
>> +		}
>> +
>
> this is configuration phase which is happening after channel is created
>
>
>>  		if (result == L2CAP_CONF_SUCCESS)
>>  			set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
>>  	}
>> @@ -6367,6 +6400,15 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
>>  					stat = L2CAP_CS_AUTHOR_PEND;
>>  					chan->ops->defer(chan);
>>  				} else {
>> +					if (chan->local_amp_id) {
>> +						/* Placeholder - accept physical
>> +						 * link
>> +						amp_accept_physical(chan,
>> +							chan->local_amp_id);
>> +						*/
>> +						continue;
>> +					}
>> +
>
> here is accept physical link

Regards,

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 18/19] Bluetooth: Do not retransmit data during a channel move

[Mat Martineau]

Do not retransmit previously-sent data when a "receiver ready" s-frame
with the "final" flag is received during a move.

The ERTM state machines will resynchronize at the end of a channel
move, and the state machine needs to avoid state changes during a
move.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 17d917b..2263c1d 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5636,8 +5636,8 @@ static int l2cap_rx_state_recv(struct l2cap_chan *chan,
 		if (control->final) {
 			clear_bit(CONN_REMOTE_BUSY, &chan->conn_state);
 
-			if (!test_and_clear_bit(CONN_REJ_ACT,
-						&chan->conn_state)) {
+			if (!test_and_clear_bit(CONN_REJ_ACT, &chan->conn_state) &&
+			    !__chan_is_moving(chan)) {
 				control->final = 0;
 				l2cap_retransmit_all(chan, control);
 			}
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 19/19] Bluetooth: Start channel move when socket option is changed

[Mat Martineau]

Channel moves are triggered by changes to the BT_CHANNEL_POLICY
sockopt when an ERTM or streaming-mode channel is connected.

Moves are only started if enable_hs is true.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/l2cap.h |  1 +
 net/bluetooth/l2cap_core.c    | 20 ++++++++++++++++++++
 net/bluetooth/l2cap_sock.c    |  5 +++++
 3 files changed, 26 insertions(+)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index b4c3c65..49783e9 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -809,5 +809,6 @@ void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan);
 void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan);
 void l2cap_chan_del(struct l2cap_chan *chan, int err);
 void l2cap_send_conn_req(struct l2cap_chan *chan);
+void l2cap_move_start(struct l2cap_chan *chan);
 
 #endif /* __L2CAP_H */
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 2263c1d..3f6410b 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4479,6 +4479,26 @@ static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
 	}
 }
 
+void l2cap_move_start(struct l2cap_chan *chan)
+{
+	BT_DBG("chan %p", chan);
+
+	if (chan->local_amp_id == 0) {
+		if (chan->chan_policy != BT_CHANNEL_POLICY_AMP_PREFERRED)
+			return;
+		chan->move_role = L2CAP_MOVE_ROLE_INITIATOR;
+		chan->move_state = L2CAP_MOVE_WAIT_PREPARE;
+		/* Placeholder - start physical link setup */
+	} else {
+		chan->move_role = L2CAP_MOVE_ROLE_INITIATOR;
+		chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
+		chan->move_id = 0;
+		l2cap_move_start(chan);
+		l2cap_send_move_chan_req(chan, 0);
+		__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+	}
+}
+
 static void l2cap_do_create(struct l2cap_chan *chan, int result,
 			    u8 local_amp_id, u8 remote_amp_id)
 {
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 5fae2bd..7cb4d73 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -735,6 +735,11 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 		}
 
 		chan->chan_policy = (u8) opt;
+
+		if (sk->sk_state == BT_CONNECTED &&
+		    chan->move_role == L2CAP_MOVE_ROLE_NONE)
+			l2cap_move_start(chan);
+
 		break;
 
 	default:
-- 
1.7.12.3

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation