Hi,
After looking at connman's bug https://bugs.meego.com/show_bug.cgi?id=25125 I figured out that PAN server code was missbehaving.
Basically, when DBus Unregister call is made: all bnep ifs and there corresponding connections are not touched at all, so on the
client side: it still thinks it's connected (it is, but it lost the tethering stuff and so on).
So here is a patch set that fixes the issue.
1 - it tracks the bnep interface per session, it will be necessary afterwards
2 - An helper to remove properly an interface from a bridge, it will be necessary afterwards also
3 - Here we are: it deletes the interface from the bridge, and puts it down when the server is freed. It's on its own function since it will be
necessary aft
erwards.
4 - It kills the underlying connection properly so the client is notified as it should
5 - Finally, when Unregister method call is made: it cleans up all sessions so clients are disconnected accordingly etc...
Note: I tested this patch against 4.101 and it worken properly.
Please review,
Tomasz Bursztyka (5):
network: Keep track of session's interface name in server
network: Add helper function to remove an interface from a bridge
network: Release session's interface from bridge when unregistering
network: Kill underlying session's connection before freeing it
network: Remove sessions from server on DBus call Unregister
profiles/network/common.c | 29 +++++++++++++++++++++++++++++
profiles/network/common.h | 1 +
profiles/network/server.c | 31 ++++++++++++++++++++++++++++---
3 files changed, 58 insertions(+), 3 deletions(-)
--
1.7.8.6
Hi Tomasz,
On Tue, Aug 21, 2012 at 1:45 PM, Tomasz Bursztyka
<[email protected]> wrote:
> Hi,
>
> After looking at connman's bug https://bugs.meego.com/show_bug.cgi?id=25125 I figured out that PAN server code was missbehaving.
> Basically, when DBus Unregister call is made: all bnep ifs and there corresponding connections are not touched at all, so on the
> client side: it still thinks it's connected (it is, but it lost the tethering stuff and so on).
>
> So here is a patch set that fixes the issue.
>
> 1 - it tracks the bnep interface per session, it will be necessary afterwards
> 2 - An helper to remove properly an interface from a bridge, it will be necessary afterwards also
> 3 - Here we are: it deletes the interface from the bridge, and puts it down when the server is freed. It's on its own function since it will be
> necessary aft
> erwards.
> 4 - It kills the underlying connection properly so the client is notified as it should
> 5 - Finally, when Unregister method call is made: it cleans up all sessions so clients are disconnected accordingly etc...
>
> Note: I tested this patch against 4.101 and it worken properly.
>
> Please review,
>
>
> Tomasz Bursztyka (5):
> network: Keep track of session's interface name in server
> network: Add helper function to remove an interface from a bridge
> network: Release session's interface from bridge when unregistering
> network: Kill underlying session's connection before freeing it
> network: Remove sessions from server on DBus call Unregister
>
> profiles/network/common.c | 29 +++++++++++++++++++++++++++++
> profiles/network/common.h | 1 +
> profiles/network/server.c | 31 ++++++++++++++++++++++++++++---
> 3 files changed, 58 insertions(+), 3 deletions(-)
>
> --
> 1.7.8.6
All 5 patches are now upstream, thanks.
--
Luiz Augusto von Dentz
---
profiles/network/common.c | 29 +++++++++++++++++++++++++++++
profiles/network/common.h | 1 +
2 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/profiles/network/common.c b/profiles/network/common.c
index a223685..da493c1 100644
--- a/profiles/network/common.c
+++ b/profiles/network/common.c
@@ -268,3 +268,32 @@ int bnep_add_to_bridge(const char *devname, const char *bridge)
return 0;
}
+
+int bnep_del_from_bridge(const char *devname, const char *bridge)
+{
+ int ifindex = if_nametoindex(devname);
+ struct ifreq ifr;
+ int sk, err;
+
+ if (!devname || !bridge)
+ return -EINVAL;
+
+ sk = socket(AF_INET, SOCK_STREAM, 0);
+ if (sk < 0)
+ return -1;
+
+ memset(&ifr, 0, sizeof(ifr));
+ strncpy(ifr.ifr_name, bridge, IFNAMSIZ - 1);
+ ifr.ifr_ifindex = ifindex;
+
+ err = ioctl(sk, SIOCBRDELIF, &ifr);
+
+ close(sk);
+
+ if (err < 0)
+ return err;
+
+ info("bridge %s: interface %s removed", bridge, devname);
+
+ return 0;
+}
diff --git a/profiles/network/common.h b/profiles/network/common.h
index cb1f08a..62f2f59 100644
--- a/profiles/network/common.h
+++ b/profiles/network/common.h
@@ -35,3 +35,4 @@ int bnep_connadd(int sk, uint16_t role, char *dev);
int bnep_if_up(const char *devname);
int bnep_if_down(const char *devname);
int bnep_add_to_bridge(const char *devname, const char *bridge);
+int bnep_del_from_bridge(const char *devname, const char *bridge);
--
1.7.8.6
---
profiles/network/server.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 8ae608c..1703f38 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -57,6 +57,7 @@
/* Pending Authorization */
struct network_session {
bdaddr_t dst; /* Remote Bluetooth Address */
+ char dev[16]; /* Interface name */
GIOChannel *io; /* Pending connect channel */
guint watch; /* BNEP socket watch */
};
@@ -272,6 +273,8 @@ static int server_connadd(struct network_server *ns,
bnep_if_up(devname);
+ strncpy(session->dev, devname, sizeof(devname));
+
ns->sessions = g_slist_append(ns->sessions, session);
return 0;
--
1.7.8.6
---
profiles/network/server.c | 24 +++++++++++++++++++++---
1 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 1703f38..5d64ee2 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -586,6 +586,25 @@ static uint32_t register_server_record(struct network_server *ns)
return record->handle;
}
+static void server_remove_sessions(struct network_server *ns)
+{
+ GSList *list;
+
+ for (list = ns->sessions; list; list = list->next) {
+ struct network_session *session = list->data;
+
+ if (*session->dev == '\0')
+ continue;
+
+ bnep_del_from_bridge(session->dev, ns->bridge);
+ bnep_if_down(session->dev);
+ }
+
+ g_slist_free_full(ns->sessions, session_free);
+
+ ns->sessions = NULL;
+}
+
static void server_disconnect(DBusConnection *conn, void *user_data)
{
struct network_server *ns = user_data;
@@ -678,7 +697,8 @@ static void server_free(struct network_server *ns)
if (!ns)
return;
- /* FIXME: Missing release/free all bnepX interfaces */
+ server_remove_sessions(ns);
+
if (ns->record_id)
remove_record_from_server(ns->record_id);
@@ -686,8 +706,6 @@ static void server_free(struct network_server *ns)
g_free(ns->name);
g_free(ns->bridge);
- g_slist_free_full(ns->sessions, session_free);
-
g_free(ns);
}
--
1.7.8.6
---
profiles/network/server.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 1b9989d..cfad893 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -611,6 +611,8 @@ static void server_disconnect(DBusConnection *conn, void *user_data)
{
struct network_server *ns = user_data;
+ server_remove_sessions(ns);
+
ns->watch_id = 0;
if (ns->record_id) {
--
1.7.8.6
---
profiles/network/server.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 5d64ee2..1b9989d 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -598,6 +598,8 @@ static void server_remove_sessions(struct network_server *ns)
bnep_del_from_bridge(session->dev, ns->bridge);
bnep_if_down(session->dev);
+
+ bnep_kill_connection(&session->dst);
}
g_slist_free_full(ns->sessions, session_free);
--
1.7.8.6