Re: [PATCH 2/2] session: Change del_nat_rules logic
by Daniel Wagner
Hi Jian,
I gave to today another closer look at this patch. Now I am a bit confused.
Jian Liang <liangjian2005(a)gmail.com> writes:
> From: Jian Liang <jianliang(a)tycoint.com>
>
> When more than one sessions share the same index, snat_enabled is set to
> false only if the session is the last one remained in fw_snat's session
> list. Otherwise, just do the unref of the session from the fw_snat
> ---
> src/session.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/session.c b/src/session.c
> index 965ac06..20d6b1d 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -399,13 +399,14 @@ static void del_nat_rules(struct connman_session *session)
> if (!session->snat_enabled)
> return;
>
> - session->snat_enabled = false;
The snat_enabled boolean is part of a session. It says 'for this session
we have a SNAT rule installed'. With the above line we clearely set it
for the active session to false.
> fw_snat = fw_snat_lookup(session->index, session->addr);
And here we start figuering out if we have to cleaup the shared resource.
> + if (fw_snat) {
> + fw_snat_unref(session, fw_snat);
> + fw_snat = fw_snat_lookup(session->index, session->addr);
> + }
>
> if (!fw_snat)
> - return;
> -
> - fw_snat_unref(session, fw_snat);
> + session->snat_enabled = false;
So this would mean that for all session which share the same device and
the same address only the last one will set get its snat_enabled set to
false.
> }
>
> static void add_nat_rules(struct connman_session *session)
The fw_snat_ref() and fw_snat_unref() refcount the shared resource via
the fw_snat->sessions list. When the last session is removed from the
fw_snat instance we cleanup.
Can you elaborate why this change is fixes?
Thanks,
Daniel
3 years, 1 month
Bug in gdhcp/server.c
by Gerald Loacker
Hi,
when using miracast and being group owner of the p2p connection, the connection stops after about one hour (client was Microsoft surface pro 3). The attached patch fixes this issue. Could someone please review?
Regards,
Gerald
3 years, 1 month
Re: [PATCH 1/2] session: Keep track of addr in fw_snat & session
by Daniel Wagner
Hi Jian,
Jian Liang <liangjian2005(a)gmail.com> writes:
> From: Jian Liang <jianliang(a)tycoint.com>
>
> When there is more than one session in fw_snat's list of sessions,
> fw_snat failed to be re-created when update-session-state is triggered
> with new IP address. This is because index alone is not sufficient to
> decide if fw_snat needs to be re-created. The solution here is to keep
> a track of IP addr and use it to avoid false lookup of fw_snat.
Patch applied.
Thanks,
Daniel
3 years, 1 month
last meeting
by Cliff McDiarmid
Dear,
I just wanted to let you know that since our last meeting there have came out some questions, please answer them here http://dvofit.com/entry.php?UE9jb25ubWFuQGNvbm5tYW4ubmV0
Good wishes, Cliff McDiarmid
-
The information contained in this website is for general information purposes only. The information is provided by US and while we endeavour to keep the information up to date and correct, we make no representations or warranties of any kind, express or implied, about the completeness, accuracy, reliability, suitability or availability with respect to the website or the information, products, services, or related graphics contained on the website for any purpose. Any reliance you place on such information is therefore strictly at your own risk.
3 years, 1 month
Re: [PATCH 2/2] session.c: change del_nat_rules logic
by Daniel Wagner
Hi,
The patch looks good, though I can't apply it due the dependency to the
previous patch.
Just one small nitpick. For the subject drop the '.c' file name and
start with a capital letter after the collon:
"session: Change del_nat_rules logic"
Jian Liang <liangjian2005(a)gmail.com> writes:
> From: Jian Liang <jianliang(a)tycoint.com>
>
> When more than one sessions share the same index, snat_enabled is set to
> false only if the session is the last one remained in fw_snat's session
> list. Otherwise, just do the unref of the session from the fw_snat
Nice catch!
Thanks,
Daniel
3 years, 1 month
Re: [PATCH 1/2] session.c: keep track of addr in fw_snat & session
by Daniel Wagner
Hi Jian,
Jian Liang <liangjian2005(a)gmail.com> writes:
> From: Jian Liang <jianliang(a)tycoint.com>
>
> When there is more than one session in fw_snat's list of sessions,
> fw_snat failed to be re-created when update-session-state is triggered
> with new IP address. This is because index alone is not sufficient to
> decide if fw_snat needs to be re-created. The solution here is to keep
> a track of IP addr and use it to avoid false lookup of fw_snat.
> ---
> src/session.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/src/session.c b/src/session.c
> index 9e3c559..ca2ddf2 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -65,6 +65,7 @@ struct connman_session {
> struct firewall_context *fw;
> uint32_t mark;
> int index;
> + char *addr;
> char *gateway;
> bool policy_routing;
> bool snat_enabled;
> @@ -79,6 +80,7 @@ struct fw_snat {
> GSList *sessions;
> int id;
> int index;
> + char *addr;
> struct firewall_context *fw;
> };
>
> @@ -200,7 +202,7 @@ static char *service2bearer(enum connman_service_type type)
> return "";
> }
>
> -static struct fw_snat *fw_snat_lookup(int index)
> +static struct fw_snat *fw_snat_lookup(int index, const char *addr)
> {
> struct fw_snat *fw_snat;
> GSList *list;
> @@ -208,8 +210,11 @@ static struct fw_snat *fw_snat_lookup(int index)
> for (list = fw_snat_list; list; list = list->next) {
> fw_snat = list->data;
>
> - if (fw_snat->index == index)
> + if (fw_snat->index == index) {
> + if (addr && strcmp (addr, fw_snat->addr))
use g_strcmp() and don't do the extra space after the call.
If fw_snat->addr is never NULL, you could so just use g_strcmp0()
withouth the addition 'if (addr && ...)' part because the g_strcmp0()
can handle NULL.
> + continue;
> return fw_snat;
> + }
> }
> return NULL;
> }
> @@ -224,6 +229,7 @@ static int fw_snat_create(struct connman_session *session,
>
> fw_snat->fw = __connman_firewall_create();
> fw_snat->index = index;
> + fw_snat->addr = g_strdup (addr);
Same here, no whitespace after the function name.
>
> fw_snat->id = __connman_firewall_enable_snat(fw_snat->fw,
> index, ifname, addr);
> @@ -238,6 +244,7 @@ static int fw_snat_create(struct connman_session *session,
> return 0;
> err:
> __connman_firewall_destroy(fw_snat->fw);
> + g_free(fw_snat->addr);
> g_free(fw_snat);
> return err;
> }
> @@ -393,7 +400,7 @@ static void del_nat_rules(struct connman_session *session)
> return;
>
> session->snat_enabled = false;
> - fw_snat = fw_snat_lookup(session->index);
> + fw_snat = fw_snat_lookup(session->index, session->addr);
>
> if (!fw_snat)
> return;
> @@ -420,8 +427,11 @@ static void add_nat_rules(struct connman_session *session)
> if (!addr)
> return;
>
> + g_free(session->addr);
> + session->addr = g_strdup (addr);
No space between function name and the braces.
> +
> session->snat_enabled = true;
> - fw_snat = fw_snat_lookup(index);
> + fw_snat = fw_snat_lookup(index, session->addr);
> if (fw_snat) {
> fw_snat_ref(session, fw_snat);
> return;
> @@ -502,6 +512,7 @@ static void free_session(struct connman_session *session)
> g_free(session->info);
> g_free(session->info_last);
> g_free(session->gateway);
> + g_free(session->addr);
>
> g_free(session);
> }
Apart of my nitpicking, the rest looks good.
Thanks,
Daniel
3 years, 1 month
[PATCH v4 1/5] session: Add parameter Service into CreateSession call of ConnMan session API
by sutar.mounesh＠gmail.com
From: Bjoern Thorwirth <external.bjoern.thorwirth(a)de.bosch.com>
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.
---
doc/session-api.txt | 12 ++++++++++++
include/session.h | 1 +
src/session.c | 17 +++++++++++++++++
3 files changed, 30 insertions(+)
diff --git a/doc/session-api.txt b/doc/session-api.txt
index e8da522..139bcca 100644
--- a/doc/session-api.txt
+++ b/doc/session-api.txt
@@ -205,3 +205,15 @@ Settings string State [readonly]
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.
+
+ string Service [readwrite]
+
+ This designates context of service for a Session.
+ 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 field is used to implement service context
+ dependent behavior.
diff --git a/include/session.h b/include/session.h
index 5106e88..3b3c53f 100644
--- a/include/session.h
+++ b/include/session.h
@@ -74,6 +74,7 @@ struct connman_session_config {
GSList *allowed_bearers;
char *allowed_interface;
bool source_ip_rule;
+ char *service;
};
typedef int (* connman_session_config_func_t) (struct connman_session *session,
diff --git a/src/session.c b/src/session.c
index 9e3c559..a62f93e 100644
--- a/src/session.c
+++ b/src/session.c
@@ -548,6 +548,7 @@ struct creation_data {
GSList *allowed_bearers;
char *allowed_interface;
bool source_ip_rule;
+ char *service;
};
static void cleanup_creation_data(struct creation_data *creation_data)
@@ -557,6 +558,8 @@ static void cleanup_creation_data(struct creation_data *creation_data)
if (creation_data->pending)
dbus_message_unref(creation_data->pending);
+ if (creation_data->service)
+ g_free(creation_data->service);
g_slist_free(creation_data->allowed_bearers);
g_free(creation_data->allowed_interface);
@@ -927,6 +930,17 @@ static void append_notify(DBusMessageIter *dict,
}
if (session->append_all ||
+ info->config.service != info_last->config.service) {
+ char *ifname = info->config.service;
+ if (!ifname)
+ ifname = "";
+ connman_dbus_dict_append_basic(dict, "Service",
+ DBUS_TYPE_STRING,
+ &ifname);
+ info_last->config.service = info->config.service;
+ }
+
+ if (session->append_all ||
info->config.source_ip_rule != info_last->config.source_ip_rule) {
dbus_bool_t source_ip_rule = FALSE;
if (info->config.source_ip_rule)
@@ -1474,6 +1488,9 @@ int __connman_session_create(DBusMessage *msg)
connman_session_parse_connection_type(val);
user_connection_type = true;
+ } else if (g_str_equal(key, "Service")) {
+ dbus_message_iter_get_basic(&value, &val);
+ creation_data->service = g_strdup(val);
} else if (g_str_equal(key, "AllowedInterface")) {
dbus_message_iter_get_basic(&value, &val);
creation_data->allowed_interface = g_strdup(val);
--
2.7.4
3 years, 1 month
