Re: [PATCH 2/2] Bluetooth: Send AdvMonitor Dev Found for all matched devices

[Marcel Holtmann <marcel@holtmann.org>]

Hi Manish,

> When an Advertisement Monitor is configured with SamplingPeriod 0xFF,
> the controller reports only one adv report along with the MSFT Monitor
> Device event.
> 
> When an advertiser matches multiple monitors, some controllers send one
> adv report for each matched monitor; whereas, some controllers send just
> one adv report for all matched monitors.
> 
> In such a case, report Adv Monitor Device Found event for each matched
> monitor.
> 
> Signed-off-by: Manish Mandlik <mmandlik@google.com>
> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> ---
> 
> net/bluetooth/mgmt.c | 70 +++++++++++++++++++++++---------------------
> 1 file changed, 37 insertions(+), 33 deletions(-)

patch has been applied to bluetooth-next tree.

> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index d59c70e9166f..e4da2318a2f6 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -9628,17 +9628,44 @@ void mgmt_adv_monitor_device_lost(struct hci_dev *hdev, u16 handle,
> 		   NULL);
> }
> 
> +static void mgmt_send_adv_monitor_device_found(struct hci_dev *hdev,
> +					       struct sk_buff *skb,
> +					       struct sock *skip_sk,
> +					       u16 handle)
> +{
> +	struct sk_buff *advmon_skb;
> +	size_t advmon_skb_len;
> +	__le16 *monitor_handle;
> +
> +	if (!skb)
> +		return;
> +
> +	advmon_skb_len = (sizeof(struct mgmt_ev_adv_monitor_device_found) -
> +			  sizeof(struct mgmt_ev_device_found)) + skb->len;
> +	advmon_skb = mgmt_alloc_skb(hdev, MGMT_EV_ADV_MONITOR_DEVICE_FOUND,
> +				    advmon_skb_len);
> +	if (!advmon_skb)
> +		return;
> +
> +	/* ADV_MONITOR_DEVICE_FOUND is similar to DEVICE_FOUND event except
> +	 * that it also has 'monitor_handle'. Make a copy of DEVICE_FOUND and
> +	 * store monitor_handle of the matched monitor.
> +	 */
> +	monitor_handle = skb_put(advmon_skb, sizeof(*monitor_handle));
> +	*monitor_handle = cpu_to_le16(handle);
> +	skb_put_data(advmon_skb, skb->data, skb->len);
> +
> +	mgmt_event_skb(advmon_skb, skip_sk);
> +}
> +

However, this is rather hackish code. It will blow up in our faces at some point and we will spent weeks to find the bug.

I realized that you already got this pattern merged by Luiz and that is why I merged your patch. I would have not accepted this in the first place. Maybe you need to spent some development cycles and check how all DEVICE_FOUND events can be properly generalized.

Regards

Marcel