Re: [PATCH v5 0/7] iptables bug fixes

2013-03-07 Thread Patrik Flykt
On Wed, 2013-03-06 at 16:08 +0100, Daniel Wagner wrote: From: Daniel Wagner daniel.wag...@bmw-carit.de changes: - patch #6: use opencoding for verdict - patch #7: fix append/insert confusion Looks nice! Applied. Thanks! Patrik ___

[PATCH] device: Check positive device filter correctly

2013-03-07 Thread Jukka Rissanen
The -i or --device command line option contains the device names that we should use. Unfortunately the check fails if there are multiple interfaces in that list and we ignore the interfaces instead. Fixes BMC#25979 --- src/device.c | 14 ++ 1 file changed, 10 insertions(+), 4

[PATCH] vpn-provider: Do not quit vpnd if there are no VPN configurations

2013-03-07 Thread Jukka Rissanen
Because we now support provisioning of VPN services via .config file, we need to keep vpnd running so that it can monitor the connman-vpn directory for new config files. --- vpn/vpn-manager.c | 2 -- vpn/vpn-provider.c | 22 -- vpn/vpn.h | 1 - 3 files changed, 25

[PATCH] doc: Typo fixes in the documentation

2013-03-07 Thread Jukka Rissanen
Fixes BMC#25978 --- doc/service-api.txt| 2 +- doc/session-api.txt| 2 +- doc/technology-api.txt | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/service-api.txt b/doc/service-api.txt index b33eb6f..e7224cc 100644 --- a/doc/service-api.txt +++

Re: [PATCH] device: Check positive device filter correctly

2013-03-07 Thread Patrik Flykt
On Thu, 2013-03-07 at 10:24 +0200, Jukka Rissanen wrote: The -i or --device command line option contains the device names that we should use. Unfortunately the check fails if there are multiple interfaces in that list and we ignore the interfaces instead. Fixes BMC#25979 Applied, thanks!

[PATCH v2] vpn-provider: Do not quit vpnd if there are no VPN configurations

2013-03-07 Thread Jukka Rissanen
Because we now support provisioning of VPN services via .config file, we need to keep vpnd running so that it can monitor the connman-vpn directory for new config files. --- Hi, One corner case check (quitting if last vpn config was removed) was missing from v1. Cheers, Jukka vpn/vpn-manager.c

[PATCH v3 00/10] iptables refactoring

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de Hi, Most notable changes compared to v2, is that test-iptables also checks if the rules are really hitting the kernel. It does this via calling iptables-save and looking for rules in the output. Kind of shirt-sleeve but I think it is worth to have

[PATCH v3 01/10] test-iptables: Add debug option

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de It is impossible to see the normal unit test output with all DBG() enabled by default. --- unit/test-iptables.c | 41 - 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/unit/test-iptables.c

[PATCH v3 03/10] iptables: Drop support for xtables 1.4.11

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de The API changed between 1.4.10 (version code 5) and 1.4.11 (version code 6) and we needed to workaround with a bunch of ugly ifdefs. 1.4.11 was released on 26.05.2011 and even Debian testing ships 1.4.14 these days. --- configure.ac | 2 +-

[PATCH v3 02/10] test-iptables: Check if rules are inserted/removed

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de Instead just relying on the error code return by ConnMan's iptables API we use iptables-save to check if the rules either are installed or removed. --- unit/test-iptables.c | 88 +++- 1 file changed,

[PATCH v3 04/10] iptables: Use glib function for string operations

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de Streamline this file with the rest of ConnMan's code base. --- src/iptables.c | 62 +- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/iptables.c b/src/iptables.c index

[PATCH v3 05/10] iptables: Improve debug log output

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de We need to see a bit more in detail what happens when CONNMAN_IPTABLES_DEBUG is not set, for example the removing/flushing during bootup. Also remove the DBG() from parse_rule_spec() because all callers already have a DBG(). So not much additional

[PATCH v3 06/10] iptables: Rename pre_load_table() to get_table()

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de The second argument is not used anymore, let's remove it. The funciton name doesn't really match to its implementation, so it's also time to rename it. --- src/iptables.c | 20 +--- 1 file changed, 9 insertions(+), 11 deletions(-)

[PATCH v3 07/10] iptables: Refactor prepare_rule_inclusion()

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de update_hooks is only necessary when the rules was part of a built in chain. And adding some documentation which explains what is happening. --- src/iptables.c | 29 + 1 file changed, 21 insertions(+), 8 deletions(-)

[PATCH v3 08/10] iptables: Refactor update_hooks()

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de There are a few useless checks in there. prepare_rule_inclusion() already does those tests. Also add some comments what is happening in here. --- src/iptables.c | 36 +--- 1 file changed, 17 insertions(+), 19

[PATCH v3 09/10] iptables: Refactor iptables_delete_rule()

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de Move the hook update part into its own function, so we have one function for updating the hooks when inserting a rule and one for updating the hoooks when removing a rule. Add some comments to explain what's happening. --- src/iptables.c | 73

[PATCH v3 10/10] iptables: find_chain_tail should return last entry

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de Currently, find_chain_tail() returns the element after the chain end. With returning the real end of chain, the code gets more readable. --- src/iptables.c | 28 1 file changed, 16 insertions(+), 12 deletions(-) diff

Re: [PATCH v3 05/10] iptables: Improve debug log output

2013-03-07 Thread Tomasz Bursztyka
Hi Daniel, @@ -1669,7 +1677,18 @@ static struct connman_iptables *pre_load_table(const char *table_name, if (table != NULL) return table; - return iptables_init(table_name); + table = g_hash_table_lookup(table_hash, table_name); + if (table != NULL) +

Re: [PATCH v3 05/10] iptables: Improve debug log output

2013-03-07 Thread Daniel Wagner
Hi Tomasz, On 03/07/2013 01:13 PM, Tomasz Bursztyka wrote: Hi Daniel, @@ -1669,7 +1677,18 @@ static struct connman_iptables *pre_load_table(const char *table_name, if (table != NULL) return table; -return iptables_init(table_name); +table =

[PATCH v4 01/11] test-iptables: Add debug option

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de It is impossible to see the normal unit test output with all DBG() enabled by default. --- unit/test-iptables.c | 41 - 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/unit/test-iptables.c

[PATCH v4 00/11] iptables refactroring

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de changes: - splitted Improve debug log output Daniel Wagner (11): test-iptables: Add debug option test-iptables: Check if rules are inserted/removed iptables: Drop support for xtables 1.4.11 iptables: Use glib function for string operations

[PATCH v4 02/11] test-iptables: Check if rules are inserted/removed

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de Instead just relying on the error code return by ConnMan's iptables API we use iptables-save to check if the rules either are installed or removed. --- unit/test-iptables.c | 88 +++- 1 file changed,

[PATCH v4 03/11] iptables: Drop support for xtables 1.4.11

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de The API changed between 1.4.10 (version code 5) and 1.4.11 (version code 6) and we needed to workaround with a bunch of ugly ifdefs. 1.4.11 was released on 26.05.2011 and even Debian testing ships 1.4.14 these days. --- configure.ac | 2 +-

[PATCH v4 04/11] iptables: Use glib function for string operations

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de Streamline this file with the rest of ConnMan's code base. --- src/iptables.c | 62 +- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/iptables.c b/src/iptables.c index

[PATCH v4 05/11] iptables: Lookup in table hash before module loading

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de pre_load_table() is called always with table == NULL, we end up keep trying to load the kernel modules even though the table is already loaded. Therefore, move the lookup one level up. --- src/iptables.c | 24 ++-- 1 file

[PATCH v4 06/11] iptables: Improve debug log output

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de We need to see a bit more in detail what happens when CONNMAN_IPTABLES_DEBUG is not set, for example the removing/flushing during bootup. Also remove the DBG() from parse_rule_spec() because all callers already have a DBG(). So not much additional

[PATCH v4 07/11] iptables: Rename pre_load_table() to get_table()

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de The second argument is not used anymore, let's remove it. The funciton name doesn't really match to its implementation, so it's also time to rename it. --- src/iptables.c | 20 +--- 1 file changed, 9 insertions(+), 11 deletions(-)

[PATCH v4 08/11] iptables: Refactor prepare_rule_inclusion()

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de update_hooks is only necessary when the rules was part of a built in chain. And adding some documentation which explains what is happening. --- src/iptables.c | 29 + 1 file changed, 21 insertions(+), 8 deletions(-)

[PATCH v4 09/11] iptables: Refactor update_hooks()

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de There are a few useless checks in there. prepare_rule_inclusion() already does those tests. Also add some comments what is happening in here. --- src/iptables.c | 36 +--- 1 file changed, 17 insertions(+), 19

[PATCH v4 10/11] iptables: Refactor iptables_delete_rule()

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de Move the hook update part into its own function, so we have one function for updating the hooks when inserting a rule and one for updating the hoooks when removing a rule. Add some comments to explain what's happening. --- src/iptables.c | 73

[PATCH v4 11/11] iptables: find_chain_tail should return last entry

2013-03-07 Thread Daniel Wagner
From: Daniel Wagner daniel.wag...@bmw-carit.de Currently, find_chain_tail() returns the element after the chain end. With returning the real end of chain, the code gets more readable. --- src/iptables.c | 28 1 file changed, 16 insertions(+), 12 deletions(-) diff

Re: [PATCH v4 02/11] test-iptables: Check if rules are inserted/removed

2013-03-07 Thread Patrik Flykt
Hi, On Thu, 2013-03-07 at 14:35 +0100, Daniel Wagner wrote: From: Daniel Wagner daniel.wag...@bmw-carit.de Instead just relying on the error code return by ConnMan's iptables API we use iptables-save to check if the rules either are installed or removed. --- unit/test-iptables.c

Re: [PATCH v4 06/11] iptables: Improve debug log output

2013-03-07 Thread Patrik Flykt
Hi, On Thu, 2013-03-07 at 14:35 +0100, Daniel Wagner wrote: From: Daniel Wagner daniel.wag...@bmw-carit.de We need to see a bit more in detail what happens when CONNMAN_IPTABLES_DEBUG is not set, for example the removing/flushing during bootup. You probably meant when the variable

Re: [PATCH v4 09/11] iptables: Refactor update_hooks()

2013-03-07 Thread Patrik Flykt
On Thu, 2013-03-07 at 14:35 +0100, Daniel Wagner wrote: From: Daniel Wagner daniel.wag...@bmw-carit.de There are a few useless checks in there. prepare_rule_inclusion() already does those tests. Also add some comments what is happening in here. Are we achieving anything significantly better

Re: [PATCH v4 10/11] iptables: Refactor iptables_delete_rule()

2013-03-07 Thread Patrik Flykt
On Thu, 2013-03-07 at 14:35 +0100, Daniel Wagner wrote: From: Daniel Wagner daniel.wag...@bmw-carit.de Move the hook update part into its own function, so we have one function for updating the hooks when inserting a rule and one for updating the hoooks when removing a rule. Add some

Re: [PATCH v4 02/11] test-iptables: Check if rules are inserted/removed

2013-03-07 Thread Daniel Wagner
On 03/07/2013 03:23 PM, Patrik Flykt wrote: Hi, On Thu, 2013-03-07 at 14:35 +0100, Daniel Wagner wrote: From: Daniel Wagner daniel.wag...@bmw-carit.de Instead just relying on the error code return by ConnMan's iptables API we use iptables-save to check if the rules either are

Re: [PATCH v4 06/11] iptables: Improve debug log output

2013-03-07 Thread Daniel Wagner
Hi Patrik, On 03/07/2013 03:27 PM, Patrik Flykt wrote: On Thu, 2013-03-07 at 14:35 +0100, Daniel Wagner wrote: From: Daniel Wagner daniel.wag...@bmw-carit.de We need to see a bit more in detail what happens when CONNMAN_IPTABLES_DEBUG is not set, for example the removing/flushing during

Re: [PATCH v4 09/11] iptables: Refactor update_hooks()

2013-03-07 Thread Daniel Wagner
On 03/07/2013 03:28 PM, Patrik Flykt wrote: On Thu, 2013-03-07 at 14:35 +0100, Daniel Wagner wrote: From: Daniel Wagner daniel.wag...@bmw-carit.de There are a few useless checks in there. prepare_rule_inclusion() already does those tests. Also add some comments what is happening in here. Are

Re: [PATCH v4 02/11] test-iptables: Check if rules are inserted/removed

2013-03-07 Thread Patrik Flykt
On Thu, 2013-03-07 at 15:30 +0100, Daniel Wagner wrote: Don't think we can make this work as long we are talking to the kernel. The only way I see would be to intercept the final table as buffer and return it. What do you propose? Remove test-iptables.c? If you make a unit test out of

Re: [PATCH v4 10/11] iptables: Refactor iptables_delete_rule()

2013-03-07 Thread Daniel Wagner
On 03/07/2013 03:29 PM, Patrik Flykt wrote: On Thu, 2013-03-07 at 14:35 +0100, Daniel Wagner wrote: From: Daniel Wagner daniel.wag...@bmw-carit.de Move the hook update part into its own function, so we have one function for updating the hooks when inserting a rule and one for updating the

Re: [PATCH v4 11/11] iptables: find_chain_tail should return last entry

2013-03-07 Thread Daniel Wagner
On 03/07/2013 03:33 PM, Patrik Flykt wrote: On Thu, 2013-03-07 at 14:35 +0100, Daniel Wagner wrote: Currently, find_chain_tail() returns the element after the chain end. Let's keep it the old way until there is a real reason to do something else. If it ain't broken, let's not fix it (yet).

Re: [PATCH v4 02/11] test-iptables: Check if rules are inserted/removed

2013-03-07 Thread Daniel Wagner
On 03/07/2013 03:37 PM, Patrik Flykt wrote: On Thu, 2013-03-07 at 15:30 +0100, Daniel Wagner wrote: Don't think we can make this work as long we are talking to the kernel. The only way I see would be to intercept the final table as buffer and return it. What do you propose? Remove

Re: [PATCH v4 06/11] iptables: Improve debug log output

2013-03-07 Thread Daniel Wagner
On 03/07/2013 03:35 PM, Daniel Wagner wrote: Hi Patrik, On 03/07/2013 03:27 PM, Patrik Flykt wrote: On Thu, 2013-03-07 at 14:35 +0100, Daniel Wagner wrote: From: Daniel Wagner daniel.wag...@bmw-carit.de We need to see a bit more in detail what happens when CONNMAN_IPTABLES_DEBUG is not set,

[PATCH 1/8] vpn: Clear the data pointer from provider when destroying

2013-03-07 Thread Jukka Rissanen
If the data pointer is left around, then vpn plugin might accidentally use it if dbus messages are received after we have cleared the vpn connection. --- plugins/vpn.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/vpn.c b/plugins/vpn.c index cbba396..f60a658 100644 ---

[PATCH 0/8] Misc VPN fixes

2013-03-07 Thread Jukka Rissanen
Hi, While debugging weird pptp vpn issues (thanks lufie!), I noticed some races and bugs in the current vpn code. In patch 1 we clear the data pointer (which points to ourselves) so that we do not accidentally try to access freed memory later. The inotify generated two events (create + modify)

[PATCH 3/8] provider: Change the name of provider remove function

2013-03-07 Thread Jukka Rissanen
Following patch will introduce provider remove function that can be used from vpn plugin so rename the current removal function to reflect better its usage. --- src/connman.h | 2 +- src/manager.c | 2 +- src/provider.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git

[PATCH 2/8] vpn-config: Ignore IN_CREATE as IN_MODIFY is called anyway

2013-03-07 Thread Jukka Rissanen
Inotify will send modify event after create event when user has copied the config file into config directory. Because of this it is useless to act on create event. As a bonus we avoid create/modify/create loop that was earlier done in the modify event handling code. --- vpn/vpn-config.c | 28

[PATCH 4/8] provider: New provider removal function callable from vpn plugin

2013-03-07 Thread Jukka Rissanen
--- include/provider.h | 1 + src/provider.c | 13 + 2 files changed, 14 insertions(+) diff --git a/include/provider.h b/include/provider.h index bfefaed..fdd8ae0 100644 --- a/include/provider.h +++ b/include/provider.h @@ -79,6 +79,7 @@ void connman_provider_unref_debug(struct

[PATCH 5/8] vpn: Remove VPN provider from service list when destroying it

2013-03-07 Thread Jukka Rissanen
When provider is destroyed in vpnd, we get a notification about that. We must then remove the provider which will also unref it so the provider data will get removed properly. Old code just unreffed the provider but it was left hanging in service list. --- plugins/vpn.c | 2 +- 1 file changed, 1

[PATCH 6/8] vpn: Add debug information when vpn state changes

2013-03-07 Thread Jukka Rissanen
--- plugins/vpn.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/vpn.c b/plugins/vpn.c index cda0c1b..f15796d 100644 --- a/plugins/vpn.c +++ b/plugins/vpn.c @@ -252,6 +252,8 @@ static void set_provider_state(struct connection_data *data) enum connman_provider_state state =

[PATCH 8/8] vpn: Make sure vpn connection really exists before removing it

2013-03-07 Thread Jukka Rissanen
--- plugins/vpn.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugins/vpn.c b/plugins/vpn.c index 04318a9..038a833 100644 --- a/plugins/vpn.c +++ b/plugins/vpn.c @@ -1508,6 +1508,7 @@ static gboolean connection_removed(DBusConnection *conn, DBusMessage *message, {

[PATCH 7/8] vpn: Check whether we have already removed the provider

2013-03-07 Thread Jukka Rissanen
The dbus messages from vpnd might come in different order so make sure we are not trying to access already removed provider. --- plugins/vpn.c | 8 1 file changed, 8 insertions(+) diff --git a/plugins/vpn.c b/plugins/vpn.c index f15796d..04318a9 100644 --- a/plugins/vpn.c +++

Connman upstream test result_20130308

2013-03-07 Thread Li, XiaX
Hi all, This is test report for commit 0ba752cd651f972554f573534794b6e17c0e15fb. In this testing, we ran 114 cases. 113 cases passed and 1 case failed. The pass rate is 99%.We found 1 new bug, reopen 0 bug and verify 4 bugs. In this commit we found 1 new bug Error outputs when connect to wifi

Re: [PATCH v4 00/11] iptables refactroring

2013-03-07 Thread Daniel Wagner
I really should spend more time in explaining stuff in the cover letter. These patches are kind of left overs from the bug fixing session and not really necessary if you think in 'does it fix a bug' or 'it's a new feature'. These patches are just plain maintaining work which improve the