2011-12-14 21:51:57

by Johan Hedberg

[permalink] [raw]
Subject: [PATCH 0/7] Update mgmt interface to latest specification

Hi,

This patch set updates the kernel side implementation of the managment
interface to match the latest specification for it. This will break
support with any older user-space versions and you will need to use the
very latest user-space git to test it (where I just a minute ago pushed
matching patches).

Johan



2011-12-16 06:26:29

by Hemant Gupta

[permalink] [raw]
Subject: Re: [PATCH 2/7] Bluetooth: Move mgmt_set_fast_connectable to the right location

Hi Johan,

On Thu, Dec 15, 2011 at 3:21 AM, <[email protected]> wrote:
> From: Johan Hedberg <[email protected]>
>
> Fast connetable is logically after the connectable property so that's
connetable -> connectable

> where it should show up in the code as well (it's also after connectable
> in the settings bitfield).
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> ?include/net/bluetooth/mgmt.h | ? ?7 ++-----
> ?net/bluetooth/mgmt.c ? ? ? ? | ? 12 ++++++------
> ?2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 85e9c6e..bf217cc 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -100,6 +100,8 @@ struct mgmt_cp_set_discoverable {
>
> ?#define MGMT_OP_SET_CONNECTABLE ? ? ? ? ? ? ? ?0x0007
>
> +#define MGMT_OP_SET_FAST_CONNECTABLE ? 0x001F
> +
> ?#define MGMT_OP_SET_PAIRABLE ? ? ? ? ? 0x0008
>
> ?#define MGMT_OP_ADD_UUID ? ? ? ? ? ? ? 0x0009
> @@ -255,11 +257,6 @@ struct mgmt_cp_unblock_device {
> ? ? ? ?bdaddr_t bdaddr;
> ?} __packed;
>
> -#define MGMT_OP_SET_FAST_CONNECTABLE ? 0x001F
> -struct mgmt_cp_set_fast_connectable {
> - ? ? ? __u8 enable;
> -} __packed;
> -
> ?#define MGMT_OP_USER_PASSKEY_REPLY ? ? 0x0020
> ?struct mgmt_cp_user_passkey_reply {
> ? ? ? ?bdaddr_t bdaddr;
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 629570c..54092c2 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -2052,7 +2052,7 @@ static int set_fast_connectable(struct sock *sk, u16 index,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned char *data, u16 len)
> ?{
> ? ? ? ?struct hci_dev *hdev;
> - ? ? ? struct mgmt_cp_set_fast_connectable *cp = (void *) data;
> + ? ? ? struct mgmt_mode *cp = (void *) data;
> ? ? ? ?struct hci_cp_write_page_scan_activity acp;
> ? ? ? ?u8 type;
> ? ? ? ?int err;
> @@ -2070,7 +2070,7 @@ static int set_fast_connectable(struct sock *sk, u16 index,
>
> ? ? ? ?hci_dev_lock(hdev);
>
> - ? ? ? if (cp->enable) {
> + ? ? ? if (cp->val) {
> ? ? ? ? ? ? ? ?type = PAGE_SCAN_TYPE_INTERLACED;
> ? ? ? ? ? ? ? ?acp.interval = 0x0024; ?/* 22.5 msec page scan interval */
> ? ? ? ?} else {
> @@ -2154,6 +2154,10 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
> ? ? ? ?case MGMT_OP_SET_CONNECTABLE:
> ? ? ? ? ? ? ? ?err = set_connectable(sk, index, buf + sizeof(*hdr), len);
> ? ? ? ? ? ? ? ?break;
> + ? ? ? case MGMT_OP_SET_FAST_CONNECTABLE:
> + ? ? ? ? ? ? ? err = set_fast_connectable(sk, index, buf + sizeof(*hdr),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? len);
> + ? ? ? ? ? ? ? break;
> ? ? ? ?case MGMT_OP_SET_PAIRABLE:
> ? ? ? ? ? ? ? ?err = set_pairable(sk, index, buf + sizeof(*hdr), len);
> ? ? ? ? ? ? ? ?break;
> @@ -2232,10 +2236,6 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
> ? ? ? ?case MGMT_OP_UNBLOCK_DEVICE:
> ? ? ? ? ? ? ? ?err = unblock_device(sk, index, buf + sizeof(*hdr), len);
> ? ? ? ? ? ? ? ?break;
> - ? ? ? case MGMT_OP_SET_FAST_CONNECTABLE:
> - ? ? ? ? ? ? ? err = set_fast_connectable(sk, index, buf + sizeof(*hdr),
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? len);
> - ? ? ? ? ? ? ? break;
> ? ? ? ?default:
> ? ? ? ? ? ? ? ?BT_DBG("Unknown op %u", opcode);
> ? ? ? ? ? ? ? ?err = cmd_status(sk, index, opcode,
> --
> 1.7.7.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html



--
Best Regards
Hemant Gupta
ST-Ericsson India

2011-12-15 09:31:09

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 1/7] Bluetooth: Update mgmt_read_info and related mgmt messages

Hi Andrei,

On Thu, Dec 15, 2011, Andrei Emeltchenko wrote:
> > +#define MGMT_SETTING_POWERED 0x00000001
> > +#define MGMT_SETTING_CONNECTABLE 0x00000002
> > +#define MGMT_SETTING_FAST_CONNECTABLE 0x00000004
> > +#define MGMT_SETTING_DISCOVERABLE 0x00000008
> > +#define MGMT_SETTING_PAIRABLE 0x00000010
> > +#define MGMT_SETTING_LINK_SECURITY 0x00000020
> > +#define MGMT_SETTING_SSP 0x00000040
> > +#define MGMT_SETTING_BREDR 0x00000080
> > +#define MGMT_SETTING_HS 0x00000100
> > +#define MGMT_SETTING_LE 0x00000200
>
> Just a minor comment.
> Can we use set_bit style for the defines above?

I was considering it, but the set_bit/test_bit functions expect pointers
to unsigned long whereas we're dealing with a fixed-size u32 in the mgmt
protocol. In the best case that'd require adding type-casts to avoid
compiler warnings/errors and in the worst case it could even cause bugs
on some architectures.

Johan

2011-12-15 09:02:47

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH 1/7] Bluetooth: Update mgmt_read_info and related mgmt messages

Hi Johan,

On Wed, Dec 14, 2011 at 11:51:58PM +0200, [email protected] wrote:
> From: Johan Hedberg <[email protected]>
>
> This patch updates the mgmt_read_info and related messages to the latest
> management API which uses a bitfield of settings instead of individual
> boolean values.
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> include/net/bluetooth/hci.h | 1 +
> include/net/bluetooth/mgmt.h | 29 +++++---
> net/bluetooth/mgmt.c | 146 +++++++++++++++++++++++++++---------------
> 3 files changed, 113 insertions(+), 63 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 67ad984..c9ad56f 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -210,6 +210,7 @@ enum {
>
> #define LMP_EV4 0x01
> #define LMP_EV5 0x02
> +#define LMP_NO_BREDR 0x20
> #define LMP_LE 0x40
>
> #define LMP_SNIFF_SUBR 0x02
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 3b68806..85e9c6e 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -61,22 +61,29 @@ struct mgmt_rp_read_index_list {
> /* Reserve one extra byte for names in management messages so that they
> * are always guaranteed to be nul-terminated */
> #define MGMT_MAX_NAME_LENGTH (HCI_MAX_NAME_LENGTH + 1)
> +#define MGMT_MAX_SHORT_NAME_LENGTH (10 + 1)
> +
> +#define MGMT_SETTING_POWERED 0x00000001
> +#define MGMT_SETTING_CONNECTABLE 0x00000002
> +#define MGMT_SETTING_FAST_CONNECTABLE 0x00000004
> +#define MGMT_SETTING_DISCOVERABLE 0x00000008
> +#define MGMT_SETTING_PAIRABLE 0x00000010
> +#define MGMT_SETTING_LINK_SECURITY 0x00000020
> +#define MGMT_SETTING_SSP 0x00000040
> +#define MGMT_SETTING_BREDR 0x00000080
> +#define MGMT_SETTING_HS 0x00000100
> +#define MGMT_SETTING_LE 0x00000200

Just a minor comment.
Can we use set_bit style for the defines above?

Best regards
Andrei Emeltchenko

>
> #define MGMT_OP_READ_INFO 0x0004
> struct mgmt_rp_read_info {
> - __u8 type;
> - __u8 powered;
> - __u8 connectable;
> - __u8 discoverable;
> - __u8 pairable;
> - __u8 sec_mode;
> bdaddr_t bdaddr;
> + __u8 version;
> + __le16 manufacturer;
> + __le32 supported_settings;
> + __le32 current_settings;
> __u8 dev_class[3];
> - __u8 features[8];
> - __u16 manufacturer;
> - __u8 hci_ver;
> - __u16 hci_rev;
> __u8 name[MGMT_MAX_NAME_LENGTH];
> + __u8 short_name[MGMT_MAX_SHORT_NAME_LENGTH];
> } __packed;
>
> struct mgmt_mode {
> @@ -285,7 +292,7 @@ struct mgmt_ev_controller_error {
>
> #define MGMT_EV_INDEX_REMOVED 0x0005
>
> -#define MGMT_EV_POWERED 0x0006
> +#define MGMT_EV_NEW_SETTINGS 0x0006
>
> #define MGMT_EV_DISCOVERABLE 0x0007
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 7a23f21..629570c 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -242,6 +242,63 @@ static int read_index_list(struct sock *sk)
> return err;
> }
>
> +static u32 get_supported_settings(struct hci_dev *hdev)
> +{
> + u32 settings = 0;
> +
> + settings |= MGMT_SETTING_POWERED;
> + settings |= MGMT_SETTING_CONNECTABLE;
> + settings |= MGMT_SETTING_FAST_CONNECTABLE;
> + settings |= MGMT_SETTING_DISCOVERABLE;
> + settings |= MGMT_SETTING_PAIRABLE;
> +
> + if (hdev->features[6] & LMP_SIMPLE_PAIR)
> + settings |= MGMT_SETTING_SSP;
> +
> + if (!(hdev->features[4] & LMP_NO_BREDR)) {
> + settings |= MGMT_SETTING_BREDR;
> + settings |= MGMT_SETTING_LINK_SECURITY;
> + }
> +
> + if (hdev->features[4] & LMP_LE)
> + settings |= MGMT_SETTING_LE;
> +
> + return settings;
> +}
> +
> +static u32 get_current_settings(struct hci_dev *hdev)
> +{
> + u32 settings = 0;
> +
> + if (test_bit(HCI_UP, &hdev->flags))
> + settings |= MGMT_SETTING_POWERED;
> + else
> + return settings;
> +
> + if (test_bit(HCI_PSCAN, &hdev->flags))
> + settings |= MGMT_SETTING_CONNECTABLE;
> +
> + if (test_bit(HCI_ISCAN, &hdev->flags))
> + settings |= MGMT_SETTING_DISCOVERABLE;
> +
> + if (test_bit(HCI_PAIRABLE, &hdev->flags))
> + settings |= MGMT_SETTING_PAIRABLE;
> +
> + if (!(hdev->features[4] & LMP_NO_BREDR))
> + settings |= MGMT_SETTING_BREDR;
> +
> + if (hdev->extfeatures[0] & LMP_HOST_LE)
> + settings |= MGMT_SETTING_LE;
> +
> + if (test_bit(HCI_AUTH, &hdev->flags))
> + settings |= MGMT_SETTING_LINK_SECURITY;
> +
> + if (hdev->ssp_mode > 0)
> + settings |= MGMT_SETTING_SSP;
> +
> + return settings;
> +}
> +
> static int read_controller_info(struct sock *sk, u16 index)
> {
> struct mgmt_rp_read_info rp;
> @@ -263,26 +320,16 @@ static int read_controller_info(struct sock *sk, u16 index)
>
> memset(&rp, 0, sizeof(rp));
>
> - rp.type = hdev->dev_type;
> + bacpy(&rp.bdaddr, &hdev->bdaddr);
>
> - rp.powered = test_bit(HCI_UP, &hdev->flags);
> - rp.connectable = test_bit(HCI_PSCAN, &hdev->flags);
> - rp.discoverable = test_bit(HCI_ISCAN, &hdev->flags);
> - rp.pairable = test_bit(HCI_PSCAN, &hdev->flags);
> + rp.version = hdev->hci_ver;
>
> - if (test_bit(HCI_AUTH, &hdev->flags))
> - rp.sec_mode = 3;
> - else if (hdev->ssp_mode > 0)
> - rp.sec_mode = 4;
> - else
> - rp.sec_mode = 2;
> + put_unaligned_le16(hdev->manufacturer, &rp.manufacturer);
> +
> + rp.supported_settings = cpu_to_le32(get_supported_settings(hdev));
> + rp.current_settings = cpu_to_le32(get_current_settings(hdev));
>
> - bacpy(&rp.bdaddr, &hdev->bdaddr);
> - memcpy(rp.features, hdev->features, 8);
> memcpy(rp.dev_class, hdev->dev_class, 3);
> - put_unaligned_le16(hdev->manufacturer, &rp.manufacturer);
> - rp.hci_ver = hdev->hci_ver;
> - put_unaligned_le16(hdev->hci_rev, &rp.hci_rev);
>
> memcpy(rp.name, hdev->dev_name, sizeof(hdev->dev_name));
>
> @@ -365,13 +412,11 @@ static void mgmt_pending_remove(struct pending_cmd *cmd)
> mgmt_pending_free(cmd);
> }
>
> -static int send_mode_rsp(struct sock *sk, u16 opcode, u16 index, u8 val)
> +static int send_settings_rsp(struct sock *sk, u16 opcode, struct hci_dev *hdev)
> {
> - struct mgmt_mode rp;
> + __le32 settings = cpu_to_le32(get_current_settings(hdev));
>
> - rp.val = val;
> -
> - return cmd_complete(sk, index, opcode, &rp, sizeof(rp));
> + return cmd_complete(sk, hdev->id, opcode, &settings, sizeof(settings));
> }
>
> static int set_powered(struct sock *sk, u16 index, unsigned char *data, u16 len)
> @@ -398,7 +443,7 @@ static int set_powered(struct sock *sk, u16 index, unsigned char *data, u16 len)
>
> up = test_bit(HCI_UP, &hdev->flags);
> if ((cp->val && up) || (!cp->val && !up)) {
> - err = send_mode_rsp(sk, index, MGMT_OP_SET_POWERED, cp->val);
> + err = send_settings_rsp(sk, MGMT_OP_SET_POWERED, hdev);
> goto failed;
> }
>
> @@ -466,8 +511,7 @@ static int set_discoverable(struct sock *sk, u16 index, unsigned char *data,
>
> if (cp->val == test_bit(HCI_ISCAN, &hdev->flags) &&
> test_bit(HCI_PSCAN, &hdev->flags)) {
> - err = send_mode_rsp(sk, index, MGMT_OP_SET_DISCOVERABLE,
> - cp->val);
> + err = send_settings_rsp(sk, MGMT_OP_SET_DISCOVERABLE, hdev);
> goto failed;
> }
>
> @@ -536,8 +580,7 @@ static int set_connectable(struct sock *sk, u16 index, unsigned char *data,
> }
>
> if (cp->val == test_bit(HCI_PSCAN, &hdev->flags)) {
> - err = send_mode_rsp(sk, index, MGMT_OP_SET_CONNECTABLE,
> - cp->val);
> + err = send_settings_rsp(sk, MGMT_OP_SET_CONNECTABLE, hdev);
> goto failed;
> }
>
> @@ -595,8 +638,9 @@ static int mgmt_event(u16 event, struct hci_dev *hdev, void *data,
> static int set_pairable(struct sock *sk, u16 index, unsigned char *data,
> u16 len)
> {
> - struct mgmt_mode *cp, ev;
> + struct mgmt_mode *cp;
> struct hci_dev *hdev;
> + __le32 ev;
> int err;
>
> cp = (void *) data;
> @@ -619,13 +663,13 @@ static int set_pairable(struct sock *sk, u16 index, unsigned char *data,
> else
> clear_bit(HCI_PAIRABLE, &hdev->flags);
>
> - err = send_mode_rsp(sk, MGMT_OP_SET_PAIRABLE, index, cp->val);
> + err = send_settings_rsp(sk, MGMT_OP_SET_PAIRABLE, hdev);
> if (err < 0)
> goto failed;
>
> - ev.val = cp->val;
> + ev = cpu_to_le32(get_current_settings(hdev));
>
> - err = mgmt_event(MGMT_EV_PAIRABLE, hdev, &ev, sizeof(ev), sk);
> + err = mgmt_event(MGMT_EV_NEW_SETTINGS, hdev, &ev, sizeof(ev), sk);
>
> failed:
> hci_dev_unlock_bh(hdev);
> @@ -2234,17 +2278,14 @@ int mgmt_index_removed(struct hci_dev *hdev)
> struct cmd_lookup {
> u8 val;
> struct sock *sk;
> + struct hci_dev *hdev;
> };
>
> -static void mode_rsp(struct pending_cmd *cmd, void *data)
> +static void settings_rsp(struct pending_cmd *cmd, void *data)
> {
> - struct mgmt_mode *cp = cmd->param;
> struct cmd_lookup *match = data;
>
> - if (cp->val != match->val)
> - return;
> -
> - send_mode_rsp(cmd->sk, cmd->opcode, cmd->index, cp->val);
> + send_settings_rsp(cmd->sk, cmd->opcode, match->hdev);
>
> list_del(&cmd->list);
>
> @@ -2258,20 +2299,21 @@ static void mode_rsp(struct pending_cmd *cmd, void *data)
>
> int mgmt_powered(struct hci_dev *hdev, u8 powered)
> {
> - struct mgmt_mode ev;
> - struct cmd_lookup match = { powered, NULL };
> + struct cmd_lookup match = { powered, NULL, hdev };
> + __le32 ev;
> int ret;
>
> - mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, mode_rsp, &match);
> + mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp, &match);
>
> if (!powered) {
> u8 status = ENETDOWN;
> mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status);
> }
>
> - ev.val = powered;
> + ev = cpu_to_le32(get_current_settings(hdev));
>
> - ret = mgmt_event(MGMT_EV_POWERED, hdev, &ev, sizeof(ev), match.sk);
> + ret = mgmt_event(MGMT_EV_NEW_SETTINGS, hdev, &ev, sizeof(ev),
> + match.sk);
>
> if (match.sk)
> sock_put(match.sk);
> @@ -2281,17 +2323,16 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
>
> int mgmt_discoverable(struct hci_dev *hdev, u8 discoverable)
> {
> - struct mgmt_mode ev;
> - struct cmd_lookup match = { discoverable, NULL };
> + struct cmd_lookup match = { discoverable, NULL, hdev };
> + __le32 ev;
> int ret;
>
> - mgmt_pending_foreach(MGMT_OP_SET_DISCOVERABLE, hdev, mode_rsp, &match);
> + mgmt_pending_foreach(MGMT_OP_SET_DISCOVERABLE, hdev, settings_rsp, &match);
>
> - ev.val = discoverable;
> + ev = cpu_to_le32(get_current_settings(hdev));
>
> - ret = mgmt_event(MGMT_EV_DISCOVERABLE, hdev, &ev, sizeof(ev),
> + ret = mgmt_event(MGMT_EV_NEW_SETTINGS, hdev, &ev, sizeof(ev),
> match.sk);
> -
> if (match.sk)
> sock_put(match.sk);
>
> @@ -2300,15 +2341,16 @@ int mgmt_discoverable(struct hci_dev *hdev, u8 discoverable)
>
> int mgmt_connectable(struct hci_dev *hdev, u8 connectable)
> {
> - struct mgmt_mode ev;
> - struct cmd_lookup match = { connectable, NULL };
> + __le32 ev;
> + struct cmd_lookup match = { connectable, NULL, hdev };
> int ret;
>
> - mgmt_pending_foreach(MGMT_OP_SET_CONNECTABLE, hdev, mode_rsp, &match);
> + mgmt_pending_foreach(MGMT_OP_SET_CONNECTABLE, hdev, settings_rsp,
> + &match);
>
> - ev.val = connectable;
> + ev = cpu_to_le32(get_current_settings(hdev));
>
> - ret = mgmt_event(MGMT_EV_CONNECTABLE, hdev, &ev, sizeof(ev), match.sk);
> + ret = mgmt_event(MGMT_EV_NEW_SETTINGS, hdev, &ev, sizeof(ev), match.sk);
>
> if (match.sk)
> sock_put(match.sk);
> --
> 1.7.7.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2011-12-14 23:12:20

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 5/7] Bluetooth: Add timer for automatically disabling the service cache

Hi Johan,

> > > - if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags))
> > > + if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags)) {
> > > + hci_dev_unlock_bh(hdev);
> > > + cancel_delayed_work_sync(&hdev->service_cache);
> > > + hci_dev_lock_bh(hdev);
> > > update_eir(hdev);
> > > + }
> >
> > We have to be a bit careful here since essentially the service cache
> > (actually UUID cache to be precise) is a per mgmt socket feature and not
> > a per hdev feature.
> >
> > I have nothing against merging this first, but keep in mind that this
> > needs to be updated since we can have two concurrent applications
> > opening a mgmt socket.
>
> I understand. It does get a bit tricky though since the UUID list is per
> HCI device. I.e. do we keep our own list for each socket and hide it
> from struct hci_dev until the (per-socket) cache gets disabled?

that might be a good idea. Every mgmt socket should keep its list of
added UUIDs and then maybe just call an update function that runs over
all mgmt sockets and picks the union of it. If a socket closes, the list
gets flushed and we update again. That way we do not have to play global
reference counting tricks.

Regards

Marcel



2011-12-14 22:25:37

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 5/7] Bluetooth: Add timer for automatically disabling the service cache

Hi Marcel,

On Wed, Dec 14, 2011, Marcel Holtmann wrote:
> > +static void service_cache_off(struct work_struct *work)
> > +{
> > + struct hci_dev *hdev = container_of(work, struct hci_dev,
> > + service_cache.work);
> > +
> > + if (!test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags))
> > + return;
> > +
> > + hci_dev_lock_bh(hdev);
> > +
> > + update_eir(hdev);
> > + update_class(hdev);
> > +
> > + hci_dev_unlock_bh(hdev);
> > +}
> > +
> > +static void mgmt_init_hdev(struct hci_dev *hdev)
> > +{
> > + if (!test_and_set_bit(HCI_MGMT, &hdev->flags))
> > + INIT_DELAYED_WORK(&hdev->service_cache, service_cache_off);
>
> Why does this depend on MGMT being enabled? Why do we bother here? Why
> not just always initialize it when registering the controller.

Because the timer callback function is static and resides within mgmt.c
(since it needs to call update_class & update_eir which are also static
within mgmt.c).

> > - if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags))
> > + if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags)) {
> > + hci_dev_unlock_bh(hdev);
> > + cancel_delayed_work_sync(&hdev->service_cache);
> > + hci_dev_lock_bh(hdev);
> > update_eir(hdev);
> > + }
>
> We have to be a bit careful here since essentially the service cache
> (actually UUID cache to be precise) is a per mgmt socket feature and not
> a per hdev feature.
>
> I have nothing against merging this first, but keep in mind that this
> needs to be updated since we can have two concurrent applications
> opening a mgmt socket.

I understand. It does get a bit tricky though since the UUID list is per
HCI device. I.e. do we keep our own list for each socket and hide it
from struct hci_dev until the (per-socket) cache gets disabled?

Johan

2011-12-14 22:17:40

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 7/7] Bluetooth: Use correct struct for user_confirm_neg_reply

Hi Johan,

> This patch fixes user_confirm_neg_reply to use the appropriate struct
> for accessing the call parameters.
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> net/bluetooth/mgmt.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 4d531b9..054229f 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1690,7 +1690,7 @@ static int user_confirm_reply(struct sock *sk, u16 index, void *data, u16 len)
> static int user_confirm_neg_reply(struct sock *sk, u16 index, void *data,
> u16 len)
> {
> - struct mgmt_cp_user_confirm_reply *cp = (void *) data;
> + struct mgmt_cp_user_confirm_neg_reply *cp = (void *) data;

the cast here seems pointless. Just remove that altogether. Otherwise
fine with me.

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2011-12-14 22:16:38

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 6/7] Bluetooth: Update ordering and opcodes of mgmt messages

Hi Johan,

> This patch updates the ordering and opcodes of mgmt messages to match
> the latest API specification.
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> include/net/bluetooth/mgmt.h | 147 +++++++++++++++++++++++-------------------
> 1 files changed, 81 insertions(+), 66 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2011-12-14 22:16:04

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 5/7] Bluetooth: Add timer for automatically disabling the service cache

Hi Johan,

> We do not want the service cache to be enabled indefinitely after
> mgmt_read_info is called. The solve this a timer is added which will
> automaticall disable the cache if mgmt_set_dev_class isn't called within

it is "automatically".

> 5 seconds of calling mgmt_read_info.
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 2 +
> net/bluetooth/hci_core.c | 3 ++
> net/bluetooth/mgmt.c | 40 +++++++++++++++++++++++++++++++++----
> 3 files changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 4e67bb8..a5d7e42 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -200,6 +200,8 @@ struct hci_dev {
> __u16 discov_timeout;
> struct delayed_work discov_off;
>
> + struct delayed_work service_cache;
> +
> struct timer_list cmd_timer;
> struct tasklet_struct cmd_task;
> struct tasklet_struct rx_task;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index ce3727e..323be52 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -597,6 +597,9 @@ static int hci_dev_do_close(struct hci_dev *hdev)
> if (test_and_clear_bit(HCI_AUTO_OFF, &hdev->flags))
> cancel_delayed_work(&hdev->power_off);
>
> + if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags))
> + cancel_delayed_work(&hdev->service_cache);
> +
> hci_dev_lock_bh(hdev);
> inquiry_cache_flush(hdev);
> hci_conn_hash_flush(hdev);
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 696e585..4d531b9 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -35,6 +35,8 @@
>
> #define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
>
> +#define SERVICE_CACHE_TIMEOUT (5 * 1000)
> +
> struct pending_cmd {
> struct list_head list;
> u16 opcode;
> @@ -472,6 +474,32 @@ static int update_class(struct hci_dev *hdev)
> return hci_send_cmd(hdev, HCI_OP_WRITE_CLASS_OF_DEV, sizeof(cod), cod);
> }
>
> +static void service_cache_off(struct work_struct *work)
> +{
> + struct hci_dev *hdev = container_of(work, struct hci_dev,
> + service_cache.work);
> +
> + if (!test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags))
> + return;
> +
> + hci_dev_lock_bh(hdev);
> +
> + update_eir(hdev);
> + update_class(hdev);
> +
> + hci_dev_unlock_bh(hdev);
> +}
> +
> +static void mgmt_init_hdev(struct hci_dev *hdev)
> +{
> + if (!test_and_set_bit(HCI_MGMT, &hdev->flags))
> + INIT_DELAYED_WORK(&hdev->service_cache, service_cache_off);

Why does this depend on MGMT being enabled? Why do we bother here? Why
not just always initialize it when registering the controller.

> + if (!test_and_set_bit(HCI_SERVICE_CACHE, &hdev->flags))
> + queue_delayed_work(hdev->workqueue, &hdev->service_cache,
> + msecs_to_jiffies(SERVICE_CACHE_TIMEOUT));
> +}
> +
> static int read_controller_info(struct sock *sk, u16 index)
> {
> struct mgmt_rp_read_info rp;
> @@ -489,10 +517,8 @@ static int read_controller_info(struct sock *sk, u16 index)
>
> hci_dev_lock_bh(hdev);
>
> - if (test_and_clear_bit(HCI_PI_MGMT_INIT, &hci_pi(sk)->flags)) {
> - set_bit(HCI_MGMT, &hdev->flags);
> - set_bit(HCI_SERVICE_CACHE, &hdev->flags);
> - }
> + if (test_and_clear_bit(HCI_PI_MGMT_INIT, &hci_pi(sk)->flags))
> + mgmt_init_hdev(hdev);
>
> memset(&rp, 0, sizeof(rp));
>
> @@ -992,8 +1018,12 @@ static int set_dev_class(struct sock *sk, u16 index, unsigned char *data,
> hdev->major_class = cp->major;
> hdev->minor_class = cp->minor;
>
> - if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags))
> + if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags)) {
> + hci_dev_unlock_bh(hdev);
> + cancel_delayed_work_sync(&hdev->service_cache);
> + hci_dev_lock_bh(hdev);
> update_eir(hdev);
> + }

We have to be a bit careful here since essentially the service cache
(actually UUID cache to be precise) is a per mgmt socket feature and not
a per hdev feature.

I have nothing against merging this first, but keep in mind that this
needs to be updated since we can have two concurrent applications
opening a mgmt socket.

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2011-12-14 22:09:48

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 4/7] Bluetooth: Move EIR and CoD update functions to a better position

Hi Johan,

> Due to the upcoming addition of a service cache timer the functions to
> update the EIR and CoD need to be higher up in mgmt.c in order to avoid
> unnecessary forward-declarations. This patch simply moves code around
> without any other changes in order to make subsequent patches more
> readable.
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> net/bluetooth/mgmt.c | 346 +++++++++++++++++++++++++-------------------------
> 1 files changed, 173 insertions(+), 173 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2011-12-14 22:08:58

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 3/7] Bluetooth: Remove mgmt_set_service_cache

Hi Johan,

> Instead of having an explicit service cache command we can make the mgmt
> API simpler by implicitly enabling the cache when mgmt_read_info is
> called for the first time and disabiling it when mgmt_set_dev_class is

it is "disabling".

> called.
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 4 +++
> include/net/bluetooth/mgmt.h | 5 ---
> net/bluetooth/hci_sock.c | 7 +++-
> net/bluetooth/mgmt.c | 56 +++++---------------------------------
> 4 files changed, 16 insertions(+), 56 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2011-12-14 22:06:01

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/7] Bluetooth: Move mgmt_set_fast_connectable to the right location

Hi Johan,

> Fast connetable is logically after the connectable property so that's
> where it should show up in the code as well (it's also after connectable
> in the settings bitfield).
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> include/net/bluetooth/mgmt.h | 7 ++-----
> net/bluetooth/mgmt.c | 12 ++++++------
> 2 files changed, 8 insertions(+), 11 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

However changing the hci_dev_lock in hci_dev_lock_bh would have been
nice as well, but that becomes obsolete anyway when moving to a
workqueue. So not worth bothering.

Regards

Marcel



2011-12-14 22:04:24

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/7] Bluetooth: Update mgmt_read_info and related mgmt messages

Hi Johan,

> This patch updates the mgmt_read_info and related messages to the latest
> management API which uses a bitfield of settings instead of individual
> boolean values.
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> include/net/bluetooth/hci.h | 1 +
> include/net/bluetooth/mgmt.h | 29 +++++---
> net/bluetooth/mgmt.c | 146 +++++++++++++++++++++++++++---------------
> 3 files changed, 113 insertions(+), 63 deletions(-)

I would have split the hci.h patch into a separate one, but since it is
so tiny, this is fine with me as well.

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2011-12-14 21:52:04

by Johan Hedberg

[permalink] [raw]
Subject: [PATCH 7/7] Bluetooth: Use correct struct for user_confirm_neg_reply

From: Johan Hedberg <[email protected]>

This patch fixes user_confirm_neg_reply to use the appropriate struct
for accessing the call parameters.

Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/mgmt.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 4d531b9..054229f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1690,7 +1690,7 @@ static int user_confirm_reply(struct sock *sk, u16 index, void *data, u16 len)
static int user_confirm_neg_reply(struct sock *sk, u16 index, void *data,
u16 len)
{
- struct mgmt_cp_user_confirm_reply *cp = (void *) data;
+ struct mgmt_cp_user_confirm_neg_reply *cp = (void *) data;

BT_DBG("");

--
1.7.7.3


2011-12-14 21:52:03

by Johan Hedberg

[permalink] [raw]
Subject: [PATCH 6/7] Bluetooth: Update ordering and opcodes of mgmt messages

From: Johan Hedberg <[email protected]>

This patch updates the ordering and opcodes of mgmt messages to match
the latest API specification.

Signed-off-by: Johan Hedberg <[email protected]>
---
include/net/bluetooth/mgmt.h | 147 +++++++++++++++++++++++-------------------
1 files changed, 81 insertions(+), 66 deletions(-)

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index bdb0a58..2b1059d 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -100,27 +100,40 @@ struct mgmt_cp_set_discoverable {

#define MGMT_OP_SET_CONNECTABLE 0x0007

-#define MGMT_OP_SET_FAST_CONNECTABLE 0x001F
+#define MGMT_OP_SET_FAST_CONNECTABLE 0x0008

-#define MGMT_OP_SET_PAIRABLE 0x0008
+#define MGMT_OP_SET_PAIRABLE 0x0009

-#define MGMT_OP_ADD_UUID 0x0009
+#define MGMT_OP_SET_LINK_SECURITY 0x000A
+
+#define MGMT_OP_SET_SSP 0x000B
+
+#define MGMT_OP_SET_HS 0x000C
+
+#define MGMT_OP_SET_LE 0x000D
+
+#define MGMT_OP_SET_DEV_CLASS 0x000E
+struct mgmt_cp_set_dev_class {
+ __u8 major;
+ __u8 minor;
+} __packed;
+
+#define MGMT_OP_SET_LOCAL_NAME 0x000F
+struct mgmt_cp_set_local_name {
+ __u8 name[MGMT_MAX_NAME_LENGTH];
+} __packed;
+
+#define MGMT_OP_ADD_UUID 0x0010
struct mgmt_cp_add_uuid {
__u8 uuid[16];
__u8 svc_hint;
} __packed;

-#define MGMT_OP_REMOVE_UUID 0x000A
+#define MGMT_OP_REMOVE_UUID 0x0011
struct mgmt_cp_remove_uuid {
__u8 uuid[16];
} __packed;

-#define MGMT_OP_SET_DEV_CLASS 0x000B
-struct mgmt_cp_set_dev_class {
- __u8 major;
- __u8 minor;
-} __packed;
-
struct mgmt_link_key_info {
bdaddr_t bdaddr;
u8 type;
@@ -128,14 +141,14 @@ struct mgmt_link_key_info {
u8 pin_len;
} __packed;

-#define MGMT_OP_LOAD_LINK_KEYS 0x000D
+#define MGMT_OP_LOAD_LINK_KEYS 0x0012
struct mgmt_cp_load_link_keys {
__u8 debug_keys;
__le16 key_count;
struct mgmt_link_key_info keys[0];
} __packed;

-#define MGMT_OP_REMOVE_KEYS 0x000E
+#define MGMT_OP_REMOVE_KEYS 0x0013
struct mgmt_cp_remove_keys {
bdaddr_t bdaddr;
__u8 disconnect;
@@ -145,7 +158,7 @@ struct mgmt_rp_remove_keys {
__u8 status;
};

-#define MGMT_OP_DISCONNECT 0x000F
+#define MGMT_OP_DISCONNECT 0x0014
struct mgmt_cp_disconnect {
bdaddr_t bdaddr;
} __packed;
@@ -164,13 +177,13 @@ struct mgmt_addr_info {
__u8 type;
} __packed;

-#define MGMT_OP_GET_CONNECTIONS 0x0010
+#define MGMT_OP_GET_CONNECTIONS 0x0015
struct mgmt_rp_get_connections {
__le16 conn_count;
struct mgmt_addr_info addr[0];
} __packed;

-#define MGMT_OP_PIN_CODE_REPLY 0x0011
+#define MGMT_OP_PIN_CODE_REPLY 0x0016
struct mgmt_cp_pin_code_reply {
bdaddr_t bdaddr;
__u8 pin_len;
@@ -181,17 +194,17 @@ struct mgmt_rp_pin_code_reply {
uint8_t status;
} __packed;

-#define MGMT_OP_PIN_CODE_NEG_REPLY 0x0012
+#define MGMT_OP_PIN_CODE_NEG_REPLY 0x0017
struct mgmt_cp_pin_code_neg_reply {
bdaddr_t bdaddr;
} __packed;

-#define MGMT_OP_SET_IO_CAPABILITY 0x0013
+#define MGMT_OP_SET_IO_CAPABILITY 0x0018
struct mgmt_cp_set_io_capability {
__u8 io_capability;
} __packed;

-#define MGMT_OP_PAIR_DEVICE 0x0014
+#define MGMT_OP_PAIR_DEVICE 0x0019
struct mgmt_cp_pair_device {
struct mgmt_addr_info addr;
__u8 io_cap;
@@ -201,7 +214,7 @@ struct mgmt_rp_pair_device {
__u8 status;
} __packed;

-#define MGMT_OP_USER_CONFIRM_REPLY 0x0015
+#define MGMT_OP_USER_CONFIRM_REPLY 0x001A
struct mgmt_cp_user_confirm_reply {
bdaddr_t bdaddr;
} __packed;
@@ -210,59 +223,61 @@ struct mgmt_rp_user_confirm_reply {
__u8 status;
} __packed;

-#define MGMT_OP_USER_CONFIRM_NEG_REPLY 0x0016
+#define MGMT_OP_USER_CONFIRM_NEG_REPLY 0x001B
+struct mgmt_cp_user_confirm_neg_reply {
+ bdaddr_t bdaddr;
+} __packed;

-#define MGMT_OP_SET_LOCAL_NAME 0x0017
-struct mgmt_cp_set_local_name {
- __u8 name[MGMT_MAX_NAME_LENGTH];
+#define MGMT_OP_USER_PASSKEY_REPLY 0x001C
+struct mgmt_cp_user_passkey_reply {
+ bdaddr_t bdaddr;
+ __le32 passkey;
+} __packed;
+struct mgmt_rp_user_passkey_reply {
+ bdaddr_t bdaddr;
+ __u8 status;
+} __packed;
+
+#define MGMT_OP_USER_PASSKEY_NEG_REPLY 0x001D
+struct mgmt_cp_user_passkey_neg_reply {
+ bdaddr_t bdaddr;
} __packed;

-#define MGMT_OP_READ_LOCAL_OOB_DATA 0x0018
+#define MGMT_OP_READ_LOCAL_OOB_DATA 0x001E
struct mgmt_rp_read_local_oob_data {
__u8 hash[16];
__u8 randomizer[16];
} __packed;

-#define MGMT_OP_ADD_REMOTE_OOB_DATA 0x0019
+#define MGMT_OP_ADD_REMOTE_OOB_DATA 0x001F
struct mgmt_cp_add_remote_oob_data {
bdaddr_t bdaddr;
__u8 hash[16];
__u8 randomizer[16];
} __packed;

-#define MGMT_OP_REMOVE_REMOTE_OOB_DATA 0x001A
+#define MGMT_OP_REMOVE_REMOTE_OOB_DATA 0x0020
struct mgmt_cp_remove_remote_oob_data {
bdaddr_t bdaddr;
} __packed;

-#define MGMT_OP_START_DISCOVERY 0x001B
+#define MGMT_OP_START_DISCOVERY 0x0021
struct mgmt_cp_start_discovery {
__u8 type;
} __packed;

-#define MGMT_OP_STOP_DISCOVERY 0x001C
+#define MGMT_OP_STOP_DISCOVERY 0x0022

-#define MGMT_OP_BLOCK_DEVICE 0x001D
+#define MGMT_OP_BLOCK_DEVICE 0x0023
struct mgmt_cp_block_device {
bdaddr_t bdaddr;
} __packed;

-#define MGMT_OP_UNBLOCK_DEVICE 0x001E
+#define MGMT_OP_UNBLOCK_DEVICE 0x0024
struct mgmt_cp_unblock_device {
bdaddr_t bdaddr;
} __packed;

-#define MGMT_OP_USER_PASSKEY_REPLY 0x0020
-struct mgmt_cp_user_passkey_reply {
- bdaddr_t bdaddr;
- __le32 passkey;
-} __packed;
-
-#define MGMT_OP_USER_PASSKEY_NEG_REPLY 0x0021
-struct mgmt_cp_user_passkey_neg_reply {
- bdaddr_t bdaddr;
-} __packed;
-
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
@@ -286,53 +301,58 @@ struct mgmt_ev_controller_error {

#define MGMT_EV_NEW_SETTINGS 0x0006

-#define MGMT_EV_DISCOVERABLE 0x0007
-
-#define MGMT_EV_CONNECTABLE 0x0008
+#define MGMT_EV_CLASS_OF_DEV_CHANGED 0x0007
+struct mgmt_ev_class_of_dev_changed {
+ __u8 dev_class[3];
+};

-#define MGMT_EV_PAIRABLE 0x0009
+#define MGMT_EV_LOCAL_NAME_CHANGED 0x0008
+struct mgmt_ev_local_name_changed {
+ __u8 name[MGMT_MAX_NAME_LENGTH];
+ __u8 short_name[MGMT_MAX_SHORT_NAME_LENGTH];
+} __packed;

-#define MGMT_EV_NEW_LINK_KEY 0x000A
+#define MGMT_EV_NEW_LINK_KEY 0x0009
struct mgmt_ev_new_link_key {
__u8 store_hint;
struct mgmt_link_key_info key;
} __packed;

-#define MGMT_EV_CONNECTED 0x000B
+#define MGMT_EV_CONNECTED 0x000A

-#define MGMT_EV_DISCONNECTED 0x000C
+#define MGMT_EV_DISCONNECTED 0x000B

-#define MGMT_EV_CONNECT_FAILED 0x000D
+#define MGMT_EV_CONNECT_FAILED 0x000C
struct mgmt_ev_connect_failed {
struct mgmt_addr_info addr;
__u8 status;
} __packed;

-#define MGMT_EV_PIN_CODE_REQUEST 0x000E
+#define MGMT_EV_PIN_CODE_REQUEST 0x000D
struct mgmt_ev_pin_code_request {
bdaddr_t bdaddr;
__u8 secure;
} __packed;

-#define MGMT_EV_USER_CONFIRM_REQUEST 0x000F
+#define MGMT_EV_USER_CONFIRM_REQUEST 0x000E
struct mgmt_ev_user_confirm_request {
bdaddr_t bdaddr;
__u8 confirm_hint;
__le32 value;
} __packed;

+#define MGMT_EV_USER_PASSKEY_REQUEST 0x000F
+struct mgmt_ev_user_passkey_request {
+ bdaddr_t bdaddr;
+} __packed;
+
#define MGMT_EV_AUTH_FAILED 0x0010
struct mgmt_ev_auth_failed {
bdaddr_t bdaddr;
__u8 status;
} __packed;

-#define MGMT_EV_LOCAL_NAME_CHANGED 0x0011
-struct mgmt_ev_local_name_changed {
- __u8 name[MGMT_MAX_NAME_LENGTH];
-} __packed;
-
-#define MGMT_EV_DEVICE_FOUND 0x0012
+#define MGMT_EV_DEVICE_FOUND 0x0011
struct mgmt_ev_device_found {
struct mgmt_addr_info addr;
__u8 dev_class[3];
@@ -340,25 +360,20 @@ struct mgmt_ev_device_found {
__u8 eir[HCI_MAX_EIR_LENGTH];
} __packed;

-#define MGMT_EV_REMOTE_NAME 0x0013
+#define MGMT_EV_REMOTE_NAME 0x0012
struct mgmt_ev_remote_name {
bdaddr_t bdaddr;
__u8 name[MGMT_MAX_NAME_LENGTH];
} __packed;

-#define MGMT_EV_DISCOVERING 0x0014
+#define MGMT_EV_DISCOVERING 0x0013

-#define MGMT_EV_DEVICE_BLOCKED 0x0015
+#define MGMT_EV_DEVICE_BLOCKED 0x0014
struct mgmt_ev_device_blocked {
bdaddr_t bdaddr;
} __packed;

-#define MGMT_EV_DEVICE_UNBLOCKED 0x0016
+#define MGMT_EV_DEVICE_UNBLOCKED 0x0015
struct mgmt_ev_device_unblocked {
bdaddr_t bdaddr;
} __packed;
-
-#define MGMT_EV_USER_PASSKEY_REQUEST 0x0017
-struct mgmt_ev_user_passkey_request {
- bdaddr_t bdaddr;
-} __packed;
--
1.7.7.3


2011-12-14 21:52:02

by Johan Hedberg

[permalink] [raw]
Subject: [PATCH 5/7] Bluetooth: Add timer for automatically disabling the service cache

From: Johan Hedberg <[email protected]>

We do not want the service cache to be enabled indefinitely after
mgmt_read_info is called. The solve this a timer is added which will
automaticall disable the cache if mgmt_set_dev_class isn't called within
5 seconds of calling mgmt_read_info.

Signed-off-by: Johan Hedberg <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 +
net/bluetooth/hci_core.c | 3 ++
net/bluetooth/mgmt.c | 40 +++++++++++++++++++++++++++++++++----
3 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4e67bb8..a5d7e42 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -200,6 +200,8 @@ struct hci_dev {
__u16 discov_timeout;
struct delayed_work discov_off;

+ struct delayed_work service_cache;
+
struct timer_list cmd_timer;
struct tasklet_struct cmd_task;
struct tasklet_struct rx_task;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index ce3727e..323be52 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -597,6 +597,9 @@ static int hci_dev_do_close(struct hci_dev *hdev)
if (test_and_clear_bit(HCI_AUTO_OFF, &hdev->flags))
cancel_delayed_work(&hdev->power_off);

+ if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags))
+ cancel_delayed_work(&hdev->service_cache);
+
hci_dev_lock_bh(hdev);
inquiry_cache_flush(hdev);
hci_conn_hash_flush(hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 696e585..4d531b9 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -35,6 +35,8 @@

#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */

+#define SERVICE_CACHE_TIMEOUT (5 * 1000)
+
struct pending_cmd {
struct list_head list;
u16 opcode;
@@ -472,6 +474,32 @@ static int update_class(struct hci_dev *hdev)
return hci_send_cmd(hdev, HCI_OP_WRITE_CLASS_OF_DEV, sizeof(cod), cod);
}

+static void service_cache_off(struct work_struct *work)
+{
+ struct hci_dev *hdev = container_of(work, struct hci_dev,
+ service_cache.work);
+
+ if (!test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags))
+ return;
+
+ hci_dev_lock_bh(hdev);
+
+ update_eir(hdev);
+ update_class(hdev);
+
+ hci_dev_unlock_bh(hdev);
+}
+
+static void mgmt_init_hdev(struct hci_dev *hdev)
+{
+ if (!test_and_set_bit(HCI_MGMT, &hdev->flags))
+ INIT_DELAYED_WORK(&hdev->service_cache, service_cache_off);
+
+ if (!test_and_set_bit(HCI_SERVICE_CACHE, &hdev->flags))
+ queue_delayed_work(hdev->workqueue, &hdev->service_cache,
+ msecs_to_jiffies(SERVICE_CACHE_TIMEOUT));
+}
+
static int read_controller_info(struct sock *sk, u16 index)
{
struct mgmt_rp_read_info rp;
@@ -489,10 +517,8 @@ static int read_controller_info(struct sock *sk, u16 index)

hci_dev_lock_bh(hdev);

- if (test_and_clear_bit(HCI_PI_MGMT_INIT, &hci_pi(sk)->flags)) {
- set_bit(HCI_MGMT, &hdev->flags);
- set_bit(HCI_SERVICE_CACHE, &hdev->flags);
- }
+ if (test_and_clear_bit(HCI_PI_MGMT_INIT, &hci_pi(sk)->flags))
+ mgmt_init_hdev(hdev);

memset(&rp, 0, sizeof(rp));

@@ -992,8 +1018,12 @@ static int set_dev_class(struct sock *sk, u16 index, unsigned char *data,
hdev->major_class = cp->major;
hdev->minor_class = cp->minor;

- if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags))
+ if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags)) {
+ hci_dev_unlock_bh(hdev);
+ cancel_delayed_work_sync(&hdev->service_cache);
+ hci_dev_lock_bh(hdev);
update_eir(hdev);
+ }

err = update_class(hdev);

--
1.7.7.3


2011-12-14 21:52:01

by Johan Hedberg

[permalink] [raw]
Subject: [PATCH 4/7] Bluetooth: Move EIR and CoD update functions to a better position

From: Johan Hedberg <[email protected]>

Due to the upcoming addition of a service cache timer the functions to
update the EIR and CoD need to be higher up in mgmt.c in order to avoid
unnecessary forward-declarations. This patch simply moves code around
without any other changes in order to make subsequent patches more
readable.

Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/mgmt.c | 346 +++++++++++++++++++++++++-------------------------
1 files changed, 173 insertions(+), 173 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 25e7bf9..696e585 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -299,6 +299,179 @@ static u32 get_current_settings(struct hci_dev *hdev)
return settings;
}

+#define EIR_FLAGS 0x01 /* flags */
+#define EIR_UUID16_SOME 0x02 /* 16-bit UUID, more available */
+#define EIR_UUID16_ALL 0x03 /* 16-bit UUID, all listed */
+#define EIR_UUID32_SOME 0x04 /* 32-bit UUID, more available */
+#define EIR_UUID32_ALL 0x05 /* 32-bit UUID, all listed */
+#define EIR_UUID128_SOME 0x06 /* 128-bit UUID, more available */
+#define EIR_UUID128_ALL 0x07 /* 128-bit UUID, all listed */
+#define EIR_NAME_SHORT 0x08 /* shortened local name */
+#define EIR_NAME_COMPLETE 0x09 /* complete local name */
+#define EIR_TX_POWER 0x0A /* transmit power level */
+#define EIR_DEVICE_ID 0x10 /* device ID */
+
+#define PNP_INFO_SVCLASS_ID 0x1200
+
+static u8 bluetooth_base_uuid[] = {
+ 0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
+ 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+};
+
+static u16 get_uuid16(u8 *uuid128)
+{
+ u32 val;
+ int i;
+
+ for (i = 0; i < 12; i++) {
+ if (bluetooth_base_uuid[i] != uuid128[i])
+ return 0;
+ }
+
+ memcpy(&val, &uuid128[12], 4);
+
+ val = le32_to_cpu(val);
+ if (val > 0xffff)
+ return 0;
+
+ return (u16) val;
+}
+
+static void create_eir(struct hci_dev *hdev, u8 *data)
+{
+ u8 *ptr = data;
+ u16 eir_len = 0;
+ u16 uuid16_list[HCI_MAX_EIR_LENGTH / sizeof(u16)];
+ int i, truncated = 0;
+ struct bt_uuid *uuid;
+ size_t name_len;
+
+ name_len = strlen(hdev->dev_name);
+
+ if (name_len > 0) {
+ /* EIR Data type */
+ if (name_len > 48) {
+ name_len = 48;
+ ptr[1] = EIR_NAME_SHORT;
+ } else
+ ptr[1] = EIR_NAME_COMPLETE;
+
+ /* EIR Data length */
+ ptr[0] = name_len + 1;
+
+ memcpy(ptr + 2, hdev->dev_name, name_len);
+
+ eir_len += (name_len + 2);
+ ptr += (name_len + 2);
+ }
+
+ memset(uuid16_list, 0, sizeof(uuid16_list));
+
+ /* Group all UUID16 types */
+ list_for_each_entry(uuid, &hdev->uuids, list) {
+ u16 uuid16;
+
+ uuid16 = get_uuid16(uuid->uuid);
+ if (uuid16 == 0)
+ return;
+
+ if (uuid16 < 0x1100)
+ continue;
+
+ if (uuid16 == PNP_INFO_SVCLASS_ID)
+ continue;
+
+ /* Stop if not enough space to put next UUID */
+ if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
+ truncated = 1;
+ break;
+ }
+
+ /* Check for duplicates */
+ for (i = 0; uuid16_list[i] != 0; i++)
+ if (uuid16_list[i] == uuid16)
+ break;
+
+ if (uuid16_list[i] == 0) {
+ uuid16_list[i] = uuid16;
+ eir_len += sizeof(u16);
+ }
+ }
+
+ if (uuid16_list[0] != 0) {
+ u8 *length = ptr;
+
+ /* EIR Data type */
+ ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
+
+ ptr += 2;
+ eir_len += 2;
+
+ for (i = 0; uuid16_list[i] != 0; i++) {
+ *ptr++ = (uuid16_list[i] & 0x00ff);
+ *ptr++ = (uuid16_list[i] & 0xff00) >> 8;
+ }
+
+ /* EIR Data length */
+ *length = (i * sizeof(u16)) + 1;
+ }
+}
+
+static int update_eir(struct hci_dev *hdev)
+{
+ struct hci_cp_write_eir cp;
+
+ if (!(hdev->features[6] & LMP_EXT_INQ))
+ return 0;
+
+ if (hdev->ssp_mode == 0)
+ return 0;
+
+ if (test_bit(HCI_SERVICE_CACHE, &hdev->flags))
+ return 0;
+
+ memset(&cp, 0, sizeof(cp));
+
+ create_eir(hdev, cp.data);
+
+ if (memcmp(cp.data, hdev->eir, sizeof(cp.data)) == 0)
+ return 0;
+
+ memcpy(hdev->eir, cp.data, sizeof(cp.data));
+
+ return hci_send_cmd(hdev, HCI_OP_WRITE_EIR, sizeof(cp), &cp);
+}
+
+static u8 get_service_classes(struct hci_dev *hdev)
+{
+ struct bt_uuid *uuid;
+ u8 val = 0;
+
+ list_for_each_entry(uuid, &hdev->uuids, list)
+ val |= uuid->svc_hint;
+
+ return val;
+}
+
+static int update_class(struct hci_dev *hdev)
+{
+ u8 cod[3];
+
+ BT_DBG("%s", hdev->name);
+
+ if (test_bit(HCI_SERVICE_CACHE, &hdev->flags))
+ return 0;
+
+ cod[0] = hdev->minor_class;
+ cod[1] = hdev->major_class;
+ cod[2] = get_service_classes(hdev);
+
+ if (memcmp(cod, hdev->dev_class, 3) == 0)
+ return 0;
+
+ return hci_send_cmd(hdev, HCI_OP_WRITE_CLASS_OF_DEV, sizeof(cod), cod);
+}
+
static int read_controller_info(struct sock *sk, u16 index)
{
struct mgmt_rp_read_info rp;
@@ -681,179 +854,6 @@ failed:
return err;
}

-#define EIR_FLAGS 0x01 /* flags */
-#define EIR_UUID16_SOME 0x02 /* 16-bit UUID, more available */
-#define EIR_UUID16_ALL 0x03 /* 16-bit UUID, all listed */
-#define EIR_UUID32_SOME 0x04 /* 32-bit UUID, more available */
-#define EIR_UUID32_ALL 0x05 /* 32-bit UUID, all listed */
-#define EIR_UUID128_SOME 0x06 /* 128-bit UUID, more available */
-#define EIR_UUID128_ALL 0x07 /* 128-bit UUID, all listed */
-#define EIR_NAME_SHORT 0x08 /* shortened local name */
-#define EIR_NAME_COMPLETE 0x09 /* complete local name */
-#define EIR_TX_POWER 0x0A /* transmit power level */
-#define EIR_DEVICE_ID 0x10 /* device ID */
-
-#define PNP_INFO_SVCLASS_ID 0x1200
-
-static u8 bluetooth_base_uuid[] = {
- 0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
- 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-};
-
-static u16 get_uuid16(u8 *uuid128)
-{
- u32 val;
- int i;
-
- for (i = 0; i < 12; i++) {
- if (bluetooth_base_uuid[i] != uuid128[i])
- return 0;
- }
-
- memcpy(&val, &uuid128[12], 4);
-
- val = le32_to_cpu(val);
- if (val > 0xffff)
- return 0;
-
- return (u16) val;
-}
-
-static void create_eir(struct hci_dev *hdev, u8 *data)
-{
- u8 *ptr = data;
- u16 eir_len = 0;
- u16 uuid16_list[HCI_MAX_EIR_LENGTH / sizeof(u16)];
- int i, truncated = 0;
- struct bt_uuid *uuid;
- size_t name_len;
-
- name_len = strlen(hdev->dev_name);
-
- if (name_len > 0) {
- /* EIR Data type */
- if (name_len > 48) {
- name_len = 48;
- ptr[1] = EIR_NAME_SHORT;
- } else
- ptr[1] = EIR_NAME_COMPLETE;
-
- /* EIR Data length */
- ptr[0] = name_len + 1;
-
- memcpy(ptr + 2, hdev->dev_name, name_len);
-
- eir_len += (name_len + 2);
- ptr += (name_len + 2);
- }
-
- memset(uuid16_list, 0, sizeof(uuid16_list));
-
- /* Group all UUID16 types */
- list_for_each_entry(uuid, &hdev->uuids, list) {
- u16 uuid16;
-
- uuid16 = get_uuid16(uuid->uuid);
- if (uuid16 == 0)
- return;
-
- if (uuid16 < 0x1100)
- continue;
-
- if (uuid16 == PNP_INFO_SVCLASS_ID)
- continue;
-
- /* Stop if not enough space to put next UUID */
- if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
- truncated = 1;
- break;
- }
-
- /* Check for duplicates */
- for (i = 0; uuid16_list[i] != 0; i++)
- if (uuid16_list[i] == uuid16)
- break;
-
- if (uuid16_list[i] == 0) {
- uuid16_list[i] = uuid16;
- eir_len += sizeof(u16);
- }
- }
-
- if (uuid16_list[0] != 0) {
- u8 *length = ptr;
-
- /* EIR Data type */
- ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
-
- ptr += 2;
- eir_len += 2;
-
- for (i = 0; uuid16_list[i] != 0; i++) {
- *ptr++ = (uuid16_list[i] & 0x00ff);
- *ptr++ = (uuid16_list[i] & 0xff00) >> 8;
- }
-
- /* EIR Data length */
- *length = (i * sizeof(u16)) + 1;
- }
-}
-
-static int update_eir(struct hci_dev *hdev)
-{
- struct hci_cp_write_eir cp;
-
- if (!(hdev->features[6] & LMP_EXT_INQ))
- return 0;
-
- if (hdev->ssp_mode == 0)
- return 0;
-
- if (test_bit(HCI_SERVICE_CACHE, &hdev->flags))
- return 0;
-
- memset(&cp, 0, sizeof(cp));
-
- create_eir(hdev, cp.data);
-
- if (memcmp(cp.data, hdev->eir, sizeof(cp.data)) == 0)
- return 0;
-
- memcpy(hdev->eir, cp.data, sizeof(cp.data));
-
- return hci_send_cmd(hdev, HCI_OP_WRITE_EIR, sizeof(cp), &cp);
-}
-
-static u8 get_service_classes(struct hci_dev *hdev)
-{
- struct bt_uuid *uuid;
- u8 val = 0;
-
- list_for_each_entry(uuid, &hdev->uuids, list)
- val |= uuid->svc_hint;
-
- return val;
-}
-
-static int update_class(struct hci_dev *hdev)
-{
- u8 cod[3];
-
- BT_DBG("%s", hdev->name);
-
- if (test_bit(HCI_SERVICE_CACHE, &hdev->flags))
- return 0;
-
- cod[0] = hdev->minor_class;
- cod[1] = hdev->major_class;
- cod[2] = get_service_classes(hdev);
-
- if (memcmp(cod, hdev->dev_class, 3) == 0)
- return 0;
-
- return hci_send_cmd(hdev, HCI_OP_WRITE_CLASS_OF_DEV, sizeof(cod), cod);
-}
-
static int add_uuid(struct sock *sk, u16 index, unsigned char *data, u16 len)
{
struct mgmt_cp_add_uuid *cp;
--
1.7.7.3


2011-12-14 21:52:00

by Johan Hedberg

[permalink] [raw]
Subject: [PATCH 3/7] Bluetooth: Remove mgmt_set_service_cache

From: Johan Hedberg <[email protected]>

Instead of having an explicit service cache command we can make the mgmt
API simpler by implicitly enabling the cache when mgmt_read_info is
called for the first time and disabiling it when mgmt_set_dev_class is
called.

Signed-off-by: Johan Hedberg <[email protected]>
---
include/net/bluetooth/hci_core.h | 4 +++
include/net/bluetooth/mgmt.h | 5 ---
net/bluetooth/hci_sock.c | 7 +++-
net/bluetooth/mgmt.c | 56 +++++---------------------------------
4 files changed, 16 insertions(+), 56 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e34cd71..4e67bb8 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -958,12 +958,16 @@ int mgmt_device_unblocked(struct hci_dev *hdev, bdaddr_t *bdaddr);
/* HCI info for socket */
#define hci_pi(sk) ((struct hci_pinfo *) sk)

+/* HCI socket flags */
+#define HCI_PI_MGMT_INIT 0
+
struct hci_pinfo {
struct bt_sock bt;
struct hci_dev *hdev;
struct hci_filter filter;
__u32 cmsg_mask;
unsigned short channel;
+ unsigned long flags;
};

/* HCI security filter */
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index bf217cc..bdb0a58 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -121,11 +121,6 @@ struct mgmt_cp_set_dev_class {
__u8 minor;
} __packed;

-#define MGMT_OP_SET_SERVICE_CACHE 0x000C
-struct mgmt_cp_set_service_cache {
- __u8 enable;
-} __packed;
-
struct mgmt_link_key_info {
bdaddr_t bdaddr;
u8 type;
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index f6afe3d..47febe8 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -343,8 +343,11 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
if (haddr.hci_channel > HCI_CHANNEL_CONTROL)
return -EINVAL;

- if (haddr.hci_channel == HCI_CHANNEL_CONTROL && !enable_mgmt)
- return -EINVAL;
+ if (haddr.hci_channel == HCI_CHANNEL_CONTROL) {
+ if (!enable_mgmt)
+ return -EINVAL;
+ set_bit(HCI_PI_MGMT_INIT, &hci_pi(sk)->flags);
+ }

lock_sock(sk);

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 54092c2..25e7bf9 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -316,7 +316,10 @@ static int read_controller_info(struct sock *sk, u16 index)

hci_dev_lock_bh(hdev);

- set_bit(HCI_MGMT, &hdev->flags);
+ if (test_and_clear_bit(HCI_PI_MGMT_INIT, &hci_pi(sk)->flags)) {
+ set_bit(HCI_MGMT, &hdev->flags);
+ set_bit(HCI_SERVICE_CACHE, &hdev->flags);
+ }

memset(&rp, 0, sizeof(rp));

@@ -989,6 +992,9 @@ static int set_dev_class(struct sock *sk, u16 index, unsigned char *data,
hdev->major_class = cp->major;
hdev->minor_class = cp->minor;

+ if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags))
+ update_eir(hdev);
+
err = update_class(hdev);

if (err == 0)
@@ -1000,51 +1006,6 @@ static int set_dev_class(struct sock *sk, u16 index, unsigned char *data,
return err;
}

-static int set_service_cache(struct sock *sk, u16 index, unsigned char *data,
- u16 len)
-{
- struct hci_dev *hdev;
- struct mgmt_cp_set_service_cache *cp;
- int err;
-
- cp = (void *) data;
-
- if (len != sizeof(*cp))
- return cmd_status(sk, index, MGMT_OP_SET_SERVICE_CACHE,
- MGMT_STATUS_INVALID_PARAMS);
-
- hdev = hci_dev_get(index);
- if (!hdev)
- return cmd_status(sk, index, MGMT_OP_SET_SERVICE_CACHE,
- MGMT_STATUS_INVALID_PARAMS);
-
- hci_dev_lock_bh(hdev);
-
- BT_DBG("hci%u enable %d", index, cp->enable);
-
- if (cp->enable) {
- set_bit(HCI_SERVICE_CACHE, &hdev->flags);
- err = 0;
- } else {
- clear_bit(HCI_SERVICE_CACHE, &hdev->flags);
- err = update_class(hdev);
- if (err == 0)
- err = update_eir(hdev);
- }
-
- if (err == 0)
- err = cmd_complete(sk, index, MGMT_OP_SET_SERVICE_CACHE, NULL,
- 0);
- else
- cmd_status(sk, index, MGMT_OP_SET_SERVICE_CACHE, -err);
-
-
- hci_dev_unlock_bh(hdev);
- hci_dev_put(hdev);
-
- return err;
-}
-
static int load_link_keys(struct sock *sk, u16 index, unsigned char *data,
u16 len)
{
@@ -2170,9 +2131,6 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
case MGMT_OP_SET_DEV_CLASS:
err = set_dev_class(sk, index, buf + sizeof(*hdr), len);
break;
- case MGMT_OP_SET_SERVICE_CACHE:
- err = set_service_cache(sk, index, buf + sizeof(*hdr), len);
- break;
case MGMT_OP_LOAD_LINK_KEYS:
err = load_link_keys(sk, index, buf + sizeof(*hdr), len);
break;
--
1.7.7.3


2011-12-14 21:51:58

by Johan Hedberg

[permalink] [raw]
Subject: [PATCH 1/7] Bluetooth: Update mgmt_read_info and related mgmt messages

From: Johan Hedberg <[email protected]>

This patch updates the mgmt_read_info and related messages to the latest
management API which uses a bitfield of settings instead of individual
boolean values.

Signed-off-by: Johan Hedberg <[email protected]>
---
include/net/bluetooth/hci.h | 1 +
include/net/bluetooth/mgmt.h | 29 +++++---
net/bluetooth/mgmt.c | 146 +++++++++++++++++++++++++++---------------
3 files changed, 113 insertions(+), 63 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 67ad984..c9ad56f 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -210,6 +210,7 @@ enum {

#define LMP_EV4 0x01
#define LMP_EV5 0x02
+#define LMP_NO_BREDR 0x20
#define LMP_LE 0x40

#define LMP_SNIFF_SUBR 0x02
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 3b68806..85e9c6e 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -61,22 +61,29 @@ struct mgmt_rp_read_index_list {
/* Reserve one extra byte for names in management messages so that they
* are always guaranteed to be nul-terminated */
#define MGMT_MAX_NAME_LENGTH (HCI_MAX_NAME_LENGTH + 1)
+#define MGMT_MAX_SHORT_NAME_LENGTH (10 + 1)
+
+#define MGMT_SETTING_POWERED 0x00000001
+#define MGMT_SETTING_CONNECTABLE 0x00000002
+#define MGMT_SETTING_FAST_CONNECTABLE 0x00000004
+#define MGMT_SETTING_DISCOVERABLE 0x00000008
+#define MGMT_SETTING_PAIRABLE 0x00000010
+#define MGMT_SETTING_LINK_SECURITY 0x00000020
+#define MGMT_SETTING_SSP 0x00000040
+#define MGMT_SETTING_BREDR 0x00000080
+#define MGMT_SETTING_HS 0x00000100
+#define MGMT_SETTING_LE 0x00000200

#define MGMT_OP_READ_INFO 0x0004
struct mgmt_rp_read_info {
- __u8 type;
- __u8 powered;
- __u8 connectable;
- __u8 discoverable;
- __u8 pairable;
- __u8 sec_mode;
bdaddr_t bdaddr;
+ __u8 version;
+ __le16 manufacturer;
+ __le32 supported_settings;
+ __le32 current_settings;
__u8 dev_class[3];
- __u8 features[8];
- __u16 manufacturer;
- __u8 hci_ver;
- __u16 hci_rev;
__u8 name[MGMT_MAX_NAME_LENGTH];
+ __u8 short_name[MGMT_MAX_SHORT_NAME_LENGTH];
} __packed;

struct mgmt_mode {
@@ -285,7 +292,7 @@ struct mgmt_ev_controller_error {

#define MGMT_EV_INDEX_REMOVED 0x0005

-#define MGMT_EV_POWERED 0x0006
+#define MGMT_EV_NEW_SETTINGS 0x0006

#define MGMT_EV_DISCOVERABLE 0x0007

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 7a23f21..629570c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -242,6 +242,63 @@ static int read_index_list(struct sock *sk)
return err;
}

+static u32 get_supported_settings(struct hci_dev *hdev)
+{
+ u32 settings = 0;
+
+ settings |= MGMT_SETTING_POWERED;
+ settings |= MGMT_SETTING_CONNECTABLE;
+ settings |= MGMT_SETTING_FAST_CONNECTABLE;
+ settings |= MGMT_SETTING_DISCOVERABLE;
+ settings |= MGMT_SETTING_PAIRABLE;
+
+ if (hdev->features[6] & LMP_SIMPLE_PAIR)
+ settings |= MGMT_SETTING_SSP;
+
+ if (!(hdev->features[4] & LMP_NO_BREDR)) {
+ settings |= MGMT_SETTING_BREDR;
+ settings |= MGMT_SETTING_LINK_SECURITY;
+ }
+
+ if (hdev->features[4] & LMP_LE)
+ settings |= MGMT_SETTING_LE;
+
+ return settings;
+}
+
+static u32 get_current_settings(struct hci_dev *hdev)
+{
+ u32 settings = 0;
+
+ if (test_bit(HCI_UP, &hdev->flags))
+ settings |= MGMT_SETTING_POWERED;
+ else
+ return settings;
+
+ if (test_bit(HCI_PSCAN, &hdev->flags))
+ settings |= MGMT_SETTING_CONNECTABLE;
+
+ if (test_bit(HCI_ISCAN, &hdev->flags))
+ settings |= MGMT_SETTING_DISCOVERABLE;
+
+ if (test_bit(HCI_PAIRABLE, &hdev->flags))
+ settings |= MGMT_SETTING_PAIRABLE;
+
+ if (!(hdev->features[4] & LMP_NO_BREDR))
+ settings |= MGMT_SETTING_BREDR;
+
+ if (hdev->extfeatures[0] & LMP_HOST_LE)
+ settings |= MGMT_SETTING_LE;
+
+ if (test_bit(HCI_AUTH, &hdev->flags))
+ settings |= MGMT_SETTING_LINK_SECURITY;
+
+ if (hdev->ssp_mode > 0)
+ settings |= MGMT_SETTING_SSP;
+
+ return settings;
+}
+
static int read_controller_info(struct sock *sk, u16 index)
{
struct mgmt_rp_read_info rp;
@@ -263,26 +320,16 @@ static int read_controller_info(struct sock *sk, u16 index)

memset(&rp, 0, sizeof(rp));

- rp.type = hdev->dev_type;
+ bacpy(&rp.bdaddr, &hdev->bdaddr);

- rp.powered = test_bit(HCI_UP, &hdev->flags);
- rp.connectable = test_bit(HCI_PSCAN, &hdev->flags);
- rp.discoverable = test_bit(HCI_ISCAN, &hdev->flags);
- rp.pairable = test_bit(HCI_PSCAN, &hdev->flags);
+ rp.version = hdev->hci_ver;

- if (test_bit(HCI_AUTH, &hdev->flags))
- rp.sec_mode = 3;
- else if (hdev->ssp_mode > 0)
- rp.sec_mode = 4;
- else
- rp.sec_mode = 2;
+ put_unaligned_le16(hdev->manufacturer, &rp.manufacturer);
+
+ rp.supported_settings = cpu_to_le32(get_supported_settings(hdev));
+ rp.current_settings = cpu_to_le32(get_current_settings(hdev));

- bacpy(&rp.bdaddr, &hdev->bdaddr);
- memcpy(rp.features, hdev->features, 8);
memcpy(rp.dev_class, hdev->dev_class, 3);
- put_unaligned_le16(hdev->manufacturer, &rp.manufacturer);
- rp.hci_ver = hdev->hci_ver;
- put_unaligned_le16(hdev->hci_rev, &rp.hci_rev);

memcpy(rp.name, hdev->dev_name, sizeof(hdev->dev_name));

@@ -365,13 +412,11 @@ static void mgmt_pending_remove(struct pending_cmd *cmd)
mgmt_pending_free(cmd);
}

-static int send_mode_rsp(struct sock *sk, u16 opcode, u16 index, u8 val)
+static int send_settings_rsp(struct sock *sk, u16 opcode, struct hci_dev *hdev)
{
- struct mgmt_mode rp;
+ __le32 settings = cpu_to_le32(get_current_settings(hdev));

- rp.val = val;
-
- return cmd_complete(sk, index, opcode, &rp, sizeof(rp));
+ return cmd_complete(sk, hdev->id, opcode, &settings, sizeof(settings));
}

static int set_powered(struct sock *sk, u16 index, unsigned char *data, u16 len)
@@ -398,7 +443,7 @@ static int set_powered(struct sock *sk, u16 index, unsigned char *data, u16 len)

up = test_bit(HCI_UP, &hdev->flags);
if ((cp->val && up) || (!cp->val && !up)) {
- err = send_mode_rsp(sk, index, MGMT_OP_SET_POWERED, cp->val);
+ err = send_settings_rsp(sk, MGMT_OP_SET_POWERED, hdev);
goto failed;
}

@@ -466,8 +511,7 @@ static int set_discoverable(struct sock *sk, u16 index, unsigned char *data,

if (cp->val == test_bit(HCI_ISCAN, &hdev->flags) &&
test_bit(HCI_PSCAN, &hdev->flags)) {
- err = send_mode_rsp(sk, index, MGMT_OP_SET_DISCOVERABLE,
- cp->val);
+ err = send_settings_rsp(sk, MGMT_OP_SET_DISCOVERABLE, hdev);
goto failed;
}

@@ -536,8 +580,7 @@ static int set_connectable(struct sock *sk, u16 index, unsigned char *data,
}

if (cp->val == test_bit(HCI_PSCAN, &hdev->flags)) {
- err = send_mode_rsp(sk, index, MGMT_OP_SET_CONNECTABLE,
- cp->val);
+ err = send_settings_rsp(sk, MGMT_OP_SET_CONNECTABLE, hdev);
goto failed;
}

@@ -595,8 +638,9 @@ static int mgmt_event(u16 event, struct hci_dev *hdev, void *data,
static int set_pairable(struct sock *sk, u16 index, unsigned char *data,
u16 len)
{
- struct mgmt_mode *cp, ev;
+ struct mgmt_mode *cp;
struct hci_dev *hdev;
+ __le32 ev;
int err;

cp = (void *) data;
@@ -619,13 +663,13 @@ static int set_pairable(struct sock *sk, u16 index, unsigned char *data,
else
clear_bit(HCI_PAIRABLE, &hdev->flags);

- err = send_mode_rsp(sk, MGMT_OP_SET_PAIRABLE, index, cp->val);
+ err = send_settings_rsp(sk, MGMT_OP_SET_PAIRABLE, hdev);
if (err < 0)
goto failed;

- ev.val = cp->val;
+ ev = cpu_to_le32(get_current_settings(hdev));

- err = mgmt_event(MGMT_EV_PAIRABLE, hdev, &ev, sizeof(ev), sk);
+ err = mgmt_event(MGMT_EV_NEW_SETTINGS, hdev, &ev, sizeof(ev), sk);

failed:
hci_dev_unlock_bh(hdev);
@@ -2234,17 +2278,14 @@ int mgmt_index_removed(struct hci_dev *hdev)
struct cmd_lookup {
u8 val;
struct sock *sk;
+ struct hci_dev *hdev;
};

-static void mode_rsp(struct pending_cmd *cmd, void *data)
+static void settings_rsp(struct pending_cmd *cmd, void *data)
{
- struct mgmt_mode *cp = cmd->param;
struct cmd_lookup *match = data;

- if (cp->val != match->val)
- return;
-
- send_mode_rsp(cmd->sk, cmd->opcode, cmd->index, cp->val);
+ send_settings_rsp(cmd->sk, cmd->opcode, match->hdev);

list_del(&cmd->list);

@@ -2258,20 +2299,21 @@ static void mode_rsp(struct pending_cmd *cmd, void *data)

int mgmt_powered(struct hci_dev *hdev, u8 powered)
{
- struct mgmt_mode ev;
- struct cmd_lookup match = { powered, NULL };
+ struct cmd_lookup match = { powered, NULL, hdev };
+ __le32 ev;
int ret;

- mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, mode_rsp, &match);
+ mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp, &match);

if (!powered) {
u8 status = ENETDOWN;
mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status);
}

- ev.val = powered;
+ ev = cpu_to_le32(get_current_settings(hdev));

- ret = mgmt_event(MGMT_EV_POWERED, hdev, &ev, sizeof(ev), match.sk);
+ ret = mgmt_event(MGMT_EV_NEW_SETTINGS, hdev, &ev, sizeof(ev),
+ match.sk);

if (match.sk)
sock_put(match.sk);
@@ -2281,17 +2323,16 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)

int mgmt_discoverable(struct hci_dev *hdev, u8 discoverable)
{
- struct mgmt_mode ev;
- struct cmd_lookup match = { discoverable, NULL };
+ struct cmd_lookup match = { discoverable, NULL, hdev };
+ __le32 ev;
int ret;

- mgmt_pending_foreach(MGMT_OP_SET_DISCOVERABLE, hdev, mode_rsp, &match);
+ mgmt_pending_foreach(MGMT_OP_SET_DISCOVERABLE, hdev, settings_rsp, &match);

- ev.val = discoverable;
+ ev = cpu_to_le32(get_current_settings(hdev));

- ret = mgmt_event(MGMT_EV_DISCOVERABLE, hdev, &ev, sizeof(ev),
+ ret = mgmt_event(MGMT_EV_NEW_SETTINGS, hdev, &ev, sizeof(ev),
match.sk);
-
if (match.sk)
sock_put(match.sk);

@@ -2300,15 +2341,16 @@ int mgmt_discoverable(struct hci_dev *hdev, u8 discoverable)

int mgmt_connectable(struct hci_dev *hdev, u8 connectable)
{
- struct mgmt_mode ev;
- struct cmd_lookup match = { connectable, NULL };
+ __le32 ev;
+ struct cmd_lookup match = { connectable, NULL, hdev };
int ret;

- mgmt_pending_foreach(MGMT_OP_SET_CONNECTABLE, hdev, mode_rsp, &match);
+ mgmt_pending_foreach(MGMT_OP_SET_CONNECTABLE, hdev, settings_rsp,
+ &match);

- ev.val = connectable;
+ ev = cpu_to_le32(get_current_settings(hdev));

- ret = mgmt_event(MGMT_EV_CONNECTABLE, hdev, &ev, sizeof(ev), match.sk);
+ ret = mgmt_event(MGMT_EV_NEW_SETTINGS, hdev, &ev, sizeof(ev), match.sk);

if (match.sk)
sock_put(match.sk);
--
1.7.7.3


2011-12-14 21:51:59

by Johan Hedberg

[permalink] [raw]
Subject: [PATCH 2/7] Bluetooth: Move mgmt_set_fast_connectable to the right location

From: Johan Hedberg <[email protected]>

Fast connetable is logically after the connectable property so that's
where it should show up in the code as well (it's also after connectable
in the settings bitfield).

Signed-off-by: Johan Hedberg <[email protected]>
---
include/net/bluetooth/mgmt.h | 7 ++-----
net/bluetooth/mgmt.c | 12 ++++++------
2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 85e9c6e..bf217cc 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -100,6 +100,8 @@ struct mgmt_cp_set_discoverable {

#define MGMT_OP_SET_CONNECTABLE 0x0007

+#define MGMT_OP_SET_FAST_CONNECTABLE 0x001F
+
#define MGMT_OP_SET_PAIRABLE 0x0008

#define MGMT_OP_ADD_UUID 0x0009
@@ -255,11 +257,6 @@ struct mgmt_cp_unblock_device {
bdaddr_t bdaddr;
} __packed;

-#define MGMT_OP_SET_FAST_CONNECTABLE 0x001F
-struct mgmt_cp_set_fast_connectable {
- __u8 enable;
-} __packed;
-
#define MGMT_OP_USER_PASSKEY_REPLY 0x0020
struct mgmt_cp_user_passkey_reply {
bdaddr_t bdaddr;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 629570c..54092c2 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2052,7 +2052,7 @@ static int set_fast_connectable(struct sock *sk, u16 index,
unsigned char *data, u16 len)
{
struct hci_dev *hdev;
- struct mgmt_cp_set_fast_connectable *cp = (void *) data;
+ struct mgmt_mode *cp = (void *) data;
struct hci_cp_write_page_scan_activity acp;
u8 type;
int err;
@@ -2070,7 +2070,7 @@ static int set_fast_connectable(struct sock *sk, u16 index,

hci_dev_lock(hdev);

- if (cp->enable) {
+ if (cp->val) {
type = PAGE_SCAN_TYPE_INTERLACED;
acp.interval = 0x0024; /* 22.5 msec page scan interval */
} else {
@@ -2154,6 +2154,10 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
case MGMT_OP_SET_CONNECTABLE:
err = set_connectable(sk, index, buf + sizeof(*hdr), len);
break;
+ case MGMT_OP_SET_FAST_CONNECTABLE:
+ err = set_fast_connectable(sk, index, buf + sizeof(*hdr),
+ len);
+ break;
case MGMT_OP_SET_PAIRABLE:
err = set_pairable(sk, index, buf + sizeof(*hdr), len);
break;
@@ -2232,10 +2236,6 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
case MGMT_OP_UNBLOCK_DEVICE:
err = unblock_device(sk, index, buf + sizeof(*hdr), len);
break;
- case MGMT_OP_SET_FAST_CONNECTABLE:
- err = set_fast_connectable(sk, index, buf + sizeof(*hdr),
- len);
- break;
default:
BT_DBG("Unknown op %u", opcode);
err = cmd_status(sk, index, opcode,
--
1.7.7.3