Return-Path: MIME-Version: 1.0 In-Reply-To: <1352830825-13987-2-git-send-email-frederic.dalleau@linux.intel.com> References: <1352830825-13987-1-git-send-email-frederic.dalleau@linux.intel.com> <1352830825-13987-2-git-send-email-frederic.dalleau@linux.intel.com> Date: Tue, 13 Nov 2012 20:45:14 +0200 Message-ID: Subject: Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket From: Luiz Augusto von Dentz To: =?ISO-8859-1?Q?Fr=E9d=E9ric_Dalleau?= Cc: linux-bluetooth@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Fr?d?ric, On Tue, Nov 13, 2012 at 8:20 PM, Fr?d?ric Dalleau wrote: > In order to authenticate and later configure an incoming SCO connection, the > BT_DEFER_SETUP option is added. > When an connection is requested, the listening socket is unblocked but the > effective connection setup happens only on first recv. > Any send between accept and recv fails with -ENOTCONN. I think you should detail when exactly we do defer, we need to defer before the audio settings negotiation takes place because latter once we add support for passing the settings this happens during this stage before accepting. > --- > include/net/bluetooth/hci_core.h | 15 +++++++ > net/bluetooth/hci_event.c | 47 +++++++++++++++++++- > net/bluetooth/sco.c | 88 ++++++++++++++++++++++++++++++++++++-- > 3 files changed, 145 insertions(+), 5 deletions(-) > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > index ef5b85d..2ee0ecb 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -377,6 +377,7 @@ extern int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, > u16 flags); > > extern int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr); > +extern int sco_defer(struct hci_dev *hdev, bdaddr_t *bdaddr); > extern void sco_connect_cfm(struct hci_conn *hcon, __u8 status); > extern void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason); > extern int sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb); > @@ -577,6 +578,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst); > int hci_conn_del(struct hci_conn *conn); > void hci_conn_hash_flush(struct hci_dev *hdev); > void hci_conn_check_pending(struct hci_dev *hdev); > +void hci_conn_accept(struct hci_conn *conn, int mask); > > struct hci_chan *hci_chan_create(struct hci_conn *conn); > void hci_chan_del(struct hci_chan *chan); > @@ -796,6 +798,19 @@ static inline int hci_proto_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, > } > } > > +static inline int hci_proto_defered(struct hci_dev *hdev, bdaddr_t *bdaddr, > + __u8 type) > +{ > + switch (type) { > + case SCO_LINK: > + case ESCO_LINK: > + return sco_defer(hdev, bdaddr); > + > + default: > + return 0; > + } > +} > + > static inline void hci_proto_connect_cfm(struct hci_conn *conn, __u8 status) > { > switch (conn->type) { > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index 9f5c5f2..167eca0 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -2047,15 +2047,54 @@ unlock: > hci_conn_check_pending(hdev); > } > > +void hci_conn_accept(struct hci_conn *conn, int mask) > +{ > + struct hci_dev *hdev = conn->hdev; > + > + BT_DBG("conn %p", conn); > + > + if (!lmp_esco_capable(hdev)) { > + struct hci_cp_accept_conn_req cp; > + > + conn->state = BT_CONNECT; > + bacpy(&cp.bdaddr, &conn->dst); > + > + if (lmp_rswitch_capable(hdev) && (mask & HCI_LM_MASTER)) > + cp.role = 0x00; /* Become master */ > + else > + cp.role = 0x01; /* Remain slave */ > + > + hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp), > + &cp); > + } else /* lmp_esco_capable(hdev)) */ { > + struct hci_cp_accept_sync_conn_req cp; > + > + conn->state = BT_CONNECT; > + bacpy(&cp.bdaddr, &conn->dst); > + cp.pkt_type = cpu_to_le16(conn->pkt_type); > + > + cp.tx_bandwidth = __constant_cpu_to_le32(0x00001f40); > + cp.rx_bandwidth = __constant_cpu_to_le32(0x00001f40); > + cp.max_latency = __constant_cpu_to_le16(0xffff); > + cp.content_format = cpu_to_le16(hdev->voice_setting); > + cp.retrans_effort = 0xff; > + > + hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ, > + sizeof(cp), &cp); > + } > +} > + > static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb) > { > struct hci_ev_conn_request *ev = (void *) skb->data; > int mask = hdev->link_mode; > + int defered; > > BT_DBG("%s bdaddr %pMR type 0x%x", hdev->name, &ev->bdaddr, > ev->link_type); > > mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ev->link_type); > + defered = hci_proto_defered(hdev, &ev->bdaddr, ev->link_type); > > if ((mask & HCI_LM_ACCEPT) && > !hci_blacklist_lookup(hdev, &ev->bdaddr)) { > @@ -2085,7 +2124,8 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb) > > hci_dev_unlock(hdev); > > - if (ev->link_type == ACL_LINK || !lmp_esco_capable(hdev)) { > + if (ev->link_type == ACL_LINK || > + (!defered && !lmp_esco_capable(hdev))) { > struct hci_cp_accept_conn_req cp; > > bacpy(&cp.bdaddr, &ev->bdaddr); > @@ -2097,7 +2137,7 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb) > > hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp), > &cp); > - } else { > + } else if (!defered) { > struct hci_cp_accept_sync_conn_req cp; > > bacpy(&cp.bdaddr, &ev->bdaddr); > @@ -2111,6 +2151,9 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb) > > hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ, > sizeof(cp), &cp); > + } else { > + hci_proto_connect_cfm(conn, 0); > + hci_conn_put(conn); > } > } else { > /* Connection rejected */ > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c > index 450cdcd..416801a 100644 > --- a/net/bluetooth/sco.c > +++ b/net/bluetooth/sco.c > @@ -662,16 +662,57 @@ static int sco_sock_sendmsg(struct kiocb *iocb, struct socket *sock, > return err; > } > > +static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock, > + struct msghdr *msg, size_t len, int flags) > +{ > + struct sock *sk = sock->sk; > + struct sco_pinfo *pi = sco_pi(sk); > + > + lock_sock(sk); > + > + if (sk->sk_state == BT_CONNECT && > + test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) { > + hci_conn_accept(pi->conn->hcon, 0); > + sk->sk_state = BT_CONNECT2; > + > + release_sock(sk); > + return 0; > + } > + > + release_sock(sk); > + > + return bt_sock_recvmsg(iocb, sock, msg, len, flags); > +} > + > static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char __user *optval, unsigned int optlen) > { > struct sock *sk = sock->sk; > int err = 0; > + u32 opt; > > BT_DBG("sk %p", sk); > > lock_sock(sk); > > switch (optname) { > + > + case BT_DEFER_SETUP: > + if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) { > + err = -EINVAL; > + break; > + } > + > + if (get_user(opt, (u32 __user *) optval)) { > + err = -EFAULT; > + break; > + } > + > + if (opt) > + set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags); > + else > + clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags); > + break; > + > default: > err = -ENOPROTOOPT; > break; > @@ -753,6 +794,19 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char > lock_sock(sk); > > switch (optname) { > + > + case BT_DEFER_SETUP: > + if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) { > + err = -EINVAL; > + break; > + } > + > + if (put_user(test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags), > + (u32 __user *) optval)) > + err = -EFAULT; > + > + break; > + > default: > err = -ENOPROTOOPT; > break; > @@ -874,7 +928,10 @@ static void sco_conn_ready(struct sco_conn *conn) > hci_conn_hold(conn->hcon); > __sco_chan_add(conn, sk, parent); > > - sk->sk_state = BT_CONNECTED; > + if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags)) > + sk->sk_state = BT_CONNECT; > + else > + sk->sk_state = BT_CONNECTED; > > /* Wake up parent */ > parent->sk_data_ready(parent, 1); > @@ -912,6 +969,32 @@ int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr) > return lm; > } > > +int sco_defer(struct hci_dev *hdev, bdaddr_t *bdaddr) > +{ > + struct sock *sk; > + struct hlist_node *node; > + int defer = 0; > + > + BT_DBG("hdev %s, bdaddr %pMR", hdev->name, bdaddr); > + > + /* Find listening sockets */ > + read_lock(&sco_sk_list.lock); > + sk_for_each(sk, node, &sco_sk_list.head) { > + if (sk->sk_state != BT_LISTEN) > + continue; > + > + if (!bacmp(&bt_sk(sk)->src, &hdev->bdaddr) || > + !bacmp(&bt_sk(sk)->src, BDADDR_ANY)) { > + if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) > + defer = 1; > + break; > + } > + } > + read_unlock(&sco_sk_list.lock); > + > + return defer; > +} > + > void sco_connect_cfm(struct hci_conn *hcon, __u8 status) > { > BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status); > @@ -992,7 +1075,7 @@ static const struct proto_ops sco_sock_ops = { > .accept = sco_sock_accept, > .getname = sco_sock_getname, > .sendmsg = sco_sock_sendmsg, > - .recvmsg = bt_sock_recvmsg, > + .recvmsg = sco_sock_recvmsg, > .poll = bt_sock_poll, > .ioctl = bt_sock_ioctl, > .mmap = sock_no_mmap, > @@ -1036,7 +1119,6 @@ int __init sco_init(void) > BT_ERR("Failed to create SCO debug file"); > } > > - BT_INFO("SCO socket layer initialized"); There doesn't seems to be any need to remove the line above, could you please fix that. -- Luiz Augusto von Dentz