This patch set add support for NAP role. It register NAP server and create
bnep bridge and listen for incoming connections from client devices.
On incoming connection request it accepts connection, creates bnep interface
and notifies control state and connection state infromation. Removes device
on disconnect request. Android related changes are required to enable this
role. Patches sent to respective ML.
Ravi kumar Veeramally (4):
android/pan: Register Network Access Point
android/pan: Listen for incoming connections and accept in NAP role
android/pan: Implement PAN enable HAL api at daemon side
android/pan: Remove connected PAN devices on profile unregister call
android/pan.c | 349 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 337 insertions(+), 12 deletions(-)
--
1.8.3.2
Hi Johan,
On 01/07/2014 12:18 PM, Johan Hedberg wrote:
> Hi Ravi,
>
> On Tue, Jan 07, 2014, Ravi kumar Veeramally wrote:
>>>> @@ -49,7 +54,7 @@
>>>> static bdaddr_t adapter_addr;
>>>> GSList *devices = NULL;
>>>> uint8_t local_role = HAL_PAN_ROLE_NONE;
>>>> -static uint32_t record_id = 0;
>>>> +char bridge[5] = "bnep\0";
>>> This last line raises several questions. Firstly, C-strings have an
>>> implicit nul-character at the end so no need to explicitly try to add
>>> one there. You also don't need to have an explicit size between the
>>> square brackets since this is automatically calculated if you do
>>> initialization upon declaration. Why isn't this static? Why isn't it
>>> const? Would a simple define make more sense instead of a dedicated
>>> variable?
>> I tried these but there are some warnings.
>>
>> #define BNEP_BRIDGE "bnep"
>> static char bridge[] = "bnep";
>> static char *bridge = "bnep";
>>
>> ==10198== Warning: noted but unhandled ioctl 0x89a1 with no
>> size/direction hints
>> ==10198== This could cause spurious value errors to appear.
>> ==10198== See README_MISSING_SYSCALL_OR_IOCTL for guidance on
>> writing a proper wrapper.
> Is this from valgrind?
Yes.
> If so, then I think it's fine to ignore it as it
> simply doesn't know the details of all ioctls. We get this kind of
> stuff for Bluetooth specific ioctls too. I.e. go with whatever is the
> simplest (probably the define).
Ok, I will change to define and send you v2.
Regards,
Ravi.
Hi Ravi,
On Tue, Jan 07, 2014, Ravi kumar Veeramally wrote:
> >>@@ -49,7 +54,7 @@
> >> static bdaddr_t adapter_addr;
> >> GSList *devices = NULL;
> >> uint8_t local_role = HAL_PAN_ROLE_NONE;
> >>-static uint32_t record_id = 0;
> >>+char bridge[5] = "bnep\0";
> >This last line raises several questions. Firstly, C-strings have an
> >implicit nul-character at the end so no need to explicitly try to add
> >one there. You also don't need to have an explicit size between the
> >square brackets since this is automatically calculated if you do
> >initialization upon declaration. Why isn't this static? Why isn't it
> >const? Would a simple define make more sense instead of a dedicated
> >variable?
>
> I tried these but there are some warnings.
>
> #define BNEP_BRIDGE "bnep"
> static char bridge[] = "bnep";
> static char *bridge = "bnep";
>
> ==10198== Warning: noted but unhandled ioctl 0x89a1 with no
> size/direction hints
> ==10198== This could cause spurious value errors to appear.
> ==10198== See README_MISSING_SYSCALL_OR_IOCTL for guidance on
> writing a proper wrapper.
Is this from valgrind? If so, then I think it's fine to ignore it as it
simply doesn't know the details of all ioctls. We get this kind of
stuff for Bluetooth specific ioctls too. I.e. go with whatever is the
simplest (probably the define).
Johan
Hi Johan,
On 01/07/2014 10:59 AM, Johan Hedberg wrote:
> Hi Ravi,
>
> On Mon, Jan 06, 2014, Ravi kumar Veeramally wrote:
>> Register NAP server and adds bnep bridge. Removes bridge
>> on destroy call. Bridge mechanism is needed when device acting
>> as a server and listen for incoming connections.
>> ---
>> android/pan.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 112 insertions(+), 4 deletions(-)
>>
>> diff --git a/android/pan.c b/android/pan.c
>> index 38e353d..93f712f 100644
>> --- a/android/pan.c
>> +++ b/android/pan.c
>> @@ -28,6 +28,11 @@
>> #include <unistd.h>
>> #include <fcntl.h>
>> #include <glib.h>
>> +#include <sys/ioctl.h>
>> +#include <sys/socket.h>
>> +#include <sys/wait.h>
>> +#include <net/if.h>
>> +#include <linux/sockios.h>
>>
>> #include "btio/btio.h"
>> #include "lib/bluetooth.h"
>> @@ -49,7 +54,7 @@
>> static bdaddr_t adapter_addr;
>> GSList *devices = NULL;
>> uint8_t local_role = HAL_PAN_ROLE_NONE;
>> -static uint32_t record_id = 0;
>> +char bridge[5] = "bnep\0";
> This last line raises several questions. Firstly, C-strings have an
> implicit nul-character at the end so no need to explicitly try to add
> one there. You also don't need to have an explicit size between the
> square brackets since this is automatically calculated if you do
> initialization upon declaration. Why isn't this static? Why isn't it
> const? Would a simple define make more sense instead of a dedicated
> variable?
I tried these but there are some warnings.
#define BNEP_BRIDGE "bnep"
static char bridge[] = "bnep";
static char *bridge = "bnep";
==10198== Warning: noted but unhandled ioctl 0x89a1 with no
size/direction hints
==10198== This could cause spurious value errors to appear.
==10198== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing
a proper wrapper.
char bridge[5] = "bnep\0"; worked fine (yes, like you said static can be used).
Regards,
Ravi.
Hi Ravi,
On Mon, Jan 06, 2014, Ravi kumar Veeramally wrote:
> Register NAP server and adds bnep bridge. Removes bridge
> on destroy call. Bridge mechanism is needed when device acting
> as a server and listen for incoming connections.
> ---
> android/pan.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 112 insertions(+), 4 deletions(-)
>
> diff --git a/android/pan.c b/android/pan.c
> index 38e353d..93f712f 100644
> --- a/android/pan.c
> +++ b/android/pan.c
> @@ -28,6 +28,11 @@
> #include <unistd.h>
> #include <fcntl.h>
> #include <glib.h>
> +#include <sys/ioctl.h>
> +#include <sys/socket.h>
> +#include <sys/wait.h>
> +#include <net/if.h>
> +#include <linux/sockios.h>
>
> #include "btio/btio.h"
> #include "lib/bluetooth.h"
> @@ -49,7 +54,7 @@
> static bdaddr_t adapter_addr;
> GSList *devices = NULL;
> uint8_t local_role = HAL_PAN_ROLE_NONE;
> -static uint32_t record_id = 0;
> +char bridge[5] = "bnep\0";
This last line raises several questions. Firstly, C-strings have an
implicit nul-character at the end so no need to explicitly try to add
one there. You also don't need to have an explicit size between the
square brackets since this is automatically calculated if you do
initialization upon declaration. Why isn't this static? Why isn't it
const? Would a simple define make more sense instead of a dedicated
variable?
Johan
---
android/pan.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/android/pan.c b/android/pan.c
index 499702f..b482d55 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -763,10 +763,20 @@ bool bt_pan_register(const bdaddr_t *addr)
return true;
}
+static void pan_device_disconnected(gpointer data, gpointer user_data)
+{
+ struct pan_device *dev = data;
+
+ bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
+}
+
void bt_pan_unregister(void)
{
DBG("");
+ g_slist_foreach(devices, pan_device_disconnected, NULL);
+ devices = NULL;
+
bnep_cleanup();
ipc_unregister(HAL_SERVICE_ID_PAN);
--
1.8.3.2
---
android/pan.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/android/pan.c b/android/pan.c
index 87753a0..499702f 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -588,18 +588,38 @@ static void bt_pan_enable(const void *buf, uint16_t len)
{
const struct hal_cmd_pan_enable *cmd = buf;
uint8_t status;
+ int err;
+
+ DBG("");
+
+ if (local_role == cmd->local_role) {
+ status = HAL_STATUS_SUCCESS;
+ goto reply;
+ }
+
+ /* destroy existing server */
+ destroy_nap_device();
switch (cmd->local_role) {
case HAL_PAN_ROLE_PANU:
- case HAL_PAN_ROLE_NAP:
- DBG("Not Implemented");
- status = HAL_STATUS_FAILED;
- break;
- default:
status = HAL_STATUS_UNSUPPORTED;
- break;
+ goto reply;
+ case HAL_PAN_ROLE_NONE:
+ status = HAL_STATUS_SUCCESS;
+ goto reply;
+ }
+
+ local_role = cmd->local_role;
+ err = register_nap_server();
+ if (err < 0) {
+ status = HAL_STATUS_FAILED;
+ destroy_nap_device();
+ goto reply;
}
+ status = HAL_STATUS_SUCCESS;
+
+reply:
ipc_send_rsp(HAL_SERVICE_ID_PAN, HAL_OP_PAN_ENABLE, status);
}
--
1.8.3.2
Listen for incoming connections and accept it. Create bnep interface
add it to bridge and notify control and connection state information
through HAL. Remove the device on disconnect request. If android
settings UI does not have bluetooth tethering enabled it immediately
sends disconnect signal.
---
android/pan.c | 191 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 189 insertions(+), 2 deletions(-)
diff --git a/android/pan.c b/android/pan.c
index 93f712f..87753a0 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -63,12 +63,17 @@ struct pan_device {
uint8_t role;
GIOChannel *io;
struct bnep *session;
+ guint watch;
};
static struct {
uint32_t record_id;
+ guint watch;
+ GIOChannel *io;
} nap_dev = {
.record_id = 0,
+ .watch = 0,
+ .io = NULL,
};
static int device_cmp(gconstpointer s, gconstpointer user_data)
@@ -81,13 +86,21 @@ static int device_cmp(gconstpointer s, gconstpointer user_data)
static void pan_device_free(struct pan_device *dev)
{
+ if (dev->watch > 0) {
+ bnep_server_delete(bridge, dev->iface, &dev->dst);
+ g_source_remove(dev->watch);
+ dev->watch = 0;
+ }
+
if (dev->io) {
g_io_channel_shutdown(dev->io, FALSE, NULL);
g_io_channel_unref(dev->io);
dev->io = NULL;
}
- bnep_free(dev->session);
+ if (dev->session)
+ bnep_free(dev->session);
+
devices = g_slist_remove(devices, dev);
g_free(dev);
@@ -298,7 +311,7 @@ static void bt_pan_disconnect(const void *buf, uint16_t len)
dev = l->data;
- if (dev->conn_state == HAL_PAN_STATE_CONNECTED)
+ if (dev->conn_state == HAL_PAN_STATE_CONNECTED && dev->session)
bnep_disconnect(dev->session);
bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
@@ -308,6 +321,153 @@ failed:
ipc_send_rsp(HAL_SERVICE_ID_PAN, HAL_OP_PAN_DISCONNECT, status);
}
+static gboolean nap_watchdog_cb(GIOChannel *chan, GIOCondition cond,
+ gpointer user_data)
+{
+ struct pan_device *dev = user_data;
+
+ DBG("disconnected");
+
+ bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
+
+ return FALSE;
+}
+static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
+ gpointer user_data)
+{
+ struct pan_device *dev = user_data;
+ 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 sk, n;
+
+ if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
+ error("Hangup or error or inval on BNEP socket");
+ return FALSE;
+ }
+
+ 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);
+ goto failed;
+ }
+
+ /* Highest known control command id BNEP_FILTER_MULT_ADDR_RSP 0x06 */
+ if (req->type == BNEP_CONTROL &&
+ req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
+ error("cmd not understood");
+ bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_CMD_NOT_UNDERSTOOD,
+ req->ctrl);
+ goto failed;
+ }
+
+ if (req->type != BNEP_CONTROL || req->ctrl != BNEP_SETUP_CONN_REQ) {
+ error("cmd is not BNEP_SETUP_CONN_REQ %02X %02X", req->type,
+ req->ctrl);
+ goto failed;
+ }
+
+ rsp = bnep_setup_decode(req, &dst_role, &src_role);
+ if (rsp) {
+ error("bnep_setup_decode failed");
+ goto failed;
+ }
+
+ rsp = bnep_setup_chk(dst_role, src_role);
+ if (rsp) {
+ error("benp_setup_chk failed");
+ goto failed;
+ }
+
+ if (bnep_server_add(sk, dst_role, bridge, dev->iface, &dev->dst) < 0) {
+ error("server_connadd failed");
+ rsp = BNEP_CONN_NOT_ALLOWED;
+ goto failed;
+ }
+
+ rsp = BNEP_SUCCESS;
+ bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_SETUP_CONN_RSP, rsp);
+
+ dev->watch = g_io_add_watch(chan, G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+ nap_watchdog_cb, dev);
+ g_io_channel_unref(dev->io);
+ dev->io = NULL;
+
+ bt_pan_notify_ctrl_state(dev, HAL_PAN_CTRL_ENABLED);
+ bt_pan_notify_conn_state(dev, HAL_PAN_STATE_CONNECTED);
+
+ return FALSE;
+
+failed:
+ bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_SETUP_CONN_RSP, rsp);
+ pan_device_free(dev);
+
+ return FALSE;
+}
+
+static void nap_connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
+{
+ struct pan_device *dev = user_data;
+
+ DBG("");
+
+ if (err) {
+ error("%s", err->message);
+ bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
+ return;
+ }
+
+ g_io_channel_set_close_on_unref(chan, TRUE);
+ dev->watch = g_io_add_watch(chan,
+ G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+ nap_setup_cb, dev);
+}
+
+static void nap_confirm_cb(GIOChannel *chan, gpointer data)
+{
+ struct pan_device *dev = NULL;
+ bdaddr_t dst;
+ char address[18];
+ GError *err = NULL;
+
+ DBG("");
+
+ bt_io_get(chan, &err, BT_IO_OPT_DEST_BDADDR, &dst,
+ BT_IO_OPT_DEST, address, BT_IO_OPT_INVALID);
+ if (err) {
+ error("%s", err->message);
+ g_error_free(err);
+ goto failed;
+ }
+
+ DBG("incoming connect request from %s", address);
+ dev = g_new0(struct pan_device, 1);
+ bacpy(&dev->dst, &dst);
+ local_role = HAL_PAN_ROLE_NAP;
+ dev->role = HAL_PAN_ROLE_PANU;
+
+ dev->io = g_io_channel_ref(chan);
+ g_io_channel_set_close_on_unref(dev->io, TRUE);
+
+ if (!bt_io_accept(dev->io, nap_connect_cb, dev, NULL, &err)) {
+ error("bt_io_accept: %s", err->message);
+ g_error_free(err);
+ goto failed;
+ }
+
+ devices = g_slist_append(devices, dev);
+ bt_pan_notify_conn_state(dev, HAL_PAN_STATE_CONNECTING);
+
+ return;
+
+failed:
+ g_free(dev);
+ bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
+}
+
static int set_forward_delay(void)
{
FILE *f;
@@ -382,10 +542,22 @@ static void destroy_nap_device(void)
nap_remove_bridge();
nap_dev.record_id = 0;
+
+ if (nap_dev.watch > 0) {
+ g_source_remove(nap_dev.watch);
+ nap_dev.watch = 0;
+ }
+
+ if (nap_dev.io) {
+ g_io_channel_shutdown(nap_dev.io, FALSE, NULL);
+ g_io_channel_unref(nap_dev.io);
+ nap_dev.io = NULL;
+ }
}
static int register_nap_server(void)
{
+ GError *gerr;
int err;
DBG("");
@@ -394,6 +566,21 @@ static int register_nap_server(void)
if (err < 0)
return err;
+ nap_dev.io = bt_io_listen(NULL, nap_confirm_cb, NULL, NULL, &gerr,
+ BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
+ BT_IO_OPT_PSM, BNEP_PSM,
+ BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
+ BT_IO_OPT_OMTU, BNEP_MTU,
+ BT_IO_OPT_IMTU, BNEP_MTU,
+ BT_IO_OPT_INVALID);
+
+ if (!nap_dev.io) {
+ destroy_nap_device();
+ error("%s", gerr->message);
+ g_error_free(gerr);
+ return -EINVAL;
+ }
+
return 0;
}
--
1.8.3.2
Register NAP server and adds bnep bridge. Removes bridge
on destroy call. Bridge mechanism is needed when device acting
as a server and listen for incoming connections.
---
android/pan.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 112 insertions(+), 4 deletions(-)
diff --git a/android/pan.c b/android/pan.c
index 38e353d..93f712f 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -28,6 +28,11 @@
#include <unistd.h>
#include <fcntl.h>
#include <glib.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <sys/wait.h>
+#include <net/if.h>
+#include <linux/sockios.h>
#include "btio/btio.h"
#include "lib/bluetooth.h"
@@ -49,7 +54,7 @@
static bdaddr_t adapter_addr;
GSList *devices = NULL;
uint8_t local_role = HAL_PAN_ROLE_NONE;
-static uint32_t record_id = 0;
+char bridge[5] = "bnep\0";
struct pan_device {
char iface[16];
@@ -60,6 +65,12 @@ struct pan_device {
struct bnep *session;
};
+static struct {
+ uint32_t record_id;
+} nap_dev = {
+ .record_id = 0,
+};
+
static int device_cmp(gconstpointer s, gconstpointer user_data)
{
const struct pan_device *dev = s;
@@ -297,6 +308,95 @@ failed:
ipc_send_rsp(HAL_SERVICE_ID_PAN, HAL_OP_PAN_DISCONNECT, status);
}
+static int set_forward_delay(void)
+{
+ FILE *f;
+ char *path;
+
+ path = g_strdup_printf("/sys/class/net/%s/bridge/forward_delay",
+ bridge);
+ if (!path)
+ return -ENOMEM;
+
+ f = fopen(path, "r+");
+ g_free(path);
+ if (!f)
+ return -errno;
+
+ fprintf(f, "%d", 0);
+ fclose(f);
+
+ return 0;
+}
+
+static int nap_create_bridge(void)
+{
+ int sk, err;
+
+ DBG(" %s", bridge);
+
+ sk = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
+ if (sk < 0)
+ return -EOPNOTSUPP;
+
+ if (ioctl(sk, SIOCBRADDBR, bridge) == -1) {
+ err = -errno;
+ if (err != -EEXIST) {
+ close(sk);
+ return -EOPNOTSUPP;
+ }
+ }
+
+ err = set_forward_delay();
+ if (err < 0)
+ ioctl(sk, SIOCBRDELBR, bridge);
+
+ close(sk);
+
+ return err;
+}
+
+static int nap_remove_bridge(void)
+{
+ int sk, err;
+
+ DBG(" %s", bridge);
+
+ sk = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
+ if (sk < 0)
+ return -EOPNOTSUPP;
+
+ err = ioctl(sk, SIOCBRDELBR, bridge);
+ close(sk);
+
+ if (err < 0)
+ return -EOPNOTSUPP;
+
+ return 0;
+}
+
+static void destroy_nap_device(void)
+{
+ DBG("");
+
+ nap_remove_bridge();
+
+ nap_dev.record_id = 0;
+}
+
+static int register_nap_server(void)
+{
+ int err;
+
+ DBG("");
+
+ err = nap_create_bridge();
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
static void bt_pan_enable(const void *buf, uint16_t len)
{
const struct hal_cmd_pan_enable *cmd = buf;
@@ -441,7 +541,15 @@ bool bt_pan_register(const bdaddr_t *addr)
return false;
}
- record_id = rec->handle;
+ err = register_nap_server();
+ if (err < 0) {
+ bt_adapter_remove_record(rec->handle);
+ sdp_record_free(rec);
+ bnep_cleanup();
+ return false;
+ }
+
+ nap_dev.record_id = rec->handle;
ipc_register(HAL_SERVICE_ID_PAN, cmd_handlers,
G_N_ELEMENTS(cmd_handlers));
@@ -455,6 +563,6 @@ void bt_pan_unregister(void)
bnep_cleanup();
ipc_unregister(HAL_SERVICE_ID_PAN);
- bt_adapter_remove_record(record_id);
- record_id = 0;
+ bt_adapter_remove_record(nap_dev.record_id);
+ destroy_nap_device();
}
--
1.8.3.2