* [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported
From: Gustavo F. Padovan @ 2010-08-04 2:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, Gustavo F. Padovan
From: Gustavo F. Padovan <padovan@profusion.mobi>
If the remote side doesn't support Enhanced Retransmission Mode neither
Streaming Mode, we shall not send the RFC option.
Some devices that only supports Basic Mode do not understanding the RFC
option. This patch fix the regression found with that devices.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/l2cap.c | 15 ++++++++++++---
1 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9ba1e8e..0f34e12 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2527,6 +2527,10 @@ done:
if (pi->imtu != L2CAP_DEFAULT_MTU)
l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
+ if (!(pi->conn->feat_mask & L2CAP_FEAT_ERTM) &&
+ !(pi->conn->feat_mask & L2CAP_FEAT_STREAMING))
+ break;
+
rfc.mode = L2CAP_MODE_BASIC;
rfc.txwin_size = 0;
rfc.max_transmit = 0;
@@ -2534,6 +2538,8 @@ done:
rfc.monitor_timeout = 0;
rfc.max_pdu_size = 0;
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long) &rfc);
break;
case L2CAP_MODE_ERTM:
@@ -2546,6 +2552,9 @@ done:
if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long) &rfc);
+
if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
break;
@@ -2566,6 +2575,9 @@ done:
if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long) &rfc);
+
if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
break;
@@ -2577,9 +2589,6 @@ done:
break;
}
- l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
- (unsigned long) &rfc);
-
/* FIXME: Need actual value of the flush timeout */
//if (flush_to != L2CAP_DEFAULT_FLUSH_TO)
// l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, pi->flush_to);
--
1.7.1.1
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported
From: Marcel Holtmann @ 2010-08-04 1:56 UTC (permalink / raw)
To: Gustavo F. Padovan
Cc: Ville Tervo, linux-bluetooth@vger.kernel.org, Gustavo F. Padovan
In-Reply-To: <20100731224144.GA29039@vigoh>
Hi Ville,
> > On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> > >From: Gustavo F. Padovan<padovan@profusion.mobi>
> > >
> > >If the remote side doesn't support Enhanced Retransmission Mode neither
> > >Streaming Mode, we shall not send the RFC option.
> > >Some devices that only supports Basic Mode do not understanding the RFC
> > >option. This patch fix the regression found with that devices.
> >
> >
> > Yes this is better. After some research i found out that quite many
> > old devices are not handling properly unknown options.
>
> Nice, we can put this upstream, do you agree?
>
> >
> > However I found another regression. And this kind of patch is needed
> > also. Otherwise the info rsp code is reading feat_mask from failed
> > response (and does invalid memory access).
> >
> >
> > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > index 0f34e12..1e174a4 100644
> > --- a/net/bluetooth/l2cap.c
> > +++ b/net/bluetooth/l2cap.c
> > @@ -3348,6 +3348,13 @@ static inline int
> > l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
> >
> > del_timer(&conn->info_timer);
> >
> > + if (result != L2CAP_IR_SUCCESS) {
> > + conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
> > + conn->info_ident = 0;
> > + l2cap_conn_start(conn);
> > + return 0;
> > + }
> > +
> > if (type == L2CAP_IT_FEAT_MASK) {
> > conn->feat_mask = get_unaligned_le32(rsp->data);
> >
>
> Ack. Send a proper GIT patch then we can push this fix.
am I getting this proper patch or not?
Regards
Marcel
^ permalink raw reply
* [PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
---
src/adapter.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 103 insertions(+), 3 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index a2ef9aa..9603ba7 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2750,8 +2750,100 @@ static void emit_device_found(const char *path, const char *address,
g_dbus_send_message(connection, signal);
}
+static char **get_eir_uuids(uint8_t *eir_data, size_t *uuid_count)
+{
+ uint8_t len = 0;
+ char **uuid_buf = NULL;
+ char **uuids;
+ size_t total = 0;
+ size_t uuid16_count = 0;
+ size_t uuid32_count = 0;
+ size_t uuid128_count = 0;
+ uint8_t *uuid16;
+ uint8_t *uuid32;
+ uint8_t *uuid128;
+ uuid_t service;
+ int i;
+
+ while (len < EIR_DATA_LENGTH) {
+ uint8_t type = eir_data[1];
+ uint8_t field_len = eir_data[0];
+
+ switch(type) {
+ case EIR_UUID16_SOME:
+ case EIR_UUID16_ALL:
+ uuid16_count = field_len / 2;
+ uuid16 = &eir_data[2];
+ break;
+ case EIR_UUID32_SOME:
+ case EIR_UUID32_ALL:
+ uuid32_count = field_len / 4;
+ uuid32 = &eir_data[2];
+ break;
+ case EIR_UUID128_SOME:
+ case EIR_UUID128_ALL:
+ uuid128_count = field_len / 16;
+ uuid128 = &eir_data[2];
+ break;
+ }
+
+ len += field_len + 1;
+ eir_data += field_len + 1;
+ }
+
+ total = uuid16_count + uuid32_count + uuid128_count;
+ *uuid_count = total;
+
+ if (!total)
+ return NULL;
+
+ uuid_buf = g_new0(char *, total + 1);
+
+ if (!uuid_buf)
+ return NULL;
+
+ uuids = uuid_buf;
+
+ /* Generate uuids in SDP format (EIR data is Little Endian) */
+ service.type = SDP_UUID16;
+ for (i = 0; i < uuid16_count; i++) {
+ uint16_t val16 = uuid16[1];
+
+ val16 = (val16<<8) + uuid16[0];
+ service.value.uuid16 = val16;
+ *uuids++ = bt_uuid2string(&service);
+ uuid16 += 2;
+ }
+
+ service.type = SDP_UUID32;
+ for (i = 0; i < uuid32_count; i++) {
+ uint32_t val32 = uuid32[3];
+ int k;
+
+ for (k = 2; k >= 0; k--)
+ val32 = (val32 << 8) + uuid32[k];
+
+ service.value.uuid32 = val32;
+ *uuids++ = bt_uuid2string(&service);
+ uuid32 += 4;
+ }
+
+ service.type = SDP_UUID128;
+ for (i = 0; i < uuid128_count; i++) {
+ int k;
+
+ for (k = 0; k < 16; k++)
+ service.value.uuid128.data[k] = uuid128[16 - k - 1];
+
+ *uuids++ = bt_uuid2string(&service);
+ uuid128 += 16;
+ }
+
+ return uuid_buf;
+}
+
void adapter_emit_device_found(struct btd_adapter *adapter,
- struct remote_dev_info *dev)
+ struct remote_dev_info *dev, uint8_t *eir_data)
{
struct btd_device *device;
char peer_addr[18], local_addr[18];
@@ -2759,6 +2851,8 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
dbus_bool_t paired = FALSE;
dbus_int16_t rssi = dev->rssi;
char *alias;
+ char **uuids = NULL;
+ size_t uuid_count = 0;
ba2str(&dev->bdaddr, peer_addr);
ba2str(&adapter->bdaddr, local_addr);
@@ -2778,6 +2872,10 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
} else
alias = g_strdup(dev->alias);
+ /* Extract UUIDs from extended inquiry response if any*/
+ if (eir_data != NULL)
+ uuids = get_eir_uuids(eir_data, &uuid_count);
+
emit_device_found(adapter->path, paddr,
"Address", DBUS_TYPE_STRING, &paddr,
"Class", DBUS_TYPE_UINT32, &dev->class,
@@ -2787,15 +2885,17 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
"Alias", DBUS_TYPE_STRING, &alias,
"LegacyPairing", DBUS_TYPE_BOOLEAN, &dev->legacy,
"Paired", DBUS_TYPE_BOOLEAN, &paired,
+ "UUIDs", DBUS_TYPE_ARRAY, &uuids, uuid_count,
NULL);
g_free(alias);
+ g_strfreev(uuids);
}
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
int8_t rssi, uint32_t class, const char *name,
const char *alias, gboolean legacy,
- name_status_t name_status)
+ name_status_t name_status, uint8_t *eir_data)
{
struct remote_dev_info *dev, match;
@@ -2834,7 +2934,7 @@ done:
adapter->found_devices = g_slist_sort(adapter->found_devices,
(GCompareFunc) dev_rssi_cmp);
- adapter_emit_device_found(adapter, dev);
+ adapter_emit_device_found(adapter, dev, eir_data);
}
int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr)
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 6/7] Handle arrays in device properties dictionary.
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
---
src/adapter.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index e1f3051..a2ef9aa 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2702,6 +2702,7 @@ static void append_dict_valist(DBusMessageIter *iter,
DBusMessageIter dict;
const char *key;
int type;
+ int n_elements;
void *val;
dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
@@ -2713,7 +2714,12 @@ static void append_dict_valist(DBusMessageIter *iter,
while (key) {
type = va_arg(var_args, int);
val = va_arg(var_args, void *);
- dict_append_entry(&dict, key, type, val);
+ if (type == DBUS_TYPE_ARRAY) {
+ n_elements = va_arg(var_args, int);
+ if (n_elements > 0)
+ dict_append_array(&dict, key, DBUS_TYPE_STRING, val, n_elements);
+ } else
+ dict_append_entry(&dict, key, type, val);
key = va_arg(var_args, char *);
}
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 5/7] Fix in logic to write EIR when SDP records are changed.
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
Whenever SDP service record is added/deleted/modified check for whether
class of device needs to be updated as well. If the update is
needed, proceed as before: new EIR will be written subsequently.
If the class of device is already present, just update EIR and return.
---
src/adapter.c | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 4615326..e1f3051 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -137,6 +137,8 @@ struct btd_adapter {
gint ref;
};
+static void update_ext_inquiry_response(struct btd_adapter *adapter);
+
static void adapter_set_pairable_timeout(struct btd_adapter *adapter,
guint interval);
@@ -216,12 +218,17 @@ static int adapter_set_service_classes(struct btd_adapter *adapter,
adapter->wanted_cod &= 0x00ffff;
adapter->wanted_cod |= (value << 16);
- /* If we already have the CoD we want or the cache is enabled or an
- * existing CoD write is in progress just bail out */
- if (adapter->current_cod == adapter->wanted_cod ||
- adapter->cache_enable || adapter->pending_cod)
+ /* If the cache is enabled or an existing CoD write is in progress
+ * just bail out */
+ if (adapter->cache_enable || adapter->pending_cod)
return 0;
+ /* If we already have the CoD we want, update EIR and return */
+ if (adapter->current_cod == adapter->wanted_cod) {
+ update_ext_inquiry_response(adapter);
+ return 0;
+ }
+
DBG("Changing service classes to 0x%06x", adapter->wanted_cod);
err = adapter_ops->set_class(adapter->dev_id, adapter->wanted_cod);
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 4/7] Support for adding UUID128 to extended inquiry response
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
---
src/sdpd-service.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 64 insertions(+), 1 deletions(-)
diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index 50c0a97..651e24c 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -49,6 +49,8 @@
#include "manager.h"
#include "adapter.h"
+#define SIZEOF_UUID128 16
+
static sdp_record_t *server = NULL;
static uint16_t did_vendor = 0x0000;
@@ -174,6 +176,63 @@ static void update_svclass_list(const bdaddr_t *src)
}
+static void eir_generate_uuid128(sdp_list_t *list,
+ uint8_t *ptr, uint16_t *eir_len)
+{
+ int i, k, index = 0;
+ uint16_t len = *eir_len;
+ uint8_t *uuid128;
+ gboolean truncated = FALSE;
+
+ /* Store UUID128 in place, skip 2 bytes to insert type and length later */
+ uuid128 = ptr + 2;
+
+ for (; list; list = list->next) {
+ sdp_record_t *rec = (sdp_record_t *) list->data;
+ uint8_t *uuid128_data = rec->svclass.value.uuid128.data;
+
+ if (rec->svclass.type != SDP_UUID128)
+ continue;
+
+ /* Stop if not enough space to put next UUID128 */
+ if ((len + 2 + SIZEOF_UUID128) > EIR_DATA_LENGTH) {
+ truncated = TRUE;
+ break;
+ }
+
+ /* Check for duplicates, EIR data is Little Endian */
+ for (i = 0; i < index; i++) {
+ for (k = 0; k < SIZEOF_UUID128; k++) {
+ if (uuid128[i * SIZEOF_UUID128 + k] !=
+ uuid128_data[SIZEOF_UUID128 - k])
+ break;
+ }
+ if (k == SIZEOF_UUID128)
+ break;
+ }
+
+ if (i < index)
+ continue;
+
+ /* EIR data is Little Endian */
+ for (k = 0; k < SIZEOF_UUID128; k++)
+ uuid128[index * SIZEOF_UUID128 + k] =
+ uuid128_data[SIZEOF_UUID128 - 1 - k];
+
+ len += SIZEOF_UUID128;
+ index++;
+ }
+
+ if (index > 0 || truncated) {
+ /* EIR Data length */
+ ptr[0] = (index * SIZEOF_UUID128) + 1;
+ /* EIR Data type */
+ ptr[1] = truncated ? EIR_UUID128_SOME : EIR_UUID128_ALL;
+ len += 2;
+ *eir_len = len;
+ }
+}
+
void create_ext_inquiry_response(const char *name,
int8_t tx_power, sdp_list_t *services,
uint8_t *data)
@@ -261,7 +320,7 @@ void create_ext_inquiry_response(const char *name,
/* EIR Data length */
ptr[0] = (index * sizeof(uint16_t)) + 1;
/* EIR Data type */
- ptr[1] = (truncated) ? EIR_UUID16_SOME : EIR_UUID16_ALL;
+ ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
ptr += 2;
eir_len += 2;
@@ -271,6 +330,10 @@ void create_ext_inquiry_response(const char *name,
*ptr++ = (uuid16[i] & 0xff00) >> 8;
}
}
+
+ /* Group all UUID128 types */
+ if (eir_len <= EIR_DATA_LENGTH - 2)
+ eir_generate_uuid128(services, ptr, &eir_len);
}
void register_public_browse_group(void)
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 3/7] Clean up code that generates extended inquiry response.
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
---
src/sdpd-service.c | 44 +++++++++++++++++++++++++++++---------------
1 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index 35e333d..50c0a97 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -180,35 +180,41 @@ void create_ext_inquiry_response(const char *name,
{
sdp_list_t *list = services;
uint8_t *ptr = data;
- uint16_t uuid[24];
+ uint16_t eir_len = 0;
+ uint16_t uuid16[EIR_DATA_LENGTH / 2];
int i, index = 0;
+ gboolean truncated = FALSE;
if (name) {
int len = strlen(name);
+ /* EIR Data type */
if (len > 48) {
len = 48;
- ptr[1] = 0x08;
+ ptr[1] = EIR_NAME_SHORT;
} else
- ptr[1] = 0x09;
+ ptr[1] = EIR_NAME_COMPLETE;
+ /* EIR Data length */
ptr[0] = len + 1;
memcpy(ptr + 2, name, len);
- ptr += len + 2;
+ eir_len += (len + 2);
+ ptr += (len + 2);
}
if (tx_power != 0) {
*ptr++ = 2;
- *ptr++ = 0x0a;
+ *ptr++ = EIR_TX_POWER;
*ptr++ = (uint8_t) tx_power;
+ eir_len += 3;
}
if (did_vendor != 0x0000) {
uint16_t source = 0x0002;
*ptr++ = 9;
- *ptr++ = 0x10;
+ *ptr++ = EIR_DEVICE_ID;
*ptr++ = (source & 0x00ff);
*ptr++ = (source & 0xff00) >> 8;
*ptr++ = (did_vendor & 0x00ff);
@@ -217,10 +223,10 @@ void create_ext_inquiry_response(const char *name,
*ptr++ = (did_product & 0xff00) >> 8;
*ptr++ = (did_version & 0x00ff);
*ptr++ = (did_version & 0xff00) >> 8;
+ eir_len += 10;
}
- ptr[1] = 0x03;
-
+ /* Group all UUID16 types */
for (; list; list = list->next) {
sdp_record_t *rec = (sdp_record_t *) list->data;
@@ -233,28 +239,36 @@ void create_ext_inquiry_response(const char *name,
if (rec->svclass.value.uuid16 == PNP_INFO_SVCLASS_ID)
continue;
- if (index > 23) {
- ptr[1] = 0x02;
+ /* Stop if not enough space to put next UUID16 */
+ if ((eir_len + 2 + sizeof(uint16_t)) > EIR_DATA_LENGTH) {
+ truncated = TRUE;
break;
}
+ /* Check for duplicates */
for (i = 0; i < index; i++)
- if (uuid[i] == rec->svclass.value.uuid16)
+ if (uuid16[i] == rec->svclass.value.uuid16)
break;
if (i < index)
continue;
- uuid[index++] = rec->svclass.value.uuid16;
+ uuid16[index++] = rec->svclass.value.uuid16;
+ eir_len += sizeof(uint16_t);
}
if (index > 0) {
- ptr[0] = (index * 2) + 1;
+ /* EIR Data length */
+ ptr[0] = (index * sizeof(uint16_t)) + 1;
+ /* EIR Data type */
+ ptr[1] = (truncated) ? EIR_UUID16_SOME : EIR_UUID16_ALL;
+
ptr += 2;
+ eir_len += 2;
for (i = 0; i < index; i++) {
- *ptr++ = (uuid[i] & 0x00ff);
- *ptr++ = (uuid[i] & 0xff00) >> 8;
+ *ptr++ = (uuid16[i] & 0x00ff);
+ *ptr++ = (uuid16[i] & 0xff00) >> 8;
}
}
}
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 2/7] Minor fix when skipping duplicate UUID16 from EIR.
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
---
src/sdpd-service.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index cdbb4f4..35e333d 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -242,7 +242,7 @@ void create_ext_inquiry_response(const char *name,
if (uuid[i] == rec->svclass.value.uuid16)
break;
- if (i == index - 1)
+ if (i < index)
continue;
uuid[index++] = rec->svclass.value.uuid16;
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH 1/7] Spec constants for Extended Inquiry Response field types
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel, Inga Stotland
In-Reply-To: <1280880237-2296-1-git-send-email-ingas@codeaurora.org>
---
src/sdpd.h | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/src/sdpd.h b/src/sdpd.h
index e93b0b6..5bab869 100644
--- a/src/sdpd.h
+++ b/src/sdpd.h
@@ -34,6 +34,19 @@
#define SDPDBG(fmt...)
#endif
+#define EIR_DATA_LENGTH 240
+
+#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 */
+
typedef struct request {
bdaddr_t device;
bdaddr_t bdaddr;
--
1.7.2
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH v6 0/7] Enhanced support for extended inquiry response
From: Inga Stotland @ 2010-08-04 0:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel
EIR needs to be updated whenever local SDP record database changes.
Previously, when an SDP record was added/removed/updated, EIR would
be written in callback fired off HCI "Command Complete" event set off by
"Write Class Of Device" command. However, if the class of device did not
need to be updated, the EIR write was not happening either. This
implementation has been augmented to write new EIR when modification
in SDP database does not lead to updating of class of device.
Added support for UUID128 service descriptors in local EIR.
Service UUIDs are written to "device properties" dictionary when emitting
"Device Found" signal. This allows peek at available services offered by
a remote device without establishing SDP connection.
--
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* Re: [PATCH][RFC] Fix SDP resolving segfault
From: Manuel Naranjo @ 2010-08-03 20:24 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Johan Hedberg, BlueZ
In-Reply-To: <AANLkTinYN=KDban-WsF4wZw-ZZDU7fQ0SFnAWdofbokd@mail.gmail.com>
Luiz,
> This looks like a different issue, at least it doesn't seems to crash
> in the same point, so I assume the patch does fix something but
> uncover another problem. But we have to find out where exactly is this
> other problem because btd_device_unref doesn't seems to be the place,
> can't you get any core dumps from the tests you did?
>
I agree it looks like it triggered another issue, I will try to get logs
tomorrow.
Manuel
^ permalink raw reply
* Re: [PATCH][RFC] Fix SDP resolving segfault
From: Luiz Augusto von Dentz @ 2010-08-03 20:17 UTC (permalink / raw)
To: Manuel Naranjo; +Cc: Johan Hedberg, BlueZ
In-Reply-To: <AANLkTi=i8MhyC80qis9TjZBmtNUJDBMNw5g2rjKPQvdj@mail.gmail.com>
Hi Manuel,
On Tue, Aug 3, 2010 at 4:21 PM, Manuel Naranjo <manuel@aircable.net> wrote:
>> I will give this a try and let you know. Maybe I can results before the
>> Tuesday, but I doubt it.
>
> As I said on IRC it keeps crashing. Here's the log, as you can see the
> crash happened in a different place now.
>
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x9411de0: ref=1
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x9411de0: ref=2
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x9409570: ref=2
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x9408b60: ref=2
> bluetoothd[1842]: src/adapter.c:create_device() C8:7E:75:D6:B3:B0
> bluetoothd[1842]: src/adapter.c:adapter_create_device() C8:7E:75:D6:B3:B0
> bluetoothd[1842]: src/device.c:device_create() Creating device
> /org/bluez/1842/hci0/dev_C8_7E_75_D6_B3_B0
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x93fd258: ref=1
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x93fd258: ref=2
> bluetoothd[1842]: src/device.c:btd_device_ref() 0x9404dc0: ref=3
> bluetoothd[1842]: Discovery session 0x93fb658 with :1.58 activated
> bluetoothd[1842]: src/adapter.c:session_ref() 0x93fb658: ref=1
> bluetoothd[1842]: src/adapter.c:adapter_get_device() 00:25:47:2D:20:98
> bluetoothd[1842]: src/adapter.c:session_unref() 0x93fb658: ref=0
> bluetoothd[1842]: src/adapter.c:session_remove() Discovery session
> 0x93fb658 with :1.58 deactivated
> bluetoothd[1842]: src/adapter.c:session_remove() Stopping discovery
> bluetoothd[1842]: Stopping discovery
> bluetoothd[1842]: /org/bluez/1842/hci0/dev_C8_7E_75_D6_B3_B0: error
> updating services: Device or resource busy (16)
> bluetoothd[1842]: src/device.c:btd_device_unref() 0x93fd258: ref=1
> bluetoothd[1842]: Discovery session 0x93fd4b0 with :1.58 activated
> bluetoothd[1842]: src/adapter.c:session_ref() 0x93fd4b0: ref=1
> bluetoothd[1842]: /org/bluez/1842/hci0/dev_00_26_E2_3C_5E_05: error
> updating services: Host is down (112)
> bluetoothd[1842]: src/device.c:btd_device_unref() 0x9404dc0: ref=2
> bluetoothd[1842]: src/adapter.c:adapter_get_device() 00:26:E2:3C:5E:05
> bluetoothd[1842]: src/adapter.c:session_unref() 0x93fd4b0: ref=0
> bluetoothd[1842]: src/adapter.c:session_remove() Discovery session
> 0x93fd4b0 with :1.58 deactivated
> bluetoothd[1842]: src/adapter.c:session_remove() Stopping discovery
> bluetoothd[1842]: Stopping discovery
> bluetoothd[1842]: /org/bluez/1842/hci0/dev_00_1C_D6_B1_58_1B: error
> updating services: Device or resource busy (16)
> bluetoothd[1842]: src/device.c:btd_device_unref() 0x9409570: ref=1
> bluetoothd[1842]: Discovery session 0x93f9f80 with :1.58 activated
> bluetoothd[1842]: src/adapter.c:session_ref() 0x93f9f80: ref=1
> bluetoothd[1842]: src/adapter.c:adapter_remove_connection() Removing
> temporary device /org/bluez/1842/hci0/dev_00_25_47_2D_20_98
> bluetoothd[1842]: src/device.c:device_remove() Removing device
> /org/bluez/1842/hci0/dev_00_25_47_2D_20_98
> bluetoothd[1842]: src/device.c:btd_device_unref() 0x9411120: ref=1
> bluetoothd[1842]: src/device.c:btd_device_unref() 0x9411120: ref=0
> bluetoothd[1842]: src/device.c:device_free() 0x9411120
> bluetoothd[1842]: src/adapter.c:adapter_get_device() 00:17:D5:65:D0:3D
> bluetoothd[1842]: src/adapter.c:session_unref() 0x93f9f80: ref=0
> bluetoothd[1842]: src/adapter.c:session_remove() Discovery session
> 0x93f9f80 with :1.58 deactivated
> bluetoothd[1842]: src/adapter.c:session_remove() Stopping discovery
> bluetoothd[1842]: Stopping discovery
> bluetoothd[1842]: Discovery session 0x9410560 with :1.58 activated
> bluetoothd[1842]: src/adapter.c:session_ref() 0x9410560: ref=1
> bluetoothd[1842]: src/device.c:device_probe_drivers() Probe drivers
> for /org/bluez/1842/hci0/dev_00_17_D5_65_D0_3D
> bluetoothd[1842]: serial/manager.c:serial_probe() path
> /org/bluez/1842/hci0/dev_00_17_D5_65_D0_3D:
> 00001105-0000-1000-8000-00805f9b34fb
> bluetoothd[1842]: serial/port.c:create_serial_device() Registered
> interface org.bluez.Serial on path
> /org/bluez/1842/hci0/dev_00_17_D5_65_D0_3D
> bluetoothd[1842]: src/device.c:btd_device_unref() 0x9408b60: ref=1
> Segmentation fault
This looks like a different issue, at least it doesn't seems to crash
in the same point, so I assume the patch does fix something but
uncover another problem. But we have to find out where exactly is this
other problem because btd_device_unref doesn't seems to be the place,
can't you get any core dumps from the tests you did?
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Gustavo F. Padovan @ 2010-08-03 18:44 UTC (permalink / raw)
To: Mat Martineau; +Cc: Marcel Holtmann, linux-bluetooth, rshaffer
In-Reply-To: <alpine.DEB.2.00.1008030814570.19861@linux-sea-02>
Hi Mat,
* Mat Martineau <mathewm@codeaurora.org> [2010-08-03 08:58:57 -0700]:
>
> On Mon, 2 Aug 2010, Marcel Holtmann wrote:
>
> >Hi Mat,
> >
> >>>* Marcel Holtmann <marcel@holtmann.org> [2010-08-02 12:38:32 -0700]:
> >>>
> >>>>Hi Mat,
> >>>>
> >>>>>Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> >>>>>---
> >>>>> net/bluetooth/l2cap.c | 12 ++++++++----
> >>>>> 1 files changed, 8 insertions(+), 4 deletions(-)
> >>>>>
> >>>>>diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> >>>>>index 9ba1e8e..aed72f2 100644
> >>>>>--- a/net/bluetooth/l2cap.c
> >>>>>+++ b/net/bluetooth/l2cap.c
> >>>>>@@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
> >>>>> goto unlock;
> >>>>>
> >>>>> if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
> >>>>>- if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>>- l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
> >>>>>+ if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
> >>>>>+ l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
> >>>>>+ (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>>+ l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
> >>>>> l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
> >>>>
> >>>>this becomes unreadable and my brain starts to throw a core dump. So it
> >>>>clearly needs to be put into a helper inline function.
> >>>
> >>>Actually we don't need that, since the code that deals with Basic Mode
> >>>never check and use the l2cap_pi(sk)->fcs. So we don't care about FCS
> >>>value in the Basic Mode.
> >>
> >>There isn't currently any Basic Mode code that triggers this latent
> >>bug, but I have a patch coming up that does require this fix.
> >>
> >>As it stands, getsockopt() on a connected basic mode socket shows FCS
> >>enabled, so this bug is visible from userspace.
> >
> >can we just fail the setsockopt() when trying to set basic mode and FCS
> >off.
>
> It definitely makes sense to have more validation of L2CAP_OPTIONS
> passed to setsockopt().
>
> >And also in case fallback to basic mode happens, then FCS should be set
> >to be enabled. Since for FCS and basic mode we always have to use FCS.
> >So that seems just fine to me.
>
> The spec says "The FCS option shall only be used when the mode is
> being, or is already configured to Enhanced Retransmission mode or
> Streaming mode." FCS is never used in basic mode (fallback or not).
>
> (Maybe I've misunderstood your point)
>
> >Maybe you need to explain a bit more in detail what you are trying to
> >achieve in conjunction with userspace API.
>
> My goal is to only have l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 when
> the FCS option is actually in use. Otherwise, any logic checking
> for FCS also has to check the L2CAP mode. Might as well check the
> mode once and set fcs accordingly -- which is what my patch does.
>
> Gustavo is correct that l2cap_pi(sk)->fcs is currently only checked
> on code paths used with ERTM and streaming mode. However, future
> code (including a patch I'll be posting soon) will depend on the fcs
> value being accurate in all modes.
So queue this togheter with the one you are going to send soon. It will
be easy the real need for the checks you are doing here.
>
> I only mentioned getsockopt() to show that this issue is not
> completely invisible, and is worth patching.
You can also set l2cap_pi(sk)->fcs to NOFCS when the mode configured
is Basic Mode.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Marcel Holtmann @ 2010-08-03 16:50 UTC (permalink / raw)
To: Mat Martineau; +Cc: Gustavo F. Padovan, linux-bluetooth, rshaffer
In-Reply-To: <alpine.DEB.2.00.1008030918360.19861@linux-sea-02>
Hi Mat,
> >>>>>>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> >>>>>>> ---
> >>>>>>> net/bluetooth/l2cap.c | 12 ++++++++----
> >>>>>>> 1 files changed, 8 insertions(+), 4 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> >>>>>>> index 9ba1e8e..aed72f2 100644
> >>>>>>> --- a/net/bluetooth/l2cap.c
> >>>>>>> +++ b/net/bluetooth/l2cap.c
> >>>>>>> @@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
> >>>>>>> goto unlock;
> >>>>>>>
> >>>>>>> if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
> >>>>>>> - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>>>> - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
> >>>>>>> + if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
> >>>>>>> + l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
> >>>>>>> + (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>>>> + l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
> >>>>>>> l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
> >>>>>>
> >>>>>> this becomes unreadable and my brain starts to throw a core dump. So it
> >>>>>> clearly needs to be put into a helper inline function.
> >>>>>
> >>>>> Actually we don't need that, since the code that deals with Basic Mode
> >>>>> never check and use the l2cap_pi(sk)->fcs. So we don't care about FCS
> >>>>> value in the Basic Mode.
> >>>>
> >>>> There isn't currently any Basic Mode code that triggers this latent
> >>>> bug, but I have a patch coming up that does require this fix.
> >>>>
> >>>> As it stands, getsockopt() on a connected basic mode socket shows FCS
> >>>> enabled, so this bug is visible from userspace.
> >>>
> >>> can we just fail the setsockopt() when trying to set basic mode and FCS
> >>> off.
> >>
> >> It definitely makes sense to have more validation of L2CAP_OPTIONS
> >> passed to setsockopt().
> >>
> >>> And also in case fallback to basic mode happens, then FCS should be set
> >>> to be enabled. Since for FCS and basic mode we always have to use FCS.
> >>> So that seems just fine to me.
> >>
> >> The spec says "The FCS option shall only be used when the mode is
> >> being, or is already configured to Enhanced Retransmission mode or
> >> Streaming mode." FCS is never used in basic mode (fallback or not).
> >>
> >> (Maybe I've misunderstood your point)
> >
> > So basic mode uses CRC16 as FCS. And with basic that is not changeable.
> > If you try basic mode + no FCS then we should clearly fail the call to
> > setsockopt() for that.
>
> That's the opposite of what I'm trying to say! :)
>
> Basic mode B-frames have *no* FCS (L2CAP spec section 3.1). The FCS
> field is only found in I-frames and S-frames used in ERTM and
> streaming modes. If you try basic mode + FCS, that should fail.
I confused myself right now. You are fully correct. The default for
basic mode is no fcs.
And as soon as an application wants to use ERTM we just make it to
specific the FCS option. That being CRC16 or no FCS. And of course the
getsockopt() call will return whatever we ended up configuring.
And yes, basic mode + crc16 should fail.
> >>> Maybe you need to explain a bit more in detail what you are trying to
> >>> achieve in conjunction with userspace API.
> >>
> >> My goal is to only have l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 when the
> >> FCS option is actually in use. Otherwise, any logic checking for FCS
> >> also has to check the L2CAP mode. Might as well check the mode once
> >> and set fcs accordingly -- which is what my patch does.
> >>
> >> Gustavo is correct that l2cap_pi(sk)->fcs is currently only checked on
> >> code paths used with ERTM and streaming mode. However, future code
> >> (including a patch I'll be posting soon) will depend on the fcs value
> >> being accurate in all modes.
> >
> > As I said, I have no problem with returning basic mode + crc16. Since
> > that is actually what you are doing in that case.
> >
> > The only thing that we have to keep in mind to be backward compatible is
> > to allow setting of basic mode and fcs = 0x00 and then change that into
> > crc16.
>
> The fcs field in L2CAP_OPTIONS is new for ERTM/Streaming modes, so
> there should be no backward compatibility issues with basic mode.
There is none. I got confused. Too many emails at the same time.
Regards
Marcel
^ permalink raw reply
* Multiple SPP connections, possible ? And, where are the docs ?
From: Banibrata Dutta @ 2010-08-03 16:38 UTC (permalink / raw)
To: linux-bluetooth
Hello,
Quite new to BlueZ, but familiar with Bluetooth on Windows, and Linux
in general. Did try to go through some documentation around BlueZ I
found on the net including a HOWTO, but suprisingly a lot seems to
have changed in terms of how to configure BlueZ. Is there a man-page,
a newer HOWTO, or user's guide or README that may help me figure out,
how to configure BlueZ ?
What I intend to do is to connect my PC to 2 devices over SPP,
simultaneously. My PC is dual-boot (boots into Win-XP SP3, and also
Ubuntu 9.10 on x86 (32-bit plaf). My Win-XP SP3 runs WIDCOMM stack,
and I can communicate with both of the devices but one-at-a-time, so I
know that my BT transceiver and end-devices (mobile phones) are
working fine, and their configuration (atleast phone side) is okay.
What I would like to do on Ubuntu, is to use BlueZ to connect to the 2
phones, simultaneously (something that I am unable to achieve in
Windows-XP). I have managed to pair with both devices from within
Ubuntu, using the Gnome BT-Manager tool, and even succeeded in
browsing file-systems on both phones over OBEX, simultaneously.
However, I find no information on how to setup/configure SP Profile,
and how to achieve the simultaneous usage.
Any help / pointers to right set of docs would really help.
regards,
BD
^ permalink raw reply
* Re: [PATCH 2/6] Clean up code that generates extended inquiry response.
From: Johan Hedberg @ 2010-08-03 16:26 UTC (permalink / raw)
To: ingas; +Cc: linux-bluetooth
In-Reply-To: <c830f11e501a70291cd4b5b3bde2f4a9.squirrel@www.codeaurora.org>
Hi Inga,
On Tue, Aug 03, 2010, ingas@codeaurora.org wrote:
> Just a clarification. I will still have to have the "clean up" patch as a
> part of this patch set since it relies on the header file with defines
> (getting rid of magic numbers).>
That's fine. I.e. the first patch of the set can remain as is.
Johan
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Mat Martineau @ 2010-08-03 16:23 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Gustavo F. Padovan, linux-bluetooth, rshaffer
In-Reply-To: <1280851870.12579.84.camel@localhost.localdomain>
Hi Marcel -
On Tue, 3 Aug 2010, Marcel Holtmann wrote:
>>>>>>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>>>>>>> ---
>>>>>>> net/bluetooth/l2cap.c | 12 ++++++++----
>>>>>>> 1 files changed, 8 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>>>>>>> index 9ba1e8e..aed72f2 100644
>>>>>>> --- a/net/bluetooth/l2cap.c
>>>>>>> +++ b/net/bluetooth/l2cap.c
>>>>>>> @@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
>>>>>>> goto unlock;
>>>>>>>
>>>>>>> if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
>>>>>>> - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
>>>>>>> - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
>>>>>>> + if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
>>>>>>> + l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
>>>>>>> + (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
>>>>>>> + l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
>>>>>>> l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
>>>>>>
>>>>>> this becomes unreadable and my brain starts to throw a core dump. So it
>>>>>> clearly needs to be put into a helper inline function.
>>>>>
>>>>> Actually we don't need that, since the code that deals with Basic Mode
>>>>> never check and use the l2cap_pi(sk)->fcs. So we don't care about FCS
>>>>> value in the Basic Mode.
>>>>
>>>> There isn't currently any Basic Mode code that triggers this latent
>>>> bug, but I have a patch coming up that does require this fix.
>>>>
>>>> As it stands, getsockopt() on a connected basic mode socket shows FCS
>>>> enabled, so this bug is visible from userspace.
>>>
>>> can we just fail the setsockopt() when trying to set basic mode and FCS
>>> off.
>>
>> It definitely makes sense to have more validation of L2CAP_OPTIONS
>> passed to setsockopt().
>>
>>> And also in case fallback to basic mode happens, then FCS should be set
>>> to be enabled. Since for FCS and basic mode we always have to use FCS.
>>> So that seems just fine to me.
>>
>> The spec says "The FCS option shall only be used when the mode is
>> being, or is already configured to Enhanced Retransmission mode or
>> Streaming mode." FCS is never used in basic mode (fallback or not).
>>
>> (Maybe I've misunderstood your point)
>
> So basic mode uses CRC16 as FCS. And with basic that is not changeable.
> If you try basic mode + no FCS then we should clearly fail the call to
> setsockopt() for that.
That's the opposite of what I'm trying to say! :)
Basic mode B-frames have *no* FCS (L2CAP spec section 3.1). The FCS
field is only found in I-frames and S-frames used in ERTM and
streaming modes. If you try basic mode + FCS, that should fail.
>>> Maybe you need to explain a bit more in detail what you are trying to
>>> achieve in conjunction with userspace API.
>>
>> My goal is to only have l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 when the
>> FCS option is actually in use. Otherwise, any logic checking for FCS
>> also has to check the L2CAP mode. Might as well check the mode once
>> and set fcs accordingly -- which is what my patch does.
>>
>> Gustavo is correct that l2cap_pi(sk)->fcs is currently only checked on
>> code paths used with ERTM and streaming mode. However, future code
>> (including a patch I'll be posting soon) will depend on the fcs value
>> being accurate in all modes.
>
> As I said, I have no problem with returning basic mode + crc16. Since
> that is actually what you are doing in that case.
>
> The only thing that we have to keep in mind to be backward compatible is
> to allow setting of basic mode and fcs = 0x00 and then change that into
> crc16.
The fcs field in L2CAP_OPTIONS is new for ERTM/Streaming modes, so
there should be no backward compatibility issues with basic mode.
Regards,
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: What is the motivation for conn->power_save
From: Nick Pelly @ 2010-08-03 16:20 UTC (permalink / raw)
To: Liang Bao; +Cc: Andrei Emeltchenko, Marcel Holtmann, linux-bluetooth
In-Reply-To: <AANLkTinxeqy3KMu0WLo7q-6yD2NbJU104BB4krSOo8gX@mail.gmail.com>
On Tue, Aug 3, 2010 at 2:31 AM, Liang Bao <tim.bao@gmail.com> wrote:
>
>
> 2010/7/9 Nick Pelly <npelly@google.com>
>>
>> On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
>> <andrei.emeltchenko.news@gmail.com> wrote:
>> > Hi,
>> >
>> > On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org=
> wrote:
>> >> Hi Nick,
>> >>
>> >>> >>> >> If I understand correctly, conn->power_save prevents the host=
stack
>> >>> >>> >> from requesting active mode if it was not the host stack that
>> >>> >>> >> requested sniff mode.
>> >>> >>> >>
>> >>> >>> >> I don't understand the motivation for this. If we have ACL da=
ta to
>> >>> >>> >> send, then it seems like a good idea for the host stack to ex=
plicitly
>> >>> >>> >> request active mode, regardless of the reason that we entered=
sniff
>> >>> >>> >> mode.
>> >>> >>> >>
>> >>> >>> >> We want to enter active mode more aggressively when setting u=
p SCO
>> >>> >>> >> connections, to avoid a 5 second delay with certain sniff mod=
es. But
>> >>> >>> >> the conn->power_save code is getting in the way and doesn't a=
ppear to
>> >>> >>> >> be useful in the first place.
>> >>> >>> >
>> >>> >>> > we have discussed this a few times. And if you lock through th=
e code
>> >>> >>> > history then you see that initially we just took devices out o=
f sniff
>> >>> >>> > mode if we had to send data. However with HID devices this fal=
ls flat on
>> >>> >>> > its face. They need to stay in control of sniff mode if they i=
nitiated
>> >>> >>> > it. Some of them crash and others just drain the battery. With=
sniff
>> >>> >>> > mode you can send small amounts of data even while in sniff an=
d for HID
>> >>> >>> > that is sometimes used. So the remote side better not interfer=
e.
>> >>> >>> >
>> >>> >>> > What we really need is a socket option where we can control th=
is on a
>> >>> >>> > per socket basis if we take devices out of sniff mode. And one=
extra
>> >>> >>> > case might be when we try to establish a SCO channel, because =
then it is
>> >>> >>> > clearly not an HID device. However even A2DP has this sort of =
problems
>> >>> >>> > sometimes where the stream setup takes time.
>> >>> >>>
>> >>> >>> Makes sense. Thanks for the explanation.
>> >>> >>
>> >>> >> this means you will be working on a patch for this :)
>> >>>
>> >>> Actually, I want to put a patch together for a socket option to not
>> >>> use power_save (so that we *always* exit sniff mode when sending ACL
>> >>> data). We're seeing this problem with the Plantronics Voyager 855
>> >>> which enters sniff mode during A2DP.
>> >>>
>> >>> Given that power_save is just for HID devices, my preferred design i=
s
>> >>> to disable power_save by default, and have an L2CAP socket option to
>> >>> turn on power_save that would be used by HID L2CAP sockets.
>> >>> Unfortunately this would require userspace HID code to use the new
>> >>> socket option to keep current behavior. But it seems preferable to t=
he
>> >>> alternative of having every single other L2CAP socket use a new sock=
et
>> >>> option just to disable power_save for the sake of HID.
>> >>
>> >> actually you still mix up the meaning of the power_save option. From =
the
>> >> stack side, the automatic sniff mode is off by default. You have to
>> >> enable via sysfs first. The power_save option is just to control when
>> >> sniff mode is activated and the remote enters sniff mode, that we are
>> >> not getting out of it if we have ACL data.
>> >>
>> >> In conclusion, I am fine with a socket option that allows to control
>> >> sniff mode (preferable with interval details) and sniff subrate detai=
ls
>> >> so that we can use them for that ACL link.
>> >>
>> >> So go ahead and work on this. We can fix userspace easily since a new
>> >> socket can be detected easily with newer kernels.
>> >
>> > We have found that some Nokia BT headsets stop working after idle
>> > period when they enter
>> > "power save" mode.
>> >
>> > Do we have any solution for this problem? So far I see good enough
>> > patch :-)) below:
>> >
>> > http://android.git.kernel.org/?p=3Dkernel/common.git;a=3Dblobdiff;f=3D=
net/bluetooth/hci_conn.c;h=3Dfa8b412205cd89546b4a325c558c68040eeaf491;hp=3D=
0cf25114a3f576fc2788a549eb96d0087dd39b44;hb=3Dd8488237646920cd71ef43e5f3ae1=
001c6f4cf7b;hpb=3D3f68e5050c5ae559f56d5da9202cb88928d42b36
>> >
>> > - =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF || !conn->power_save)
>> > + =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF /* || !conn->power_save=
*/)
>> >
>> > Nick have you done socket option for "power_save"?
>>
>> No. We'll do it once we need it for HID support.
>
> Can we do this: try to do check against some existing fields in structure=
hci_conn, for example device_class and then determine if conn->power_save =
shall be used here? By doing this, applications in user space can be saved =
from having knowledge how sniff mode works for different devices.
The class bits cannot be trusted - many devices have the wrong class
bits. And even with the right class bits, what about a device that
supports both HID and A2DP? A socket option still seems like the best
approach.
Nick
^ permalink raw reply
* Re: What is the motivation for conn->power_save
From: Nick Pelly @ 2010-08-03 16:16 UTC (permalink / raw)
To: Liang Bao; +Cc: Andrei Emeltchenko, Marcel Holtmann, linux-bluetooth
In-Reply-To: <AANLkTinxeqy3KMu0WLo7q-6yD2NbJU104BB4krSOo8gX@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5217 bytes --]
On Tue, Aug 3, 2010 at 2:31 AM, Liang Bao <tim.bao@gmail.com> wrote:
>
>
> 2010/7/9 Nick Pelly <npelly@google.com>
>
> On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
>> <andrei.emeltchenko.news@gmail.com> wrote:
>> > Hi,
>> >
>> > On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org>
>> wrote:
>> >> Hi Nick,
>> >>
>> >>> >>> >> If I understand correctly, conn->power_save prevents the host
>> stack
>> >>> >>> >> from requesting active mode if it was not the host stack that
>> >>> >>> >> requested sniff mode.
>> >>> >>> >>
>> >>> >>> >> I don't understand the motivation for this. If we have ACL data
>> to
>> >>> >>> >> send, then it seems like a good idea for the host stack to
>> explicitly
>> >>> >>> >> request active mode, regardless of the reason that we entered
>> sniff
>> >>> >>> >> mode.
>> >>> >>> >>
>> >>> >>> >> We want to enter active mode more aggressively when setting up
>> SCO
>> >>> >>> >> connections, to avoid a 5 second delay with certain sniff
>> modes. But
>> >>> >>> >> the conn->power_save code is getting in the way and doesn't
>> appear to
>> >>> >>> >> be useful in the first place.
>> >>> >>> >
>> >>> >>> > we have discussed this a few times. And if you lock through the
>> code
>> >>> >>> > history then you see that initially we just took devices out of
>> sniff
>> >>> >>> > mode if we had to send data. However with HID devices this falls
>> flat on
>> >>> >>> > its face. They need to stay in control of sniff mode if they
>> initiated
>> >>> >>> > it. Some of them crash and others just drain the battery. With
>> sniff
>> >>> >>> > mode you can send small amounts of data even while in sniff and
>> for HID
>> >>> >>> > that is sometimes used. So the remote side better not interfere.
>> >>> >>> >
>> >>> >>> > What we really need is a socket option where we can control this
>> on a
>> >>> >>> > per socket basis if we take devices out of sniff mode. And one
>> extra
>> >>> >>> > case might be when we try to establish a SCO channel, because
>> then it is
>> >>> >>> > clearly not an HID device. However even A2DP has this sort of
>> problems
>> >>> >>> > sometimes where the stream setup takes time.
>> >>> >>>
>> >>> >>> Makes sense. Thanks for the explanation.
>> >>> >>
>> >>> >> this means you will be working on a patch for this :)
>> >>>
>> >>> Actually, I want to put a patch together for a socket option to not
>> >>> use power_save (so that we *always* exit sniff mode when sending ACL
>> >>> data). We're seeing this problem with the Plantronics Voyager 855
>> >>> which enters sniff mode during A2DP.
>> >>>
>> >>> Given that power_save is just for HID devices, my preferred design is
>> >>> to disable power_save by default, and have an L2CAP socket option to
>> >>> turn on power_save that would be used by HID L2CAP sockets.
>> >>> Unfortunately this would require userspace HID code to use the new
>> >>> socket option to keep current behavior. But it seems preferable to the
>> >>> alternative of having every single other L2CAP socket use a new socket
>> >>> option just to disable power_save for the sake of HID.
>> >>
>> >> actually you still mix up the meaning of the power_save option. From
>> the
>> >> stack side, the automatic sniff mode is off by default. You have to
>> >> enable via sysfs first. The power_save option is just to control when
>> >> sniff mode is activated and the remote enters sniff mode, that we are
>> >> not getting out of it if we have ACL data.
>> >>
>> >> In conclusion, I am fine with a socket option that allows to control
>> >> sniff mode (preferable with interval details) and sniff subrate details
>> >> so that we can use them for that ACL link.
>> >>
>> >> So go ahead and work on this. We can fix userspace easily since a new
>> >> socket can be detected easily with newer kernels.
>> >
>> > We have found that some Nokia BT headsets stop working after idle
>> > period when they enter
>> > "power save" mode.
>> >
>> > Do we have any solution for this problem? So far I see good enough
>> > patch :-)) below:
>> >
>> >
>> http://android.git.kernel.org/?p=kernel/common.git;a=blobdiff;f=net/bluetooth/hci_conn.c;h=fa8b412205cd89546b4a325c558c68040eeaf491;hp=0cf25114a3f576fc2788a549eb96d0087dd39b44;hb=d8488237646920cd71ef43e5f3ae1001c6f4cf7b;hpb=3f68e5050c5ae559f56d5da9202cb88928d42b36
>> >
>> > - if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
>> > + if (conn->mode != HCI_CM_SNIFF /* || !conn->power_save */)
>> >
>> > Nick have you done socket option for "power_save"?
>>
>> No. We'll do it once we need it for HID support.
>>
> Can we do this: try to do check against some existing fields in structure
> hci_conn, for example device_class and then determine if conn->power_save
> shall be used here? By doing this, applications in user space can be saved
> from having knowledge how sniff mode works for different devices.
>
>
The class bits cannot be trusted - many devices have the wrong class bits.
And even with the right class bits, what about a device that supports both
HID and A2DP? A socket option still seems like the best approach.
Nick
[-- Attachment #2: Type: text/html, Size: 7242 bytes --]
^ permalink raw reply
* Re: Enhancements to allow l2cap channel to use either AMP or BR/EDR
From: Marcel Holtmann @ 2010-08-03 16:14 UTC (permalink / raw)
To: David Vrabel
Cc: tmonahan, Inga Stotland, linux-bluetooth, rshaffer, johan.hedberg
In-Reply-To: <4C58129E.3080306@csr.com>
Hi David,
> > That said, nothing stops us from allowing to make our AMP polices
> > dynamic and allow the application to change it threshold during
> > lifetime. For example it starts out as BR/EDR only and then during the
> > usage of the link it decides that now AMP preferred would make sense. In
> > that sense you would have manual switching to some level.
>
> If the policy is dynamic throughout the lifetime of the connection then
> that's okay for best effort links.
>
> I think applications (especially those that stream real-time data at
> high rates) will need to know the currently available bandwidth. This
> will be useful for even simple file transfer profiles -- if that file
> transfer is going to take 100x as long the user probably want to know
> about this so they can (for example) turn HS mode on the other device.
>
> This information could be conveyed in some sort of "channel move
> complete" event or a more generic "available bandwidth changed" event.
so here you have the following problem, the only interface you get from
an application point of view is a L2CAP socket. So you have to work with
these constraints. Potentially we can provide something additional via
the OOB msg structs (like we do for timestamps), but I am really careful
in adding to much into these ones.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 8/8] Bluetooth: Use 3-DH5 payload size for default ERTM max PDU size.
From: Mat Martineau @ 2010-08-03 16:13 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1280778508.12579.48.camel@localhost.localdomain>
Marcel,
On Mon, 2 Aug 2010, Marcel Holtmann wrote:
> Hi Mat,
>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>> include/net/bluetooth/l2cap.h | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> this should have a commit message with technical details to explain why
> it is a good idea.
>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index 16e412f..93aba17 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -35,7 +35,7 @@
>> #define L2CAP_DEFAULT_MAX_TX 3
>> #define L2CAP_DEFAULT_RETRANS_TO 2000 /* 2 seconds */
>> #define L2CAP_DEFAULT_MONITOR_TO 12000 /* 12 seconds */
>> -#define L2CAP_DEFAULT_MAX_PDU_SIZE 672
>> +#define L2CAP_DEFAULT_MAX_PDU_SIZE 1011 /* Sized for 3-DH5 packet */
>> #define L2CAP_DEFAULT_ACK_TO 200
>> #define L2CAP_LOCAL_BUSY_TRIES 12
>
> Also the default PDU size for basic mode should stay with 672. And only
> for ERTM move it to 1010. Is this ensured?
>
> And then again here. Is that explained in the commit message?
It is ensured that the max PDU size is only used for ERTM and
streaming mode. The default MTU for basic mode and ERTM/streaming
remains unchanged.
I will update the commit message to fully describe this.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Marcel Holtmann @ 2010-08-03 16:11 UTC (permalink / raw)
To: Mat Martineau; +Cc: Gustavo F. Padovan, linux-bluetooth, rshaffer
In-Reply-To: <alpine.DEB.2.00.1008030814570.19861@linux-sea-02>
Hi mat,
> >>>>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> >>>>> ---
> >>>>> net/bluetooth/l2cap.c | 12 ++++++++----
> >>>>> 1 files changed, 8 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> >>>>> index 9ba1e8e..aed72f2 100644
> >>>>> --- a/net/bluetooth/l2cap.c
> >>>>> +++ b/net/bluetooth/l2cap.c
> >>>>> @@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
> >>>>> goto unlock;
> >>>>>
> >>>>> if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
> >>>>> - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>> - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
> >>>>> + if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
> >>>>> + l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
> >>>>> + (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>> + l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
> >>>>> l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
> >>>>
> >>>> this becomes unreadable and my brain starts to throw a core dump. So it
> >>>> clearly needs to be put into a helper inline function.
> >>>
> >>> Actually we don't need that, since the code that deals with Basic Mode
> >>> never check and use the l2cap_pi(sk)->fcs. So we don't care about FCS
> >>> value in the Basic Mode.
> >>
> >> There isn't currently any Basic Mode code that triggers this latent
> >> bug, but I have a patch coming up that does require this fix.
> >>
> >> As it stands, getsockopt() on a connected basic mode socket shows FCS
> >> enabled, so this bug is visible from userspace.
> >
> > can we just fail the setsockopt() when trying to set basic mode and FCS
> > off.
>
> It definitely makes sense to have more validation of L2CAP_OPTIONS
> passed to setsockopt().
>
> > And also in case fallback to basic mode happens, then FCS should be set
> > to be enabled. Since for FCS and basic mode we always have to use FCS.
> > So that seems just fine to me.
>
> The spec says "The FCS option shall only be used when the mode is
> being, or is already configured to Enhanced Retransmission mode or
> Streaming mode." FCS is never used in basic mode (fallback or not).
>
> (Maybe I've misunderstood your point)
So basic mode uses CRC16 as FCS. And with basic that is not changeable.
If you try basic mode + no FCS then we should clearly fail the call to
setsockopt() for that.
> > Maybe you need to explain a bit more in detail what you are trying to
> > achieve in conjunction with userspace API.
>
> My goal is to only have l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 when the
> FCS option is actually in use. Otherwise, any logic checking for FCS
> also has to check the L2CAP mode. Might as well check the mode once
> and set fcs accordingly -- which is what my patch does.
>
> Gustavo is correct that l2cap_pi(sk)->fcs is currently only checked on
> code paths used with ERTM and streaming mode. However, future code
> (including a patch I'll be posting soon) will depend on the fcs value
> being accurate in all modes.
As I said, I have no problem with returning basic mode + crc16. Since
that is actually what you are doing in that case.
The only thing that we have to keep in mind to be backward compatible is
to allow setting of basic mode and fcs = 0x00 and then change that into
crc16.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Mat Martineau @ 2010-08-03 15:58 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Gustavo F. Padovan, linux-bluetooth, rshaffer
In-Reply-To: <1280789488.12579.60.camel@localhost.localdomain>
On Mon, 2 Aug 2010, Marcel Holtmann wrote:
> Hi Mat,
>
>>> * Marcel Holtmann <marcel@holtmann.org> [2010-08-02 12:38:32 -0700]:
>>>
>>>> Hi Mat,
>>>>
>>>>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>>>>> ---
>>>>> net/bluetooth/l2cap.c | 12 ++++++++----
>>>>> 1 files changed, 8 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>>>>> index 9ba1e8e..aed72f2 100644
>>>>> --- a/net/bluetooth/l2cap.c
>>>>> +++ b/net/bluetooth/l2cap.c
>>>>> @@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
>>>>> goto unlock;
>>>>>
>>>>> if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
>>>>> - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
>>>>> - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
>>>>> + if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
>>>>> + l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
>>>>> + (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
>>>>> + l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
>>>>> l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
>>>>
>>>> this becomes unreadable and my brain starts to throw a core dump. So it
>>>> clearly needs to be put into a helper inline function.
>>>
>>> Actually we don't need that, since the code that deals with Basic Mode
>>> never check and use the l2cap_pi(sk)->fcs. So we don't care about FCS
>>> value in the Basic Mode.
>>
>> There isn't currently any Basic Mode code that triggers this latent
>> bug, but I have a patch coming up that does require this fix.
>>
>> As it stands, getsockopt() on a connected basic mode socket shows FCS
>> enabled, so this bug is visible from userspace.
>
> can we just fail the setsockopt() when trying to set basic mode and FCS
> off.
It definitely makes sense to have more validation of L2CAP_OPTIONS
passed to setsockopt().
> And also in case fallback to basic mode happens, then FCS should be set
> to be enabled. Since for FCS and basic mode we always have to use FCS.
> So that seems just fine to me.
The spec says "The FCS option shall only be used when the mode is
being, or is already configured to Enhanced Retransmission mode or
Streaming mode." FCS is never used in basic mode (fallback or not).
(Maybe I've misunderstood your point)
> Maybe you need to explain a bit more in detail what you are trying to
> achieve in conjunction with userspace API.
My goal is to only have l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 when the
FCS option is actually in use. Otherwise, any logic checking for FCS
also has to check the L2CAP mode. Might as well check the mode once
and set fcs accordingly -- which is what my patch does.
Gustavo is correct that l2cap_pi(sk)->fcs is currently only checked on
code paths used with ERTM and streaming mode. However, future code
(including a patch I'll be posting soon) will depend on the fcs value
being accurate in all modes.
I only mentioned getsockopt() to show that this issue is not
completely invisible, and is worth patching.
Regards,
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCH 2/6] Clean up code that generates extended inquiry response.
From: ingas @ 2010-08-03 15:37 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100803081258.GA5831@jh-x301>
Hi Johan,
>> ---
>> src/sdpd-service.c | 46
>> ++++++++++++++++++++++++++++++----------------
>> 1 files changed, 30 insertions(+), 16 deletions(-)
>
> So your commit message says two things: "Clean up code..." and "Minor
> fix..". Cleanups and fixes should be in separate patches. Don't be
> afraid of "too small" or trivial patches. As long as a patch contains a
> single logically independent change it's fine.
>
Just a clarification. I will still have to have the "clean up" patch as a
part of this patch set since it relies on the header file with defines
(getting rid of magic numbers). Not sure, if you want me to cut the "fix"
one to be separate patch submission altogether...
Thanks,
Inga
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* btd_adapter_driver
From: Daniel Örstadius @ 2010-08-03 14:38 UTC (permalink / raw)
To: linux-bluetooth
Hi Marcel,
I have a question about the btd_adapter_driver struct.
The Maemo6 MCE plugin (which uses that struct) needs to be notified
when the adapter is powered on or off. One idea is to add a callback
to btd_adapter_driver to be called in adapter.c, but from discussion
with Johan the struct was intended to stay very simple, so that this
might not be an acceptable solution.
What would be a good way to handle it? Could listening to a raw hci
socket inside the driver be an option?
Thanks in advance,
Daniel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox