2024-04-02 16:41:10

by Pauli Virtanen

[permalink] [raw]
Subject: [PATCH v3] Bluetooth: add experimental BT_NO_ERRQUEUE_POLL socket option

Add experimental feature that enables a SOL_BLUETOOTH socket option to
disable POLLERR on non-empty socket error queue. Add corresponding MGMT
experimental feature UUID for it.

This is intended to allow applications disable the POLLERR wakeups due
to TX timestamps, and allow upper layers of the stack to enable and
consume TX timestamps, without needing to complicate the lower layer
POLLERR handling which is only interested in real socket errors.

It is a socket option disabled by default, as it enables a deviation
from the common net/ TX timestamping API.

Signed-off-by: Pauli Virtanen <[email protected]>
---
include/net/bluetooth/bluetooth.h | 12 +++-
net/bluetooth/af_bluetooth.c | 98 ++++++++++++++++++++++++++++++-
net/bluetooth/iso.c | 8 +--
net/bluetooth/l2cap_sock.c | 8 +--
net/bluetooth/mgmt.c | 63 +++++++++++++++++++-
net/bluetooth/sco.c | 8 +--
6 files changed, 181 insertions(+), 16 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index c95afcd9c605..0f11b436e5b3 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -242,6 +242,8 @@ struct bt_codecs {

#define BT_ISO_BASE 20

+#define BT_NO_ERRQUEUE_POLL 21
+
__printf(1, 2)
void bt_info(const char *fmt, ...);
__printf(1, 2)
@@ -389,7 +391,8 @@ struct bt_sock {
enum {
BT_SK_DEFER_SETUP,
BT_SK_SUSPEND,
- BT_SK_PKT_STATUS
+ BT_SK_PKT_STATUS,
+ BT_SK_NO_ERRQUEUE_POLL
};

struct bt_sock_list {
@@ -412,6 +415,10 @@ int bt_sock_stream_recvmsg(struct socket *sock, struct msghdr *msg,
size_t len, int flags);
__poll_t bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
+int bt_sock_setsockopt(struct socket *sock, int level, int optname,
+ sockptr_t optval, unsigned int optlen);
+int bt_sock_getsockopt(struct socket *sock, int level, int optname,
+ char __user *optval, int __user *optlen);
int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
int bt_sock_wait_ready(struct sock *sk, unsigned int msg_flags);

@@ -652,4 +659,7 @@ void mgmt_cleanup(struct sock *sk);

void bt_sock_reclassify_lock(struct sock *sk, int proto);

+int bt_no_errqueue_poll_set_enabled(bool enabled);
+bool bt_no_errqueue_poll_enabled(void);
+
#endif /* __BLUETOOTH_H */
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 67604ccec2f4..cee4814f2aae 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -68,6 +68,8 @@ static const char *const bt_slock_key_strings[BT_MAX_PROTO] = {
"slock-AF_BLUETOOTH-BTPROTO_ISO",
};

+static bool no_errqueue_poll_enabled;
+
void bt_sock_reclassify_lock(struct sock *sk, int proto)
{
BUG_ON(!sk);
@@ -500,6 +502,26 @@ static inline __poll_t bt_accept_poll(struct sock *parent)
return 0;
}

+int bt_no_errqueue_poll_set_enabled(bool enabled)
+{
+ if (enabled != no_errqueue_poll_enabled) {
+ WRITE_ONCE(no_errqueue_poll_enabled, enabled);
+ return 0;
+ }
+ return 1;
+}
+
+bool bt_no_errqueue_poll_enabled(void)
+{
+ return READ_ONCE(no_errqueue_poll_enabled);
+}
+
+static bool bt_sock_error_queue_poll(struct sock *sk)
+{
+ return !test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags) &&
+ !skb_queue_empty_lockless(&sk->sk_error_queue);
+}
+
__poll_t bt_sock_poll(struct file *file, struct socket *sock,
poll_table *wait)
{
@@ -511,7 +533,7 @@ __poll_t bt_sock_poll(struct file *file, struct socket *sock,
if (sk->sk_state == BT_LISTEN)
return bt_accept_poll(sk);

- if (sk->sk_err || !skb_queue_empty_lockless(&sk->sk_error_queue))
+ if (sk->sk_err || bt_sock_error_queue_poll(sk))
mask |= EPOLLERR |
(sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? EPOLLPRI : 0);

@@ -582,6 +604,80 @@ int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
}
EXPORT_SYMBOL(bt_sock_ioctl);

+int bt_sock_setsockopt(struct socket *sock, int level, int optname,
+ sockptr_t optval, unsigned int optlen)
+{
+ struct sock *sk = sock->sk;
+ int err = 0;
+ u32 opt;
+
+ if (level != SOL_BLUETOOTH)
+ return -ENOPROTOOPT;
+
+ lock_sock(sk);
+
+ switch (optname) {
+ case BT_NO_ERRQUEUE_POLL:
+ if (!bt_no_errqueue_poll_enabled()) {
+ err = -ENOPROTOOPT;
+ break;
+ }
+
+ if (copy_from_sockptr(&opt, optval, sizeof(opt))) {
+ err = -EFAULT;
+ break;
+ }
+
+ if (opt)
+ set_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
+ else
+ clear_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
+ break;
+
+ default:
+ err = -ENOPROTOOPT;
+ break;
+ }
+
+ release_sock(sk);
+ return err;
+}
+EXPORT_SYMBOL(bt_sock_setsockopt);
+
+int bt_sock_getsockopt(struct socket *sock, int level, int optname,
+ char __user *optval, int __user *optlen)
+{
+ struct sock *sk = sock->sk;
+ int err = 0;
+ u32 opt;
+
+ if (level != SOL_BLUETOOTH)
+ return -ENOPROTOOPT;
+
+ lock_sock(sk);
+
+ switch (optname) {
+ case BT_NO_ERRQUEUE_POLL:
+ if (!bt_no_errqueue_poll_enabled()) {
+ err = -ENOPROTOOPT;
+ break;
+ }
+
+ opt = test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
+ if (put_user(opt, (u32 __user *)optval))
+ err = -EFAULT;
+ break;
+
+ default:
+ err = -ENOPROTOOPT;
+ break;
+ }
+
+ release_sock(sk);
+ return err;
+}
+EXPORT_SYMBOL(bt_sock_getsockopt);
+
/* This function expects the sk lock to be held when called */
int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
{
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index 42b4495e019e..3c5cf7789d38 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -1602,8 +1602,8 @@ static int iso_sock_setsockopt(struct socket *sock, int level, int optname,
break;

default:
- err = -ENOPROTOOPT;
- break;
+ release_sock(sk);
+ return bt_sock_setsockopt(sock, level, optname, optval, optlen);
}

release_sock(sk);
@@ -1673,8 +1673,8 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname,
break;

default:
- err = -ENOPROTOOPT;
- break;
+ release_sock(sk);
+ return bt_sock_getsockopt(sock, level, optname, optval, optlen);
}

release_sock(sk);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 7846a068bf60..a0f7c1bcdec8 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -698,8 +698,8 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
break;

default:
- err = -ENOPROTOOPT;
- break;
+ release_sock(sk);
+ return bt_sock_getsockopt(sock, level, optname, optval, optlen);
}

release_sock(sk);
@@ -1103,8 +1103,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
break;

default:
- err = -ENOPROTOOPT;
- break;
+ release_sock(sk);
+ return bt_sock_setsockopt(sock, level, optname, optval, optlen);
}

release_sock(sk);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 32ed6e9245a3..8f62850023a0 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4339,6 +4339,12 @@ static const u8 mgmt_mesh_uuid[16] = {
0x8d, 0x4d, 0x03, 0x7a, 0xd7, 0x63, 0xe4, 0x2c,
};

+/* 69518c4c-b69f-4679-8bc1-c021b47b5733 */
+static const u8 no_errqueue_poll_uuid[16] = {
+ 0x33, 0x57, 0x7b, 0xb4, 0x21, 0xc0, 0xc1, 0x8b,
+ 0x79, 0x46, 0x9f, 0xb6, 0x4c, 0x8c, 0x51, 0x69,
+};
+
static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
void *data, u16 data_len)
{
@@ -4350,8 +4356,8 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,

bt_dev_dbg(hdev, "sock %p", sk);

- /* Enough space for 7 features */
- len = sizeof(*rp) + (sizeof(rp->features[0]) * 7);
+ /* Enough space for 8 features */
+ len = sizeof(*rp) + (sizeof(rp->features[0]) * 8);
rp = kzalloc(len, GFP_KERNEL);
if (!rp)
return -ENOMEM;
@@ -4429,6 +4435,11 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
idx++;
}

+ flags = bt_no_errqueue_poll_enabled() ? BIT(0) : 0;
+ memcpy(rp->features[idx].uuid, no_errqueue_poll_uuid, 16);
+ rp->features[idx].flags = cpu_to_le32(flags);
+ idx++;
+
rp->feature_count = cpu_to_le16(idx);

/* After reading the experimental features information, enable
@@ -4926,6 +4937,53 @@ static int set_iso_socket_func(struct sock *sk, struct hci_dev *hdev,
}
#endif

+static int set_no_errqueue_poll_func(struct sock *sk, struct hci_dev *hdev,
+ struct mgmt_cp_set_exp_feature *cp,
+ u16 data_len)
+{
+ struct mgmt_rp_set_exp_feature rp;
+ bool val, changed = false;
+ int err;
+
+ /* Command requires to use the non-controller index */
+ if (hdev)
+ return mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_EXP_FEATURE,
+ MGMT_STATUS_INVALID_INDEX);
+
+ /* Parameters are limited to a single octet */
+ if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
+ return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
+ MGMT_OP_SET_EXP_FEATURE,
+ MGMT_STATUS_INVALID_PARAMS);
+
+ /* Only boolean on/off is supported */
+ if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
+ return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
+ MGMT_OP_SET_EXP_FEATURE,
+ MGMT_STATUS_INVALID_PARAMS);
+
+ val = cp->param[0] ? true : false;
+
+ err = bt_no_errqueue_poll_set_enabled(val);
+ if (!err)
+ changed = true;
+
+ memcpy(rp.uuid, no_errqueue_poll_uuid, 16);
+ rp.flags = cpu_to_le32(val ? BIT(0) : 0);
+
+ hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
+
+ err = mgmt_cmd_complete(sk, MGMT_INDEX_NONE,
+ MGMT_OP_SET_EXP_FEATURE, 0,
+ &rp, sizeof(rp));
+
+ if (changed)
+ exp_feature_changed(hdev, no_errqueue_poll_uuid, val, sk);
+
+ return err;
+}
+
static const struct mgmt_exp_feature {
const u8 *uuid;
int (*set_func)(struct sock *sk, struct hci_dev *hdev,
@@ -4943,6 +5001,7 @@ static const struct mgmt_exp_feature {
#ifdef CONFIG_BT_LE
EXP_FEAT(iso_socket_uuid, set_iso_socket_func),
#endif
+ EXP_FEAT(no_errqueue_poll_uuid, set_no_errqueue_poll_func),

/* end with a null feature */
EXP_FEAT(NULL, NULL)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 99c2b713d826..d4697f147b5a 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -968,8 +968,8 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
break;

default:
- err = -ENOPROTOOPT;
- break;
+ release_sock(sk);
+ return bt_sock_setsockopt(sock, level, optname, optval, optlen);
}

release_sock(sk);
@@ -1212,8 +1212,8 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
break;

default:
- err = -ENOPROTOOPT;
- break;
+ release_sock(sk);
+ return bt_sock_getsockopt(sock, level, optname, optval, optlen);
}

release_sock(sk);
--
2.44.0



2024-04-02 17:33:05

by bluez.test.bot

[permalink] [raw]
Subject: RE: [v3] Bluetooth: add experimental BT_NO_ERRQUEUE_POLL socket option

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=840716

---Test result---

Test Summary:
CheckPatch PASS 1.65 seconds
GitLint PASS 0.30 seconds
SubjectPrefix PASS 0.11 seconds
BuildKernel PASS 30.13 seconds
CheckAllWarning PASS 32.94 seconds
CheckSparse WARNING 38.69 seconds
CheckSmatch FAIL 36.02 seconds
BuildKernel32 PASS 29.44 seconds
TestRunnerSetup PASS 527.55 seconds
TestRunner_l2cap-tester PASS 20.01 seconds
TestRunner_iso-tester FAIL 36.46 seconds
TestRunner_bnep-tester PASS 4.75 seconds
TestRunner_mgmt-tester FAIL 110.86 seconds
TestRunner_rfcomm-tester PASS 7.38 seconds
TestRunner_sco-tester PASS 14.92 seconds
TestRunner_ioctl-tester PASS 7.85 seconds
TestRunner_mesh-tester PASS 5.92 seconds
TestRunner_smp-tester PASS 6.89 seconds
TestRunner_userchan-tester PASS 4.98 seconds
IncrementalBuild PASS 28.46 seconds

Details
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/af_bluetooth.c:225:25: warning: context imbalance in 'bt_accept_enqueue' - different lock contexts for basic blocknet/bluetooth/sco.c: note: in included file:./include/net/bluetooth/hci_core.h:150:35: warning: array of flexible structures
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139
make[4]: *** Deleting file 'net/bluetooth/hci_core.o'
make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: net] Error 2
make[2]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o'
make[4]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bpa10x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bpa10x.o'
make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: drivers] Error 2
make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
Total: 117, Passed: 116 (99.1%), Failed: 1, Not Run: 0

Failed Test Cases
ISO Connect2 Suspend - Success Failed 4.208 seconds
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 492, Passed: 488 (99.2%), Failed: 2, Not Run: 2

Failed Test Cases
Read Exp Feature - Success Failed 0.076 seconds
Read Exp Feature - Success (Index None) Failed 0.073 seconds


---
Regards,
Linux Bluetooth

2024-04-02 18:10:43

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v3] Bluetooth: add experimental BT_NO_ERRQUEUE_POLL socket option

Hi Pauli,

On Tue, Apr 2, 2024 at 12:41 PM Pauli Virtanen <[email protected]> wrote:
>
> Add experimental feature that enables a SOL_BLUETOOTH socket option to
> disable POLLERR on non-empty socket error queue. Add corresponding MGMT
> experimental feature UUID for it.
>
> This is intended to allow applications disable the POLLERR wakeups due
> to TX timestamps, and allow upper layers of the stack to enable and
> consume TX timestamps, without needing to complicate the lower layer
> POLLERR handling which is only interested in real socket errors.
>
> It is a socket option disabled by default, as it enables a deviation
> from the common net/ TX timestamping API.
>
> Signed-off-by: Pauli Virtanen <[email protected]>
> ---
> include/net/bluetooth/bluetooth.h | 12 +++-
> net/bluetooth/af_bluetooth.c | 98 ++++++++++++++++++++++++++++++-
> net/bluetooth/iso.c | 8 +--
> net/bluetooth/l2cap_sock.c | 8 +--
> net/bluetooth/mgmt.c | 63 +++++++++++++++++++-
> net/bluetooth/sco.c | 8 +--
> 6 files changed, 181 insertions(+), 16 deletions(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index c95afcd9c605..0f11b436e5b3 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -242,6 +242,8 @@ struct bt_codecs {
>
> #define BT_ISO_BASE 20
>
> +#define BT_NO_ERRQUEUE_POLL 21

Perhaps we should revert the logic here and have it as
BT_POLL_ERRQUEUE which by default shall be considered enabled, that
said don't we have a race if SO_TXTIMESTAMP is send ahead it could
enqueue events on errqueue thus triggering the socket to wake up even
if it later sends BT_POLL_ERRQUEUE? Or that stops the wake ups
regardless?

> __printf(1, 2)
> void bt_info(const char *fmt, ...);
> __printf(1, 2)
> @@ -389,7 +391,8 @@ struct bt_sock {
> enum {
> BT_SK_DEFER_SETUP,
> BT_SK_SUSPEND,
> - BT_SK_PKT_STATUS
> + BT_SK_PKT_STATUS,
> + BT_SK_NO_ERRQUEUE_POLL
> };
>
> struct bt_sock_list {
> @@ -412,6 +415,10 @@ int bt_sock_stream_recvmsg(struct socket *sock, struct msghdr *msg,
> size_t len, int flags);
> __poll_t bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
> int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
> +int bt_sock_setsockopt(struct socket *sock, int level, int optname,
> + sockptr_t optval, unsigned int optlen);
> +int bt_sock_getsockopt(struct socket *sock, int level, int optname,
> + char __user *optval, int __user *optlen);
> int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
> int bt_sock_wait_ready(struct sock *sk, unsigned int msg_flags);
>
> @@ -652,4 +659,7 @@ void mgmt_cleanup(struct sock *sk);
>
> void bt_sock_reclassify_lock(struct sock *sk, int proto);
>
> +int bt_no_errqueue_poll_set_enabled(bool enabled);
> +bool bt_no_errqueue_poll_enabled(void);
> +
> #endif /* __BLUETOOTH_H */
> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> index 67604ccec2f4..cee4814f2aae 100644
> --- a/net/bluetooth/af_bluetooth.c
> +++ b/net/bluetooth/af_bluetooth.c
> @@ -68,6 +68,8 @@ static const char *const bt_slock_key_strings[BT_MAX_PROTO] = {
> "slock-AF_BLUETOOTH-BTPROTO_ISO",
> };
>
> +static bool no_errqueue_poll_enabled;
> +
> void bt_sock_reclassify_lock(struct sock *sk, int proto)
> {
> BUG_ON(!sk);
> @@ -500,6 +502,26 @@ static inline __poll_t bt_accept_poll(struct sock *parent)
> return 0;
> }
>
> +int bt_no_errqueue_poll_set_enabled(bool enabled)
> +{
> + if (enabled != no_errqueue_poll_enabled) {
> + WRITE_ONCE(no_errqueue_poll_enabled, enabled);
> + return 0;
> + }
> + return 1;
> +}
> +
> +bool bt_no_errqueue_poll_enabled(void)
> +{
> + return READ_ONCE(no_errqueue_poll_enabled);
> +}
> +
> +static bool bt_sock_error_queue_poll(struct sock *sk)
> +{
> + return !test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags) &&
> + !skb_queue_empty_lockless(&sk->sk_error_queue);
> +}
> +
> __poll_t bt_sock_poll(struct file *file, struct socket *sock,
> poll_table *wait)
> {
> @@ -511,7 +533,7 @@ __poll_t bt_sock_poll(struct file *file, struct socket *sock,
> if (sk->sk_state == BT_LISTEN)
> return bt_accept_poll(sk);
>
> - if (sk->sk_err || !skb_queue_empty_lockless(&sk->sk_error_queue))
> + if (sk->sk_err || bt_sock_error_queue_poll(sk))
> mask |= EPOLLERR |
> (sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? EPOLLPRI : 0);
>
> @@ -582,6 +604,80 @@ int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> }
> EXPORT_SYMBOL(bt_sock_ioctl);
>
> +int bt_sock_setsockopt(struct socket *sock, int level, int optname,
> + sockptr_t optval, unsigned int optlen)
> +{
> + struct sock *sk = sock->sk;
> + int err = 0;
> + u32 opt;
> +
> + if (level != SOL_BLUETOOTH)
> + return -ENOPROTOOPT;
> +
> + lock_sock(sk);
> +
> + switch (optname) {
> + case BT_NO_ERRQUEUE_POLL:
> + if (!bt_no_errqueue_poll_enabled()) {
> + err = -ENOPROTOOPT;
> + break;
> + }
> +
> + if (copy_from_sockptr(&opt, optval, sizeof(opt))) {
> + err = -EFAULT;
> + break;
> + }
> +
> + if (opt)
> + set_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
> + else
> + clear_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
> + break;
> +
> + default:
> + err = -ENOPROTOOPT;
> + break;
> + }
> +
> + release_sock(sk);
> + return err;
> +}
> +EXPORT_SYMBOL(bt_sock_setsockopt);
> +
> +int bt_sock_getsockopt(struct socket *sock, int level, int optname,
> + char __user *optval, int __user *optlen)
> +{
> + struct sock *sk = sock->sk;
> + int err = 0;
> + u32 opt;
> +
> + if (level != SOL_BLUETOOTH)
> + return -ENOPROTOOPT;
> +
> + lock_sock(sk);
> +
> + switch (optname) {
> + case BT_NO_ERRQUEUE_POLL:
> + if (!bt_no_errqueue_poll_enabled()) {
> + err = -ENOPROTOOPT;
> + break;
> + }
> +
> + opt = test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
> + if (put_user(opt, (u32 __user *)optval))
> + err = -EFAULT;
> + break;
> +
> + default:
> + err = -ENOPROTOOPT;
> + break;
> + }
> +
> + release_sock(sk);
> + return err;
> +}
> +EXPORT_SYMBOL(bt_sock_getsockopt);
> +
> /* This function expects the sk lock to be held when called */
> int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
> {
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index 42b4495e019e..3c5cf7789d38 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -1602,8 +1602,8 @@ static int iso_sock_setsockopt(struct socket *sock, int level, int optname,
> break;
>
> default:
> - err = -ENOPROTOOPT;
> - break;
> + release_sock(sk);
> + return bt_sock_setsockopt(sock, level, optname, optval, optlen);
> }
>
> release_sock(sk);
> @@ -1673,8 +1673,8 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname,
> break;
>
> default:
> - err = -ENOPROTOOPT;
> - break;
> + release_sock(sk);
> + return bt_sock_getsockopt(sock, level, optname, optval, optlen);
> }
>
> release_sock(sk);
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 7846a068bf60..a0f7c1bcdec8 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -698,8 +698,8 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
> break;
>
> default:
> - err = -ENOPROTOOPT;
> - break;
> + release_sock(sk);
> + return bt_sock_getsockopt(sock, level, optname, optval, optlen);
> }
>
> release_sock(sk);
> @@ -1103,8 +1103,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
> break;
>
> default:
> - err = -ENOPROTOOPT;
> - break;
> + release_sock(sk);
> + return bt_sock_setsockopt(sock, level, optname, optval, optlen);
> }
>
> release_sock(sk);
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 32ed6e9245a3..8f62850023a0 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -4339,6 +4339,12 @@ static const u8 mgmt_mesh_uuid[16] = {
> 0x8d, 0x4d, 0x03, 0x7a, 0xd7, 0x63, 0xe4, 0x2c,
> };
>
> +/* 69518c4c-b69f-4679-8bc1-c021b47b5733 */
> +static const u8 no_errqueue_poll_uuid[16] = {
> + 0x33, 0x57, 0x7b, 0xb4, 0x21, 0xc0, 0xc1, 0x8b,
> + 0x79, 0x46, 0x9f, 0xb6, 0x4c, 0x8c, 0x51, 0x69,
> +};
> +
> static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> void *data, u16 data_len)
> {
> @@ -4350,8 +4356,8 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
>
> bt_dev_dbg(hdev, "sock %p", sk);
>
> - /* Enough space for 7 features */
> - len = sizeof(*rp) + (sizeof(rp->features[0]) * 7);
> + /* Enough space for 8 features */
> + len = sizeof(*rp) + (sizeof(rp->features[0]) * 8);
> rp = kzalloc(len, GFP_KERNEL);
> if (!rp)
> return -ENOMEM;
> @@ -4429,6 +4435,11 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> idx++;
> }
>
> + flags = bt_no_errqueue_poll_enabled() ? BIT(0) : 0;
> + memcpy(rp->features[idx].uuid, no_errqueue_poll_uuid, 16);
> + rp->features[idx].flags = cpu_to_le32(flags);
> + idx++;
> +
> rp->feature_count = cpu_to_le16(idx);
>
> /* After reading the experimental features information, enable
> @@ -4926,6 +4937,53 @@ static int set_iso_socket_func(struct sock *sk, struct hci_dev *hdev,
> }
> #endif
>
> +static int set_no_errqueue_poll_func(struct sock *sk, struct hci_dev *hdev,
> + struct mgmt_cp_set_exp_feature *cp,
> + u16 data_len)
> +{
> + struct mgmt_rp_set_exp_feature rp;
> + bool val, changed = false;
> + int err;
> +
> + /* Command requires to use the non-controller index */
> + if (hdev)
> + return mgmt_cmd_status(sk, hdev->id,
> + MGMT_OP_SET_EXP_FEATURE,
> + MGMT_STATUS_INVALID_INDEX);
> +
> + /* Parameters are limited to a single octet */
> + if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
> + return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
> + MGMT_OP_SET_EXP_FEATURE,
> + MGMT_STATUS_INVALID_PARAMS);
> +
> + /* Only boolean on/off is supported */
> + if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
> + return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
> + MGMT_OP_SET_EXP_FEATURE,
> + MGMT_STATUS_INVALID_PARAMS);
> +
> + val = cp->param[0] ? true : false;
> +
> + err = bt_no_errqueue_poll_set_enabled(val);
> + if (!err)
> + changed = true;
> +
> + memcpy(rp.uuid, no_errqueue_poll_uuid, 16);
> + rp.flags = cpu_to_le32(val ? BIT(0) : 0);
> +
> + hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
> +
> + err = mgmt_cmd_complete(sk, MGMT_INDEX_NONE,
> + MGMT_OP_SET_EXP_FEATURE, 0,
> + &rp, sizeof(rp));
> +
> + if (changed)
> + exp_feature_changed(hdev, no_errqueue_poll_uuid, val, sk);
> +
> + return err;
> +}
> +
> static const struct mgmt_exp_feature {
> const u8 *uuid;
> int (*set_func)(struct sock *sk, struct hci_dev *hdev,
> @@ -4943,6 +5001,7 @@ static const struct mgmt_exp_feature {
> #ifdef CONFIG_BT_LE
> EXP_FEAT(iso_socket_uuid, set_iso_socket_func),
> #endif
> + EXP_FEAT(no_errqueue_poll_uuid, set_no_errqueue_poll_func),
>
> /* end with a null feature */
> EXP_FEAT(NULL, NULL)
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 99c2b713d826..d4697f147b5a 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -968,8 +968,8 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> break;
>
> default:
> - err = -ENOPROTOOPT;
> - break;
> + release_sock(sk);
> + return bt_sock_setsockopt(sock, level, optname, optval, optlen);
> }
>
> release_sock(sk);
> @@ -1212,8 +1212,8 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
> break;
>
> default:
> - err = -ENOPROTOOPT;
> - break;
> + release_sock(sk);
> + return bt_sock_getsockopt(sock, level, optname, optval, optlen);
> }
>
> release_sock(sk);
> --
> 2.44.0
>
>


--
Luiz Augusto von Dentz

2024-04-02 19:45:02

by Pauli Virtanen

[permalink] [raw]
Subject: Re: [PATCH v3] Bluetooth: add experimental BT_NO_ERRQUEUE_POLL socket option

Hi Luiz,

ti, 2024-04-02 kello 14:10 -0400, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
>
> On Tue, Apr 2, 2024 at 12:41 PM Pauli Virtanen <[email protected]> wrote:
> >
> > Add experimental feature that enables a SOL_BLUETOOTH socket option to
> > disable POLLERR on non-empty socket error queue. Add corresponding MGMT
> > experimental feature UUID for it.
> >
> > This is intended to allow applications disable the POLLERR wakeups due
> > to TX timestamps, and allow upper layers of the stack to enable and
> > consume TX timestamps, without needing to complicate the lower layer
> > POLLERR handling which is only interested in real socket errors.
> >
> > It is a socket option disabled by default, as it enables a deviation
> > from the common net/ TX timestamping API.
> >
> > Signed-off-by: Pauli Virtanen <[email protected]>
> > ---
> > include/net/bluetooth/bluetooth.h | 12 +++-
> > net/bluetooth/af_bluetooth.c | 98 ++++++++++++++++++++++++++++++-
> > net/bluetooth/iso.c | 8 +--
> > net/bluetooth/l2cap_sock.c | 8 +--
> > net/bluetooth/mgmt.c | 63 +++++++++++++++++++-
> > net/bluetooth/sco.c | 8 +--
> > 6 files changed, 181 insertions(+), 16 deletions(-)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > index c95afcd9c605..0f11b436e5b3 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -242,6 +242,8 @@ struct bt_codecs {
> >
> > #define BT_ISO_BASE 20
> >
> > +#define BT_NO_ERRQUEUE_POLL 21
>
> Perhaps we should revert the logic here and have it as
> BT_POLL_ERRQUEUE which by default shall be considered enabled,

I'll change it and remove the negation in the name -> v4

> that said don't we have a race if SO_TXTIMESTAMP is send ahead it could
> enqueue events on errqueue thus triggering the socket to wake up even
> if it later sends BT_POLL_ERRQUEUE? Or that stops the wake ups
> regardless?

Setting BT_NO_ERRQUEUE_POLL disables POLLERR wakeup in subsequent
poll() also if errqueue already has items. If you are doing poll() and
in another thread set SO_TIMESTAMPING while sending data and then set
BT_NO_ERRQUEUE_POLL there can be POLLERR wakeup, but I guess this is
expected.

Idea here was that the application that enables timestamping does it in
order

setsockopt(BT_NO_ERRQUEUE_POLL)
setsockopt(SO_TIMESTAMPING)

IOW, BlueZ wouldn't enable any of these socket options, and leaves it
to sound server.

SO_TIMESTAMPING only affects skbs that are created after setting it.

If the flags are set as above, the racing scenario would have to be
something like:

1. T1: enter poll()
2. T2: set BT_NO_ERRQUEUE_POLL -> set_bit(BT_SK_NO_ERRQUEUE_POLL)
3. T2: set SO_TIMESTAMPING
4. T3: send() generates skb with enabled TX tstamp flag, and queues it
5. hdev->workqueue: push skb to driver, TX SCHED tstamp to errqueue
6. T1: wake up, see stale value for test_bit(BT_SK_NO_ERRQUEUE_POLL)
but fresh value for sk->sk_error_queue content

The two setsockopt are ordered by lock_sock(), so if 4. generates
timestamped skb it is ordered after 2. by lock_sock(), so it looks like
6. should not be possible. (Did not think memory barriers through,
though feels unlikely when there's lock & atomics in between...)

***

The read_exp_features_info() below also needs fixup to return the new
option only for Index None -> v4

>
> > __printf(1, 2)
> > void bt_info(const char *fmt, ...);
> > __printf(1, 2)
> > @@ -389,7 +391,8 @@ struct bt_sock {
> > enum {
> > BT_SK_DEFER_SETUP,
> > BT_SK_SUSPEND,
> > - BT_SK_PKT_STATUS
> > + BT_SK_PKT_STATUS,
> > + BT_SK_NO_ERRQUEUE_POLL
> > };
> >
> > struct bt_sock_list {
> > @@ -412,6 +415,10 @@ int bt_sock_stream_recvmsg(struct socket *sock, struct msghdr *msg,
> > size_t len, int flags);
> > __poll_t bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
> > int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
> > +int bt_sock_setsockopt(struct socket *sock, int level, int optname,
> > + sockptr_t optval, unsigned int optlen);
> > +int bt_sock_getsockopt(struct socket *sock, int level, int optname,
> > + char __user *optval, int __user *optlen);
> > int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
> > int bt_sock_wait_ready(struct sock *sk, unsigned int msg_flags);
> >
> > @@ -652,4 +659,7 @@ void mgmt_cleanup(struct sock *sk);
> >
> > void bt_sock_reclassify_lock(struct sock *sk, int proto);
> >
> > +int bt_no_errqueue_poll_set_enabled(bool enabled);
> > +bool bt_no_errqueue_poll_enabled(void);
> > +
> > #endif /* __BLUETOOTH_H */
> > diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> > index 67604ccec2f4..cee4814f2aae 100644
> > --- a/net/bluetooth/af_bluetooth.c
> > +++ b/net/bluetooth/af_bluetooth.c
> > @@ -68,6 +68,8 @@ static const char *const bt_slock_key_strings[BT_MAX_PROTO] = {
> > "slock-AF_BLUETOOTH-BTPROTO_ISO",
> > };
> >
> > +static bool no_errqueue_poll_enabled;
> > +
> > void bt_sock_reclassify_lock(struct sock *sk, int proto)
> > {
> > BUG_ON(!sk);
> > @@ -500,6 +502,26 @@ static inline __poll_t bt_accept_poll(struct sock *parent)
> > return 0;
> > }
> >
> > +int bt_no_errqueue_poll_set_enabled(bool enabled)
> > +{
> > + if (enabled != no_errqueue_poll_enabled) {
> > + WRITE_ONCE(no_errqueue_poll_enabled, enabled);
> > + return 0;
> > + }
> > + return 1;
> > +}
> > +
> > +bool bt_no_errqueue_poll_enabled(void)
> > +{
> > + return READ_ONCE(no_errqueue_poll_enabled);
> > +}
> > +
> > +static bool bt_sock_error_queue_poll(struct sock *sk)
> > +{
> > + return !test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags) &&
> > + !skb_queue_empty_lockless(&sk->sk_error_queue);
> > +}
> > +
> > __poll_t bt_sock_poll(struct file *file, struct socket *sock,
> > poll_table *wait)
> > {
> > @@ -511,7 +533,7 @@ __poll_t bt_sock_poll(struct file *file, struct socket *sock,
> > if (sk->sk_state == BT_LISTEN)
> > return bt_accept_poll(sk);
> >
> > - if (sk->sk_err || !skb_queue_empty_lockless(&sk->sk_error_queue))
> > + if (sk->sk_err || bt_sock_error_queue_poll(sk))
> > mask |= EPOLLERR |
> > (sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? EPOLLPRI : 0);
> >
> > @@ -582,6 +604,80 @@ int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> > }
> > EXPORT_SYMBOL(bt_sock_ioctl);
> >
> > +int bt_sock_setsockopt(struct socket *sock, int level, int optname,
> > + sockptr_t optval, unsigned int optlen)
> > +{
> > + struct sock *sk = sock->sk;
> > + int err = 0;
> > + u32 opt;
> > +
> > + if (level != SOL_BLUETOOTH)
> > + return -ENOPROTOOPT;
> > +
> > + lock_sock(sk);
> > +
> > + switch (optname) {
> > + case BT_NO_ERRQUEUE_POLL:
> > + if (!bt_no_errqueue_poll_enabled()) {
> > + err = -ENOPROTOOPT;
> > + break;
> > + }
> > +
> > + if (copy_from_sockptr(&opt, optval, sizeof(opt))) {
> > + err = -EFAULT;
> > + break;
> > + }
> > +
> > + if (opt)
> > + set_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
> > + else
> > + clear_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
> > + break;
> > +
> > + default:
> > + err = -ENOPROTOOPT;
> > + break;
> > + }
> > +
> > + release_sock(sk);
> > + return err;
> > +}
> > +EXPORT_SYMBOL(bt_sock_setsockopt);
> > +
> > +int bt_sock_getsockopt(struct socket *sock, int level, int optname,
> > + char __user *optval, int __user *optlen)
> > +{
> > + struct sock *sk = sock->sk;
> > + int err = 0;
> > + u32 opt;
> > +
> > + if (level != SOL_BLUETOOTH)
> > + return -ENOPROTOOPT;
> > +
> > + lock_sock(sk);
> > +
> > + switch (optname) {
> > + case BT_NO_ERRQUEUE_POLL:
> > + if (!bt_no_errqueue_poll_enabled()) {
> > + err = -ENOPROTOOPT;
> > + break;
> > + }
> > +
> > + opt = test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
> > + if (put_user(opt, (u32 __user *)optval))
> > + err = -EFAULT;
> > + break;
> > +
> > + default:
> > + err = -ENOPROTOOPT;
> > + break;
> > + }
> > +
> > + release_sock(sk);
> > + return err;
> > +}
> > +EXPORT_SYMBOL(bt_sock_getsockopt);
> > +
> > /* This function expects the sk lock to be held when called */
> > int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
> > {
> > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> > index 42b4495e019e..3c5cf7789d38 100644
> > --- a/net/bluetooth/iso.c
> > +++ b/net/bluetooth/iso.c
> > @@ -1602,8 +1602,8 @@ static int iso_sock_setsockopt(struct socket *sock, int level, int optname,
> > break;
> >
> > default:
> > - err = -ENOPROTOOPT;
> > - break;
> > + release_sock(sk);
> > + return bt_sock_setsockopt(sock, level, optname, optval, optlen);
> > }
> >
> > release_sock(sk);
> > @@ -1673,8 +1673,8 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname,
> > break;
> >
> > default:
> > - err = -ENOPROTOOPT;
> > - break;
> > + release_sock(sk);
> > + return bt_sock_getsockopt(sock, level, optname, optval, optlen);
> > }
> >
> > release_sock(sk);
> > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > index 7846a068bf60..a0f7c1bcdec8 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -698,8 +698,8 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
> > break;
> >
> > default:
> > - err = -ENOPROTOOPT;
> > - break;
> > + release_sock(sk);
> > + return bt_sock_getsockopt(sock, level, optname, optval, optlen);
> > }
> >
> > release_sock(sk);
> > @@ -1103,8 +1103,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
> > break;
> >
> > default:
> > - err = -ENOPROTOOPT;
> > - break;
> > + release_sock(sk);
> > + return bt_sock_setsockopt(sock, level, optname, optval, optlen);
> > }
> >
> > release_sock(sk);
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 32ed6e9245a3..8f62850023a0 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -4339,6 +4339,12 @@ static const u8 mgmt_mesh_uuid[16] = {
> > 0x8d, 0x4d, 0x03, 0x7a, 0xd7, 0x63, 0xe4, 0x2c,
> > };
> >
> > +/* 69518c4c-b69f-4679-8bc1-c021b47b5733 */
> > +static const u8 no_errqueue_poll_uuid[16] = {
> > + 0x33, 0x57, 0x7b, 0xb4, 0x21, 0xc0, 0xc1, 0x8b,
> > + 0x79, 0x46, 0x9f, 0xb6, 0x4c, 0x8c, 0x51, 0x69,
> > +};
> > +
> > static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> > void *data, u16 data_len)
> > {
> > @@ -4350,8 +4356,8 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> >
> > bt_dev_dbg(hdev, "sock %p", sk);
> >
> > - /* Enough space for 7 features */
> > - len = sizeof(*rp) + (sizeof(rp->features[0]) * 7);
> > + /* Enough space for 8 features */
> > + len = sizeof(*rp) + (sizeof(rp->features[0]) * 8);
> > rp = kzalloc(len, GFP_KERNEL);
> > if (!rp)
> > return -ENOMEM;
> > @@ -4429,6 +4435,11 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> > idx++;
> > }
> >
> > + flags = bt_no_errqueue_poll_enabled() ? BIT(0) : 0;
> > + memcpy(rp->features[idx].uuid, no_errqueue_poll_uuid, 16);
> > + rp->features[idx].flags = cpu_to_le32(flags);
> > + idx++;
> > +
> > rp->feature_count = cpu_to_le16(idx);
> >
> > /* After reading the experimental features information, enable
> > @@ -4926,6 +4937,53 @@ static int set_iso_socket_func(struct sock *sk, struct hci_dev *hdev,
> > }
> > #endif
> >
> > +static int set_no_errqueue_poll_func(struct sock *sk, struct hci_dev *hdev,
> > + struct mgmt_cp_set_exp_feature *cp,
> > + u16 data_len)
> > +{
> > + struct mgmt_rp_set_exp_feature rp;
> > + bool val, changed = false;
> > + int err;
> > +
> > + /* Command requires to use the non-controller index */
> > + if (hdev)
> > + return mgmt_cmd_status(sk, hdev->id,
> > + MGMT_OP_SET_EXP_FEATURE,
> > + MGMT_STATUS_INVALID_INDEX);
> > +
> > + /* Parameters are limited to a single octet */
> > + if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
> > + return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
> > + MGMT_OP_SET_EXP_FEATURE,
> > + MGMT_STATUS_INVALID_PARAMS);
> > +
> > + /* Only boolean on/off is supported */
> > + if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
> > + return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
> > + MGMT_OP_SET_EXP_FEATURE,
> > + MGMT_STATUS_INVALID_PARAMS);
> > +
> > + val = cp->param[0] ? true : false;
> > +
> > + err = bt_no_errqueue_poll_set_enabled(val);
> > + if (!err)
> > + changed = true;
> > +
> > + memcpy(rp.uuid, no_errqueue_poll_uuid, 16);
> > + rp.flags = cpu_to_le32(val ? BIT(0) : 0);
> > +
> > + hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
> > +
> > + err = mgmt_cmd_complete(sk, MGMT_INDEX_NONE,
> > + MGMT_OP_SET_EXP_FEATURE, 0,
> > + &rp, sizeof(rp));
> > +
> > + if (changed)
> > + exp_feature_changed(hdev, no_errqueue_poll_uuid, val, sk);
> > +
> > + return err;
> > +}
> > +
> > static const struct mgmt_exp_feature {
> > const u8 *uuid;
> > int (*set_func)(struct sock *sk, struct hci_dev *hdev,
> > @@ -4943,6 +5001,7 @@ static const struct mgmt_exp_feature {
> > #ifdef CONFIG_BT_LE
> > EXP_FEAT(iso_socket_uuid, set_iso_socket_func),
> > #endif
> > + EXP_FEAT(no_errqueue_poll_uuid, set_no_errqueue_poll_func),
> >
> > /* end with a null feature */
> > EXP_FEAT(NULL, NULL)
> > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > index 99c2b713d826..d4697f147b5a 100644
> > --- a/net/bluetooth/sco.c
> > +++ b/net/bluetooth/sco.c
> > @@ -968,8 +968,8 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> > break;
> >
> > default:
> > - err = -ENOPROTOOPT;
> > - break;
> > + release_sock(sk);
> > + return bt_sock_setsockopt(sock, level, optname, optval, optlen);
> > }
> >
> > release_sock(sk);
> > @@ -1212,8 +1212,8 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
> > break;
> >
> > default:
> > - err = -ENOPROTOOPT;
> > - break;
> > + release_sock(sk);
> > + return bt_sock_getsockopt(sock, level, optname, optval, optlen);
> > }
> >
> > release_sock(sk);
> > --
> > 2.44.0
> >
> >
>
>