Linux bluetooth development
 help / color / mirror / Atom feed
* RE: [PATCH 3/3] Bluetooth: Ignore key unauthenticated for high security
From: Waldemar.Rymarkiewicz @ 2011-04-18 13:19 UTC (permalink / raw)
  To: Waldemar.Rymarkiewicz, linux-bluetooth; +Cc: johan.hedberg
In-Reply-To: <1302865617-32704-4-git-send-email-waldemar.rymarkiewicz@tieto.com>

Hi, 

>Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com>
>---
> net/bluetooth/hci_event.c |   21 +++++++++++++++++----
> 1 files changed, 17 insertions(+), 4 deletions(-)
>
>diff --git a/net/bluetooth/hci_event.c 
>b/net/bluetooth/hci_event.c index 5c5e614..337da2b 100644
>--- a/net/bluetooth/hci_event.c
>+++ b/net/bluetooth/hci_event.c
>@@ -2044,11 +2044,24 @@ static inline void 
>hci_link_key_request_evt(struct hci_dev *hdev, struct sk_buff
> 	}
> 
> 	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
>+	if (conn) {
>+		if (key->type == HCI_LK_UNAUTH_COMBINATION &&
>+				conn->auth_type != 0xff &&
>+				(conn->auth_type & 0x01)) {
>+			BT_DBG("%s ignoring unauthenticated 
>key", hdev->name);
>+			goto not_found;
>+		}
> 
>-	if (key->type == HCI_LK_UNAUTH_COMBINATION && conn &&
>-			conn->auth_type != 0xff && 
>(conn->auth_type & 0x01)) {
>-		BT_DBG("%s ignoring unauthenticated key", hdev->name);
>-		goto not_found;
>+		if (key->type == HCI_LK_COMBINATION &&
>+					conn->sec_level == 
>BT_SECURITY_HIGH &&
>+					conn->pin_length < 16) {


That's wrong. I should check it against stored key->pin_len  and  conn->pending_sec_level. 
We are in the middle of authentication so we don't have conn->sec_level set properly yet. The same apply for conn->pin_length.

if (key->type == HCI_LK_COMBINATION && key->pin_len < 16 &&
	conn->pending_sec_level == BT_SECURITY_HIGH) {
			goto not_found;
		}

/Waldek

^ permalink raw reply

* Re: [Regression] Bluetooth pairing does not work anymore in 2.6.39-rc3 (works in 2.6.38.3)
From: Pavel Machek @ 2011-04-18 13:21 UTC (permalink / raw)
  To: Gottfried Haider; +Cc: linux-kernel, linux-bluetooth
In-Reply-To: <BANLkTi=LvyZ+7BHfVL849pztfvsYaVM4SQ@mail.gmail.com>

Hi!

> I am having problems pairing bluetooth devices in 2.6.39. Scanning
> works fine (hcitool scan), but when I try to pair a device using
> "bluez-simple-agent hci0 $MAC", I receive the following error message:
> "Creating device failed: org.bluez.Error.Failed: Input/output error"
> 
> The same command reliably works on a 2.6.38.3 kernel with pretty much
> the same config.
> 
> According to lsusb, I have a Broadcom Bluetooth Controller with ID
> 0a5c:217f. I don't see anything related in either of the dmesgs.
> My lspci output is here: http://sukzessiv.net/x120e/
> Userspace is an up-to-date Ubuntu 11.04 beta (with bluez 4.91).
> 
> If there are any debugging steps that I could do or patches to try or
> revert, let me know. Unfortunately I can't do a proper bisect since
> the machine is a netbook type.

I guess strace of the pairing attempt would be helpful.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [Regression] Bluetooth pairing does not work anymore in 2.6.39-rc3 (works in 2.6.38.3)
From: Justin P. Mattock @ 2011-04-18 15:47 UTC (permalink / raw)
  To: Gottfried Haider; +Cc: linux-kernel, linux-bluetooth, padovan
In-Reply-To: <BANLkTikihzxtp9ep3dE1zpPGKUGm1gij8Q@mail.gmail.com>

On 04/18/2011 12:34 AM, Gottfried Haider wrote:
>>> I checked: I have this commit in -rc3, so it must be something else here.
>> hmm... seems to be working over here with the latest Mainline. one thing  I
>> have notice though is the daemon is not starting during boot(systemd),
>> manually starting bluetoothd gets me to connect.(system is fedora 15).
>
> It was my fault after all:
>
> I had based my kernel config on 2.6.39-rc2 from Ubuntu's
> mainline-kernel PPA, and this doesn't have BT_L2CAP..
>
> Case closed.
>

thats alright...(your human!!!)

>
> I am just wondering: is it possible to make BT_L2CAP (and SCO?)
> default to yes when BT is compiled in or build as a module?

tweaking the kconfig can do this.

>
>> From commit 6427451: "The L2CAP layer is needed by almost all
> Bluetooth protocols and profiles. There isn't any real use case
> without having L2CAP loaded." Yet when I am doing oldconfig from a
> config that had L2CAP build as a module, I am getting these easily
> overlooked lines:
>
> L2CAP protocol support (BT_L2CAP) [N/y/?] (NEW)
> SCO links support (BT_SCO) [N/y/?] (NEW)
>
>
> cheers,
> gohai
>

Justin P. Mattock

^ permalink raw reply

* Re: [PATCH] Bluetooth: Only keeping SAR bits when retransmitting one frame.
From: Gustavo F. Padovan @ 2011-04-18 17:53 UTC (permalink / raw)
  To: Ruiyi Zhang; +Cc: linux-bluetooth
In-Reply-To: <1303095870-8744-1-git-send-email-Ruiyi.zhang@atheros.com>

Hi Ruiyi,

* Ruiyi Zhang <Ruiyi.zhang@atheros.com> [2011-04-18 11:04:30 +0800]:

> When retrasmitting one frame, only SAR bits in control field should
> be kept.
> 
> Signed-off-by: Ruiyi Zhang <Ruiyi.zhang@atheros.com>
> ---
>  net/bluetooth/l2cap_core.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

Good catch! Applied, thanks.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH] bluetooth: fix shutdown on SCO sockets
From: Gustavo F. Padovan @ 2011-04-18 17:56 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <BANLkTin2kuhT9vyANhLHTqw8G8maTEbQWg@mail.gmail.com>

* Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-04-17 20:26:53 +0300]:

> Hi Gustavo,
> 
> On Fri, Apr 15, 2011 at 9:58 PM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> > Hi Luiz,
> >
> > * Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-04-08 17:10:41 +0300]:
> >
> >> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
> >>
> >> shutdown should wait for SCO link to be properly disconnected before
> >> detroying the socket, otherwise an application using the socket may
> >> assume link is properly disconnected before it really happens which
> >> can be a problem when e.g synchronizing profile switch.
> >>
> >> Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
> >
> > I applied it, but in bluetooth-next. Let's see its behaviour there and if no
> > problems show up we can move it to bluetooth-2.6
> 
> I tested this against Nokia BH-504 and Sony Ericsson W600, both have
> problem when switching from hfp to a2dp where the avdtp start is sent
> before SCO is fully disconnected, this patch fixes with those
> headsets.

Ok, I also pushed it to bluetooth-2.6.


-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [RFC] Proposal to distinguish address device types
From: Claudio Takahasi @ 2011-04-18 18:20 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Gustavo F. Padovan
  Cc: Claudio Takahasi, linux-bluetooth
In-Reply-To: <BANLkTi=QNU7MS80fKfsAvRRKAX+80hmQtA@mail.gmail.com>

Hi Marcel/Johan/Gustavo,

On Sat, Apr 16, 2011 at 5:04 PM, Claudio Takahasi
<claudio.takahasi@openbossa.org> wrote:
> Hi Marcel,
>
> On Sat, Apr 16, 2011 at 7:00 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> Hi Marcel,
>>
>> On Fri, Apr 15, 2011, Marcel Holtmann wrote:
>>> > > + BDADDR_TYPE_LE_PUBLIC,
>>> > > + BRADDR_TYPE_LE_RANDOM
>>> > > +};
>>>
>>> I am also not sure the we should have this BR/EDR differentiation since
>>> the specification only talks about public and random addresses. And we
>>> should follow the specification type value here. I am against
>>> introducing our enum here.
>>
>> The HCI specification only has values for public and random because
>> everywhere they are used it is already clear from the context (the HCI
>> command or event in question) if we're talking about LE or BR/EDR. We on
>> the other hand don't have this contextual information with the
>> mgmt_pair_device command. Saying "public" there could mean both BR/EDR
>> public or LE public, i.e. an enum with just two possible values is not
>> going to be of much use to us. Because of this difference between our
>> API and that of HCI I don't think it's fair to apply the HCI
>> convention/restriction to us.
>>
>> Johan
>>
>
> I added 3 values because it gives more flexibility. Possible use cases:
> - Whitelist needs the address type
> - SMP
> - As input to decide to store or not information about the device
> since private address can change every 15minutes
>
>  At the moment we only need to know if the address is basic rate or LE
> to select the discovery type: SDP or LE Discovery primary. For
> pairing, Vinicius is using the kernel advertising cache to discover
> the address type, passing the address type could avoid wrong fallback
> to basic rate if the entry is not found in the cache.
>
> Claudio
>

Any objection to add the address type in the MGMT_EV_DEVICE_CONNECTED event?
Inside event.c there are a lot of get_adapter_and_device calls, for
some contexts it creates a new device object if it doesn't exist(eg:
incoming pairing). But the type is required to create a new device,
otherwise it will not be possible to trigger the reverse service
search. Obtain the type later based on the link key type will not
work, unless we create a device with unknown type to be able to call
the agent methods.

BTW, is there a reason why it is necessary to "force" device
creation(get_adapter_and_device option)? In my opinion we could create
the device(if it doesn't exist) it inside btd_event_conn_complete
only. There is a potential race condition: other application calling
RemoveDevice, but for this case reference counting should work.

Currently, controllers doesn't support simultaneous BR/EDR/LE, this is
another argument to export the address type or connection type through
management interface avoiding future changes on the API.


Regards,
Claudio.

^ permalink raw reply

* pull request: bluetooth-next-2.6 2011-04-18
From: Gustavo F. Padovan @ 2011-04-18 22:48 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, linux-bluetooth

Hi John,

Biggest changes are the initial patches for an refactor inside the L2CAP code:
we have to separate the L2CAP socket stuff from the L2CAP core code to make
life of L2CAP users(rfcomm and AMP manager in the future) inside the kernel easy.

Other than that we basically bugfixes and cleanups.

Please pull, thanks!


The following changes since commit a90c7a313a1c5b4fc99f987a2ae8f92ab0ae35c7:

  cfg80211: add a timer for invalid user reg hints (2011-04-07 15:52:29 -0400)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-next-2.6.git master

Gustavo F. Padovan (26):
      Bluetooth: Create struct l2cap_chan
      Bluetooth: Use struct list_head for L2CAP channels list
      Bluetooth: Remove struct del_list
      Bluetooth: Move ident to struct l2cap_chan
      Bluetooth: Move conf_{req,rsp} stuff to struct l2cap_chan
      Bluetooth: clean up l2cap_sock_recvmsg()
      Bluetooth: Move conn_state to struct l2cap_chan
      Bluetooth: Move of ERTM *_seq vars to struct l2cap_chan
      Bluetooth: Move more ERTM stuff to struct l2cap_chan
      Bluetooth: Move SDU related vars to struct l2cap_chan
      Bluetooth: Move remote info to struct l2cap_chan
      Bluetooth: Move ERTM timers to struct l2cap_chan
      Bluetooth: Move srej and busy queues to struct l2cap_chan
      Bluetooth: Move busy workqueue to struct l2cap_chan
      Bluetooth: Fix lockdep warning with skb list lock
      Bluetooth: Move SREJ list to struct l2cap_chan
      Bluetooth: Remove some sk references from l2cap_core.c
      Bluetooth: Remove unneeded uninitialized_vars()
      Bluetooth: Move tx queue to struct l2cap_chan
      Bluetooth: Fix wrong comparison in listen()
      Bluetooth: Clean up ath3k_load_firmware()
      Bluetooth: Add proper handling of received LE data
      Bluetooth: Check return value of hci_recv_stream_fragment()
      Bluetooth: Don't lock sock inside l2cap_get_sock_by_scid()
      Bluetooth: Fix another locking unbalance
      Bluetooth: Fix lockdep warning in L2CAP

Jiejing Zhang (1):
      Bluetooth: hci_uart: check the return value of recv()

Kevin Gan (1):
      Bluetooth: btmrvl: support Marvell Bluetooth device SD8787

Szymon Janc (1):
      Bluetooth: Fix Out Of Band pairing when mgmt interface is disabled

 drivers/bluetooth/Kconfig       |    4 +-
 drivers/bluetooth/ath3k.c       |    3 -
 drivers/bluetooth/btmrvl_sdio.c |  124 +++-
 drivers/bluetooth/btmrvl_sdio.h |   68 +-
 drivers/bluetooth/hci_ath.c     |    7 +-
 drivers/bluetooth/hci_h4.c      |    7 +-
 drivers/bluetooth/hci_ldisc.c   |    6 +-
 include/net/bluetooth/l2cap.h   |  130 ++--
 net/bluetooth/hci_event.c       |    4 +
 net/bluetooth/l2cap_core.c      | 1344 +++++++++++++++++++++------------------
 net/bluetooth/l2cap_sock.c      |   72 +--
 11 files changed, 964 insertions(+), 805 deletions(-)

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [BUG] bluetooth doesn't work from 2.6.39-rc1+ to 2.6.39-rc3+
From: Dave Young @ 2011-04-19  7:46 UTC (permalink / raw)
  To: Hui Zhu; +Cc: linux-kernel, Bluettooth Linux
In-Reply-To: <BANLkTi=Ei91GRcL2oP0G16LDy7mpLoDEGQ@mail.gmail.com>

On Tue, Apr 19, 2011 at 10:53 AM, Hui Zhu <teawater@gmail.com> wrote:
> Cannot connect to any outside devices through bluetooth.
>
> And I think this is not a bug of low level driver.  Because both my
> laptop and a usb bluetooth card cannot work.

Hi, please explain the "cannot work" in detail,
Firstly check dmesg and hciconfig to see if there's anything wrong.

-- 
Regards
dave

^ permalink raw reply

* [PATCH v2] Fix not waiting for POLLERR when disconnecting SCO
From: Luiz Augusto von Dentz @ 2011-04-19 10:38 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

To make sure the SCO link is really disconnected we should wait for
POLLERR since POLLHUP does not necessarily means the link is
completely disconnected just that no further data can be sent/received.

Note that this depend on a fix of SCO socket shutdown in kernel to wait
for disconnect confimation to then kill/destroy the socket indicating
the err/reason using POLLERR.
---
 audio/headset.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/audio/headset.c b/audio/headset.c
index c605e9d..8fc83be 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -2547,8 +2547,11 @@ void headset_set_state(struct audio_device *dev, headset_state_t state)
 		emit_property_changed(dev->conn, dev->path,
 					AUDIO_HEADSET_INTERFACE, "State",
 					DBUS_TYPE_STRING, &state_str);
+
+		/* Do not watch HUP since we need to know when the link is
+		   really disconnected */
 		hs->sco_id = g_io_add_watch(hs->sco,
-					G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+					G_IO_ERR | G_IO_NVAL,
 					(GIOFunc) sco_cb, dev);
 
 		g_dbus_emit_signal(dev->conn, dev->path,
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH v2] Fix not waiting for POLLERR when disconnecting SCO
From: Johan Hedberg @ 2011-04-19 12:30 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1303209490-14760-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Tue, Apr 19, 2011, Luiz Augusto von Dentz wrote:
> To make sure the SCO link is really disconnected we should wait for
> POLLERR since POLLHUP does not necessarily means the link is
> completely disconnected just that no further data can be sent/received.
> 
> Note that this depend on a fix of SCO socket shutdown in kernel to wait
> for disconnect confimation to then kill/destroy the socket indicating
> the err/reason using POLLERR.
> ---
>  audio/headset.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [BUG] bluetooth doesn't work from 2.6.39-rc1+ to 2.6.39-rc3+
From: Hui Zhu @ 2011-04-19 15:34 UTC (permalink / raw)
  To: Dave Young; +Cc: linux-kernel, Bluettooth Linux
In-Reply-To: <BANLkTi=-RnUyuC2VGuRix_J4KOx+9BhroQ@mail.gmail.com>

Do you part is OK?

I think it will be very easy to reproduce.

Thanks,
Hui

On Tue, Apr 19, 2011 at 15:46, Dave Young <hidave.darkstar@gmail.com> wrote:
> On Tue, Apr 19, 2011 at 10:53 AM, Hui Zhu <teawater@gmail.com> wrote:
>> Cannot connect to any outside devices through bluetooth.
>>
>> And I think this is not a bug of low level driver.  Because both my
>> laptop and a usb bluetooth card cannot work.
>
> Hi, please explain the "cannot work" in detail,
> Firstly check dmesg and hciconfig to see if there's anything wrong.
>
> --
> Regards
> dave
>

^ permalink raw reply

* BT 3.0 HS Support in BlueZ
From: Anurag Gupta @ 2011-04-19 17:36 UTC (permalink / raw)
  To: linux-bluetooth

Hello all,

Does anybody know if bluetooth 3.0 HS spec is supported in BlueZ?
Is AMP supported in BlueZ?

Thanks & Regards
Anurag

^ permalink raw reply

* Re: BT 3.0 HS Support in BlueZ
From: Gustavo F. Padovan @ 2011-04-19 18:04 UTC (permalink / raw)
  To: Anurag Gupta; +Cc: linux-bluetooth
In-Reply-To: <BANLkTimKJNo9WgwkfC2Hci-dW6kjF3VWGw@mail.gmail.com>

Hi Anurag,

* Anurag Gupta <anurag1303@gmail.com> [2011-04-19 23:06:57 +0530]:

> Hello all,
> 
> Does anybody know if bluetooth 3.0 HS spec is supported in BlueZ?
> Is AMP supported in BlueZ?

There is two different implementations, one by Atheros and another by QuIC.
Both never reach upstream, if you look to the list logs you will find them.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* [PATCH 0/4] Remove deprecated fields from struct hci_dev
From: Bruna Moreira @ 2011-04-19 20:33 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira

These patches remove unused fields from struct hci_dev. The adapter name is
moved to struct btd_adapter. The entire struct hci_dev will be gone once the
device discovery logic is moved to the hciops/mgmtops plugin.

The last patch removes the read_local_version() from struct btd_adapter_ops. It
was being used only by code which was removed, but we left it in a separate
patch because we don't know if it is okay to remove it completely.

Note that no removed fields are used on D-Bus API nor internally.

Bruna Moreira (4):
  Remove ssp_mode from struct hci_dev
  Remove LMP/HCI version and manufacturer from struct hci_dev
  Move adapter name from struct hci_dev to struct btd_adapter
  Remove read_local_version() from struct btd_adapter_ops

 plugins/hciops.c  |   21 ---------------------
 plugins/mgmtops.c |   18 ------------------
 src/adapter.c     |   52 ++++++++++++++--------------------------------------
 src/adapter.h     |    9 ---------
 4 files changed, 14 insertions(+), 86 deletions(-)


^ permalink raw reply

* [PATCH 1/4] Remove ssp_mode from struct hci_dev
From: Bruna Moreira @ 2011-04-19 20:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1303245243-12116-1-git-send-email-bruna.moreira@openbossa.org>

ssp_mode is duplicated information and is not being used anywhere.
---
 plugins/hciops.c |    9 ---------
 src/adapter.c    |    9 ---------
 src/adapter.h    |    3 ---
 3 files changed, 0 insertions(+), 21 deletions(-)

diff --git a/plugins/hciops.c b/plugins/hciops.c
index 93f6f21..7d2eca3 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -1658,7 +1658,6 @@ static void read_simple_pairing_mode_complete(int index, void *ptr)
 {
 	struct dev_info *dev = &devs[index];
 	read_simple_pairing_mode_rp *rp = ptr;
-	struct btd_adapter *adapter;
 
 	DBG("hci%d status %u", index, rp->status);
 
@@ -1667,14 +1666,6 @@ static void read_simple_pairing_mode_complete(int index, void *ptr)
 
 	dev->ssp_mode = rp->mode;
 	update_ext_inquiry_response(index);
-
-	adapter = manager_find_adapter(&dev->bdaddr);
-	if (!adapter) {
-		error("No matching adapter found");
-		return;
-	}
-
-	adapter_update_ssp_mode(adapter, rp->mode);
 }
 
 static void read_local_ext_features_complete(int index,
diff --git a/src/adapter.c b/src/adapter.c
index dbc2c93..06119dd 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2623,15 +2623,6 @@ int btd_adapter_stop(struct btd_adapter *adapter)
 	return 0;
 }
 
-int adapter_update_ssp_mode(struct btd_adapter *adapter, uint8_t mode)
-{
-	struct hci_dev *dev = &adapter->dev;
-
-	dev->ssp_mode = mode;
-
-	return 0;
-}
-
 static void adapter_free(gpointer user_data)
 {
 	struct btd_adapter *adapter = user_data;
diff --git a/src/adapter.h b/src/adapter.h
index 308af75..14338b5 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -97,7 +97,6 @@ struct hci_dev {
 	uint16_t hci_rev;
 	uint16_t manufacturer;
 
-	uint8_t  ssp_mode;
 	char     name[MAX_NAME_LENGTH + 1];
 };
 
@@ -108,8 +107,6 @@ int btd_adapter_stop(struct btd_adapter *adapter);
 void btd_adapter_get_mode(struct btd_adapter *adapter, uint8_t *mode,
 					uint8_t *on_mode, gboolean *pairable);
 
-int adapter_update_ssp_mode(struct btd_adapter *adapter, uint8_t mode);
-
 struct btd_device *adapter_get_device(DBusConnection *conn,
 				struct btd_adapter *adapter, const char *address);
 
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 2/4] Remove LMP/HCI version and manufacturer from struct hci_dev
From: Bruna Moreira @ 2011-04-19 20:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1303245243-12116-1-git-send-email-bruna.moreira@openbossa.org>

These fields were not being used anywhere inside BlueZ.
---
 src/adapter.c |   13 -------------
 src/adapter.h |    4 ----
 2 files changed, 0 insertions(+), 17 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 06119dd..6707241 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2675,7 +2675,6 @@ void btd_adapter_unref(struct btd_adapter *adapter)
 
 gboolean adapter_init(struct btd_adapter *adapter)
 {
-	struct hci_version ver;
 	struct hci_dev *dev;
 	int err;
 
@@ -2690,20 +2689,8 @@ gboolean adapter_init(struct btd_adapter *adapter)
 		return FALSE;
 	}
 
-	err = adapter_ops->read_local_version(adapter->dev_id, &ver);
-	if (err < 0) {
-		error("Can't read version info for hci%d: %s (%d)",
-					adapter->dev_id, strerror(-err), -err);
-		return FALSE;
-	}
-
 	dev = &adapter->dev;
 
-	dev->hci_rev = ver.hci_rev;
-	dev->lmp_ver = ver.lmp_ver;
-	dev->lmp_subver = ver.lmp_subver;
-	dev->manufacturer = ver.manufacturer;
-
 	err = adapter_ops->read_local_features(adapter->dev_id, dev->features);
 	if (err < 0) {
 		error("Can't read features for hci%d: %s (%d)",
diff --git a/src/adapter.h b/src/adapter.h
index 14338b5..01659f1 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -92,10 +92,6 @@ struct remote_dev_info {
 struct hci_dev {
 	uint8_t  features[8];
 	uint8_t  extfeatures[8];
-	uint8_t  lmp_ver;
-	uint16_t lmp_subver;
-	uint16_t hci_rev;
-	uint16_t manufacturer;
 
 	char     name[MAX_NAME_LENGTH + 1];
 };
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 3/4] Move adapter name from struct hci_dev to struct btd_adapter
From: Bruna Moreira @ 2011-04-19 20:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1303245243-12116-1-git-send-email-bruna.moreira@openbossa.org>

It makes more sense to have this information on the adapter structure,
as it is not HCI specific.
---
 src/adapter.c |   30 ++++++++++++++----------------
 src/adapter.h |    1 -
 2 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 6707241..2b9f719 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -113,6 +113,7 @@ struct btd_adapter {
 	char *path;			/* adapter object path */
 	bdaddr_t bdaddr;		/* adapter Bluetooth Address */
 	uint32_t dev_class;		/* Class of Device */
+	char name[MAX_NAME_LENGTH + 1]; /* adapter name */
 	guint discov_timeout_id;	/* discoverable timeout id */
 	guint stop_discov_id;		/* stop inquiry/scanning id */
 	uint32_t discov_timeout;	/* discoverable time(sec) */
@@ -926,21 +927,19 @@ void btd_adapter_class_changed(struct btd_adapter *adapter, uint32_t new_class)
 
 void adapter_update_local_name(struct btd_adapter *adapter, const char *name)
 {
-	struct hci_dev *dev = &adapter->dev;
-
-	if (strncmp(name, dev->name, MAX_NAME_LENGTH) == 0)
+	if (strncmp(name, adapter->name, MAX_NAME_LENGTH) == 0)
 		return;
 
-	strncpy(dev->name, name, MAX_NAME_LENGTH);
+	strncpy(adapter->name, name, MAX_NAME_LENGTH);
 
 	if (main_opts.attrib_server)
 		attrib_gap_set(GATT_CHARAC_DEVICE_NAME,
-			(const uint8_t *) dev->name, strlen(dev->name));
+			(const uint8_t *) adapter->name, strlen(adapter->name));
 
 	if (!adapter->name_stored) {
-		char *name_ptr = dev->name;
+		char *name_ptr = adapter->name;
 
-		write_local_name(&adapter->bdaddr, dev->name);
+		write_local_name(&adapter->bdaddr, adapter->name);
 
 		if (connection)
 			emit_property_changed(connection, adapter->path,
@@ -955,18 +954,17 @@ static DBusMessage *set_name(DBusConnection *conn, DBusMessage *msg,
 					const char *name, void *data)
 {
 	struct btd_adapter *adapter = data;
-	struct hci_dev *dev = &adapter->dev;
-	char *name_ptr = dev->name;
+	char *name_ptr = adapter->name;
 
 	if (!g_utf8_validate(name, -1, NULL)) {
 		error("Name change failed: supplied name isn't valid UTF-8");
 		return btd_error_invalid_args(msg);
 	}
 
-	if (strncmp(name, dev->name, MAX_NAME_LENGTH) == 0)
+	if (strncmp(name, adapter->name, MAX_NAME_LENGTH) == 0)
 		goto done;
 
-	strncpy(dev->name, name, MAX_NAME_LENGTH);
+	strncpy(adapter->name, name, MAX_NAME_LENGTH);
 	write_local_name(&adapter->bdaddr, name);
 	emit_property_changed(connection, adapter->path,
 					ADAPTER_INTERFACE, "Name",
@@ -1362,7 +1360,7 @@ static DBusMessage *get_properties(DBusConnection *conn,
 
 	/* Name */
 	memset(str, 0, sizeof(str));
-	strncpy(str, (char *) adapter->dev.name, MAX_NAME_LENGTH);
+	strncpy(str, (char *) adapter->name, MAX_NAME_LENGTH);
 	property = str;
 
 	dict_append_entry(&dict, "Name", DBUS_TYPE_STRING, &property);
@@ -2454,7 +2452,7 @@ void btd_adapter_start(struct btd_adapter *adapter)
 	if (main_opts.le)
 		adapter_ops->enable_le(adapter->dev_id);
 
-	adapter_ops->set_name(adapter->dev_id, adapter->dev.name);
+	adapter_ops->set_name(adapter->dev_id, adapter->name);
 
 	if (read_local_class(&adapter->bdaddr, cls) < 0) {
 		uint32_t class = htobl(main_opts.class);
@@ -2698,13 +2696,13 @@ gboolean adapter_init(struct btd_adapter *adapter)
 		return FALSE;
 	}
 
-	if (read_local_name(&adapter->bdaddr, adapter->dev.name) < 0)
-		expand_name(adapter->dev.name, MAX_NAME_LENGTH, main_opts.name,
+	if (read_local_name(&adapter->bdaddr, adapter->name) < 0)
+		expand_name(adapter->name, MAX_NAME_LENGTH, main_opts.name,
 							adapter->dev_id);
 
 	if (main_opts.attrib_server)
 		attrib_gap_set(GATT_CHARAC_DEVICE_NAME,
-			(const uint8_t *) dev->name, strlen(dev->name));
+			(const uint8_t *) adapter->name, strlen(adapter->name));
 
 	sdp_init_services_list(&adapter->bdaddr);
 	load_drivers(adapter);
diff --git a/src/adapter.h b/src/adapter.h
index 01659f1..4d8824e 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -93,7 +93,6 @@ struct hci_dev {
 	uint8_t  features[8];
 	uint8_t  extfeatures[8];
 
-	char     name[MAX_NAME_LENGTH + 1];
 };
 
 void btd_adapter_start(struct btd_adapter *adapter);
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 4/4] Remove read_local_version() from struct btd_adapter_ops
From: Bruna Moreira @ 2011-04-19 20:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1303245243-12116-1-git-send-email-bruna.moreira@openbossa.org>

This callback is used only for getting HCI version/manufacturer
information, and is not being called anywhere in BlueZ anymore.
---
 plugins/hciops.c  |   12 ------------
 plugins/mgmtops.c |   18 ------------------
 src/adapter.h     |    1 -
 3 files changed, 0 insertions(+), 31 deletions(-)

diff --git a/plugins/hciops.c b/plugins/hciops.c
index 7d2eca3..2b9be3f 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -3231,17 +3231,6 @@ static int hciops_get_conn_list(int index, GSList **conns)
 	return 0;
 }
 
-static int hciops_read_local_version(int index, struct hci_version *ver)
-{
-	struct dev_info *dev = &devs[index];
-
-	DBG("hci%d", index);
-
-	memcpy(ver, &dev->ver, sizeof(*ver));
-
-	return 0;
-}
-
 static int hciops_read_local_features(int index, uint8_t *features)
 {
 	struct dev_info *dev = &devs[index];
@@ -3696,7 +3685,6 @@ static struct btd_adapter_ops hci_ops = {
 	.block_device = hciops_block_device,
 	.unblock_device = hciops_unblock_device,
 	.get_conn_list = hciops_get_conn_list,
-	.read_local_version = hciops_read_local_version,
 	.read_local_features = hciops_read_local_features,
 	.disconnect = hciops_disconnect,
 	.remove_bonding = hciops_remove_bonding,
diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
index 042afc5..bb53e82 100644
--- a/plugins/mgmtops.c
+++ b/plugins/mgmtops.c
@@ -1706,23 +1706,6 @@ static int mgmt_get_conn_list(int index, GSList **conns)
 	return 0;
 }
 
-static int mgmt_read_local_version(int index, struct hci_version *ver)
-{
-	struct controller_info *info = &controllers[index];
-
-	DBG("index %d", index);
-
-	if (!info->valid)
-		return -ENODEV;
-
-	memset(ver, 0, sizeof(*ver));
-	ver->manufacturer = info->manufacturer;
-	ver->hci_ver = info->hci_ver;
-	ver->hci_rev = info->hci_rev;
-
-	return 0;
-}
-
 static int mgmt_read_local_features(int index, uint8_t *features)
 {
 	struct controller_info *info = &controllers[index];
@@ -2024,7 +2007,6 @@ static struct btd_adapter_ops mgmt_ops = {
 	.block_device = mgmt_block_device,
 	.unblock_device = mgmt_unblock_device,
 	.get_conn_list = mgmt_get_conn_list,
-	.read_local_version = mgmt_read_local_version,
 	.read_local_features = mgmt_read_local_features,
 	.disconnect = mgmt_disconnect,
 	.remove_bonding = mgmt_remove_bonding,
diff --git a/src/adapter.h b/src/adapter.h
index 4d8824e..ee3cae9 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -209,7 +209,6 @@ struct btd_adapter_ops {
 	int (*block_device) (int index, bdaddr_t *bdaddr);
 	int (*unblock_device) (int index, bdaddr_t *bdaddr);
 	int (*get_conn_list) (int index, GSList **conns);
-	int (*read_local_version) (int index, struct hci_version *ver);
 	int (*read_local_features) (int index, uint8_t *features);
 	int (*disconnect) (int index, bdaddr_t *bdaddr);
 	int (*remove_bonding) (int index, bdaddr_t *bdaddr);
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 1/2] Bluetooth: Add basic discovery commands to the management interface
From: anderson.briglia @ 2011-04-19 21:35 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Johan Hedberg, Anderson Briglia

From: Johan Hedberg <johan.hedberg@nokia.com>

This patch adds start_discovery and stop_discovery commands to the
management interface. Right now their implementation is fairly
simplistic and the parameters are fixed to what user space has
defaulted to so far.
This is the very initial phase for discovery implementation into
the kernel. Next steps include name resolution, LE scanning and
bdaddr type handling.

Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
Signed-off-by: Anderson Briglia <anderson.briglia@openbossa.org>
---
 include/net/bluetooth/mgmt.h |    4 ++
 net/bluetooth/mgmt.c         |   77 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 80 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 6b6ff92..be93dd0 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -195,6 +195,10 @@ struct mgmt_cp_remove_remote_oob_data {
 	bdaddr_t bdaddr;
 } __packed;
 
+#define MGMT_OP_START_DISCOVERY		0x001B
+
+#define MGMT_OP_STOP_DISCOVERY		0x001C
+
 #define MGMT_EV_CMD_COMPLETE		0x0001
 struct mgmt_ev_cmd_complete {
 	__le16 opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index c304688..958544a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1569,6 +1569,75 @@ static int remove_remote_oob_data(struct sock *sk, u16 index,
 	return err;
 }
 
+static int start_discovery(struct sock *sk, u16 index)
+{
+	u8 lap[3] = { 0x33, 0x8b, 0x9e };
+	struct hci_cp_inquiry cp;
+	struct pending_cmd *cmd;
+	struct hci_dev *hdev;
+	int err;
+
+	BT_DBG("hci%u ", index);
+
+	hdev = hci_dev_get(index);
+	if (!hdev)
+		return cmd_status(sk, index, MGMT_OP_START_DISCOVERY, ENODEV);
+
+	hci_dev_lock_bh(hdev);
+
+	cmd = mgmt_pending_add(sk, MGMT_OP_START_DISCOVERY, index, NULL, 0);
+	if (!cmd) {
+		err = -ENOMEM;
+		goto failed;
+	}
+
+	memset(&cp, 0, sizeof(cp));
+	memcpy(&cp.lap, lap, 3);
+	cp.length  = 0x08;
+	cp.num_rsp = 0x00;
+
+	err = hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+	if (err < 0)
+		mgmt_pending_remove(cmd);
+
+failed:
+	hci_dev_unlock_bh(hdev);
+	hci_dev_put(hdev);
+
+	return err;
+}
+
+static int stop_discovery(struct sock *sk, u16 index)
+{
+	struct hci_dev *hdev;
+	struct pending_cmd *cmd;
+	int err;
+
+	BT_DBG("hci%u ", index);
+
+	hdev = hci_dev_get(index);
+	if (!hdev)
+		return cmd_status(sk, index, MGMT_OP_STOP_DISCOVERY, ENODEV);
+
+	hci_dev_lock_bh(hdev);
+
+	cmd = mgmt_pending_add(sk, MGMT_OP_STOP_DISCOVERY, index, NULL, 0);
+	if (!cmd) {
+		err = -ENOMEM;
+		goto failed;
+	}
+
+	err = hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
+	if (err < 0)
+		mgmt_pending_remove(cmd);
+
+failed:
+	hci_dev_unlock_bh(hdev);
+	hci_dev_put(hdev);
+
+	return err;
+}
+
 int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
 {
 	unsigned char *buf;
@@ -1677,7 +1746,12 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
 		err = remove_remote_oob_data(sk, index, buf + sizeof(*hdr),
 									len);
 		break;
-
+	case MGMT_OP_START_DISCOVERY:
+		err = start_discovery(sk, index);
+		break;
+	case MGMT_OP_STOP_DISCOVERY:
+		err = stop_discovery(sk, index);
+		break;
 	default:
 		BT_DBG("Unknown op %u", opcode);
 		err = cmd_status(sk, index, opcode, 0x01);
@@ -2075,3 +2149,4 @@ int mgmt_remote_name(u16 index, bdaddr_t *bdaddr, u8 *name)
 
 	return mgmt_event(MGMT_EV_REMOTE_NAME, index, &ev, sizeof(ev), NULL);
 }
+
-- 
1.7.1


^ permalink raw reply related

* [PATCH 2/2] Bluetooth: Add discovering event to the Management interface
From: anderson.briglia @ 2011-04-19 21:35 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Johan Hedberg, Anderson Briglia
In-Reply-To: <1303248926-16607-1-git-send-email-y>

From: Johan Hedberg <johan.hedberg@nokia.com>

This patch adds a new event to the Management interface to track when
local adapters are discovering remote devices. For now this only tracks
BR/EDR discovery procedures.

Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
Signed-off-by: Anderson Briglia <anderson.briglia@openbossa.org>
---
 include/net/bluetooth/hci_core.h |    1 +
 include/net/bluetooth/mgmt.h     |    2 +
 net/bluetooth/hci_event.c        |   44 +++++++++++++++++++++++++++++++++++--
 net/bluetooth/mgmt.c             |    6 +++++
 4 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4093133..69967e5 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -790,6 +790,7 @@ int mgmt_read_local_oob_data_reply_complete(u16 index, u8 *hash, u8 *randomizer,
 int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 *dev_class, s8 rssi,
 								u8 *eir);
 int mgmt_remote_name(u16 index, bdaddr_t *bdaddr, u8 *name);
+int mgmt_discovering(u16 index, u8 discovering);
 
 /* HCI info for socket */
 #define hci_pi(sk) ((struct hci_pinfo *) sk)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index be93dd0..7434406 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -285,3 +285,5 @@ struct mgmt_ev_remote_name {
 	bdaddr_t bdaddr;
 	__u8 name[MGMT_MAX_NAME_LENGTH];
 } __packed;
+
+#define MGMT_EV_DISCOVERING		0x0014
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index c7eb073..f356ed7 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -56,6 +56,10 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
 	if (status)
 		return;
 
+	if (test_bit(HCI_MGMT, &hdev->flags) &&
+					test_bit(HCI_INQUIRY, &hdev->flags))
+		mgmt_discovering(hdev->id, 0);
+
 	clear_bit(HCI_INQUIRY, &hdev->flags);
 
 	hci_req_complete(hdev, HCI_OP_INQUIRY_CANCEL, status);
@@ -72,6 +76,10 @@ static void hci_cc_exit_periodic_inq(struct hci_dev *hdev, struct sk_buff *skb)
 	if (status)
 		return;
 
+	if (test_bit(HCI_MGMT, &hdev->flags) &&
+					test_bit(HCI_INQUIRY, &hdev->flags))
+		mgmt_discovering(hdev->id, 0);
+
 	clear_bit(HCI_INQUIRY, &hdev->flags);
 
 	hci_conn_check_pending(hdev);
@@ -841,10 +849,15 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 
 	if (status) {
 		hci_req_complete(hdev, HCI_OP_INQUIRY, status);
-
 		hci_conn_check_pending(hdev);
-	} else
-		set_bit(HCI_INQUIRY, &hdev->flags);
+		return;
+	}
+
+	if (test_bit(HCI_MGMT, &hdev->flags) &&
+					!test_bit(HCI_INQUIRY, &hdev->flags))
+		mgmt_discovering(hdev->id, 1);
+
+	set_bit(HCI_INQUIRY, &hdev->flags);
 }
 
 static inline void hci_cs_create_conn(struct hci_dev *hdev, __u8 status)
@@ -1208,6 +1221,10 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
 
 	BT_DBG("%s status %d", hdev->name, status);
 
+	if (test_bit(HCI_MGMT, &hdev->flags) &&
+					test_bit(HCI_INQUIRY, &hdev->flags))
+		mgmt_discovering(hdev->id, 0);
+
 	clear_bit(HCI_INQUIRY, &hdev->flags);
 
 	hci_req_complete(hdev, HCI_OP_INQUIRY, status);
@@ -1228,6 +1245,13 @@ static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *
 
 	hci_dev_lock(hdev);
 
+	if (!test_bit(HCI_INQUIRY, &hdev->flags)) {
+		set_bit(HCI_INQUIRY, &hdev->flags);
+
+		if (test_bit(HCI_MGMT, &hdev->flags))
+			mgmt_discovering(hdev->id, 1);
+	}
+
 	for (; num_rsp; num_rsp--, info++) {
 		bacpy(&data.bdaddr, &info->bdaddr);
 		data.pscan_rep_mode	= info->pscan_rep_mode;
@@ -2158,6 +2182,13 @@ static inline void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev, struct
 
 	hci_dev_lock(hdev);
 
+	if (!test_bit(HCI_INQUIRY, &hdev->flags)) {
+		set_bit(HCI_INQUIRY, &hdev->flags);
+
+		if (test_bit(HCI_MGMT, &hdev->flags))
+			mgmt_discovering(hdev->id, 1);
+	}
+
 	if ((skb->len - 1) / num_rsp != sizeof(struct inquiry_info_with_rssi)) {
 		struct inquiry_info_with_rssi_and_pscan_mode *info;
 		info = (void *) (skb->data + 1);
@@ -2320,6 +2351,13 @@ static inline void hci_extended_inquiry_result_evt(struct hci_dev *hdev, struct
 	if (!num_rsp)
 		return;
 
+	if (!test_bit(HCI_INQUIRY, &hdev->flags)) {
+		set_bit(HCI_INQUIRY, &hdev->flags);
+
+		if (test_bit(HCI_MGMT, &hdev->flags))
+			mgmt_discovering(hdev->id, 1);
+	}
+
 	hci_dev_lock(hdev);
 
 	for (; num_rsp; num_rsp--, info++) {
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 958544a..f623b8a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2150,3 +2150,9 @@ int mgmt_remote_name(u16 index, bdaddr_t *bdaddr, u8 *name)
 	return mgmt_event(MGMT_EV_REMOTE_NAME, index, &ev, sizeof(ev), NULL);
 }
 
+int mgmt_discovering(u16 index, u8 discovering)
+{
+	return mgmt_event(MGMT_EV_DISCOVERING, index, &discovering,
+						sizeof(discovering), NULL);
+}
+
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH] Bluetooth: MTU configuration for LE links
From: Anderson Briglia @ 2011-04-19 21:50 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Anderson Lizardo, linux-bluetooth, ville.tervo, johan.hedberg,
	Vinicius Costa Gomes
In-Reply-To: <1302736838.2572.262.camel@aeonflux>

Hi Marcel,

On Wed, Apr 13, 2011 at 7:20 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Anderson,
>
>> >> One thing to consider is that there are a couple of MTU checks along
>> >> the L2CAP code. The issue which originated this patch is code such as=
:
>> >>
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Check outgoing MTU */
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (len > pi->omtu) {
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -EMSGSIZE;
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto done;
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
>> >>
>> >> We understood that just letting omtu/imtu values on kernel reflect th=
e
>> >> current connection MTU settings was the cleanest solution. What do yo=
u
>> >> propose? Bypassing these checks for LE?
>> >
>> > this does not look clean to me. And we might have an internal MTU
>> > variable as part of L2CAP, but the way how it gets set and thus its
>> > semantic differs clearly between BR/EDR and LE. So shoehorning an
>> > existing socket option this is clearly a bad idea.
>>
>> Sure. What to do then if:
>>
>> 1) LE links have MTU (omtu specifically) hard-coded to 23 on kernel.
>> 2) the kernel rejects sending data longer than omtu.
>>
>> (this is the current situation)
>>
>> This clearly needs some fix on kernel side, otherwise we can't send
>> PDUs longer than the LE default MTU (23), *even* if the peer device
>> supports a bigger MTU.
>
> I agree with that. Userspace needs to be able to change the kernel MTU
> if a different one gets negotiated. That is not the problem. The problem
> is trying to shoehorn this into an existing (and already declared
> legacy) socket option.
>
>> Suggestions are welcome regarding how to best approach this, without
>> affecting current BR/EDR MTU semantics.
>
> We need to have a socket option that allows us the dynamically change
> the MTU of a L2CAP link. And right now this option will only be valid if
> the link is actually an LE link.

Ok, so we have some options IMHO:

a) Do not add a new option in struct l2cap_options because it will
still be hijacking.

b) Add a new option for LE OMTU in struct l2cap_conninfo. Is it valid?

c) Add a new struct (l2cap_le_options ?) and add a omtu field there.

The problem in c) is that we will have to implement significant
modifications in userspace and the code should be redundant: we will
have two options to represent the same value.

Regards,

Anderson Briglia

>
> Regards
>
> Marcel
>
>
>



--=20
INdT - Instituto Nokia de tecnologia
+55 2126 1122
http://techblog.briglia.net

^ permalink raw reply

* Re: [PATCH 1/2] Bluetooth: Add basic discovery commands to the management interface
From: Johan Hedberg @ 2011-04-19 21:58 UTC (permalink / raw)
  To: anderson.briglia; +Cc: linux-bluetooth
In-Reply-To: <4dae0084.a905ec0a.709a.12f6@mx.google.com>

Hi Briglia,

On Tue, Apr 19, 2011, anderson.briglia@openbossa.org wrote:
> From: Johan Hedberg <johan.hedberg@nokia.com>
> 
> This patch adds start_discovery and stop_discovery commands to the
> management interface. Right now their implementation is fairly
> simplistic and the parameters are fixed to what user space has
> defaulted to so far.
> This is the very initial phase for discovery implementation into
> the kernel. Next steps include name resolution, LE scanning and
> bdaddr type handling.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> Signed-off-by: Anderson Briglia <anderson.briglia@openbossa.org>
> ---
>  include/net/bluetooth/mgmt.h |    4 ++
>  net/bluetooth/mgmt.c         |   77 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 80 insertions(+), 1 deletions(-)

I'd appreciate it if you'd check with me first before going about
sending patches in my name that I haven't 100% authored. There are e.g.
some coding style things I'd have cleaned up before making this public.
Depending on the amount of code changed (maybe not enough for this case
though) you could have even put yourself as author and simply mentioned
"Based on original code by Johan..." somewhere in the commit message.

Johan

^ permalink raw reply

* Re: [PATCH 1/2] Bluetooth: Add basic discovery commands to the management interface
From: Anderson Briglia @ 2011-04-20 12:33 UTC (permalink / raw)
  To: anderson.briglia, linux-bluetooth
In-Reply-To: <20110419215807.GA26778@jh-x301>

Hi Johan,

On Tue, Apr 19, 2011 at 5:58 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Briglia,
>
> On Tue, Apr 19, 2011, anderson.briglia@openbossa.org wrote:
>> From: Johan Hedberg <johan.hedberg@nokia.com>
>>
>> This patch adds start_discovery and stop_discovery commands to the
>> management interface. Right now their implementation is fairly
>> simplistic and the parameters are fixed to what user space has
>> defaulted to so far.
>> This is the very initial phase for discovery implementation into
>> the kernel. Next steps include name resolution, LE scanning and
>> bdaddr type handling.
>>
>> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
>> Signed-off-by: Anderson Briglia <anderson.briglia@openbossa.org>
>> ---
>>  include/net/bluetooth/mgmt.h |    4 ++
>>  net/bluetooth/mgmt.c         |   77 +++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 80 insertions(+), 1 deletions(-)
>
> I'd appreciate it if you'd check with me first before going about
> sending patches in my name that I haven't 100% authored. There are e.g.
> some coding style things I'd have cleaned up before making this public.
> Depending on the amount of code changed (maybe not enough for this case
> though) you could have even put yourself as author and simply mentioned
> "Based on original code by Johan..." somewhere in the commit message.

Yes, you're right. Actually I thought just the "Signed-off-by" adding
myself would be enough since the code is pretty much the same and you
had already sent this few weeks ago. Anyway I should contact you first
before send to the mailing list, sorry.
What do you prefer? I modify the commit message and the patch
authoring (and send it again)? Which coding style issues did you
notice?

Regards,

Anderson Briglia

>
> Johan
>



-- 
INdT - Instituto Nokia de tecnologia
+55 2126 1122
http://techblog.briglia.net

^ permalink raw reply

* Re: [PATCH 1/2] Bluetooth: Add basic discovery commands to the management interface
From: Johan Hedberg @ 2011-04-20 13:09 UTC (permalink / raw)
  To: Anderson Briglia; +Cc: linux-bluetooth
In-Reply-To: <BANLkTik5UF9hPgHG1hwzWV2hG5ZLS8GXnQ@mail.gmail.com>

Hi,

On Wed, Apr 20, 2011, Anderson Briglia wrote:
> you had already sent this few weeks ago.

Could you point me to that thread? I'm not aware of sending these
patches before. AFAIK they were only available in my kernel.org tree
(which I use for experimental stuff not yet ready for upstream).

> What do you prefer? I modify the commit message and the patch
> authoring (and send it again)?

As I said, the changes are probably too minor to justify an author
change. My main point was that don't send stuff in other peoples name
without talking to them first.

> Which coding style issues did you notice?

Minor things like adding an extra empty line at the end of both patches
(shouldn't checkpatch.pl complain about that?) and an extra space in the
format string of a BT_DBG("hci%u ", index) debug print in the first patch.

Johan

^ permalink raw reply

* [PATCH] Cleanup: remove unused structure
From: Claudio Takahasi @ 2011-04-20 13:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi

---
 src/adapter.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index dbc2c93..5ee6d7f 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1320,11 +1320,6 @@ static DBusMessage *adapter_stop_discovery(DBusConnection *conn,
 	return dbus_message_new_method_return(msg);
 }
 
-struct remote_device_list_t {
-	GSList *list;
-	time_t time;
-};
-
 static DBusMessage *get_properties(DBusConnection *conn,
 					DBusMessage *msg, void *data)
 {
-- 
1.7.5.rc1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox