Skip to contentSkip to conversation

Enable use of L2CAP_MODE_EXT_FLOWCTL

PATCH Superseded Luiz Augusto von Dentz 2 participants 8 messages Mar 18, 2020 – Mar 23, 2020

Subsystems (2)

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Luiz Augusto von Dentz

author maintainer
#0 lore raw

v3 cover Enable use of L2CAP_MODE_EXT_FLOWCTL

From: Luiz Augusto von Dentz <[email protected]>

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(-)
Signature · 1 line
2.21.1
author maintainer
#1 lore raw

PATCH v3 1/3 Bluetooth: L2CAP: Add get_peer_pid callback

From: Luiz Augusto von Dentz <[email protected]>

This adds a callback to read the socket pid.

---
 include/net/bluetooth/l2cap.h | 1 +
 net/bluetooth/l2cap_sock.c    | 8 ++++++++
 2 files changed, 9 insertions(+)
include/net/bluetooth/l2cap.h +1 -0
index 537aaead259f..2d7d28474d34 100644
@@ -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);
net/bluetooth/l2cap_sock.c +8 -0
index 40fb10b591bd..117ba20ea194 100644
@@ -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,
 };
 
Signature · 1 line
2.21.1
author maintainer
#2 lore raw

PATCH v3 2/3 Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections

In reply to Luiz Augusto von Dentz

From: Luiz Augusto von Dentz <[email protected]>

This uses the DEFER_SETUP flag to group channels with L2CAP_CREDIT_BASED_CONNECTION_REQ.

---
 include/net/bluetooth/l2cap.h |   4 +
 net/bluetooth/l2cap_core.c    | 136 +++++++++++++++++++++++++++++++---
 2 files changed, 130 insertions(+), 10 deletions(-)
include/net/bluetooth/l2cap.h +4 -0
index 2d7d28474d34..dada14d0622c 100644
@@ -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);
net/bluetooth/l2cap_core.c +126 -10

173 lines not on this page · raw message

Signature · 1 line
2.21.1
author maintainer
#3 lore raw

PATCH v3 3/3 Bluetooth: Add BT_MODE socket option

In reply to Luiz Augusto von Dentz

From: Luiz Augusto von Dentz <[email protected]>

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.

---
 include/net/bluetooth/bluetooth.h | 11 ++++
 net/bluetooth/l2cap_sock.c        | 96 +++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+)
include/net/bluetooth/bluetooth.h +11 -0
index 1576353a2773..34191e34bfdc 100644
@@ -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)
net/bluetooth/l2cap_sock.c +96 -0

126 lines not on this page · raw message

Signature · 1 line
2.21.1
maintainer
#4 lore raw

Re: [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option

Hi Luiz,

Quoted 19 lines

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.

---
include/net/bluetooth/bluetooth.h | 11 ++++
net/bluetooth/l2cap_sock.c | 96 +++++++++++++++++++++++++++++++

2 files changed, 107 insertions(+)
include/net/bluetooth/bluetooth.h +1 -0
index 1576353a2773..34191e34bfdc 100644
@@ -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

include/net/bluetooth/bluetooth.h struct bt_voice { · line 152

Quoted 10 lines
include/net/bluetooth/bluetooth.h +10 -0
+
+#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.

Quoted 20 lines
include/net/bluetooth/bluetooth.h +0 -0
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
 __printf(1, 2)
net/bluetooth/l2cap_sock.c +9 -0
index 117ba20ea194..f2bb376c699f 100644
@@ -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 */

net/bluetooth/l2cap_sock.c static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, · line 521

Quoted 10 lines
net/bluetooth/l2cap_sock.c +10 -0
+		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

author maintainer
#5 lore raw

Re: [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option

Hi Marcel,

Quoted 35 lines · Marcel Holtmann

Hi Luiz,

Quoted 19 lines

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.

---
include/net/bluetooth/bluetooth.h | 11 ++++
net/bluetooth/l2cap_sock.c | 96 +++++++++++++++++++++++++++++++

2 files changed, 107 insertions(+)
include/net/bluetooth/bluetooth.h +1 -0
index 1576353a2773..34191e34bfdc 100644
@@ -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

include/net/bluetooth/bluetooth.h struct bt_voice { · line 152

Quoted 10 lines
include/net/bluetooth/bluetooth.h +10 -0
+
+#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:

Quoted 7 lines
        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?

Quoted 26 lines

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

Quoted 24 lines
include/net/bluetooth/bluetooth.h +0 -0
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
 __printf(1, 2)
net/bluetooth/l2cap_sock.c +13 -0
index 117ba20ea194..f2bb376c699f 100644
@@ -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:
Quoted 8 lines

net/bluetooth/l2cap_sock.c static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, · line 521

Quoted 6 lines
net/bluetooth/l2cap_sock.c +6 -0
+             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.

Quoted 4 lines

Regards

Marcel

Signature · 1 line
Luiz Augusto von Dentz
maintainer
#6 lore raw

Re: [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option

Hi Luiz,

Quoted 38 lines
Quoted 38 lines
Quoted 19 lines

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.

---
include/net/bluetooth/bluetooth.h | 11 ++++
net/bluetooth/l2cap_sock.c | 96 +++++++++++++++++++++++++++++++

2 files changed, 107 insertions(+)
include/net/bluetooth/bluetooth.h +1 -0
index 1576353a2773..34191e34bfdc 100644
@@ -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

include/net/bluetooth/bluetooth.h struct bt_voice { · line 152

Quoted 10 lines
include/net/bluetooth/bluetooth.h +10 -0
+
+#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
Quoted 4 lines
Quoted 1 line

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.

Quoted 35 lines
Quoted 26 lines

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

Quoted 24 lines
include/net/bluetooth/bluetooth.h +0 -0
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
 __printf(1, 2)
net/bluetooth/l2cap_sock.c +13 -0
index 117ba20ea194..f2bb376c699f 100644
@@ -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:
Quoted 8 lines

net/bluetooth/l2cap_sock.c static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, · line 521

Quoted 6 lines
net/bluetooth/l2cap_sock.c +6 -0
+             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.

Quoted 5 lines

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

author maintainer
#7 lore raw

Re: [PATCH v3 3/3] Bluetooth: Add BT_MODE socket option

Hi Marcel.

Quoted 41 lines · Marcel Holtmann

Hi Luiz,

Quoted 38 lines
Quoted 38 lines
Quoted 19 lines

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.

---
include/net/bluetooth/bluetooth.h | 11 ++++
net/bluetooth/l2cap_sock.c | 96 +++++++++++++++++++++++++++++++

2 files changed, 107 insertions(+)
include/net/bluetooth/bluetooth.h +1 -0
index 1576353a2773..34191e34bfdc 100644
@@ -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

include/net/bluetooth/bluetooth.h struct bt_voice { · line 152

Quoted 10 lines
include/net/bluetooth/bluetooth.h +10 -0
+
+#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
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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.

Quoted 35 lines
Quoted 35 lines
Quoted 34 lines

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

Quoted 20 lines
include/net/bluetooth/bluetooth.h +0 -0
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
 __printf(1, 2)
net/bluetooth/l2cap_sock.c +9 -0
index 117ba20ea194..f2bb376c699f 100644
@@ -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 */

net/bluetooth/l2cap_sock.c static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, · line 521

Quoted 10 lines
net/bluetooth/l2cap_sock.c +10 -0
+             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.

Quoted 7 lines
Quoted 5 lines

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.

Quoted 4 lines

Regards

Marcel

Signature · 1 line
Luiz Augusto von Dentz