From: Marcel Holtmann <marcel@holtmann.org>
To: Mat Martineau <mathewm@codeaurora.org>
Cc: linux-bluetooth@vger.kernel.org, gustavo@padovan.org,
sunnyk@codeaurora.org
Subject: Re: [PATCH 09/19] Bluetooth: Move channel response
Date: Mon, 15 Oct 2012 20:29:52 +0200 [thread overview]
Message-ID: <1350325792.26318.13.camel@aeonflux> (raw)
In-Reply-To: <1350315248-7690-10-git-send-email-mathewm@codeaurora.org>
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 | 137 +++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 137 insertions(+), 2 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 ef744a9..c687cc1 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4221,6 +4221,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,6 +4324,7 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
> u16 cmd_len, void *data)
> {
> struct l2cap_move_chan_rsp *rsp = data;
> + struct l2cap_chan *chan = NULL;
do we need the = NULL assignment here?
> u16 icid, result;
>
> if (cmd_len != sizeof(*rsp))
> @@ -4327,8 +4335,133 @@ 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);
> + switch (result) {
> + case L2CAP_MR_SUCCESS:
> + case L2CAP_MR_PEND:
> + chan = l2cap_get_chan_by_scid(conn, icid);
> + if (!chan) {
> + l2cap_send_move_chan_cfm(conn, NULL, icid,
> + L2CAP_MC_UNCONFIRMED);
> + break;
> + }
> +
> + __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) {
> + break;
> + } 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);
> + break;
> + }
> +
> + /* If the logical link is not yet connected, do not
> + * send confirmation.
> + */
> + if (hchan->state != BT_CONNECTED)
> + break;
> +
> + /* 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);
> + }
> + break;
> + default:
> + /* Failed (including collision case) */
> + mutex_lock(&conn->chan_lock);
> + chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
> + if (chan)
> + l2cap_chan_lock(chan);
> + mutex_unlock(&conn->chan_lock);
Don't we have a function for this?
> +
> + if (!chan) {
> + /* Could not locate channel, icid is best guess */
> + l2cap_send_move_chan_cfm(conn, NULL, icid,
> + L2CAP_MC_UNCONFIRMED);
> + break;
> + }
> +
> + __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);
> + break;
> + }
> +
> + if (chan)
> + l2cap_chan_unlock(chan);
I rather prefer that we unlock in the case statements. And not a global
one. And we might have to split out the case statements into separate
functions for readability.
Regards
Marcel
next prev parent reply other threads:[~2012-10-15 18:29 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-15 15:33 [PATCH 00/19] L2CAP signaling for AMP channel create/move Mat Martineau
2012-10-15 15:33 ` [PATCH 01/19] Bluetooth: Add new l2cap_chan struct members for high speed channels Mat Martineau
2012-10-15 15:33 ` [PATCH 02/19] Bluetooth: Add L2CAP create channel request handling Mat Martineau
2012-10-15 15:33 ` [PATCH 03/19] Bluetooth: Remove unnecessary intermediate function Mat Martineau
2012-10-15 18:10 ` Marcel Holtmann
2012-10-15 15:33 ` [PATCH 04/19] Bluetooth: Lookup channel structure based on DCID Mat Martineau
2012-10-15 18:13 ` Marcel Holtmann
2012-10-15 15:33 ` [PATCH 05/19] Bluetooth: Channel move request handling Mat Martineau
2012-10-15 18:17 ` Marcel Holtmann
2012-10-16 10:18 ` Andrei Emeltchenko
2012-10-15 15:33 ` [PATCH 06/19] Bluetooth: Add new ERTM receive states for channel move Mat Martineau
2012-10-15 18:23 ` Marcel Holtmann
2012-10-16 10:21 ` Andrei Emeltchenko
2012-10-15 15:33 ` [PATCH 07/19] Bluetooth: Add move channel confirm handling Mat Martineau
2012-10-15 18:25 ` Marcel Holtmann
2012-10-16 17:30 ` Mat Martineau
2012-10-17 17:10 ` Marcel Holtmann
2012-10-16 10:27 ` Andrei Emeltchenko
2012-10-16 23:00 ` Mat Martineau
2012-10-15 15:33 ` [PATCH 08/19] Bluetooth: Add state to hci_chan Mat Martineau
2012-10-15 18:26 ` Marcel Holtmann
2012-10-15 15:33 ` [PATCH 09/19] Bluetooth: Move channel response Mat Martineau
2012-10-15 18:29 ` Marcel Holtmann [this message]
2012-10-16 17:56 ` Mat Martineau
2012-10-15 15:33 ` [PATCH 10/19] Bluetooth: Add logical link confirm Mat Martineau
2012-10-15 18:37 ` Marcel Holtmann
2012-10-16 10:38 ` Andrei Emeltchenko
2012-10-16 22:25 ` Mat Martineau
2012-10-15 15:34 ` [PATCH 11/19] Bluetooth: Add move confirm response handling Mat Martineau
2012-10-15 18:38 ` Marcel Holtmann
2012-10-15 15:34 ` [PATCH 12/19] Bluetooth: Handle physical link completion Mat Martineau
2012-10-15 18:40 ` Marcel Holtmann
2012-10-15 15:34 ` [PATCH 13/19] Bluetooth: Flag ACL frames as complete for AMP controllers Mat Martineau
2012-10-15 18:41 ` Marcel Holtmann
2012-10-15 15:34 ` [PATCH 14/19] Bluetooth: Do not send data during channel move Mat Martineau
2012-10-15 18:42 ` Marcel Holtmann
2012-10-16 10:50 ` Andrei Emeltchenko
2012-10-15 15:34 ` [PATCH 15/19] Bluetooth: Configure appropriate timeouts for AMP controllers Mat Martineau
2012-10-15 18:44 ` Marcel Holtmann
2012-10-15 15:34 ` [PATCH 16/19] Bluetooth: Ignore BR/EDR packet size constraints when fragmenting for AMP Mat Martineau
2012-10-15 15:34 ` [PATCH 17/19] Bluetooth: Send create channel request instead of connect " Mat Martineau
2012-10-15 15:34 ` [PATCH 18/19] Bluetooth: Do not retransmit data during a channel move Mat Martineau
2012-10-15 18:46 ` Marcel Holtmann
2012-10-15 15:34 ` [PATCH 19/19] Bluetooth: Start channel move when socket option is changed Mat Martineau
2012-10-15 18:47 ` Marcel Holtmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1350325792.26318.13.camel@aeonflux \
--to=marcel@holtmann.org \
--cc=gustavo@padovan.org \
--cc=linux-bluetooth@vger.kernel.org \
--cc=mathewm@codeaurora.org \
--cc=sunnyk@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).