[RFC 1/3] Bluetooth: HCI: Add support for LE PHY Update Complete event

[RFC 1/3] Bluetooth: HCI: Add support for LE PHY Update Complete event

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This handles LE PHY Update Complete event and store both tx_phy and
rx_phy into hci_conn.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 include/net/bluetooth/hci.h      |  8 ++++++++
 include/net/bluetooth/hci_core.h |  2 ++
 net/bluetooth/hci_event.c        | 27 +++++++++++++++++++++++++++
 3 files changed, 37 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 5bc1e30dedde..07b6ecedc6ce 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -2186,6 +2186,14 @@ struct hci_ev_le_direct_adv_info {
 	__s8	 rssi;
 } __packed;
 
+#define HCI_EV_LE_PHY_UPDATE_COMPLETE	0x0c
+struct hci_ev_le_phy_update_complete {
+	__u8  status;
+	__u16 handle;
+	__u8  tx_phy;
+	__u8  rx_phy;
+} __packed;
+
 #define HCI_EV_LE_EXT_ADV_REPORT    0x0d
 struct hci_ev_le_ext_adv_report {
 	__le16 	 evt_type;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b689aceb636b..faebe3859931 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -493,6 +493,8 @@ struct hci_conn {
 	__u16		le_supv_timeout;
 	__u8		le_adv_data[HCI_MAX_AD_LENGTH];
 	__u8		le_adv_data_len;
+	__u8		le_tx_phy;
+	__u8		le_rx_phy;
 	__s8		rssi;
 	__s8		tx_power;
 	__s8		max_tx_power;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 1941f120a376..6ddc4a74a5e4 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5718,6 +5718,29 @@ static void hci_le_direct_adv_report_evt(struct hci_dev *hdev,
 	hci_dev_unlock(hdev);
 }
 
+static void hci_le_phy_update_evt(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_ev_le_phy_update_complete *ev = (void *) skb->data;
+	struct hci_conn *conn;
+
+	BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
+
+	if (!ev->status)
+		return;
+
+	hci_dev_lock(hdev);
+
+	conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(ev->handle));
+	if (!conn)
+		goto unlock;
+
+	conn->le_tx_phy = ev->tx_phy;
+	conn->le_rx_phy = ev->rx_phy;
+
+unlock:
+	hci_dev_unlock(hdev);
+}
+
 static void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_le_meta *le_ev = (void *) skb->data;
@@ -5753,6 +5776,10 @@ static void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_le_direct_adv_report_evt(hdev, skb);
 		break;
 
+	case HCI_EV_LE_PHY_UPDATE_COMPLETE:
+		hci_le_phy_update_evt(hdev, skb);
+		break;
+
 	case HCI_EV_LE_EXT_ADV_REPORT:
 		hci_le_ext_adv_report_evt(hdev, skb);
 		break;
-- 
2.21.0

[RFC 2/3] Bluetooth: Add BT_PHYS socket option

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds BT_PHYS socket option which can be used to read the PHYs in
use by the underline connection.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 include/net/bluetooth/bluetooth.h | 17 ++++++++
 include/net/bluetooth/hci_core.h  |  2 +
 net/bluetooth/hci_conn.c          | 64 +++++++++++++++++++++++++++++++
 net/bluetooth/l2cap_sock.c        | 13 +++++++
 net/bluetooth/sco.c               | 13 +++++++
 5 files changed, 109 insertions(+)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index e42bb8e03c09..69c0e7eb26d9 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -121,6 +121,23 @@ struct bt_voice {
 
 #define BT_SNDMTU		12
 #define BT_RCVMTU		13
+#define BT_PHYS			14
+
+#define BT_PHY_BR_1M_1SLOT	0x00000001
+#define BT_PHY_BR_1M_3SLOT	0x00000002
+#define BT_PHY_BR_1M_5SLOT	0x00000004
+#define BT_PHY_EDR_2M_1SLOT	0x00000008
+#define BT_PHY_EDR_2M_3SLOT	0x00000010
+#define BT_PHY_EDR_2M_5SLOT	0x00000020
+#define BT_PHY_EDR_3M_1SLOT	0x00000040
+#define BT_PHY_EDR_3M_3SLOT	0x00000080
+#define BT_PHY_EDR_3M_5SLOT	0x00000100
+#define BT_PHY_LE_1M_TX		0x00000200
+#define BT_PHY_LE_1M_RX		0x00000400
+#define BT_PHY_LE_2M_TX		0x00000800
+#define BT_PHY_LE_2M_RX		0x00001000
+#define BT_PHY_LE_CODED_TX	0x00002000
+#define BT_PHY_LE_CODED_RX	0x00004000
 
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index faebe3859931..03cf3f0f22b9 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1467,6 +1467,8 @@ void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode);
 struct sk_buff *hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen,
 			     const void *param, u32 timeout);
 
+u32 hci_conn_get_phys(struct hci_conn *conn);
+
 /* ----- HCI Sockets ----- */
 void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb);
 void hci_send_to_channel(unsigned short channel, struct sk_buff *skb,
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 87691404d0c6..386e6b0bd2ab 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1725,3 +1725,67 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle)
 
 	return hchan;
 }
+
+u32 hci_conn_get_phys(struct hci_conn *conn)
+{
+	u32 phys = 0;
+
+	hci_dev_lock(conn->hdev);
+
+	switch (conn->type) {
+	case ACL_LINK:
+	case SCO_LINK:
+		phys |= BT_PHY_BR_1M_1SLOT;
+
+		if (conn->pkt_type & (HCI_DM3 | HCI_DH3))
+			phys |= BT_PHY_BR_1M_3SLOT;
+
+		if (conn->pkt_type & (HCI_DM5 | HCI_DH5))
+			phys |= BT_PHY_BR_1M_5SLOT;
+
+		if (!(conn->pkt_type & HCI_2DH1))
+			phys |= BT_PHY_EDR_2M_1SLOT;
+
+		if (!(conn->pkt_type & HCI_2DH3))
+			phys |= BT_PHY_EDR_2M_3SLOT;
+
+		if (!(conn->pkt_type & HCI_2DH5))
+			phys |= BT_PHY_EDR_2M_5SLOT;
+
+		if (!(conn->pkt_type & HCI_3DH1))
+			phys |= BT_PHY_EDR_3M_1SLOT;
+
+		if (!(conn->pkt_type & HCI_3DH3))
+			phys |= BT_PHY_EDR_3M_3SLOT;
+
+		if (!(conn->pkt_type & HCI_3DH5))
+			phys |= BT_PHY_EDR_3M_5SLOT;
+
+		break;
+
+	case LE_LINK:
+		if (conn->le_tx_phy & HCI_LE_SET_PHY_1M)
+			phys |= BT_PHY_LE_1M_TX;
+
+		if (conn->le_rx_phy & HCI_LE_SET_PHY_1M)
+			phys |= BT_PHY_LE_1M_RX;
+
+		if (conn->le_tx_phy & HCI_LE_SET_PHY_2M)
+			phys |= BT_PHY_LE_2M_TX;
+
+		if (conn->le_rx_phy & HCI_LE_SET_PHY_2M)
+			phys |= BT_PHY_LE_2M_RX;
+
+		if (conn->le_tx_phy & HCI_LE_SET_PHY_CODED)
+			phys |= BT_PHY_LE_CODED_TX;
+
+		if (conn->le_rx_phy & HCI_LE_SET_PHY_CODED)
+			phys |= BT_PHY_LE_CODED_RX;
+
+		break;
+	}
+
+	hci_dev_unlock(conn->hdev);
+
+	return phys;
+}
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index a7be8b59b3c2..fb011c6c67be 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -499,6 +499,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
 	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
 	struct bt_security sec;
 	struct bt_power pwr;
+	u32 phys;
 	int len, err = 0;
 
 	BT_DBG("sk %p", sk);
@@ -603,6 +604,18 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
 			err = -EFAULT;
 		break;
 
+	case BT_PHYS:
+		if (sk->sk_state == BT_CONNECTED) {
+			err = -EINVAL;
+			break;
+		}
+
+		phys = hci_conn_get_phys(chan->conn->hcon);
+
+		if (put_user(phys, (u32 __user *) optval))
+			err = -EFAULT;
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index b91d6b440fdf..dcd297f2acc6 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -922,6 +922,7 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
 	struct sock *sk = sock->sk;
 	int len, err = 0;
 	struct bt_voice voice;
+	u32 phys;
 
 	BT_DBG("sk %p", sk);
 
@@ -956,6 +957,18 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
 
 		break;
 
+	case BT_PHYS:
+		if (sk->sk_state == BT_CONNECTED) {
+			err = -EINVAL;
+			break;
+		}
+
+		phys = hci_conn_get_phys(sco_pi(sk)->conn->hcon);
+
+		if (put_user(phys, (u32 __user *) optval))
+			err = -EFAULT;
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
-- 
2.21.0

Re: [RFC 2/3] Bluetooth: Add BT_PHYS socket option

[Marcel Holtmann]

Hi Luiz,

> This adds BT_PHYS socket option which can be used to read the PHYs in
> use by the underline connection.
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> include/net/bluetooth/bluetooth.h | 17 ++++++++
> include/net/bluetooth/hci_core.h  |  2 +
> net/bluetooth/hci_conn.c          | 64 +++++++++++++++++++++++++++++++
> net/bluetooth/l2cap_sock.c        | 13 +++++++
> net/bluetooth/sco.c               | 13 +++++++
> 5 files changed, 109 insertions(+)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index e42bb8e03c09..69c0e7eb26d9 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -121,6 +121,23 @@ struct bt_voice {
> 
> #define BT_SNDMTU		12
> #define BT_RCVMTU		13
> +#define BT_PHYS			14
> +
> +#define BT_PHY_BR_1M_1SLOT	0x00000001
> +#define BT_PHY_BR_1M_3SLOT	0x00000002
> +#define BT_PHY_BR_1M_5SLOT	0x00000004
> +#define BT_PHY_EDR_2M_1SLOT	0x00000008
> +#define BT_PHY_EDR_2M_3SLOT	0x00000010
> +#define BT_PHY_EDR_2M_5SLOT	0x00000020
> +#define BT_PHY_EDR_3M_1SLOT	0x00000040
> +#define BT_PHY_EDR_3M_3SLOT	0x00000080
> +#define BT_PHY_EDR_3M_5SLOT	0x00000100
> +#define BT_PHY_LE_1M_TX		0x00000200
> +#define BT_PHY_LE_1M_RX		0x00000400
> +#define BT_PHY_LE_2M_TX		0x00000800
> +#define BT_PHY_LE_2M_RX		0x00001000
> +#define BT_PHY_LE_CODED_TX	0x00002000
> +#define BT_PHY_LE_CODED_RX	0x00004000
> 
> __printf(1, 2)
> void bt_info(const char *fmt, ...);
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index faebe3859931..03cf3f0f22b9 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1467,6 +1467,8 @@ void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode);
> struct sk_buff *hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen,
> 			     const void *param, u32 timeout);
> 
> +u32 hci_conn_get_phys(struct hci_conn *conn);
> +
> /* ----- HCI Sockets ----- */
> void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb);
> void hci_send_to_channel(unsigned short channel, struct sk_buff *skb,
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 87691404d0c6..386e6b0bd2ab 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -1725,3 +1725,67 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle)
> 
> 	return hchan;
> }
> +
> +u32 hci_conn_get_phys(struct hci_conn *conn)
> +{
> +	u32 phys = 0;
> +
> +	hci_dev_lock(conn->hdev);
> +
> +	switch (conn->type) {
> +	case ACL_LINK:
> +	case SCO_LINK:
> +		phys |= BT_PHY_BR_1M_1SLOT;
> +
> +		if (conn->pkt_type & (HCI_DM3 | HCI_DH3))
> +			phys |= BT_PHY_BR_1M_3SLOT;
> +
> +		if (conn->pkt_type & (HCI_DM5 | HCI_DH5))
> +			phys |= BT_PHY_BR_1M_5SLOT;
> +
> +		if (!(conn->pkt_type & HCI_2DH1))
> +			phys |= BT_PHY_EDR_2M_1SLOT;
> +
> +		if (!(conn->pkt_type & HCI_2DH3))
> +			phys |= BT_PHY_EDR_2M_3SLOT;
> +
> +		if (!(conn->pkt_type & HCI_2DH5))
> +			phys |= BT_PHY_EDR_2M_5SLOT;
> +
> +		if (!(conn->pkt_type & HCI_3DH1))
> +			phys |= BT_PHY_EDR_3M_1SLOT;
> +
> +		if (!(conn->pkt_type & HCI_3DH3))
> +			phys |= BT_PHY_EDR_3M_3SLOT;
> +
> +		if (!(conn->pkt_type & HCI_3DH5))
> +			phys |= BT_PHY_EDR_3M_5SLOT;
> +
> +		break;

Actually ACL packet types and eSCO packet types are different. You need to split this into packet helpers for L2CAP BR/EDR and LE and SCO/eSCO packet types.

I hope we actually do this all correctly to track the packets types. Especially since the host just sets the allowed packet types and the controller can choose whatever type they want.

Regards

Marcel

Re: [RFC 2/3] Bluetooth: Add BT_PHYS socket option

[Luiz Augusto von Dentz]

Hi Marcel,

On Sat, Jan 4, 2020 at 1:56 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> > This adds BT_PHYS socket option which can be used to read the PHYs in
> > use by the underline connection.
> >
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> > include/net/bluetooth/bluetooth.h | 17 ++++++++
> > include/net/bluetooth/hci_core.h  |  2 +
> > net/bluetooth/hci_conn.c          | 64 +++++++++++++++++++++++++++++++
> > net/bluetooth/l2cap_sock.c        | 13 +++++++
> > net/bluetooth/sco.c               | 13 +++++++
> > 5 files changed, 109 insertions(+)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > index e42bb8e03c09..69c0e7eb26d9 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -121,6 +121,23 @@ struct bt_voice {
> >
> > #define BT_SNDMTU             12
> > #define BT_RCVMTU             13
> > +#define BT_PHYS                      14
> > +
> > +#define BT_PHY_BR_1M_1SLOT   0x00000001
> > +#define BT_PHY_BR_1M_3SLOT   0x00000002
> > +#define BT_PHY_BR_1M_5SLOT   0x00000004
> > +#define BT_PHY_EDR_2M_1SLOT  0x00000008
> > +#define BT_PHY_EDR_2M_3SLOT  0x00000010
> > +#define BT_PHY_EDR_2M_5SLOT  0x00000020
> > +#define BT_PHY_EDR_3M_1SLOT  0x00000040
> > +#define BT_PHY_EDR_3M_3SLOT  0x00000080
> > +#define BT_PHY_EDR_3M_5SLOT  0x00000100
> > +#define BT_PHY_LE_1M_TX              0x00000200
> > +#define BT_PHY_LE_1M_RX              0x00000400
> > +#define BT_PHY_LE_2M_TX              0x00000800
> > +#define BT_PHY_LE_2M_RX              0x00001000
> > +#define BT_PHY_LE_CODED_TX   0x00002000
> > +#define BT_PHY_LE_CODED_RX   0x00004000
> >
> > __printf(1, 2)
> > void bt_info(const char *fmt, ...);
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index faebe3859931..03cf3f0f22b9 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -1467,6 +1467,8 @@ void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode);
> > struct sk_buff *hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen,
> >                            const void *param, u32 timeout);
> >
> > +u32 hci_conn_get_phys(struct hci_conn *conn);
> > +
> > /* ----- HCI Sockets ----- */
> > void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb);
> > void hci_send_to_channel(unsigned short channel, struct sk_buff *skb,
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 87691404d0c6..386e6b0bd2ab 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -1725,3 +1725,67 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle)
> >
> >       return hchan;
> > }
> > +
> > +u32 hci_conn_get_phys(struct hci_conn *conn)
> > +{
> > +     u32 phys = 0;
> > +
> > +     hci_dev_lock(conn->hdev);
> > +
> > +     switch (conn->type) {
> > +     case ACL_LINK:
> > +     case SCO_LINK:
> > +             phys |= BT_PHY_BR_1M_1SLOT;
> > +
> > +             if (conn->pkt_type & (HCI_DM3 | HCI_DH3))
> > +                     phys |= BT_PHY_BR_1M_3SLOT;
> > +
> > +             if (conn->pkt_type & (HCI_DM5 | HCI_DH5))
> > +                     phys |= BT_PHY_BR_1M_5SLOT;
> > +
> > +             if (!(conn->pkt_type & HCI_2DH1))
> > +                     phys |= BT_PHY_EDR_2M_1SLOT;
> > +
> > +             if (!(conn->pkt_type & HCI_2DH3))
> > +                     phys |= BT_PHY_EDR_2M_3SLOT;
> > +
> > +             if (!(conn->pkt_type & HCI_2DH5))
> > +                     phys |= BT_PHY_EDR_2M_5SLOT;
> > +
> > +             if (!(conn->pkt_type & HCI_3DH1))
> > +                     phys |= BT_PHY_EDR_3M_1SLOT;
> > +
> > +             if (!(conn->pkt_type & HCI_3DH3))
> > +                     phys |= BT_PHY_EDR_3M_3SLOT;
> > +
> > +             if (!(conn->pkt_type & HCI_3DH5))
> > +                     phys |= BT_PHY_EDR_3M_5SLOT;
> > +
> > +             break;
>
> Actually ACL packet types and eSCO packet types are different. You need to split this into packet helpers for L2CAP BR/EDR and LE and SCO/eSCO packet types.

Right the SCO and ESCO actually use EV packet types, though the
question is then should we map them to new bit-fields on BT_PHY
namespace or just use the same defines (RATE+SLOT) since the socket
family should be enough to indicate if they are EV rather than DH
packets? @Pali do you need the exact packet type for sco/esco, afaik
there is not so much use of them in that case because the codecs are
normally fixed rate over HFP.

> I hope we actually do this all correctly to track the packets types. Especially since the host just sets the allowed packet types and the controller can choose whatever type they want.

At least we do seem to store them properly for ACL.

> Regards
>
> Marcel
>


-- 
Luiz Augusto von Dentz

Re: [RFC 2/3] Bluetooth: Add BT_PHYS socket option

[Marcel Holtmann]

Hi Luiz,

>>> This adds BT_PHYS socket option which can be used to read the PHYs in
>>> use by the underline connection.
>>> 
>>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>> ---
>>> include/net/bluetooth/bluetooth.h | 17 ++++++++
>>> include/net/bluetooth/hci_core.h  |  2 +
>>> net/bluetooth/hci_conn.c          | 64 +++++++++++++++++++++++++++++++
>>> net/bluetooth/l2cap_sock.c        | 13 +++++++
>>> net/bluetooth/sco.c               | 13 +++++++
>>> 5 files changed, 109 insertions(+)
>>> 
>>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
>>> index e42bb8e03c09..69c0e7eb26d9 100644
>>> --- a/include/net/bluetooth/bluetooth.h
>>> +++ b/include/net/bluetooth/bluetooth.h
>>> @@ -121,6 +121,23 @@ struct bt_voice {
>>> 
>>> #define BT_SNDMTU             12
>>> #define BT_RCVMTU             13
>>> +#define BT_PHYS                      14
>>> +
>>> +#define BT_PHY_BR_1M_1SLOT   0x00000001
>>> +#define BT_PHY_BR_1M_3SLOT   0x00000002
>>> +#define BT_PHY_BR_1M_5SLOT   0x00000004
>>> +#define BT_PHY_EDR_2M_1SLOT  0x00000008
>>> +#define BT_PHY_EDR_2M_3SLOT  0x00000010
>>> +#define BT_PHY_EDR_2M_5SLOT  0x00000020
>>> +#define BT_PHY_EDR_3M_1SLOT  0x00000040
>>> +#define BT_PHY_EDR_3M_3SLOT  0x00000080
>>> +#define BT_PHY_EDR_3M_5SLOT  0x00000100
>>> +#define BT_PHY_LE_1M_TX              0x00000200
>>> +#define BT_PHY_LE_1M_RX              0x00000400
>>> +#define BT_PHY_LE_2M_TX              0x00000800
>>> +#define BT_PHY_LE_2M_RX              0x00001000
>>> +#define BT_PHY_LE_CODED_TX   0x00002000
>>> +#define BT_PHY_LE_CODED_RX   0x00004000
>>> 
>>> __printf(1, 2)
>>> void bt_info(const char *fmt, ...);
>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>> index faebe3859931..03cf3f0f22b9 100644
>>> --- a/include/net/bluetooth/hci_core.h
>>> +++ b/include/net/bluetooth/hci_core.h
>>> @@ -1467,6 +1467,8 @@ void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode);
>>> struct sk_buff *hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen,
>>>                           const void *param, u32 timeout);
>>> 
>>> +u32 hci_conn_get_phys(struct hci_conn *conn);
>>> +
>>> /* ----- HCI Sockets ----- */
>>> void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb);
>>> void hci_send_to_channel(unsigned short channel, struct sk_buff *skb,
>>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>>> index 87691404d0c6..386e6b0bd2ab 100644
>>> --- a/net/bluetooth/hci_conn.c
>>> +++ b/net/bluetooth/hci_conn.c
>>> @@ -1725,3 +1725,67 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle)
>>> 
>>>      return hchan;
>>> }
>>> +
>>> +u32 hci_conn_get_phys(struct hci_conn *conn)
>>> +{
>>> +     u32 phys = 0;
>>> +
>>> +     hci_dev_lock(conn->hdev);
>>> +
>>> +     switch (conn->type) {
>>> +     case ACL_LINK:
>>> +     case SCO_LINK:
>>> +             phys |= BT_PHY_BR_1M_1SLOT;
>>> +
>>> +             if (conn->pkt_type & (HCI_DM3 | HCI_DH3))
>>> +                     phys |= BT_PHY_BR_1M_3SLOT;
>>> +
>>> +             if (conn->pkt_type & (HCI_DM5 | HCI_DH5))
>>> +                     phys |= BT_PHY_BR_1M_5SLOT;
>>> +
>>> +             if (!(conn->pkt_type & HCI_2DH1))
>>> +                     phys |= BT_PHY_EDR_2M_1SLOT;
>>> +
>>> +             if (!(conn->pkt_type & HCI_2DH3))
>>> +                     phys |= BT_PHY_EDR_2M_3SLOT;
>>> +
>>> +             if (!(conn->pkt_type & HCI_2DH5))
>>> +                     phys |= BT_PHY_EDR_2M_5SLOT;
>>> +
>>> +             if (!(conn->pkt_type & HCI_3DH1))
>>> +                     phys |= BT_PHY_EDR_3M_1SLOT;
>>> +
>>> +             if (!(conn->pkt_type & HCI_3DH3))
>>> +                     phys |= BT_PHY_EDR_3M_3SLOT;
>>> +
>>> +             if (!(conn->pkt_type & HCI_3DH5))
>>> +                     phys |= BT_PHY_EDR_3M_5SLOT;
>>> +
>>> +             break;
>> 
>> Actually ACL packet types and eSCO packet types are different. You need to split this into packet helpers for L2CAP BR/EDR and LE and SCO/eSCO packet types.
> 
> Right the SCO and ESCO actually use EV packet types, though the
> question is then should we map them to new bit-fields on BT_PHY
> namespace or just use the same defines (RATE+SLOT) since the socket
> family should be enough to indicate if they are EV rather than DH
> packets? @Pali do you need the exact packet type for sco/esco, afaik
> there is not so much use of them in that case because the codecs are
> normally fixed rate over HFP.

I am fine using the same definitions that we use in MGMT API. Do we plan also allowing to change packet types and also set up the initial packet types. So you can do socket(), ioctl(BT_PHYS), connect().

Regards

Marcel

Re: [RFC 2/3] Bluetooth: Add BT_PHYS socket option

[Luiz Augusto von Dentz]

Hi Marcel,

On Wed, Jan 8, 2020 at 12:52 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> >>> This adds BT_PHYS socket option which can be used to read the PHYs in
> >>> use by the underline connection.
> >>>
> >>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >>> ---
> >>> include/net/bluetooth/bluetooth.h | 17 ++++++++
> >>> include/net/bluetooth/hci_core.h  |  2 +
> >>> net/bluetooth/hci_conn.c          | 64 +++++++++++++++++++++++++++++++
> >>> net/bluetooth/l2cap_sock.c        | 13 +++++++
> >>> net/bluetooth/sco.c               | 13 +++++++
> >>> 5 files changed, 109 insertions(+)
> >>>
> >>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> >>> index e42bb8e03c09..69c0e7eb26d9 100644
> >>> --- a/include/net/bluetooth/bluetooth.h
> >>> +++ b/include/net/bluetooth/bluetooth.h
> >>> @@ -121,6 +121,23 @@ struct bt_voice {
> >>>
> >>> #define BT_SNDMTU             12
> >>> #define BT_RCVMTU             13
> >>> +#define BT_PHYS                      14
> >>> +
> >>> +#define BT_PHY_BR_1M_1SLOT   0x00000001
> >>> +#define BT_PHY_BR_1M_3SLOT   0x00000002
> >>> +#define BT_PHY_BR_1M_5SLOT   0x00000004
> >>> +#define BT_PHY_EDR_2M_1SLOT  0x00000008
> >>> +#define BT_PHY_EDR_2M_3SLOT  0x00000010
> >>> +#define BT_PHY_EDR_2M_5SLOT  0x00000020
> >>> +#define BT_PHY_EDR_3M_1SLOT  0x00000040
> >>> +#define BT_PHY_EDR_3M_3SLOT  0x00000080
> >>> +#define BT_PHY_EDR_3M_5SLOT  0x00000100
> >>> +#define BT_PHY_LE_1M_TX              0x00000200
> >>> +#define BT_PHY_LE_1M_RX              0x00000400
> >>> +#define BT_PHY_LE_2M_TX              0x00000800
> >>> +#define BT_PHY_LE_2M_RX              0x00001000
> >>> +#define BT_PHY_LE_CODED_TX   0x00002000
> >>> +#define BT_PHY_LE_CODED_RX   0x00004000
> >>>
> >>> __printf(1, 2)
> >>> void bt_info(const char *fmt, ...);
> >>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >>> index faebe3859931..03cf3f0f22b9 100644
> >>> --- a/include/net/bluetooth/hci_core.h
> >>> +++ b/include/net/bluetooth/hci_core.h
> >>> @@ -1467,6 +1467,8 @@ void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode);
> >>> struct sk_buff *hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen,
> >>>                           const void *param, u32 timeout);
> >>>
> >>> +u32 hci_conn_get_phys(struct hci_conn *conn);
> >>> +
> >>> /* ----- HCI Sockets ----- */
> >>> void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb);
> >>> void hci_send_to_channel(unsigned short channel, struct sk_buff *skb,
> >>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> >>> index 87691404d0c6..386e6b0bd2ab 100644
> >>> --- a/net/bluetooth/hci_conn.c
> >>> +++ b/net/bluetooth/hci_conn.c
> >>> @@ -1725,3 +1725,67 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle)
> >>>
> >>>      return hchan;
> >>> }
> >>> +
> >>> +u32 hci_conn_get_phys(struct hci_conn *conn)
> >>> +{
> >>> +     u32 phys = 0;
> >>> +
> >>> +     hci_dev_lock(conn->hdev);
> >>> +
> >>> +     switch (conn->type) {
> >>> +     case ACL_LINK:
> >>> +     case SCO_LINK:
> >>> +             phys |= BT_PHY_BR_1M_1SLOT;
> >>> +
> >>> +             if (conn->pkt_type & (HCI_DM3 | HCI_DH3))
> >>> +                     phys |= BT_PHY_BR_1M_3SLOT;
> >>> +
> >>> +             if (conn->pkt_type & (HCI_DM5 | HCI_DH5))
> >>> +                     phys |= BT_PHY_BR_1M_5SLOT;
> >>> +
> >>> +             if (!(conn->pkt_type & HCI_2DH1))
> >>> +                     phys |= BT_PHY_EDR_2M_1SLOT;
> >>> +
> >>> +             if (!(conn->pkt_type & HCI_2DH3))
> >>> +                     phys |= BT_PHY_EDR_2M_3SLOT;
> >>> +
> >>> +             if (!(conn->pkt_type & HCI_2DH5))
> >>> +                     phys |= BT_PHY_EDR_2M_5SLOT;
> >>> +
> >>> +             if (!(conn->pkt_type & HCI_3DH1))
> >>> +                     phys |= BT_PHY_EDR_3M_1SLOT;
> >>> +
> >>> +             if (!(conn->pkt_type & HCI_3DH3))
> >>> +                     phys |= BT_PHY_EDR_3M_3SLOT;
> >>> +
> >>> +             if (!(conn->pkt_type & HCI_3DH5))
> >>> +                     phys |= BT_PHY_EDR_3M_5SLOT;
> >>> +
> >>> +             break;
> >>
> >> Actually ACL packet types and eSCO packet types are different. You need to split this into packet helpers for L2CAP BR/EDR and LE and SCO/eSCO packet types.
> >
> > Right the SCO and ESCO actually use EV packet types, though the
> > question is then should we map them to new bit-fields on BT_PHY
> > namespace or just use the same defines (RATE+SLOT) since the socket
> > family should be enough to indicate if they are EV rather than DH
> > packets? @Pali do you need the exact packet type for sco/esco, afaik
> > there is not so much use of them in that case because the codecs are
> > normally fixed rate over HFP.
>
> I am fine using the same definitions that we use in MGMT API. Do we plan also allowing to change packet types and also set up the initial packet types. So you can do socket(), ioctl(BT_PHYS), connect().

I guess we could enable BT_PHYS on setsocketopt, though we would need
to have some logic to aggregate in case there are multiple sockets
requesting different things, we would have to decide on the likes of
latency over bandwidth or vice versa, with this we could in theory
allow overwriting the ISO packets types instead of always using the
defaults from the spec (which some people claims are not the great),
for ACL Im not sure it would make much of a difference since we allow
the controller to use any of the packets it would support, for LE
though it might be useful to allow user to use 1M if 2M doesn't work
for some reason.

-- 
Luiz Augusto von Dentz

[RFC 3/3] Bluetooth: Auto tune if input MTU is set to 0

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This enables the code to set the input MTU using the underline link
packet types when set to 0, previously this would likely be rejected by
the remote peer since it would be bellow the minimal of 48 for BR/EDR
or 23 for LE, that way it shall be safe to use 0 without causing any
side effects.

This is convenient for the likes of A2DP transport, see:

https://habr.com/en/post/456182/

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/l2cap_core.c | 54 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index a845786258a0..1bca608e0170 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1289,6 +1289,9 @@ static void l2cap_le_connect(struct l2cap_chan *chan)
 	if (test_and_set_bit(FLAG_LE_CONN_REQ_SENT, &chan->flags))
 		return;
 
+	if (!chan->imtu)
+		chan->imtu = chan->conn->mtu;
+
 	l2cap_le_flowctl_init(chan, 0);
 
 	req.psm     = chan->psm;
@@ -3226,6 +3229,49 @@ static inline void l2cap_txwin_setup(struct l2cap_chan *chan)
 	chan->ack_win = chan->tx_win;
 }
 
+static void l2cap_mtu_auto(struct l2cap_chan *chan)
+{
+	struct hci_conn *conn = chan->conn->hcon;
+
+	chan->imtu = L2CAP_DEFAULT_MIN_MTU;
+
+	/* The 2-DH1 packet has between 2 and 56 information bytes
+	 * (including the 2-byte payload header)
+	 */
+	if (!(conn->pkt_type & HCI_2DH1))
+		chan->imtu = 54;
+
+	/* The 3-DH1 packet has between 2 and 85 information bytes
+	 * (including the 2-byte payload header)
+	 */
+	if (!(conn->pkt_type & HCI_3DH1))
+		chan->imtu = 83;
+
+	/* The 2-DH3 packet has between 2 and 369 information bytes
+	 * (including the 2-byte payload header)
+	 */
+	if (!(conn->pkt_type & HCI_2DH3))
+		chan->imtu = 367;
+
+	/* The 3-DH3 packet has between 2 and 554 information bytes
+	 * (including the 2-byte payload header)
+	 */
+	if (!(conn->pkt_type & HCI_3DH3))
+		chan->imtu = 552;
+
+	/* The 2-DH5 packet has between 2 and 681 information bytes
+	 * (including the 2-byte payload header)
+	 */
+	if (!(conn->pkt_type & HCI_2DH5))
+		chan->imtu = 679;
+
+	/* The 3-DH5 packet has between 2 and 1023 information bytes
+	 * (including the 2-byte payload header)
+	 */
+	if (!(conn->pkt_type & HCI_3DH5))
+		chan->imtu = 1021;
+}
+
 static int l2cap_build_conf_req(struct l2cap_chan *chan, void *data, size_t data_size)
 {
 	struct l2cap_conf_req *req = data;
@@ -3255,8 +3301,12 @@ static int l2cap_build_conf_req(struct l2cap_chan *chan, void *data, size_t data
 	}
 
 done:
-	if (chan->imtu != L2CAP_DEFAULT_MTU)
-		l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, endptr - ptr);
+	if (chan->imtu != L2CAP_DEFAULT_MTU) {
+		if (!chan->imtu)
+			l2cap_mtu_auto(chan);
+		l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu,
+				   endptr - ptr);
+	}
 
 	switch (chan->mode) {
 	case L2CAP_MODE_BASIC:
-- 
2.21.0

Re: [RFC 3/3] Bluetooth: Auto tune if input MTU is set to 0

[Marcel Holtmann]

Hi Luiz,

> This enables the code to set the input MTU using the underline link
> packet types when set to 0, previously this would likely be rejected by
> the remote peer since it would be bellow the minimal of 48 for BR/EDR
> or 23 for LE, that way it shall be safe to use 0 without causing any
> side effects.
> 
> This is convenient for the likes of A2DP transport, see:
> 
> https://habr.com/en/post/456182/
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> net/bluetooth/l2cap_core.c | 54 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 52 insertions(+), 2 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [RFC 1/3] Bluetooth: HCI: Add support for LE PHY Update Complete event

[Marcel Holtmann]

Hi Luiz,

> This handles LE PHY Update Complete event and store both tx_phy and
> rx_phy into hci_conn.
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> include/net/bluetooth/hci.h      |  8 ++++++++
> include/net/bluetooth/hci_core.h |  2 ++
> net/bluetooth/hci_event.c        | 27 +++++++++++++++++++++++++++
> 3 files changed, 37 insertions(+)

patch has been applied to bluetooth-next tree.

Regards

Marcel