This reverts commit 4b95a24ce12c4545fd7d2e3075841dc3119d1d71.
My USB bluetooth device does not show up with this patch in place.
Signed-off-by: Keith Packard <[email protected]>
---
This patch seems so innocuous, but when added to the kernel, it keeps
my USB bluetooth device from appearing to user mode at all. With the
mgmt interface enabled:
$ hcitool dev
Devices:
With this patch reverted:
$ hcitool dev
Devices:
hci0 04:0C:CE:DF:7B:C9
I bisected to this revision, and then reverted it on top of version
v3.3-6972-ge22057c e22057c8599373e5caef0bc42bdb95d2a361ab0d
(which is where I'm trying to move drm-intel-fixes to)
I think this is the third consecutive merge window that has broken my
bluetooth interface in some way? At least I know enough to check
now...
net/bluetooth/hci_sock.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 63afd23..d8b16cf 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -50,6 +50,8 @@
#include <net/bluetooth/hci_core.h>
#include <net/bluetooth/hci_mon.h>
+static bool enable_mgmt;
+
static atomic_t monitor_promisc = ATOMIC_INIT(0);
/* ----- HCI socket interface ----- */
@@ -649,7 +651,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
break;
case HCI_CHANNEL_CONTROL:
- if (haddr.hci_dev != HCI_DEV_NONE) {
+ if (haddr.hci_dev != HCI_DEV_NONE || !enable_mgmt) {
err = -EINVAL;
goto done;
}
@@ -1127,3 +1129,6 @@ void hci_sock_cleanup(void)
proto_unregister(&hci_sk_proto);
}
+
+module_param(enable_mgmt, bool, 0644);
+MODULE_PARM_DESC(enable_mgmt, "Enable Management interface");
--
1.7.9
Hi Keith,
* Keith Packard <[email protected]> [2012-03-28 19:35:44 -0700]:
> <#part sign=pgpmime>
> On Wed, 28 Mar 2012 23:28:39 -0300, Gustavo Padovan <[email protected]> wrote:
>
> > A consequence is that you'll need to use newer version of bluez, at least 4.99.
> > Or call the bluetooth daemon (from a pre 4.99 version) with a parameter to
> > disable the new interface:
>
> That's not OK -- you're breaking user space with this kernel change. I
> know I get bashed every time I suggest that we 'fix' the kernel and
> require new user space X bits...
Can you try the following patch? It should fix the compatibility problem you had.
Gustavo
--
commit d21c1177b9cf067809ccee2746633cfea3a8b062
Author: Gustavo Padovan <[email protected]>
Date: Thu Mar 29 09:47:53 2012 -0300
Bluetooth: Fix userspace compatibility issue with mgmt interface
To ensure that old user space versions do not accidentally pick up and
try to use the management channel, use a different channel number.
Reported-by: Keith Packard <[email protected]>
Acked-by: Johan Hedberg <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
Signed-off-by: Gustavo Padovan <[email protected]>
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 344b0f9..ba7f148 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1327,8 +1327,8 @@ struct sockaddr_hci {
#define HCI_DEV_NONE 0xffff
#define HCI_CHANNEL_RAW 0
-#define HCI_CHANNEL_CONTROL 1
#define HCI_CHANNEL_MONITOR 2
+#define HCI_CHANNEL_CONTROL 3
struct hci_filter {
unsigned long type_mask;
From: Gustavo Padovan <[email protected]>
Date: Wed, 28 Mar 2012 23:28:39 -0300
> A consequence is that you'll need to use newer version of bluez, at
> least 4.99.
That's complete bullshit and you know it.
Existing userspace worked, you guys broke it with a kernel change.
Therefore the kernel change needs to be reverted.
From: Keith Packard <[email protected]>
Date: Wed, 28 Mar 2012 19:35:44 -0700
> <#part sign=pgpmime>
> On Wed, 28 Mar 2012 23:28:39 -0300, Gustavo Padovan <[email protected]> wrote:
>
>> A consequence is that you'll need to use newer version of bluez, at least 4.99.
>> Or call the bluetooth daemon (from a pre 4.99 version) with a parameter to
>> disable the new interface:
>
> That's not OK -- you're breaking user space with this kernel change.
+1
From: Marcel Holtmann <[email protected]>
Date: Thu, 29 Mar 2012 05:33:42 +0200
> I think we actually have a broken user space in this case. I do have a
> hunch on what went wrong. It is not the kernel's fault here.
I call bullshit.
It doesn't matter what userspace is doing, "broken" or however you
want to call it. The bluez on everyone's computer broke because of
the kernel change.
Therefore you must revert the kernel change or modify it so that
existing userland continues to work properly.
There is no other proper course of action.
And to think, I gave you bluetooth guys shit because of coding style
problems, that appears to have been just the tip of the iceberg.
<#part sign=pgpmime>
On Wed, 28 Mar 2012 23:28:39 -0300, Gustavo Padovan <[email protected]> wrote:
> $ bluetoothd -P mgmtops
>
> Then your bluetooth will be back.
Indeed. Please fix the kernel so that existing user space works again.
--
[email protected]
Hi Keith,
> > A consequence is that you'll need to use newer version of bluez, at least 4.99.
> > Or call the bluetooth daemon (from a pre 4.99 version) with a parameter to
> > disable the new interface:
>
> That's not OK -- you're breaking user space with this kernel change. I
> know I get bashed every time I suggest that we 'fix' the kernel and
> require new user space X bits...
I think we actually have a broken user space in this case. I do have a
hunch on what went wrong. It is not the kernel's fault here.
If I am anywhere right, then bluetoothd fails to bring up the device.
That is why you don't see anything with "hcitool dev", but the device
should be present. Check "hciconfig -a".
Check your bluetoothd startup line and add "-P mgmtops". That will
disable the broken plugin.
Regards
Marcel
<#part sign=pgpmime>
On Wed, 28 Mar 2012 23:28:39 -0300, Gustavo Padovan <[email protected]> wrote:
> A consequence is that you'll need to use newer version of bluez, at least 4.99.
> Or call the bluetooth daemon (from a pre 4.99 version) with a parameter to
> disable the new interface:
That's not OK -- you're breaking user space with this kernel change. I
know I get bashed every time I suggest that we 'fix' the kernel and
require new user space X bits...
--
[email protected]
Hi Keith,
* Keith Packard <[email protected]> [2012-03-28 18:19:18 -0700]:
> This reverts commit 4b95a24ce12c4545fd7d2e3075841dc3119d1d71.
>
> My USB bluetooth device does not show up with this patch in place.
>
> Signed-off-by: Keith Packard <[email protected]>
> ---
>
> This patch seems so innocuous, but when added to the kernel, it keeps
> my USB bluetooth device from appearing to user mode at all. With the
> mgmt interface enabled:
It is because it changes the way the userspace <-> kernel interface. We added a
new interface and that flag was preventing the code to use it by default, but
now that we finished to implement the new interface it will be enabled by default.
A consequence is that you'll need to use newer version of bluez, at least 4.99.
Or call the bluetooth daemon (from a pre 4.99 version) with a parameter to
disable the new interface:
$ bluetoothd -P mgmtops
Then your bluetooth will be back.
Gustavo
Hi Keith,
> This reverts commit 4b95a24ce12c4545fd7d2e3075841dc3119d1d71.
>
> My USB bluetooth device does not show up with this patch in place.
>
> Signed-off-by: Keith Packard <[email protected]>
> ---
>
> This patch seems so innocuous, but when added to the kernel, it keeps
> my USB bluetooth device from appearing to user mode at all. With the
> mgmt interface enabled:
>
> $ hcitool dev
> Devices:
>
> With this patch reverted:
>
> $ hcitool dev
> Devices:
> hci0 04:0C:CE:DF:7B:C9
so this is seriously strange and I don't have an explanation for it
right now. Something breaking on this level means that Bluetooth devices
are not even registered. And that would be an obvious bug to detect.
Can you check with hciconfig -a if the device is actually there, but
might have some weird flags set.
> I bisected to this revision, and then reverted it on top of version
>
> v3.3-6972-ge22057c e22057c8599373e5caef0bc42bdb95d2a361ab0d
>
> (which is where I'm trying to move drm-intel-fixes to)
>
> I think this is the third consecutive merge window that has broken my
> bluetooth interface in some way? At least I know enough to check
> now...
>
> net/bluetooth/hci_sock.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> index 63afd23..d8b16cf 100644
> --- a/net/bluetooth/hci_sock.c
> +++ b/net/bluetooth/hci_sock.c
> @@ -50,6 +50,8 @@
> #include <net/bluetooth/hci_core.h>
> #include <net/bluetooth/hci_mon.h>
>
> +static bool enable_mgmt;
> +
> static atomic_t monitor_promisc = ATOMIC_INIT(0);
>
> /* ----- HCI socket interface ----- */
> @@ -649,7 +651,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
> break;
>
> case HCI_CHANNEL_CONTROL:
> - if (haddr.hci_dev != HCI_DEV_NONE) {
> + if (haddr.hci_dev != HCI_DEV_NONE || !enable_mgmt) {
> err = -EINVAL;
> goto done;
> }
I have no idea on how this can break anything for you. Since tools like
hcitool and hciconfig use HCI_CHANNEL_RAW. This must be a nasty side
effect somewhere if this is really fix it for you.
Can you start your bluetoothd with -P mgmtops to check if this might be
causing some side effects.
Regards
Marcel
From: Keith Packard <[email protected]>
Date: Wed, 28 Mar 2012 18:19:18 -0700
> This reverts commit 4b95a24ce12c4545fd7d2e3075841dc3119d1d71.
>
> My USB bluetooth device does not show up with this patch in place.
>
> Signed-off-by: Keith Packard <[email protected]>
Bluetooth folks please look over this, thanks.
Hi Keith,
* Keith Packard <[email protected]> [2012-04-05 15:24:58 -0700]:
> <#part sign=pgpmime>
> On Fri, 30 Mar 2012 03:54:56 -0300, Gustavo Padovan <[email protected]> wrote:
>
> > Can you try the following patch? It should fix the compatibility
> > problem you had.
>
> Yes, the kernel with that patch (without the revert) works now.
Thanks for testing, I will push this to mainline ASAP.
Gustavo
<#part sign=pgpmime>
On Fri, 30 Mar 2012 03:54:56 -0300, Gustavo Padovan <[email protected]> wrote:
> Can you try the following patch? It should fix the compatibility
> problem you had.
Yes, the kernel with that patch (without the revert) works now.
--
[email protected]
Hi Keith,
* Gustavo Padovan <[email protected]> [2012-03-30 03:54:56 -0300]:
> Hi Keith,
>
> * Keith Packard <[email protected]> [2012-03-28 19:35:44 -0700]:
>
> > <#part sign=pgpmime>
> > On Wed, 28 Mar 2012 23:28:39 -0300, Gustavo Padovan <[email protected]> wrote:
> >
> > > A consequence is that you'll need to use newer version of bluez, at least 4.99.
> > > Or call the bluetooth daemon (from a pre 4.99 version) with a parameter to
> > > disable the new interface:
> >
> > That's not OK -- you're breaking user space with this kernel change. I
> > know I get bashed every time I suggest that we 'fix' the kernel and
> > require new user space X bits...
>
> Can you try the following patch? It should fix the compatibility problem you had.
Did you had chance to test this patch?
Gustavo
>
> commit d21c1177b9cf067809ccee2746633cfea3a8b062
> Author: Gustavo Padovan <[email protected]>
> Date: Thu Mar 29 09:47:53 2012 -0300
>
> Bluetooth: Fix userspace compatibility issue with mgmt interface
>
> To ensure that old user space versions do not accidentally pick up and
> try to use the management channel, use a different channel number.
>
> Reported-by: Keith Packard <[email protected]>
> Acked-by: Johan Hedberg <[email protected]>
> Acked-by: Marcel Holtmann <[email protected]>
> Signed-off-by: Gustavo Padovan <[email protected]>
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 344b0f9..ba7f148 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1327,8 +1327,8 @@ struct sockaddr_hci {
> #define HCI_DEV_NONE 0xffff
>
> #define HCI_CHANNEL_RAW 0
> -#define HCI_CHANNEL_CONTROL 1
> #define HCI_CHANNEL_MONITOR 2
> +#define HCI_CHANNEL_CONTROL 3
>
> struct hci_filter {
> unsigned long type_mask;