* [PATCH 0/2] Enable use of L2CAP_MODE_EXT_FLOWCTL @ 2020-03-12 22:24 Luiz Augusto von Dentz 2020-03-12 22:24 ` [PATCH 1/2] Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections Luiz Augusto von Dentz 2020-03-12 22:24 ` [PATCH 2/2] Bluetooth: Add BT_MODE socket option Luiz Augusto von Dentz 0 siblings, 2 replies; 9+ messages in thread From: Luiz Augusto von Dentz @ 2020-03-12 22:24 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> This enables use of L2CAP_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 (2): Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections Bluetooth: Add BT_MODE socket option include/net/bluetooth/bluetooth.h | 2 + include/net/bluetooth/l2cap.h | 5 ++ net/bluetooth/l2cap_core.c | 130 +++++++++++++++++++++++++++--- net/bluetooth/l2cap_sock.c | 92 ++++++++++++++++----- 4 files changed, 199 insertions(+), 30 deletions(-) -- 2.21.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections 2020-03-12 22:24 [PATCH 0/2] Enable use of L2CAP_MODE_EXT_FLOWCTL Luiz Augusto von Dentz @ 2020-03-12 22:24 ` Luiz Augusto von Dentz 2020-03-18 11:13 ` Marcel Holtmann 2020-03-12 22:24 ` [PATCH 2/2] Bluetooth: Add BT_MODE socket option Luiz Augusto von Dentz 1 sibling, 1 reply; 9+ messages in thread From: Luiz Augusto von Dentz @ 2020-03-12 22:24 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 | 5 ++ net/bluetooth/l2cap_core.c | 130 +++++++++++++++++++++++++++++++--- net/bluetooth/l2cap_sock.c | 13 ++-- 3 files changed, 133 insertions(+), 15 deletions(-) diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index 537aaead259f..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) @@ -660,6 +661,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); @@ -983,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..20c1d5f9c238 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,77 @@ 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; + + /* 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 +7765,29 @@ 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; + + if (chan == d->chan) + return; + + /* Only count deferred channels with the same PID/PSM */ + if (d->pid != chan->ops->get_peer_pid(chan) || + !test_bit(FLAG_DEFER_SETUP, &chan->flags) || + chan->psm != d->chan->psm || chan->ident || + 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 +7907,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); diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index 40fb10b591bd..e43a90e05972 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -549,11 +549,6 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, break; case BT_DEFER_SETUP: - if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) { - err = -EINVAL; - break; - } - if (put_user(test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags), (u32 __user *) optval)) err = -EFAULT; @@ -1504,6 +1499,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 +1527,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] 9+ messages in thread
* Re: [PATCH 1/2] Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections 2020-03-12 22:24 ` [PATCH 1/2] Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections Luiz Augusto von Dentz @ 2020-03-18 11:13 ` Marcel Holtmann 2020-03-18 16:58 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 9+ messages in thread From: Marcel Holtmann @ 2020-03-18 11:13 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth Hi Luiz, > 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 | 5 ++ > net/bluetooth/l2cap_core.c | 130 +++++++++++++++++++++++++++++++--- > net/bluetooth/l2cap_sock.c | 13 ++-- > 3 files changed, 133 insertions(+), 15 deletions(-) > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h > index 537aaead259f..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) > @@ -660,6 +661,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); I would move this support into a separate patch. I think that can be applied independently. > struct sk_buff *(*alloc_skb) (struct l2cap_chan *chan, > unsigned long hdr_len, > unsigned long len, int nb); > @@ -983,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..20c1d5f9c238 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,77 @@ 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; > + > + /* 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 +7765,29 @@ 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; > + > + if (chan == d->chan) > + return; > + > + /* Only count deferred channels with the same PID/PSM */ > + if (d->pid != chan->ops->get_peer_pid(chan) || > + !test_bit(FLAG_DEFER_SETUP, &chan->flags) || > + chan->psm != d->chan->psm || chan->ident || > + 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 +7907,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); > > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c > index 40fb10b591bd..e43a90e05972 100644 > --- a/net/bluetooth/l2cap_sock.c > +++ b/net/bluetooth/l2cap_sock.c > @@ -549,11 +549,6 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, > break; > > case BT_DEFER_SETUP: > - if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) { > - err = -EINVAL; > - break; > - } > - I removing this really a good idea. I think it is not really so bad to force at least BT_BOUND so that a local controller has been at least somehow selected. Just doing setsockopt(DEFER_SETUP) and then connect() seems weird. Let us force the application to at least bind the local controller for this specific usage. They can still bind with BDADDR_ANY, but that gives us a bit cleaner state handling. > if (put_user(test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags), > (u32 __user *) optval)) > err = -EFAULT; > @@ -1504,6 +1499,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 +1527,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, > }; Regards Marcel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections 2020-03-18 11:13 ` Marcel Holtmann @ 2020-03-18 16:58 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 9+ messages in thread From: Luiz Augusto von Dentz @ 2020-03-18 16:58 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth Hi Marcel, On Wed, Mar 18, 2020 at 4:13 AM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Luiz, > > > 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 | 5 ++ > > net/bluetooth/l2cap_core.c | 130 +++++++++++++++++++++++++++++++--- > > net/bluetooth/l2cap_sock.c | 13 ++-- > > 3 files changed, 133 insertions(+), 15 deletions(-) > > > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h > > index 537aaead259f..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) > > @@ -660,6 +661,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); > > I would move this support into a separate patch. I think that can be applied independently. Will do. > > struct sk_buff *(*alloc_skb) (struct l2cap_chan *chan, > > unsigned long hdr_len, > > unsigned long len, int nb); > > @@ -983,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..20c1d5f9c238 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,77 @@ 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; > > + > > + /* 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 +7765,29 @@ 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; > > + > > + if (chan == d->chan) > > + return; > > + > > + /* Only count deferred channels with the same PID/PSM */ > > + if (d->pid != chan->ops->get_peer_pid(chan) || > > + !test_bit(FLAG_DEFER_SETUP, &chan->flags) || > > + chan->psm != d->chan->psm || chan->ident || > > + 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 +7907,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); > > > > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c > > index 40fb10b591bd..e43a90e05972 100644 > > --- a/net/bluetooth/l2cap_sock.c > > +++ b/net/bluetooth/l2cap_sock.c > > @@ -549,11 +549,6 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, > > break; > > > > case BT_DEFER_SETUP: > > - if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) { > > - err = -EINVAL; > > - break; > > - } > > - > > I removing this really a good idea. I think it is not really so bad to force at least BT_BOUND so that a local controller has been at least somehow selected. Just doing setsockopt(DEFER_SETUP) and then connect() seems weird. Let us force the application to at least bind the local controller for this specific usage. They can still bind with BDADDR_ANY, but that gives us a bit cleaner state handling. Right, for some odd reason I remember this check being specific for listen only but in fact it should work with l2cap-tester as it does bind before doing BT_DEFER_SETUP. > > > if (put_user(test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags), > > (u32 __user *) optval)) > > err = -EFAULT; > > @@ -1504,6 +1499,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 +1527,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, > > }; > > Regards > > Marcel > -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] Bluetooth: Add BT_MODE socket option 2020-03-12 22:24 [PATCH 0/2] Enable use of L2CAP_MODE_EXT_FLOWCTL Luiz Augusto von Dentz 2020-03-12 22:24 ` [PATCH 1/2] Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections Luiz Augusto von Dentz @ 2020-03-12 22:24 ` Luiz Augusto von Dentz 2020-03-18 11:19 ` Marcel Holtmann 1 sibling, 1 reply; 9+ messages in thread From: Luiz Augusto von Dentz @ 2020-03-12 22:24 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 | 2 + net/bluetooth/l2cap_sock.c | 79 +++++++++++++++++++++++++------ 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h index 1576353a2773..c361ec7b06aa 100644 --- a/include/net/bluetooth/bluetooth.h +++ b/include/net/bluetooth/bluetooth.h @@ -139,6 +139,8 @@ struct bt_voice { #define BT_PHY_LE_CODED_TX 0x00002000 #define BT_PHY_LE_CODED_RX 0x00004000 +#define BT_MODE 15 + __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 e43a90e05972..7a8a199c373d 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -619,6 +619,21 @@ 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; + } + + if (put_user(chan->mode, (u8 __user *) optval)) + err = -EFAULT; + break; + default: err = -ENOPROTOOPT; break; @@ -644,6 +659,29 @@ static bool l2cap_valid_mtu(struct l2cap_chan *chan, u16 mtu) return true; } +static int l2cap_set_mode(struct l2cap_chan *chan, u8 mode) +{ + switch (chan->mode) { + case L2CAP_MODE_LE_FLOWCTL: + case L2CAP_MODE_EXT_FLOWCTL: + break; + case L2CAP_MODE_BASIC: + clear_bit(CONF_STATE2_DEVICE, &chan->conf_state); + break; + case L2CAP_MODE_ERTM: + case L2CAP_MODE_STREAMING: + if (!disable_ertm) + break; + /* fall through */ + default: + return -EINVAL; + } + + chan->mode = mode; + + return 0; +} + static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __user *optval, unsigned int optlen) { @@ -693,22 +731,9 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, break; } - chan->mode = opts.mode; - switch (chan->mode) { - case L2CAP_MODE_LE_FLOWCTL: - break; - case L2CAP_MODE_BASIC: - clear_bit(CONF_STATE2_DEVICE, &chan->conf_state); - break; - case L2CAP_MODE_ERTM: - case L2CAP_MODE_STREAMING: - if (!disable_ertm) - break; - /* fall through */ - default: - err = -EINVAL; + err = l2cap_set_mode(chan, opts.mode); + if (err) break; - } BT_DBG("mode 0x%2.2x", chan->mode); @@ -963,6 +988,30 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, break; + case BT_MODE: + if (!enable_ecred) { + err = -ENOPROTOOPT; + 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] 9+ messages in thread
* Re: [PATCH 2/2] Bluetooth: Add BT_MODE socket option 2020-03-12 22:24 ` [PATCH 2/2] Bluetooth: Add BT_MODE socket option Luiz Augusto von Dentz @ 2020-03-18 11:19 ` Marcel Holtmann 2020-03-18 17:17 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 9+ messages in thread From: Marcel Holtmann @ 2020-03-18 11:19 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 | 2 + > net/bluetooth/l2cap_sock.c | 79 +++++++++++++++++++++++++------ > 2 files changed, 66 insertions(+), 15 deletions(-) > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h > index 1576353a2773..c361ec7b06aa 100644 > --- a/include/net/bluetooth/bluetooth.h > +++ b/include/net/bluetooth/bluetooth.h > @@ -139,6 +139,8 @@ struct bt_voice { > #define BT_PHY_LE_CODED_TX 0x00002000 > #define BT_PHY_LE_CODED_RX 0x00004000 > > +#define BT_MODE 15 > + I would cleanly define the BT_MODE_xx constants here. They don’t need to match spec numbers from L2CAP actually. I would also leave the not used BR/EDR constants out of it and just add modes we do support. > __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 e43a90e05972..7a8a199c373d 100644 > --- a/net/bluetooth/l2cap_sock.c > +++ b/net/bluetooth/l2cap_sock.c > @@ -619,6 +619,21 @@ 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; > + } > + > + if (put_user(chan->mode, (u8 __user *) optval)) > + err = -EFAULT; > + break; > + > default: > err = -ENOPROTOOPT; > break; > @@ -644,6 +659,29 @@ static bool l2cap_valid_mtu(struct l2cap_chan *chan, u16 mtu) > return true; > } > > +static int l2cap_set_mode(struct l2cap_chan *chan, u8 mode) > +{ > + switch (chan->mode) { > + case L2CAP_MODE_LE_FLOWCTL: > + case L2CAP_MODE_EXT_FLOWCTL: > + break; > + case L2CAP_MODE_BASIC: > + clear_bit(CONF_STATE2_DEVICE, &chan->conf_state); > + break; > + case L2CAP_MODE_ERTM: > + case L2CAP_MODE_STREAMING: > + if (!disable_ertm) > + break; > + /* fall through */ > + default: > + return -EINVAL; > + } > + > + chan->mode = mode; > + > + return 0; > +} > + > static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, > char __user *optval, unsigned int optlen) > { > @@ -693,22 +731,9 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, > break; > } > > - chan->mode = opts.mode; > - switch (chan->mode) { > - case L2CAP_MODE_LE_FLOWCTL: > - break; > - case L2CAP_MODE_BASIC: > - clear_bit(CONF_STATE2_DEVICE, &chan->conf_state); > - break; > - case L2CAP_MODE_ERTM: > - case L2CAP_MODE_STREAMING: > - if (!disable_ertm) > - break; > - /* fall through */ > - default: > - err = -EINVAL; > + err = l2cap_set_mode(chan, opts.mode); > + if (err) > break; > - } > I would not yet try to break this out into a separate helper. I think L2CAP_OPTIONS should maybe just stay as it is. And as noted above, we define our own list of constants. > BT_DBG("mode 0x%2.2x", chan->mode); > > @@ -963,6 +988,30 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, > > break; > > + case BT_MODE: > + if (!enable_ecred) { > + err = -ENOPROTOOPT; > + break; > + } > + > + if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) { > + err = -EINVAL; > + break; > + } Especially here I would also check if the mode is valid on the choose transport. For me this would also mean we should require a bind() before allowing to set the mode. So we have at least some idea what transport is planned to be used. > + > + 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; Regards Marcel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Bluetooth: Add BT_MODE socket option 2020-03-18 11:19 ` Marcel Holtmann @ 2020-03-18 17:17 ` Luiz Augusto von Dentz 2020-03-18 19:27 ` Marcel Holtmann 0 siblings, 1 reply; 9+ messages in thread From: Luiz Augusto von Dentz @ 2020-03-18 17:17 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth Hi Marcel, On Wed, Mar 18, 2020 at 4:19 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 | 2 + > > net/bluetooth/l2cap_sock.c | 79 +++++++++++++++++++++++++------ > > 2 files changed, 66 insertions(+), 15 deletions(-) > > > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h > > index 1576353a2773..c361ec7b06aa 100644 > > --- a/include/net/bluetooth/bluetooth.h > > +++ b/include/net/bluetooth/bluetooth.h > > @@ -139,6 +139,8 @@ struct bt_voice { > > #define BT_PHY_LE_CODED_TX 0x00002000 > > #define BT_PHY_LE_CODED_RX 0x00004000 > > > > +#define BT_MODE 15 > > + > > I would cleanly define the BT_MODE_xx constants here. They don’t need to match spec numbers from L2CAP actually. I would also leave the not used BR/EDR constants out of it and just add modes we do support. Id leave the same values since it makes easier to fallback if BT_MODE is not supported e.g. BT_IO_MODE would have to declare 2 different namespaces for the new and the old sockopt. > > __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 e43a90e05972..7a8a199c373d 100644 > > --- a/net/bluetooth/l2cap_sock.c > > +++ b/net/bluetooth/l2cap_sock.c > > @@ -619,6 +619,21 @@ 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; > > + } > > + > > + if (put_user(chan->mode, (u8 __user *) optval)) > > + err = -EFAULT; > > + break; > > + > > default: > > err = -ENOPROTOOPT; > > break; > > @@ -644,6 +659,29 @@ static bool l2cap_valid_mtu(struct l2cap_chan *chan, u16 mtu) > > return true; > > } > > > > +static int l2cap_set_mode(struct l2cap_chan *chan, u8 mode) > > +{ > > + switch (chan->mode) { > > + case L2CAP_MODE_LE_FLOWCTL: > > + case L2CAP_MODE_EXT_FLOWCTL: > > + break; > > + case L2CAP_MODE_BASIC: > > + clear_bit(CONF_STATE2_DEVICE, &chan->conf_state); > > + break; > > + case L2CAP_MODE_ERTM: > > + case L2CAP_MODE_STREAMING: > > + if (!disable_ertm) > > + break; > > + /* fall through */ > > + default: > > + return -EINVAL; > > + } > > + > > + chan->mode = mode; > > + > > + return 0; > > +} > > + > > static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, > > char __user *optval, unsigned int optlen) > > { > > @@ -693,22 +731,9 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, > > break; > > } > > > > - chan->mode = opts.mode; > > - switch (chan->mode) { > > - case L2CAP_MODE_LE_FLOWCTL: > > - break; > > - case L2CAP_MODE_BASIC: > > - clear_bit(CONF_STATE2_DEVICE, &chan->conf_state); > > - break; > > - case L2CAP_MODE_ERTM: > > - case L2CAP_MODE_STREAMING: > > - if (!disable_ertm) > > - break; > > - /* fall through */ > > - default: > > - err = -EINVAL; > > + err = l2cap_set_mode(chan, opts.mode); > > + if (err) > > break; > > - } > > > > I would not yet try to break this out into a separate helper. I think L2CAP_OPTIONS should maybe just stay as it is. And as noted above, we define our own list of constants. That would complicate userspace code a little to much since that means we would have 2 different namespace for BT_IO_MODE, as mentioned about Id keep the same values for modes as in L2CAP_OPTIONS just adding new definitions names, otherwise we will need a new option for bt_io to avoid the namespaces to overlap but I rather not do that as BT_IO_MODE already maps quite well to BT_MODE. > > BT_DBG("mode 0x%2.2x", chan->mode); > > > > @@ -963,6 +988,30 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, > > > > break; > > > > + case BT_MODE: > > + if (!enable_ecred) { > > + err = -ENOPROTOOPT; > > + break; > > + } > > + > > + if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) { > > + err = -EINVAL; > > + break; > > + } > > Especially here I would also check if the mode is valid on the choose transport. For me this would also mean we should require a bind() before allowing to set the mode. So we have at least some idea what transport is planned to be used. I will make it check for BOUND state, though l2cap_set_mode does check the mode already, or you are saying that if we do use a different namespace we would have to convert, well I guess this further reinforces my argument that making the values compatible makes things a lot simpler. > > + > > + 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; > > Regards > > Marcel > -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Bluetooth: Add BT_MODE socket option 2020-03-18 17:17 ` Luiz Augusto von Dentz @ 2020-03-18 19:27 ` Marcel Holtmann 2020-03-18 22:04 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 9+ messages in thread From: Marcel Holtmann @ 2020-03-18 19:27 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 | 2 + >>> net/bluetooth/l2cap_sock.c | 79 +++++++++++++++++++++++++------ >>> 2 files changed, 66 insertions(+), 15 deletions(-) >>> >>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h >>> index 1576353a2773..c361ec7b06aa 100644 >>> --- a/include/net/bluetooth/bluetooth.h >>> +++ b/include/net/bluetooth/bluetooth.h >>> @@ -139,6 +139,8 @@ struct bt_voice { >>> #define BT_PHY_LE_CODED_TX 0x00002000 >>> #define BT_PHY_LE_CODED_RX 0x00004000 >>> >>> +#define BT_MODE 15 >>> + >> >> I would cleanly define the BT_MODE_xx constants here. They don’t need to match spec numbers from L2CAP actually. I would also leave the not used BR/EDR constants out of it and just add modes we do support. > > Id leave the same values since it makes easier to fallback if BT_MODE > is not supported e.g. BT_IO_MODE would have to declare 2 different > namespaces for the new and the old sockopt. I would actually not do that since we already made up a mode that isn’t in spec. And I don’t want to keep having to make up modes until this tiny namespace explodes. We need to accept that for BlueZ we have to define our own API and can not rely on values defined in the specification. It was fine for Bluetooth 2.1 and earlier, but it isn’t anymore. > >>> __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 e43a90e05972..7a8a199c373d 100644 >>> --- a/net/bluetooth/l2cap_sock.c >>> +++ b/net/bluetooth/l2cap_sock.c >>> @@ -619,6 +619,21 @@ 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; >>> + } >>> + >>> + if (put_user(chan->mode, (u8 __user *) optval)) >>> + err = -EFAULT; >>> + break; >>> + >>> default: >>> err = -ENOPROTOOPT; >>> break; >>> @@ -644,6 +659,29 @@ static bool l2cap_valid_mtu(struct l2cap_chan *chan, u16 mtu) >>> return true; >>> } >>> >>> +static int l2cap_set_mode(struct l2cap_chan *chan, u8 mode) >>> +{ >>> + switch (chan->mode) { >>> + case L2CAP_MODE_LE_FLOWCTL: >>> + case L2CAP_MODE_EXT_FLOWCTL: >>> + break; >>> + case L2CAP_MODE_BASIC: >>> + clear_bit(CONF_STATE2_DEVICE, &chan->conf_state); >>> + break; >>> + case L2CAP_MODE_ERTM: >>> + case L2CAP_MODE_STREAMING: >>> + if (!disable_ertm) >>> + break; >>> + /* fall through */ >>> + default: >>> + return -EINVAL; >>> + } >>> + >>> + chan->mode = mode; >>> + >>> + return 0; >>> +} >>> + >>> static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, >>> char __user *optval, unsigned int optlen) >>> { >>> @@ -693,22 +731,9 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, >>> break; >>> } >>> >>> - chan->mode = opts.mode; >>> - switch (chan->mode) { >>> - case L2CAP_MODE_LE_FLOWCTL: >>> - break; >>> - case L2CAP_MODE_BASIC: >>> - clear_bit(CONF_STATE2_DEVICE, &chan->conf_state); >>> - break; >>> - case L2CAP_MODE_ERTM: >>> - case L2CAP_MODE_STREAMING: >>> - if (!disable_ertm) >>> - break; >>> - /* fall through */ >>> - default: >>> - err = -EINVAL; >>> + err = l2cap_set_mode(chan, opts.mode); >>> + if (err) >>> break; >>> - } >>> >> >> I would not yet try to break this out into a separate helper. I think L2CAP_OPTIONS should maybe just stay as it is. And as noted above, we define our own list of constants. > > That would complicate userspace code a little to much since that means > we would have 2 different namespace for BT_IO_MODE, as mentioned about > Id keep the same values for modes as in L2CAP_OPTIONS just adding new > definitions names, otherwise we will need a new option for bt_io to > avoid the namespaces to overlap but I rather not do that as BT_IO_MODE > already maps quite well to BT_MODE. Actually we need to change BT_IO_MODE to be abstract and handle it internally. Most likely BT_IO_MODE should just follow what we do for BT_MODE and internally it should re-map it to L2CAP_OPTIONS if needed. > >>> BT_DBG("mode 0x%2.2x", chan->mode); >>> >>> @@ -963,6 +988,30 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, >>> >>> break; >>> >>> + case BT_MODE: >>> + if (!enable_ecred) { >>> + err = -ENOPROTOOPT; >>> + break; >>> + } >>> + >>> + if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) { >>> + err = -EINVAL; >>> + break; >>> + } >> >> Especially here I would also check if the mode is valid on the choose transport. For me this would also mean we should require a bind() before allowing to set the mode. So we have at least some idea what transport is planned to be used. > > I will make it check for BOUND state, though l2cap_set_mode does check > the mode already, or you are saying that if we do use a different > namespace we would have to convert, well I guess this further > reinforces my argument that making the values compatible makes things > a lot simpler. I care about the new socket options that they are clean and well defined when it comes to error conditions. Regards Marcel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Bluetooth: Add BT_MODE socket option 2020-03-18 19:27 ` Marcel Holtmann @ 2020-03-18 22:04 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 9+ messages in thread From: Luiz Augusto von Dentz @ 2020-03-18 22:04 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth Hi Marcel, On Wed, Mar 18, 2020 at 12:27 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 | 2 + > >>> net/bluetooth/l2cap_sock.c | 79 +++++++++++++++++++++++++------ > >>> 2 files changed, 66 insertions(+), 15 deletions(-) > >>> > >>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h > >>> index 1576353a2773..c361ec7b06aa 100644 > >>> --- a/include/net/bluetooth/bluetooth.h > >>> +++ b/include/net/bluetooth/bluetooth.h > >>> @@ -139,6 +139,8 @@ struct bt_voice { > >>> #define BT_PHY_LE_CODED_TX 0x00002000 > >>> #define BT_PHY_LE_CODED_RX 0x00004000 > >>> > >>> +#define BT_MODE 15 > >>> + > >> > >> I would cleanly define the BT_MODE_xx constants here. They don’t need to match spec numbers from L2CAP actually. I would also leave the not used BR/EDR constants out of it and just add modes we do support. > > > > Id leave the same values since it makes easier to fallback if BT_MODE > > is not supported e.g. BT_IO_MODE would have to declare 2 different > > namespaces for the new and the old sockopt. > > I would actually not do that since we already made up a mode that isn’t in spec. And I don’t want to keep having to make up modes until this tiny namespace explodes. We need to accept that for BlueZ we have to define our own API and can not rely on values defined in the specification. It was fine for Bluetooth 2.1 and earlier, but it isn’t anymore. > > > > >>> __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 e43a90e05972..7a8a199c373d 100644 > >>> --- a/net/bluetooth/l2cap_sock.c > >>> +++ b/net/bluetooth/l2cap_sock.c > >>> @@ -619,6 +619,21 @@ 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; > >>> + } > >>> + > >>> + if (put_user(chan->mode, (u8 __user *) optval)) > >>> + err = -EFAULT; > >>> + break; > >>> + > >>> default: > >>> err = -ENOPROTOOPT; > >>> break; > >>> @@ -644,6 +659,29 @@ static bool l2cap_valid_mtu(struct l2cap_chan *chan, u16 mtu) > >>> return true; > >>> } > >>> > >>> +static int l2cap_set_mode(struct l2cap_chan *chan, u8 mode) > >>> +{ > >>> + switch (chan->mode) { > >>> + case L2CAP_MODE_LE_FLOWCTL: > >>> + case L2CAP_MODE_EXT_FLOWCTL: > >>> + break; > >>> + case L2CAP_MODE_BASIC: > >>> + clear_bit(CONF_STATE2_DEVICE, &chan->conf_state); > >>> + break; > >>> + case L2CAP_MODE_ERTM: > >>> + case L2CAP_MODE_STREAMING: > >>> + if (!disable_ertm) > >>> + break; > >>> + /* fall through */ > >>> + default: > >>> + return -EINVAL; > >>> + } > >>> + > >>> + chan->mode = mode; > >>> + > >>> + return 0; > >>> +} > >>> + > >>> static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, > >>> char __user *optval, unsigned int optlen) > >>> { > >>> @@ -693,22 +731,9 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, > >>> break; > >>> } > >>> > >>> - chan->mode = opts.mode; > >>> - switch (chan->mode) { > >>> - case L2CAP_MODE_LE_FLOWCTL: > >>> - break; > >>> - case L2CAP_MODE_BASIC: > >>> - clear_bit(CONF_STATE2_DEVICE, &chan->conf_state); > >>> - break; > >>> - case L2CAP_MODE_ERTM: > >>> - case L2CAP_MODE_STREAMING: > >>> - if (!disable_ertm) > >>> - break; > >>> - /* fall through */ > >>> - default: > >>> - err = -EINVAL; > >>> + err = l2cap_set_mode(chan, opts.mode); > >>> + if (err) > >>> break; > >>> - } > >>> > >> > >> I would not yet try to break this out into a separate helper. I think L2CAP_OPTIONS should maybe just stay as it is. And as noted above, we define our own list of constants. > > > > That would complicate userspace code a little to much since that means > > we would have 2 different namespace for BT_IO_MODE, as mentioned about > > Id keep the same values for modes as in L2CAP_OPTIONS just adding new > > definitions names, otherwise we will need a new option for bt_io to > > avoid the namespaces to overlap but I rather not do that as BT_IO_MODE > > already maps quite well to BT_MODE. > > Actually we need to change BT_IO_MODE to be abstract and handle it internally. Most likely BT_IO_MODE should just follow what we do for BT_MODE and internally it should re-map it to L2CAP_OPTIONS if needed. > > > > >>> BT_DBG("mode 0x%2.2x", chan->mode); > >>> > >>> @@ -963,6 +988,30 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, > >>> > >>> break; > >>> > >>> + case BT_MODE: > >>> + if (!enable_ecred) { > >>> + err = -ENOPROTOOPT; > >>> + break; > >>> + } > >>> + > >>> + if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) { > >>> + err = -EINVAL; > >>> + break; > >>> + } > >> > >> Especially here I would also check if the mode is valid on the choose transport. For me this would also mean we should require a bind() before allowing to set the mode. So we have at least some idea what transport is planned to be used. > > > > I will make it check for BOUND state, though l2cap_set_mode does check > > the mode already, or you are saying that if we do use a different > > namespace we would have to convert, well I guess this further > > reinforces my argument that making the values compatible makes things > > a lot simpler. > > I care about the new socket options that they are clean and well defined when it comes to error conditions. Fair enough, Ive made the changes so BT_MODE has its own set of modes including BT_MODE_EXT_FLOWCTL, for the most part they are backward compatible but Id move the LE_FLOWCTL to just map to BT_MODE_FLOWCTL which is possible since we do require bind before setting BT_MODE we can check the address type. -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-03-18 22:04 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-12 22:24 [PATCH 0/2] Enable use of L2CAP_MODE_EXT_FLOWCTL Luiz Augusto von Dentz 2020-03-12 22:24 ` [PATCH 1/2] Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections Luiz Augusto von Dentz 2020-03-18 11:13 ` Marcel Holtmann 2020-03-18 16:58 ` Luiz Augusto von Dentz 2020-03-12 22:24 ` [PATCH 2/2] Bluetooth: Add BT_MODE socket option Luiz Augusto von Dentz 2020-03-18 11:19 ` Marcel Holtmann 2020-03-18 17:17 ` Luiz Augusto von Dentz 2020-03-18 19:27 ` Marcel Holtmann 2020-03-18 22:04 ` Luiz Augusto von Dentz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).