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. Re: [PATCH 3/4] inet: Implement APIs for creating and
deleting subnet route (Daniel Wagner)
2. Re: Create subnet route in specific table created by session
(Daniel Wagner)
3. net.connman.Error.InvalidArguments is returned in response to
net.connman.Agent.Error.Canceled (Vasyl Vavrychuk)
4. Re: [PATCH] dns: Use hash table instead of binary tree
(Puustinen, Ismo)
----------------------------------------------------------------------
Message: 1
Date: Thu, 5 Oct 2017 22:09:55 +0200
From: Daniel Wagner <[email protected]>
To: Jian Liang <[email protected]>
Cc: [email protected], Jian Liang <[email protected]>
Subject: Re: [PATCH 3/4] inet: Implement APIs for creating and
deleting subnet route
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]>
>
> Signed-off-by: Jian Liang <[email protected]>
> ---
> src/connman.h | 2 ++
> src/inet.c | 14 ++++++++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/src/connman.h b/src/connman.h
> index 21b7080..7ac179e 100644
> --- a/src/connman.h
> +++ b/src/connman.h
> @@ -240,7 +240,9 @@ int __connman_inet_rtnl_addattr32(struct nlmsghdr *n,
> size_t maxlen,
> int __connman_inet_add_fwmark_rule(uint32_t table_id, int family, uint32_t
> fwmark);
> int __connman_inet_del_fwmark_rule(uint32_t table_id, int family, uint32_t
> fwmark);
> int __connman_inet_add_default_to_table(uint32_t table_id, int ifindex,
> const char *gateway);
> +int __connman_inet_add_subnet_to_table(uint32_t table_id, int ifindex, const
> char *gateway, unsigned char prefixlen);
> int __connman_inet_del_default_from_table(uint32_t table_id, int ifindex,
> const char *gateway);
> +int __connman_inet_del_subnet_from_table(uint32_t table_id, int
> ifindex,const char *gateway, unsigned char prefixlen);
The lines are getting a bit long here. Already 'gateway' is over the
limit. I guess it is time to break all lines here :)
> int __connman_inet_get_address_netmask(int ifindex,
> struct sockaddr_in *address, struct sockaddr_in *netmask);
>
> diff --git a/src/inet.c b/src/inet.c
> index dc8a97f..9fa47de 100644
> --- a/src/inet.c
> +++ b/src/inet.c
> @@ -2881,6 +2881,13 @@ int __connman_inet_add_default_to_table(uint32_t
> table_id, int ifindex,
> return iproute_default_modify(RTM_NEWROUTE, table_id, ifindex, gateway,
> 0);
> }
>
> +int __connman_inet_add_subnet_to_table(uint32_t table_id, int ifindex,
> + const char *gateway, unsigned
> char prefixlen)
> +{
> + /* ip route add 1.2.3.4/24 dev eth0 table 1234 */
> + return iproute_default_modify(RTM_NEWROUTE, table_id, ifindex, gateway,
> prefixlen);
> +}
> +
> int __connman_inet_del_default_from_table(uint32_t table_id, int ifindex,
> const char *gateway)
> {
> @@ -2889,6 +2896,13 @@ int __connman_inet_del_default_from_table(uint32_t
> table_id, int ifindex,
> return iproute_default_modify(RTM_DELROUTE, table_id, ifindex, gateway,
> 0);
> }
>
> +int __connman_inet_del_subnet_from_table(uint32_t table_id, int ifindex,
> + const char *gateway, unsigned
> char prefixlen)
> +{
> + /* ip route del 1.2.3.4/24 dev eth0 table 1234 */
> + return iproute_default_modify(RTM_DELROUTE, table_id, ifindex, gateway,
> prefixlen);
> +}
> +
> int __connman_inet_get_interface_ll_address(int index, int family,
> void *address)
> {
>
Thanks,
Daniel
------------------------------
Message: 2
Date: Thu, 5 Oct 2017 22:11:32 +0200
From: Daniel Wagner <[email protected]>
To: Jian Liang <[email protected]>
Cc: [email protected]
Subject: Re: Create subnet route in specific table created by session
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Hi Jian,
On 10/05/2017 12:34 PM, Jian Liang wrote:
> Currently, in the table created by a session only includes a default
> route to the gateway, e.g.
>
> # ip route show table 256
> default via 192.168.100.1 dev eth0
>
> So all the traffic goes to other host on the same subnet from the
> specified interface still goes to the gateway first. This work is
> to add subnet route to avoid unnecessary extra routing by the
> gateway, e.g.
>
> # ip route show table 256
> default via 192.168.100.1 dev eth0
> 192.168.100.0/24 dev eth0
>
> [PATCH 1/4] inet: Add prefixlen to iproute_default_function
> [PATCH 2/4] inet: Implement subnet route creation/deletion in
> [PATCH 3/4] inet: Implement APIs for creating and deleting subnet
> [PATCH 4/4] session: Use subnet route creation and deletion APIs
>
While testing I managed to trigger this. IIRC I added during
runtime a policy file and recreated a session:
connmand[21327]: ++++++++ backtrace ++++++++
connmand[21327]: #0 0x7f57bfec2950 in /lib64/libc.so.6
connmand[21327]: #1 0x46d36f in __connman_ipconfig_get_prefixlen() at
src/ipconfig.c:1089
connmand[21327]: #2 0x48ec5f in del_default_subnet_route() at
src/session.c:379 (discriminator 1)
connmand[21327]: #3 0x48f1be in cleanup_routing_table() at src/session.c:513
connmand[21327]: #4 0x48f447 in cleanup_session() at src/session.c:586
connmand[21327]: #5 0x7f57c12aa658 in /lib64/libglib-2.0.so.0
connmand[21327]: #6 0x490b7d in session_disconnect() at src/session.c:1343
connmand[21327]: #7 0x491710 in connman_session_destroy() at src/session.c:1681
connmand[21327]: #8 0x4917e9 in __connman_session_destroy() at
src/session.c:1706
connmand[21327]: #9 0x454331 in destroy_session() at src/manager.c:341
connmand[21327]: #10 0x4a9826 in process_message() at gdbus/object.c:259
connmand[21327]: #11 0x4ab33b in generic_message() at gdbus/object.c:1071
connmand[21327]: #12 0x7f57c10468c3 in /lib64/libdbus-1.so.3
connmand[21327]: #13 0x7f57c1037494 in /lib64/libdbus-1.so.3
connmand[21327]: #14 0x4a71da in message_dispatch() at gdbus/mainloop.c:72
(discriminator 1)
connmand[21327]: #15 0x7f57c12b88e7 in /lib64/libglib-2.0.so.0
connmand[21327]: #16 0x7f57c12bbe52 in /lib64/libglib-2.0.so.0
connmand[21327]: #17 0x7f57c12bc1d0 in /lib64/libglib-2.0.so.0
connmand[21327]: #18 0x7f57c12bc4f2 in /lib64/libglib-2.0.so.0
connmand[21327]: #19 0x449c2f in main() at src/main.c:782
connmand[21327]: #20 0x7f57bfead431 in /lib64/libc.so.6
connmand[21327]: +++++++++++++++++++++++++++
After restarting a simple disconnect of the session results in:
connmand[21878]: ++++++++ backtrace ++++++++
connmand[21878]: #0 0x7feed2e72950 in /lib64/libc.so.6
connmand[21878]: #1 0x46d36f in __connman_ipconfig_get_prefixlen() at
src/ipconfig.c:1089
connmand[21878]: #2 0x48ec5f in del_default_subnet_route() at
src/session.c:379 (discriminator 1)
connmand[21878]: #3 0x48f1be in cleanup_routing_table() at src/session.c:513
connmand[21878]: #4 0x48f20c in update_routing_table() at src/session.c:525
connmand[21878]: #5 0x491982 in update_session_state() at src/session.c:1761
connmand[21878]: #6 0x4905ba in disconnect_session() at src/session.c:1196
connmand[21878]: #7 0x4a9826 in process_message() at gdbus/object.c:259
connmand[21878]: #8 0x4ab33b in generic_message() at gdbus/object.c:1071
connmand[21878]: #9 0x7feed3ff68c3 in /lib64/libdbus-1.so.3
connmand[21878]: #10 0x7feed3fe7494 in /lib64/libdbus-1.so.3
connmand[21878]: #11 0x4a71da in message_dispatch() at gdbus/mainloop.c:72
(discriminator 1)
connmand[21878]: #12 0x7feed42688e7 in /lib64/libglib-2.0.so.0
connmand[21878]: #13 0x7feed426be52 in /lib64/libglib-2.0.so.0
connmand[21878]: #14 0x7feed426c1d0 in /lib64/libglib-2.0.so.0
connmand[21878]: #15 0x7feed426c4f2 in /lib64/libglib-2.0.so.0
connmand[21878]: #16 0x449c2f in main() at src/main.c:782
connmand[21878]: #17 0x7feed2e5d431 in /lib64/libc.so.6
connmand[21878]: +++++++++++++++++++++++++++
Apart of this problem there are few small nitpicks (see other mails).
Looks pretty good to me!
Thanks,
Daniel
------------------------------
Message: 3
Date: Thu, 5 Oct 2017 23:34:22 +0300
From: Vasyl Vavrychuk <[email protected]>
To: [email protected]
Subject: net.connman.Error.InvalidArguments is returned in response to
net.connman.Agent.Error.Canceled
Message-ID:
<cagj4m+7-acsfxpwzilczm8b30-d8rra0ocfd_f_mjwa5kyc...@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
Hi,
service.c:request_input_cb contains code
if (g_strcmp0(error,
"net.connman.Agent.Error.Canceled") == 0) {
err = -EINVAL;
if (service->hidden)
__connman_service_return_error(service,
ECONNABORTED,
user_data);
goto done;
} else {
This means that when we send net.connman.Agent.Error.Canceled reply from
agent in response we are getting net.connman.Error.InvalidArguments.
Is it possible to get more specialized response like 'ConnectionAborted'
instead of InvalidArguments. Since we do not know if anything gonna wrong
or canceling was successful.
Sincerelly,
Vasyl
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.01.org/pipermail/connman/attachments/20171005/09590386/attachment-0001.html>
------------------------------
Message: 4
Date: Fri, 6 Oct 2017 11:22:19 +0000
From: "Puustinen, Ismo" <[email protected]>
To: "[email protected]" <[email protected]>, "[email protected]"
<[email protected]>
Subject: Re: [PATCH] dns: Use hash table instead of binary tree
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"
Thanks for doing the changes! I was already thinking that I'll need to
do them today. :-)
On Thu, 2017-10-05 at 21:19 +0200, Daniel Wagner wrote:
> 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)
Isn't this function now exactly backwards? During the g_tree times it
was a GCompareDataFunc, now it's a GEqualFunc. It needs to return TRUE
if a = b, FALSE otherwise. The return value should be gboolean.
> {
> 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;
There's a bug here: ret == 0 means that the setup succeeded. Replace
this with
if (ret < 0)
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) {
Ismo
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 24, Issue 10
***************************************
