Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe 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 2/3] vpn-provider: Implement setting of multiple VPN
properties with one call
(Daniel Wagner)
2. Re: [PATCH 2/3 v2] vpn-provider: Implement setting of multiple VPN
properties with one call
(Daniel Wagner)
3. Re: [PATCH 3/3 v2] doc: Describe property array use with VPN connection
SetProperty D-Bus method
(Daniel Wagner)
4. Re: wifi AP disconnect(disconnect reason code 1) (Daniel Wagner)
5. Re: [PATCH 0/7] A few more WireGuard fixes (Daniel Wagner)
6. Re: [PATCH 0/7] A few more WireGuard fixes (Daniel Wagner)
7. [PATCH] firewall-nftables: Use nftnl_{chain|rule}_set_str() for string
attributes
(Daniel Wagner)
----------------------------------------------------------------------
Date: Sat, 21 Dec 2019 12:25:37 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: [PATCH 2/3] vpn-provider: Implement setting of multiple
VPN properties with one call
To: Jussi Laakkonen <[email protected]>
Cc: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Jussi,
On 18.12.19 14:19, Jussi Laakkonen wrote:
>> I was pondering on this proposal for a while. I am just not convience
>> to introduce a SetProperties method is a good idea. We don't have
>> this kind of API in any other daemon and also we might eventually like
>> to move (connman v2) to the freedesktop verion of the properties
>> interface:
>>
>> https://dbus.freedesktop.org/doc/dbus-java/api/org/freedesktop/DBus.Prope...
>>
>> So one way you could do push this one level down behind SetProperty
>> and do a parsing of a config set there. It's almost the same just
>> avoid to do introduce a D-Bus API.
>>
>
> Ah, right. So the SetProperty should accept both, a single {sv} or a similar
> set
> what GetProperties returns. I think that is doable and a good suggestion.
> I'll try
> to manage a v2 before holidays.
Yes, that is what I had in mind. No hurries :)
>> Don't you also need to set all vpn_provider_set_string() back? This
>> just avoids to store the current configuration but the configuration
>> is still there. Unless I err again...
>>
>
> I'm not sure if I'm following this comment.. But are you suggesting a revert
> of
> some sort in case there are errors?
>
> The idea behind this is to write the config only once if there is a single
> property
> value change. In case there are errors on some, simply report back them in the
> D-Bus error message. If no changes are done, there is no need to save the
> config.
>
> Or did I completely misinterpret that last comment of yours?
Let me try again. I know I can be confusing :)
I think we are on the same page regarding how the method call should
behave. So it's either everything okay or something went wrong. If
something went wrong nothing will change, correct?
And with that in mind I looked at the code and if I am not mistaken the
'nothing will change' promise is not completely fulfilled. The change
wont be written on disk but some changes might have been done on the
config settings in memory.
> P.S. The first reply on 1/3 arrived to my mailbox, this 2/3 reply didn't :)
Argh, I've complained once more about it. If nothing happens we might
need to move to another hosting service.
Thanks,
Daniel
------------------------------
Date: Sat, 21 Dec 2019 12:53:23 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: [PATCH 2/3 v2] vpn-provider: Implement setting of
multiple VPN properties with one call
To: Jussi Laakkonen <[email protected]>
Cc: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
Hi Jussi,
On Fri, Dec 20, 2019 at 11:55:53AM +0200, Jussi Laakkonen wrote:
> Enhance SetProperty D-Bus method to accept an array of VPN properties in
> order to enable setting of multiple properties with one D-Bus call. For
> enabling this set_properties() method is implented, and is called when
> an array is received that does not have name "UserRoutes". This improves
> the performance when multiple (or all) properties are to be changed but
> reduces the granularity of errors reported back to the caller. Properties
> are accepted in the same format as with GetProperties D-Bus method sends
> them, a{sv}, wrapped inside a variant value.
>
> When an array of properties is sent each changed property value emits a
> PropertyChanged signal. If there is at least one changed property
> value then the VPN provider config is written to disk. Disk write is
> done after all properties in the array are processed.
The missing concept in the Property API for changing multiple settings
at once leads to a bit strange things. Obvioulsy, it would be more
naturual to have single value set/get/clear/changed and the same thing
for multiple properties at once. Well, since this doesn't exist
neither for ConnMan nor for the freedesktop API let's go with this.
Maybe we should bring this up at freedesktop and see what the
maintainers of the Property API think about it.
> Empty strings as property values, or an empty array for "UserRoutes" are
> allowed with this change when using SetProperty. In both cases the
> effect is the same as calling ClearProperty on a property. Thus, this
> change allows to clear multiple properties with one call when sending an
> array of (empty) properties.
>
> If there is at least one invalid property in the array then
> InvalidProperty D-Bus error is sent as reply. Invalid properties >
> immutable properties in case of error and there can be permission errors
> caused by immutable properties when InvalidProperty error is sent. Only
> when there are only permission errors then PermissionDenied D-Bus error
> is sent. In both cases the D-Bus error message is amended with the
> property names that caused the errors. If there are also properties that
> had proper values and the values were changed a PropertyChange signal is
> sent for each and VPN provider settings are written to disk.
>
> Move setting of a single property into its own function, usable by both
> set_property() and set_properties(). The set_vpn_property() retains the
> functionality by changing either the routes or a single string value,
> returns 0 when success, -EALREADY when there is no change on property
> value and appropriate error (-EINVAL/-EPERM/-ENOMEM) otherwise.
Patch applied.
Thankk,
Daniel
------------------------------
Date: Sat, 21 Dec 2019 12:54:43 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: [PATCH 3/3 v2] doc: Describe property array use with VPN
connection SetProperty D-Bus method
To: Jussi Laakkonen <[email protected]>
Cc: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
On Fri, Dec 20, 2019 at 11:56:46AM +0200, Jussi Laakkonen wrote:
> Include documentation for the use of dict array with the SetProperty
> D-Bus method in VPN connection API.
>
> Add error PermissionDenied for SetProperty and ClearProperty into VPN
> connection API. Add missing NotSupported error for SetProperty into VPN
> connection API.
> ---
>
> Changes since v2:
> * Removed SetProperties D-Bus method description from API.
> * Reformatted the SetProperties description under SetProperty
> description in the API.
> * Changed the commit title to be more appropriate.
>
> doc/vpn-connection-api.txt | 43 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/doc/vpn-connection-api.txt b/doc/vpn-connection-api.txt
> index 1fd3be26..80080ebe 100644
> --- a/doc/vpn-connection-api.txt
> +++ b/doc/vpn-connection-api.txt
> @@ -14,13 +14,47 @@ Methods dict GetProperties() [experimental]
>
> void SetProperty(string name, variant value) [experimental]
>
> - Changes the value of the specified property. Only
> - properties that are listed as read-write are
> - changeable. On success a PropertyChanged signal
> - will be emitted.
> + Changes the value of the specified property or the
> + properties defined as a dict passed as variant, where
> + the format is equal to the dict returned by
> + GetProperties(). Only properties that are listed as
> + read-write are changeable. On success a
> + PropertyChanged signal will be emitted for the
> + specified property or for all changed properties
> + individually. If there is no change in property value
> + no PropertyChanged signal is sent. Configuration is
> + written to disk when one or more values are changed.
> + In case a dict of properties are given, configuration
> + write is done after all properties are processed.
> + Specifics in dict use in contrast to setting a single
> + property:
> + - Dict can contain values set as empty strings
> + or arrays. This causes the values to be
> + cleared as if using ClearProperty().
> + - If there are errors with the properties,
> + InvalidProperty or PermissionDenied error is
> + returned. InvalidProperty is sent when there
> + is at least one invalid property, in this
> + case there can be also properties that
> + cannot be changed (immutable properties).
> + If there are only immutable properties
> + PermissionDenied error is returned.
> + - The properties that are invalid or immutable
> + are reported back at the end of the error
> + message as a comma separated property name
> + list.
> + - One invalid/immutable property does not
> + cause the rest of the properties to be
> + ignored. If there are valid and invalid
> + properties, the valid properties emit
> + PropertyChanged signal and invalid are
> + reported back with an InvalidProperty
> + message.
In the case of the dictionary, the property name can be anything,
correct? Maybe we should define it to something like SetProperties?
Patch applied.
Thanks,
Daniel
------------------------------
Date: Sat, 21 Dec 2019 13:10:20 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: wifi AP disconnect(disconnect reason code 1)
To: Deepu Paul <[email protected]>
Cc: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
Hi Deepu,
On Wed, Dec 18, 2019 at 12:54:14PM +0000, Deepu Paul wrote:
> Hi Daniel,
>
> Thank you for the answer.
>
> I am able to fix the issue with the attached patch.
>
> I have my board with connman service running for 2-3 days now and still
> that AP is connected with the patch fix(from the logs i can see it
> disconnected more than 50 times, but successfully reconnected by the auto
> scan).
This is when reauth in wpa_supplicant fails and the autoconnect code
of ConnMan retries to connect to the network?
> connmanctl> services
> *AO purpleline
> wifi_7804731c31f0_707572706c656c696e65_managed_psk
>
> Can you please review the patch?
Adding the connman_network_set_connect() in disconnect_reasoncode()
looks wrong. Unless wpa_s decides never sends a state transition to
disconnect state? Did you add it because you never got the state
transition? Can you provide the logs for this? Which version of
wpa_supplicant are you using?
Also please write a proper patch with commit message.
> In the wifi.c, this is done in the interface_state
> case G_SUPPLICANT_STATE_DISCONNECTED:
> if (network != wifi->pending_network) {
> connman_network_set_connected(network, false);
> connman_network_set_associating(network, false);
> }
>
Correct, that is what I am referring to. Don't you get the state
transition?
> but in my case when it disconnect with reason code 1, it enters in this
> state only like 90% of the times. That's why i am forcing connman network
> connected flag to false in the disconnect callback.
But thist indicates that wpa_supplicant is not behaving correctly. I
rather see wpa_supplicant fixed than adding a workaround in
ConnMan. If wpa_supplicant disconnects from the network it should
clearly tell ConnMan. In this case the normal auto connection
algorithm should kick in and you get a reconnect after the scan.
Thanks,
Daniel
------------------------------
Date: Sat, 21 Dec 2019 14:42:11 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: [PATCH 0/7] A few more WireGuard fixes
To: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
On Sun, Dec 08, 2019 at 06:00:19PM +0100, Daniel Wagner wrote:
> Fix a few bugs reported by Christian Hewitt. Thanks!
>
> Daniel Wagner (7):
> vpn: Use correct format specifier for size_t
> client: Prefix VPN serives identifiers with "vpn_"
> wireguard: Add PersistentKeepalive config option
> doc: Mention new PersistentKeepalive option
> doc: Add missing WireGuard to Type field
> wireguard: Make the ListenPort optional
> vpn: Call disconnect() also for non TUN VPNs
All patches applied.
------------------------------
Date: Sat, 21 Dec 2019 14:42:11 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: [PATCH 0/7] A few more WireGuard fixes
To: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset="us-ascii"
On Sun, Dec 08, 2019 at 06:00:19PM +0100, Daniel Wagner wrote:
> Fix a few bugs reported by Christian Hewitt. Thanks!
>
> Daniel Wagner (7):
> vpn: Use correct format specifier for size_t
> client: Prefix VPN serives identifiers with "vpn_"
> wireguard: Add PersistentKeepalive config option
> doc: Mention new PersistentKeepalive option
> doc: Add missing WireGuard to Type field
> wireguard: Make the ListenPort optional
> vpn: Call disconnect() also for non TUN VPNs
All patches applied.
_______________________________________________
connman mailing list -- [email protected]
To unsubscribe send an email to [email protected]
------------------------------
Date: Sat, 21 Dec 2019 15:27:39 +0100
From: Daniel Wagner <[email protected]>
Subject: [PATCH] firewall-nftables: Use nftnl_{chain|rule}_set_str()
for string attributes
To: [email protected]
Cc: Daniel Wagner <[email protected]>
Message-ID: <[email protected]>
Replace nftnl_{chain|rule}_set() with nftnl_{chain|rule}_set_str() to
set string attributes. nftnl_{chain|rule}_set() have been deprecated
with the commit message:
These functions make assumptions on size of passed data pointer and
therefore tend to hide programming mistakes. Instead either one of the
type-specific setters or the generic *_set_data() setter should be
used.
---
src/firewall-nftables.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/firewall-nftables.c b/src/firewall-nftables.c
index 262b2a904e9a..8815c29cb7aa 100644
--- a/src/firewall-nftables.c
+++ b/src/firewall-nftables.c
@@ -507,8 +507,8 @@ static int rule_delete(struct firewall_handle *handle)
if (!rule)
return -ENOMEM;
- nftnl_rule_set(rule, NFTNL_RULE_TABLE, CONNMAN_TABLE);
- nftnl_rule_set(rule, NFTNL_RULE_CHAIN, handle->chain);
+ nftnl_rule_set_str(rule, NFTNL_RULE_TABLE, CONNMAN_TABLE);
+ nftnl_rule_set_str(rule, NFTNL_RULE_CHAIN, handle->chain);
nftnl_rule_set_u64(rule, NFTNL_RULE_HANDLE, handle->handle);
err = socket_open_and_bind(&nl);
@@ -568,8 +568,8 @@ static int build_rule_nat(const char *address, unsigned
char prefixlen,
if (!rule)
return -ENOMEM;
- nftnl_rule_set(rule, NFTNL_RULE_TABLE, CONNMAN_TABLE);
- nftnl_rule_set(rule, NFTNL_RULE_CHAIN, CONNMAN_CHAIN_NAT_POST);
+ nftnl_rule_set_str(rule, NFTNL_RULE_TABLE, CONNMAN_TABLE);
+ nftnl_rule_set_str(rule, NFTNL_RULE_CHAIN, CONNMAN_CHAIN_NAT_POST);
/* family ipv4 */
nftnl_rule_set_u32(rule, NFTNL_RULE_FAMILY, NFPROTO_IPV4);
@@ -673,8 +673,8 @@ static int build_rule_snat(int index, const char *address,
if (!rule)
return -ENOMEM;
- nftnl_rule_set(rule, NFTNL_RULE_TABLE, CONNMAN_TABLE);
- nftnl_rule_set(rule, NFTNL_RULE_CHAIN, CONNMAN_CHAIN_NAT_POST);
+ nftnl_rule_set_str(rule, NFTNL_RULE_TABLE, CONNMAN_TABLE);
+ nftnl_rule_set_str(rule, NFTNL_RULE_CHAIN, CONNMAN_CHAIN_NAT_POST);
/* OIF */
expr = nftnl_expr_alloc("meta");
@@ -770,8 +770,8 @@ static int build_rule_marking(uid_t uid, uint32_t mark,
struct nftnl_rule **res)
if (!rule)
return -ENOMEM;
- nftnl_rule_set(rule, NFTNL_RULE_TABLE, CONNMAN_TABLE);
- nftnl_rule_set(rule, NFTNL_RULE_CHAIN, CONNMAN_CHAIN_ROUTE_OUTPUT);
+ nftnl_rule_set_str(rule, NFTNL_RULE_TABLE, CONNMAN_TABLE);
+ nftnl_rule_set_str(rule, NFTNL_RULE_CHAIN, CONNMAN_CHAIN_ROUTE_OUTPUT);
expr = nftnl_expr_alloc("meta");
if (!expr)
@@ -826,8 +826,8 @@ static int build_rule_src_ip(const char *src_ip, uint32_t
mark, struct nftnl_rul
if (!rule)
return -ENOMEM;
- nftnl_rule_set(rule, NFTNL_RULE_TABLE, CONNMAN_TABLE);
- nftnl_rule_set(rule, NFTNL_RULE_CHAIN, CONNMAN_CHAIN_ROUTE_OUTPUT);
+ nftnl_rule_set_str(rule, NFTNL_RULE_TABLE, CONNMAN_TABLE);
+ nftnl_rule_set_str(rule, NFTNL_RULE_CHAIN, CONNMAN_CHAIN_ROUTE_OUTPUT);
/* family ipv4 */
nftnl_rule_set_u32(rule, NFTNL_RULE_FAMILY, NFPROTO_IPV4);
@@ -954,8 +954,8 @@ static struct nftnl_chain *build_chain(const char *name,
const char *table,
if (!chain)
return NULL;
- nftnl_chain_set(chain, NFTNL_CHAIN_TABLE, table);
- nftnl_chain_set(chain, NFTNL_CHAIN_NAME, name);
+ nftnl_chain_set_str(chain, NFTNL_CHAIN_TABLE, table);
+ nftnl_chain_set_str(chain, NFTNL_CHAIN_NAME, name);
if (type)
nftnl_chain_set_str(chain, NFTNL_CHAIN_TYPE, type);
--
2.24.0
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list -- [email protected]
To unsubscribe send an email to [email protected]
------------------------------
End of connman Digest, Vol 50, Issue 11
***************************************
