Date:   Thu, 21 Mar 2019 10:23:47 +0100
From:   Stephan Gerhold <stephan@gerhold.net>
To:     Ferry Toth <ftoth@exalondelft.nl>
Cc:     Marcel Holtmann <marcel@holtmann.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH 2/2] Bluetooth: btbcm: Add default address for BCM2076B1
Message-ID: <20190321092347.GA921@gerhold.net>
References: <20190305130901.56660-1-stephan@gerhold.net>
 <20190305130901.56660-2-stephan@gerhold.net>
 <7b48753f-f103-f522-68c5-38479feb4552@exalondelft.nl>
 <20190319140356.GA1076@gerhold.net>
 <0898b45c-b59a-0873-d8fc-1755e5c5e23b@exalondelft.nl>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <0898b45c-b59a-0873-d8fc-1755e5c5e23b@exalondelft.nl>
User-Agent: Mutt/1.11.4 (2019-03-13)
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

On Wed, Mar 20, 2019 at 10:11:45PM +0100, Ferry Toth wrote:
> 
> Op 19-03-19 om 15:03 schreef Stephan Gerhold:
> > Hi,
> > 
> > Sorry for the late response. I've been quite busy lately...
> Thanks for responding anyway!
> > 
> > On Tue, Mar 05, 2019 at 07:26:14PM +0100, Ferry Toth wrote:
> > > Op 05-03-19 om 14:09 schreef Stephan Gerhold:
> > > > BCM2076B1 appears to use 20:76:A0:00:56:79 as default address.
> > > > This address is used by at least 5 devices with the AMPAK AP6476
> > > > module and is also suspicious because it starts with the chip name
> > > > 2076 (followed by a different revision A0 for some reason).
> > > With BCM43340 (Edison) it's the same principle. With the fake address I
> > > found everything (in user space) seems to work except PAN/NAP (bnep) and no
> > > decent error message anywhere making this quite hard to debug.
> > Not familiar with PAN/NAP, but the main reason to avoid default
> > addresses is that they will cause issues as soon as you have two of
> > those devices next to each other.
> 
> That might be the main issue with other services, but if you have only a
> single device you won't see this.
> 

Well, the other obvious thing is that it goes against the BT 
specification if you use an address that is not unique. :)

But you are right, I've also been using the default address for more 
than a year, simply because I wasn't aware that the BT controller does 
not have a proper BT device address. I did not run into any problems.

> With PAN/NAP I found bnep network device is created and immediately removed,
> which confuses connman and give no other message than 'I/O error'. Even with
> only one device.
> 
> > > > Add it to the list of default addresses and leave it up to the
> > > > user to configure a valid one.
> > > Which way is the user supposed to configure the valid one?
> > > 
> > It took me a while to figure this out - there is not much
> > documentation for this. The following is only based on my own
> > investigations:
> > 
> > As far as the kernel is concerned, HCI_QUIRK_INVALID_BDADDR is exposed
> > through the btmgmt API [1]. With the quirk, the BT controller will
> > appear as "unconfigured" controller to userspace, and will have the
> > public address as missing option. As soon as the address is configured
> > with the "Set Public Address Command", the unconfigured controller is
> > removed and replaced with a regular controller.
> >    Only then it will be usable through bluetoothd.
> Yes, Andy Shevchenko already discovered we are missing to put the device on
> the blacklist. I was all ready worried I would get a unconfigured
> controller. Now I see, that is a good thing. Thanks, this is really useful.
> > The btmgmt API has to be controlled from user space.
> > As far as I know, the only tool in bluez to set the public address
> > is the "btmgmt" tool:
> > 
> >    btmgmt -i hci0 public-addr xx:xx:xx:xx:xx:xx
> > 
> > ... will run the "Set Public Address Command" described above.
> Especially in combination with this. I will be documenting this for Edison,
> so hopefully other can benefit.
> > However, it was mentioned several times that "btmgmt" is not
> > a "stable" tool. In other words, it is not installed by default
> > and is only intended for debugging. It may change any time.
> > 
> > There was a related discussion on IRC recently: (I was not involved)
> > 
> >    10:50 <Mis012[m]> sjanc: do I need to convince my distro to package
> >          btmgmt, or will you revert that commit?
> >    10:51 <sjanc> Mis012[m]: I'd ask holtmann about this :) but btmgmt
> >          was more like testing tool than real end user tool
> >          (ie, no manpages, rather ad-hoc design and command set etc
> >    10:53 <sjanc> Mis012[m]: but if you really need this feature,
> >          I'd consider starting discussion on mailing list
> >          on how this could be handled by bluetoothd
> >    10:53 <sjanc> as mentioned, this could be in plugin,
> >          either common plugin or maybe platform specific,
> >          which would set public address via mgmt api
> >    10:54 <sjanc> open point is how bluetootd would extract public address,
> >          which is always platform specific
> >    10:55 <holtmann> Mis012[m]: btmgmt is not a stable tool.
> >    10:55 <Mis012[m]> holtmann: but it's the only one which can do this
> >    10:56 <holtmann> Then add support to bluetoothd for it or do it via an
> >          external tool / daemon. I wrote a long post to the mailing list
> >          about that.
> > 
> > I have such an external daemon for Android (since it does not use bluez)
> > at [2]. It would be easy to refactor it to something more portable.
> Do I understand correct: support needs to go into bluetoothd first and then
> into bluetoothctl?

I'm not very familiar with the bluez code base to be honest.
As far as I know, bluetoothctl only controls bluetoothd, so this sounds 
correct. The way such a feature would be exposed (command line tool,
configuration option, ...) would need to be discussed further on the 
mailing list first.

> > 
> > [1]: https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/mgmt-api.txt
> > [2]: https://github.com/me176c-dev/android_device_asus_K013/blob/lineage-16.0/bluetooth/bdaddr.c
> > 
> > > The only way I found is bd_addr (tool from bluez that is not normally
> > > built/installed).
> > > 
> > > Using this tool requires hci0 to be up and bluetoothd to be restarted if
> > > that was already running, which is quite inconvenient.
> > > 
> > bdaddr does not use the btmgmt API. It sends vendor-specific commands to
> > the HCI interface. This might be the reason why you need to restart
> > bluetoothd.
> I see.
> > 
> > With HCI_QUIRK_INVALID_BDADDR set, the BT controller should not show up
> > at all in bluetoothctl until its public address is configured.
> > It does not matter if bluetoothd is started too early.
> > 
> > > I also saw there was a patch to put the address in dt.
> > > 
> > > But nowhere did I find a kernel parameter to pass the address. Did I miss
> > > something?
> > > 
> > I'm not familiar with setting the address from DT/kernel parameters.
> > Sorry :(
> To me sounds like reading from a configuration file on disk would make more
> sense.

On my device, the real WiFi/BT address resides on an extra partition 
with a device-specific file system layout, so a configuration file
would not really help me personally.

A simple option that would make BT work out of the box for all devices 
with this quirk would be to generate a random MAC address once,
and then keep using it across reboots.
But I'm not sure if such a feature would be accepted for bluez.