Module Name: src
Committed By: christos
Date: Sat Jan 5 16:34:43 UTC 2013
Modified Files:
src/sys/external/bsd/ipf/netinet: ip_nat.c ip_nat.h
Log Message:
Fix bucket and chain counts on nat lists from Geoff Adams:
The problem was that ipf_nat_delete wasn't swapping inbound and
outbound hash codes for inbound NAT entries, so it was essentially
always looking in the wrong buckets in those cases. But because of
the way the linked list works, I don't think any NAT entries were
actually leaked. But since all the bucket counters and chain count
were getting messed up, things did start to go bad after a while.
(New NAT entries wouldn't be created, for instance.)
The fix is in the ipf_nat_delete function, itself; the other changes
are a slight refactoring of one method and adding some comments
that helped me figure out how the linked list with pointer-back-pointers
worked.
Also note that I haven't looked through the logic in ipf_nat_rehash;
it's likely that that might miss some things for the same reason.
I also haven't yet looked into the ipf_nat_newrdr problem with mappings
already existing. That'll be next.
To generate a diff of this commit:
cvs rdiff -u -r1.7 -r1.8 src/sys/external/bsd/ipf/netinet/ip_nat.c
cvs rdiff -u -r1.4 -r1.5 src/sys/external/bsd/ipf/netinet/ip_nat.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/external/bsd/ipf/netinet/ip_nat.c
diff -u src/sys/external/bsd/ipf/netinet/ip_nat.c:1.7 src/sys/external/bsd/ipf/netinet/ip_nat.c:1.8
--- src/sys/external/bsd/ipf/netinet/ip_nat.c:1.7 Thu Dec 20 16:42:28 2012
+++ src/sys/external/bsd/ipf/netinet/ip_nat.c Sat Jan 5 11:34:43 2013
@@ -1,4 +1,4 @@
-/* $NetBSD: ip_nat.c,v 1.7 2012/12/20 21:42:28 christos Exp $ */
+/* $NetBSD: ip_nat.c,v 1.8 2013/01/05 16:34:43 christos Exp $ */
/*
* Copyright (C) 2012 by Darren Reed.
@@ -113,7 +113,7 @@ extern struct ifnet vpnif;
#if !defined(lint)
#if defined(__NetBSD__)
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ip_nat.c,v 1.7 2012/12/20 21:42:28 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ip_nat.c,v 1.8 2013/01/05 16:34:43 christos Exp $");
#else
static const char sccsid[] = "@(#)ip_nat.c 1.11 6/5/96 (C) 1995 Darren Reed";
static const char rcsid[] = "@(#)Id: ip_nat.c,v 1.1.1.2 2012/07/22 13:45:27 darrenr Exp";
@@ -226,6 +226,7 @@ static void ipf_nat_addrdr(ipf_nat_softc
static int ipf_nat_builddivertmp(ipf_nat_softc_t *, ipnat_t *);
static int ipf_nat_clearlist(ipf_main_softc_t *, ipf_nat_softc_t *);
static int ipf_nat_cmp_rules(ipnat_t *, ipnat_t *);
+static void ipf_nat_compute_hashes(nat_t *nat);
static int ipf_nat_decap(fr_info_t *, nat_t *);
static void ipf_nat_delrule(ipf_main_softc_t *, ipf_nat_softc_t *,
ipnat_t *, int);
@@ -2253,13 +2254,27 @@ void
ipf_nat_delete(ipf_main_softc_t *softc, struct nat *nat, int logtype)
{
ipf_nat_softc_t *softn = softc->ipf_nat_soft;
- int madeorphan = 0, bkt, removed = 0;
+ int madeorphan = 0, removed = 0;
+ u_int hv0;
+ u_int hv1;
nat_stat_side_t *nss;
struct ipnat *ipn;
if (logtype != 0 && softn->ipf_nat_logging != 0)
ipf_nat_log(softc, softn, nat, logtype);
+ /* Get the hash values, swapped as necessary. */
+ hv0 = nat->nat_hv[0] % softn->ipf_nat_table_sz;
+ hv1 = nat->nat_hv[1] % softn->ipf_nat_table_sz;
+
+ if (nat->nat_dir == NAT_INBOUND || nat->nat_dir == NAT_DIVERTIN) {
+ u_int swap;
+
+ swap = hv0;
+ hv0 = hv1;
+ hv1 = swap;
+ }
+
/*
* Take it as a general indication that all the pointers are set if
* nat_pnext is set.
@@ -2267,17 +2282,15 @@ ipf_nat_delete(ipf_main_softc_t *softc,
if (nat->nat_pnext != NULL) {
removed = 1;
- bkt = nat->nat_hv[0] % softn->ipf_nat_table_sz;
nss = &softn->ipf_nat_stats.ns_side[0];
- nss->ns_bucketlen[bkt]--;
- if (nss->ns_bucketlen[bkt] == 0) {
+ nss->ns_bucketlen[hv0]--;
+ if (nss->ns_bucketlen[hv0] == 0) {
nss->ns_inuse--;
}
- bkt = nat->nat_hv[1] % softn->ipf_nat_table_sz;
nss = &softn->ipf_nat_stats.ns_side[1];
- nss->ns_bucketlen[bkt]--;
- if (nss->ns_bucketlen[bkt] == 0) {
+ nss->ns_bucketlen[hv1]--;
+ if (nss->ns_bucketlen[hv1] == 0) {
nss->ns_inuse--;
}
@@ -3337,58 +3350,48 @@ ipf_nat_finalise(fr_info_t *fin, nat_t *
/* ------------------------------------------------------------------------ */
-/* Function: ipf_nat_insert */
-/* Returns: int - 0 == sucess, -1 == failure */
-/* Parameters: softc(I) - pointer to soft context main structure */
-/* softn(I) - pointer to NAT context structure */
-/* nat(I) - pointer to NAT structure */
+/* Function: ipf_nat_compute_hashes */
+/* Parameters: nat(I) - pointer to NAT structure */
/* Write Lock: ipf_nat */
/* */
-/* Insert a NAT entry into the hash tables for searching and add it to the */
-/* list of active NAT entries. Adjust global counters when complete. */
+/* Compute and set the values for nat->nat_hv[0] and nat->nat_hv[1] */
/* ------------------------------------------------------------------------ */
-int
-ipf_nat_insert(ipf_main_softc_t *softc, ipf_nat_softc_t *softn, nat_t *nat)
+void
+ipf_nat_compute_hashes(nat_t *nat)
{
u_int hv0, hv1;
- u_int sp, dp;
- ipnat_t *in;
- int ret;
+ u_int sport, dport;
- /*
- * Try and return an error as early as possible, so calculate the hash
- * entry numbers first and then proceed.
- */
if ((nat->nat_flags & (SI_W_SPORT|SI_W_DPORT)) == 0) {
if ((nat->nat_flags & IPN_TCPUDP) != 0) {
- sp = nat->nat_osport;
- dp = nat->nat_odport;
+ sport = nat->nat_osport;
+ dport = nat->nat_odport;
} else if ((nat->nat_flags & IPN_ICMPQUERY) != 0) {
- sp = 0;
- dp = nat->nat_oicmpid;
+ sport = 0;
+ dport = nat->nat_oicmpid;
} else {
- sp = 0;
- dp = 0;
+ sport = 0;
+ dport = 0;
}
- hv0 = NAT_HASH_FN(nat->nat_osrcaddr, sp, 0xffffffff);
- hv0 = NAT_HASH_FN(nat->nat_odstaddr, hv0 + dp, 0xffffffff);
+ hv0 = NAT_HASH_FN(nat->nat_osrcaddr, sport, 0xffffffff);
+ hv0 = NAT_HASH_FN(nat->nat_odstaddr, hv0 + dport, 0xffffffff);
/*
* TRACE nat_osrcaddr, nat_osport, nat_odstaddr,
* nat_odport, hv0
*/
if ((nat->nat_flags & IPN_TCPUDP) != 0) {
- sp = nat->nat_nsport;
- dp = nat->nat_ndport;
+ sport = nat->nat_nsport;
+ dport = nat->nat_ndport;
} else if ((nat->nat_flags & IPN_ICMPQUERY) != 0) {
- sp = 0;
- dp = nat->nat_nicmpid;
+ sport = 0;
+ dport = nat->nat_nicmpid;
} else {
- sp = 0;
- dp = 0;
+ sport = 0;
+ dport = 0;
}
- hv1 = NAT_HASH_FN(nat->nat_nsrcaddr, sp, 0xffffffff);
- hv1 = NAT_HASH_FN(nat->nat_ndstaddr, hv1 + dp, 0xffffffff);
+ hv1 = NAT_HASH_FN(nat->nat_nsrcaddr, sport, 0xffffffff);
+ hv1 = NAT_HASH_FN(nat->nat_ndstaddr, hv1 + dport, 0xffffffff);
/*
* TRACE nat_nsrcaddr, nat_nsport, nat_ndstaddr,
* nat_ndport, hv1
@@ -3405,6 +3408,31 @@ ipf_nat_insert(ipf_main_softc_t *softc,
nat->nat_hv[0] = hv0;
nat->nat_hv[1] = hv1;
+}
+
+
+/* ------------------------------------------------------------------------ */
+/* Function: ipf_nat_insert */
+/* Returns: int - 0 == sucess, -1 == failure */
+/* Parameters: softc(I) - pointer to soft context main structure */
+/* softn(I) - pointer to NAT context structure */
+/* nat(I) - pointer to NAT structure */
+/* Write Lock: ipf_nat */
+/* */
+/* Insert a NAT entry into the hash tables for searching and add it to the */
+/* list of active NAT entries. Adjust global counters when complete. */
+/* ------------------------------------------------------------------------ */
+int
+ipf_nat_insert(ipf_main_softc_t *softc, ipf_nat_softc_t *softn, nat_t *nat)
+{
+ ipnat_t *in;
+ int ret;
+
+ /*
+ * Try and return an error as early as possible, so calculate the hash
+ * entry numbers first and then proceed.
+ */
+ ipf_nat_compute_hashes(nat);
MUTEX_INIT(&nat->nat_lock, "nat entry lock");
@@ -4269,14 +4297,14 @@ ipf_nat_tabmove(ipf_nat_softc_t *softn,
if (nat->nat_hnext[0])
nat->nat_hnext[0]->nat_phnext[0] = nat->nat_phnext[0];
*nat->nat_phnext[0] = nat->nat_hnext[0];
- nsp->ns_side[0].ns_bucketlen[nat->nat_hv[0] %
- softn->ipf_nat_table_sz]--;
+ hv0 = nat->nat_hv[0] % softn->ipf_nat_table_sz;
+ nsp->ns_side[0].ns_bucketlen[hv0]--;
if (nat->nat_hnext[1])
nat->nat_hnext[1]->nat_phnext[1] = nat->nat_phnext[1];
*nat->nat_phnext[1] = nat->nat_hnext[1];
- nsp->ns_side[1].ns_bucketlen[nat->nat_hv[1] %
- softn->ipf_nat_table_sz]--;
+ hv1 = nat->nat_hv[1] % softn->ipf_nat_table_sz;
+ nsp->ns_side[1].ns_bucketlen[hv1]--;
/*
* Add into the NAT table in the new position
@@ -7987,6 +8015,8 @@ ipf_nat_rehash(ipf_main_softc_t *softc,
softn->ipf_nat_stats.ns_side[0].ns_inuse = 0;
softn->ipf_nat_stats.ns_side[1].ns_inuse = 0;
+ /* XXX(gadams): Still need to deal with hv0/hv1 swapping! */
+
for (nat = softn->ipf_nat_instances; nat != NULL; nat = nat->nat_next) {
nat->nat_hnext[0] = NULL;
nat->nat_phnext[0] = NULL;
Index: src/sys/external/bsd/ipf/netinet/ip_nat.h
diff -u src/sys/external/bsd/ipf/netinet/ip_nat.h:1.4 src/sys/external/bsd/ipf/netinet/ip_nat.h:1.5
--- src/sys/external/bsd/ipf/netinet/ip_nat.h:1.4 Sat Sep 15 12:56:45 2012
+++ src/sys/external/bsd/ipf/netinet/ip_nat.h Sat Jan 5 11:34:43 2013
@@ -1,4 +1,4 @@
-/* $NetBSD: ip_nat.h,v 1.4 2012/09/15 16:56:45 plunky Exp $ */
+/* $NetBSD: ip_nat.h,v 1.5 2013/01/05 16:34:43 christos Exp $ */
/*
* Copyright (C) 2012 by Darren Reed.
@@ -97,10 +97,10 @@ struct ap_session;
*/
typedef struct nat {
ipfmutex_t nat_lock;
- struct nat *nat_next;
- struct nat **nat_pnext;
- struct nat *nat_hnext[2];
- struct nat **nat_phnext[2];
+ struct nat *nat_next; /* next nat_t in global linked list */
+ struct nat **nat_pnext; /* ptr to the nat_next that points here */
+ struct nat *nat_hnext[2]; /* next nat_t in hashtable bucket */
+ struct nat **nat_phnext[2]; /* ptr to nat_hnext that points here */
struct hostmap *nat_hm;
void *nat_data;
struct nat **nat_me;
