Hi Marcel, On Fri, 4 Dec 2020 at 17:51, Marcel Holtmann <mar...@holtmann.org> wrote: > > Hi Archie, > > >>> MSFT needs rssi parameter for monitoring advertisement packet, > >>> therefore we should supply them from mgmt. > >>> > >>> Signed-off-by: Archie Pusaka <apus...@chromium.org> > >>> Reviewed-by: Miao-chen Chou <mcc...@chromium.org> > >>> Reviewed-by: Yun-Hao Chung <howardch...@google.com> > >> > >> I don’t need any Reviewed-by if they are not catching an obvious user API > >> breakage. > >> > >>> --- > >>> > >>> include/net/bluetooth/hci_core.h | 9 +++++++++ > >>> include/net/bluetooth/mgmt.h | 9 +++++++++ > >>> net/bluetooth/mgmt.c | 8 ++++++++ > >>> 3 files changed, 26 insertions(+) > >>> > >>> diff --git a/include/net/bluetooth/hci_core.h > >>> b/include/net/bluetooth/hci_core.h > >>> index 9873e1c8cd16..42d446417817 100644 > >>> --- a/include/net/bluetooth/hci_core.h > >>> +++ b/include/net/bluetooth/hci_core.h > >>> @@ -246,8 +246,17 @@ struct adv_pattern { > >>> __u8 value[HCI_MAX_AD_LENGTH]; > >>> }; > >>> > >>> +struct adv_rssi_thresholds { > >>> + __s8 low_threshold; > >>> + __s8 high_threshold; > >>> + __u16 low_threshold_timeout; > >>> + __u16 high_threshold_timeout; > >>> + __u8 sampling_period; > >>> +}; > >>> + > >>> struct adv_monitor { > >>> struct list_head patterns; > >>> + struct adv_rssi_thresholds rssi; > >>> bool active; > >>> __u16 handle; > >>> }; > >>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h > >>> index d8367850e8cd..dc534837be0e 100644 > >>> --- a/include/net/bluetooth/mgmt.h > >>> +++ b/include/net/bluetooth/mgmt.h > >>> @@ -763,9 +763,18 @@ struct mgmt_adv_pattern { > >>> __u8 value[31]; > >>> } __packed; > >>> > >>> +struct mgmt_adv_rssi_thresholds { > >>> + __s8 high_threshold; > >>> + __le16 high_threshold_timeout; > >>> + __s8 low_threshold; > >>> + __le16 low_threshold_timeout; > >>> + __u8 sampling_period; > >>> +} __packed; > >>> + > >>> #define MGMT_OP_ADD_ADV_PATTERNS_MONITOR 0x0052 > >>> struct mgmt_cp_add_adv_patterns_monitor { > >>> __u8 pattern_count; > >>> + struct mgmt_adv_rssi_thresholds rssi; > >>> struct mgmt_adv_pattern patterns[]; > >>> } __packed; > >> > >> This is something we can not do. It breaks an userspace facing API. Is the > >> mgmt opcode 0x0052 in an already released kernel? > > > > Yes, the opcode does exist in an already released kernel. > > > > The DBus method which accesses this API is put behind the experimental > > flag, therefore we expect they are flexible enough to support changes. > > Previously, we already had a discussion in an email thread with the > > title "Offload RSSI tracking to controller", and the outcome supports > > this change. > > > > Here is an excerpt of the discussion. > > it doesn’t matter. This is fixed API now and so we can not just change it. > The argument above is void. What matters if it is in already released kernel.
If that is the case, do you have a suggestion to allow RSSI to be considered when monitoring advertisement? Would a new MGMT opcode with these parameters suffice? #define MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI 0x005? struct mgmt_cp_add_adv_patterns_monitor { __u8 pattern_count; struct mgmt_adv_rssi_thresholds rssi; struct mgmt_adv_pattern patterns[]; } __packed; Thanks, Archie