Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/net/npf - npf_nat_freepolicy: handle a race condition wh...



details:   https://anonhg.NetBSD.org/src/rev/8927b59d9ee7
branches:  trunk
changeset: 796383:8927b59d9ee7
user:      rmind <rmind%NetBSD.org@localhost>
date:      Fri May 30 23:26:06 2014 +0000

description:
- npf_nat_freepolicy: handle a race condition when a new connection might
  be associated with a NAT policy which is going away and npfctl reload
  would wait for its natural expiration (potentially long time).
- Remove npf_ruleset_natreload() by merging into npf_ruleset_reload().
- npf_ruleset_reload: eliminate a small time period when a valid NAT
  policy might be inactive during the reload operation.

diffstat:

 sys/net/npf/npf.h         |   21 +++++----
 sys/net/npf/npf_conf.c    |    6 +-
 sys/net/npf/npf_impl.h    |    3 +-
 sys/net/npf/npf_nat.c     |   49 ++++++++++------------
 sys/net/npf/npf_ruleset.c |  101 ++++++++++++++++++++++++++-------------------
 5 files changed, 96 insertions(+), 84 deletions(-)

diffs (truncated from 389 to 300 lines):

diff -r 57e6490fa424 -r 8927b59d9ee7 sys/net/npf/npf.h
--- a/sys/net/npf/npf.h Fri May 30 22:20:48 2014 +0000
+++ b/sys/net/npf/npf.h Fri May 30 23:26:06 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: npf.h,v 1.39 2014/05/19 18:45:51 jakllsch Exp $        */
+/*     $NetBSD: npf.h,v 1.40 2014/05/30 23:26:06 rmind Exp $   */
 
 /*-
  * Copyright (c) 2009-2014 The NetBSD Foundation, Inc.
@@ -203,14 +203,14 @@
 #endif /* _KERNEL */
 
 /* Rule attributes. */
-#define        NPF_RULE_PASS                   0x0001
-#define        NPF_RULE_GROUP                  0x0002
-#define        NPF_RULE_FINAL                  0x0004
-#define        NPF_RULE_STATEFUL               0x0008
-#define        NPF_RULE_RETRST                 0x0010
-#define        NPF_RULE_RETICMP                0x0020
-#define        NPF_RULE_DYNAMIC                0x0040
-#define        NPF_RULE_MULTIENDS              0x0080
+#define        NPF_RULE_PASS                   0x00000001
+#define        NPF_RULE_GROUP                  0x00000002
+#define        NPF_RULE_FINAL                  0x00000004
+#define        NPF_RULE_STATEFUL               0x00000008
+#define        NPF_RULE_RETRST                 0x00000010
+#define        NPF_RULE_RETICMP                0x00000020
+#define        NPF_RULE_DYNAMIC                0x00000040
+#define        NPF_RULE_MULTIENDS              0x00000080
 
 #define        NPF_DYNAMIC_GROUP               (NPF_RULE_GROUP | NPF_RULE_DYNAMIC)
 
@@ -219,6 +219,9 @@
 #define        NPF_RULE_DIMASK                 (NPF_RULE_IN | NPF_RULE_OUT)
 #define        NPF_RULE_FORW                   0x40000000
 
+/* Private range of rule attributes (not public and should not be set). */
+#define        NPF_RULE_PRIVMASK               0x0f000000
+
 #define        NPF_RULE_MAXNAMELEN             64
 #define        NPF_RULE_MAXKEYLEN              32
 
diff -r 57e6490fa424 -r 8927b59d9ee7 sys/net/npf/npf_conf.c
--- a/sys/net/npf/npf_conf.c    Fri May 30 22:20:48 2014 +0000
+++ b/sys/net/npf/npf_conf.c    Fri May 30 23:26:06 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: npf_conf.c,v 1.5 2013/11/22 00:25:51 rmind Exp $       */
+/*     $NetBSD: npf_conf.c,v 1.6 2014/05/30 23:26:06 rmind Exp $       */
 
 /*-
  * Copyright (c) 2013 The NetBSD Foundation, Inc.
@@ -48,7 +48,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_conf.c,v 1.5 2013/11/22 00:25:51 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_conf.c,v 1.6 2014/05/30 23:26:06 rmind Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -146,7 +146,7 @@
        if ((onc = npf_config) != NULL) {
                npf_ruleset_reload(rset, onc->n_rules);
                npf_tableset_reload(tset, onc->n_tables);
-               npf_ruleset_natreload(nset, onc->n_nat_rules);
+               npf_ruleset_reload(nset, onc->n_nat_rules);
        }
 
        /*
diff -r 57e6490fa424 -r 8927b59d9ee7 sys/net/npf/npf_impl.h
--- a/sys/net/npf/npf_impl.h    Fri May 30 22:20:48 2014 +0000
+++ b/sys/net/npf/npf_impl.h    Fri May 30 23:26:06 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: npf_impl.h,v 1.51 2014/05/19 18:45:51 jakllsch Exp $   */
+/*     $NetBSD: npf_impl.h,v 1.52 2014/05/30 23:26:06 rmind Exp $      */
 
 /*-
  * Copyright (c) 2009-2014 The NetBSD Foundation, Inc.
@@ -258,7 +258,6 @@
 void           npf_ruleset_destroy(npf_ruleset_t *);
 void           npf_ruleset_insert(npf_ruleset_t *, npf_rule_t *);
 void           npf_ruleset_reload(npf_ruleset_t *, npf_ruleset_t *);
-void           npf_ruleset_natreload(npf_ruleset_t *, npf_ruleset_t *);
 npf_rule_t *   npf_ruleset_matchnat(npf_ruleset_t *, npf_natpolicy_t *);
 npf_rule_t *   npf_ruleset_sharepm(npf_ruleset_t *, npf_natpolicy_t *);
 void           npf_ruleset_freealg(npf_ruleset_t *, npf_alg_t *);
diff -r 57e6490fa424 -r 8927b59d9ee7 sys/net/npf/npf_nat.c
--- a/sys/net/npf/npf_nat.c     Fri May 30 22:20:48 2014 +0000
+++ b/sys/net/npf/npf_nat.c     Fri May 30 23:26:06 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: npf_nat.c,v 1.27 2014/03/14 11:29:44 rmind Exp $       */
+/*     $NetBSD: npf_nat.c,v 1.28 2014/05/30 23:26:06 rmind Exp $       */
 
 /*-
  * Copyright (c) 2014 Mindaugas Rasiukevicius <rmind at netbsd org>
@@ -71,7 +71,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_nat.c,v 1.27 2014/03/14 11:29:44 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_nat.c,v 1.28 2014/05/30 23:26:06 rmind Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -115,7 +115,6 @@
        LIST_HEAD(, npf_nat)    n_nat_list;
        volatile u_int          n_refcnt;
        kmutex_t                n_lock;
-       kcondvar_t              n_cv;
        npf_portmap_t *         n_portmap;
        /* NPF_NP_CMP_START */
        int                     n_type;
@@ -137,18 +136,23 @@
  * NAT translation entry for a session.
  */
 struct npf_nat {
-       /* Association (list entry and a link pointer) with NAT policy. */
-       LIST_ENTRY(npf_nat)     nt_entry;
+       /* Associated NAT policy. */
        npf_natpolicy_t *       nt_natpolicy;
-       npf_session_t *         nt_session;
-       /* Original address and port (for backwards translation). */
+
+       /*
+        * Original address and port (for backwards translation).
+        * Translation port (for redirects).
+        */
        npf_addr_t              nt_oaddr;
        in_port_t               nt_oport;
-       /* Translation port (for redirects). */
        in_port_t               nt_tport;
+
        /* ALG (if any) associated with this NAT entry. */
        npf_alg_t *             nt_alg;
        uintptr_t               nt_alg_arg;
+
+       LIST_ENTRY(npf_nat)     nt_entry;
+       npf_session_t *         nt_session;
 };
 
 static pool_cache_t            nat_cache       __read_mostly;
@@ -195,7 +199,6 @@
                goto err;
        }
        mutex_init(&np->n_lock, MUTEX_DEFAULT, IPL_SOFTNET);
-       cv_init(&np->n_cv, "npfnatcv");
        LIST_INIT(&np->n_nat_list);
 
        /* Translation IP, mask and port (if applicable). */
@@ -262,20 +265,17 @@
         * Disassociate all entries from the policy.  At this point,
         * new entries can no longer be created for this policy.
         */
-       mutex_enter(&np->n_lock);
-       LIST_FOREACH(nt, &np->n_nat_list, nt_entry) {
-               se = nt->nt_session;
-               KASSERT(se != NULL);
-               npf_session_expire(se);
-       }
-       while (!LIST_EMPTY(&np->n_nat_list)) {
-               cv_wait(&np->n_cv, &np->n_lock);
-       }
-       mutex_exit(&np->n_lock);
+       while (np->n_refcnt) {
+               mutex_enter(&np->n_lock);
+               LIST_FOREACH(nt, &np->n_nat_list, nt_entry) {
+                       se = nt->nt_session;
+                       KASSERT(se != NULL);
+                       npf_session_expire(se);
+               }
+               mutex_exit(&np->n_lock);
 
-       /* Kick the worker - all references should be going away. */
-       npf_worker_signal();
-       while (np->n_refcnt) {
+               /* Kick the worker - all references should be going away. */
+               npf_worker_signal();
                kpause("npfgcnat", false, 1, NULL);
        }
        KASSERT(LIST_EMPTY(&np->n_nat_list));
@@ -285,7 +285,6 @@
                KASSERT((np->n_flags & NPF_NAT_PORTMAP) != 0);
                kmem_free(pm, PORTMAP_MEM_SIZE);
        }
-       cv_destroy(&np->n_cv);
        mutex_destroy(&np->n_lock);
        kmem_free(np, sizeof(npf_natpolicy_t));
 }
@@ -767,10 +766,6 @@
 
        mutex_enter(&np->n_lock);
        LIST_REMOVE(nt, nt_entry);
-       if (LIST_EMPTY(&np->n_nat_list)) {
-               /* Notify any waiters if empty. */
-               cv_broadcast(&np->n_cv);
-       }
        atomic_dec_uint(&np->n_refcnt);
        mutex_exit(&np->n_lock);
 
diff -r 57e6490fa424 -r 8927b59d9ee7 sys/net/npf/npf_ruleset.c
--- a/sys/net/npf/npf_ruleset.c Fri May 30 22:20:48 2014 +0000
+++ b/sys/net/npf/npf_ruleset.c Fri May 30 23:26:06 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: npf_ruleset.c,v 1.30 2013/12/04 01:38:49 rmind Exp $   */
+/*     $NetBSD: npf_ruleset.c,v 1.31 2014/05/30 23:26:06 rmind Exp $   */
 
 /*-
  * Copyright (c) 2009-2013 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_ruleset.c,v 1.30 2013/12/04 01:38:49 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_ruleset.c,v 1.31 2014/05/30 23:26:06 rmind Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -117,6 +117,11 @@
        uint8_t                 r_key[NPF_RULE_MAXKEYLEN];
 };
 
+/*
+ * Private attributes - must be in the NPF_RULE_PRIVMASK range.
+ */
+#define        NPF_RULE_KEEPNAT        (0x01000000 & NPF_RULE_PRIVMASK)
+
 #define        NPF_DYNAMIC_GROUP_P(attr) \
     (((attr) & NPF_DYNAMIC_GROUP) == NPF_DYNAMIC_GROUP)
 
@@ -374,21 +379,27 @@
 }
 
 /*
- * npf_ruleset_reload: share the dynamic rules.
+ * npf_ruleset_reload: prepare the new ruleset by scanning the active
+ * ruleset and 1) sharing the dynamic rules 2) sharing NAT policies.
  *
- * => Active ruleset should be exclusively locked.
+ * => The active (old) ruleset should be exclusively locked.
  */
 void
-npf_ruleset_reload(npf_ruleset_t *rlset, npf_ruleset_t *arlset)
+npf_ruleset_reload(npf_ruleset_t *newset, npf_ruleset_t *oldset)
 {
-       npf_rule_t *rg;
+       npf_rule_t *rg, *rl;
 
        KASSERT(npf_config_locked_p());
 
-       LIST_FOREACH(rg, &rlset->rs_dynamic, r_dentry) {
-               npf_rule_t *arg, *rl;
+       /*
+        * Scan the dynamic rules and share (migrate) if needed.
+        */
+       LIST_FOREACH(rg, &newset->rs_dynamic, r_dentry) {
+               npf_rule_t *actrg;
 
-               if ((arg = npf_ruleset_lookup(arlset, rg->r_name)) == NULL) {
+               /* Look for a dynamic ruleset group with such name. */
+               actrg = npf_ruleset_lookup(oldset, rg->r_name);
+               if (actrg == NULL) {
                        continue;
                }
 
@@ -397,20 +408,52 @@
                 * the rules are still active and therefore accessible for
                 * inspection via the old ruleset.
                 */
-               memcpy(&rg->r_subset, &arg->r_subset, sizeof(rg->r_subset));
+               memcpy(&rg->r_subset, &actrg->r_subset, sizeof(rg->r_subset));
                TAILQ_FOREACH(rl, &rg->r_subset, r_entry) {
                        /*
                         * We can safely migrate to the new all-rule list
                         * and re-set the parent rule, though.
                         */
                        LIST_REMOVE(rl, r_aentry);
-                       LIST_INSERT_HEAD(&rlset->rs_all, rl, r_aentry);
+                       LIST_INSERT_HEAD(&newset->rs_all, rl, r_aentry);
                        rl->r_parent = rg;
                }
        }
 
+       /*
+        * Scan all rules in the new ruleset and share NAT policies.
+        */
+       LIST_FOREACH(rl, &newset->rs_all, r_aentry) {
+               npf_natpolicy_t *np;
+               npf_rule_t *actrl;
+
+               /* Does the rule have a NAT policy associated? */
+               if ((np = rl->r_natp) == NULL) {
+                       continue;
+               }
+               /* Does it match with any policy in the active ruleset? */
+               if ((actrl = npf_ruleset_matchnat(oldset, np)) == NULL) {
+                       continue;
+               }
+
+               /*
+                * Inherit the matching NAT policy and check other ones



Home | Main Index | Thread Index | Old Index