Return-Path: MIME-Version: 1.0 In-Reply-To: <4B7BB746.1040806@nokia.com> References: <1265918068-14721-1-git-send-email-npelly@google.com> <35c90d961002111159h6727bc2cw28b55ce6e919fb4f@mail.gmail.com> <35c90d961002151315i5fc0f5b9y36aaba4415987a2f@mail.gmail.com> <4B7BB746.1040806@nokia.com> From: Nick Pelly Date: Wed, 17 Feb 2010 08:49:11 -0800 Message-ID: <35c90d961002170849m1d13743fg725a87594d63b80c@mail.gmail.com> Subject: Re: [PATCH] Bluetooth: Allow SCO/eSCO packet type selection for outgoing SCO connections. To: Ville Tervo Cc: "linux-bluetooth@vger.kernel.org" Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: On Wed, Feb 17, 2010 at 1:30 AM, Ville Tervo wrote: > ext Nick Pelly wrote: >> >> On Thu, Feb 11, 2010 at 11:59 AM, Nick Pelly wrote: >>> >>> On Thu, Feb 11, 2010 at 11:54 AM, Nick Pelly wrote: >>>> >>>> __u16 sco_pkt_type is introduced to struct sockaddr_sco. It allows >>>> bitwise >>>> selection of SCO/eSCO packet types as per the Packet_Type parameter in >>>> HCI Setup Synchrnous Connection Command. >>>> >>>> 0x0001 HV1 may be used. >>>> 0x0002 HV2 may be used. >>>> 0x0004 HV3 may be used. >>>> 0x0008 EV3 may be used. >>>> 0x0010 EV4 may be used. >>>> 0x0020 EV5 may be used. >>>> 0x0040 2-EV3 may not be used. >>>> 0x0080 3-EV3 may not be used. >>>> 0x0100 2-EV5 may not be used. >>>> 0x0200 3-EV5 may not be used. >>>> >>>> We've followed the Bluetooth SIG convention of reversing the logic on >>>> the >>>> EDR bits. >>>> >>>> Packet type selection is just a request made to the Bluetooth chipset, >>>> and >>>> it is up to the link manager on the chipset to negiotiate and decide on >>>> the >>>> actual packet types used. Furthermore, when a SCO/eSCO connection is >>>> eventually >>>> made there is no way for the host stack to determine which packet type >>>> was used >>>> (however it is possible to get the link type of SCO or eSCO). >>>> >>>> sco_pkt_type is ignored for incoming SCO connections. It is possible >>>> to add this in the future as a parameter to the Accept Synchronous >>>> Connection >>>> Command, however its a little trickier because the kernel does not >>>> currently preserve sockaddr_sco data between userspace calls to >>>> accept(). >>>> >>>> Typically userspace just wants to select eSCO or SCO packet types, which >>>> can be >>>> done with the new constants ALL_SCO_PKTS or ALL_ESCO_PKTS. If >>>> sco_pkt_type is >>>> zero, or userspace uses the old sockaddr_sco structure size, then >>>> ALL_ESCO_PKTS >>>> will be selected as default. >>>> >>>> This patch is motivated by broken Bluetooth carkits such as the Motorolo >>>> HF850 (it claims to support eSCO, but will actually reject eSCO >>>> connections >>>> after 5 seconds) and the 2007/2008 Infiniti G35/37 (fails to route audio >>>> if a 2-EV5 packet type is negiotiated). With this patch userspace can >>>> maintain >>>> a blacklist of packet types to workaround broken remote devices such as >>>> these. >>>> >>>> Signed-off-by: Nick Pelly >>>> --- >>>> ?include/net/bluetooth/hci.h ? ? ?| ? ?7 +++- >>>> ?include/net/bluetooth/hci_core.h | ? ?7 +++- >>>> ?include/net/bluetooth/sco.h ? ? ?| ? ?4 ++- >>>> ?net/bluetooth/hci_conn.c ? ? ? ? | ? 25 +++++++++------ >>>> ?net/bluetooth/hci_event.c ? ? ? ?| ? ?6 ++- >>>> ?net/bluetooth/l2cap.c ? ? ? ? ? ?| ? ?2 +- >>>> ?net/bluetooth/sco.c ? ? ? ? ? ? ?| ? 61 >>>> +++++++++++++++++++++++++------------ >>>> ?7 files changed, 74 insertions(+), 38 deletions(-) >>>> >>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h >>>> index 4f7795f..72efc94 100644 >>>> --- a/include/net/bluetooth/hci.h >>>> +++ b/include/net/bluetooth/hci.h >>>> @@ -139,8 +139,11 @@ enum { >>>> ?#define ESCO_2EV5 ? ? ?0x0100 >>>> ?#define ESCO_3EV5 ? ? ?0x0200 >>>> >>>> -#define SCO_ESCO_MASK ?(ESCO_HV1 | ESCO_HV2 | ESCO_HV3) >>>> -#define EDR_ESCO_MASK ?(ESCO_2EV3 | ESCO_3EV3 | ESCO_2EV5 | ESCO_3EV5) >>>> +#define SCO_ESCO_MASK ?(ESCO_HV1 | ESCO_HV2 | ESCO_HV3) >>>> +#define EDR_ESCO_MASK ?(ESCO_2EV3 | ESCO_3EV3 | ESCO_2EV5 | ESCO_3EV5) >>>> + >>>> +#define ALL_SCO_PKTS ? (SCO_ESCO_MASK | EDR_ESCO_MASK) >>>> +#define ALL_ESCO_PKTS ?(SCO_ESCO_MASK | ESCO_EV3 | ESCO_EV4 | ESCO_EV5) >>>> >>>> ?/* ACL flags */ >>>> ?#define ACL_START ? ? ? ? ? ? ?0x00 >>>> diff --git a/include/net/bluetooth/hci_core.h >>>> b/include/net/bluetooth/hci_core.h >>>> index b6d36cb..cbcc5b1 100644 >>>> --- a/include/net/bluetooth/hci_core.h >>>> +++ b/include/net/bluetooth/hci_core.h >>>> @@ -326,12 +326,15 @@ void hci_acl_disconn(struct hci_conn *conn, __u8 >>>> reason); >>>> ?void hci_add_sco(struct hci_conn *conn, __u16 handle); >>>> ?void hci_setup_sync(struct hci_conn *conn, __u16 handle); >>>> >>>> -struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t >>>> *dst); >>>> +struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __u16 pkt_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); >>>> >>>> -struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t >>>> *dst, __u8 sec_level, __u8 auth_type); >>>> +struct hci_conn *hci_connect(struct hci_dev *hdev, int type, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __u16 pkt_type, bdaddr_t *dst, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __u8 sec_level, __u8 auth_type); >>>> ?int hci_conn_check_link_mode(struct hci_conn *conn); >>>> ?int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 >>>> auth_type); >>>> ?int hci_conn_change_link_key(struct hci_conn *conn); >>>> diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h >>>> index e28a2a7..924338a 100644 >>>> --- a/include/net/bluetooth/sco.h >>>> +++ b/include/net/bluetooth/sco.h >>>> @@ -37,6 +37,7 @@ >>>> ?struct sockaddr_sco { >>>> ? ? ? sa_family_t ? ? sco_family; >>>> ? ? ? bdaddr_t ? ? ? ?sco_bdaddr; >>>> + ? ? ? __u16 ? ? ? ? ? sco_pkt_type; >>>> ?}; >>>> >>>> ?/* SCO socket options */ >>>> @@ -72,7 +73,8 @@ struct sco_conn { >>>> >>>> ?struct sco_pinfo { >>>> ? ? ? struct bt_sock ?bt; >>>> - ? ? ? __u32 ? ? ? ? ? flags; >>>> + ? ? ? __u16 ? ? ? ? ? pkt_type; >>>> + >>>> ? ? ? struct sco_conn *conn; >>>> ?}; >>>> >>>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c >>>> index fa8b412..8ced057 100644 >>>> --- a/net/bluetooth/hci_conn.c >>>> +++ b/net/bluetooth/hci_conn.c >>>> @@ -196,7 +196,8 @@ static void hci_conn_idle(unsigned long arg) >>>> ? ? ? hci_conn_enter_sniff_mode(conn); >>>> ?} >>>> >>>> -struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t >>>> *dst) >>>> +struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __u16 pkt_type, bdaddr_t *dst) >>>> ?{ >>>> ? ? ? struct hci_conn *conn; >>>> >>>> @@ -221,14 +222,16 @@ struct hci_conn *hci_conn_add(struct hci_dev >>>> *hdev, int type, bdaddr_t *dst) >>>> ? ? ? ? ? ? ? conn->pkt_type = hdev->pkt_type & ACL_PTYPE_MASK; >>>> ? ? ? ? ? ? ? break; >>>> ? ? ? case SCO_LINK: >>>> + ? ? ? ? ? ? ? if (!pkt_type) >>>> + ? ? ? ? ? ? ? ? ? ? ? pkt_type = ALL_SCO_PKTS; >>>> + ? ? ? case ESCO_LINK: >>>> + ? ? ? ? ? ? ? if (!pkt_type) >>>> + ? ? ? ? ? ? ? ? ? ? ? pkt_type = ALL_ESCO_PKTS; >>>> ? ? ? ? ? ? ? if (lmp_esco_capable(hdev)) >>>> - ? ? ? ? ? ? ? ? ? ? ? conn->pkt_type = (hdev->esco_type & >>>> SCO_ESCO_MASK) | >>>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (hdev->esco_type & >>>> EDR_ESCO_MASK); >>>> + ? ? ? ? ? ? ? ? ? ? ? conn->pkt_type = pkt_type & hdev->esco_type; >>>> ? ? ? ? ? ? ? else >>>> - ? ? ? ? ? ? ? ? ? ? ? conn->pkt_type = hdev->pkt_type & >>>> SCO_PTYPE_MASK; >>>> - ? ? ? ? ? ? ? break; >>>> - ? ? ? case ESCO_LINK: >>>> - ? ? ? ? ? ? ? conn->pkt_type = hdev->esco_type & ~EDR_ESCO_MASK; >>>> + ? ? ? ? ? ? ? ? ? ? ? conn->pkt_type = (pkt_type << 5) & >>>> hdev->pkt_type & >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? SCO_PTYPE_MASK; >>>> ? ? ? ? ? ? ? break; >>>> ? ? ? } >>>> >>>> @@ -340,7 +343,9 @@ EXPORT_SYMBOL(hci_get_route); >>>> >>>> ?/* Create SCO or ACL connection. >>>> ?* Device _must_ be locked */ >>>> -struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t >>>> *dst, __u8 sec_level, __u8 auth_type) >>>> +struct hci_conn *hci_connect(struct hci_dev *hdev, int type, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __u16 pkt_type, bdaddr_t *dst, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __u8 sec_level, __u8 auth_type) >>>> ?{ >>>> ? ? ? struct hci_conn *acl; >>>> ? ? ? struct hci_conn *sco; >>>> @@ -348,7 +353,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, >>>> int type, bdaddr_t *dst, __u8 >>>> ? ? ? BT_DBG("%s dst %s", hdev->name, batostr(dst)); >>>> >>>> ? ? ? if (!(acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst))) { >>>> - ? ? ? ? ? ? ? if (!(acl = hci_conn_add(hdev, ACL_LINK, dst))) >>>> + ? ? ? ? ? ? ? if (!(acl = hci_conn_add(hdev, ACL_LINK, 0, dst))) >>>> ? ? ? ? ? ? ? ? ? ? ? return NULL; >>>> ? ? ? } >>>> >>>> @@ -364,7 +369,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, >>>> int type, bdaddr_t *dst, __u8 >>>> ? ? ? ? ? ? ? return acl; >>>> >>>> ? ? ? if (!(sco = hci_conn_hash_lookup_ba(hdev, type, dst))) { >>>> - ? ? ? ? ? ? ? if (!(sco = hci_conn_add(hdev, type, dst))) { >>>> + ? ? ? ? ? ? ? if (!(sco = hci_conn_add(hdev, type, pkt_type, dst))) { >>>> ? ? ? ? ? ? ? ? ? ? ? hci_conn_put(acl); >>>> ? ? ? ? ? ? ? ? ? ? ? return NULL; >>>> ? ? ? ? ? ? ? } >>>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c >>>> index 10edd1a..5343e0f 100644 >>>> --- a/net/bluetooth/hci_event.c >>>> +++ b/net/bluetooth/hci_event.c >>>> @@ -579,7 +579,7 @@ static inline void hci_cs_create_conn(struct hci_dev >>>> *hdev, __u8 status) >>>> ? ? ? ? ? ? ? } >>>> ? ? ? } else { >>>> ? ? ? ? ? ? ? if (!conn) { >>>> - ? ? ? ? ? ? ? ? ? ? ? conn = hci_conn_add(hdev, ACL_LINK, >>>> &cp->bdaddr); >>>> + ? ? ? ? ? ? ? ? ? ? ? conn = hci_conn_add(hdev, ACL_LINK, 0, >>>> &cp->bdaddr); >>>> ? ? ? ? ? ? ? ? ? ? ? if (conn) { >>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? conn->out = 1; >>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? conn->link_mode |= HCI_LM_MASTER; >>>> @@ -964,7 +964,9 @@ static inline void hci_conn_request_evt(struct >>>> hci_dev *hdev, struct sk_buff *sk >>>> >>>> ? ? ? ? ? ? ? conn = hci_conn_hash_lookup_ba(hdev, ev->link_type, >>>> &ev->bdaddr); >>>> ? ? ? ? ? ? ? if (!conn) { >>>> - ? ? ? ? ? ? ? ? ? ? ? if (!(conn = hci_conn_add(hdev, ev->link_type, >>>> &ev->bdaddr))) { >>>> + ? ? ? ? ? ? ? ? ? ? ? /* pkt_type not yet used for incoming >>>> connections */ >>>> + ? ? ? ? ? ? ? ? ? ? ? if (!(conn = hci_conn_add(hdev, ev->link_type, >>>> 0, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &ev->bdaddr))) { >>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? BT_ERR("No memmory for new connection"); >>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_dev_unlock(hdev); >>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? return; >>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c >>>> index d056886..b342f06 100644 >>>> --- a/net/bluetooth/l2cap.c >>>> +++ b/net/bluetooth/l2cap.c >>>> @@ -955,7 +955,7 @@ static int l2cap_do_connect(struct sock *sk) >>>> ? ? ? ? ? ? ? } >>>> ? ? ? } >>>> >>>> - ? ? ? hcon = hci_connect(hdev, ACL_LINK, dst, >>>> + ? ? ? hcon = hci_connect(hdev, ACL_LINK, 0, dst, >>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? l2cap_pi(sk)->sec_level, >>>> auth_type); >>>> ? ? ? if (!hcon) >>>> ? ? ? ? ? ? ? goto done; >>>> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c >>>> index 77f4153..4976ad5 100644 >>>> --- a/net/bluetooth/sco.c >>>> +++ b/net/bluetooth/sco.c >>>> @@ -176,6 +176,7 @@ static int sco_connect(struct sock *sk) >>>> ?{ >>>> ? ? ? bdaddr_t *src = &bt_sk(sk)->src; >>>> ? ? ? bdaddr_t *dst = &bt_sk(sk)->dst; >>>> + ? ? ? __u16 pkt_type = sco_pi(sk)->pkt_type; >>>> ? ? ? struct sco_conn *conn; >>>> ? ? ? struct hci_conn *hcon; >>>> ? ? ? struct hci_dev ?*hdev; >>>> @@ -192,10 +193,14 @@ static int sco_connect(struct sock *sk) >>>> >>>> ? ? ? if (lmp_esco_capable(hdev) && !disable_esco) >>>> ? ? ? ? ? ? ? type = ESCO_LINK; >>>> - ? ? ? else >>>> + ? ? ? else { >>>> ? ? ? ? ? ? ? type = SCO_LINK; >>>> + ? ? ? ? ? ? ? pkt_type &= SCO_ESCO_MASK; >>>> + ? ? ? ? ? ? ? pkt_type |= EDR_ESCO_MASK; >>>> + ? ? ? } >>>> >>>> - ? ? ? hcon = hci_connect(hdev, type, dst, BT_SECURITY_LOW, >>>> HCI_AT_NO_BONDING); >>>> + ? ? ? hcon = hci_connect(hdev, type, pkt_type, dst, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? BT_SECURITY_LOW, >>>> HCI_AT_NO_BONDING); >>>> ? ? ? if (!hcon) >>>> ? ? ? ? ? ? ? goto done; >>>> >>>> @@ -451,18 +456,22 @@ static int sco_sock_create(struct net *net, struct >>>> socket *sock, int protocol) >>>> ? ? ? return 0; >>>> ?} >>>> >>>> -static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, >>>> int addr_len) >>>> +static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, >>>> int alen) >>>> ?{ >>>> - ? ? ? struct sockaddr_sco *sa = (struct sockaddr_sco *) addr; >>>> + ? ? ? struct sockaddr_sco sa; >>>> ? ? ? struct sock *sk = sock->sk; >>>> - ? ? ? bdaddr_t *src = &sa->sco_bdaddr; >>>> - ? ? ? int err = 0; >>>> + ? ? ? bdaddr_t *src = &sa.sco_bdaddr; >>>> + ? ? ? int len, err = 0; >>>> >>>> - ? ? ? BT_DBG("sk %p %s", sk, batostr(&sa->sco_bdaddr)); >>>> + ? ? ? BT_DBG("sk %p %s", sk, batostr(&sa.sco_bdaddr)); >>>> >>>> ? ? ? if (!addr || addr->sa_family != AF_BLUETOOTH) >>>> ? ? ? ? ? ? ? return -EINVAL; >>>> >>>> + ? ? ? memset(&sa, 0, sizeof(sa)); >>>> + ? ? ? len = min_t(unsigned int, sizeof(sa), alen); >>>> + ? ? ? memcpy(&sa, addr, len); >>>> + >>>> ? ? ? lock_sock(sk); >>>> >>>> ? ? ? if (sk->sk_state != BT_OPEN) { >>>> @@ -476,7 +485,8 @@ static int sco_sock_bind(struct socket *sock, struct >>>> sockaddr *addr, int addr_le >>>> ? ? ? ? ? ? ? err = -EADDRINUSE; >>>> ? ? ? } else { >>>> ? ? ? ? ? ? ? /* Save source address */ >>>> - ? ? ? ? ? ? ? bacpy(&bt_sk(sk)->src, &sa->sco_bdaddr); >>>> + ? ? ? ? ? ? ? bacpy(&bt_sk(sk)->src, &sa.sco_bdaddr); >>>> + ? ? ? ? ? ? ? sco_pi(sk)->pkt_type = sa.sco_pkt_type; >>>> ? ? ? ? ? ? ? sk->sk_state = BT_BOUND; >>>> ? ? ? } >>>> >>>> @@ -489,26 +499,34 @@ done: >>>> >>>> ?static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, >>>> int alen, int flags) >>>> ?{ >>>> - ? ? ? struct sockaddr_sco *sa = (struct sockaddr_sco *) addr; >>>> ? ? ? struct sock *sk = sock->sk; >>>> - ? ? ? int err = 0; >>>> - >>>> + ? ? ? struct sockaddr_sco sa; >>>> + ? ? ? int len, err = 0; >>>> >>>> ? ? ? BT_DBG("sk %p", sk); >>>> >>>> - ? ? ? if (addr->sa_family != AF_BLUETOOTH || alen < sizeof(struct >>>> sockaddr_sco)) >>>> + ? ? ? if (!addr || addr->sa_family != AF_BLUETOOTH) >>>> ? ? ? ? ? ? ? return -EINVAL; >>>> >>>> - ? ? ? if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND) >>>> - ? ? ? ? ? ? ? return -EBADFD; >>>> - >>>> - ? ? ? if (sk->sk_type != SOCK_SEQPACKET) >>>> - ? ? ? ? ? ? ? return -EINVAL; >>>> + ? ? ? memset(&sa, 0, sizeof(sa)); >>>> + ? ? ? len = min_t(unsigned int, sizeof(sa), alen); >>>> + ? ? ? memcpy(&sa, addr, len); >>>> >>>> ? ? ? lock_sock(sk); >>>> >>>> + ? ? ? if (sk->sk_type != SOCK_SEQPACKET) { >>>> + ? ? ? ? ? ? ? err = -EINVAL; >>>> + ? ? ? ? ? ? ? goto done; >>>> + ? ? ? } >>>> + >>>> + ? ? ? if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND) { >>>> + ? ? ? ? ? ? ? err = -EBADFD; >>>> + ? ? ? ? ? ? ? goto done; >>>> + ? ? ? } >>>> + >>>> ? ? ? /* Set destination address and psm */ >>>> - ? ? ? bacpy(&bt_sk(sk)->dst, &sa->sco_bdaddr); >>>> + ? ? ? bacpy(&bt_sk(sk)->dst, &sa.sco_bdaddr); >>>> + ? ? ? sco_pi(sk)->pkt_type = sa.sco_pkt_type; >>>> >>>> ? ? ? if ((err = sco_connect(sk))) >>>> ? ? ? ? ? ? ? goto done; >>>> @@ -610,10 +628,13 @@ static int sco_sock_getname(struct socket *sock, >>>> struct sockaddr *addr, int *len >>>> ? ? ? addr->sa_family = AF_BLUETOOTH; >>>> ? ? ? *len = sizeof(struct sockaddr_sco); >>>> >>>> - ? ? ? if (peer) >>>> + ? ? ? if (peer) { >>>> ? ? ? ? ? ? ? bacpy(&sa->sco_bdaddr, &bt_sk(sk)->dst); >>>> - ? ? ? else >>>> + ? ? ? ? ? ? ? sa->sco_pkt_type = sco_pi(sk)->pkt_type; >>>> + ? ? ? } else { >>>> ? ? ? ? ? ? ? bacpy(&sa->sco_bdaddr, &bt_sk(sk)->src); >>>> + ? ? ? ? ? ? ? sa->sco_pkt_type = sco_pi(sk)->pkt_type; >>>> + ? ? ? } >>>> >>>> ? ? ? return 0; >>>> ?} >>>> -- >>>> 1.6.5.3 >>>> >>>> >>> BTW i'm happy to switch the logic on the EDR bits, such that 1 is >>> always 'allow this packet type'. I stuck with the Bluetooth SIG >>> convention in this patch because the logic actually came out cleanly. >>> >>> Either way, its confusing for userspace, and we should document it >>> carefully in the patch to userspace headers. >> >> Ping. >> >> As a first step, can we get consensus on the userspace API: >> >> --- a/include/net/bluetooth/hci.h >> +++ b/include/net/bluetooth/hci.h >> @@ -139,8 +139,11 @@ enum { >> +#define ALL_SCO_PKTS ? (SCO_ESCO_MASK | EDR_ESCO_MASK) >> +#define ALL_ESCO_PKTS ?(SCO_ESCO_MASK | ESCO_EV3 | ESCO_EV4 | ESCO_EV5) >> >> --- a/include/net/bluetooth/sco.h >> +++ b/include/net/bluetooth/sco.h >> @@ -37,6 +37,7 @@ >> struct sockaddr_sco { >> ? ? ? sa_family_t ? ? sco_family; >> ? ? ? bdaddr_t ? ? ? ?sco_bdaddr; >> + ? ? ? __u16 ? ? ? ? ? sco_pkt_type; >> }; >> >> This will at least unblock my work. > > Would it be better to add new sockopt for sco socket? What advantage does that have? Putting it in struct sockaddr_sco seems to make more sense since packet types can only be selected during SCO connection establishment. They can't be changed once the socket is connected. Nick