Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200218144840.2854-1-sathish.narasimman@intel.com> <F347B999-AB83-4E11-BAD3-7FC08D40AB9B@holtmann.org>
In-Reply-To: <F347B999-AB83-4E11-BAD3-7FC08D40AB9B@holtmann.org>
From:   Sathish Narasimman <nsathish41@gmail.com>
Date:   Mon, 24 Feb 2020 10:55:30 +0530
Message-ID: <CAOVXEJLNS9hK8SxL+12mtvhe7CTPA3fZwYrtY7-x291+s9c45Q@mail.gmail.com>
Subject: Re: [PATCH] Bluetooth: Fix - Remove adv set for directed advertising
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     Bluez mailing list <linux-bluetooth@vger.kernel.org>,
        Sathish Narsimman <sathish.narasimman@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Marcel

On Thu, Feb 20, 2020 at 1:07 PM Marcel Holtmann <marcel@holtmann.org> wrote=
:
>
> Hi Sathish,
>
> > Extended advertising Data is set during bluetooth initialization
> > by default which causes InvalidHCICommandParameters when setting
> > Extended advertising parameters.
> >
> > As per Core Spec 5.2 Vol 2, PART E, Sec 7.8.53, for
> > advertising_event_property LE_LEGACY_ADV_DIRECT_IND does not
> > supports advertising data when the advertising set already
> > contains some, the controller shall return erroc code
> > 'InvalidHCICommandParameters(0x12).
> >
> > So it is required to remove adv set for handle 0x00. since we use
> > instance 0 for directed adv.
> >
> > Signed-off-by: Sathish Narsimman <sathish.narasimman@intel.com>
> > ---
> > include/net/bluetooth/hci.h |  2 ++
> > net/bluetooth/hci_conn.c    | 10 ++++++++++
> > net/bluetooth/hci_request.c |  5 +++++
> > net/bluetooth/hci_request.h |  1 +
> > 4 files changed, 18 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 6293bdd7d862..0d7e36c54733 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -1724,6 +1724,8 @@ struct hci_cp_le_set_ext_scan_rsp_data {
> >
> > #define LE_SET_ADV_DATA_NO_FRAG               0x01
> >
> > +#define HCI_OP_LE_REMOVE_ADV_SET     0x203c
> > +
> > #define HCI_OP_LE_CLEAR_ADV_SETS      0x203d
> >
> > #define HCI_OP_LE_SET_ADV_SET_RAND_ADDR       0x2035
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 65fa44cbe514..1887da39a93d 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -898,6 +898,16 @@ static void hci_req_directed_advertising(struct hc=
i_request *req,
> >               cp.peer_addr_type =3D conn->dst_type;
> >               bacpy(&cp.peer_addr, &conn->dst);
> >
> > +             /* As per Core Spec 5.2 Vol 2, PART E, Sec 7.8.53, for
> > +              * advertising_event_property LE_LEGACY_ADV_DIRECT_IND
> > +              * does not supports advertising data when the advertisin=
g set already
> > +              * contains some, the controller shall return erroc code =
'Invalid
> > +              * HCI Command Parameters(0x12).
> > +              * So it is required to remove adv set for handle 0x00. s=
ince we use
> > +              * instance 0 for directed adv.
> > +              */
> > +             hci_req_add(req, HCI_OP_LE_REMOVE_ADV_SET, sizeof(cp.hand=
le), &cp.handle);
> > +
> >               hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(cp)=
, &cp);
> >
> >               if (own_addr_type =3D=3D ADDR_LE_DEV_RANDOM &&
> > diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> > index 2a1b64dbf76e..63da7acbb48c 100644
> > --- a/net/bluetooth/hci_request.c
> > +++ b/net/bluetooth/hci_request.c
> > @@ -1550,6 +1550,11 @@ int hci_get_random_address(struct hci_dev *hdev,=
 bool require_privacy,
> >       return 0;
> > }
> >
> > +void __hci_req_remove_adv_set(struct hci_request *req, u8 handle)
> > +{
> > +     hci_req_add(req, HCI_OP_LE_REMOVE_ADV_SET, sizeof(handle), &handl=
e);
> > +}
> > +
> > void __hci_req_clear_ext_adv_sets(struct hci_request *req)
> > {
> >       hci_req_add(req, HCI_OP_LE_CLEAR_ADV_SETS, 0, NULL);
> > diff --git a/net/bluetooth/hci_request.h b/net/bluetooth/hci_request.h
> > index a7019fbeadd3..8dd40c6c33c8 100644
> > --- a/net/bluetooth/hci_request.h
> > +++ b/net/bluetooth/hci_request.h
> > @@ -84,6 +84,7 @@ void hci_req_clear_adv_instance(struct hci_dev *hdev,=
 struct sock *sk,
> > int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instan=
ce);
> > int __hci_req_start_ext_adv(struct hci_request *req, u8 instance);
> > int __hci_req_enable_ext_advertising(struct hci_request *req, u8 instan=
ce);
> > +void __hci_req_remove_adv_set(struct hci_request *req, u8 handle);
>
> I don=E2=80=99t get this part of the patch. It is actually not used.
>
> Regards
>
> Marcel
>

it is a mistake. corected and updated the right patch set v2

Regards
Sathish N