All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Enable use of L2CAP_MODE_EXT_FLOWCTL
@ 2020-03-18 21:50 Luiz Augusto von Dentz
  2020-03-18 21:50 ` [PATCH v3 1/3] Bluetooth: L2CAP: Add get_peer_pid callback Luiz Augusto von Dentz
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-03-18 21:50 UTC (permalink / raw)
  To: linux-bluetooth

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

This enables use of BT_MODE_EXT_FLOWCTL with BT_MODE socketopt, in
addition to that add support for DEFER_SETUP to be used with client
sockets so they can be grouped and sent together.

This has been tested by introducing support for L2CAP_MODE_EXT_FLOWCTL
into l2cap-tester which now produces the following trace:

Enable ecred PDUs:
#echo 1 > /sys/module/bluetooth/parameters/enable_ecred

#sudo tools/l2cap-tester -p "L2CAP Ext-Flowctl"
L2CAP Ext-Flowctl Client - Success - init
  Read Index List callback
    Status: 0x00
  New hciemu instance created
  Index Added callback
    Index: 0x0000
  Read Info callback
    Status: 0x00
    Address: 00:AA:01:00:00:00
    Version: 0x09
    Manufacturer: 0x003f
    Supported settings: 0x0001be1b
    Current settings: 0x00000200
    Class: 0x000000
    Name:
    Short name:
L2CAP Ext-Flowctl Client - Success - setup
  Powering on controller
  Controller powered on
  Client set connectable status 0x00
L2CAP Ext-Flowctl Client - Success - setup complete
L2CAP Ext-Flowctl Client - Success - run
  Connect in progress
  Successfully connected
L2CAP Ext-Flowctl Client - Success - test passed
L2CAP Ext-Flowctl Client - Success - teardown
  Index Removed callback
    Index: 0x0000
L2CAP Ext-Flowctl Client - Success - teardown complete
L2CAP Ext-Flowctl Client - Success - done

L2CAP Ext-Flowctl Client, Direct Advertising - Success - init
  Read Index List callback
    Status: 0x00
  New hciemu instance created
  Index Added callback
    Index: 0x0000
  Read Info callback
    Status: 0x00
    Address: 00:AA:01:00:00:00
    Version: 0x09
    Manufacturer: 0x003f
    Supported settings: 0x0001be1b
    Current settings: 0x00000200
    Class: 0x000000
    Name:
    Short name:
L2CAP Ext-Flowctl Client, Direct Advertising - Success - setup
  Powering on controller
  Controller powered on
  Client set connectable status 0x00
L2CAP Ext-Flowctl Client, Direct Advertising - Success - setup complete
L2CAP Ext-Flowctl Client, Direct Advertising - Success - run
  Connect in progress
  Received advertising parameters HCI command
L2CAP Ext-Flowctl Client, Direct Advertising - Success - test passed
L2CAP Ext-Flowctl Client, Direct Advertising - Success - teardown
  Index Removed callback
    Index: 0x0000
L2CAP Ext-Flowctl Client, Direct Advertising - Success - teardown complete
L2CAP Ext-Flowctl Client, Direct Advertising - Success - done

L2CAP Ext-Flowctl Client SMP - Success - init
  Read Index List callback
    Status: 0x00
  New hciemu instance created
  Index Added callback
    Index: 0x0000
  Read Info callback
    Status: 0x00
    Address: 00:AA:01:00:00:00
    Version: 0x09
    Manufacturer: 0x003f
    Supported settings: 0x0001be1b
    Current settings: 0x00000200
    Class: 0x000000
    Name:
    Short name:
L2CAP Ext-Flowctl Client SMP - Success - setup
  Powering on controller
  Controller powered on
  Client set connectable status 0x00
L2CAP Ext-Flowctl Client SMP - Success - setup complete
L2CAP Ext-Flowctl Client SMP - Success - run
  Connect in progress
  Successfully connected
L2CAP Ext-Flowctl Client SMP - Success - test passed
L2CAP Ext-Flowctl Client SMP - Success - teardown
  Index Removed callback
    Index: 0x0000
L2CAP Ext-Flowctl Client SMP - Success - teardown complete
L2CAP Ext-Flowctl Client SMP - Success - done

L2CAP Ext-Flowctl Client - Command Reject - init
  Read Index List callback
    Status: 0x00
  New hciemu instance created
  Index Added callback
    Index: 0x0000
  Read Info callback
    Status: 0x00
    Address: 00:AA:01:00:00:00
    Version: 0x09
    Manufacturer: 0x003f
    Supported settings: 0x0001be1b
    Current settings: 0x00000200
    Class: 0x000000
    Name:
    Short name:
L2CAP Ext-Flowctl Client - Command Reject - setup
  Powering on controller
  Controller powered on
  Client set connectable status 0x00
L2CAP Ext-Flowctl Client - Command Reject - setup complete
L2CAP Ext-Flowctl Client - Command Reject - run
  Connect in progress
  New connection with handle 0x002a
  Connect failed: Connection refused (111)
L2CAP Ext-Flowctl Client - Command Reject - test passed
L2CAP Ext-Flowctl Client - Command Reject - teardown
  Index Removed callback
    Index: 0x0000
L2CAP Ext-Flowctl Client - Command Reject - teardown complete
L2CAP Ext-Flowctl Client - Command Reject - done

L2CAP Ext-Flowctl Client - Open two sockets - init
  Read Index List callback
    Status: 0x00
  New hciemu instance created
  Index Added callback
    Index: 0x0000
  Read Info callback
    Status: 0x00
    Address: 00:AA:01:00:00:00
    Version: 0x09
    Manufacturer: 0x003f
    Supported settings: 0x0001be1b
    Current settings: 0x00000200
    Class: 0x000000
    Name:
    Short name:
L2CAP Ext-Flowctl Client - Open two sockets - setup
  Powering on controller
  Controller powered on
L2CAP Ext-Flowctl Client - Open two sockets - setup complete
L2CAP Ext-Flowctl Client - Open two sockets - run
  Connect in progress, sk = 19 (deferred)
  HCI Command 0x200b length 7
  HCI Command 0x200c length 2
  Connect in progress, sk = 20
  Client set connectable status 0x00
  HCI Command 0x200c length 2
  HCI Command 0x200d length 25
  HCI Command 0x2016 length 2
  Successfully connected
  Successfully connected
L2CAP Ext-Flowctl Client - Open two sockets - test passed
L2CAP Ext-Flowctl Client - Open two sockets - teardown
  Index Removed callback
    Index: 0x0000
L2CAP Ext-Flowctl Client - Open two sockets - teardown complete
L2CAP Ext-Flowctl Client - Open two sockets - done

L2CAP Ext-Flowctl Client - Open two sockets close one - init
  Read Index List callback
    Status: 0x00
  New hciemu instance created
  Index Added callback
    Index: 0x0000
  Read Info callback
    Status: 0x00
    Address: 00:AA:01:00:00:00
    Version: 0x09
    Manufacturer: 0x003f
    Supported settings: 0x0001be1b
    Current settings: 0x00000200
    Class: 0x000000
    Name:
    Short name:
L2CAP Ext-Flowctl Client - Open two sockets close one - setup
  Powering on controller
  Controller powered on
L2CAP Ext-Flowctl Client - Open two sockets close one - setup complete
L2CAP Ext-Flowctl Client - Open two sockets close one - run
  Connect in progress, sk = 19 (deferred)
  HCI Command 0x200b length 7
  HCI Command 0x200c length 2
  Connect in progress, sk = 20
  Closing first socket! -1
  Client set connectable status 0x00
  HCI Command 0x200c length 2
  HCI Command 0x200d length 25
  HCI Command 0x2016 length 2
  Successfully connected
L2CAP Ext-Flowctl Client - Open two sockets close one - test passed
L2CAP Ext-Flowctl Client - Open two sockets close one - teardown
  Index Removed callback
    Index: 0x0000
L2CAP Ext-Flowctl Client - Open two sockets close one - teardown complete
L2CAP Ext-Flowctl Client - Open two sockets close one - done


Test Summary
------------
L2CAP Ext-Flowctl Client - Success                   Passed       0.068 seconds
L2CAP Ext-Flowctl Client, Direct Advertising - Success Passed       0.023 seconds
L2CAP Ext-Flowctl Client SMP - Success               Passed       0.027 seconds
L2CAP Ext-Flowctl Client - Command Reject            Passed       0.046 seconds
L2CAP Ext-Flowctl Client - Open two sockets          Passed       0.014 seconds
L2CAP Ext-Flowctl Client - Open two sockets close one Passed       0.066 seconds
Total: 6, Passed: 6 (100.0%), Failed: 0, Not Run: 0
Overall execution time: 0.247 seconds

Luiz Augusto von Dentz (3):
  Bluetooth: L2CAP: Add get_peer_pid callback
  Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections
  Bluetooth: Add BT_MODE socket option

 include/net/bluetooth/bluetooth.h |  11 +++
 include/net/bluetooth/l2cap.h     |   5 ++
 net/bluetooth/l2cap_core.c        | 136 +++++++++++++++++++++++++++---
 net/bluetooth/l2cap_sock.c        | 104 +++++++++++++++++++++++
 4 files changed, 246 insertions(+), 10 deletions(-)

-- 
2.21.1


^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH v3 1/3] Bluetooth: L2CAP: Add get_peer_pid callback
  2020-03-18 21:50 [PATCH v3 0/3] Enable use of L2CAP_MODE_EXT_FLOWCTL Luiz Augusto von Dentz
@ 2020-03-18 21:50 ` Luiz Augusto von Dentz
  2020-03-18 21:50 ` [PATCH v3 2/3] Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections Luiz Augusto von Dentz
  2020-03-18 21:50 ` [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option Luiz Augusto von Dentz
  2 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-03-18 21:50 UTC (permalink / raw)
  To: linux-bluetooth

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

This adds a callback to read the socket pid.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 include/net/bluetooth/l2cap.h | 1 +
 net/bluetooth/l2cap_sock.c    | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 537aaead259f..2d7d28474d34 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -660,6 +660,7 @@ struct l2cap_ops {
 	void			(*suspend) (struct l2cap_chan *chan);
 	void			(*set_shutdown) (struct l2cap_chan *chan);
 	long			(*get_sndtimeo) (struct l2cap_chan *chan);
+	struct pid		*(*get_peer_pid) (struct l2cap_chan *chan);
 	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
 					       unsigned long hdr_len,
 					       unsigned long len, int nb);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 40fb10b591bd..117ba20ea194 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1504,6 +1504,13 @@ static long l2cap_sock_get_sndtimeo_cb(struct l2cap_chan *chan)
 	return sk->sk_sndtimeo;
 }
 
+static struct pid *l2cap_sock_get_peer_pid_cb(struct l2cap_chan *chan)
+{
+	struct sock *sk = chan->data;
+
+	return sk->sk_peer_pid;
+}
+
 static void l2cap_sock_suspend_cb(struct l2cap_chan *chan)
 {
 	struct sock *sk = chan->data;
@@ -1525,6 +1532,7 @@ static const struct l2cap_ops l2cap_chan_ops = {
 	.suspend		= l2cap_sock_suspend_cb,
 	.set_shutdown		= l2cap_sock_set_shutdown_cb,
 	.get_sndtimeo		= l2cap_sock_get_sndtimeo_cb,
+	.get_peer_pid		= l2cap_sock_get_peer_pid_cb,
 	.alloc_skb		= l2cap_sock_alloc_skb_cb,
 };
 
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH v3 2/3] Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections
  2020-03-18 21:50 [PATCH v3 0/3] Enable use of L2CAP_MODE_EXT_FLOWCTL Luiz Augusto von Dentz
  2020-03-18 21:50 ` [PATCH v3 1/3] Bluetooth: L2CAP: Add get_peer_pid callback Luiz Augusto von Dentz
@ 2020-03-18 21:50 ` Luiz Augusto von Dentz
  2020-03-18 21:50 ` [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option Luiz Augusto von Dentz
  2 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-03-18 21:50 UTC (permalink / raw)
  To: linux-bluetooth

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

This uses the DEFER_SETUP flag to group channels with
L2CAP_CREDIT_BASED_CONNECTION_REQ.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 include/net/bluetooth/l2cap.h |   4 +
 net/bluetooth/l2cap_core.c    | 136 +++++++++++++++++++++++++++++++---
 2 files changed, 130 insertions(+), 10 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 2d7d28474d34..dada14d0622c 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -47,6 +47,7 @@
 #define L2CAP_DEFAULT_ACC_LAT		0xFFFFFFFF
 #define L2CAP_BREDR_MAX_PAYLOAD		1019    /* 3-DH5 packet */
 #define L2CAP_LE_MIN_MTU		23
+#define L2CAP_ECRED_CONN_SCID_MAX	5
 
 #define L2CAP_DISC_TIMEOUT		msecs_to_jiffies(100)
 #define L2CAP_DISC_REJ_TIMEOUT		msecs_to_jiffies(5000)
@@ -984,6 +985,9 @@ void l2cap_chan_set_defaults(struct l2cap_chan *chan);
 int l2cap_ertm_init(struct l2cap_chan *chan);
 void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan);
 void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan);
+typedef void (*l2cap_chan_func_t)(struct l2cap_chan *chan, void *data);
+void l2cap_chan_list(struct l2cap_conn *conn, l2cap_chan_func_t func,
+		     void *data);
 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);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 5e6e35ab44dd..9f699d09d973 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -678,6 +678,29 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
 }
 EXPORT_SYMBOL_GPL(l2cap_chan_del);
 
+static void __l2cap_chan_list(struct l2cap_conn *conn, l2cap_chan_func_t func,
+			      void *data)
+{
+	struct l2cap_chan *chan;
+
+	list_for_each_entry(chan, &conn->chan_l, list) {
+		func(chan, data);
+	}
+}
+
+void l2cap_chan_list(struct l2cap_conn *conn, l2cap_chan_func_t func,
+		     void *data)
+{
+	if (!conn)
+		return;
+
+	mutex_lock(&conn->chan_lock);
+	__l2cap_chan_list(conn, func, data);
+	mutex_unlock(&conn->chan_lock);
+}
+
+EXPORT_SYMBOL_GPL(l2cap_chan_list);
+
 static void l2cap_conn_update_id_addr(struct work_struct *work)
 {
 	struct l2cap_conn *conn = container_of(work, struct l2cap_conn,
@@ -1356,29 +1379,79 @@ static void l2cap_le_connect(struct l2cap_chan *chan)
 		       sizeof(req), &req);
 }
 
-static void l2cap_ecred_connect(struct l2cap_chan *chan)
-{
-	struct l2cap_conn *conn = chan->conn;
+struct l2cap_ecred_conn_data {
 	struct {
 		struct l2cap_ecred_conn_req req;
-		__le16 scid;
+		__le16 scid[5];
 	} __packed pdu;
+	struct l2cap_chan *chan;
+	struct pid *pid;
+	int count;
+};
+
+static void l2cap_ecred_defer_connect(struct l2cap_chan *chan, void *data)
+{
+	struct l2cap_ecred_conn_data *conn = data;
+	struct pid *pid;
+
+	if (chan == conn->chan)
+		return;
+
+	if (!test_and_clear_bit(FLAG_DEFER_SETUP, &chan->flags))
+		return;
+
+	pid = chan->ops->get_peer_pid(chan);
+
+	/* Only add deferred channels with the same PID/PSM */
+	if (conn->pid != pid || chan->psm != conn->chan->psm || chan->ident ||
+	    chan->mode != L2CAP_MODE_EXT_FLOWCTL || chan->state != BT_CONNECT)
+		return;
+
+	if (test_and_set_bit(FLAG_ECRED_CONN_REQ_SENT, &chan->flags))
+		return;
+
+	l2cap_ecred_init(chan, 0);
+
+	/* Set the same ident so we can match on the rsp */
+	chan->ident = conn->chan->ident;
+
+	/* Include all channels deferred */
+	conn->pdu.scid[conn->count] = cpu_to_le16(chan->scid);
+
+	conn->count++;
+}
+
+static void l2cap_ecred_connect(struct l2cap_chan *chan)
+{
+	struct l2cap_conn *conn = chan->conn;
+	struct l2cap_ecred_conn_data data;
+
+	if (test_bit(FLAG_DEFER_SETUP, &chan->flags))
+		return;
 
 	if (test_and_set_bit(FLAG_ECRED_CONN_REQ_SENT, &chan->flags))
 		return;
 
 	l2cap_ecred_init(chan, 0);
 
-	pdu.req.psm     = chan->psm;
-	pdu.req.mtu     = cpu_to_le16(chan->imtu);
-	pdu.req.mps     = cpu_to_le16(chan->mps);
-	pdu.req.credits = cpu_to_le16(chan->rx_credits);
-	pdu.scid        = cpu_to_le16(chan->scid);
+	data.pdu.req.psm     = chan->psm;
+	data.pdu.req.mtu     = cpu_to_le16(chan->imtu);
+	data.pdu.req.mps     = cpu_to_le16(chan->mps);
+	data.pdu.req.credits = cpu_to_le16(chan->rx_credits);
+	data.pdu.scid[0]     = cpu_to_le16(chan->scid);
 
 	chan->ident = l2cap_get_ident(conn);
+	data.pid = chan->ops->get_peer_pid(chan);
+
+	data.count = 1;
+	data.chan = chan;
+	data.pid = chan->ops->get_peer_pid(chan);
+
+	__l2cap_chan_list(conn, l2cap_ecred_defer_connect, &data);
 
 	l2cap_send_cmd(conn, chan->ident, L2CAP_ECRED_CONN_REQ,
-		       sizeof(pdu), &pdu);
+		       sizeof(data.pdu.req) + data.count * sizeof(__le16),
+		       &data.pdu);
 }
 
 static void l2cap_le_start(struct l2cap_chan *chan)
@@ -7694,6 +7767,33 @@ static bool is_valid_psm(u16 psm, u8 dst_type) {
 	return ((psm & 0x0101) == 0x0001);
 }
 
+struct l2cap_chan_data {
+	struct l2cap_chan *chan;
+	struct pid *pid;
+	int count;
+};
+
+static void l2cap_chan_by_pid(struct l2cap_chan *chan, void *data)
+{
+	struct l2cap_chan_data *d = data;
+	struct pid *pid;
+
+	if (chan == d->chan)
+		return;
+
+	if (!test_bit(FLAG_DEFER_SETUP, &chan->flags))
+		return;
+
+	pid = chan->ops->get_peer_pid(chan);
+
+	/* Only count deferred channels with the same PID/PSM */
+	if (d->pid != pid || chan->psm != d->chan->psm || chan->ident ||
+	    chan->mode != L2CAP_MODE_EXT_FLOWCTL || chan->state != BT_CONNECT)
+		return;
+
+	d->count++;
+}
+
 int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 		       bdaddr_t *dst, u8 dst_type)
 {
@@ -7813,6 +7913,22 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 		goto done;
 	}
 
+	if (chan->mode == L2CAP_MODE_EXT_FLOWCTL) {
+		struct l2cap_chan_data data;
+
+		data.chan = chan;
+		data.pid = chan->ops->get_peer_pid(chan);
+		data.count = 0;
+
+		l2cap_chan_list(conn, l2cap_chan_by_pid, &data);
+		/* Check if there isn't too many channels being connected */
+		if (!(data.count < L2CAP_ECRED_CONN_SCID_MAX - 1)) {
+			hci_conn_drop(hcon);
+			err = -EPROTO;
+			goto done;
+		}
+	}
+
 	mutex_lock(&conn->chan_lock);
 	l2cap_chan_lock(chan);
 
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option
  2020-03-18 21:50 [PATCH v3 0/3] Enable use of L2CAP_MODE_EXT_FLOWCTL Luiz Augusto von Dentz
  2020-03-18 21:50 ` [PATCH v3 1/3] Bluetooth: L2CAP: Add get_peer_pid callback Luiz Augusto von Dentz
  2020-03-18 21:50 ` [PATCH v3 2/3] Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections Luiz Augusto von Dentz
@ 2020-03-18 21:50 ` Luiz Augusto von Dentz
  2020-03-20  0:58   ` Marcel Holtmann
  2 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-03-18 21:50 UTC (permalink / raw)
  To: linux-bluetooth

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

This adds BT_MODE socket option which can be used to set L2CAP modes,
including modes only supported over LE which were not supported using
the L2CAP_OPTIONS.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 include/net/bluetooth/bluetooth.h | 11 ++++
 net/bluetooth/l2cap_sock.c        | 96 +++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 1576353a2773..34191e34bfdc 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -139,6 +139,17 @@ struct bt_voice {
 #define BT_PHY_LE_CODED_TX	0x00002000
 #define BT_PHY_LE_CODED_RX	0x00004000
 
+#define BT_MODE			15
+
+#define BT_MODE_BASIC		0x00
+#define BT_MODE_RETRANS		0x01
+#define BT_MODE_FLOWCTL		0x02
+#define BT_MODE_ERTM		0x03
+#define BT_MODE_STREAMING	0x04
+#define BT_MODE_EXT_FLOWCTL	0x05
+
+#define BT_MODE_LE_FLOWCTL	0x80
+
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
 __printf(1, 2)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 117ba20ea194..f2bb376c699f 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -500,6 +500,25 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname,
 	return err;
 }
 
+static u8 l2cap_get_mode(struct l2cap_chan *chan)
+{
+	switch (chan->mode) {
+	case L2CAP_MODE_BASIC:
+	case L2CAP_MODE_RETRANS:
+	case L2CAP_MODE_FLOWCTL:
+	case L2CAP_MODE_ERTM:
+	case L2CAP_MODE_STREAMING:
+		/* Mode above are the same on both old and new sockopt */
+		return chan->mode;
+	case L2CAP_MODE_LE_FLOWCTL:
+		return BT_MODE_FLOWCTL;
+	case L2CAP_MODE_EXT_FLOWCTL:
+		return BT_MODE_EXT_FLOWCTL;
+	}
+
+	return chan->mode;
+}
+
 static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
 				 char __user *optval, int __user *optlen)
 {
@@ -508,6 +527,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
 	struct bt_security sec;
 	struct bt_power pwr;
 	u32 phys;
+	u8 mode;
 	int len, err = 0;
 
 	BT_DBG("sk %p", sk);
@@ -624,6 +644,23 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
 			err = -EFAULT;
 		break;
 
+	case BT_MODE:
+		if (!enable_ecred) {
+			err = -ENOPROTOOPT;
+			break;
+		}
+
+		if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
+			err = -EINVAL;
+			break;
+		}
+
+		mode = l2cap_get_mode(chan);
+
+		if (put_user(mode, (u8 __user *) optval))
+			err = -EFAULT;
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
@@ -763,6 +800,36 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname,
 	return err;
 }
 
+static int l2cap_set_mode(struct l2cap_chan *chan, u8 mode)
+{
+	switch (mode) {
+	case BT_MODE_BASIC:
+		clear_bit(CONF_STATE2_DEVICE, &chan->conf_state);
+		break;
+	case BT_MODE_FLOWCTL:
+		if (bdaddr_type_is_le(chan->src_type))
+			mode = L2CAP_MODE_LE_FLOWCTL;
+		break;
+	case BT_MODE_EXT_FLOWCTL:
+		/* TODO: Add support for ECRED PDUs to BR/EDR */
+		if (!bdaddr_type_is_le(chan->src_type))
+			return -EINVAL;
+		mode = L2CAP_MODE_EXT_FLOWCTL;
+		break;
+	case BT_MODE_ERTM:
+	case BT_MODE_STREAMING:
+		if (!disable_ertm || bdaddr_type_is_le(chan->src_type))
+			break;
+		/* fall through */
+	default:
+		return -EINVAL;
+	}
+
+	chan->mode = mode;
+
+	return 0;
+}
+
 static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 				 char __user *optval, unsigned int optlen)
 {
@@ -968,6 +1035,35 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 
 		break;
 
+	case BT_MODE:
+		if (!enable_ecred) {
+			err = -ENOPROTOOPT;
+			break;
+		}
+
+		if (sk->sk_state != BT_BOUND) {
+			err = -EINVAL;
+			break;
+		}
+
+		if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
+			err = -EINVAL;
+			break;
+		}
+
+		if (get_user(opt, (u8 __user *) optval)) {
+			err = -EFAULT;
+			break;
+		}
+
+		err = l2cap_set_mode(chan, opt);
+		if (err)
+			break;
+
+		BT_DBG("mode 0x%2.2x", chan->mode);
+
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option
  2020-03-18 21:50 ` [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option Luiz Augusto von Dentz
@ 2020-03-20  0:58   ` Marcel Holtmann
  2020-03-20 17:44     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 8+ messages in thread
From: Marcel Holtmann @ 2020-03-20  0:58 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

> This adds BT_MODE socket option which can be used to set L2CAP modes,
> including modes only supported over LE which were not supported using
> the L2CAP_OPTIONS.
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> include/net/bluetooth/bluetooth.h | 11 ++++
> net/bluetooth/l2cap_sock.c        | 96 +++++++++++++++++++++++++++++++
> 2 files changed, 107 insertions(+)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 1576353a2773..34191e34bfdc 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -139,6 +139,17 @@ struct bt_voice {
> #define BT_PHY_LE_CODED_TX	0x00002000
> #define BT_PHY_LE_CODED_RX	0x00004000
> 
> +#define BT_MODE			15
> +
> +#define BT_MODE_BASIC		0x00
> +#define BT_MODE_RETRANS		0x01
> +#define BT_MODE_FLOWCTL		0x02
> +#define BT_MODE_ERTM		0x03
> +#define BT_MODE_STREAMING	0x04
> +#define BT_MODE_EXT_FLOWCTL	0x05
> +
> +#define BT_MODE_LE_FLOWCTL	0x80
> +

what I would do is just this:

	BASIC		0x00
	ERTM		0x01
	STREAMING	0x02
	LE_FLOWCTL	0x03
	EXT_FLOWCTL	0x04

Trying to cling onto some old L2CAP definition from the 2.1 days is not helpful. I would really make a clean cut here.

This way we can also cleanly check the available modes per selected socket and have either setsockopt or connect fail appropriately.

> __printf(1, 2)
> void bt_info(const char *fmt, ...);
> __printf(1, 2)
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 117ba20ea194..f2bb376c699f 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -500,6 +500,25 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname,
> 	return err;
> }
> 
> +static u8 l2cap_get_mode(struct l2cap_chan *chan)
> +{
> +	switch (chan->mode) {
> +	case L2CAP_MODE_BASIC:
> +	case L2CAP_MODE_RETRANS:
> +	case L2CAP_MODE_FLOWCTL:
> +	case L2CAP_MODE_ERTM:
> +	case L2CAP_MODE_STREAMING:
> +		/* Mode above are the same on both old and new sockopt */
> +		return chan->mode;
> +	case L2CAP_MODE_LE_FLOWCTL:
> +		return BT_MODE_FLOWCTL;
> +	case L2CAP_MODE_EXT_FLOWCTL:
> +		return BT_MODE_EXT_FLOWCTL;
> +	}
> +
> +	return chan->mode;
> +}
> +

Don’t bother with this. Keep the old socket and new socket independent code. I also want to add Kconfig option later that will allow us to disable the old socket options once we have SOL_L2CAP requirement eradicated.

Regards

Marcel


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option
  2020-03-20  0:58   ` Marcel Holtmann
@ 2020-03-20 17:44     ` Luiz Augusto von Dentz
  2020-03-23 17:42       ` Marcel Holtmann
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-03-20 17:44 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth

Hi Marcel,

On Thu, Mar 19, 2020 at 5:58 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> > This adds BT_MODE socket option which can be used to set L2CAP modes,
> > including modes only supported over LE which were not supported using
> > the L2CAP_OPTIONS.
> >
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> > include/net/bluetooth/bluetooth.h | 11 ++++
> > net/bluetooth/l2cap_sock.c        | 96 +++++++++++++++++++++++++++++++
> > 2 files changed, 107 insertions(+)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > index 1576353a2773..34191e34bfdc 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -139,6 +139,17 @@ struct bt_voice {
> > #define BT_PHY_LE_CODED_TX    0x00002000
> > #define BT_PHY_LE_CODED_RX    0x00004000
> >
> > +#define BT_MODE                      15
> > +
> > +#define BT_MODE_BASIC                0x00
> > +#define BT_MODE_RETRANS              0x01
> > +#define BT_MODE_FLOWCTL              0x02
> > +#define BT_MODE_ERTM         0x03
> > +#define BT_MODE_STREAMING    0x04
> > +#define BT_MODE_EXT_FLOWCTL  0x05
> > +
> > +#define BT_MODE_LE_FLOWCTL   0x80
> > +
>
> what I would do is just this:
>
>         BASIC           0x00
>         ERTM            0x01
>         STREAMING       0x02
>         LE_FLOWCTL      0x03
>         EXT_FLOWCTL     0x04
>
> Trying to cling onto some old L2CAP definition from the 2.1 days is not helpful. I would really make a clean cut here.

Just to confirm, that means we will not going to support the old
flowctl and retransmit modes from BR/EDR with BT_MODE?

> This way we can also cleanly check the available modes per selected socket and have either setsockopt or connect fail appropriately.
>
> > __printf(1, 2)
> > void bt_info(const char *fmt, ...);
> > __printf(1, 2)
> > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > index 117ba20ea194..f2bb376c699f 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -500,6 +500,25 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname,
> >       return err;
> > }
> >
> > +static u8 l2cap_get_mode(struct l2cap_chan *chan)
> > +{
> > +     switch (chan->mode) {
> > +     case L2CAP_MODE_BASIC:
> > +     case L2CAP_MODE_RETRANS:
> > +     case L2CAP_MODE_FLOWCTL:
> > +     case L2CAP_MODE_ERTM:
> > +     case L2CAP_MODE_STREAMING:
> > +             /* Mode above are the same on both old and new sockopt */
> > +             return chan->mode;
> > +     case L2CAP_MODE_LE_FLOWCTL:
> > +             return BT_MODE_FLOWCTL;
> > +     case L2CAP_MODE_EXT_FLOWCTL:
> > +             return BT_MODE_EXT_FLOWCTL;
> > +     }
> > +
> > +     return chan->mode;
> > +}
> > +
>
> Don’t bother with this. Keep the old socket and new socket independent code. I also want to add Kconfig option later that will allow us to disable the old socket options once we have SOL_L2CAP requirement eradicated.

The reason I had the defines intermixed was that application would be
able to use the old sockopt to set it and then use BT_MODE to read it,
in which case I may need to store the actual socket mode separately
from the chan->mode and then perhaps fail if application attempt to
read it with BT_MODE if was not set using it.

> Regards
>
> Marcel
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option
  2020-03-20 17:44     ` Luiz Augusto von Dentz
@ 2020-03-23 17:42       ` Marcel Holtmann
  2020-03-23 18:39         ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 8+ messages in thread
From: Marcel Holtmann @ 2020-03-23 17:42 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

>>> This adds BT_MODE socket option which can be used to set L2CAP modes,
>>> including modes only supported over LE which were not supported using
>>> the L2CAP_OPTIONS.
>>> 
>>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>> ---
>>> include/net/bluetooth/bluetooth.h | 11 ++++
>>> net/bluetooth/l2cap_sock.c        | 96 +++++++++++++++++++++++++++++++
>>> 2 files changed, 107 insertions(+)
>>> 
>>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
>>> index 1576353a2773..34191e34bfdc 100644
>>> --- a/include/net/bluetooth/bluetooth.h
>>> +++ b/include/net/bluetooth/bluetooth.h
>>> @@ -139,6 +139,17 @@ struct bt_voice {
>>> #define BT_PHY_LE_CODED_TX    0x00002000
>>> #define BT_PHY_LE_CODED_RX    0x00004000
>>> 
>>> +#define BT_MODE                      15
>>> +
>>> +#define BT_MODE_BASIC                0x00
>>> +#define BT_MODE_RETRANS              0x01
>>> +#define BT_MODE_FLOWCTL              0x02
>>> +#define BT_MODE_ERTM         0x03
>>> +#define BT_MODE_STREAMING    0x04
>>> +#define BT_MODE_EXT_FLOWCTL  0x05
>>> +
>>> +#define BT_MODE_LE_FLOWCTL   0x80
>>> +
>> 
>> what I would do is just this:
>> 
>>        BASIC           0x00
>>        ERTM            0x01
>>        STREAMING       0x02
>>        LE_FLOWCTL      0x03
>>        EXT_FLOWCTL     0x04
>> 
>> Trying to cling onto some old L2CAP definition from the 2.1 days is not helpful. I would really make a clean cut here.
> 
> Just to confirm, that means we will not going to support the old
> flowctl and retransmit modes from BR/EDR with BT_MODE?

we currently don’t support these two modes, nor are there any specs that require it.

>> This way we can also cleanly check the available modes per selected socket and have either setsockopt or connect fail appropriately.
>> 
>>> __printf(1, 2)
>>> void bt_info(const char *fmt, ...);
>>> __printf(1, 2)
>>> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
>>> index 117ba20ea194..f2bb376c699f 100644
>>> --- a/net/bluetooth/l2cap_sock.c
>>> +++ b/net/bluetooth/l2cap_sock.c
>>> @@ -500,6 +500,25 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname,
>>>      return err;
>>> }
>>> 
>>> +static u8 l2cap_get_mode(struct l2cap_chan *chan)
>>> +{
>>> +     switch (chan->mode) {
>>> +     case L2CAP_MODE_BASIC:
>>> +     case L2CAP_MODE_RETRANS:
>>> +     case L2CAP_MODE_FLOWCTL:
>>> +     case L2CAP_MODE_ERTM:
>>> +     case L2CAP_MODE_STREAMING:
>>> +             /* Mode above are the same on both old and new sockopt */
>>> +             return chan->mode;
>>> +     case L2CAP_MODE_LE_FLOWCTL:
>>> +             return BT_MODE_FLOWCTL;
>>> +     case L2CAP_MODE_EXT_FLOWCTL:
>>> +             return BT_MODE_EXT_FLOWCTL;
>>> +     }
>>> +
>>> +     return chan->mode;
>>> +}
>>> +
>> 
>> Don’t bother with this. Keep the old socket and new socket independent code. I also want to add Kconfig option later that will allow us to disable the old socket options once we have SOL_L2CAP requirement eradicated.
> 
> The reason I had the defines intermixed was that application would be
> able to use the old sockopt to set it and then use BT_MODE to read it,
> in which case I may need to store the actual socket mode separately
> from the chan->mode and then perhaps fail if application attempt to
> read it with BT_MODE if was not set using it.

If someone used L2CAP_OTIONS, then just lets fail BT_MODE read and write.

Regards

Marcel


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option
  2020-03-23 17:42       ` Marcel Holtmann
@ 2020-03-23 18:39         ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-03-23 18:39 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth

Hi Marcel.

On Mon, Mar 23, 2020 at 10:42 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> >>> This adds BT_MODE socket option which can be used to set L2CAP modes,
> >>> including modes only supported over LE which were not supported using
> >>> the L2CAP_OPTIONS.
> >>>
> >>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >>> ---
> >>> include/net/bluetooth/bluetooth.h | 11 ++++
> >>> net/bluetooth/l2cap_sock.c        | 96 +++++++++++++++++++++++++++++++
> >>> 2 files changed, 107 insertions(+)
> >>>
> >>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> >>> index 1576353a2773..34191e34bfdc 100644
> >>> --- a/include/net/bluetooth/bluetooth.h
> >>> +++ b/include/net/bluetooth/bluetooth.h
> >>> @@ -139,6 +139,17 @@ struct bt_voice {
> >>> #define BT_PHY_LE_CODED_TX    0x00002000
> >>> #define BT_PHY_LE_CODED_RX    0x00004000
> >>>
> >>> +#define BT_MODE                      15
> >>> +
> >>> +#define BT_MODE_BASIC                0x00
> >>> +#define BT_MODE_RETRANS              0x01
> >>> +#define BT_MODE_FLOWCTL              0x02
> >>> +#define BT_MODE_ERTM         0x03
> >>> +#define BT_MODE_STREAMING    0x04
> >>> +#define BT_MODE_EXT_FLOWCTL  0x05
> >>> +
> >>> +#define BT_MODE_LE_FLOWCTL   0x80
> >>> +
> >>
> >> what I would do is just this:
> >>
> >>        BASIC           0x00
> >>        ERTM            0x01
> >>        STREAMING       0x02
> >>        LE_FLOWCTL      0x03
> >>        EXT_FLOWCTL     0x04
> >>
> >> Trying to cling onto some old L2CAP definition from the 2.1 days is not helpful. I would really make a clean cut here.
> >
> > Just to confirm, that means we will not going to support the old
> > flowctl and retransmit modes from BR/EDR with BT_MODE?
>
> we currently don’t support these two modes, nor are there any specs that require it.

Right, it seems we didn't validate these modes in L2CAP_OPTION though,
I left that unchanged in the last set.

> >> This way we can also cleanly check the available modes per selected socket and have either setsockopt or connect fail appropriately.
> >>
> >>> __printf(1, 2)
> >>> void bt_info(const char *fmt, ...);
> >>> __printf(1, 2)
> >>> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> >>> index 117ba20ea194..f2bb376c699f 100644
> >>> --- a/net/bluetooth/l2cap_sock.c
> >>> +++ b/net/bluetooth/l2cap_sock.c
> >>> @@ -500,6 +500,25 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname,
> >>>      return err;
> >>> }
> >>>
> >>> +static u8 l2cap_get_mode(struct l2cap_chan *chan)
> >>> +{
> >>> +     switch (chan->mode) {
> >>> +     case L2CAP_MODE_BASIC:
> >>> +     case L2CAP_MODE_RETRANS:
> >>> +     case L2CAP_MODE_FLOWCTL:
> >>> +     case L2CAP_MODE_ERTM:
> >>> +     case L2CAP_MODE_STREAMING:
> >>> +             /* Mode above are the same on both old and new sockopt */
> >>> +             return chan->mode;
> >>> +     case L2CAP_MODE_LE_FLOWCTL:
> >>> +             return BT_MODE_FLOWCTL;
> >>> +     case L2CAP_MODE_EXT_FLOWCTL:
> >>> +             return BT_MODE_EXT_FLOWCTL;
> >>> +     }
> >>> +
> >>> +     return chan->mode;
> >>> +}
> >>> +
> >>
> >> Don’t bother with this. Keep the old socket and new socket independent code. I also want to add Kconfig option later that will allow us to disable the old socket options once we have SOL_L2CAP requirement eradicated.
> >
> > The reason I had the defines intermixed was that application would be
> > able to use the old sockopt to set it and then use BT_MODE to read it,
> > in which case I may need to store the actual socket mode separately
> > from the chan->mode and then perhaps fail if application attempt to
> > read it with BT_MODE if was not set using it.
>
> If someone used L2CAP_OTIONS, then just lets fail BT_MODE read and write.

Right, Ive made it fail with lastest change, I can probably have a
flag to make it exclusively thogh with one or another, I will send an
update shortly.

> Regards
>
> Marcel
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-23 18:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-18 21:50 [PATCH v3 0/3] Enable use of L2CAP_MODE_EXT_FLOWCTL Luiz Augusto von Dentz
2020-03-18 21:50 ` [PATCH v3 1/3] Bluetooth: L2CAP: Add get_peer_pid callback Luiz Augusto von Dentz
2020-03-18 21:50 ` [PATCH v3 2/3] Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections Luiz Augusto von Dentz
2020-03-18 21:50 ` [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option Luiz Augusto von Dentz
2020-03-20  0:58   ` Marcel Holtmann
2020-03-20 17:44     ` Luiz Augusto von Dentz
2020-03-23 17:42       ` Marcel Holtmann
2020-03-23 18:39         ` Luiz Augusto von Dentz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.