* Re: [PATCH 3/3] bnep: Return errno instead of -1 and print error
From: Andrei Emeltchenko @ 2014-10-22 13:37 UTC (permalink / raw)
To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth
In-Reply-To: <CAMv5Gbck+=b9kY-s+gd3shJsLUDAEZib5dKGM09ZBWkP==ufFw@mail.gmail.com>
Hi Grzegorz,
On Wed, Oct 22, 2014 at 02:55:54PM +0200, Grzegorz Kolodziejczyk wrote:
> Hi Andrei,
>
> On 22 October 2014 13:10, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > Make code consistent with the rest returning -errno and printing error
> > message.
> > ---
> > profiles/network/bnep.c | 18 +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> > index 4cf38d9..09d4b65 100644
> > --- a/profiles/network/bnep.c
> > +++ b/profiles/network/bnep.c
> > @@ -231,7 +231,7 @@ static int bnep_if_down(const char *devname)
> > if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
> > err = -errno;
> > error("bnep: Could not bring down %s: %s(%d)",
> > - devname, strerror(-err), -err);
> > + devname, strerror(-err), -err);
>
> Why don't you fix this indention in first patch ? (this line is added
> in previous patch)
Sorry, resend the series.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [PATCHv5 02/14] shared/gatt: Add initial implementation of discover_included_services
From: Szymon Janc @ 2014-10-22 13:37 UTC (permalink / raw)
To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413454646-23076-3-git-send-email-marcin.kraglak@tieto.com>
Hi Marcin,
On Thursday 16 of October 2014 12:17:14 Marcin Kraglak wrote:
> Current implementation allow to discover included services with
> 16 bit UUID only.
> ---
> src/shared/gatt-helpers.c | 108 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 106 insertions(+), 2 deletions(-)
>
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 4dc787f..dcb2a2f 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -692,6 +692,82 @@ bool bt_gatt_discover_secondary_services(struct bt_att *att, bt_uuid_t *uuid,
> destroy, false);
> }
>
> +static void discover_included_cb(uint8_t opcode, const void *pdu,
> + uint16_t length, void *user_data)
> +{
> + struct bt_gatt_result *final_result = NULL;
> + struct discovery_op *op = user_data;
> + struct bt_gatt_result *cur_result;
> + uint8_t att_ecode = 0;
> + uint16_t last_handle;
> + size_t data_length;
> + bool success;
> +
> + if (opcode == BT_ATT_OP_ERROR_RSP) {
> + success = false;
set this right before goto , otherwise it looks strange to
set success to false and then jump to success label.
> + att_ecode = process_error(pdu, length);
> +
> + if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
> + op->result_head)
> + goto success;
> +
> + goto done;
> + }
> +
> + if (opcode != BT_ATT_OP_READ_BY_TYPE_RSP || !pdu || length < 6) {
> + success = false;
> + goto done;
> + }
> +
> + data_length = ((uint8_t *) pdu)[0];
pdu is const pointer so cast it to (const uint8_t *).
Also I'm not entirely sure about this byte pdu processing, maybe we should have
proper packed structs for those (not sure how feasible that would be, though)
> +
Please comment this in code.
> + if ((length - 1) % data_length || data_length != 8) {
> + success = false;
> + goto done;
> + }
> +
> + cur_result = result_create(opcode, pdu + 1, length - 1,
> + data_length, op);
> + if (!cur_result) {
> + success = false;
> + goto done;
> + }
> +
> + if (!op->result_head)
> + op->result_head = op->result_tail = cur_result;
Both branches should in braces.
> + else {
> + op->result_tail->next = cur_result;
> + op->result_tail = cur_result;
> + }
> +
> + last_handle = get_le16(pdu + length - data_length);
> + if (last_handle != op->end_handle) {
> + uint8_t pdu[6];
> +
> + put_le16(last_handle + 1, pdu);
> + put_le16(op->end_handle, pdu + 2);
> + put_le16(GATT_INCLUDE_UUID, pdu + 4);
> +
> + if (bt_att_send(op->att, BT_ATT_OP_READ_BY_TYPE_REQ,
> + pdu, sizeof(pdu),
> + discover_included_cb,
> + discovery_op_ref(op),
> + discovery_op_unref))
> + return;
> +
> + discovery_op_unref(op);
> + success = false;
goto done: missing maybe?
also I'd change labels' names a bit:
done -> failed
success -> done
> + }
> +
> +success:
> + success = true;
> + final_result = op->result_head;
> +
> +done:
> + if (op->callback)
> + op->callback(success, att_ecode, final_result, op->user_data);
> +}
> +
> bool bt_gatt_discover_included_services(struct bt_att *att,
> uint16_t start, uint16_t end,
> bt_uuid_t *uuid,
> @@ -699,8 +775,36 @@ bool bt_gatt_discover_included_services(struct bt_att *att,
> void *user_data,
> bt_gatt_destroy_func_t destroy)
> {
> - /* TODO */
> - return false;
> + struct discovery_op *op;
> + uint8_t pdu[6];
> +
> + if (!att)
> + return false;
> +
> + op = new0(struct discovery_op, 1);
> + if (!op)
> + return false;
> +
> + op->att = att;
> + op->callback = callback;
> + op->user_data = user_data;
> + op->destroy = destroy;
> + op->end_handle = end;
> +
> + put_le16(start, pdu);
> + put_le16(end, pdu + 2);
> + put_le16(GATT_INCLUDE_UUID, pdu + 4);
> +
> + if (!bt_att_send(att, BT_ATT_OP_READ_BY_TYPE_REQ,
> + pdu, sizeof(pdu),
> + discover_included_cb,
> + discovery_op_ref(op),
> + discovery_op_unref)) {
> + free(op);
> + return false;
> + }
> +
> + return true;
> }
>
> static void discover_chrcs_cb(uint8_t opcode, const void *pdu,
>
--
Best regards,
Szymon Janc
^ permalink raw reply
* Re: [PATCHv5 01/14] shared/gatt: Add discover_secondary_services()
From: Szymon Janc @ 2014-10-22 13:37 UTC (permalink / raw)
To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413454646-23076-2-git-send-email-marcin.kraglak@tieto.com>
Hi Marcin,
On Thursday 16 of October 2014 12:17:13 Marcin Kraglak wrote:
> This pach implements searching Secondary Services in given range.
typo pach -> patch
> ---
> src/shared/gatt-helpers.c | 56 +++++++++++++++++++++++++++++++++--------------
> src/shared/gatt-helpers.h | 5 +++++
> 2 files changed, 45 insertions(+), 16 deletions(-)
>
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 55e6992..4dc787f 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -175,6 +175,7 @@ struct discovery_op {
> uint16_t end_handle;
> int ref_count;
> bt_uuid_t uuid;
> + uint16_t service_type;
> struct bt_gatt_result *result_head;
> struct bt_gatt_result *result_tail;
> bt_gatt_discovery_callback_t callback;
> @@ -487,7 +488,7 @@ static void read_by_grp_type_cb(uint8_t opcode, const void *pdu,
>
> put_le16(last_end + 1, pdu);
> put_le16(op->end_handle, pdu + 2);
> - put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
> + put_le16(op->service_type, pdu + 4);
>
> if (bt_att_send(op->att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
> pdu, sizeof(pdu),
> @@ -569,7 +570,7 @@ static void find_by_type_val_cb(uint8_t opcode, const void *pdu,
>
> put_le16(last_end + 1, pdu);
> put_le16(op->end_handle, pdu + 2);
> - put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
> + put_le16(op->service_type, pdu + 4);
> put_uuid_le(&op->uuid, pdu + 6);
>
> if (bt_att_send(op->att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ,
> @@ -594,21 +595,12 @@ done:
> op->callback(success, att_ecode, final_result, op->user_data);
> }
>
> -bool bt_gatt_discover_all_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> - bt_gatt_discovery_callback_t callback,
> - void *user_data,
> - bt_gatt_destroy_func_t destroy)
> -{
> - return bt_gatt_discover_primary_services(att, uuid, 0x0001, 0xffff,
> - callback, user_data,
> - destroy);
> -}
> -
> -bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> +static bool discover_services(struct bt_att *att, bt_uuid_t *uuid,
> uint16_t start, uint16_t end,
> bt_gatt_discovery_callback_t callback,
> void *user_data,
> - bt_gatt_destroy_func_t destroy)
> + bt_gatt_destroy_func_t destroy,
> + bool primary)
> {
> struct discovery_op *op;
> bool result;
> @@ -625,6 +617,8 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> op->callback = callback;
> op->user_data = user_data;
> op->destroy = destroy;
> + /* set service uuid to primary or secondary */
> + op->service_type = primary ? GATT_PRIM_SVC_UUID : GATT_SND_SVC_UUID;
>
> /* If UUID is NULL, then discover all primary services */
> if (!uuid) {
> @@ -632,7 +626,7 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
>
> put_le16(start, pdu);
> put_le16(end, pdu + 2);
> - put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
> + put_le16(op->service_type, pdu + 4);
>
> result = bt_att_send(att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
> pdu, sizeof(pdu),
> @@ -652,7 +646,7 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
>
> put_le16(start, pdu);
> put_le16(end, pdu + 2);
> - put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
> + put_le16(op->service_type, pdu + 4);
> put_uuid_le(&op->uuid, pdu + 6);
>
> result = bt_att_send(att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ,
> @@ -668,6 +662,36 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> return result;
> }
>
> +bool bt_gatt_discover_all_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + return bt_gatt_discover_primary_services(att, uuid, 0x0001, 0xffff,
> + callback, user_data,
> + destroy);
> +}
> +
> +bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> + uint16_t start, uint16_t end,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + return discover_services(att, uuid, start, end, callback, user_data,
> + destroy, true);
> +}
> +
> +bool bt_gatt_discover_secondary_services(struct bt_att *att, bt_uuid_t *uuid,
> + uint16_t start, uint16_t end,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + return discover_services(att, uuid, start, end, callback, user_data,
> + destroy, false);
> +}
> +
> bool bt_gatt_discover_included_services(struct bt_att *att,
> uint16_t start, uint16_t end,
> bt_uuid_t *uuid,
> diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
> index f6f4b62..8a25dea 100644
> --- a/src/shared/gatt-helpers.h
> +++ b/src/shared/gatt-helpers.h
> @@ -72,6 +72,11 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> bt_gatt_discovery_callback_t callback,
> void *user_data,
> bt_gatt_destroy_func_t destroy);
> +bool bt_gatt_discover_secondary_services(struct bt_att *att, bt_uuid_t *uuid,
> + uint16_t start, uint16_t end,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy);
> bool bt_gatt_discover_included_services(struct bt_att *att,
> uint16_t start, uint16_t end,
> bt_uuid_t *uuid,
This is not related to this patch only but I have a feeling that those
functions names are getting to long. Maybe we should think of something
more compact?
--
Best regards,
Szymon Janc
^ permalink raw reply
* [PATCHv2 3/3] bnep: Return errno instead of -1 and print error
From: Andrei Emeltchenko @ 2014-10-22 13:34 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413984870-24023-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Make code consistent with the rest returning -errno and printing error
message.
---
profiles/network/bnep.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index d8883d6..09d4b65 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -520,7 +520,7 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
{
int ifindex;
struct ifreq ifr;
- int sk, err;
+ int sk, err = 0;
if (!devname || !bridge)
return -EINVAL;
@@ -535,16 +535,16 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
strncpy(ifr.ifr_name, bridge, IFNAMSIZ - 1);
ifr.ifr_ifindex = ifindex;
- err = ioctl(sk, SIOCBRDELIF, &ifr);
+ if (ioctl(sk, SIOCBRDELIF, &ifr) < 0) {
+ err = -errno;
+ error("bnep: Can't delete %s from the bridge %s: %s(%d)",
+ devname, bridge, strerror(-err), -err);
+ } else
+ info("bridge %s: interface %s removed", bridge, devname);
close(sk);
- if (err < 0)
- return err;
-
- info("bridge %s: interface %s removed", bridge, devname);
-
- return 0;
+ return err;
}
int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
--
1.9.1
^ permalink raw reply related
* [PATCHv2 2/3] bnep: trivial: code cleanup
From: Andrei Emeltchenko @ 2014-10-22 13:34 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413984870-24023-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
profiles/network/bnep.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index f8fcd1f..d8883d6 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -133,7 +133,6 @@ const char *bnep_name(uint16_t id)
int bnep_init(void)
{
ctl = socket(PF_BLUETOOTH, SOCK_RAW, BTPROTO_BNEP);
-
if (ctl < 0) {
int err = -errno;
@@ -141,7 +140,7 @@ int bnep_init(void)
warn("kernel lacks bnep-protocol support");
else
error("bnep: Failed to open control socket: %s (%d)",
- strerror(-err), -err);
+ strerror(-err), -err);
return err;
}
@@ -165,7 +164,7 @@ static int bnep_conndel(const bdaddr_t *dst)
if (ioctl(ctl, BNEPCONNDEL, &req) < 0) {
int err = -errno;
error("bnep: Failed to kill connection: %s (%d)",
- strerror(-err), -err);
+ strerror(-err), -err);
return err;
}
return 0;
@@ -184,7 +183,7 @@ static int bnep_connadd(int sk, uint16_t role, char *dev)
if (ioctl(ctl, BNEPCONNADD, &req) < 0) {
int err = -errno;
error("bnep: Failed to add device %s: %s(%d)",
- dev, strerror(-err), -err);
+ dev, strerror(-err), -err);
return err;
}
@@ -208,7 +207,7 @@ static int bnep_if_up(const char *devname)
if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
err = -errno;
error("bnep: Could not bring up %s: %s(%d)",
- devname, strerror(-err), -err);
+ devname, strerror(-err), -err);
}
close(sk);
--
1.9.1
^ permalink raw reply related
* [PATCHv2 1/3] bnep: Add error print and return errno instead of -1
From: Andrei Emeltchenko @ 2014-10-22 13:34 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <CAMv5Gbck+=b9kY-s+gd3shJsLUDAEZib5dKGM09ZBWkP==ufFw@mail.gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
profiles/network/bnep.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 927367c..f8fcd1f 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -219,7 +219,7 @@ static int bnep_if_up(const char *devname)
static int bnep_if_down(const char *devname)
{
struct ifreq ifr;
- int sk, err;
+ int sk, err = 0;
sk = socket(AF_INET, SOCK_DGRAM, 0);
@@ -229,16 +229,15 @@ static int bnep_if_down(const char *devname)
ifr.ifr_flags &= ~IFF_UP;
/* Bring down the interface */
- err = ioctl(sk, SIOCSIFFLAGS, (void *) &ifr);
+ if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
+ err = -errno;
+ error("bnep: Could not bring down %s: %s(%d)",
+ devname, strerror(-err), -err);
+ }
close(sk);
- if (err < 0) {
- error("bnep: Could not bring down %s", devname);
- return err;
- }
-
- return 0;
+ return err;
}
static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
--
1.9.1
^ permalink raw reply related
* Re: rtk_btusb issues
From: Marcel Holtmann @ 2014-10-22 13:05 UTC (permalink / raw)
To: Patrick Shirkey; +Cc: linux-bluetooth
In-Reply-To: <59774.86.105.94.79.1413974113.squirrel@boosthardware.com>
Hi Patrick,
> I have an android device running the rtk_btusb driver. The original driver
> was a couple of years old and there was a problem with stability with BLE.
> In addition the localname was not found in the scan_response data.
>
> I upgraded the bluetooth stack by manually backporting from a recent
> kernel. That fixed the issue wit the scan_repsonse data but there were
> still stability issues with BLE devices (although slightly improved
> compared to the older bluetooth stack).
>
> I have tried upgrading the driver to the latest version of the rtk_btusb
> driver from the git repo and ported the hooks for bluedroid from the old
> driver however I get issues with hci send command when loading the driver.
>
> http://pastebin.com/hXALmXBr
>
> I traced the command but I am not sure where the send value is generated.
> I can see the function definition in the hci_dev struct but not the
> location where the value is actually set.
>
> I thought it might be an issue specific to bluedroid so I installed bluez
> but the issue is still there so it appears to be a bug in my version of
> the driver or something wrong with the bluetooth kernel layer.
>
> I went back to the older version of the driver running against bluez
> instead of bluedroid. That gets me further along but I still cannot
> initialise the bluetooth system on the device.
>
> http://pastebin.com/HcSZXiMu
>
> Does anyone have any suggestions for how to go about resolving this
> situation I have got myself into?
the rtk_btusb driver is not an upstream driver and that is the main problem. There has been recently an effort to get btusb support the Realtek devices, but then it went silent again.
What you need is having proper Realtek support in btusb and nothing else. Out of tree hacked up vendor drivers are not something any of us will likely have a look at and trying to fix.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 3/3] bnep: Return errno instead of -1 and print error
From: Grzegorz Kolodziejczyk @ 2014-10-22 12:55 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1413976251-32402-3-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
On 22 October 2014 13:10, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Make code consistent with the rest returning -errno and printing error
> message.
> ---
> profiles/network/bnep.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index 4cf38d9..09d4b65 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -231,7 +231,7 @@ static int bnep_if_down(const char *devname)
> if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
> err = -errno;
> error("bnep: Could not bring down %s: %s(%d)",
> - devname, strerror(-err), -err);
> + devname, strerror(-err), -err);
Why don't you fix this indention in first patch ? (this line is added
in previous patch)
> }
>
> close(sk);
> @@ -520,7 +520,7 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
> {
> int ifindex;
> struct ifreq ifr;
> - int sk, err;
> + int sk, err = 0;
>
> if (!devname || !bridge)
> return -EINVAL;
> @@ -535,16 +535,16 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
> strncpy(ifr.ifr_name, bridge, IFNAMSIZ - 1);
> ifr.ifr_ifindex = ifindex;
>
> - err = ioctl(sk, SIOCBRDELIF, &ifr);
> + if (ioctl(sk, SIOCBRDELIF, &ifr) < 0) {
> + err = -errno;
> + error("bnep: Can't delete %s from the bridge %s: %s(%d)",
> + devname, bridge, strerror(-err), -err);
> + } else
> + info("bridge %s: interface %s removed", bridge, devname);
>
> close(sk);
>
> - if (err < 0)
> - return err;
> -
> - info("bridge %s: interface %s removed", bridge, devname);
> -
> - return 0;
> + return err;
> }
>
> int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
BR,
Grzegorz
^ permalink raw reply
* Re: [PATCH] android/pts: Update PTS files for HOGP
From: Szymon Janc @ 2014-10-22 11:50 UTC (permalink / raw)
To: Sebastian Chlad; +Cc: linux-bluetooth
In-Reply-To: <1413885347-28938-1-git-send-email-sebastian.chlad@tieto.com>
Hi Sebastian,
On Tuesday 21 of October 2014 11:55:47 Sebastian Chlad wrote:
> PICS and PIXITs updated to PTS 5.3. Regression done for Android
> 4.4.4.
> ---
> android/pics-hogp.txt | 2 +-
> android/pixit-hogp.txt | 2 +-
> android/pts-hogp.txt | 4 ++--
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/android/pics-hogp.txt b/android/pics-hogp.txt
> index d7192bf..61cf56a 100644
> --- a/android/pics-hogp.txt
> +++ b/android/pics-hogp.txt
> @@ -1,6 +1,6 @@
> HOGP PICS for the PTS tool.
>
> -PTS version: 5.2
> +PTS version: 5.3
>
> * - different than PTS defaults
> # - not yet implemented/supported
> diff --git a/android/pixit-hogp.txt b/android/pixit-hogp.txt
> index 067c280..d32fe69 100644
> --- a/android/pixit-hogp.txt
> +++ b/android/pixit-hogp.txt
> @@ -1,6 +1,6 @@
> HOGP PIXIT for the PTS tool.
>
> -PTS version: 5.2
> +PTS version: 5.3
>
> * - different than PTS defaults
> & - should be set to IUT Bluetooth address
> diff --git a/android/pts-hogp.txt b/android/pts-hogp.txt
> index 19292d1..d86522d 100644
> --- a/android/pts-hogp.txt
> +++ b/android/pts-hogp.txt
> @@ -1,7 +1,7 @@
> PTS test results for HoG
>
> -PTS version: 5.2
> -Tested: 22-July-2014
> +PTS version: 5.3
> +Tested: 21-October-2014
> Android version: 4.4.4
>
> Results:
>
Patch applied, thanks.
--
Best regards,
Szymon Janc
^ permalink raw reply
* [PATCH 3/3] bnep: Return errno instead of -1 and print error
From: Andrei Emeltchenko @ 2014-10-22 11:10 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413976251-32402-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Make code consistent with the rest returning -errno and printing error
message.
---
profiles/network/bnep.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 4cf38d9..09d4b65 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -231,7 +231,7 @@ static int bnep_if_down(const char *devname)
if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
err = -errno;
error("bnep: Could not bring down %s: %s(%d)",
- devname, strerror(-err), -err);
+ devname, strerror(-err), -err);
}
close(sk);
@@ -520,7 +520,7 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
{
int ifindex;
struct ifreq ifr;
- int sk, err;
+ int sk, err = 0;
if (!devname || !bridge)
return -EINVAL;
@@ -535,16 +535,16 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
strncpy(ifr.ifr_name, bridge, IFNAMSIZ - 1);
ifr.ifr_ifindex = ifindex;
- err = ioctl(sk, SIOCBRDELIF, &ifr);
+ if (ioctl(sk, SIOCBRDELIF, &ifr) < 0) {
+ err = -errno;
+ error("bnep: Can't delete %s from the bridge %s: %s(%d)",
+ devname, bridge, strerror(-err), -err);
+ } else
+ info("bridge %s: interface %s removed", bridge, devname);
close(sk);
- if (err < 0)
- return err;
-
- info("bridge %s: interface %s removed", bridge, devname);
-
- return 0;
+ return err;
}
int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
--
1.9.1
^ permalink raw reply related
* [PATCH 2/3] bnep: trivial: code cleanup
From: Andrei Emeltchenko @ 2014-10-22 11:10 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413976251-32402-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
profiles/network/bnep.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 1024919..4cf38d9 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -133,7 +133,6 @@ const char *bnep_name(uint16_t id)
int bnep_init(void)
{
ctl = socket(PF_BLUETOOTH, SOCK_RAW, BTPROTO_BNEP);
-
if (ctl < 0) {
int err = -errno;
@@ -141,7 +140,7 @@ int bnep_init(void)
warn("kernel lacks bnep-protocol support");
else
error("bnep: Failed to open control socket: %s (%d)",
- strerror(-err), -err);
+ strerror(-err), -err);
return err;
}
@@ -165,7 +164,7 @@ static int bnep_conndel(const bdaddr_t *dst)
if (ioctl(ctl, BNEPCONNDEL, &req) < 0) {
int err = -errno;
error("bnep: Failed to kill connection: %s (%d)",
- strerror(-err), -err);
+ strerror(-err), -err);
return err;
}
return 0;
@@ -184,7 +183,7 @@ static int bnep_connadd(int sk, uint16_t role, char *dev)
if (ioctl(ctl, BNEPCONNADD, &req) < 0) {
int err = -errno;
error("bnep: Failed to add device %s: %s(%d)",
- dev, strerror(-err), -err);
+ dev, strerror(-err), -err);
return err;
}
@@ -208,7 +207,7 @@ static int bnep_if_up(const char *devname)
if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
err = -errno;
error("bnep: Could not bring up %s: %s(%d)",
- devname, strerror(-err), -err);
+ devname, strerror(-err), -err);
}
close(sk);
--
1.9.1
^ permalink raw reply related
* [PATCH 1/3] bnep: Add error print and return errno instead of -1
From: Andrei Emeltchenko @ 2014-10-22 11:10 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
profiles/network/bnep.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 927367c..1024919 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -219,7 +219,7 @@ static int bnep_if_up(const char *devname)
static int bnep_if_down(const char *devname)
{
struct ifreq ifr;
- int sk, err;
+ int sk, err = 0;
sk = socket(AF_INET, SOCK_DGRAM, 0);
@@ -229,16 +229,15 @@ static int bnep_if_down(const char *devname)
ifr.ifr_flags &= ~IFF_UP;
/* Bring down the interface */
- err = ioctl(sk, SIOCSIFFLAGS, (void *) &ifr);
+ if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
+ err = -errno;
+ error("bnep: Could not bring down %s: %s(%d)",
+ devname, strerror(-err), -err);
+ }
close(sk);
- if (err < 0) {
- error("bnep: Could not bring down %s", devname);
- return err;
- }
-
- return 0;
+ return err;
}
static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v4 01/11] shared/hfp: Add support for HFP HF
From: Szymon Janc @ 2014-10-22 11:00 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
In-Reply-To: <1412898611-12199-2-git-send-email-lukasz.rymanowski@tieto.com>
Hi Łukasz,
On Friday 10 of October 2014 01:50:01 Lukasz Rymanowski wrote:
> This patch add struct hfp_hf plus fuctions to create an instance ref and
> unref. This code based on existing hfp_gw
> ---
> src/shared/hfp.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/hfp.h | 6 ++++
> 2 files changed, 98 insertions(+)
>
> diff --git a/src/shared/hfp.c b/src/shared/hfp.c
> index efc981f..dbd049a 100644
> --- a/src/shared/hfp.c
> +++ b/src/shared/hfp.c
> @@ -62,6 +62,18 @@ struct hfp_gw {
> bool destroyed;
> };
>
> +struct hfp_hf {
> + int ref_count;
> + int fd;
> + bool close_on_unref;
> + struct io *io;
> + struct ringbuf *read_buf;
> + struct ringbuf *write_buf;
> +
> + bool in_disconnect;
> + bool destroyed;
> +};
> +
> struct cmd_handler {
> char *prefix;
> void *user_data;
> @@ -807,3 +819,83 @@ bool hfp_gw_disconnect(struct hfp_gw *hfp)
>
> return io_shutdown(hfp->io);
> }
> +
> +struct hfp_hf *hfp_hf_new(int fd)
> +{
> + struct hfp_hf *hfp;
> +
> + if (fd < 0)
> + return NULL;
> +
> + hfp = new0(struct hfp_hf, 1);
> + if (!hfp)
> + return NULL;
> +
> + hfp->fd = fd;
> + hfp->close_on_unref = false;
> +
> + hfp->read_buf = ringbuf_new(4096);
> + if (!hfp->read_buf) {
> + free(hfp);
> + return NULL;
> + }
> +
> + hfp->write_buf = ringbuf_new(4096);
> + if (!hfp->write_buf) {
> + ringbuf_free(hfp->read_buf);
> + free(hfp);
> + return NULL;
> + }
> +
> + hfp->io = io_new(fd);
> + if (!hfp->io) {
> + ringbuf_free(hfp->write_buf);
> + ringbuf_free(hfp->read_buf);
> + free(hfp);
> + return NULL;
> + }
> +
> + return hfp_hf_ref(hfp);
> +}
> +
> +struct hfp_hf *hfp_hf_ref(struct hfp_hf *hfp)
> +{
> + if (!hfp)
> + return NULL;
> +
> + __sync_fetch_and_add(&hfp->ref_count, 1);
> +
> + return hfp;
> +}
> +
> +void hfp_hf_unref(struct hfp_hf *hfp)
> +{
> + if (!hfp)
> + return;
> +
> + if (__sync_sub_and_fetch(&hfp->ref_count, 1))
> + return;
> +
> + io_set_write_handler(hfp->io, NULL, NULL, NULL);
> + io_set_read_handler(hfp->io, NULL, NULL, NULL);
> + io_set_disconnect_handler(hfp->io, NULL, NULL, NULL);
> +
> + io_destroy(hfp->io);
> + hfp->io = NULL;
> +
> + if (hfp->close_on_unref)
> + close(hfp->fd);
> +
> + ringbuf_free(hfp->read_buf);
> + hfp->read_buf = NULL;
> +
> + ringbuf_free(hfp->write_buf);
> + hfp->write_buf = NULL;
> +
> + if (!hfp->in_disconnect) {
> + free(hfp);
> + return;
> + }
> +
> + hfp->destroyed = true;
> +}
> diff --git a/src/shared/hfp.h b/src/shared/hfp.h
> index 743db65..50d9c4b 100644
> --- a/src/shared/hfp.h
> +++ b/src/shared/hfp.h
> @@ -76,9 +76,11 @@ typedef void (*hfp_destroy_func_t)(void *user_data);
> typedef void (*hfp_debug_func_t)(const char *str, void *user_data);
>
> typedef void (*hfp_command_func_t)(const char *command, void *user_data);
> +
Unrelated.
> typedef void (*hfp_disconnect_func_t)(void *user_data);
>
> struct hfp_gw;
> +struct hfp_hf;
I'd prefer if we have all hfp_hf stuff in same section.
>
> struct hfp_gw *hfp_gw_new(int fd);
>
> @@ -124,3 +126,7 @@ bool hfp_gw_result_get_string(struct hfp_gw_result *result, char *buf,
> bool hfp_gw_result_get_unquoted_string(struct hfp_gw_result *result, char *buf,
> uint8_t len);
> bool hfp_gw_result_has_next(struct hfp_gw_result *result);
> +
> +struct hfp_hf *hfp_hf_new(int fd);
> +struct hfp_hf *hfp_hf_ref(struct hfp_hf *hfp);
> +void hfp_hf_unref(struct hfp_hf *hfp);
>
--
Best regards,
Szymon Janc
^ permalink raw reply
* Re: [PATCH v4 04/11] shared/hfp: Add register/unregister event for HFP HF
From: Szymon Janc @ 2014-10-22 11:00 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
In-Reply-To: <1412898611-12199-5-git-send-email-lukasz.rymanowski@tieto.com>
Hi Łukasz,
On Friday 10 of October 2014 01:50:04 Lukasz Rymanowski wrote:
> This patch adds API which allows to register/unregister for unsolicited
> responses.
> ---
> src/shared/hfp.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/hfp.h | 8 +++++
> 2 files changed, 116 insertions(+)
>
> diff --git a/src/shared/hfp.c b/src/shared/hfp.c
> index b7855ed..b1cf08e 100644
> --- a/src/shared/hfp.c
> +++ b/src/shared/hfp.c
> @@ -70,6 +70,8 @@ struct hfp_hf {
> struct ringbuf *read_buf;
> struct ringbuf *write_buf;
>
> + struct queue *event_handlers;
> +
> hfp_debug_func_t debug_callback;
> hfp_destroy_func_t debug_destroy;
> void *debug_data;
> @@ -94,6 +96,18 @@ struct hfp_gw_result {
> unsigned int offset;
> };
>
> +struct hfp_hf_result {
> + const char *data;
> + unsigned int offset;
> +};
> +
> +struct event_handler {
> + char *prefix;
> + void *user_data;
> + hfp_destroy_func_t destroy;
> + hfp_hf_result_func_t callback;
> +};
> +
> static void destroy_cmd_handler(void *data)
> {
> struct cmd_handler *handler = data;
> @@ -828,6 +842,32 @@ bool hfp_gw_disconnect(struct hfp_gw *hfp)
> return io_shutdown(hfp->io);
> }
>
> +static bool match_handler_event_prefix(const void *a, const void *b)
> +{
> + const struct event_handler *handler = a;
> + const char *prefix = b;
> +
> + if (strlen(handler->prefix) != strlen(prefix))
> + return false;
> +
> + if (memcmp(handler->prefix, prefix, strlen(prefix)))
> + return false;
Why not just use strcmp() here?
I'm aware that this was based on gw code so you may also fix it there :)
> +
> + return true;
> +}
> +
> +static void destroy_event_handler(void *data)
> +{
> + struct event_handler *handler = data;
> +
> + if (handler->destroy)
> + handler->destroy(handler->user_data);
> +
> + free(handler->prefix);
> +
> + free(handler);
> +}
> +
> struct hfp_hf *hfp_hf_new(int fd)
> {
> struct hfp_hf *hfp;
> @@ -863,6 +903,15 @@ struct hfp_hf *hfp_hf_new(int fd)
> return NULL;
> }
>
> + hfp->event_handlers = queue_new();
> + if (!hfp->event_handlers) {
> + io_destroy(hfp->io);
> + ringbuf_free(hfp->write_buf);
> + ringbuf_free(hfp->read_buf);
> + free(hfp);
> + return NULL;
> + }
> +
> return hfp_hf_ref(hfp);
> }
>
> @@ -902,6 +951,9 @@ void hfp_hf_unref(struct hfp_hf *hfp)
> ringbuf_free(hfp->write_buf);
> hfp->write_buf = NULL;
>
> + queue_destroy(hfp->event_handlers, destroy_event_handler);
> + hfp->event_handlers = NULL;
> +
> if (!hfp->in_disconnect) {
> free(hfp);
> return;
> @@ -961,6 +1013,62 @@ bool hfp_hf_set_close_on_unref(struct hfp_hf *hfp, bool do_close)
> return true;
> }
>
> +bool hfp_hf_register(struct hfp_hf *hfp, hfp_hf_result_func_t callback,
> + const char *prefix,
> + void *user_data,
> + hfp_destroy_func_t destroy)
> +{
> + struct event_handler *handler;
> +
> + if (!callback)
> + return false;
> +
> + handler = new0(struct event_handler, 1);
> + if (!handler)
> + return false;
> +
> + handler->callback = callback;
> + handler->user_data = user_data;
> +
> + handler->prefix = strdup(prefix);
> + if (!handler->prefix) {
> + free(handler);
> + return false;
> + }
> +
> + if (queue_find(hfp->event_handlers, match_handler_event_prefix,
> + handler->prefix)) {
> + destroy_event_handler(handler);
> + return false;
> + }
> +
> + handler->destroy = destroy;
> +
> + return queue_push_tail(hfp->event_handlers, handler);
> +}
> +
> +bool hfp_hf_unregister(struct hfp_hf *hfp, const char *prefix)
> +{
> + struct cmd_handler *handler;
> + char *lookup_prefix;
> +
> + lookup_prefix = strdup(prefix);
> + if (!lookup_prefix)
> + return false;
This strdup seems superfluous. If this is only due to to queue api being
non-const then I'd just cast it to (void *).
> +
> + handler = queue_remove_if(hfp->event_handlers,
> + match_handler_event_prefix,
> + lookup_prefix);
> + free(lookup_prefix);
> +
> + if (!handler)
> + return false;
> +
> + destroy_event_handler(handler);
> +
> + return true;
> +}
> +
> static void hf_disconnect_watch_destroy(void *user_data)
> {
> struct hfp_hf *hfp = user_data;
> diff --git a/src/shared/hfp.h b/src/shared/hfp.h
> index a9a169b..85037b1 100644
> --- a/src/shared/hfp.h
> +++ b/src/shared/hfp.h
> @@ -68,10 +68,14 @@ enum hfp_gw_cmd_type {
> };
>
> struct hfp_gw_result;
> +struct hfp_hf_result;
As before, I'd keep hf stuff in single section.
>
> typedef void (*hfp_result_func_t)(struct hfp_gw_result *result,
> enum hfp_gw_cmd_type type, void *user_data);
>
> +typedef void (*hfp_hf_result_func_t)(struct hfp_hf_result *result,
> + void *user_data);
> +
Same here.
> typedef void (*hfp_destroy_func_t)(void *user_data);
> typedef void (*hfp_debug_func_t)(const char *str, void *user_data);
>
> @@ -138,3 +142,7 @@ bool hfp_hf_set_disconnect_handler(struct hfp_hf *hfp,
> void *user_data,
> hfp_destroy_func_t destroy);
> bool hfp_hf_disconnect(struct hfp_hf *hfp);
> +bool hfp_hf_register(struct hfp_hf *hfp, hfp_hf_result_func_t callback,
> + const char *prefix, void *user_data,
> + hfp_destroy_func_t destroy);
> +bool hfp_hf_unregister(struct hfp_hf *hfp, const char *prefix);
>
--
Best regards,
Szymon Janc
^ permalink raw reply
* Re: [PATCH v4 05/11] shared/hfp: Add HFP HF parser
From: Szymon Janc @ 2014-10-22 11:00 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
In-Reply-To: <1412898611-12199-6-git-send-email-lukasz.rymanowski@tieto.com>
Hi Łukasz,
On Friday 10 of October 2014 01:50:05 Lukasz Rymanowski wrote:
> This patch adds parser for AT responses and unsolicited results.
> ---
> src/shared/hfp.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 129 insertions(+)
>
> diff --git a/src/shared/hfp.c b/src/shared/hfp.c
> index b1cf08e..5179092 100644
> --- a/src/shared/hfp.c
> +++ b/src/shared/hfp.c
> @@ -868,6 +868,126 @@ static void destroy_event_handler(void *data)
> free(handler);
> }
>
> +static void hf_skip_whitespace(struct hfp_hf_result *result)
> +{
> + while (result->data[result->offset] == ' ')
> + result->offset++;
> +}
> +
> +static void hf_call_prefix_handler(struct hfp_hf *hfp, const char *data)
> +{
> + struct event_handler *handler;
> + const char *separators = ";:\0";
> + struct hfp_hf_result result_data;
> + char lookup_prefix[18];
> + uint8_t pref_len = 0;
> + const char *prefix;
> + int i;
> +
> + result_data.offset = 0;
> + result_data.data = data;
> +
> + hf_skip_whitespace(&result_data);
> +
> + if (strlen(data + result_data.offset) < 2)
> + return;
> +
> + prefix = data + result_data.offset;
> +
> + pref_len = strcspn(prefix, separators);
> + if (pref_len > 17 || pref_len < 2)
> + return;
> +
> + for (i = 0; i < pref_len; i++)
> + lookup_prefix[i] = toupper(prefix[i]);
> +
> + lookup_prefix[pref_len] = '\0';
> + result_data.offset += pref_len + 1;
> +
> + handler = queue_find(hfp->event_handlers, match_handler_event_prefix,
> + lookup_prefix);
> + if (!handler)
> + return;
> +
> + handler->callback(&result_data, handler->user_data);
> +}
> +
> +static char *find_cr_lf(char *str, size_t len)
> +{
> + char *ptr;
> + int count;
> + int offset;
> +
> + offset = 0;
> +
> + ptr = memchr(str, '\r', len);
> + while (ptr) {
> + /*
> + * Check if there is more data after '\r'. If so check for
> + * '\n'
> + */
> + count = ptr-str;
Style: spaces around -
> + if ((count < (int) (len - 1)) && *(ptr + 1) == '\n')
> + return ptr;
If you make count size_t then this cast is not needed.
> +
> + /* There is only '\r'? Let's try to find next one */
> + offset += count + 1;
> +
> + if (offset >= (int)len)
If you make offset size_t then this cast is not needed.
Also such casting should have space '(int) foo'.
> + return NULL;
> +
> + ptr = memchr(str + offset, '\r', len - offset);
> + }
> +
> + return NULL;
> +}
> +
> +static void hf_process_input(struct hfp_hf *hfp)
> +{
> + char *str, *ptr;
> + size_t len, count, offset;
> +
> + str = ringbuf_peek(hfp->read_buf, 0, &len);
> + if (!str)
> + return;
> +
> + offset = 0;
> +
> + ptr = find_cr_lf(str, len);
> + while (ptr) {
> + count = ptr - (str + offset);
If you would adjust str pointer instead of using str + offset everywhere
then this code would be a bit simpler to follow.
Also this would not handle wrapped string correctly. Check how this is handled
in process_input().
> + if (count == 0) {
> + /* 2 is for <cr><lf> */
> + offset += 2;
> + } else {
> + *ptr = '\0';
> + hf_call_prefix_handler(hfp, str + offset);
> + offset += count + 2;
> + }
> +
> + if (offset >= len)
> + break;
> +
> + ptr = find_cr_lf(str + offset, len - offset);
> + }
> +
> + ringbuf_drain(hfp->read_buf, offset < len ? offset : len);
> +}
> +
> +static bool hf_can_read_data(struct io *io, void *user_data)
> +{
> + struct hfp_hf *hfp = user_data;
> + ssize_t bytes_read;
> +
> + bytes_read = ringbuf_read(hfp->read_buf, hfp->fd);
> + if (bytes_read < 0)
> + return false;
> +
> + hf_process_input(hfp);
> +
> + return true;
> +}
> +
> struct hfp_hf *hfp_hf_new(int fd)
> {
> struct hfp_hf *hfp;
> @@ -912,6 +1032,15 @@ struct hfp_hf *hfp_hf_new(int fd)
> return NULL;
> }
>
> + if (!io_set_read_handler(hfp->io, hf_can_read_data, hfp,
> + read_watch_destroy)) {
> + queue_destroy(hfp->event_handlers,
> + destroy_event_handler);
> + io_destroy(hfp->io);
> + ringbuf_free(hfp->write_buf);
> + ringbuf_free(hfp->read_buf);
You are missing free(hfp); return NULL; here.
> + }
> +
> return hfp_hf_ref(hfp);
> }
>
>
--
Best regards,
Szymon Janc
^ permalink raw reply
* Re: [PATCH v4 06/11] shared/hfp: Add send AT command API for HFP HF
From: Szymon Janc @ 2014-10-22 11:00 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
In-Reply-To: <1412898611-12199-7-git-send-email-lukasz.rymanowski@tieto.com>
Hi Łukasz,
On Friday 10 of October 2014 01:50:06 Lukasz Rymanowski wrote:
> This patch adds handling send and response of AT command.
> Note that we always wait for AT command response before sending next
> command, however user can fill hfp_hf with more than one command.
> All the commands are queued and send one by one.
> ---
> src/shared/hfp.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/hfp.h | 8 +++
> 2 files changed, 183 insertions(+)
>
> diff --git a/src/shared/hfp.c b/src/shared/hfp.c
> index 5179092..8bebe97 100644
> --- a/src/shared/hfp.c
> +++ b/src/shared/hfp.c
> @@ -70,6 +70,9 @@ struct hfp_hf {
> struct ringbuf *read_buf;
> struct ringbuf *write_buf;
>
> + bool writer_active;
> + struct queue *cmd_queue;
> +
> struct queue *event_handlers;
>
> hfp_debug_func_t debug_callback;
> @@ -101,6 +104,14 @@ struct hfp_hf_result {
> unsigned int offset;
> };
>
> +struct cmd_response {
> + char *prefix;
> + hfp_response_func_t resp_cb;
> + struct hfp_hf_result *response;
> + char *resp_data;
> + void *user_data;
> +};
> +
> struct event_handler {
> char *prefix;
> void *user_data;
> @@ -868,17 +879,103 @@ static void destroy_event_handler(void *data)
> free(handler);
> }
>
> +static bool hf_can_write_data(struct io *io, void *user_data)
> +{
> + struct hfp_hf *hfp = user_data;
> + ssize_t bytes_written;
> +
> + bytes_written = ringbuf_write(hfp->write_buf, hfp->fd);
> + if (bytes_written < 0)
> + return false;
> +
> + if (ringbuf_len(hfp->write_buf) > 0)
> + return true;
> +
> + return false;
> +}
> +
> +static void hf_write_watch_destroy(void *user_data)
> +{
> + struct hfp_hf *hfp = user_data;
> +
> + hfp->writer_active = false;
> +}
> +
> static void hf_skip_whitespace(struct hfp_hf_result *result)
> {
> while (result->data[result->offset] == ' ')
> result->offset++;
> }
>
> +static bool is_response(const char *prefix, enum hfp_result *result)
> +{
> + if (strcmp(prefix, "OK") == 0) {
> + *result = HFP_RESULT_OK;
> + return true;
> + }
> +
> + if (strcmp(prefix, "ERROR") == 0) {
> + *result = HFP_RESULT_ERROR;
> + return true;
> + }
> +
> + if (strcmp(prefix, "NO CARRIER") == 0) {
> + *result = HFP_RESULT_NO_CARRIER;
> + return true;
> + }
> +
> + if (strcmp(prefix, "CONNECT") == 0) {
Shouldn't this be "BUSY"?
> + *result = HFP_RESULT_CONNECT;
And this enum value looks bogus to me.
I couldn't find it in HFP nor HSP spec. Probably leftover from AT spec.
I'd just handle what is defined in HFP spec here.
> + return true;
> + }
> +
> + if (strcmp(prefix, "NO ANSWER") == 0) {
> + *result = HFP_RESULT_NO_ANSWER;
> + return true;
> + }
> +
> + if (strcmp(prefix, "DELAYED") == 0) {
> + *result = HFP_RESULT_DELAYED;
> + return true;
> + }
> +
> + if (strcmp(prefix, "BLACKLISTED") == 0) {
> + *result = HFP_RESULT_BLACKLISTED;
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static void hf_wakeup_writer(struct hfp_hf *hfp)
> +{
> + if (hfp->writer_active)
> + return;
> +
> + if (!ringbuf_len(hfp->write_buf))
> + return;
> +
> + if (!io_set_write_handler(hfp->io, hf_can_write_data,
> + hfp, hf_write_watch_destroy))
> + return;
> +
> + hfp->writer_active = true;
> +}
> +
> +static void destroy_cmd_response(void *data)
> +{
> + struct cmd_response *cmd = data;
> +
> + free(cmd->prefix);
> + free(cmd);
> +}
> +
> static void hf_call_prefix_handler(struct hfp_hf *hfp, const char *data)
> {
> struct event_handler *handler;
> const char *separators = ";:\0";
> struct hfp_hf_result result_data;
> + enum hfp_result result;
> char lookup_prefix[18];
> uint8_t pref_len = 0;
> const char *prefix;
> @@ -904,6 +1001,22 @@ static void hf_call_prefix_handler(struct hfp_hf *hfp, const char *data)
> lookup_prefix[pref_len] = '\0';
> result_data.offset += pref_len + 1;
>
> + if (is_response(lookup_prefix, &result)) {
> + struct cmd_response *cmd;
> +
> + cmd = queue_peek_head(hfp->cmd_queue);
> + if (!cmd)
> + return;
> +
> + cmd->resp_cb(cmd->prefix, result, cmd->user_data);
> +
> + queue_remove(hfp->cmd_queue, cmd);
> + destroy_cmd_response(cmd);
> +
> + hf_wakeup_writer(hfp);
> + return;
> + }
> +
> handler = queue_find(hfp->event_handlers, match_handler_event_prefix,
> lookup_prefix);
> if (!handler)
> @@ -1032,6 +1145,18 @@ struct hfp_hf *hfp_hf_new(int fd)
> return NULL;
> }
>
> + hfp->cmd_queue = queue_new();
> + if (!hfp->cmd_queue) {
> + io_destroy(hfp->io);
> + ringbuf_free(hfp->write_buf);
> + ringbuf_free(hfp->read_buf);
> + queue_destroy(hfp->event_handlers, NULL);
> + free(hfp);
> + return NULL;
> + }
> +
> + hfp->writer_active = false;
> +
> if (!io_set_read_handler(hfp->io, hf_can_read_data, hfp,
> read_watch_destroy)) {
> queue_destroy(hfp->event_handlers,
> @@ -1083,6 +1208,9 @@ void hfp_hf_unref(struct hfp_hf *hfp)
> queue_destroy(hfp->event_handlers, destroy_event_handler);
> hfp->event_handlers = NULL;
>
> + queue_destroy(hfp->cmd_queue, destroy_cmd_response);
> + hfp->cmd_queue = NULL;
> +
> if (!hfp->in_disconnect) {
> free(hfp);
> return;
> @@ -1142,6 +1270,53 @@ bool hfp_hf_set_close_on_unref(struct hfp_hf *hfp, bool do_close)
> return true;
> }
>
> +bool hfp_hf_send_command(struct hfp_hf *hfp, hfp_response_func_t resp_cb,
> + void *user_data, const char *format, ...)
> +{
> + va_list ap;
> + char *fmt;
> + int len;
> + const char *separators = ";?=\0";
> + uint8_t prefix_len;
> + struct cmd_response *cmd;
> +
> + if (!hfp || !format || !resp_cb)
> + return false;
> +
> + if (asprintf(&fmt, "%s\r", format) < 0)
> + return false;
> +
> + cmd = new0(struct cmd_response, 1);
> + if (!cmd)
> + return false;
> +
> + va_start(ap, format);
> + len = ringbuf_vprintf(hfp->write_buf, fmt, ap);
> + va_end(ap);
> +
> + free(fmt);
> +
> + if (len < 0) {
> + free(cmd);
> + return false;
> + }
> +
> + prefix_len = strcspn(format, separators);
I'd explore possibility of passing prefix as separate argument to the
function to avoid need of this extra parsing. Also do we really need this
prefix at all? We should not have more than one pending AT command anyway.
> + cmd->prefix = strndup(format, prefix_len);
> + cmd->resp_cb = resp_cb;
> + cmd->user_data = user_data;
> +
> + if (!queue_push_tail(hfp->cmd_queue, cmd)) {
> + ringbuf_drain(hfp->write_buf, len);
> + free(cmd);
> + return false;
> + }
> +
> + hf_wakeup_writer(hfp);
> +
> + return true;
> +}
> +
> bool hfp_hf_register(struct hfp_hf *hfp, hfp_hf_result_func_t callback,
> const char *prefix,
> void *user_data,
> diff --git a/src/shared/hfp.h b/src/shared/hfp.h
> index 85037b1..5ee3017 100644
> --- a/src/shared/hfp.h
> +++ b/src/shared/hfp.h
> @@ -32,6 +32,8 @@ enum hfp_result {
> HFP_RESULT_NO_DIALTONE = 6,
> HFP_RESULT_BUSY = 7,
> HFP_RESULT_NO_ANSWER = 8,
> + HFP_RESULT_DELAYED = 9,
> + HFP_RESULT_BLACKLISTED = 10,
> };
>
> enum hfp_error {
> @@ -83,6 +85,10 @@ typedef void (*hfp_command_func_t)(const char *command, void *user_data);
>
> typedef void (*hfp_disconnect_func_t)(void *user_data);
>
> +typedef void (*hfp_response_func_t)(const char *prefix,
> + enum hfp_result result,
> + void *user_data);
> +
> struct hfp_gw;
> struct hfp_hf;
>
> @@ -146,3 +152,5 @@ bool hfp_hf_register(struct hfp_hf *hfp, hfp_hf_result_func_t callback,
> const char *prefix, void *user_data,
> hfp_destroy_func_t destroy);
> bool hfp_hf_unregister(struct hfp_hf *hfp, const char *prefix);
> +bool hfp_hf_send_command(struct hfp_hf *hfp, hfp_response_func_t resp_cb,
> + void *user_data, const char *format, ...);
>
--
Best regards,
Szymon Janc
^ permalink raw reply
* Re: [PATCH v4 08/11] unit/test-hfp: Add init test for HFP HF
From: Szymon Janc @ 2014-10-22 11:00 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
In-Reply-To: <1412898611-12199-9-git-send-email-lukasz.rymanowski@tieto.com>
Hi Łukasz,
On Friday 10 of October 2014 01:50:08 Lukasz Rymanowski wrote:
> This patch adds basic infrastruction for HFP HF test plus
> init test.
>
> It also moves send_pdu function in the file so it can be used by
> test_hf_handler
> ---
> unit/test-hfp.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 84 insertions(+), 18 deletions(-)
>
> diff --git a/unit/test-hfp.c b/unit/test-hfp.c
> index 4b3473b..274ee55 100644
> --- a/unit/test-hfp.c
> +++ b/unit/test-hfp.c
> @@ -36,6 +36,7 @@ struct context {
> int fd_server;
> int fd_client;
> struct hfp_gw *hfp;
> + struct hfp_hf *hfp_hf;
> const struct test_data *data;
> unsigned int pdu_offset;
> };
> @@ -52,6 +53,8 @@ struct test_data {
> char *test_name;
> struct test_pdu *pdu_list;
> hfp_result_func_t result_func;
> + hfp_response_func_t response_func;
> + hfp_hf_result_func_t hf_result_func;
> GIOFunc test_handler;
> };
>
> @@ -99,6 +102,22 @@ struct test_data {
> data.test_handler = test_handler; \
> } while (0)
>
> +#define define_hf_test(name, function, result_func, response_function, \
> + args...)\
> + do { \
> + const struct test_pdu pdus[] = { \
> + args, { } \
> + }; \
> + static struct test_data data; \
> + data.test_name = g_strdup(name); \
> + data.pdu_list = g_malloc(sizeof(pdus)); \
> + data.hf_result_func = result_func; \
> + data.response_func = response_function; \
> + memcpy(data.pdu_list, pdus, sizeof(pdus)); \
> + g_test_add_data_func(name, &data, function); \
> + data.test_handler = test_hf_handler; \
> + } while (0)
> +
> static void context_quit(struct context *context)
> {
> g_main_loop_quit(context->main_loop);
> @@ -128,6 +147,52 @@ static gboolean test_handler(GIOChannel *channel, GIOCondition cond,
> return FALSE;
> }
>
> +static gboolean send_pdu(gpointer user_data)
> +{
> + struct context *context = user_data;
> + const struct test_pdu *pdu;
> + ssize_t len;
> +
> + pdu = &context->data->pdu_list[context->pdu_offset++];
> +
> + if (pdu && !pdu->valid)
> + return FALSE;
> +
> + len = write(context->fd_server, pdu->data, pdu->size);
> + g_assert_cmpint(len, ==, pdu->size);
> +
> + pdu = &context->data->pdu_list[context->pdu_offset];
> + if (pdu->fragmented)
> + g_idle_add(send_pdu, context);
> +
> + return FALSE;
> +}
> +
> +static gboolean test_hf_handler(GIOChannel *channel, GIOCondition cond,
> + gpointer user_data)
> +{
> + struct context *context = user_data;
> + gchar buf[60];
> + gsize bytes_read;
> + GError *error = NULL;
> +
> + if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL))
> + goto done;
> +
> + /* dummy read */
> + g_io_channel_read_chars(channel, buf, 60, &bytes_read, &error);
> +
> + send_pdu(context);
> +
> + return TRUE;
> +
> +done:
> + context_quit(context);
> + context->watch_id = 0;
> +
> + return FALSE;
> +}
> +
> static void cmd_handler(const char *command, void *user_data)
> {
> struct context *context = user_data;
> @@ -203,6 +268,9 @@ static void execute_context(struct context *context)
> if (context->hfp)
> hfp_gw_unref(context->hfp);
>
> + if (context->hfp_hf)
> + hfp_hf_unref(context->hfp_hf);
> +
> g_free(context);
> }
>
> @@ -275,24 +343,6 @@ static void test_register(gconstpointer data)
> execute_context(context);
> }
>
> -static gboolean send_pdu(gpointer user_data)
> -{
> - struct context *context = user_data;
> - const struct test_pdu *pdu;
> - ssize_t len;
> -
> - pdu = &context->data->pdu_list[context->pdu_offset++];
> -
> - len = write(context->fd_server, pdu->data, pdu->size);
> - g_assert_cmpint(len, ==, pdu->size);
> -
> - pdu = &context->data->pdu_list[context->pdu_offset];
> - if (pdu->fragmented)
> - g_idle_add(send_pdu, context);
> -
> - return FALSE;
> -}
> -
> static void test_fragmented(gconstpointer data)
> {
> struct context *context = create_context(data);
> @@ -404,6 +454,20 @@ static void check_string_2(struct hfp_gw_result *result,
> hfp_gw_send_result(context->hfp, HFP_RESULT_ERROR);
> }
>
> +static void test_hf_init(gconstpointer data)
> +{
> + struct context *context = create_context(data);
> +
> + context->hfp_hf = hfp_hf_new(context->fd_client);
> + g_assert(context->hfp_hf);
> + g_assert(hfp_hf_set_close_on_unref(context->hfp_hf, true));
> +
> + hfp_hf_unref(context->hfp_hf);
> + context->hfp_hf = NULL;
> +
> + execute_context(context);
> +}
> +
> int main(int argc, char *argv[])
> {
> g_test_init(&argc, &argv, NULL);
> @@ -473,5 +537,7 @@ int main(int argc, char *argv[])
> raw_pdu('\r'),
> data_end());
>
> + define_hf_test("/hfp/test_init", test_hf_init, NULL, NULL, data_end());
I'd prefer if all hfp_hf tests were prefixed like this:
"/hfp_hf/test_foo"
This will allow to avoid doubling tests name like this one (there is already
/hfp/test_init test).
> +
> return g_test_run();
> }
>
--
Best regards,
Szymon Janc
^ permalink raw reply
* Re: [PATCH 1/3] obexd/map: Add support for MAP feature bits
From: Luiz Augusto von Dentz @ 2014-10-22 10:44 UTC (permalink / raw)
To: Gowtham Anandha Babu
Cc: linux-bluetooth@vger.kernel.org, Dmitry Kasatkin, Bharat Panda,
cpgs
In-Reply-To: <1413817469-4255-1-git-send-email-gowtham.ab@samsung.com>
Hi,
On Mon, Oct 20, 2014 at 6:04 PM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> Handles MAP supported feature bits as per the
> MAP 1.2 specs section 7.1.1.
> ---
> obexd/client/map.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/obexd/client/map.c b/obexd/client/map.c
> index 44db96c..43b0471 100644
> --- a/obexd/client/map.c
> +++ b/obexd/client/map.c
> @@ -100,6 +100,7 @@ struct map_data {
> GHashTable *messages;
> int16_t mas_instance_id;
> uint8_t supported_message_types;
> + uint32_t supported_features;
> };
>
> struct pending_request {
> @@ -2003,6 +2004,14 @@ static void parse_service_record(struct map_data *map)
> map->supported_message_types = *(uint8_t *)data;
> else
> DBG("Failed to read supported message types");
> +
> + /* Supported Feature Bits */
> + data = obc_session_get_attribute(map->session,
> + SDP_ATTR_MAP_SUPPORTED_FEATURES);
> + if(data != NULL)
> + map->supported_features = *(uint32_t *)data;
> + else
> + map->supported_features = 0x0000001f;
> }
>
> static int map_probe(struct obc_session *session)
> --
> 1.9.1
Applied, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply
* rtk_btusb issues
From: Patrick Shirkey @ 2014-10-22 10:35 UTC (permalink / raw)
To: linux-bluetooth
Hi,
I have an android device running the rtk_btusb driver. The original driver
was a couple of years old and there was a problem with stability with BLE.
In addition the localname was not found in the scan_response data.
I upgraded the bluetooth stack by manually backporting from a recent
kernel. That fixed the issue wit the scan_repsonse data but there were
still stability issues with BLE devices (although slightly improved
compared to the older bluetooth stack).
I have tried upgrading the driver to the latest version of the rtk_btusb
driver from the git repo and ported the hooks for bluedroid from the old
driver however I get issues with hci send command when loading the driver.
http://pastebin.com/hXALmXBr
I traced the command but I am not sure where the send value is generated.
I can see the function definition in the hci_dev struct but not the
location where the value is actually set.
I thought it might be an issue specific to bluedroid so I installed bluez
but the issue is still there so it appears to be a bug in my version of
the driver or something wrong with the bluetooth kernel layer.
I went back to the older version of the driver running against bluez
instead of bluedroid. That gets me further along but I still cannot
initialise the bluetooth system on the device.
http://pastebin.com/HcSZXiMu
Does anyone have any suggestions for how to go about resolving this
situation I have got myself into?
--
Patrick Shirkey
Boost Hardware Ltd
^ permalink raw reply
* [PATCH v3 bluetooth-next 4/4] ieee802154: 6lowpan: rename process_data and lowpan_process_data
From: Martin Townsend @ 2014-10-22 8:39 UTC (permalink / raw)
To: linux-bluetooth, linux-wpan
Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend
In-Reply-To: <1413967190-28890-1-git-send-email-martin.townsend@xsilon.com>
From: Martin Townsend <mtownsend1973@gmail.com>
As we have decouple decompression from data delivery we can now rename all
occurences of process_data in receive path.
Signed-off-by: Martin Townsend <mtownsend1973@gmail.com>
---
include/net/6lowpan.h | 10 ++++++----
net/6lowpan/iphc.c | 12 +++++++-----
net/bluetooth/6lowpan.c | 15 ++++++++-------
net/ieee802154/6lowpan_rtnl.c | 15 ++++++++-------
4 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index abfa359..dc03d77 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -372,10 +372,12 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
return skb->len + uncomp_header - ret;
}
-int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
- const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
- const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
- u8 iphc0, u8 iphc1);
+int
+lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev,
+ const u8 *saddr, const u8 saddr_type,
+ const u8 saddr_len, const u8 *daddr,
+ const u8 daddr_type, const u8 daddr_len,
+ u8 iphc0, u8 iphc1);
int lowpan_header_compress(struct sk_buff *skb, struct net_device *dev,
unsigned short type, const void *_daddr,
const void *_saddr, unsigned int len);
diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 45714fe..73a7065 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -301,10 +301,12 @@ err:
/* TTL uncompression values */
static const u8 lowpan_ttl_values[] = { 0, 1, 64, 255 };
-int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
- const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
- const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
- u8 iphc0, u8 iphc1)
+int
+lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev,
+ const u8 *saddr, const u8 saddr_type,
+ const u8 saddr_len, const u8 *daddr,
+ const u8 daddr_type, const u8 daddr_len,
+ u8 iphc0, u8 iphc1)
{
struct ipv6hdr hdr = {};
u8 tmp, num_context = 0;
@@ -480,7 +482,7 @@ drop:
kfree_skb(skb);
return -EINVAL;
}
-EXPORT_SYMBOL_GPL(lowpan_process_data);
+EXPORT_SYMBOL_GPL(lowpan_header_decompress);
static u8 lowpan_compress_addr_64(u8 **hc_ptr, u8 shift,
const struct in6_addr *ipaddr,
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 40e2cec..aa6ebbf 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -257,8 +257,8 @@ static int give_skb_to_upper(struct sk_buff *skb, struct net_device *dev)
return netif_rx(skb_cp);
}
-static int process_data(struct sk_buff *skb, struct net_device *netdev,
- struct l2cap_chan *chan)
+static int iphc_decompress(struct sk_buff *skb, struct net_device *netdev,
+ struct l2cap_chan *chan)
{
const u8 *saddr, *daddr;
u8 iphc0, iphc1;
@@ -287,10 +287,11 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev,
if (lowpan_fetch_skb_u8(skb, &iphc1))
goto drop;
- return lowpan_process_data(skb, netdev,
- saddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
- daddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
- iphc0, iphc1);
+ return lowpan_header_decompress(skb, netdev,
+ saddr, IEEE802154_ADDR_LONG,
+ EUI64_ADDR_LEN, daddr,
+ IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
+ iphc0, iphc1);
drop:
kfree_skb(skb);
@@ -346,7 +347,7 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
if (!local_skb)
goto drop;
- ret = process_data(local_skb, dev, chan);
+ ret = iphc_decompress(local_skb, dev, chan);
if (ret < 0)
goto drop;
diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 83a0731..e033f13 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -163,7 +163,8 @@ static int lowpan_give_skb_to_devices(struct sk_buff *skb,
return stat;
}
-static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
+static int
+iphc_decompress(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
{
u8 iphc0, iphc1;
struct ieee802154_addr_sa sa, da;
@@ -193,9 +194,9 @@ static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
else
dap = &da.hwaddr;
- return lowpan_process_data(skb, skb->dev, sap, sa.addr_type,
- IEEE802154_ADDR_LEN, dap, da.addr_type,
- IEEE802154_ADDR_LEN, iphc0, iphc1);
+ return lowpan_header_decompress(skb, skb->dev, sap, sa.addr_type,
+ IEEE802154_ADDR_LEN, dap, da.addr_type,
+ IEEE802154_ADDR_LEN, iphc0, iphc1);
drop:
kfree_skb(skb);
@@ -537,14 +538,14 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
} else {
switch (skb->data[0] & 0xe0) {
case LOWPAN_DISPATCH_IPHC: /* ipv6 datagram */
- ret = process_data(skb, &hdr);
+ ret = iphc_decompress(skb, &hdr);
if (ret < 0)
goto drop;
break;
case LOWPAN_DISPATCH_FRAG1: /* first fragment header */
ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
if (ret == 1) {
- ret = process_data(skb, &hdr);
+ ret = iphc_decompress(skb, &hdr);
if (ret < 0)
goto drop;
}
@@ -552,7 +553,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
case LOWPAN_DISPATCH_FRAGN: /* next fragments headers */
ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAGN);
if (ret == 1) {
- ret = process_data(skb, &hdr);
+ ret = iphc_decompress(skb, &hdr);
if (ret < 0)
goto drop;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v3 bluetooth-next 3/4] bluetooth:6lowpan: use consume_skb when packet processed successfully
From: Martin Townsend @ 2014-10-22 8:39 UTC (permalink / raw)
To: linux-bluetooth, linux-wpan
Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend
In-Reply-To: <1413967190-28890-1-git-send-email-martin.townsend@xsilon.com>
From: Martin Townsend <mtownsend1973@gmail.com>
Signed-off-by: Martin Townsend <mtownsend1973@gmail.com>
---
net/bluetooth/6lowpan.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 94bbb66..40e2cec 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -337,8 +337,8 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
dev->stats.rx_bytes += skb->len;
dev->stats.rx_packets++;
- kfree_skb(local_skb);
- kfree_skb(skb);
+ consume_skb(local_skb);
+ consume_skb(skb);
} else {
switch (skb->data[0] & 0xe0) {
case LOWPAN_DISPATCH_IPHC: /* ipv6 datagram */
@@ -363,7 +363,8 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
dev->stats.rx_bytes += skb->len;
dev->stats.rx_packets++;
- kfree_skb(skb);
+ consume_skb(local_skb);
+ consume_skb(skb);
break;
default:
break;
--
1.9.1
^ permalink raw reply related
* [PATCH v3 bluetooth-next 2/4] 6lowpan: fix process_data return values
From: Martin Townsend @ 2014-10-22 8:39 UTC (permalink / raw)
To: linux-bluetooth, linux-wpan
Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend,
Martin Townsend
In-Reply-To: <1413967190-28890-1-git-send-email-martin.townsend@xsilon.com>
As process_data now returns just error codes fix up the calls to this
function to only drop the skb if an error code is returned.
Signed-off-by: Martin Townsend <mtownsend1973@gmail.com>
---
net/bluetooth/6lowpan.c | 2 +-
net/ieee802154/6lowpan_rtnl.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 45d9a9f..94bbb66 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -347,7 +347,7 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
goto drop;
ret = process_data(local_skb, dev, chan);
- if (ret != NET_RX_SUCCESS)
+ if (ret < 0)
goto drop;
local_skb->protocol = htons(ETH_P_IPV6);
diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 1cdb8e4..83a0731 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -538,14 +538,14 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
switch (skb->data[0] & 0xe0) {
case LOWPAN_DISPATCH_IPHC: /* ipv6 datagram */
ret = process_data(skb, &hdr);
- if (ret == NET_RX_DROP)
+ if (ret < 0)
goto drop;
break;
case LOWPAN_DISPATCH_FRAG1: /* first fragment header */
ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
if (ret == 1) {
ret = process_data(skb, &hdr);
- if (ret == NET_RX_DROP)
+ if (ret < 0)
goto drop;
}
break;
@@ -553,7 +553,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAGN);
if (ret == 1) {
ret = process_data(skb, &hdr);
- if (ret == NET_RX_DROP)
+ if (ret < 0)
goto drop;
}
break;
--
1.9.1
^ permalink raw reply related
* [PATCH v3 bluetooth-next 1/4] 6lowpan: remove skb_deliver from IPHC
From: Martin Townsend @ 2014-10-22 8:39 UTC (permalink / raw)
To: linux-bluetooth, linux-wpan
Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend,
Martin Townsend
In-Reply-To: <1413967190-28890-1-git-send-email-martin.townsend@xsilon.com>
Separating skb delivery from decompression ensures that we can support further
decompression schemes and removes the mixed return value of error codes with
NET_RX_FOO.
Signed-off-by: Martin Townsend <mtownsend1973@gmail.com>
---
include/net/6lowpan.h | 4 +---
net/6lowpan/iphc.c | 32 ++++++--------------------------
net/bluetooth/6lowpan.c | 14 ++++++++++++--
net/ieee802154/6lowpan_rtnl.c | 25 +++++++++++++------------
4 files changed, 32 insertions(+), 43 deletions(-)
diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index d184df1..abfa359 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -372,12 +372,10 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
return skb->len + uncomp_header - ret;
}
-typedef int (*skb_delivery_cb)(struct sk_buff *skb, struct net_device *dev);
-
int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
- u8 iphc0, u8 iphc1, skb_delivery_cb skb_deliver);
+ u8 iphc0, u8 iphc1);
int lowpan_header_compress(struct sk_buff *skb, struct net_device *dev,
unsigned short type, const void *_daddr,
const void *_saddr, unsigned int len);
diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 747b3cc..45714fe 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -171,29 +171,6 @@ static int uncompress_context_based_src_addr(struct sk_buff *skb,
return 0;
}
-static int skb_deliver(struct sk_buff *skb, struct ipv6hdr *hdr,
- struct net_device *dev, skb_delivery_cb deliver_skb)
-{
- int stat;
-
- skb_push(skb, sizeof(struct ipv6hdr));
- skb_reset_network_header(skb);
- skb_copy_to_linear_data(skb, hdr, sizeof(struct ipv6hdr));
-
- skb->protocol = htons(ETH_P_IPV6);
- skb->pkt_type = PACKET_HOST;
- skb->dev = dev;
-
- raw_dump_table(__func__, "raw skb data dump before receiving",
- skb->data, skb->len);
-
- stat = deliver_skb(skb, dev);
-
- consume_skb(skb);
-
- return stat;
-}
-
/* Uncompress function for multicast destination address,
* when M bit is set.
*/
@@ -327,7 +304,7 @@ static const u8 lowpan_ttl_values[] = { 0, 1, 64, 255 };
int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
- u8 iphc0, u8 iphc1, skb_delivery_cb deliver_skb)
+ u8 iphc0, u8 iphc1)
{
struct ipv6hdr hdr = {};
u8 tmp, num_context = 0;
@@ -492,10 +469,13 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
hdr.version, ntohs(hdr.payload_len), hdr.nexthdr,
hdr.hop_limit, &hdr.daddr);
- raw_dump_table(__func__, "raw header dump", (u8 *)&hdr, sizeof(hdr));
+ skb_push(skb, sizeof(hdr));
+ skb_reset_network_header(skb);
+ skb_copy_to_linear_data(skb, &hdr, sizeof(hdr));
- return skb_deliver(skb, &hdr, dev, deliver_skb);
+ raw_dump_table(__func__, "raw header dump", (u8 *)&hdr, sizeof(hdr));
+ return 0;
drop:
kfree_skb(skb);
return -EINVAL;
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 6c5c2ef..45d9a9f 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -252,7 +252,7 @@ static int give_skb_to_upper(struct sk_buff *skb, struct net_device *dev)
skb_cp = skb_copy(skb, GFP_ATOMIC);
if (!skb_cp)
- return -ENOMEM;
+ return NET_RX_DROP;
return netif_rx(skb_cp);
}
@@ -290,7 +290,7 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev,
return lowpan_process_data(skb, netdev,
saddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
daddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
- iphc0, iphc1, give_skb_to_upper);
+ iphc0, iphc1);
drop:
kfree_skb(skb);
@@ -350,6 +350,16 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
if (ret != NET_RX_SUCCESS)
goto drop;
+ local_skb->protocol = htons(ETH_P_IPV6);
+ local_skb->pkt_type = PACKET_HOST;
+ local_skb->dev = dev;
+
+ if (give_skb_to_upper(local_skb, dev)
+ != NET_RX_SUCCESS) {
+ kfree_skb(local_skb);
+ goto drop;
+ }
+
dev->stats.rx_bytes += skb->len;
dev->stats.rx_packets++;
diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 0c1a49b..1cdb8e4 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -146,15 +146,20 @@ static int lowpan_give_skb_to_devices(struct sk_buff *skb,
if (lowpan_dev_info(entry->ldev)->real_dev == skb->dev) {
skb_cp = skb_copy(skb, GFP_ATOMIC);
if (!skb_cp) {
- stat = -ENOMEM;
- break;
+ kfree_skb(skb);
+ rcu_read_unlock();
+ return NET_RX_DROP;
}
skb_cp->dev = entry->ldev;
stat = netif_rx(skb_cp);
+ if (stat == NET_RX_DROP)
+ break;
}
rcu_read_unlock();
+ consume_skb(skb);
+
return stat;
}
@@ -190,8 +195,7 @@ static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
return lowpan_process_data(skb, skb->dev, sap, sa.addr_type,
IEEE802154_ADDR_LEN, dap, da.addr_type,
- IEEE802154_ADDR_LEN, iphc0, iphc1,
- lowpan_give_skb_to_devices);
+ IEEE802154_ADDR_LEN, iphc0, iphc1);
drop:
kfree_skb(skb);
@@ -528,15 +532,8 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
/* check that it's our buffer */
if (skb->data[0] == LOWPAN_DISPATCH_IPV6) {
- skb->protocol = htons(ETH_P_IPV6);
- skb->pkt_type = PACKET_HOST;
-
/* Pull off the 1-byte of 6lowpan header. */
skb_pull(skb, 1);
-
- ret = lowpan_give_skb_to_devices(skb, NULL);
- if (ret == NET_RX_DROP)
- goto drop;
} else {
switch (skb->data[0] & 0xe0) {
case LOWPAN_DISPATCH_IPHC: /* ipv6 datagram */
@@ -565,7 +562,11 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
}
}
- return NET_RX_SUCCESS;
+ /* Pass IPv6 packet up to the next layer */
+ skb->protocol = htons(ETH_P_IPV6);
+ skb->pkt_type = PACKET_HOST;
+ return lowpan_give_skb_to_devices(skb, NULL);
+
drop_skb:
kfree_skb(skb);
drop:
--
1.9.1
^ permalink raw reply related
* [PATCH v3 bluetooth-next 0/4] 6lowpan: Move skb delivery out of IPHC
From: Martin Townsend @ 2014-10-22 8:39 UTC (permalink / raw)
To: linux-bluetooth, linux-wpan
Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend
This series moves skb delivery out of IPHC and into the receive routines of
both bluetooth and 802.15.4. The reason is that we need to support more
(de)compression schemes in the future. It also means that calling
lowpan_process_data now only returns error codes or 0 for success so
this has been cleaned up. The final patch then renames occurences of
lowpan_process_data and process_data to something more meaningful.
v1 -> v2
* Handle freeing of skb in lowpan_give_skb_to_devices for 802.15.14
* Added another skb_consume in bluetooth code for uncompressed IPv6 headers
* Added missing skb_consume for local_skb for bluetooth in compreesed IPv6 header
path
* Combine patch 4 into 1.
v2 -> v3
* Ensure error codes aren't returned in the bluetooth skb delivery function.
* In lowpan_give_skb_to_devices return NET_RX_DROP when the first call to
netif_rx fails.
Martin Townsend (4):
6lowpan: remove skb_deliver from IPHC
6lowpan: fix process_data return values
bluetooth:6lowpan: use consume_skb when packet processed successfully
ieee802154: 6lowpan: rename process_data and lowpan_process_data
include/net/6lowpan.h | 12 ++++++------
net/6lowpan/iphc.c | 42 ++++++++++++-----------------------------
net/bluetooth/6lowpan.c | 36 +++++++++++++++++++++++------------
net/ieee802154/6lowpan_rtnl.c | 44 ++++++++++++++++++++++---------------------
4 files changed, 65 insertions(+), 69 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH] android/pts: Updated AVCTP PICS and PIXITs for PTS 5.3
From: Szymon Janc @ 2014-10-22 7:55 UTC (permalink / raw)
To: Ruslan Mstoi; +Cc: linux-bluetooth
In-Reply-To: <1413961860-11390-1-git-send-email-ruslan.mstoi@linux.intel.com>
Hi Ruslan,
On Wednesday 22 of October 2014 10:11:00 Ruslan Mstoi wrote:
> ---
> android/pics-avctp.txt | 2 +-
> android/pixit-avctp.txt | 6 +++---
> android/pts-avctp.txt | 4 ++--
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/android/pics-avctp.txt b/android/pics-avctp.txt
> index 5900d00..0afa33c 100644
> --- a/android/pics-avctp.txt
> +++ b/android/pics-avctp.txt
> @@ -1,6 +1,6 @@
> AVCTP PICS for the PTS tool.
>
> -PTS version: 5.2
> +PTS version: 5.3
>
> * - different than PTS defaults
> # - not yet implemented/supported
> diff --git a/android/pixit-avctp.txt b/android/pixit-avctp.txt
> index 3c6a864..06a0798 100644
> --- a/android/pixit-avctp.txt
> +++ b/android/pixit-avctp.txt
> @@ -1,6 +1,6 @@
> AVCTP PIXIT for the PTS tool.
>
> -PTS version: 5.2
> +PTS version: 5.3
>
> * - different than PTS defaults
> & - should be set to IUT Bluetooth address
> @@ -12,7 +12,7 @@ Parameter Name Value
> -------------------------------------------------------------------------------
> TSPX_avctp_psm 0017
> TSPX_avctp_profile_id 110E
> -TSPX_connect_avdtp FALSE (*)
> +TSPX_connect_avdtp TRUE
> TSPX_avctp_tester_command_data
> TSPX_avctp_tester_response_data
> TSPX_avctp_iut_command_data
> @@ -25,7 +25,7 @@ TSPX_class_of_device 20050C
> TSPX_player_feature_bitmask 0000000000000007FFF00070000000000
> TSPX_avrcp_version
> TSPX_establish_avdtp_stream TRUE
> -TSPX_tester_av_role
> +TSPX_tester_av_role SNK
> TSPX_time_guard 300000
> TSPX_avrcp_only FALSE
> TSPX_use_implicit_send TRUE
> diff --git a/android/pts-avctp.txt b/android/pts-avctp.txt
> index a461ee9..fd22ee1 100644
> --- a/android/pts-avctp.txt
> +++ b/android/pts-avctp.txt
> @@ -1,7 +1,7 @@
> PTS test results for AVCTP
>
> -PTS version: 5.2
> -Tested: 10-July-2014
> +PTS version: 5.3
> +Tested: 22-October-2014
> Android version: 4.4.4
>
> Results:
>
Applied, thanks.
--
Best regards,
Szymon Janc
^ 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