From owner-svn-src-projects@FreeBSD.ORG Wed Apr 4 14:31:49 2012 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 46AB0106566B; Wed, 4 Apr 2012 14:31:49 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 306D48FC0A; Wed, 4 Apr 2012 14:31:49 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id q34EVnTG008033; Wed, 4 Apr 2012 14:31:49 GMT (envelope-from glebius@svn.freebsd.org) Received: (from glebius@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id q34EVmVE008028; Wed, 4 Apr 2012 14:31:48 GMT (envelope-from glebius@svn.freebsd.org) Message-Id: <201204041431.q34EVmVE008028@svn.freebsd.org> From: Gleb Smirnoff Date: Wed, 4 Apr 2012 14:31:48 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org X-SVN-Group: projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r233873 - projects/pf/head/sys/contrib/pf/net X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Apr 2012 14:31:49 -0000 Author: glebius Date: Wed Apr 4 14:31:48 2012 New Revision: 233873 URL: http://svn.freebsd.org/changeset/base/233873 Log: - ID lookup structure should match only on state id and on creator id, ignoring direction and padding. This fixes state lookup mismatches after r233782. - Make pf_find_state_byid() take couple of id arguments. This almost retires struct pf_state_key usage and simplifies code. - Change struct pfsync_state to use uint64_t for id. This is wire-compatible with old struct, and simplifies code. OpenBSD also did this. Modified: projects/pf/head/sys/contrib/pf/net/if_pfsync.c projects/pf/head/sys/contrib/pf/net/pf.c projects/pf/head/sys/contrib/pf/net/pf_ioctl.c projects/pf/head/sys/contrib/pf/net/pfvar.h Modified: projects/pf/head/sys/contrib/pf/net/if_pfsync.c ============================================================================== --- projects/pf/head/sys/contrib/pf/net/if_pfsync.c Wed Apr 4 13:49:22 2012 (r233872) +++ projects/pf/head/sys/contrib/pf/net/if_pfsync.c Wed Apr 4 14:31:48 2012 (r233873) @@ -227,7 +227,8 @@ struct pfsync_softc { u_int32_t sc_ureq_received; int sc_bulk_hash_id; - struct pf_state_cmp sc_bulk_state; + uint64_t sc_bulk_stateid; + uint32_t sc_bulk_creatorid; struct callout sc_bulk_tmo; struct callout sc_tmo; @@ -517,7 +518,7 @@ pfsync_state_import(struct pfsync_state st->timeout = sp->timeout; st->state_flags = sp->state_flags; - bcopy(sp->id, &st->id, sizeof(st->id)); + st->id = sp->id; st->creatorid = sp->creatorid; pf_state_peer_ntoh(&sp->src, &st->src); pf_state_peer_ntoh(&sp->dst, &st->dst); @@ -751,7 +752,6 @@ static int pfsync_in_iack(struct pfsync_pkt *pkt, struct mbuf *m, int offset, int count) { struct pfsync_ins_ack *ia, *iaa; - struct pf_state_cmp id_key; struct pf_state *st; struct mbuf *mp; @@ -769,10 +769,7 @@ pfsync_in_iack(struct pfsync_pkt *pkt, s for (i = 0; i < count; i++) { ia = &iaa[i]; - bcopy(&ia->id, &id_key.id, sizeof(id_key.id)); - id_key.creatorid = ia->creatorid; - - st = pf_find_state_byid(&id_key); + st = pf_find_state_byid(ia->id, ia->creatorid); if (st == NULL) continue; @@ -827,7 +824,6 @@ static int pfsync_in_upd(struct pfsync_pkt *pkt, struct mbuf *m, int offset, int count) { struct pfsync_state *sa, *sp; - struct pf_state_cmp id_key; struct pf_state_key *sk; struct pf_state *st; int sfail; @@ -859,10 +855,7 @@ pfsync_in_upd(struct pfsync_pkt *pkt, st continue; } - bcopy(sp->id, &id_key.id, sizeof(id_key.id)); - id_key.creatorid = sp->creatorid; - - st = pf_find_state_byid(&id_key); + st = pf_find_state_byid(sp->id, sp->creatorid); if (st == NULL) { /* insert the update */ if (pfsync_state_import(sp, 0)) @@ -921,7 +914,6 @@ pfsync_in_upd_c(struct pfsync_pkt *pkt, { struct pfsync_upd_c *ua, *up; struct pf_state_key *sk; - struct pf_state_cmp id_key; struct pf_state *st; int len = count * sizeof(*up); @@ -954,13 +946,10 @@ pfsync_in_upd_c(struct pfsync_pkt *pkt, continue; } - bcopy(&up->id, &id_key.id, sizeof(id_key.id)); - id_key.creatorid = up->creatorid; - - st = pf_find_state_byid(&id_key); + st = pf_find_state_byid(up->id, up->creatorid); if (st == NULL) { /* We don't have this state. Ask for it. */ - pfsync_request_update(id_key.creatorid, id_key.id); + pfsync_request_update(up->creatorid, up->id); continue; } @@ -1017,7 +1006,6 @@ pfsync_in_ureq(struct pfsync_pkt *pkt, s int len = count * sizeof(*ur); int i, offp; - struct pf_state_cmp id_key; struct pf_state *st; mp = m_pulldown(m, offset, len, &offp); @@ -1031,13 +1019,10 @@ pfsync_in_ureq(struct pfsync_pkt *pkt, s for (i = 0; i < count; i++) { ur = &ura[i]; - bcopy(&ur->id, &id_key.id, sizeof(id_key.id)); - id_key.creatorid = ur->creatorid; - - if (id_key.id == 0 && id_key.creatorid == 0) + if (ur->id == 0 && ur->creatorid == 0) pfsync_bulk_start(); else { - st = pf_find_state_byid(&id_key); + st = pf_find_state_byid(ur->id, ur->creatorid); if (st == NULL) { V_pfsyncstats.pfsyncs_badstate++; continue; @@ -1061,7 +1046,6 @@ pfsync_in_del(struct pfsync_pkt *pkt, st { struct mbuf *mp; struct pfsync_state *sa, *sp; - struct pf_state_cmp id_key; struct pf_state *st; int len = count * sizeof(*sp); int offp, i; @@ -1077,10 +1061,7 @@ pfsync_in_del(struct pfsync_pkt *pkt, st for (i = 0; i < count; i++) { sp = &sa[i]; - bcopy(sp->id, &id_key.id, sizeof(id_key.id)); - id_key.creatorid = sp->creatorid; - - st = pf_find_state_byid(&id_key); + st = pf_find_state_byid(sp->id, sp->creatorid); if (st == NULL) { V_pfsyncstats.pfsyncs_badstate++; continue; @@ -1098,7 +1079,6 @@ pfsync_in_del_c(struct pfsync_pkt *pkt, { struct mbuf *mp; struct pfsync_del_c *sa, *sp; - struct pf_state_cmp id_key; struct pf_state *st; int len = count * sizeof(*sp); int offp, i; @@ -1114,10 +1094,7 @@ pfsync_in_del_c(struct pfsync_pkt *pkt, for (i = 0; i < count; i++) { sp = &sa[i]; - bcopy(&sp->id, &id_key.id, sizeof(id_key.id)); - id_key.creatorid = sp->creatorid; - - st = pf_find_state_byid(&id_key); + st = pf_find_state_byid(sp->id, sp->creatorid); if (st == NULL) { V_pfsyncstats.pfsyncs_badstate++; continue; @@ -2032,7 +2009,7 @@ pfsync_bulk_start(void) sc->sc_ureq_received = time_uptime; sc->sc_bulk_hash_id = 0; - bzero(&sc->sc_bulk_state, sizeof(struct pf_state_cmp)); + sc->sc_bulk_stateid = 0; pfsync_bulk_status(PFSYNC_BUS_START); callout_reset(&sc->sc_bulk_tmo, 1, pfsync_bulk_update, sc); } @@ -2052,7 +2029,7 @@ pfsync_bulk_update(void *arg) * It may had gone, in this case start from the * hash slot. */ - s = pf_find_state_byid(&sc->sc_bulk_state); + s = pf_find_state_byid(sc->sc_bulk_stateid, sc->sc_bulk_creatorid); if (s != NULL) i = PF_IDHASH(s); @@ -2075,8 +2052,8 @@ pfsync_bulk_update(void *arg) sizeof(struct pfsync_state)) { /* We've filled a packet. */ sc->sc_bulk_hash_id = i; - bcopy(s, &sc->sc_bulk_state, - sizeof(struct pf_state_cmp)); + sc->sc_bulk_stateid = s->id; + sc->sc_bulk_creatorid = s->creatorid; PF_HASHROW_UNLOCK(ih); callout_reset(&sc->sc_bulk_tmo, 1, pfsync_bulk_update, sc); Modified: projects/pf/head/sys/contrib/pf/net/pf.c ============================================================================== --- projects/pf/head/sys/contrib/pf/net/pf.c Wed Apr 4 13:49:22 2012 (r233872) +++ projects/pf/head/sys/contrib/pf/net/pf.c Wed Apr 4 14:31:48 2012 (r233873) @@ -981,7 +981,7 @@ pf_state_insert(struct pfi_kif *kif, str ih = &V_pf_idhash[PF_IDHASH(s)]; PF_HASHROW_LOCK(ih); LIST_FOREACH(cur, &ih->states, entry) - if (bcmp(cur, s, sizeof(struct pf_state_cmp)) == 0) + if (cur->id == s->id && cur->creatorid == s->creatorid) break; if (cur != NULL) { @@ -1014,16 +1014,18 @@ pf_state_insert(struct pfi_kif *kif, str * Find state by ID: returns with locked row on success. */ struct pf_state * -pf_find_state_byid(struct pf_state_cmp *key) +pf_find_state_byid(uint64_t id, uint32_t creatorid) { - struct pf_idhash *ih = &V_pf_idhash[PF_IDHASH(key)]; + struct pf_idhash *ih; struct pf_state *s; V_pf_status.fcounters[FCNT_STATE_SEARCH]++; + ih = &V_pf_idhash[(be64toh(id) % (V_pf_hashmask + 1))]; + PF_HASHROW_LOCK(ih); LIST_FOREACH(s, &ih->states, entry) - if (bcmp(s, key, sizeof(struct pf_state_cmp)) == 0) + if (s->id == id && s->creatorid == creatorid) break; if (s == NULL) Modified: projects/pf/head/sys/contrib/pf/net/pf_ioctl.c ============================================================================== --- projects/pf/head/sys/contrib/pf/net/pf_ioctl.c Wed Apr 4 13:49:22 2012 (r233872) +++ projects/pf/head/sys/contrib/pf/net/pf_ioctl.c Wed Apr 4 14:31:48 2012 (r233873) @@ -1709,7 +1709,8 @@ relock_DIOCCLRSTATES: if (psk->psk_pfcmp.id) { if (psk->psk_pfcmp.creatorid == 0) psk->psk_pfcmp.creatorid = V_pf_status.hostid; - if ((s = pf_find_state_byid(&psk->psk_pfcmp))) { + if ((s = pf_find_state_byid(psk->psk_pfcmp.id, + psk->psk_pfcmp.creatorid))) { pf_unlink_state(s, PF_ENTER_LOCKED); psk->psk_killed = 1; } @@ -1793,13 +1794,9 @@ relock_DIOCKILLSTATES: case DIOCGETSTATE: { struct pfioc_state *ps = (struct pfioc_state *)addr; struct pf_state *s; - struct pf_state_cmp id_key; - - bcopy(ps->state.id, &id_key.id, sizeof(id_key.id)); - id_key.creatorid = ps->state.creatorid; PF_LOCK(); - s = pf_find_state_byid(&id_key); + s = pf_find_state_byid(ps->state.id, ps->state.creatorid); if (s == NULL) { PF_UNLOCK(); error = ENOENT; Modified: projects/pf/head/sys/contrib/pf/net/pfvar.h ============================================================================== --- projects/pf/head/sys/contrib/pf/net/pfvar.h Wed Apr 4 13:49:22 2012 (r233872) +++ projects/pf/head/sys/contrib/pf/net/pfvar.h Wed Apr 4 14:31:48 2012 (r233873) @@ -872,7 +872,7 @@ struct pfsync_state_key { }; struct pfsync_state { - u_int32_t id[2]; + u_int64_t id; char ifname[IFNAMSIZ]; struct pfsync_state_key key[2]; struct pfsync_state_peer src; @@ -1809,7 +1809,7 @@ pf_release_state(struct pf_state *s) pf_free_state(s); } -extern struct pf_state *pf_find_state_byid(struct pf_state_cmp *); +extern struct pf_state *pf_find_state_byid(uint64_t, uint32_t); extern struct pf_state *pf_find_state_all(struct pf_state_key_cmp *, u_int, int *); extern void pf_print_state(struct pf_state *);