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 2/3] src/proxy.c: modify the proxy_lookup ()
      supporting non-browser schemes (David Woodhouse)
   2. Re: [PATCH 3/3] unit: Add tests for BrowserOnly Key
      (David Woodhouse)
   3. [PATCH v4 0/6] [PATCH v3 0/6] Add nftables support (Daniel Wagner)
   4. [PATCH v4 1/6] session: Install SNAT rules only once per
      device (Daniel Wagner)


----------------------------------------------------------------------

Message: 1
Date: Thu, 18 Aug 2016 09:24:01 +0100
From: David Woodhouse <[email protected]>
To: Atul Anand <[email protected]>, [email protected]
Subject: Re: [PATCH 2/3] src/proxy.c: modify the proxy_lookup ()
        supporting non-browser schemes
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"

On Tue, 2016-08-16 at 20:19 +0530, Atul Anand wrote:
> As discussed, the proxy lookup for browser and non browser schemes should
> be handled in an order as follows:
> A request for a "browser" protocol would match the following configs
> order of preference (if they exist):
>  ? Matching "Domains", BrowserOnly==TRUE
>  ? Matching "Domains", BrowserOnly==FALSE
>  ? Domains==NULL, BrowserOnly==TRUE
>  ? Domains==NULL, BrowserOnly==FALSE
> 
> A request for a non-browser protocol would match the following:
>  ? Matching "Domains", BrowserOnly==FALSE
>  ? Domains==NULL, BrowserOnly==FALSE (except if a config exists with
>    Matching "Domains", BrowserOnly==TRUE, in which case we need to
>    return NULL).

I might be inclined to actually copy the above 'table' into a comment
right above pacrunner_proxy_lookup() instead of leaving it only in the
commit log.

And then yes, I like this version more than the previous one. I find it
easier to see what's going on in the match code. Thanks.

--?
-- 
David Woodhouse                            Open Source Technology Centre
[email protected]                              Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5760 bytes
Desc: not available
URL: 
<http://lists.01.org/pipermail/connman/attachments/20160818/82f46057/attachment-0001.bin>

------------------------------

Message: 2
Date: Thu, 18 Aug 2016 09:28:51 +0100
From: David Woodhouse <[email protected]>
To: Atul Anand <[email protected]>, [email protected]
Subject: Re: [PATCH 3/3] unit: Add tests for BrowserOnly Key
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"

On Tue, 2016-08-16 at 20:19 +0530, Atul Anand wrote:
> test-pacrunner and suites have been fixed to test BrowserOnly key
> and it's function.

http://www.angryflower.com/itsits.gif?:)

The table in patch 2 had about 6 different cases to test, if not
more...

> A request for a "browser" protocol would match the following configs
> order of preference (if they exist):
>??? Matching "Domains", BrowserOnly==TRUE
>??? Matching "Domains", BrowserOnly==FALSE
>??? Domains==NULL, BrowserOnly==TRUE
>??? Domains==NULL, BrowserOnly==FALSE

So here you want tests which install a subset of those *four* different
proxy configs (let's call them A, B, C and D) and then check the
results.

So one test would install just B, C and D... and check that the result
it gets is actually from B, not from one of the others.

Another test would install only C and D, and check it gets C.

> A request for a non-browser protocol would match the following:
>??? Matching "Domains", BrowserOnly==FALSE
>??? Domains==NULL, BrowserOnly==FALSE (except if a config exists with
>????Matching "Domains", BrowserOnly==TRUE, in which case we need to
>????return NULL).

Likewise we need tests to exercise these and especially the special
case.

I don't think your added tests are covering everything they need to,
are they?

-- 
David Woodhouse                            Open Source Technology Centre
[email protected]                              Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5760 bytes
Desc: not available
URL: 
<http://lists.01.org/pipermail/connman/attachments/20160818/a7f810a8/attachment-0001.bin>

------------------------------

Message: 3
Date: Thu, 18 Aug 2016 11:06:11 +0200
From: Daniel Wagner <[email protected]>
To: [email protected]
Cc: Daniel Wagner <[email protected]>
Subject: [PATCH v4 0/6] [PATCH v3 0/6] Add nftables support
Message-ID: <[email protected]>

From: Daniel Wagner <[email protected]>

Thanks Dragos for testing and feedback. I tested NAT so far which
works nicely for me.

changes v4:
        - fixed nat rule (routing)
        - rebased on current HEAD

changes v3:
        - fixed error handling when cleaning up (this time for real)
        - dropped chain handlers (not used)
        - tell kernel to load modules if needed via NLM_F_CREATE
        - mask saddr address with netmask for NAT rule (bug fix)

changes v2:
        - rebased to current master
        - fixed some error handling path (memory leak)
        - fixed typo and error handling reported by dtatulea
        - compiler complains
        - issue no warning if table cleaning up was successful

Daniel Wagner (6):
  session: Install SNAT rules only once per device
  firewall: Initialize iptables directly from firewall.c
  firewall: Add explicit feature API
  firewall: Rename firewall.c to firewall-iptables.c
  firewall: Add nftables build infrastructure
  firewall-nftables: Add nftable support for firewall

 Makefile.am             |   48 +-
 configure.ac            |   31 +-
 src/connman.h           |   22 +-
 src/firewall-iptables.c |  622 +++++++++++++++++++++++++
 src/firewall-nftables.c | 1153 +++++++++++++++++++++++++++++++++++++++++++++++
 src/firewall.c          |  542 ----------------------
 src/main.c              |    2 -
 src/nat.c               |   21 +-
 src/session.c           |  187 ++++----
 tools/iptables-unit.c   |  112 -----
 10 files changed, 1940 insertions(+), 800 deletions(-)
 create mode 100644 src/firewall-iptables.c
 create mode 100644 src/firewall-nftables.c
 delete mode 100644 src/firewall.c

-- 
2.7.4


------------------------------

Message: 4
Date: Thu, 18 Aug 2016 11:06:12 +0200
From: Daniel Wagner <[email protected]>
To: [email protected]
Cc: Daniel Wagner <[email protected]>
Subject: [PATCH v4 1/6] session: Install SNAT rules only once per
        device
Message-ID: <[email protected]>

From: Daniel Wagner <[email protected]>

The marking rules are different from each session. The SNAT
routing rule is only needed once per devices. Therefore, we share the
rule between the session and should only install unique rules.

Note the live time management needs to track which session
installed/refd the rule to avoid removal from update_nat_rules()
on a session create event.
---
 src/session.c | 148 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 115 insertions(+), 33 deletions(-)

diff --git a/src/session.c b/src/session.c
index 388aae7..1205935 100644
--- a/src/session.c
+++ b/src/session.c
@@ -69,7 +69,6 @@ struct connman_session {
 
        enum connman_session_id_type id_type;
        struct firewall_context *fw;
-       int snat_id;
        uint32_t mark;
        int index;
        char *gateway;
@@ -81,6 +80,15 @@ struct connman_service_info {
        GSList *sessions;
 };
 
+struct fw_snat {
+       GSList *sessions;
+       int id;
+       int index;
+       struct firewall_context *fw;
+};
+
+GSList *fw_snat_list;
+
 static struct connman_session_policy *policy;
 static void session_activate(struct connman_session *session);
 static void session_deactivate(struct connman_session *session);
@@ -197,6 +205,87 @@ static char *service2bearer(enum connman_service_type type)
        return "";
 }
 
+static struct fw_snat *fw_snat_lookup(int index)
+{
+       struct fw_snat *fw_snat;
+       GSList *list;
+
+       for (list = fw_snat_list; list; list = list->next) {
+               fw_snat = list->data;
+
+               if (fw_snat->index == index)
+                       return fw_snat;
+       }
+       return NULL;
+}
+
+static int fw_snat_create(struct connman_session *session,
+                               int index, const char *ifname, const char *addr)
+{
+       struct fw_snat *fw_snat;
+       int err;
+
+       fw_snat = g_new0(struct fw_snat, 1);
+
+       fw_snat->fw = __connman_firewall_create();
+       fw_snat->index = index;
+
+       fw_snat->id = __connman_firewall_add_rule(fw_snat->fw,
+                               "nat", "POSTROUTING",
+                               "-o %s -j SNAT --to-source %s",
+                               ifname, addr);
+       if (fw_snat->id < 0) {
+               err = fw_snat->id;
+               goto err;
+       }
+
+       err = __connman_firewall_enable_rule(fw_snat->fw, fw_snat->id);
+       if (err < 0) {
+               __connman_firewall_remove_rule(fw_snat->fw, fw_snat->id);
+               goto err;
+       }
+
+       fw_snat_list = g_slist_prepend(fw_snat_list, fw_snat);
+       fw_snat->sessions = g_slist_prepend(fw_snat->sessions, session);
+
+       return 0;
+err:
+       __connman_firewall_destroy(fw_snat->fw);
+       g_free(fw_snat);
+       return err;
+}
+
+static void fw_snat_ref(struct connman_session *session,
+                               struct fw_snat *fw_snat)
+{
+       if (g_slist_find(fw_snat->sessions, session))
+               return;
+       fw_snat->sessions = g_slist_prepend(fw_snat->sessions, session);
+}
+
+static void fw_snat_unref(struct connman_session *session,
+                               struct fw_snat *fw_snat)
+{
+       int err;
+
+       fw_snat->sessions = g_slist_remove(fw_snat->sessions, session);
+       if (fw_snat->sessions)
+               return;
+
+       fw_snat_list = g_slist_remove(fw_snat_list, fw_snat);
+
+       err = __connman_firewall_disable_rule(fw_snat->fw, fw_snat->id);
+       if (err < 0)
+               DBG("could not disable SNAT rule");
+
+       err = __connman_firewall_remove_rule(fw_snat->fw, fw_snat->id);
+       if (err < 0)
+               DBG("could not remove SNAT rule");
+
+       __connman_firewall_destroy(fw_snat->fw);
+       g_free(fw_snat);
+}
+
 static int init_firewall(void)
 {
        struct firewall_context *fw;
@@ -368,59 +457,46 @@ static void add_default_route(struct connman_session 
*session)
 
 static void del_nat_rules(struct connman_session *session)
 {
-       int err;
+       struct fw_snat *fw_snat;
 
-       if (!session->fw || session->snat_id == 0)
-               return;
+       fw_snat = fw_snat_lookup(session->index);
 
-       err = __connman_firewall_disable_rule(session->fw, session->snat_id);
-       if (err < 0) {
-               DBG("could not disable SNAT rule");
+       if (!fw_snat)
                return;
-       }
-
-       err = __connman_firewall_remove_rule(session->fw, session->snat_id);
-       if (err < 0)
-               DBG("could not remove SNAT rule");
 
-
-       session->snat_id = 0;
+       fw_snat_unref(session, fw_snat);
 }
 
 static void add_nat_rules(struct connman_session *session)
 {
        struct connman_ipconfig *ipconfig;
+       struct fw_snat *fw_snat;
        const char *addr;
        char *ifname;
-       int index, id, err;
-
-       if (!session->fw)
-               return;
+       int index, err;
 
        DBG("");
 
+       if (!session->service)
+               return;
+
        ipconfig = __connman_service_get_ip4config(session->service);
        index = __connman_ipconfig_get_index(ipconfig);
-       ifname = connman_inet_ifname(index);
-       addr = __connman_ipconfig_get_local(ipconfig);
 
-       id = __connman_firewall_add_rule(session->fw, "nat", "POSTROUTING",
-                               "-o %s -j SNAT --to-source %s",
-                               ifname, addr);
-       g_free(ifname);
-       if (id < 0) {
-               DBG("failed to add SNAT rule");
+       fw_snat = fw_snat_lookup(index);
+       if (fw_snat) {
+               fw_snat_ref(session, fw_snat);
                return;
        }
 
-       err = __connman_firewall_enable_rule(session->fw, id);
-       if (err < 0) {
-               DBG("could not enable SNAT rule");
-               __connman_firewall_remove_rule(session->fw, id);
-               return;
-       }
+       ifname = connman_inet_ifname(index);
+       addr = __connman_ipconfig_get_local(ipconfig);
+
+       err = fw_snat_create(session, index, ifname, addr);
+       if (err < 0)
+               DBG("failed to add SNAT rule");
 
-       session->snat_id = id;
+       g_free(ifname);
 }
 
 static void cleanup_routing_table(struct connman_session *session)
@@ -445,6 +521,11 @@ static void update_routing_table(struct connman_session 
*session)
        add_default_route(session);
 }
 
+static void cleanup_nat_rules(struct connman_session *session)
+{
+       del_nat_rules(session);
+}
+
 static void update_nat_rules(struct connman_session *session)
 {
        del_nat_rules(session);
@@ -498,6 +579,7 @@ static void cleanup_session(gpointer user_data)
 
        DBG("remove %s", session->session_path);
 
+       cleanup_nat_rules(session);
        cleanup_routing_table(session);
        cleanup_firewall_session(session);
 
-- 
2.7.4


------------------------------

Subject: Digest Footer

_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman


------------------------------

End of connman Digest, Vol 10, Issue 19
***************************************

Reply via email to