Re: [PATCH 0/6] HID: Add a stable method for retrieving the client MAC address of a HID device

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Mon, Feb 03, 2014 at 06:45:52PM +0100, David Herrmann wrote:
> Hi
> 
> Adding Dmitry+Jiri, maybe they can clarify this.
> 
> [snip]
> >>>
> >>> Yes
> >>>
> >>>> hidp sets it to point to the hci_conn struct from which src address for the
> >>>> Bluetooth connection can be obtained, but making assumptions about an opaque
> >>>> pointer like that seems dangerous.  Is the parent pointer guaranteed to
> >>>> point to the hci_conn struct as long as the bus type is Bluetooth?
> >>>
> >>> And nope. If you use uhid, then the parent will not be a hci_conn.
> >>>
> >>> With enough guards, you should be able to use it, but it's not ideal I agree.
> >>> I really want to have David's opinion regarding the UNIQ field. IMO,
> >>> this seems to be the most transport-agnostic way of doing it.
> >>
> >> UNIQ is definitely wrong. It is used to assign a run-time *unique*
> >> value to the connection. So ideally it should be different if a device
> >> is reconnected. The PHYS field is actually used to identify a physical
> >> device. So please, if we're doing this, then we should do it via PHYS.
> >>
> >> I'm fine with hard-coding PHYS as bt-address for BT-HID, but please
> >> add a comment to hidp_setup_hid() in net/bluetooth/hidp/core.c before
> >> doing the snprintf() there.
> >>
> >> The reason why I disliked hard-coding this behavior is that if a
> >> single BT-device is connected twice to us, we usually want two
> >> different PHYS entries for both depending on which service this
> >> connection provides. However, this seems like an unlikely and
> >> overengineered problem so lets not do that. Furthermore, while BT-HID
> >> would allow such setups with some hacks, it isn't supported in a clean
> >> way. So yeah, I'm actually fine with using PHYS for that.
> >>
> >> Thanks
> >> David
> >
> > PHYS should definitely be changed if this is the case because right
> > now it is set to the MAC of the host adapter which means that it's the
> > same for every Bluetooth device and connection.  I believe that the
> > hidraw documentation also specifies that for Bluetooth devices the
> > HIDIOCGRAWPHYS ioctl should return a string with the MAC address of
> > the associated device rather than that of the host adapter as the
> > current behavior does.
> 
> Oh, yes, nice catch!
> Ok, maybe we need to clear up what PHYS and UNIQ do before relying on
> them. I thought, they were defined as:
> 
> PHYS: A string describing the physical device. It should be the same
> if a device reconnects. It can be used by user-space to track devices
> across disconnects
> 
> UNIQ: A string describing the current connection to a device. If the
> device reconnects, the UNIQ string should change. It can be used by
> user-space to track a single device-session.
> 
> afaik both strings have no common format and drivers are free to
> provide any kind of information, as long as it follows the given
> rules. That's why I disliked the idea of relying on them and parsing
> them. But maybe I just have no idea what the original intention was
> behind them?

PHYS: describes physical connection of the device in a given box,
supposed to be unique within a box.

UNIQ: unique identifier (if exists) assigned to the device, should
ideally be unique globally and should not change when device is moved
within a box out between boxes. Think serial number in USB descriptors.

Thanks.

-- 
Dmitry