Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH_v2 6/6] bnep: Refactor bnep setup response validation functionality
From: Ravi kumar Veeramally @ 2013-12-16 22:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1387231516-4127-1-git-send-email-ravikumar.veeramally@linux.intel.com>

Providing single api to validate bnep setup resp and hide
other functions.
---
 profiles/network/bnep.c   | 49 ++++++++++++++++++++++++++++++++++++++++++-----
 profiles/network/bnep.h   |  4 +---
 profiles/network/server.c | 46 +++++++++-----------------------------------
 3 files changed, 54 insertions(+), 45 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 5deac81..6e1af74 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -535,19 +535,19 @@ ssize_t bnep_send_ctrl_rsp(int sk, uint8_t type, uint8_t ctrl, uint16_t resp)
 	return send(sk, &rsp, sizeof(rsp), 0);
 }
 
-uint16_t bnep_setup_chk(uint16_t dst, uint16_t src)
+static uint16_t bnep_setup_chk(uint16_t dst, uint16_t src)
 {
 	/* Allowed PAN Profile scenarios */
 	switch (dst) {
 	case BNEP_SVC_NAP:
 	case BNEP_SVC_GN:
 		if (src == BNEP_SVC_PANU)
-			return 0;
+			return BNEP_SUCCESS;
 		return BNEP_CONN_INVALID_SRC;
 	case BNEP_SVC_PANU:
 		if (src == BNEP_SVC_PANU ||  src == BNEP_SVC_GN ||
 							src == BNEP_SVC_NAP)
-			return 0;
+			return BNEP_SUCCESS;
 
 		return BNEP_CONN_INVALID_SRC;
 	}
@@ -555,8 +555,8 @@ uint16_t bnep_setup_chk(uint16_t dst, uint16_t src)
 	return BNEP_CONN_INVALID_DST;
 }
 
-uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
-								uint16_t *src)
+static uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req,
+						uint16_t *dst, uint16_t *src)
 {
 	const uint8_t bt_base[] = { 0x00, 0x00, 0x10, 0x00, 0x80, 0x00,
 					0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB };
@@ -602,6 +602,45 @@ uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
 	return BNEP_SUCCESS;
 }
 
+int bnep_validate_setup_rsp(int sk, uint16_t *dst)
+{
+	uint8_t packet[BNEP_MTU];
+	struct bnep_setup_conn_req *req = (void *) packet;
+	uint16_t src;
+	uint8_t pkt[3];
+	int n, rsp = BNEP_CONN_NOT_ALLOWED;
+
+	/* Reading BNEP_SETUP_CONNECTION_REQUEST_MSG */
+	n = read(sk, packet, sizeof(packet));
+	if (n < 0) {
+		error("read(): %s(%d)", strerror(errno), errno);
+		return n;
+	}
+
+	/* Highest known Control command ID
+	 * is BNEP_FILTER_MULT_ADDR_RSP = 0x06 */
+	if (req->type == BNEP_CONTROL &&
+				req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
+		pkt[0] = BNEP_CONTROL;
+		pkt[1] = BNEP_CMD_NOT_UNDERSTOOD;
+		pkt[2] = req->ctrl;
+
+		send(sk, pkt, sizeof(pkt), 0);
+		return -EINVAL;
+	}
+
+	if (req->type != BNEP_CONTROL || req->ctrl != BNEP_SETUP_CONN_REQ)
+		return -EINVAL;
+
+	rsp = bnep_setup_decode(req, dst, &src);
+	if (rsp)
+		return rsp;
+
+	rsp = bnep_setup_chk(*dst, src);
+
+	return rsp;
+}
+
 int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
 						const bdaddr_t *addr)
 {
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index 734055d..36cfee0 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -43,7 +43,5 @@ int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
 							const bdaddr_t *addr);
 void bnep_server_delete(char *bridge, char *iface, const bdaddr_t *addr);
 
+int bnep_validate_setup_rsp(int sk, uint16_t *dst);
 ssize_t bnep_send_ctrl_rsp(int sk, uint8_t type, uint8_t ctrl, uint16_t resp);
-uint16_t bnep_setup_chk(uint16_t dst_role, uint16_t src_role);
-uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
-								uint16_t *src);
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 7cb5a1e..432b6d5 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -282,10 +282,8 @@ static gboolean bnep_setup(GIOChannel *chan,
 {
 	struct network_adapter *na = user_data;
 	struct network_server *ns;
-	uint8_t packet[BNEP_MTU];
-	struct bnep_setup_conn_req *req = (void *) packet;
-	uint16_t src_role, dst_role, rsp = BNEP_CONN_NOT_ALLOWED;
-	int n, sk;
+	uint16_t dst;
+	int sk, rsp = BNEP_CONN_NOT_ALLOWED;
 
 	if (cond & G_IO_NVAL)
 		return FALSE;
@@ -296,45 +294,18 @@ static gboolean bnep_setup(GIOChannel *chan,
 	}
 
 	sk = g_io_channel_unix_get_fd(chan);
-
-	/* Reading BNEP_SETUP_CONNECTION_REQUEST_MSG */
-	n = read(sk, packet, sizeof(packet));
-	if (n < 0) {
-		error("read(): %s(%d)", strerror(errno), errno);
-		return FALSE;
-	}
-
-	/* Highest known Control command ID
-	 * is BNEP_FILTER_MULT_ADDR_RSP = 0x06 */
-	if (req->type == BNEP_CONTROL &&
-				req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
-		uint8_t pkt[3];
-
-		pkt[0] = BNEP_CONTROL;
-		pkt[1] = BNEP_CMD_NOT_UNDERSTOOD;
-		pkt[2] = req->ctrl;
-
-		send(sk, pkt, sizeof(pkt), 0);
-
-		return FALSE;
-	}
-
-	if (req->type != BNEP_CONTROL || req->ctrl != BNEP_SETUP_CONN_REQ)
+	rsp = bnep_validate_setup_rsp(sk, &dst);
+	if (rsp < 0)
 		return FALSE;
 
-	rsp = bnep_setup_decode(req, &dst_role, &src_role);
-	if (rsp)
-		goto reply;
-
-	rsp = bnep_setup_chk(dst_role, src_role);
-	if (rsp)
+	if (rsp > 0)
 		goto reply;
 
 	rsp = BNEP_CONN_NOT_ALLOWED;
 
-	ns = find_server(na->servers, dst_role);
+	ns = find_server(na->servers, dst);
 	if (!ns) {
-		error("Server unavailable: (0x%x)", dst_role);
+		error("Server unavailable: (0x%x)", dst);
 		goto reply;
 	}
 
@@ -348,10 +319,11 @@ static gboolean bnep_setup(GIOChannel *chan,
 		goto reply;
 	}
 
-	if (bnep_server_add(sk, dst_role, ns->bridge, na->setup->dev,
+	if (bnep_server_add(sk, dst, ns->bridge, na->setup->dev,
 							&na->setup->dst) < 0)
 		goto reply;
 
+	ns->sessions = g_slist_append(ns->sessions, na->setup);
 	na->setup = NULL;
 
 	rsp = BNEP_SUCCESS;
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH 2/2] android/tester: Enable bthost after device is enabled
From: Johan Hedberg @ 2013-12-17  7:50 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1387205568-14461-2-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Mon, Dec 16, 2013, Andrei Emeltchenko wrote:
> +static void client_connectable_complete(uint16_t opcode, uint8_t status,
> +					const void *param, uint8_t len,
> +					void *user_data)
> +{
> +	switch (opcode) {
> +	case BT_HCI_CMD_WRITE_SCAN_ENABLE:
> +	case BT_HCI_CMD_LE_SET_ADV_ENABLE:
> +		break;
> +	default:
> +		return;
> +	}
> +
> +	tester_print("Client set connectable status 0x%02x", status);
> +
> +	if (status)
> +		tester_setup_failed();
> +	else
> +		tester_setup_complete();
> +}
> +
> +static void setup_powered_client(void)
> +{
> +	struct test_data *data = tester_get_data();
> +	struct bthost *bthost;
> +
> +	tester_print("Controller powered on");
> +
> +	bthost = hciemu_client_get_host(data->hciemu);
> +	bthost_set_cmd_complete_cb(bthost, client_connectable_complete, data);
> +
> +	if (data->hciemu_type == HCIEMU_TYPE_LE)
> +		bthost_set_adv_enable(bthost, 0x01);
> +	else
> +		bthost_write_scan_enable(bthost, 0x03);
> +}
> +
>  static void adapter_state_changed_cb(bt_state_t state)
>  {
>  	enum hal_bluetooth_callbacks_id hal_cb;
> @@ -484,7 +525,7 @@ static void adapter_state_changed_cb(bt_state_t state)
>  		break;
>  	case adapter_test_setup_mode:
>  		if (state == BT_STATE_ON)
> -			tester_setup_complete();
> +			setup_powered_client();
>  		else
>  			tester_setup_failed();
>  		break;

You seem to have copied this from l2cap-tester without understanding why
the naming of these functions are as they are. The "client" in the names
are for testing client sockets (as opposed to server ones). However,
you're calling these for every test case, including the server tests
(the ones with _listen in them), which doesn't seem right. If you're the
server you don't need to make the bthost side connectable.

Johan

^ permalink raw reply

* Re: [PATCH 1/2] android/tester: Add missing break
From: Johan Hedberg @ 2013-12-17  7:51 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1387205568-14461-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Mon, Dec 16, 2013, Andrei Emeltchenko wrote:
> ---
>  android/android-tester.c | 1 +
>  1 file changed, 1 insertion(+)

This patch has been applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH 2/2] android/tester: Enable bthost after device is enabled
From: Andrei Emeltchenko @ 2013-12-17  8:04 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <20131217075057.GA19701@x220.p-661hnu-f1>

Hi Johan,

On Tue, Dec 17, 2013 at 09:50:57AM +0200, Johan Hedberg wrote:
> Hi Andrei,
> 
> On Mon, Dec 16, 2013, Andrei Emeltchenko wrote:
> > +static void client_connectable_complete(uint16_t opcode, uint8_t status,
> > +					const void *param, uint8_t len,
> > +					void *user_data)
> > +{
> > +	switch (opcode) {
> > +	case BT_HCI_CMD_WRITE_SCAN_ENABLE:
> > +	case BT_HCI_CMD_LE_SET_ADV_ENABLE:
> > +		break;
> > +	default:
> > +		return;
> > +	}
> > +
> > +	tester_print("Client set connectable status 0x%02x", status);
> > +
> > +	if (status)
> > +		tester_setup_failed();
> > +	else
> > +		tester_setup_complete();
> > +}
> > +
> > +static void setup_powered_client(void)
> > +{
> > +	struct test_data *data = tester_get_data();
> > +	struct bthost *bthost;
> > +
> > +	tester_print("Controller powered on");
> > +
> > +	bthost = hciemu_client_get_host(data->hciemu);
> > +	bthost_set_cmd_complete_cb(bthost, client_connectable_complete, data);
> > +
> > +	if (data->hciemu_type == HCIEMU_TYPE_LE)
> > +		bthost_set_adv_enable(bthost, 0x01);
> > +	else
> > +		bthost_write_scan_enable(bthost, 0x03);
> > +}
> > +
> >  static void adapter_state_changed_cb(bt_state_t state)
> >  {
> >  	enum hal_bluetooth_callbacks_id hal_cb;
> > @@ -484,7 +525,7 @@ static void adapter_state_changed_cb(bt_state_t state)
> >  		break;
> >  	case adapter_test_setup_mode:
> >  		if (state == BT_STATE_ON)
> > -			tester_setup_complete();
> > +			setup_powered_client();
> >  		else
> >  			tester_setup_failed();
> >  		break;
> 
> You seem to have copied this from l2cap-tester without understanding why
> the naming of these functions are as they are. The "client" in the names
> are for testing client sockets (as opposed to server ones). However,
> you're calling these for every test case, including the server tests
> (the ones with _listen in them), which doesn't seem right. If you're the
> server you don't need to make the bthost side connectable.

So what is appropriate way of setting emulated remote device connectible?
This actually does not hurt in every case.

Best regards 
Andrei Emeltchenko 

^ permalink raw reply

* Re: [PATCH 2/2] android/tester: Enable bthost after device is enabled
From: Johan Hedberg @ 2013-12-17  8:20 UTC (permalink / raw)
  To: Andrei Emeltchenko, linux-bluetooth
In-Reply-To: <20131217080214.GA3209@aemeltch-MOBL1>

Hi Andrei,

On Tue, Dec 17, 2013, Andrei Emeltchenko wrote:
> On Tue, Dec 17, 2013 at 09:50:57AM +0200, Johan Hedberg wrote:
> > Hi Andrei,
> > 
> > On Mon, Dec 16, 2013, Andrei Emeltchenko wrote:
> > > +static void client_connectable_complete(uint16_t opcode, uint8_t status,
> > > +					const void *param, uint8_t len,
> > > +					void *user_data)
> > > +{
> > > +	switch (opcode) {
> > > +	case BT_HCI_CMD_WRITE_SCAN_ENABLE:
> > > +	case BT_HCI_CMD_LE_SET_ADV_ENABLE:
> > > +		break;
> > > +	default:
> > > +		return;
> > > +	}
> > > +
> > > +	tester_print("Client set connectable status 0x%02x", status);
> > > +
> > > +	if (status)
> > > +		tester_setup_failed();
> > > +	else
> > > +		tester_setup_complete();
> > > +}
> > > +
> > > +static void setup_powered_client(void)
> > > +{
> > > +	struct test_data *data = tester_get_data();
> > > +	struct bthost *bthost;
> > > +
> > > +	tester_print("Controller powered on");
> > > +
> > > +	bthost = hciemu_client_get_host(data->hciemu);
> > > +	bthost_set_cmd_complete_cb(bthost, client_connectable_complete, data);
> > > +
> > > +	if (data->hciemu_type == HCIEMU_TYPE_LE)
> > > +		bthost_set_adv_enable(bthost, 0x01);
> > > +	else
> > > +		bthost_write_scan_enable(bthost, 0x03);
> > > +}
> > > +
> > >  static void adapter_state_changed_cb(bt_state_t state)
> > >  {
> > >  	enum hal_bluetooth_callbacks_id hal_cb;
> > > @@ -484,7 +525,7 @@ static void adapter_state_changed_cb(bt_state_t state)
> > >  		break;
> > >  	case adapter_test_setup_mode:
> > >  		if (state == BT_STATE_ON)
> > > -			tester_setup_complete();
> > > +			setup_powered_client();
> > >  		else
> > >  			tester_setup_failed();
> > >  		break;
> > 
> > You seem to have copied this from l2cap-tester without understanding why
> > the naming of these functions are as they are. The "client" in the names
> > are for testing client sockets (as opposed to server ones). However,
> > you're calling these for every test case, including the server tests
> > (the ones with _listen in them), which doesn't seem right. If you're the
> > server you don't need to make the bthost side connectable.
> 
> So what is appropriate way of setting emulated remote device connectible?
> This actually does not hurt in every case.

The you shouldn't at least have "client" in the naming since this is in
no way client-side specific.

Johan

^ permalink raw reply

* Re: [PATCH_v2 1/6] android/pan: Rename pan_device_free to destroy_pan_device
From: Luiz Augusto von Dentz @ 2013-12-17  9:09 UTC (permalink / raw)
  To: Ravi kumar Veeramally; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1387231516-4127-1-git-send-email-ravikumar.veeramally@linux.intel.com>

Hi Ravi,

On Tue, Dec 17, 2013 at 12:05 AM, Ravi kumar Veeramally
<ravikumar.veeramally@linux.intel.com> wrote:
> Renaming function name because it does more than freeing memory.
> Also moving disconnect notification call to destory_pan_device
> reduce redundancy.
> ---
>  android/pan.c | 51 +++++++++++++++++++++++----------------------------
>  1 file changed, 23 insertions(+), 28 deletions(-)
>
> diff --git a/android/pan.c b/android/pan.c
> index e410f54..ec589cf 100644
> --- a/android/pan.c
> +++ b/android/pan.c
> @@ -65,24 +65,6 @@ static int device_cmp(gconstpointer s, gconstpointer user_data)
>         return bacmp(&dev->dst, dst);
>  }
>
> -static void pan_device_free(struct pan_device *dev)
> -{
> -       local_role = HAL_PAN_ROLE_NONE;
> -
> -       if (dev->watch > 0) {
> -               g_source_remove(dev->watch);
> -               dev->watch = 0;
> -       }
> -
> -       if (dev->io) {
> -               g_io_channel_unref(dev->io);
> -               dev->io = NULL;
> -       }
> -
> -       devices = g_slist_remove(devices, dev);
> -       g_free(dev);
> -}
> -
>  static void bt_pan_notify_conn_state(struct pan_device *dev, uint8_t state)
>  {
>         struct hal_ev_pan_conn_state ev;
> @@ -121,6 +103,25 @@ static void bt_pan_notify_ctrl_state(struct pan_device *dev, uint8_t state)
>                                                                         &ev);
>  }
>
> +static void destroy_pan_device(struct pan_device *dev)
> +{
> +       local_role = HAL_PAN_ROLE_NONE;
> +
> +       if (dev->watch > 0) {
> +               g_source_remove(dev->watch);
> +               dev->watch = 0;
> +       }
> +
> +       if (dev->io) {
> +               g_io_channel_unref(dev->io);
> +               dev->io = NULL;
> +       }
> +
> +       bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
> +       devices = g_slist_remove(devices, dev);
> +       g_free(dev);
> +}
> +
>  static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
>                                                                 gpointer data)
>  {
> @@ -130,8 +131,7 @@ static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
>
>         bnep_if_down(dev->iface);
>         bnep_conndel(&dev->dst);
> -       bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
> -       pan_device_free(dev);
> +       destroy_pan_device(dev);
>
>         return FALSE;
>  }
> @@ -145,8 +145,7 @@ static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
>         if (err < 0) {
>                 error("bnep connect req failed: %s", strerror(-err));
>                 bnep_conndel(&dev->dst);
> -               bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
> -               pan_device_free(dev);
> +               destroy_pan_device(dev);
>                 return;
>         }
>
> @@ -189,8 +188,7 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
>         return;
>
>  fail:
> -       bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
> -       pan_device_free(dev);
> +       destroy_pan_device(dev);
>  }
>
>  static void bt_pan_connect(const void *buf, uint16_t len)
> @@ -292,10 +290,7 @@ static void bt_pan_disconnect(const void *buf, uint16_t len)
>
>         bnep_if_down(dev->iface);
>         bnep_conndel(&dst);
> -
> -       bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
> -       pan_device_free(dev);
> -
> +       destroy_pan_device(dev);
>         status = HAL_STATUS_SUCCESS;
>
>  failed:
> --
> 1.8.3.2

Have a look at android/a2dp.c, after sending disconnected you can
probably free the device since there is no use for it while
disconnected.



-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH_v2 3/6] bnep: Add bnep_new and bnep_free api's
From: Luiz Augusto von Dentz @ 2013-12-17  9:19 UTC (permalink / raw)
  To: Ravi kumar Veeramally; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1387231516-4127-3-git-send-email-ravikumar.veeramally@linux.intel.com>

Hi Ravi,

On Tue, Dec 17, 2013 at 12:05 AM, Ravi kumar Veeramally
<ravikumar.veeramally@linux.intel.com> wrote:
> Refacoring connect and disconnect mechanisms. It would be more
> convinient for caller to maintain just bnep connection reference
> and delete whenever it is not required.
> ---
>  profiles/network/bnep.c | 37 +++++++++++++++++++++++++++++++++++++
>  profiles/network/bnep.h |  5 +++++
>  2 files changed, 42 insertions(+)
>
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index 08037e6..d7d8832 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -71,6 +71,7 @@ struct bnep_conn {
>         GIOChannel      *io;
>         uint16_t        src;
>         uint16_t        dst;
> +       bdaddr_t        dst_addr;
>         guint   attempts;
>         guint   setup_to;
>         void    *data;
> @@ -246,6 +247,42 @@ int bnep_if_down(const char *devname)
>         return 0;
>  }
>
> +struct bnep_conn *bnep_new(uint16_t src, uint16_t dst,
> +                                               const bdaddr_t *dst_addr)

I would change the order of the parameters, also do not use bdaddr_t
otherwise you wont be able to do unit tests with it, something like
bnep_new(int fd, uint16_t local_role, uint16_t remote_role) looks
better, but perhaps you gonna need the MTU as well.

> +{
> +       struct bnep_conn *bc;
> +
> +       DBG("");
> +
> +       if (!dst_addr)
> +               return NULL;
> +
> +       bc = g_new0(struct bnep_conn, 1);
> +       if (!bc)
> +               return NULL;

No need to check the return of g_new0, if it fails it will exit so
this code will never be triggered.

^ permalink raw reply

* Re: [PATCH_v2 2/6] android/pan: Rename connect_cb to bt_io_connect_cb
From: Johan Hedberg @ 2013-12-17  9:27 UTC (permalink / raw)
  To: Ravi kumar Veeramally; +Cc: linux-bluetooth
In-Reply-To: <1387231516-4127-2-git-send-email-ravikumar.veeramally@linux.intel.com>

Hi Ravi,

On Tue, Dec 17, 2013, Ravi kumar Veeramally wrote:
> Renaming for easy readability.
> ---
>  android/pan.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/android/pan.c b/android/pan.c
> index ec589cf..03db350 100644
> --- a/android/pan.c
> +++ b/android/pan.c
> @@ -162,7 +162,7 @@ static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
>  	dev->io = NULL;
>  }
>  
> -static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
> +static void bt_io_connect_cb(GIOChannel *chan, GError *err, gpointer data)
>  {
>  	struct pan_device *dev = data;
>  	uint16_t src, dst;
> @@ -238,7 +238,7 @@ static void bt_pan_connect(const void *buf, uint16_t len)
>  	ba2str(&dev->dst, addr);
>  	DBG("connecting to %s %s", addr, dev->iface);
>  
> -	dev->io = bt_io_connect(connect_cb, dev, NULL, &gerr,
> +	dev->io = bt_io_connect(bt_io_connect_cb, dev, NULL, &gerr,
>  					BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
>  					BT_IO_OPT_DEST_BDADDR, &dev->dst,
>  					BT_IO_OPT_PSM, BNEP_PSM,

This might actually cause more confusion since the assumption is that
bt_io_* prefixed symbols are part of the btio code. The current naming doesn't
bother me, but if you wanna suggest something else like pan_connected
I'm fine with that too).

Johan

^ permalink raw reply

* RE: [PATCH] Add PICS, PIXITs and PTS test results for L2CAP
From: Chlad, SebastianX @ 2013-12-17  9:27 UTC (permalink / raw)
  To: Johan Hedberg, Sebastian Chlad; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <20131216081736.GA8323@x220.p-661hnu-f1>

Hi Johan,

[ '#' notation ]
True and I can simply change it however there was (is) one thing which prevented me from doing this in the first place: having set any PICS which is not in place however it will be eventually implemented to False and marking it (#), gives clear indication what are the development objectives and at the same time some not relevant PTS tests are simply N/A. That is of benefit to anyone who wishes to test L2CAP in current state. 

[testing tools]
Good point. I was thinking about having in place some more tools besides l2test. I was considering i.e. hcitool or bluetoothctl however it wasn't straight forward to bring these to Android.mk (i.e. readline).
Anyhow having l2test (in Android) was straight forward so I'm planning to do the second round of L2CAP testing. 

In essence my idea was to do testing (L2CAP or anything for the matter) in repetitive way so surely first txt could lack some elements however with next iterations it would improve (i.e. we would have more and more test tools and scripts in place on Android so we could better describe and understand needs for certain set of PTS test). 

To summarize: please let me know what is you stand on [ '#' notation ] - I'll act accordingly and then I'll push L2CAP PTS test results once I finish second round of testing.

Thanks,
Seb

Hi Sebastian,

On Sun, Dec 15, 2013, Sebastian Chlad wrote:
> +* - different than PTS defaults
> +# - not yet implemented/supported
> +
> +M - mandatory
> +O - optional
> +
> +             Roles
> +-------------------------------------------------------------------------------
> +Parameter Name       Selected        Description
> +-------------------------------------------------------------------------------
> +TSPC_L2CAP_1_1       True            Data Channel Initiator (C.1)
> +TSPC_L2CAP_1_2       True            Data Channel Acceptor (C.1)
> +TSPC_L2CAP_1_3       False (#)       LE Master (C.2)
> +TSPC_L2CAP_1_4       False (#)       LE Slave (C.2)

I thought the idea of the '#' notation was that we could mark entries as
True if they eventually will be supported but are not yet implemented.
Wouldn't these last two entries fall under this category. 

> +-------------------------------------------------------------------------------
> +Test Name              Result  Notes
> +-------------------------------------------------------------------------------
> +TC_COS_CED_BV_01_C     INC     IUT should introduce testing tools
> +TC_COS_CED_BV_03_C     INC     IUT should introduce testing tools

What does the note here mean. l2test? If so wouldn't it be more helpful
to list the exact l2test command that's needs to be run?

Johan
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply

* Re: [PATCH_v2 2/6] android/pan: Rename connect_cb to bt_io_connect_cb
From: Ravi kumar Veeramally @ 2013-12-17  9:38 UTC (permalink / raw)
  To: linux-bluetooth, johan.hedberg
In-Reply-To: <20131217092750.GA26363@x220.p-661hnu-f1>

Hi Johan,

On 17.12.2013 11:27, Johan Hedberg wrote:
> Hi Ravi,
>
> On Tue, Dec 17, 2013, Ravi kumar Veeramally wrote:
>> Renaming for easy readability.
>> ---
>>   android/pan.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/android/pan.c b/android/pan.c
>> index ec589cf..03db350 100644
>> --- a/android/pan.c
>> +++ b/android/pan.c
>> @@ -162,7 +162,7 @@ static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
>>   	dev->io = NULL;
>>   }
>>   
>> -static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
>> +static void bt_io_connect_cb(GIOChannel *chan, GError *err, gpointer data)
>>   {
>>   	struct pan_device *dev = data;
>>   	uint16_t src, dst;
>> @@ -238,7 +238,7 @@ static void bt_pan_connect(const void *buf, uint16_t len)
>>   	ba2str(&dev->dst, addr);
>>   	DBG("connecting to %s %s", addr, dev->iface);
>>   
>> -	dev->io = bt_io_connect(connect_cb, dev, NULL, &gerr,
>> +	dev->io = bt_io_connect(bt_io_connect_cb, dev, NULL, &gerr,
>>   					BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
>>   					BT_IO_OPT_DEST_BDADDR, &dev->dst,
>>   					BT_IO_OPT_PSM, BNEP_PSM,
> This might actually cause more confusion since the assumption is that
> bt_io_* prefixed symbols are part of the btio code. The current naming doesn't
> bother me, but if you wanna suggest something else like pan_connected
> I'm fine with that too).
  Ok, ignore this patch, let the same cb name continue.

Ravi.

^ permalink raw reply

* Re: [PATCH_v2 3/6] bnep: Add bnep_new and bnep_free api's
From: Ravi kumar Veeramally @ 2013-12-17  9:42 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZKMECuAd2WBQ8-FE5OO9Qtp0g=iKTBA3zV8JOa6cxAN9w@mail.gmail.com>

Hi Luiz,

On 17.12.2013 11:19, Luiz Augusto von Dentz wrote:
> Hi Ravi,
>
> On Tue, Dec 17, 2013 at 12:05 AM, Ravi kumar Veeramally
> <ravikumar.veeramally@linux.intel.com> wrote:
>> Refacoring connect and disconnect mechanisms. It would be more
>> convinient for caller to maintain just bnep connection reference
>> and delete whenever it is not required.
>> ---
>>   profiles/network/bnep.c | 37 +++++++++++++++++++++++++++++++++++++
>>   profiles/network/bnep.h |  5 +++++
>>   2 files changed, 42 insertions(+)
>>
>> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
>> index 08037e6..d7d8832 100644
>> --- a/profiles/network/bnep.c
>> +++ b/profiles/network/bnep.c
>> @@ -71,6 +71,7 @@ struct bnep_conn {
>>          GIOChannel      *io;
>>          uint16_t        src;
>>          uint16_t        dst;
>> +       bdaddr_t        dst_addr;
>>          guint   attempts;
>>          guint   setup_to;
>>          void    *data;
>> @@ -246,6 +247,42 @@ int bnep_if_down(const char *devname)
>>          return 0;
>>   }
>>
>> +struct bnep_conn *bnep_new(uint16_t src, uint16_t dst,
>> +                                               const bdaddr_t *dst_addr)
> I would change the order of the parameters, also do not use bdaddr_t
> otherwise you wont be able to do unit tests with it, something like
> bnep_new(int fd, uint16_t local_role, uint16_t remote_role) looks
> better, but perhaps you gonna need the MTU as well.
   bdaddr_t is required incase if it unable to up the interface, connection
  needs to be deleted and it is difficult to track on which error we have to
  delete the connection. And one more thing I am moving many of these
  apis to local to the bnep.c.
  Regarding MTU at least now it is not required by connection.c or 
android/pan.c.
>> +{
>> +       struct bnep_conn *bc;
>> +
>> +       DBG("");
>> +
>> +       if (!dst_addr)
>> +               return NULL;
>> +
>> +       bc = g_new0(struct bnep_conn, 1);
>> +       if (!bc)
>> +               return NULL;
> No need to check the return of g_new0, if it fails it will exit so
> this code will never be triggered.
Ok.

Thanks,
Ravi.

^ permalink raw reply

* Re: [PATCH] Add PICS, PIXITs and PTS test results for L2CAP
From: Johan Hedberg @ 2013-12-17 10:29 UTC (permalink / raw)
  To: Chlad, SebastianX; +Cc: Sebastian Chlad, linux-bluetooth@vger.kernel.org
In-Reply-To: <2115852EF878384FA984E6B76A9F379E164297@IRSMSX104.ger.corp.intel.com>

Hi Sebastian,

On Tue, Dec 17, 2013, Chlad, SebastianX wrote:
> To summarize: please let me know what is you stand on [ '#' notation ]
> - I'll act accordingly and then I'll push L2CAP PTS test results once
> I finish second round of testing.

I don't really care as long as it's consistent throughout all files.
Either change all places which have "True + #" to "False + #" or use the
existing convention in your new file.

Johan

^ permalink raw reply

* [PATCH v2 bluetooth-next 0/7] 6lowpan: udp compression/uncompression fix
From: Alexander Aring @ 2013-12-17 10:32 UTC (permalink / raw)
  To: linux-zigbee-devel; +Cc: werner, linux-bluetooth, Alexander Aring

The current 6LoWPAN udp compression/uncompression is completely broken.
This patch series fix a lot of udp compression/uncompression issues and
add support parsing with lowpan_fetch_skb function.

I already sent this patch series to netdev some time ago. That's why I add a
v2 to this series. Now we doing all 6lowpan things into bluetooth-next to
avoid merge conflicts.

Changes since v2:
 - remove unnecessary casts and brackes, suggested by Werner Almesberger.
 - use pr_debug_ratelimited instead of pr_debug on patch 4/7.

Alexander Aring (7):
  6lowpan: fix udp nullpointer dereferencing
  6lowpan: fix udp compress ordering
  6lowpan: fix udp byte ordering
  6lowpan: add udp warning for elided checksum
  6lowpan: udp use lowpan_fetch_skb function
  6lowpan: udp use subtraction on both conditions
  6lowpan: cleanup udp compress function

 net/ieee802154/6lowpan.h      |  1 +
 net/ieee802154/6lowpan_iphc.c | 79 +++++++++++++++++++++++--------------------
 2 files changed, 43 insertions(+), 37 deletions(-)

-- 
1.8.5.1


^ permalink raw reply

* [PATCH v2 bluetooth-next 1/7] 6lowpan: fix udp nullpointer dereferencing
From: Alexander Aring @ 2013-12-17 10:32 UTC (permalink / raw)
  To: linux-zigbee-devel; +Cc: werner, linux-bluetooth, Alexander Aring
In-Reply-To: <1387276373-23882-1-git-send-email-alex.aring@gmail.com>

Sometimes a nullpointer dereferencing occurs because of using a wrong
pointer arithmetic in udp_uncompression.

This patch changes "**(hc06_ptr + 3)" to the right one "*(*hc06_ptr +
3)". Dereferencing like "**(hc06_ptr + 3)" works in a random case only.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/6lowpan_iphc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ieee802154/6lowpan_iphc.c b/net/ieee802154/6lowpan_iphc.c
index 88e7da5..8bbe28e 100644
--- a/net/ieee802154/6lowpan_iphc.c
+++ b/net/ieee802154/6lowpan_iphc.c
@@ -548,7 +548,7 @@ static void compress_udp_header(u8 **hc06_ptr, struct sk_buff *skb)
 				LOWPAN_NHC_UDP_4BIT_PORT)) {
 		pr_debug("UDP header: both ports compression to 4 bits\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_11;
-		**(hc06_ptr + 1) = /* subtraction is faster */
+		*(*hc06_ptr + 1) = /* subtraction is faster */
 		   (u8)((uh->dest - LOWPAN_NHC_UDP_4BIT_PORT) +
 		       ((uh->source & LOWPAN_NHC_UDP_4BIT_PORT) << 4));
 		*hc06_ptr += 2;
@@ -557,14 +557,14 @@ static void compress_udp_header(u8 **hc06_ptr, struct sk_buff *skb)
 		pr_debug("UDP header: remove 8 bits of dest\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_01;
 		memcpy(*hc06_ptr + 1, &uh->source, 2);
-		**(hc06_ptr + 3) = (u8)(uh->dest - LOWPAN_NHC_UDP_8BIT_PORT);
+		*(*hc06_ptr + 3) = (u8)(uh->dest - LOWPAN_NHC_UDP_8BIT_PORT);
 		*hc06_ptr += 4;
 	} else if ((uh->source & LOWPAN_NHC_UDP_8BIT_MASK) ==
 			LOWPAN_NHC_UDP_8BIT_PORT) {
 		pr_debug("UDP header: remove 8 bits of source\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_10;
 		memcpy(*hc06_ptr + 1, &uh->dest, 2);
-		**(hc06_ptr + 3) = (u8)(uh->source - LOWPAN_NHC_UDP_8BIT_PORT);
+		*(*hc06_ptr + 3) = (u8)(uh->source - LOWPAN_NHC_UDP_8BIT_PORT);
 		*hc06_ptr += 4;
 	} else {
 		pr_debug("UDP header: can't compress\n");
-- 
1.8.5.1


^ permalink raw reply related

* [PATCH v2 bluetooth-next 2/7] 6lowpan: fix udp compress ordering
From: Alexander Aring @ 2013-12-17 10:32 UTC (permalink / raw)
  To: linux-zigbee-devel; +Cc: werner, linux-bluetooth, Alexander Aring
In-Reply-To: <1387276373-23882-1-git-send-email-alex.aring@gmail.com>

In case ((ntohs(uh->source) & LOWPAN_NHC_UDP_8BIT_MASK) the order of
uncompression is wrong. It's always first source port then destination
port as second.

See:
http://tools.ietf.org/html/rfc6282#section-4.3.3

"Fields carried in-line (in part or in whole) appear in the same order
as they do in the UDP header format"

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/6lowpan_iphc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ieee802154/6lowpan_iphc.c b/net/ieee802154/6lowpan_iphc.c
index 8bbe28e..0537b33 100644
--- a/net/ieee802154/6lowpan_iphc.c
+++ b/net/ieee802154/6lowpan_iphc.c
@@ -563,8 +563,8 @@ static void compress_udp_header(u8 **hc06_ptr, struct sk_buff *skb)
 			LOWPAN_NHC_UDP_8BIT_PORT) {
 		pr_debug("UDP header: remove 8 bits of source\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_10;
-		memcpy(*hc06_ptr + 1, &uh->dest, 2);
-		*(*hc06_ptr + 3) = (u8)(uh->source - LOWPAN_NHC_UDP_8BIT_PORT);
+		*(*hc06_ptr + 1) = (u8)(uh->source - LOWPAN_NHC_UDP_8BIT_PORT);
+		memcpy(*hc06_ptr + 2, &uh->dest, 2);
 		*hc06_ptr += 4;
 	} else {
 		pr_debug("UDP header: can't compress\n");
-- 
1.8.5.1


^ permalink raw reply related

* [PATCH v2 bluetooth-next 3/7] 6lowpan: fix udp byte ordering
From: Alexander Aring @ 2013-12-17 10:32 UTC (permalink / raw)
  To: linux-zigbee-devel; +Cc: werner, linux-bluetooth, Alexander Aring
In-Reply-To: <1387276373-23882-1-git-send-email-alex.aring@gmail.com>

The incoming udp header in lowpan_compress_udp_header function is
already in network byte order.

Everytime we read this values for source and destination port we need
to convert this value to host byte order.

In the outcoming header we need to set this value in network byte order
which the upcoming process assumes.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/6lowpan_iphc.c | 45 +++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/net/ieee802154/6lowpan_iphc.c b/net/ieee802154/6lowpan_iphc.c
index 0537b33..a7ae340 100644
--- a/net/ieee802154/6lowpan_iphc.c
+++ b/net/ieee802154/6lowpan_iphc.c
@@ -283,20 +283,21 @@ uncompress_udp_header(struct sk_buff *skb, struct udphdr *uh)
 			break;
 		case LOWPAN_NHC_UDP_CS_P_01:
 			memcpy(&uh->source, &skb->data[0], 2);
-			uh->dest =
-			   skb->data[2] + LOWPAN_NHC_UDP_8BIT_PORT;
+			uh->dest = htons(skb->data[2] +
+					 LOWPAN_NHC_UDP_8BIT_PORT);
 			skb_pull(skb, 3);
 			break;
 		case LOWPAN_NHC_UDP_CS_P_10:
-			uh->source = skb->data[0] + LOWPAN_NHC_UDP_8BIT_PORT;
+			uh->source = htons(skb->data[0] +
+					   LOWPAN_NHC_UDP_8BIT_PORT);
 			memcpy(&uh->dest, &skb->data[1], 2);
 			skb_pull(skb, 3);
 			break;
 		case LOWPAN_NHC_UDP_CS_P_11:
-			uh->source =
-			   LOWPAN_NHC_UDP_4BIT_PORT + (skb->data[0] >> 4);
-			uh->dest =
-			   LOWPAN_NHC_UDP_4BIT_PORT + (skb->data[0] & 0x0f);
+			uh->source = htons(LOWPAN_NHC_UDP_4BIT_PORT +
+					   (skb->data[0] >> 4));
+			uh->dest = htons(LOWPAN_NHC_UDP_4BIT_PORT +
+					 (skb->data[0] & 0x0f));
 			skb_pull(skb, 1);
 			break;
 		default:
@@ -306,7 +307,7 @@ uncompress_udp_header(struct sk_buff *skb, struct udphdr *uh)
 		}
 
 		pr_debug("uncompressed UDP ports: src = %d, dst = %d\n",
-			 uh->source, uh->dest);
+			 ntohs(uh->source), ntohs(uh->dest));
 
 		/* copy checksum */
 		memcpy(&uh->check, &skb->data[0], 2);
@@ -318,7 +319,7 @@ uncompress_udp_header(struct sk_buff *skb, struct udphdr *uh)
 		 * frame
 		 */
 		uh->len = htons(skb->len + sizeof(struct udphdr));
-		pr_debug("uncompressed UDP length: src = %d", uh->len);
+		pr_debug("uncompressed UDP length: src = %d", ntohs(uh->len));
 	} else {
 		pr_debug("ERROR: unsupported NH format\n");
 		goto err;
@@ -542,28 +543,30 @@ static void compress_udp_header(u8 **hc06_ptr, struct sk_buff *skb)
 {
 	struct udphdr *uh = udp_hdr(skb);
 
-	if (((uh->source & LOWPAN_NHC_UDP_4BIT_MASK) ==
-				LOWPAN_NHC_UDP_4BIT_PORT) &&
-	    ((uh->dest & LOWPAN_NHC_UDP_4BIT_MASK) ==
-				LOWPAN_NHC_UDP_4BIT_PORT)) {
+	if (((ntohs(uh->source) & LOWPAN_NHC_UDP_4BIT_MASK) ==
+	     LOWPAN_NHC_UDP_4BIT_PORT) &&
+	    ((ntohs(uh->dest) & LOWPAN_NHC_UDP_4BIT_MASK) ==
+	     LOWPAN_NHC_UDP_4BIT_PORT)) {
 		pr_debug("UDP header: both ports compression to 4 bits\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_11;
 		*(*hc06_ptr + 1) = /* subtraction is faster */
-		   (u8)((uh->dest - LOWPAN_NHC_UDP_4BIT_PORT) +
-		       ((uh->source & LOWPAN_NHC_UDP_4BIT_PORT) << 4));
+		   (u8)((ntohs(uh->dest) - LOWPAN_NHC_UDP_4BIT_PORT) +
+		       ((ntohs(uh->source) & LOWPAN_NHC_UDP_4BIT_PORT) << 4));
 		*hc06_ptr += 2;
-	} else if ((uh->dest & LOWPAN_NHC_UDP_8BIT_MASK) ==
-			LOWPAN_NHC_UDP_8BIT_PORT) {
+	} else if ((ntohs(uh->dest) & LOWPAN_NHC_UDP_8BIT_MASK) ==
+		   LOWPAN_NHC_UDP_8BIT_PORT) {
 		pr_debug("UDP header: remove 8 bits of dest\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_01;
 		memcpy(*hc06_ptr + 1, &uh->source, 2);
-		*(*hc06_ptr + 3) = (u8)(uh->dest - LOWPAN_NHC_UDP_8BIT_PORT);
+		*(*hc06_ptr + 3) = (u8)(ntohs(uh->dest) -
+					LOWPAN_NHC_UDP_8BIT_PORT);
 		*hc06_ptr += 4;
-	} else if ((uh->source & LOWPAN_NHC_UDP_8BIT_MASK) ==
-			LOWPAN_NHC_UDP_8BIT_PORT) {
+	} else if ((ntohs(uh->source) & LOWPAN_NHC_UDP_8BIT_MASK) ==
+		   LOWPAN_NHC_UDP_8BIT_PORT) {
 		pr_debug("UDP header: remove 8 bits of source\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_10;
-		*(*hc06_ptr + 1) = (u8)(uh->source - LOWPAN_NHC_UDP_8BIT_PORT);
+		*(*hc06_ptr + 1) = (u8)(ntohs(uh->source) -
+					LOWPAN_NHC_UDP_8BIT_PORT);
 		memcpy(*hc06_ptr + 2, &uh->dest, 2);
 		*hc06_ptr += 4;
 	} else {
-- 
1.8.5.1


^ permalink raw reply related

* [PATCH v2 bluetooth-next 4/7] 6lowpan: add udp warning for elided checksum
From: Alexander Aring @ 2013-12-17 10:32 UTC (permalink / raw)
  To: linux-zigbee-devel; +Cc: werner, linux-bluetooth, Alexander Aring
In-Reply-To: <1387276373-23882-1-git-send-email-alex.aring@gmail.com>

Bit 5 of "UDP LOWPAN_NHC Format" indicate that the checksum can be
elided.
The host need to calculate the udp checksum afterwards but this isn't
supported right now.

See:
http://tools.ietf.org/html/rfc6282#section-4.3.3
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/6lowpan.h      |  1 +
 net/ieee802154/6lowpan_iphc.c | 11 ++++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/ieee802154/6lowpan.h b/net/ieee802154/6lowpan.h
index 10909e5..4942c5d 100644
--- a/net/ieee802154/6lowpan.h
+++ b/net/ieee802154/6lowpan.h
@@ -231,6 +231,7 @@
 #define LOWPAN_NHC_UDP_CS_P_10	0xF2 /* source = 0xF0 + 8bit inline,
 					dest = 16 bit inline */
 #define LOWPAN_NHC_UDP_CS_P_11	0xF3 /* source & dest = 0xF0B + 4bit inline */
+#define LOWPAN_NHC_UDP_CS_C	0x04 /* checksum elided */
 
 #ifdef DEBUG
 /* print data in line */
diff --git a/net/ieee802154/6lowpan_iphc.c b/net/ieee802154/6lowpan_iphc.c
index a7ae340..8303c93 100644
--- a/net/ieee802154/6lowpan_iphc.c
+++ b/net/ieee802154/6lowpan_iphc.c
@@ -309,9 +309,14 @@ uncompress_udp_header(struct sk_buff *skb, struct udphdr *uh)
 		pr_debug("uncompressed UDP ports: src = %d, dst = %d\n",
 			 ntohs(uh->source), ntohs(uh->dest));
 
-		/* copy checksum */
-		memcpy(&uh->check, &skb->data[0], 2);
-		skb_pull(skb, 2);
+		/* checksum */
+		if (tmp & LOWPAN_NHC_UDP_CS_C) {
+			pr_debug_ratelimited("checksum elided currently not supported");
+			goto err;
+		} else {
+			memcpy(&uh->check, &skb->data[0], 2);
+			skb_pull(skb, 2);
+		}
 
 		/*
 		 * UDP lenght needs to be infered from the lower layers
-- 
1.8.5.1


^ permalink raw reply related

* [PATCH v2 bluetooth-next 5/7] 6lowpan: udp use lowpan_fetch_skb function
From: Alexander Aring @ 2013-12-17 10:32 UTC (permalink / raw)
  To: linux-zigbee-devel; +Cc: werner, linux-bluetooth, Alexander Aring
In-Reply-To: <1387276373-23882-1-git-send-email-alex.aring@gmail.com>

Cleanup the lowpan_uncompress_udp_header function to use the
lowpan_fetch_skb function.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/6lowpan_iphc.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/net/ieee802154/6lowpan_iphc.c b/net/ieee802154/6lowpan_iphc.c
index 8303c93..ee6891f 100644
--- a/net/ieee802154/6lowpan_iphc.c
+++ b/net/ieee802154/6lowpan_iphc.c
@@ -265,40 +265,37 @@ lowpan_uncompress_multicast_daddr(struct sk_buff *skb,
 static int
 uncompress_udp_header(struct sk_buff *skb, struct udphdr *uh)
 {
-	u8 tmp;
+	bool fail;
+	u8 tmp = 0, val = 0;
 
 	if (!uh)
 		goto err;
 
-	if (lowpan_fetch_skb_u8(skb, &tmp))
-		goto err;
+	fail = lowpan_fetch_skb(skb, &tmp, 1);
 
 	if ((tmp & LOWPAN_NHC_UDP_MASK) == LOWPAN_NHC_UDP_ID) {
 		pr_debug("UDP header uncompression\n");
 		switch (tmp & LOWPAN_NHC_UDP_CS_P_11) {
 		case LOWPAN_NHC_UDP_CS_P_00:
-			memcpy(&uh->source, &skb->data[0], 2);
-			memcpy(&uh->dest, &skb->data[2], 2);
-			skb_pull(skb, 4);
+			fail |= lowpan_fetch_skb(skb, &uh->source, 2);
+			fail |= lowpan_fetch_skb(skb, &uh->dest, 2);
 			break;
 		case LOWPAN_NHC_UDP_CS_P_01:
-			memcpy(&uh->source, &skb->data[0], 2);
-			uh->dest = htons(skb->data[2] +
-					 LOWPAN_NHC_UDP_8BIT_PORT);
-			skb_pull(skb, 3);
+			fail |= lowpan_fetch_skb(skb, &uh->source, 2);
+			fail |= lowpan_fetch_skb(skb, &val, 1);
+			uh->dest = htons(val + LOWPAN_NHC_UDP_8BIT_PORT);
 			break;
 		case LOWPAN_NHC_UDP_CS_P_10:
-			uh->source = htons(skb->data[0] +
-					   LOWPAN_NHC_UDP_8BIT_PORT);
-			memcpy(&uh->dest, &skb->data[1], 2);
-			skb_pull(skb, 3);
+			fail |= lowpan_fetch_skb(skb, &val, 1);
+			uh->source = htons(val + LOWPAN_NHC_UDP_8BIT_PORT);
+			fail |= lowpan_fetch_skb(skb, &uh->dest, 2);
 			break;
 		case LOWPAN_NHC_UDP_CS_P_11:
+			fail |= lowpan_fetch_skb(skb, &val, 1);
 			uh->source = htons(LOWPAN_NHC_UDP_4BIT_PORT +
-					   (skb->data[0] >> 4));
+					   (val >> 4));
 			uh->dest = htons(LOWPAN_NHC_UDP_4BIT_PORT +
-					 (skb->data[0] & 0x0f));
-			skb_pull(skb, 1);
+					 (val & 0x0f));
 			break;
 		default:
 			pr_debug("ERROR: unknown UDP format\n");
@@ -314,8 +311,7 @@ uncompress_udp_header(struct sk_buff *skb, struct udphdr *uh)
 			pr_debug_ratelimited("checksum elided currently not supported");
 			goto err;
 		} else {
-			memcpy(&uh->check, &skb->data[0], 2);
-			skb_pull(skb, 2);
+			fail |= lowpan_fetch_skb(skb, &uh->check, 2);
 		}
 
 		/*
@@ -330,6 +326,9 @@ uncompress_udp_header(struct sk_buff *skb, struct udphdr *uh)
 		goto err;
 	}
 
+	if (fail)
+		goto err;
+
 	return 0;
 err:
 	return -EINVAL;
-- 
1.8.5.1


^ permalink raw reply related

* [PATCH v2 bluetooth-next 6/7] 6lowpan: udp use subtraction on both conditions
From: Alexander Aring @ 2013-12-17 10:32 UTC (permalink / raw)
  To: linux-zigbee-devel; +Cc: werner, linux-bluetooth, Alexander Aring
In-Reply-To: <1387276373-23882-1-git-send-email-alex.aring@gmail.com>

Cleanup code to handle both calculation in the same way.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/6lowpan_iphc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ieee802154/6lowpan_iphc.c b/net/ieee802154/6lowpan_iphc.c
index ee6891f..d49cc1b 100644
--- a/net/ieee802154/6lowpan_iphc.c
+++ b/net/ieee802154/6lowpan_iphc.c
@@ -555,7 +555,7 @@ static void compress_udp_header(u8 **hc06_ptr, struct sk_buff *skb)
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_11;
 		*(*hc06_ptr + 1) = /* subtraction is faster */
 		   (u8)((ntohs(uh->dest) - LOWPAN_NHC_UDP_4BIT_PORT) +
-		       ((ntohs(uh->source) & LOWPAN_NHC_UDP_4BIT_PORT) << 4));
+		       ((ntohs(uh->source) - LOWPAN_NHC_UDP_4BIT_PORT) << 4));
 		*hc06_ptr += 2;
 	} else if ((ntohs(uh->dest) & LOWPAN_NHC_UDP_8BIT_MASK) ==
 		   LOWPAN_NHC_UDP_8BIT_PORT) {
-- 
1.8.5.1


^ permalink raw reply related

* [PATCH v2 bluetooth-next 7/7] 6lowpan: cleanup udp compress function
From: Alexander Aring @ 2013-12-17 10:32 UTC (permalink / raw)
  To: linux-zigbee-devel; +Cc: werner, linux-bluetooth, Alexander Aring
In-Reply-To: <1387276373-23882-1-git-send-email-alex.aring@gmail.com>

This patch remove unnecessary casts and brackets in compress_udp_header
function.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/ieee802154/6lowpan_iphc.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/net/ieee802154/6lowpan_iphc.c b/net/ieee802154/6lowpan_iphc.c
index d49cc1b..78bbb76 100644
--- a/net/ieee802154/6lowpan_iphc.c
+++ b/net/ieee802154/6lowpan_iphc.c
@@ -553,24 +553,22 @@ static void compress_udp_header(u8 **hc06_ptr, struct sk_buff *skb)
 	     LOWPAN_NHC_UDP_4BIT_PORT)) {
 		pr_debug("UDP header: both ports compression to 4 bits\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_11;
-		*(*hc06_ptr + 1) = /* subtraction is faster */
-		   (u8)((ntohs(uh->dest) - LOWPAN_NHC_UDP_4BIT_PORT) +
-		       ((ntohs(uh->source) - LOWPAN_NHC_UDP_4BIT_PORT) << 4));
+		*(*hc06_ptr + 1) = ntohs(uh->dest) - LOWPAN_NHC_UDP_4BIT_PORT +
+				   ((ntohs(uh->source) -
+				     LOWPAN_NHC_UDP_4BIT_PORT) << 4);
 		*hc06_ptr += 2;
 	} else if ((ntohs(uh->dest) & LOWPAN_NHC_UDP_8BIT_MASK) ==
 		   LOWPAN_NHC_UDP_8BIT_PORT) {
 		pr_debug("UDP header: remove 8 bits of dest\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_01;
 		memcpy(*hc06_ptr + 1, &uh->source, 2);
-		*(*hc06_ptr + 3) = (u8)(ntohs(uh->dest) -
-					LOWPAN_NHC_UDP_8BIT_PORT);
+		*(*hc06_ptr + 3) = ntohs(uh->dest) - LOWPAN_NHC_UDP_8BIT_PORT;
 		*hc06_ptr += 4;
 	} else if ((ntohs(uh->source) & LOWPAN_NHC_UDP_8BIT_MASK) ==
 		   LOWPAN_NHC_UDP_8BIT_PORT) {
 		pr_debug("UDP header: remove 8 bits of source\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_10;
-		*(*hc06_ptr + 1) = (u8)(ntohs(uh->source) -
-					LOWPAN_NHC_UDP_8BIT_PORT);
+		*(*hc06_ptr + 1) = ntohs(uh->source) - LOWPAN_NHC_UDP_8BIT_PORT;
 		memcpy(*hc06_ptr + 2, &uh->dest, 2);
 		*hc06_ptr += 4;
 	} else {
-- 
1.8.5.1


^ permalink raw reply related

* [PATCH 1/2] android/pts: Add PTS PICS and PIXIT for MAP
From: Jakub Tyszkowski @ 2013-12-17 10:53 UTC (permalink / raw)
  To: linux-bluetooth

Add PICS and PIXIT targetting Android 4.4 and PTS version 5.0.
---
 android/Makefile.am   |   3 +-
 android/pics-map.txt  | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++
 android/pixit-map.txt |  45 +++++++++++++
 3 files changed, 222 insertions(+), 1 deletion(-)
 create mode 100644 android/pics-map.txt
 create mode 100644 android/pixit-map.txt

diff --git a/android/Makefile.am b/android/Makefile.am
index 0034487..abeda84 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -122,4 +122,5 @@ EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
 		android/pixit-opp.txt android/pixit-pan.txt \
 		android/pixit-pbap.txt android/pts-gap.txt android/pts-hid.txt \
 		android/pts-opp.txt android/pts-pbap.txt \
-		android/audio-ipc-api.txt
+		android/audio-ipc-api.txt android/pics-map.txt \
+		android/pixit-map.txt
diff --git a/android/pics-map.txt b/android/pics-map.txt
new file mode 100644
index 0000000..2875885
--- /dev/null
+++ b/android/pics-map.txt
@@ -0,0 +1,175 @@
+MAP PICS for the PTS tool.
+
+PTS version: 5.0
+
+* - different than PTS defaults
+# - not yet implemented/supported
+
+M - mandatory
+O - optional
+
+	Profile Version
+-------------------------------------------------------------------------------
+Parameter Name	Selected	Description
+-------------------------------------------------------------------------------
+TSPC_MAP_0_1	False		Role: Map 1.0 (C1)
+TSPC_MAP_0_2	True (*)	Role: Map 1.1 (C1)
+TSPC_MAP_0_3	False		Role: Map 1.2 (C1)
+-------------------------------------------------------------------------------
+C.1: Mandatory to support only one Profile version.
+-------------------------------------------------------------------------------
+
+
+	Roles
+-------------------------------------------------------------------------------
+Parameter Name	Selected	Description
+-------------------------------------------------------------------------------
+TSPC_MAP_1_1	True (*)	Role: Messaging Server Equipment (C1)
+TSPC_MAP_1_2	False		Role: Messaging Client Equipment (C1)
+-------------------------------------------------------------------------------
+C.1: It is mandatory to support at least one of the defined roles.
+-------------------------------------------------------------------------------
+
+
+	Supported features MCE
+-------------------------------------------------------------------------------
+Parameter Name	Selected	Description
+-------------------------------------------------------------------------------
+TSPC_MAP_2_1	False		MCE: Message Notification (C1)
+TSPC_MAP_2_1a	False		MCE: SendEvent (C4)
+TSPC_MAP_2_2	False		MCE: Message Browsing (C1)
+TSPC_MAP_2_2a	False		MCE: SetFolder (C5)
+TSPC_MAP_2_2b	False		MCE: GetFoldersListing (C5)
+TSPC_MAP_2_2c	False		MCE: GetMessagesListing (C5)
+TSPC_MAP_2_2d	False		MCE: GetMessage (O)
+TSPC_MAP_2_2e	False		MCE: SetMessageStatus (O)
+TSPC_MAP_2_2f	False		MCE: UpdateInbox (O)
+TSPC_MAP_2_2g	False		MCE: Filtering (O)
+TSPC_MAP_2_2h	False		MCE: Multiple simultaneous MAS instances (O)
+TSPC_MAP_2_3	False		MCE: Message Uploading (O)
+TSPC_MAP_2_3a	False		MCE: SetFolder (C6)
+TSPC_MAP_2_3b	False		MCE: GetFoldersListing (C6)
+TSPC_MAP_2_3c	False		MCE: PushMessage (C6)
+TSPC_MAP_2_4	False		MCE: Message Delete (O)
+TSPC_MAP_2_4a	False		MCE: SetMessageStatus (C7)
+TSPC_MAP_2_5	False		MCE: Notification Registration (C2)
+TSPC_MAP_2_5a	False		MCE: SetNotificationRegistration off (O)
+TSPC_MAP_2_5b	False		MCE: SetNotificationRegistration on (C8)
+TSPC_MAP_2_6	False		MCE: Supported Message Types
+TSPC_MAP_2_6a	False (*)	MCE: EMAIL (C3)
+TSPC_MAP_2_6b	False (*)	MCE: SMS_GSM (C3)
+TSPC_MAP_2_6c	False (*)	MCE: SMS_CDMA (C3)
+TSPC_MAP_2_6d	False (*)	MCE: MMS (C3)
+TSPC_MAP_2_7	False		MCE: Instance Information (Not Supported)
+TSPC_MAP_2_7a	False (*)	MCE: GetMASInstanceInformation (Not Supported)
+TSPC_MAP_2_8	False		MCE: Extended MAP-Event-Report (Not Supported)
+TSPC_MAP_2_8a	False (*)	MCE: MAP-Event-Report: Version 1.1
+					(Not Supported)
+-------------------------------------------------------------------------------
+C.1: Mandatory to support at least one of the defined features TSPC_MAP_2_1 or
+	TSPC_MAP_2_2.
+C.2: Mandatory to support TSPC_MAP_2_5 if TSPC_MAP_2_1 is supported.
+C.3: Mandatory to support at least one of the defined message types
+	TSPC_MAP_2_6a to TSPC_MAP_2_6d IF TSPC_MAP_2_2 or TSPC_MAP_2_3 is
+	supported.
+C.4: Support of functionality TSPC_MAP_2_1a mandatory IF related feature
+	TSPC_MAP_2_1 supported.
+C.5: Support of functionality mandatory IF TSPC_MAP_2_2 supported.
+C.6: Support of functionality mandatory IF TSPC_MAP_2_3 supported.
+C.7: Support of functionality mandatory IF TSPC_MAP_2_4 supported.
+C.8: Mandatory to support IF TSPC_MAP_2_5 (Notification Registration) is
+	supported, otherwise excluded.
+C.9: Optional to support IF TSPC_MAP_0_3 (MAP v1.2) is supported, otherwise
+	excluded.
+C.10: Mandatory to support IF TSPC_MAP_0_3 (MAP v1.2) and TSPC_MAP_2_1
+	(Message Notification) is supported, otherwise excluded.
+-------------------------------------------------------------------------------
+
+
+	Supported features MSE
+-------------------------------------------------------------------------------
+Parameter Name	Selected	Description
+-------------------------------------------------------------------------------
+TSPC_MAP_3_1	True		MSE: Message Notification (M)
+TSPC_MAP_3_1a	True		MSE: SendEvent (M)
+TSPC_MAP_3_2	True		MSE: Message Browsing (M)
+TSPC_MAP_3_2a	True		MSE: SetFolder (M)
+TSPC_MAP_3_2b	True		MSE: GetFoldersListing (M)
+TSPC_MAP_3_2c	True		MSE: GetMessagesListing (M)
+TSPC_MAP_3_2d	True		MSE: GetMessage (M)
+TSPC_MAP_3_2e	True		MSE: SetMessageStatus (M)
+TSPC_MAP_3_2f	True		MSE: UpdateInbox (M)
+TSPC_MAP_3_2g	False		MSE: Multiple simultaneous MAS instances (O)
+TSPC_MAP_3_3	True		MSE: Message Uploading (M)
+TSPC_MAP_3_3a	True		MSE: SetFolder (M)
+TSPC_MAP_3_3b	True		MSE: GetFoldersListing (M)
+TSPC_MAP_3_3c	True		MSE: PushMessage (M)
+TSPC_MAP_3_4	True		MSE: Message Delete (M)
+TSPC_MAP_3_4a	True		MSE: SetMessageStatus (M)
+TSPC_MAP_3_5	True		MSE: Notification Registration (M)
+TSPC_MAP_3_5a	True		MSE: SetNotificationRegistration (M)
+TSPC_MAP_3_6	False		MSE: Supported Message Types
+TSPC_MAP_3_6a	False		MSE: EMAIL (C1)
+TSPC_MAP_3_6b	True		MSE: SMS_GSM (C1)
+TSPC_MAP_3_6c	False		MSE: SMS_CDMA (C1)
+TSPC_MAP_3_6d	False (*)	MSE: MMS (C1)
+TSPC_MAP_3_7	False		MSE: Instance Information (Not Supported)
+TSPC_MAP_3_7a	False (*)	MSE: GetMASInstanceInformation (Not Supported)
+TSPC_MAP_3_8	False		MSE: Extended MAP-Event-Report (Not Supported)
+TSPC_MAP_3_8a	False (*)	MSE: MAP-Event-Report: Version 1.1
+					(Not Supported)
+-------------------------------------------------------------------------------
+C.1: Mandatory to support at least one of the defined message types
+	TSPC_MAP_3_6a to TSPC_MAP_3_6d IF TSPC_MAP_3_2 or TSPC_MAP_3_3
+	is supported.
+C.2: Mandatory to support IF TSPC_MAP_0_3 (MAP v1.2) is supported,
+	otherwise excluded.
+-------------------------------------------------------------------------------
+
+
+	GOEP v2.0 or later Features
+-------------------------------------------------------------------------------
+Parameter Name	Selected	Description
+-------------------------------------------------------------------------------
+TSPC_MAP_7b_1	False		GOEP v2.0 or later (C1)
+TSPC_MAP_7b_2	False		GOEP v2 Backwards Compatibility (C1)
+TSPC_MAP_7b_3	False		OBEX over L2CAP (C1)
+-------------------------------------------------------------------------------
+C.1: Mandatory if TSPC_MAP_0_3 (MAP v1.2) is supported else excluded.
+-------------------------------------------------------------------------------
+
+
+	MCE OBEX Header Support
+-------------------------------------------------------------------------------
+Parameter Name	Selected	Description
+-------------------------------------------------------------------------------
+TSPC_MAP_10_1	False (*)	Name (M)
+TSPC_MAP_10_2	False (*)	Typr (M)
+TSPC_MAP_10_3	False (*)	Body (M)
+TSPC_MAP_10_4	False (*)	End of Body (M)
+TSPC_MAP_10_5	False (*)	Target (M)
+TSPC_MAP_10_6	False (*)	Who (M)
+TSPC_MAP_10_7	False (*)	Connection ID (M)
+TSPC_MAP_10_8	False (*)	Application Parameters (M)
+TSPC_MAP_10_9	False		SRM (C2)
+TSPC_MAP_10_10	False		Receive SRMP (C2)
+TSPC_MAP_10_11	False		Send SRMP (C2)
+-------------------------------------------------------------------------------
+C.1: Mandatory if TSPC_MAP_0_3 (MAP v1.2) is supported else excluded.
+C.2: Optional if TSPC_MAP_0_3 (MAP v1.2) is supported else excluded.
+-------------------------------------------------------------------------------
+
+
+	GetMessagesListing Filtering Parameter Support
+-------------------------------------------------------------------------------
+Parameter Name	Selected	Description
+-------------------------------------------------------------------------------
+TSPC_MAP_20_1	False (*)	MCE: FilterMessageType (O)
+TSPC_MAP_20_2	False (*)	MCE: FilterPeriodBegin (O)
+TSPC_MAP_20_3	False (*)	MCE: FilterPeriodEnd (O)
+TSPC_MAP_20_4	False (*)	MCE: FilterReadStatus (O)
+TSPC_MAP_20_5	False (*)	MCE: FilterRecipient (O)
+TSPC_MAP_20_6	False (*)	MCE: FilterOriginator (O)
+TSPC_MAP_20_7	False (*)	MCE: FilterPriority (O)
+TSPC_ALL	False (*)	Turns on all the test cases
+-------------------------------------------------------------------------------
diff --git a/android/pixit-map.txt b/android/pixit-map.txt
new file mode 100644
index 0000000..c8e2591
--- /dev/null
+++ b/android/pixit-map.txt
@@ -0,0 +1,45 @@
+MAP PIXIT for the PTS tool.
+
+PTS version: 5.0
+
+* - different than PTS defaults
+& - should be set to IUT Bluetooth address
+
+		Required PIXIT settings
+-------------------------------------------------------------------------------
+Parameter Name						Value
+-------------------------------------------------------------------------------
+TSPX_auth_password					0000
+TSPX_auth_user_id					PTS
+TSPX_bd_addr_iut					08606E414394 (*&)
+TSPX_client_class_of_device				100204
+TSPX_delete_link_key					FALSE
+TSPX_get_object_name					put.gif
+TSPX_initial_path
+TSPX_l2cap_psm						1001
+TSPX_no_confirmations					FALSE
+TSPX_pin_code						0000
+TSPX_rfcomm_channel					8
+TSPX_secure_simple_pairing_pass_key_confirmation	FALSE
+TSPX_security_enabled					TRUE
+TSPX_server_class_of_device				100204
+TSPX_time_guard						300000
+TSPX_use_implicit_send					TRUE
+TSPX_Message_Access_rfcomm_channel			1
+TSPX_Message_Notification_rfcomm_channel		2
+TSPX_SPP_rfcomm_channel					03
+TSPX_filter_period_begin				20100101T000000
+TSPX_filter_period_end					20111231T125959
+TSPX_filter_recipient					PTS
+TSPX_filter_originator					PTS
+TSPX_default_message_upload_folder_in_msg		draft
+TSPX_default_test_folder_in_msg				inbox
+TSPX_use_fixed_MASInstanceID				FALSE
+TSPX_MASInstanceID_SMS					0
+TSPX_MASInstanceID_MMS					0
+TSPX_MASInstanceID_Email				0
+TSPX_message_notification_l2cap_psm			1003
+TSPX_message_notification_rfcomm_channel		9
+TSPX_upload_msg_phonenumber				123456789
+TSPX_upload_msg_emailaddress				PTS_Test@bluetooth.com
+-------------------------------------------------------------------------------
-- 
1.8.5


^ permalink raw reply related

* [PATCH 2/2] android/pts: Add PTS test results for MAP
From: Jakub Tyszkowski @ 2013-12-17 10:53 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1387277633-6008-1-git-send-email-jakub.tyszkowski@tieto.com>

Stock Bluedroid PTS results.
---
 android/Makefile.am |  2 +-
 android/pts-map.txt | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 android/pts-map.txt

diff --git a/android/Makefile.am b/android/Makefile.am
index abeda84..909846e 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -123,4 +123,4 @@ EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
 		android/pixit-pbap.txt android/pts-gap.txt android/pts-hid.txt \
 		android/pts-opp.txt android/pts-pbap.txt \
 		android/audio-ipc-api.txt android/pics-map.txt \
-		android/pixit-map.txt
+		android/pixit-map.txt android/pts-map.txt
diff --git a/android/pts-map.txt b/android/pts-map.txt
new file mode 100644
index 0000000..26702af
--- /dev/null
+++ b/android/pts-map.txt
@@ -0,0 +1,93 @@
+PTS test results for MAP
+
+PTS version: 5.0
+Tested: 17.12.2013
+
+Results:
+PASS	test passed
+FAIL	test failed
+INC	test is inconclusive
+N/A	test is disabled due to PICS setup
+
+-------------------------------------------------------------------------------
+Test Name		Result	Notes
+-------------------------------------------------------------------------------
+TC_MCE_MSM_BV_01_I	N/A
+TC_MCE_MSM_BV_02_I	N/A
+TC_MCE_MSM_BV_03_I	N/A
+TC_MCE_MSM_BV_04_I	N/A
+TC_MCE_MSM_BV_13_I	N/A
+TC_MCE_MSM_BV_14_I	N/A
+TC_MCE_MNR_BV_01_I	N/A
+TC_MCE_MNR_BV_02_I	N/A
+TC_MCE_MMB_BV_01_I	N/A
+TC_MCE_MMB_BV_02_I	N/A
+TC_MCE_MMB_BV_03_I	N/A
+TC_MCE_MMB_BV_19_I	N/A
+TC_MCE_MMB_BV_04_I	N/A
+TC_MCE_MMB_BV_17_I	N/A
+TC_MCE_MMB_BV_06_I	N/A
+TC_MCE_MMB_BV_07_I	N/A
+TC_MCE_MMB_BV_08_I	N/A
+TC_MCE_MMD_BV_01_I	N/A
+TC_MCE_MMU_BV_01_I	N/A
+TC_MCE_MMN_BV_01_I	N/A
+TC_MCE_MMN_BV_03_I	N/A
+TC_MCE_MMI_BV_01_I	N/A
+TC_MCE_MFB_BV_01_I	N/A
+TC_MCE_MFB_BV_03_I	N/A
+TC_MCE_MFB_BV_04_I	N/A
+TC_MCE_BC_BV_02_I	N/A
+TC_MCE_BC_BV_04_I	N/A
+TC_MCE_CON_BV_01_I	N/A
+TC_MCE_CON_BV_02_I	N/A
+TC_MCE_ROB_BV_01_I	N/A
+TC_MCE_SRM_BV_03_I	N/A
+TC_MCE_SRM_BV_07_I	N/A
+TC_MCE_SRMP_BI_01_I	N/A
+TC_MCE_SRMP_BV_01_I	N/A
+TC_MCE_SRMP_BV_04_I	N/A
+TC_MCE_SRMP_BV_05_I	N/A
+TC_MCE_SRMP_BV_06_I	N/A
+TC_MSE_MSM_BV_05_I	PASS
+TC_MSE_MSM_BV_06_I	PASS
+TC_MSE_MSM_BV_07_I	PASS
+TC_MSE_MSM_BV_08_I	PASS
+TC_MSE_MSM_BV_09_I	N/A
+TC_MSE_MSM_BV_10_I	N/A
+TC_MSE_MSM_BV_11_I	N/A
+TC_MSE_MSM_BV_12_I	N/A
+TC_MSE_MNR_BV_03_I	PASS
+TC_MSE_MNR_BV_04_I	PASS
+TC_MSE_MMB_BV_09_I	PASS
+TC_MSE_MMB_BV_10_I	PASS
+TC_MSE_MMB_BV_11_I	PASS
+TC_MSE_MMB_BV_20_I	PASS
+TC_MSE_MMB_BV_12_I	N/A
+TC_MSE_MMB_BV_18_I	N/A
+TC_MSE_MMB_BV_13_I	PASS
+TC_MSE_MMB_BV_14_I	PASS
+TC_MSE_MMB_BV_15_I	PASS
+TC_MSE_MMB_BV_16_I	INC	Mailbox MAS instance not available on IUT.
+TC_MSE_MMD_BV_02_I	PASS
+TC_MSE_MMU_BV_02_I	PASS
+TC_MSE_MMU_BV_03_I	PASS
+TC_MSE_MMN_BV_02_I	PASS
+TC_MSE_MMN_BV_04_I	N/A
+TC_MSE_MMI_BV_02_I	N/A
+TC_MSE_MFB_BV_02_I	N/A
+TC_MSE_MFB_BV_05_I	N/A
+TC_MSE_BC_BV_01_I	N/A
+TC_MSE_BC_BV_03_I	N/A
+TC_MSE_CON_BV_01_I	N/A
+TC_MSE_CON_BV_02_I	N/A
+TC_MSE_ROB_BV_01_I	N/A
+TC_MSE_SRM_BI_02_I	N/A
+TC_MSE_SRM_BI_03_I	N/A
+TC_MSE_SRM_BI_05_I	N/A
+TC_MSE_SRM_BV_04_I	N/A
+TC_MSE_SRM_BV_08_I	N/A
+TC_MSE_SRMP_BI_02_I	N/A
+TC_MSE_SRMP_BV_02_I	N/A
+TC_MSE_SRMP_BV_03_I	N/A
+-------------------------------------------------------------------------------
-- 
1.8.5


^ permalink raw reply related

* Re: [PATCH v2 bluetooth-next 1/7] 6lowpan: fix udp nullpointer dereferencing
From: Anderson Lizardo @ 2013-12-17 10:58 UTC (permalink / raw)
  To: Alexander Aring; +Cc: linux-zigbee-devel, werner, BlueZ development
In-Reply-To: <1387276373-23882-2-git-send-email-alex.aring@gmail.com>

Hi Alexander,

On Tue, Dec 17, 2013 at 6:32 AM, Alexander Aring <alex.aring@gmail.com> wrote:
> Sometimes a nullpointer dereferencing occurs because of using a wrong
> pointer arithmetic in udp_uncompression.
>
> This patch changes "**(hc06_ptr + 3)" to the right one "*(*hc06_ptr +
> 3)". Dereferencing like "**(hc06_ptr + 3)" works in a random case only.

And why not use hc06_ptr[0][3] ? IMHO it is more readable and the
arithmetic is the same (as far as I know).

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

^ permalink raw reply

* Re: [PATCH v2 bluetooth-next 1/7] 6lowpan: fix udp nullpointer dereferencing
From: Alexander Aring @ 2013-12-17 11:06 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-zigbee-devel, werner, BlueZ development
In-Reply-To: <CAJdJm_MC7DOYZ_nf-Sv6c61s0X-i2yY-7chgd5gA9ScFvVkRBg@mail.gmail.com>

On Tue, Dec 17, 2013 at 06:58:12AM -0400, Anderson Lizardo wrote:
> Hi Alexander,
> 
> On Tue, Dec 17, 2013 at 6:32 AM, Alexander Aring <alex.aring@gmail.com> wrote:
> > Sometimes a nullpointer dereferencing occurs because of using a wrong
> > pointer arithmetic in udp_uncompression.
> >
> > This patch changes "**(hc06_ptr + 3)" to the right one "*(*hc06_ptr +
> > 3)". Dereferencing like "**(hc06_ptr + 3)" works in a random case only.
> 
> And why not use hc06_ptr[0][3] ? IMHO it is more readable and the
> arithmetic is the same (as far as I know).
>

mhh maybe we change it to *hc06_ptr[3] ? Otherwise we have always
something like [0][#] for access.

- Alex

^ permalink raw reply

* [PATCH] Bluetooth: Fix HCI User Channel permission check in hci_sock_sendmsg
From: Marcel Holtmann @ 2013-12-17 11:21 UTC (permalink / raw)
  To: linux-bluetooth

The HCI User Channel is an admin operation which enforces CAP_NET_ADMIN
when binding the socket. Problem now is that it then requires also
CAP_NET_RAW when calling into hci_sock_sendmsg. This is not intended
and just an oversight since general HCI sockets (which do not require
special permission to bind) and HCI User Channel share the same code
path here.

Remove the extra CAP_NET_RAW check for HCI User Channel write operation
since the permission check has already been enforced when binding the
socket. This also makes it possible to open HCI User Channel from a
privileged process and then hand the file descriptor to an unprivilged
process.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/hci_sock.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 6a6c8bb4fd72..7552f9e3089c 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -940,8 +940,22 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 	bt_cb(skb)->pkt_type = *((unsigned char *) skb->data);
 	skb_pull(skb, 1);
 
-	if (hci_pi(sk)->channel == HCI_CHANNEL_RAW &&
-	    bt_cb(skb)->pkt_type == HCI_COMMAND_PKT) {
+	if (hci_pi(sk)->channel == HCI_CHANNEL_USER) {
+		/* No permission check is needed for user channel
+		 * since that gets enforced when binding the socket.
+		 *
+		 * However check that the packet type is valid.
+		 */
+		if (bt_cb(skb)->pkt_type != HCI_COMMAND_PKT &&
+		    bt_cb(skb)->pkt_type != HCI_ACLDATA_PKT &&
+		    bt_cb(skb)->pkt_type != HCI_SCODATA_PKT) {
+			err = -EINVAL;
+			goto drop;
+		}
+
+		skb_queue_tail(&hdev->raw_q, skb);
+		queue_work(hdev->workqueue, &hdev->tx_work);
+	} else if (bt_cb(skb)->pkt_type == HCI_COMMAND_PKT) {
 		u16 opcode = get_unaligned_le16(skb->data);
 		u16 ogf = hci_opcode_ogf(opcode);
 		u16 ocf = hci_opcode_ocf(opcode);
@@ -972,14 +986,6 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 			goto drop;
 		}
 
-		if (hci_pi(sk)->channel == HCI_CHANNEL_USER &&
-		    bt_cb(skb)->pkt_type != HCI_COMMAND_PKT &&
-		    bt_cb(skb)->pkt_type != HCI_ACLDATA_PKT &&
-		    bt_cb(skb)->pkt_type != HCI_SCODATA_PKT) {
-			err = -EINVAL;
-			goto drop;
-		}
-
 		skb_queue_tail(&hdev->raw_q, skb);
 		queue_work(hdev->workqueue, &hdev->tx_work);
 	}
-- 
1.8.3.1


^ permalink raw reply related


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