On Mon, Feb 3, 2014 at 1:06 PM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote: > Hi > > On Mon, Feb 3, 2014 at 6:57 PM, Dmitry Torokhov > <dmitry.torokhov@xxxxxxxxx> wrote: >> 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, so it's basically the other way around as I thought. So I > think using UNIQ is the way to go, sorry for the confusion. > > Thanks > David Thanks for clearing this up. It sounds like UNIQ should be safe for getting the client MAC of a Bluetooth device as long as the appropriate sanity checks are performed to make sure that it's not a custom string from a uhid device. Or, going back to the earlier question, is there still some reason as to why HIDP might change the behavior of UNIQ in the future? -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
