linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Kemnade <andreas@kemnade.info>
To: Carey Sonsino <csonsino@gmail.com>
Cc: Marcel Holtmann <marcel@holtmann.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	linux-bluetooth@vger.kernel.org, Sasha Levin <sashal@kernel.org>
Subject: Re: [PATCH 1/1] bluetooth: validate BLE connection interval updates
Date: Mon, 19 Aug 2019 18:08:13 +0200	[thread overview]
Message-ID: <20190819180813.04a8e771@kemnade.info> (raw)
In-Reply-To: <0cda8242-304e-a073-90d8-63e656e3600c@gmail.com>

Hi Carey,

On Mon, 19 Aug 2019 07:58:19 -0600
Carey Sonsino <csonsino@gmail.com> wrote:

> This seems like the exact "downside" situation that I described in the 
> patch writeup.
> 
> I would still claim that as a Linux system administrator, I should have 
> control over my system.  If I am operating in a low power environment, I 
> do not want to allow a remote device to apply a setting which causes me 
> to use more power without any say in the matter.
> 
In principle I agree here. High connection interval has its downsides,
low connection interval also. Just curios: What are the numbers about
power consumption here? A few mA? I have only compared these values on
peripherals running on low-power SoCs like e.g. the nrf stuff from nordic.
I see around 1 mA difference there with a power consumption besides of that 
usually measured in the µA range. Never tested these things on a linux machine.

The point here is that with this patch there is insufficient control
about this. Yes, there is the debugfs interface.

But if you want to provide a driver to a gatt service living on top of
bluetoothd dbus api? Ask people to not use distribution kernels?
What options do you have?
using the monitor interface to sniff the connection handle and then
call hci_le_conn_update() to set things?

> The connection min/max interval settings are configuration options that 
> control how my bluetooth device operates.  Why are these down in debugfs 
> anyway?  I think that a much more appropriate fix would be to migrate 
> some of the BLE configuration options to sysconfdir (some place like 
> /etc/bluetooth/ble.conf).  That would also help in the persistence of 
> these configuration settings, which is kind of a pain with the debugfs 
> mechanism that gets wiped out and recreated on bootup.
> 
I think that these things should be part of the  dbus api provided
by bluetoothd so that a driver could decide and having defaults outside
of such a dark corner like the debug fs.

> A quicker fix would be to simply set the default connection min interval 
> and connection max interval values to the full range (6, 3200).

Or just maybe a flag allowing such behavior?

> *think* that this could be done by simply updating the values in 
> hci_core.c, the hci_alloc_dev() function:
> 
>      hdev->le_conn_min_interval = 0x0018;
>      hdev->le_conn_max_interval = 0x0028;
> 
> would become:
> 
>      hdev->le_conn_min_interval = 0x0006;
>      hdev->le_conn_max_interval = 0x0c80;
> 
> This should allow all devices to negotiate whatever connection interval 
> they want by default.  If I'm running on a device with debugfs (which I 
> happen to be most of the time), then I can still override these defaults 
> to control my system.
> 
> Please let me know if you would like me to do any further work towards 
> resolving this issue.  I'd be happy to test and submit a patch that 
> changes the default connection min/max interval values- I could probably 
> get that done in the next day or two.  If you would like me to 
> investigate migrating configuration settings to /etc then I'd be happy 
> to do that as well, but it might take a bit more effort and time.
> 
Well, all these things are important, but are new features but there is a
problem:
The kernel patch has gone into the stable trees and from there into distributions,
so how can these new features flow down the same path.

Regards,
Andreas

  reply	other threads:[~2019-08-19 16:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-12 21:00 [PATCH 1/1] bluetooth: validate BLE connection interval updates csonsino
2019-07-06 13:34 ` Marcel Holtmann
2019-08-15  8:44   ` Andreas Kemnade
2019-08-19 13:58     ` Carey Sonsino
2019-08-19 16:08       ` Andreas Kemnade [this message]
2019-08-19 17:10         ` Carey Sonsino
2019-08-20  6:45           ` Jamie Mccrae
2019-08-20 13:23             ` Carey Sonsino

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190819180813.04a8e771@kemnade.info \
    --to=andreas@kemnade.info \
    --cc=csonsino@gmail.com \
    --cc=johan.hedberg@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=marcel@holtmann.org \
    --cc=sashal@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).