* [RFC] Bluetooth: AMP: Do not set name for AMP ctrl @ 2012-06-06 15:00 Andrei Emeltchenko 2012-06-06 15:22 ` Johan Hedberg 0 siblings, 1 reply; 7+ messages in thread From: Andrei Emeltchenko @ 2012-06-06 15:00 UTC (permalink / raw) To: linux-bluetooth From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> AMP controllers do not understand this command ... < HCI Command: Write Local Name (0x03|0x0013) plen 248 name '' > HCI Event: Command Complete (0x0e) plen 4 Write Local Name (0x03|0x0013) ncmd 1 status 0x01 Error: Unknown HCI Command ... Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com> --- net/bluetooth/mgmt.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 958f764..9afcc84 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -2135,6 +2135,9 @@ static int set_local_name(struct sock *sk, struct hci_dev *hdev, void *data, BT_DBG(""); + if (hdev->amp_type != HCI_BREDR) + return -ENOTSUPP; + hci_dev_lock(hdev); memcpy(hdev->short_name, cp->short_name, sizeof(hdev->short_name)); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC] Bluetooth: AMP: Do not set name for AMP ctrl 2012-06-06 15:00 [RFC] Bluetooth: AMP: Do not set name for AMP ctrl Andrei Emeltchenko @ 2012-06-06 15:22 ` Johan Hedberg 2012-06-06 15:27 ` Johan Hedberg 0 siblings, 1 reply; 7+ messages in thread From: Johan Hedberg @ 2012-06-06 15:22 UTC (permalink / raw) To: Andrei Emeltchenko; +Cc: linux-bluetooth Hi Andrei, On Wed, Jun 06, 2012, Andrei Emeltchenko wrote: > AMP controllers do not understand this command > > ... > < HCI Command: Write Local Name (0x03|0x0013) plen 248 > name '' > > HCI Event: Command Complete (0x0e) plen 4 > Write Local Name (0x03|0x0013) ncmd 1 > status 0x01 > Error: Unknown HCI Command > ... > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com> > --- > net/bluetooth/mgmt.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c > index 958f764..9afcc84 100644 > --- a/net/bluetooth/mgmt.c > +++ b/net/bluetooth/mgmt.c > @@ -2135,6 +2135,9 @@ static int set_local_name(struct sock *sk, struct hci_dev *hdev, void *data, > > BT_DBG(""); > > + if (hdev->amp_type != HCI_BREDR) > + return -ENOTSUPP; > + > hci_dev_lock(hdev); > > memcpy(hdev->short_name, cp->short_name, sizeof(hdev->short_name)); This will essentially make write() fail on the mgmt socket which isn't necessarily the most intuitive behavior (one might think there's something wrong with the socket). What would probably make more sense is to send a proper cmd_status reply with MGMT_STATUS_NOT_SUPPORTED. Johan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Bluetooth: AMP: Do not set name for AMP ctrl 2012-06-06 15:22 ` Johan Hedberg @ 2012-06-06 15:27 ` Johan Hedberg 2012-06-07 6:40 ` Andrei Emeltchenko 0 siblings, 1 reply; 7+ messages in thread From: Johan Hedberg @ 2012-06-06 15:27 UTC (permalink / raw) To: Andrei Emeltchenko, linux-bluetooth Hi, On Wed, Jun 06, 2012, Johan Hedberg wrote: > On Wed, Jun 06, 2012, Andrei Emeltchenko wrote: > > AMP controllers do not understand this command > > > > ... > > < HCI Command: Write Local Name (0x03|0x0013) plen 248 > > name '' > > > HCI Event: Command Complete (0x0e) plen 4 > > Write Local Name (0x03|0x0013) ncmd 1 > > status 0x01 > > Error: Unknown HCI Command > > ... > > > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com> > > --- > > net/bluetooth/mgmt.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c > > index 958f764..9afcc84 100644 > > --- a/net/bluetooth/mgmt.c > > +++ b/net/bluetooth/mgmt.c > > @@ -2135,6 +2135,9 @@ static int set_local_name(struct sock *sk, struct hci_dev *hdev, void *data, > > > > BT_DBG(""); > > > > + if (hdev->amp_type != HCI_BREDR) > > + return -ENOTSUPP; > > + > > hci_dev_lock(hdev); > > > > memcpy(hdev->short_name, cp->short_name, sizeof(hdev->short_name)); > > This will essentially make write() fail on the mgmt socket which isn't > necessarily the most intuitive behavior (one might think there's > something wrong with the socket). What would probably make more sense is > to send a proper cmd_status reply with MGMT_STATUS_NOT_SUPPORTED. Actually, should we even be exposing AMP controllers to begin with through mgmt? Maybe we shouldn't even tell user space about them through mgmt? Johan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Bluetooth: AMP: Do not set name for AMP ctrl 2012-06-06 15:27 ` Johan Hedberg @ 2012-06-07 6:40 ` Andrei Emeltchenko 2012-06-07 15:18 ` Johan Hedberg 0 siblings, 1 reply; 7+ messages in thread From: Andrei Emeltchenko @ 2012-06-07 6:40 UTC (permalink / raw) To: linux-bluetooth Hi Johan, On Wed, Jun 06, 2012 at 11:27:36PM +0800, Johan Hedberg wrote: > Hi, > > On Wed, Jun 06, 2012, Johan Hedberg wrote: > > On Wed, Jun 06, 2012, Andrei Emeltchenko wrote: > > > AMP controllers do not understand this command > > > > > > ... > > > < HCI Command: Write Local Name (0x03|0x0013) plen 248 > > > name '' > > > > HCI Event: Command Complete (0x0e) plen 4 > > > Write Local Name (0x03|0x0013) ncmd 1 > > > status 0x01 > > > Error: Unknown HCI Command > > > ... > > > > > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com> > > > --- > > > net/bluetooth/mgmt.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c > > > index 958f764..9afcc84 100644 > > > --- a/net/bluetooth/mgmt.c > > > +++ b/net/bluetooth/mgmt.c > > > @@ -2135,6 +2135,9 @@ static int set_local_name(struct sock *sk, struct hci_dev *hdev, void *data, > > > > > > BT_DBG(""); > > > > > > + if (hdev->amp_type != HCI_BREDR) > > > + return -ENOTSUPP; > > > + > > > hci_dev_lock(hdev); > > > > > > memcpy(hdev->short_name, cp->short_name, sizeof(hdev->short_name)); > > > > This will essentially make write() fail on the mgmt socket which isn't > > necessarily the most intuitive behavior (one might think there's > > something wrong with the socket). What would probably make more sense is > > to send a proper cmd_status reply with MGMT_STATUS_NOT_SUPPORTED. > > Actually, should we even be exposing AMP controllers to begin with > through mgmt? Maybe we shouldn't even tell user space about them through > mgmt? This seems to be right. BTW: What is the best way to disable mgmt for device? - Clear HCI_MGMT flag ? - Do not run mgmt_powered ? BTW: Where that flag HCI_MGMT is set and how this check supposed to be working? test_and_set_bit(HCI_MGMT, &hdev->dev_flags)) Best regards Andrei Emeltchenko ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Bluetooth: AMP: Do not set name for AMP ctrl 2012-06-07 6:40 ` Andrei Emeltchenko @ 2012-06-07 15:18 ` Johan Hedberg 2012-06-08 8:09 ` [PATCH] Bluetooth: mgmt: Fix managing AMP device Andrei Emeltchenko 0 siblings, 1 reply; 7+ messages in thread From: Johan Hedberg @ 2012-06-07 15:18 UTC (permalink / raw) To: Andrei Emeltchenko, linux-bluetooth Hi Andrei, On Thu, Jun 07, 2012, Andrei Emeltchenko wrote: > > Actually, should we even be exposing AMP controllers to begin with > > through mgmt? Maybe we shouldn't even tell user space about them through > > mgmt? > > This seems to be right. BTW: What is the best way to disable mgmt for > device? > - Clear HCI_MGMT flag ? > - Do not run mgmt_powered ? > > BTW: Where that flag HCI_MGMT is set and how this check supposed to be > working? > > test_and_set_bit(HCI_MGMT, &hdev->dev_flags)) The HCI_MGMT flag indicates that there's a mgmt-capable user-space that is aware of the hci_dev. The first thing that comes to mind is to add checks to mgmt_index_added, mgmt_index_removed, mgmt_get_index_list and the place in mgmt_control that calls hci_dev_get. Since all of these places essentially would need to do the the same check it might be worth to wrap it into a macro, e.g. mgmt_valid_hdev(). Johan ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] Bluetooth: mgmt: Fix managing AMP device 2012-06-07 15:18 ` Johan Hedberg @ 2012-06-08 8:09 ` Andrei Emeltchenko 2012-06-12 13:31 ` [PATCHv2] " Andrei Emeltchenko 0 siblings, 1 reply; 7+ messages in thread From: Andrei Emeltchenko @ 2012-06-08 8:09 UTC (permalink / raw) To: linux-bluetooth From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> AMP device shall not be managed by user space for now. Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com> --- include/net/bluetooth/hci_core.h | 2 +- net/bluetooth/hci_core.c | 6 ++++-- net/bluetooth/mgmt.c | 20 ++++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 288eaa8..78d5fe2 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1080,7 +1080,7 @@ int mgmt_discovering(struct hci_dev *hdev, u8 discovering); int mgmt_interleaved_discovery(struct hci_dev *hdev); int mgmt_device_blocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type); int mgmt_device_unblocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type); - +bool mgmt_valid_hdev(struct hci_dev *hdev); int mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, u8 persistent); /* HCI info for socket */ diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 88d996b..25a86cd 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -699,7 +699,8 @@ int hci_dev_open(__u16 dev) hci_dev_hold(hdev); set_bit(HCI_UP, &hdev->flags); hci_notify(hdev, HCI_DEV_UP); - if (!test_bit(HCI_SETUP, &hdev->dev_flags)) { + if (!test_bit(HCI_SETUP, &hdev->dev_flags) && + mgmt_valid_hdev(hdev)) { hci_dev_lock(hdev); mgmt_powered(hdev, 1); hci_dev_unlock(hdev); @@ -801,7 +802,8 @@ static int hci_dev_do_close(struct hci_dev *hdev) * and no tasks are scheduled. */ hdev->close(hdev); - if (!test_and_clear_bit(HCI_AUTO_OFF, &hdev->dev_flags)) { + if (!test_and_clear_bit(HCI_AUTO_OFF, &hdev->dev_flags) && + mgmt_valid_hdev(hdev)) { hci_dev_lock(hdev); mgmt_powered(hdev, 0); hci_dev_unlock(hdev); diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 958f764..5ea1305 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -193,6 +193,11 @@ static u8 mgmt_status_table[] = { MGMT_STATUS_CONNECT_FAILED, /* MAC Connection Failed */ }; +bool mgmt_valid_hdev(struct hci_dev *hdev) +{ + return hdev->amp_type == HCI_BREDR; +} + static u8 mgmt_status(u8 hci_status) { if (hci_status < ARRAY_SIZE(mgmt_status_table)) @@ -329,6 +334,9 @@ static int read_index_list(struct sock *sk, struct hci_dev *hdev, void *data, count = 0; list_for_each(p, &hci_dev_list) { + if (!mgmt_valid_hdev(hdev)) + continue; + count++; } @@ -346,6 +354,9 @@ static int read_index_list(struct sock *sk, struct hci_dev *hdev, void *data, if (test_bit(HCI_SETUP, &d->dev_flags)) continue; + if (!mgmt_valid_hdev(hdev)) + continue; + rp->index[i++] = cpu_to_le16(d->id); BT_DBG("Added hci%u", d->id); } @@ -2135,6 +2146,9 @@ static int set_local_name(struct sock *sk, struct hci_dev *hdev, void *data, BT_DBG(""); + if (hdev->amp_type != HCI_BREDR) + return -ENOTSUPP; + hci_dev_lock(hdev); memcpy(hdev->short_name, cp->short_name, sizeof(hdev->short_name)); @@ -2813,6 +2827,9 @@ static void cmd_status_rsp(struct pending_cmd *cmd, void *data) int mgmt_index_added(struct hci_dev *hdev) { + if (!mgmt_valid_hdev(hdev)) + return -ENOTSUPP; + return mgmt_event(MGMT_EV_INDEX_ADDED, hdev, NULL, 0, NULL); } @@ -2820,6 +2837,9 @@ int mgmt_index_removed(struct hci_dev *hdev) { u8 status = MGMT_STATUS_INVALID_INDEX; + if (!mgmt_valid_hdev(hdev)) + return -ENOTSUPP; + mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status); return mgmt_event(MGMT_EV_INDEX_REMOVED, hdev, NULL, 0, NULL); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCHv2] Bluetooth: mgmt: Fix managing AMP device 2012-06-08 8:09 ` [PATCH] Bluetooth: mgmt: Fix managing AMP device Andrei Emeltchenko @ 2012-06-12 13:31 ` Andrei Emeltchenko 0 siblings, 0 replies; 7+ messages in thread From: Andrei Emeltchenko @ 2012-06-12 13:31 UTC (permalink / raw) To: linux-bluetooth From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> AMP device shall not be managed by user space for now. Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com> --- include/net/bluetooth/hci_core.h | 2 +- net/bluetooth/hci_core.c | 6 ++++-- net/bluetooth/mgmt.c | 23 +++++++++++++++++++++-- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 05d3340..40ce2fa 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1074,7 +1074,7 @@ int mgmt_discovering(struct hci_dev *hdev, u8 discovering); int mgmt_interleaved_discovery(struct hci_dev *hdev); int mgmt_device_blocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type); int mgmt_device_unblocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type); - +bool mgmt_valid_hdev(struct hci_dev *hdev); int mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, u8 persistent); /* HCI info for socket */ diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index f02325d..43ca889 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -698,7 +698,8 @@ int hci_dev_open(__u16 dev) hci_dev_hold(hdev); set_bit(HCI_UP, &hdev->flags); hci_notify(hdev, HCI_DEV_UP); - if (!test_bit(HCI_SETUP, &hdev->dev_flags)) { + if (!test_bit(HCI_SETUP, &hdev->dev_flags) && + mgmt_valid_hdev(hdev)) { hci_dev_lock(hdev); mgmt_powered(hdev, 1); hci_dev_unlock(hdev); @@ -799,7 +800,8 @@ static int hci_dev_do_close(struct hci_dev *hdev) * and no tasks are scheduled. */ hdev->close(hdev); - if (!test_and_clear_bit(HCI_AUTO_OFF, &hdev->dev_flags)) { + if (!test_and_clear_bit(HCI_AUTO_OFF, &hdev->dev_flags) && + mgmt_valid_hdev(hdev)) { hci_dev_lock(hdev); mgmt_powered(hdev, 0); hci_dev_unlock(hdev); diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index c72307c..68cc079 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -193,6 +193,11 @@ static u8 mgmt_status_table[] = { MGMT_STATUS_CONNECT_FAILED, /* MAC Connection Failed */ }; +bool mgmt_valid_hdev(struct hci_dev *hdev) +{ + return hdev->amp_type == HCI_BREDR; +} + static u8 mgmt_status(u8 hci_status) { if (hci_status < ARRAY_SIZE(mgmt_status_table)) @@ -317,7 +322,6 @@ static int read_index_list(struct sock *sk, struct hci_dev *hdev, void *data, u16 data_len) { struct mgmt_rp_read_index_list *rp; - struct list_head *p; struct hci_dev *d; size_t rp_len; u16 count; @@ -328,7 +332,10 @@ static int read_index_list(struct sock *sk, struct hci_dev *hdev, void *data, read_lock(&hci_dev_list_lock); count = 0; - list_for_each(p, &hci_dev_list) { + list_for_each_entry(d, &hci_dev_list, list) { + if (!mgmt_valid_hdev(d)) + continue; + count++; } @@ -346,6 +353,9 @@ static int read_index_list(struct sock *sk, struct hci_dev *hdev, void *data, if (test_bit(HCI_SETUP, &d->dev_flags)) continue; + if (!mgmt_valid_hdev(d)) + continue; + rp->index[i++] = cpu_to_le16(d->id); BT_DBG("Added hci%u", d->id); } @@ -2153,6 +2163,9 @@ static int set_local_name(struct sock *sk, struct hci_dev *hdev, void *data, BT_DBG(""); + if (hdev->amp_type != HCI_BREDR) + return -ENOTSUPP; + hci_dev_lock(hdev); memcpy(hdev->short_name, cp->short_name, sizeof(hdev->short_name)); @@ -2831,6 +2844,9 @@ static void cmd_status_rsp(struct pending_cmd *cmd, void *data) int mgmt_index_added(struct hci_dev *hdev) { + if (!mgmt_valid_hdev(hdev)) + return -ENOTSUPP; + return mgmt_event(MGMT_EV_INDEX_ADDED, hdev, NULL, 0, NULL); } @@ -2838,6 +2854,9 @@ int mgmt_index_removed(struct hci_dev *hdev) { u8 status = MGMT_STATUS_INVALID_INDEX; + if (!mgmt_valid_hdev(hdev)) + return -ENOTSUPP; + mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status); return mgmt_event(MGMT_EV_INDEX_REMOVED, hdev, NULL, 0, NULL); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-06-12 13:31 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-06-06 15:00 [RFC] Bluetooth: AMP: Do not set name for AMP ctrl Andrei Emeltchenko 2012-06-06 15:22 ` Johan Hedberg 2012-06-06 15:27 ` Johan Hedberg 2012-06-07 6:40 ` Andrei Emeltchenko 2012-06-07 15:18 ` Johan Hedberg 2012-06-08 8:09 ` [PATCH] Bluetooth: mgmt: Fix managing AMP device Andrei Emeltchenko 2012-06-12 13:31 ` [PATCHv2] " Andrei Emeltchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox