Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. [PATCH] dns: Use hash table instead of binary tree (Daniel Wagner)
2. [PATCH] ipv6pd: Remove unused timer_hash (Daniel Wagner)
3. Re: [PATCH v2 1/2] dns: support for systemd-resolved and
backend selection. (Daniel Wagner)
4. Re: [PATCH 1/2] dns: support for systemd-resolved and backend
selection. (Daniel Wagner)
5. Re: [PATCH] session: Add ContextIdentifier (Daniel Wagner)
6. Re: [PATCH 2/4] inet: Implement subnet route
creation/deletion in iproute_default_modify (Daniel Wagner)
7. Re: [PATCH 4/4] session: Use subnet route creation and
deletion APIs (Daniel Wagner)
----------------------------------------------------------------------
Message: 1
Date: Thu, 5 Oct 2017 21:19:56 +0200
From: Daniel Wagner <[email protected]>
To: [email protected]
Cc: Daniel Wagner <[email protected]>
Subject: [PATCH] dns: Use hash table instead of binary tree
Message-ID: <[email protected]>
ConnMan doesn't use the binary tree data structures from glib
anywhere. Since there are still plans to move to ELL which has no
support for a binary tree. Let's not make our live more complex than
necessary.
---
src/dns-systemd-resolved.c | 48 ++++++++++++++++++++++++----------------------
1 file changed, 25 insertions(+), 23 deletions(-)
diff --git a/src/dns-systemd-resolved.c b/src/dns-systemd-resolved.c
index 93d63b5bf0ae..1fb361099f0b 100644
--- a/src/dns-systemd-resolved.c
+++ b/src/dns-systemd-resolved.c
@@ -36,7 +36,7 @@
#define SYSTEMD_RESOLVED_SERVICE "org.freedesktop.resolve1"
#define SYSTEMD_RESOLVED_PATH "/org/freedesktop/resolve1"
-static GTree *interface_map;
+static GHashTable *interface_hash;
static DBusConnection *connection;
static GDBusClient *client;
static GDBusProxy *resolved_proxy;
@@ -52,7 +52,7 @@ struct dns_interface {
bool needs_server_update;
};
-static gint int_cmp(gconstpointer a, gconstpointer b, void *data)
+static gint compare_index(gconstpointer a, gconstpointer b)
{
gint ai = GPOINTER_TO_UINT(a);
gint bi = GPOINTER_TO_UINT(b);
@@ -217,7 +217,7 @@ static bool is_empty(struct dns_interface *iface)
return (!iface->domains && !iface->servers);
}
-static gboolean update_interface(gpointer key, gpointer value, gpointer data)
+static void update_interface(gpointer key, gpointer value, gpointer data)
{
struct dns_interface *iface = value;
GList **removed_items = data;
@@ -226,25 +226,25 @@ static gboolean update_interface(gpointer key, gpointer
value, gpointer data)
if (is_empty(iface))
*removed_items = g_list_prepend(*removed_items, iface);
-
- /* don't stop the tree traversal */
- return FALSE;
}
static int update_systemd_resolved(gpointer data)
{
GList *removed_items = NULL, *list;
- if (!interface_map) {
- DBG("no interface map when updating");
+ if (!interface_hash) {
+ DBG("no interface hash when updating");
+
return G_SOURCE_REMOVE;
}
- g_tree_foreach(interface_map, update_interface, &removed_items);
+ g_hash_table_foreach(interface_hash, update_interface, &removed_items);
for (list = removed_items; list; list = g_list_next(list)) {
struct dns_interface *iface = list->data;
- g_tree_remove(interface_map, GUINT_TO_POINTER(iface->index));
+
+ g_hash_table_remove(interface_hash,
+ GUINT_TO_POINTER(iface->index));
}
g_list_free(removed_items);
@@ -293,10 +293,10 @@ int __connman_dnsproxy_remove(int index, const char
*domain,
DBG("%d, %s, %s", index, domain ? domain : "no domain",
server ? server : "no server");
- if (!interface_map || index < 0)
+ if (!interface_hash || index < 0)
return -EINVAL;
- iface = g_tree_lookup(interface_map, GUINT_TO_POINTER(index));
+ iface = g_hash_table_lookup(interface_hash, GUINT_TO_POINTER(index));
if (!iface)
return -EINVAL;
@@ -335,10 +335,10 @@ int __connman_dnsproxy_append(int index, const char
*domain,
DBG("%d, %s, %s", index, domain ? domain : "no domain",
server ? server : "no server");
- if (!interface_map || index < 0)
+ if (!interface_hash || index < 0)
return -EINVAL;
- iface = g_tree_lookup(interface_map, GUINT_TO_POINTER(index));
+ iface = g_hash_table_lookup(interface_hash, GUINT_TO_POINTER(index));
if (!iface) {
iface = g_new0(struct dns_interface, 1);
@@ -346,7 +346,7 @@ int __connman_dnsproxy_append(int index, const char *domain,
return -ENOMEM;
iface->index = index;
- g_tree_insert(interface_map, GUINT_TO_POINTER(index), iface);
+ g_hash_table_replace(interface_hash, GUINT_TO_POINTER(index),
iface);
}
if (domain) {
@@ -405,13 +405,15 @@ int __connman_dnsproxy_init(void)
DBG("");
- if ((ret = setup_resolved()) < 0)
+ ret = setup_resolved();
+ if (!ret)
return ret;
- interface_map = g_tree_new_full(int_cmp, NULL, NULL,
- free_dns_interface);
-
- if (!interface_map)
+ interface_hash = g_hash_table_new_full(g_direct_hash,
+ compare_index,
+ NULL,
+ free_dns_interface);
+ if (!interface_hash)
return -ENOMEM;
return 0;
@@ -431,9 +433,9 @@ void __connman_dnsproxy_cleanup(void)
update_interfaces_source = 0;
}
- if (interface_map) {
- g_tree_destroy(interface_map);
- interface_map = NULL;
+ if (interface_hash) {
+ g_hash_table_destroy(interface_hash);
+ interface_hash = NULL;
}
if (resolved_proxy) {
--
2.9.5
------------------------------
Message: 2
Date: Thu, 5 Oct 2017 21:20:14 +0200
From: Daniel Wagner <[email protected]>
To: [email protected]
Cc: Daniel Wagner <[email protected]>
Subject: [PATCH] ipv6pd: Remove unused timer_hash
Message-ID: <[email protected]>
There is no user of timer_hash, hence remove it.
---
src/ipv6pd.c | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/src/ipv6pd.c b/src/ipv6pd.c
index 0d221f07c05e..87da3d8139c3 100644
--- a/src/ipv6pd.c
+++ b/src/ipv6pd.c
@@ -41,7 +41,6 @@ static guint timer_uplink;
static guint timer_ra;
static char *default_interface;
static GSList *prefixes;
-static GHashTable *timer_hash;
static void *rs_context;
static int setup_prefix_delegation(struct connman_service *service);
@@ -165,11 +164,6 @@ static void cleanup(void)
timer_uplink = 0;
}
- if (timer_hash) {
- g_hash_table_destroy(timer_hash);
- timer_hash = NULL;
- }
-
if (prefixes) {
g_slist_free_full(prefixes, g_free);
prefixes = NULL;
@@ -260,13 +254,6 @@ static int setup_prefix_delegation(struct connman_service
*service)
return err;
}
-static void cleanup_timer(gpointer user_data)
-{
- guint timer = GPOINTER_TO_UINT(user_data);
-
- g_source_remove(timer);
-}
-
static void update_default_interface(struct connman_service *service)
{
setup_prefix_delegation(service);
@@ -333,9 +320,6 @@ int __connman_ipv6pd_setup(const char *bridge)
bridge_index = connman_inet_ifindex(bridge);
- timer_hash = g_hash_table_new_full(g_direct_hash, g_direct_equal,
- NULL, cleanup_timer);
-
err = __connman_inet_ipv6_start_recv_rs(bridge_index, rs_received,
NULL, &rs_context);
if (err < 0)
--
2.9.5
------------------------------
Message: 3
Date: Thu, 5 Oct 2017 21:21:14 +0200
From: Daniel Wagner <[email protected]>
To: Ismo Puustinen <[email protected]>, [email protected]
Subject: Re: [PATCH v2 1/2] dns: support for systemd-resolved and
backend selection.
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Ismo,
On 10/05/2017 03:50 PM, Ismo Puustinen wrote:
> During configure, use --with-dns-backend to declare which DNS backend
> you want to use during build. The two options available are "internal"
> and "systemd-resolved".
>
> The internal backend works as previously. The systemd-resolved backend
> configures systemd-resolved over D-Bus.
>
> The "-r" command line option for connmand still works. It means that the
> DNS servers (either internal DNS proxy or systemd-resolved) aren't used,
> and instead connmand just updates /etc/resolv.conf file.
I applied both patches with some small whitespace changes.
Thanks,
Daniel
------------------------------
Message: 4
Date: Thu, 5 Oct 2017 21:25:20 +0200
From: Daniel Wagner <[email protected]>
To: "Puustinen, Ismo" <[email protected]>
Cc: "[email protected]" <[email protected]>
Subject: Re: [PATCH 1/2] dns: support for systemd-resolved and backend
selection.
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Ismo,
10/05/2017 03:31 PM, Puustinen, Ismo wrote:
>> connmand[22701]: wlp2s0b1 {add} address 192.168.154.54/24 label wlp2s0b1
>> family 2
>> connmand[22701]: src/dns-systemd-resolved.c:__connman_dnsproxy_remove()
>> connmand[22701]: src/dns-systemd-resolved.c:setlinkdns_append() index: 3,
>> server: 192.168.154.1
>> connmand[22701]: src/dns-systemd-resolved.c:setlinkdomains_append() index:
>> 3, domain: localdomain
>> connmand[22701]: src/dns-systemd-resolved.c:__connman_dnsproxy_remove()
>> connmand[22701]: src/dns-systemd-resolved.c:setlinkdns_append() index: 3,
>> server: 192.168.154.1
>> connmand[22701]: src/dns-systemd-resolved.c:setlinkdomains_append() index:
>> 3, domain: localdomain
>> connmand[22701]: src/dns-systemd-resolved.c:__connman_dnsproxy_append()
>> connmand[22701]: src/dns-systemd-resolved.c:setlinkdns_append() index: 3,
>> server: 192.168.154.1
>> connmand[22701]: src/dns-systemd-resolved.c:setlinkdomains_append() index:
>> 3, domain: localdomain
>> connmand[22701]: src/dns-systemd-resolved.c:__connman_dnsproxy_append()
>> connmand[22701]: src/dns-systemd-resolved.c:setlinkdns_append() index: 3,
>> server: 192.168.154.1
>> connmand[22701]: src/dns-systemd-resolved.c:setlinkdomains_append() index:
>> 3, domain: localdomain
>>
>>
>> Is this supposed to be like this? I mean appending two times the server for
>> interface with index 3?
>
> Unfortunately I'm not sure. The __connman_dnsproxy_append() calls
> reflect what Connman thinks is going on. I added logging so that
> we'll know for sure the parameters to the function call. I also fixed
> a bug when removing DNS servers (I was comparing pointers instead of
> strings, so nothing got removed). Another change is that I moved
> sending the DNS D-Bus messages to the idle loop -- this means that we
> send only one message when __append and __remove have been called for
> multiple times. I'm posting a revised patch set shortly.
One more thing I noticed: When I disconnect a service and reconnect it,
the systemd-resolved is not updated anymore. There seems to be still
a bug in there. I went ahead and applied v2 anyway, because we can fix
those things up in the tree.
> I was thinking that I would do some fixes to the overall DNS code
> after the systemd-resolved work is done. There are some oddities (for
> example, Connman doesn't appear to request search domain removal from
> an interface when Connman shuts down). There's also some dead code
> which we could be removed.
Cleanup patches are highly appreciated.
Thanks,
Daniel
------------------------------
Message: 5
Date: Thu, 5 Oct 2017 21:30:36 +0200
From: Daniel Wagner <[email protected]>
To: Bjoern Thorwirth <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] session: Add ContextIdentifier
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Bjoern,
On 09/28/2017 04:52 PM, Daniel Wagner wrote:
> From: Bjoern Thorwirth <[email protected]>
>
> The calling process, that implements the session API is identified by
> the user ID as it is runs. All processes of the same user share the
> same list of allowed bearers, and the same priority for choosing
> between available bearers.
>
> This extension allows processes to select a service context dependent
> behaviour for which the routing decision is made.
>
> This is an extention to session API interface. Helps to enable a
> service differentiation for processes run by the same user. Allows
> ConnMan to differentiate between bearer usage permissions and the
> respective priorities based on the requested service type.
> ---
>
> Hi,
>
> after an offline discussion with Bjorn, we came to the conclusion that
> my idea with AllowedServices is going into the wrong
> direction. At least for his use case.
>
> Propably I got confused by the name 'Session'. By calling this
> ContextIdentifier we should avoid this confusion. In the end it is
> just an additonal information given by the application to support the
> bearer selection algorithm. Currently, the session code in ConnMan
> doesn't make use of it, but a session plugin can use it. That is what
> Bjorn does.
Patch applied. Happy hacking on your session plugin! Looking forward
when you release it :)
Thanks,
Daniel
------------------------------
Message: 6
Date: Thu, 5 Oct 2017 22:03:07 +0200
From: Daniel Wagner <[email protected]>
To: Jian Liang <[email protected]>, [email protected]
Subject: Re: [PATCH 2/4] inet: Implement subnet route
creation/deletion in iproute_default_modify
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Jian,
On 10/05/2017 12:34 PM, Jian Liang wrote:
> From: Jian Liang <[email protected]>
>
> - Calculate subnet address base on gateway address and prefixlen
> - Differentiate creation of routes to gateway and subnet
>
> Signed-off-by: Jian Liang <[email protected]>
We don't do the SoB in this project.
> ---
> src/inet.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/src/inet.c b/src/inet.c
> index ab8aec8..dc8a97f 100644
> --- a/src/inet.c
> +++ b/src/inet.c
> @@ -2802,6 +2802,9 @@ static int iproute_default_modify(int cmd, uint32_t
> table_id, int ifindex,
> unsigned char buf[sizeof(struct in6_addr)];
> int ret, len;
> int family = connman_inet_check_ipaddress(gateway);
> + char *dst = NULL;
> +
> + DBG("gateway %s prefixlen %u table %u", gateway, prefixlen, table_id);
What about "gateway %/%u" ?
>
> switch (family) {
> case AF_INET:
> @@ -2814,7 +2817,20 @@ static int iproute_default_modify(int cmd, uint32_t
> table_id, int ifindex,
> return -EINVAL;
> }
>
> - ret = inet_pton(family, gateway, buf);
> + if (prefixlen) {
> + struct in_addr ipv4_subnet_addr, ipv4_mask;
> +
> + memset(&ipv4_subnet_addr, 0, sizeof(ipv4_subnet_addr));
> + ipv4_mask.s_addr = htonl((0xffffffff << (32 - prefixlen)) &
> 0xffffffff);
> + ipv4_subnet_addr.s_addr = inet_addr(gateway);
> + ipv4_subnet_addr.s_addr &= ipv4_mask.s_addr;
> +
> + DBG("subnet %s", inet_ntoa(ipv4_subnet_addr));
Is this DBG() really helpful? It looks like a low level debug statement.
> + dst = g_strdup(inet_ntoa(ipv4_subnet_addr));
> + }
> +
> + ret = inet_pton(family, dst ? dst : gateway, buf);
> + g_free(dst);
> if (ret <= 0)
> return -EINVAL;
>
> @@ -2831,8 +2847,9 @@ static int iproute_default_modify(int cmd, uint32_t
> table_id, int ifindex,
> rth.req.u.r.rt.rtm_type = RTN_UNICAST;
> rth.req.u.r.rt.rtm_dst_len = prefixlen;
>
> - __connman_inet_rtnl_addattr_l(&rth.req.n, sizeof(rth.req), RTA_GATEWAY,
> - buf, len);
> + __connman_inet_rtnl_addattr_l(&rth.req.n, sizeof(rth.req),
> + prefixlen > 0 ? RTA_DST : RTA_GATEWAY, buf, len);
> +
> if (table_id < 256) {
> rth.req.u.r.rt.rtm_table = table_id;
> } else {
>
Thanks,
Daniel
------------------------------
Message: 7
Date: Thu, 5 Oct 2017 22:07:56 +0200
From: Daniel Wagner <[email protected]>
To: Jian Liang <[email protected]>
Cc: [email protected], Jian Liang <[email protected]>
Subject: Re: [PATCH 4/4] session: Use subnet route creation and
deletion APIs
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Jian,
On 10/05/2017 12:34 PM, Jian Liang wrote:
> From: Jian Liang <[email protected]>
>
> Subnet route needs to be create after the default route in order to have
> the gateway information ready. It needs to be deleted before default route
> to the gateway is deleted for the same reason.
>
> Signed-off-by: Jian Liang <[email protected]>
Not needed
> ---
> src/session.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/src/session.c b/src/session.c
> index 965ac06..e43411e 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -367,6 +367,24 @@ static void del_default_route(struct connman_session
> *session)
> session->index = -1;
> }
>
> +static void del_default_subnet_route(struct connman_session *session)
> +{
> + struct connman_ipconfig *ipconfig;
> +
> + if (!session->gateway)
> + return;
> +
> + ipconfig = __connman_service_get_ip4config(session->service);
> +
> + DBG("index %d routing table %d default gateway %s prefixlen %u",
> + session->index, session->mark, session->gateway,
> + __connman_ipconfig_get_prefixlen(ipconfig));
> +
> + __connman_inet_del_subnet_from_table(session->mark,
> + session->index, session->gateway,
> +
> __connman_ipconfig_get_prefixlen(ipconfig));
Please move the __connman_inet_del_subnet_from_table call into the
del_default_route() function. We are going to call both function
back-to-back and there is much code in common.
> +}
> +
> static void add_default_route(struct connman_session *session)
> {
> struct connman_ipconfig *ipconfig;
> @@ -392,6 +410,33 @@ static void add_default_route(struct connman_session
> *session)
> DBG("session %p %s", session, strerror(-err));
> }
>
> +static void add_default_subnet_route(struct connman_session *session)
> +{
> + struct connman_ipconfig *ipconfig;
> + int err;
> + struct in_addr addr = { INADDR_ANY };
> +
> + if (!session->service)
> + return;
> +
> + ipconfig = __connman_service_get_ip4config(session->service);
> + session->index = __connman_ipconfig_get_index(ipconfig);
> + session->gateway = g_strdup(__connman_ipconfig_get_gateway(ipconfig));
> +
> + if (!session->gateway)
> + session->gateway = g_strdup(inet_ntoa(addr));
> +
> + DBG("index %d routing table %d default gateway %s prefixlen %u",
> + session->index, session->mark, session->gateway,
> + __connman_ipconfig_get_prefixlen(ipconfig));
What about "gateway %s/%u" ?
> +
> + err = __connman_inet_add_subnet_to_table(session->mark,
> + session->index, session->gateway,
> + __connman_ipconfig_get_prefixlen(ipconfig));
Same here. Just move it to add_default_route().
> + if (err < 0)
> + DBG("session add subnet route %p %s", session, strerror(-err));
> +}
> +
> static void del_nat_rules(struct connman_session *session)
> {
> struct fw_snat *fw_snat;
> @@ -464,6 +509,7 @@ static void cleanup_routing_table(struct connman_session
> *session)
> session->policy_routing = false;
> }
>
> + del_default_subnet_route(session);
> del_default_route(session);
> }
>
> @@ -478,6 +524,7 @@ static void update_routing_table(struct connman_session
> *session)
> cleanup_routing_table(session);
> init_routing_table(session);
> add_default_route(session);
> + add_default_subnet_route(session);
> }
>
> static void cleanup_nat_rules(struct connman_session *session)
>
Thanks,
Daniel
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 24, Issue 9
**************************************
