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] dhcp: Add support for setting DHCP Vendor Class
ID (Option 60) (Daniel Wagner)
2. Re: Session policy with multiple AllowedBearers (Daniel Wagner)
3. [PATCH v2] Replace g_timeout_add_seconds() with 0 as timeout
to g_idle_add() (Saurav Babu)
4. Re: [PATCH v2 2/7] session: add allowed interface config
parameter (Lukasz Nowak)
5. Re: [PATCH v2 7/7] doc: session multi-interface routing
(Lukasz Nowak)
----------------------------------------------------------------------
Message: 1
Date: Sun, 22 Jan 2017 21:00:35 +0100
From: Daniel Wagner <[email protected]>
To: Andreas Smas <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] dhcp: Add support for setting DHCP Vendor Class
ID (Option 60)
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
On 01/13/2017 02:01 PM, Andreas Smas wrote:
> Anyone else with opinions?
In absence of any voice against it, I went ahead and applied it. Sorry
for the long delay.
Thanks,
Daniel
------------------------------
Message: 2
Date: Sun, 22 Jan 2017 21:12:26 +0100
From: Daniel Wagner <[email protected]>
To: Daryl Nebrich <[email protected]>
Cc: [email protected]
Subject: Re: Session policy with multiple AllowedBearers
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Daryl,
On 01/22/2017 08:33 PM, Daryl Nebrich wrote:
> Hi Daniel,
>
> Thanks for response. First post on the list. I'll try to reply inline
> using gmail so I'm not top posting, hopefully comes out ok.
Lot's of HTML I'd say.
> @@ -1556,14 +1559,27 @@
> enum connman_service_type bearer_type;
> enum connman_service_type service_type;
> GSList *list;
> + bool current_service_connected;
> + enum connman_service_type current_service_type;
>
> if (policy && policy->allowed)
> return policy->allowed(session, service);
>
> + // Handle case where session policy has multiple AllowedBearers.
> + // Check if current service in use is connected.
> + current_service_connected =
> __connman_service_is_connected_state(session->service,
> CONNMAN_IPCONFIG_TYPE_IPV4);
> + current_service_type = connman_service_get_type(session->service);
> +
> for (list = session->info->config.allowed_bearers; list; list =
> list->next) {
> bearer_type = GPOINTER_TO_INT(list->data);
> service_type = connman_service_get_type(service);
>
> + // Keep using the current service if found first (higher priority)
> + if (current_service_connected && (bearer_type ==
> current_service_type)) {
> + DBG("Keeping higher priority %s over %s",
> __connman_service_type2string(current_service_type),
> __connman_service_type2string(service_type));
> + return false;
> + }
> +
> if (bearer_type == service_type)
> return true;
> }
>
> But for the above to work, I also had to change the order of looping in
> apply_policy_on_bearers(). At least in 1.33, the func loops through
> available bearers first, and then policy bearers.
That code is quite likely still the same. You could make a patch out of
your work above. Your approach is better than mine, since you are
addressing the ordering at the source.
> Which means the order
> of bearers in the policy file may not be preserved. For me, if I put
> "cellular wifi" in the policy file, I would get "wifi cellular" as the
> order in allowed_bearers. I think that change is still needed with your
> approach above.
As I said my snippet was just compile tested. So if you have something
working, please share your patch. (btw, use /* */ then :))
>
> > I would expect it to always be wifi.
>
> Me too.
>
> > If the
> > route is for cellular, and I manually bring down cellular, the session
> > will not revert to the wifi service (the route table stays empty).
>
> That is another bug I guess. Can you post the debug output part of this?
> This is supposed to work. BTW use the current HEAD for this. 1.33 is
> pretty old :)
>
>
> I fixed this by adding the below to handle_service_state_offline(). Not
> sure if this is the right way but it tested ok for me.
>
> @@ -1695,6 +1711,9 @@
>
> session->service = NULL;
> update_session_state(session);
> +
> + DBG("Try to find another service");
> + session_activate(session);
> }
> }
I have to admit I can't remember all the details in that file. Your
change looks reasonable. If you can provide a short log attach that your
patch and send it to reivew, that would be great!
>
>
> > Shouldn't it revert? The logs show handle_service_state_offline()
> > setting the session->service to null when cellular goes away. And
> after
> > that I'll get a "type wifi busy" log from the auto_connect_service()
> > function in service.c. I see code in session.c to process the policy
> > bearers and setup allowed_bearers, but I'm not clear on where/how
> it's used.
>
> Sorry about that messy code. It was quite difficult to get it
> initially working
> but it is for sure not really great code. If you find a way to simplify
> I am more than happy to take those patches.
>
>
> Sorry, I wasn't suggesting that. I just hadn't spent the time to review
> it enough. Things are working well now with the changes above.
It was a long way to the current state, so I am glad that it starts to work.
>
> > I've reviewed the latest commits and see the change to nftables.
> But I
> > didn't see any code changes in session.c or session_local_policy.c
> that
> > might address this. Although I do see changes in service.c.
>
> Best thing for now is to use HEAD and port patches back if you need
> to stay at
> 1.33.
>
> > I'm
> > building for an ARM (i.MX6) target using Yocto. And the NXP repos are
> > setup to use connman 1.30. I've updated to 1.33 and ported the
> > patches. I can move to the tip if it would help.
>
> Sure. I'd like to see those kind of bugs gone. If you can provide some
> logs I can try to figure out what is going wrong.
>
> Thanks,
> Daniel
>
>
> I'll back out my session_activate() change in
> handle_service_state_offline() and reply later with a log.
Cool. Thanks.
> I'm a bit
> hesitant to move to the tip now that I've got it going with iptables.
Understood. If you have problems with iptables you might try nftables.
At least it was really really easy to get it working compared to
iptables. I have very strong feelings about iptables...
> Do you know what the plans are for a 1.34 release?
I have pinged Marcel about it a couple of times already. We really need
1.34 release soon.
> The bitbake recipe
> is pulling down the code from kernel.org/pub/linux/network/connman
> <http://kernel.org/pub/linux/network/connman>.
Yep, yocto likes to do that :) Let's see if we can get Marcel to his
magic to get a release!
Thanks,
Daniel
------------------------------
Message: 3
Date: Mon, 23 Jan 2017 14:57:48 +0530
From: Saurav Babu <[email protected]>
To: [email protected]
Cc: [email protected], Saurav Babu <[email protected]>
Subject: [PATCH v2] Replace g_timeout_add_seconds() with 0 as timeout
to g_idle_add()
Message-ID:
<[email protected]>
Passing 0 as timeout to g_timeout_add()/g_timeout_add_seconds() is equivalent
to g_idle_add(). According to glib documentation "the first call of the timer
may not be precise for timeouts of one second."
---
gweb/gweb.c | 3 +--
plugins/vpn.c | 2 +-
src/config.c | 2 +-
src/dhcpv6.c | 7 +++----
src/peer.c | 2 +-
src/proxy.c | 2 +-
src/service.c | 7 +++----
src/task.c | 3 +--
src/wispr.c | 3 +--
9 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/gweb/gweb.c b/gweb/gweb.c
index 10e8c8f..393afe0 100644
--- a/gweb/gweb.c
+++ b/gweb/gweb.c
@@ -1348,8 +1348,7 @@ static guint do_request(GWeb *web, const char *url,
g_free(session->address);
session->address = g_strdup(host);
}
- session->address_action = g_timeout_add(0, already_resolved,
- session);
+ session->address_action = g_idle_add(already_resolved, session);
} else {
session->resolv_action = g_resolv_lookup_hostname(web->resolv,
host, resolv_result, session);
diff --git a/plugins/vpn.c b/plugins/vpn.c
index 7202734..c2a332b 100644
--- a/plugins/vpn.c
+++ b/plugins/vpn.c
@@ -203,7 +203,7 @@ static void resolv_result(GResolvResultStatus status,
* We cannot unref the resolver here as resolv struct is manipulated
* by gresolv.c after we return from this callback.
*/
- g_timeout_add_seconds(0, remove_resolv, data);
+ g_idle_add(remove_resolv, data);
data->resolv_id = 0;
}
diff --git a/src/config.c b/src/config.c
index c40f76c..a6e45ae 100644
--- a/src/config.c
+++ b/src/config.c
@@ -1397,7 +1397,7 @@ static int try_provision_service(struct
connman_config_service *config,
virtual->service = service;
virtual->vfile = config->virtual_file;
- g_timeout_add(0, remove_virtual_config, virtual);
+ g_idle_add(remove_virtual_config, virtual);
return 0;
}
diff --git a/src/dhcpv6.c b/src/dhcpv6.c
index 4346817..cbf7974 100644
--- a/src/dhcpv6.c
+++ b/src/dhcpv6.c
@@ -1197,7 +1197,7 @@ static int check_restart(struct connman_dhcpv6 *dhcp)
if (current >= expired) {
DBG("expired by %d secs", (int)(current - expired));
- g_timeout_add(0, dhcpv6_restart, dhcp);
+ g_idle_add(dhcpv6_restart, dhcp);
return -ETIMEDOUT;
}
@@ -1456,8 +1456,7 @@ int __connman_dhcpv6_start_renew(struct connman_network
*network,
/* RFC 3315, chapter 18.1.3, start rebind */
DBG("start rebind immediately");
- dhcp->timeout = g_timeout_add_seconds(0, start_rebind,
- dhcp);
+ dhcp->timeout = g_idle_add(start_rebind, dhcp);
} else if ((unsigned)current < (unsigned)started + T1) {
delta = started + T1 - current;
@@ -2159,7 +2158,7 @@ static int check_pd_restart(struct connman_dhcpv6 *dhcp)
if (current > expired) {
DBG("expired by %d secs", (int)(current - expired));
- g_timeout_add(0, dhcpv6_restart, dhcp);
+ g_idle_add(dhcpv6_restart, dhcp);
return -ETIMEDOUT;
}
diff --git a/src/peer.c b/src/peer.c
index ad4e445..d00c76f 100644
--- a/src/peer.c
+++ b/src/peer.c
@@ -176,7 +176,7 @@ static int start_dhcp_server(struct connman_peer *peer)
if (err < 0)
goto error;
- g_timeout_add_seconds(0, dhcp_server_started, connman_peer_ref(peer));
+ g_idle_add(dhcp_server_started, connman_peer_ref(peer));
return 0;
diff --git a/src/proxy.c b/src/proxy.c
index f331de9..e1bc420 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -123,7 +123,7 @@ unsigned int connman_proxy_lookup(const char *interface,
const char *url,
lookup->url = g_strdup(url);
lookup->service = connman_service_ref(service);
- lookup->watch = g_timeout_add_seconds(0, lookup_callback, lookup);
+ lookup->watch = g_idle_add(lookup_callback, lookup);
if (lookup->watch == 0) {
g_free(lookup->url);
g_free(lookup);
diff --git a/src/service.c b/src/service.c
index 8480350..4e97a15 100644
--- a/src/service.c
+++ b/src/service.c
@@ -961,8 +961,7 @@ static int nameservers_changed_cb(void *user_data)
static void nameservers_changed(struct connman_service *service)
{
if (!service->nameservers_timeout)
- service->nameservers_timeout = g_timeout_add_seconds(0,
- nameservers_changed_cb,
+ service->nameservers_timeout =
g_idle_add(nameservers_changed_cb,
service);
}
@@ -4001,7 +4000,7 @@ void __connman_service_auto_connect(enum
connman_service_connect_reason reason)
if (!__connman_session_policy_autoconnect(reason))
return;
- autoconnect_timeout = g_timeout_add_seconds(0, run_auto_connect,
+ autoconnect_timeout = g_idle_add(run_auto_connect,
GUINT_TO_POINTER(reason));
}
@@ -4055,7 +4054,7 @@ static void vpn_auto_connect(void)
return;
vpn_autoconnect_timeout =
- g_timeout_add_seconds(0, run_vpn_auto_connect, NULL);
+ g_idle_add(run_vpn_auto_connect, NULL);
}
bool
diff --git a/src/task.c b/src/task.c
index 8b9e1d9..953cc40 100644
--- a/src/task.c
+++ b/src/task.c
@@ -401,8 +401,7 @@ int connman_task_stop(struct connman_task *task)
if (task->pid > 0) {
kill(task->pid, SIGTERM);
- g_timeout_add_seconds(0, check_kill,
- GINT_TO_POINTER(task->pid));
+ g_idle_add(check_kill, GINT_TO_POINTER(task->pid));
}
return 0;
diff --git a/src/wispr.c b/src/wispr.c
index 2cf8746..03b38bb 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -906,8 +906,7 @@ static int wispr_portal_detect(struct
connman_wispr_portal_context *wp_context)
free_connman_wispr_portal_context(wp_context);
}
} else if (wp_context->timeout == 0) {
- wp_context->timeout =
- g_timeout_add_seconds(0, no_proxy_callback, wp_context);
+ wp_context->timeout = g_idle_add(no_proxy_callback, wp_context);
}
done:
--
1.9.1
------------------------------
Message: 4
Date: Mon, 23 Jan 2017 12:30:41 +0000
From: Lukasz Nowak <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH v2 2/7] session: add allowed interface config
parameter
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252
Hi Daniel,
On 22/01/17 16:45, Daniel Wagner wrote:
> Hi Lukasz,
>
> On 01/17/2017 10:17 AM, [email protected] wrote:
>> From: Lukasz Nowak <[email protected]>
>>
>> In order to be able to bind a session to a specific interface,
>> a config option has been added. It is supported through both
>> dbus and session policy. Policy always overrides dbus.
>
> 'config option has been added': That is with this patch, right? Maybe mention
> that you are added AllowedInterface which does following things... bla bla
I will change the description.
>
>> This is the first step to implement per-interface routing.
>> Next step will be to add src ip rules to iptables, which
>> will allow the application to transmit through multiple
>> interfaces.
>
> I think you can leave that information away. This is something for the cover
> letter.
>
>> connman_session_parse_connection_type(val));
>> + } else if (g_str_equal(name, "AllowedInterface")) {
>> + dbus_message_iter_get_basic(&value, &val);
>> + if (session->active)
>> + set_active_session(session, false);
>> +
>> + session->active = false;
>> + session_deactivate(session);
>> +
>> + g_free(session->user_allowed_interface);
>> + /* empty string means allow any interface */
>> + if (0 == g_strcmp0(val, ""))
>> + session->user_allowed_interface = NULL;
>> + else
>> + session->user_allowed_interface = g_strdup(val);
>> +
>> + info->config.allowed_interface = apply_policy_on_interface(
>> + session->policy_config->allowed_interface,
>> + session->user_allowed_interface);
>> +
>> + session_activate(session);
>> } else {
>> goto err;
>> }
>
> For AllowedBearer we have 'any' as match any available bearer. If the string
> is empty it means no bearer is allowed. I wonder if we should follow the
> AllowedBearer approach to keep it more consistent.
I am not sure if this is the case. "any" is used for ConnectionType, but there
is no equivalent for AllowedBearers - user has to explicitly provide an array
of strings. There is no way to specify any/all as far as I can see. For
AllowedInterface, I chose an empty string, as an interface can have an
arbitrary name. So in theory you could have a real interface named "any".
>
> The rest looks good to me.
>
> Thanks,
> Daniel
------------------------------
Message: 5
Date: Mon, 23 Jan 2017 12:35:16 +0000
From: Lukasz Nowak <[email protected]>
To: Daniel Wagner <[email protected]>, [email protected]
Subject: Re: [PATCH v2 7/7] doc: session multi-interface routing
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252
On 22/01/17 17:03, Daniel Wagner wrote:
> Hi Lukasz,
>
> On 01/17/2017 10:17 AM, [email protected] wrote:
>> From: Lukasz Nowak <[email protected]>
>>
>> Update session overview and API documents to demonstrate how sessions
>> can be used to maintain multiple connections in parallel.
>> ---
>> doc/session-api.txt | 22 ++++++++++++++++++++++
>> doc/session-overview.txt | 31 +++++++++++++++++++++++++++++++
>> 2 files changed, 53 insertions(+)
>>
>> diff --git a/doc/session-api.txt b/doc/session-api.txt
>> index 3aac535..42766e7 100644
>> --- a/doc/session-api.txt
>> +++ b/doc/session-api.txt
>> @@ -182,3 +182,25 @@ Settings string State [readonly]
>> (This setting will be removed when the unique process
>> identification problem is solved.)
>>
>> + string AllowedInterface [readwrite]
>> +
>> + This field is used to bind a session to a specific
>> + network interface. If this field is empty, the first
>> + interface from a list of available ones will be used.
>> +
>> + Only one interface may be specified.
>> +
>> + If a specified network interface is not available
>> + (e.g. because AllowedBearers filters it out), the
>> + session will not go online.
>> +
>> + boolean SourceIPRule [readwrite]
>> +
>> + If set to true the session will create source ip
>> + address rule in the firewall, which redirects traffic
>> + to that session's routing table.
>> +
>> + Each session maintains a dedicated routing table, with
>> + a default route. When the source ip rule is enabled,
>> + an application can select which session/interface to
>> + send traffic on, using bind-before-connect mechanism.
>
> nitpick s/ip/IP ?
I am not sure if I follow - would you like to change the "SourceIPRule" name?
Or "source ip" in the description text?
>
> And if I read it correctly it is only IPv4, correct? Hmm, the whole firewall
> code is not IPv6 ready... another todo on my list...
Yes, the whole thing is IPv4. It is not just firewall - session itself has
several places without IPv6 as well. I don't think that the per-session routing
tables would set up IPv6.
>
> cheers,
> daniel
>
>
>> diff --git a/doc/session-overview.txt b/doc/session-overview.txt
>> index 2393167..a18600d 100644
>> --- a/doc/session-overview.txt
>> +++ b/doc/session-overview.txt
>> @@ -92,3 +92,34 @@ The default session configuration does not enable the per
>> application
>> routing. Sessions are still useful in this setup, because the
>> notification of sessions is still available, e.g. the online/offline
>> notification.
>> +
>> +
>> +Multiple per-session routing tables
>> +===================================
>> +
>> +Sessions can be used in an environment with multiple network interfaces,
>> +where an application needs to direct outside traffic through a selected
>> +interface(s). Connman can maintain multiple sessions in a connected
>> +stated, and the application can dynamically, on a per-socket basis,
>> +select which session is used to route traffic.
>> +
>> +Example use cases are:
>> +- monitoring liveness of multiple connected interfaces, by sending
>> + end-to-end heartbeat traffic on all of them in parallel.
>> +- prioritising traffic - e.g. sensitive data can be transferred over a slow,
>> + but secure connection, while big, public downloads use a second session
>> +
>> +By default, Connman maintains only one online service. So it is impossible
>> +to send external traffic (routed through a gateway) on multiple interfaces.
>> +In order to enable this functionality, an application needs to issue the
>> +following API calls:
>> +- create multiple sessions, one for each interface to be used
>> +- set each session's AllowedInterface config field to the required interface
>> + name (eth0, eth1, wlan0, ppp0, etc.)
>> +- set each session's SourceIPRule config field to true
>> +- connect each session (or the service it is using)
>> +
>> +That will instruct Connman to create multiple routing tables, with default
>> +routes in them. After that, the application can issue a bind() call on each
>> +socket, using required interface's source IP address. The bind() call must
>> +be made before a connect() call on a socket.
>>
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 15, Issue 23
***************************************
