On 08/04/15(Wed) 12:15, Mike Belopuhov wrote:
> OK to rename gettdbbyaddr to gettdbbydst since that's what it does
> and it aligns well with gettdbbysrc?
Makes sense to me. This is also coherent to the comments in ip_ipsp.h.
One comment though, you're moving a splsoftnet() in tdb_delete() but it
seems that the hash function does not need this protection, or am I
misunderstanding something?
> ---
> sys/netinet/ip_ipsp.c | 47 +++++++++++++++++++++++++----------------------
> sys/netinet/ip_ipsp.h | 4 ++--
> sys/netinet/ip_spd.c | 6 +++---
> 3 files changed, 30 insertions(+), 27 deletions(-)
>
> diff --git sys/netinet/ip_ipsp.c sys/netinet/ip_ipsp.c
> index f6e598f..edeabc8 100644
> --- sys/netinet/ip_ipsp.c
> +++ sys/netinet/ip_ipsp.c
> @@ -118,21 +118,21 @@ struct xformsw xformsw[] = {
> tcp_signature_tdb_zeroize, tcp_signature_tdb_input,
> tcp_signature_tdb_output, }
> #endif /* TCP_SIGNATURE */
> };
>
> struct xformsw *xformswNXFORMSW = &xformsw[nitems(xformsw)];
>
> #define TDB_HASHSIZE_INIT 32
>
> static struct tdb **tdbh = NULL;
> -static struct tdb **tdbaddr = NULL;
> +static struct tdb **tdbdst = NULL;
> static struct tdb **tdbsrc = NULL;
> static u_int tdb_hashmask = TDB_HASHSIZE_INIT - 1;
> static int tdb_count;
>
> /*
> * Our hashing function needs to stir things with a non-zero random
> multiplier
> * so we cannot be DoS-attacked via choosing of the data to hash.
> */
> int
> tdb_hash(u_int rdomain, u_int32_t spi, union sockaddr_union *dst,
> @@ -393,34 +393,34 @@ ipsp_aux_match(struct tdb *tdb,
> }
>
> return 1;
> }
>
> /*
> * Get an SA given the remote address, the security protocol type, and
> * the desired IDs.
> */
> struct tdb *
> -gettdbbyaddr(u_int rdomain, union sockaddr_union *dst, u_int8_t sproto,
> +gettdbbydst(u_int rdomain, union sockaddr_union *dst, u_int8_t sproto,
> struct ipsec_ref *srcid, struct ipsec_ref *dstid,
> struct ipsec_ref *local_cred, struct sockaddr_encap *filter,
> struct sockaddr_encap *filtermask)
> {
> u_int32_t hashval;
> struct tdb *tdbp;
>
> - if (tdbaddr == NULL)
> + if (tdbdst == NULL)
> return (struct tdb *) NULL;
>
> hashval = tdb_hash(rdomain, 0, dst, sproto);
>
> - for (tdbp = tdbaddr[hashval]; tdbp != NULL; tdbp = tdbp->tdb_anext)
> + for (tdbp = tdbdst[hashval]; tdbp != NULL; tdbp = tdbp->tdb_dnext)
> if ((tdbp->tdb_sproto == sproto) &&
> (tdbp->tdb_rdomain == rdomain) &&
> ((tdbp->tdb_flags & TDBF_INVALID) == 0) &&
> (!memcmp(&tdbp->tdb_dst, dst, SA_LEN(&dst->sa)))) {
> /* Do IDs and local credentials match ? */
> if (!ipsp_aux_match(tdbp, srcid, dstid,
> local_cred, NULL, filter, filtermask))
> continue;
> break;
> }
> @@ -576,85 +576,85 @@ tdb_soft_firstuse(void *v)
> pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
> tdb->tdb_flags &= ~TDBF_SOFT_FIRSTUSE;
> }
>
> /*
> * Caller is responsible for splsoftnet().
> */
> void
> tdb_rehash(void)
> {
> - struct tdb **new_tdbh, **new_tdbaddr, **new_srcaddr, *tdbp, *tdbnp;
> + struct tdb **new_tdbh, **new_tdbdst, **new_srcaddr, *tdbp, *tdbnp;
> u_int i, old_hashmask = tdb_hashmask;
> u_int32_t hashval;
>
> tdb_hashmask = (tdb_hashmask << 1) | 1;
>
> new_tdbh = mallocarray(tdb_hashmask + 1, sizeof(struct tdb *), M_TDB,
> M_WAITOK | M_ZERO);
> - new_tdbaddr = mallocarray(tdb_hashmask + 1, sizeof(struct tdb *), M_TDB,
> + new_tdbdst = mallocarray(tdb_hashmask + 1, sizeof(struct tdb *), M_TDB,
> M_WAITOK | M_ZERO);
> new_srcaddr = mallocarray(tdb_hashmask + 1, sizeof(struct tdb *), M_TDB,
> M_WAITOK | M_ZERO);
>
> for (i = 0; i <= old_hashmask; i++) {
> for (tdbp = tdbh[i]; tdbp != NULL; tdbp = tdbnp) {
> tdbnp = tdbp->tdb_hnext;
> hashval = tdb_hash(tdbp->tdb_rdomain,
> tdbp->tdb_spi, &tdbp->tdb_dst,
> tdbp->tdb_sproto);
> tdbp->tdb_hnext = new_tdbh[hashval];
> new_tdbh[hashval] = tdbp;
> }
>
> - for (tdbp = tdbaddr[i]; tdbp != NULL; tdbp = tdbnp) {
> - tdbnp = tdbp->tdb_anext;
> + for (tdbp = tdbdst[i]; tdbp != NULL; tdbp = tdbnp) {
> + tdbnp = tdbp->tdb_dnext;
> hashval = tdb_hash(tdbp->tdb_rdomain,
> 0, &tdbp->tdb_dst,
> tdbp->tdb_sproto);
> - tdbp->tdb_anext = new_tdbaddr[hashval];
> - new_tdbaddr[hashval] = tdbp;
> + tdbp->tdb_dnext = new_tdbdst[hashval];
> + new_tdbdst[hashval] = tdbp;
> }
>
> for (tdbp = tdbsrc[i]; tdbp != NULL; tdbp = tdbnp) {
> tdbnp = tdbp->tdb_snext;
> hashval = tdb_hash(tdbp->tdb_rdomain,
> 0, &tdbp->tdb_src,
> tdbp->tdb_sproto);
> tdbp->tdb_snext = new_srcaddr[hashval];
> new_srcaddr[hashval] = tdbp;
> }
> }
>
> free(tdbh, M_TDB, 0);
> tdbh = new_tdbh;
>
> - free(tdbaddr, M_TDB, 0);
> - tdbaddr = new_tdbaddr;
> + free(tdbdst, M_TDB, 0);
> + tdbdst = new_tdbdst;
>
> free(tdbsrc, M_TDB, 0);
> tdbsrc = new_srcaddr;
> }
>
> /*
> * Add TDB in the hash table.
> */
> void
> puttdb(struct tdb *tdbp)
> {
> u_int32_t hashval;
> int s = splsoftnet();
>
> if (tdbh == NULL) {
> tdbh = mallocarray(tdb_hashmask + 1, sizeof(struct tdb *),
> M_TDB, M_WAITOK | M_ZERO);
> - tdbaddr = mallocarray(tdb_hashmask + 1, sizeof(struct tdb *),
> + tdbdst = mallocarray(tdb_hashmask + 1, sizeof(struct tdb *),
> M_TDB, M_WAITOK | M_ZERO);
> tdbsrc = mallocarray(tdb_hashmask + 1, sizeof(struct tdb *),
> M_TDB, M_WAITOK | M_ZERO);
> }
>
> hashval = tdb_hash(tdbp->tdb_rdomain, tdbp->tdb_spi,
> &tdbp->tdb_dst, tdbp->tdb_sproto);
>
> /*
> * Rehash if this tdb would cause a bucket to have more than
> @@ -669,22 +669,22 @@ puttdb(struct tdb *tdbp)
> tdb_rehash();
> hashval = tdb_hash(tdbp->tdb_rdomain, tdbp->tdb_spi,
> &tdbp->tdb_dst, tdbp->tdb_sproto);
> }
>
> tdbp->tdb_hnext = tdbh[hashval];
> tdbh[hashval] = tdbp;
>
> hashval = tdb_hash(tdbp->tdb_rdomain, 0, &tdbp->tdb_dst,
> tdbp->tdb_sproto);
> - tdbp->tdb_anext = tdbaddr[hashval];
> - tdbaddr[hashval] = tdbp;
> + tdbp->tdb_dnext = tdbdst[hashval];
> + tdbdst[hashval] = tdbp;
>
> hashval = tdb_hash(tdbp->tdb_rdomain, 0, &tdbp->tdb_src,
> tdbp->tdb_sproto);
> tdbp->tdb_snext = tdbsrc[hashval];
> tdbsrc[hashval] = tdbp;
>
> tdb_count++;
>
> ipsec_last_added = time_second;
>
> @@ -697,53 +697,56 @@ puttdb(struct tdb *tdbp)
> void
> tdb_delete(struct tdb *tdbp)
> {
> struct tdb *tdbpp;
> u_int32_t hashval;
> int s;
>
> if (tdbh == NULL)
> return;
>
> + s = splsoftnet();
> +
> hashval = tdb_hash(tdbp->tdb_rdomain, tdbp->tdb_spi,
> &tdbp->tdb_dst, tdbp->tdb_sproto);
>
> - s = splsoftnet();
> if (tdbh[hashval] == tdbp) {
> tdbh[hashval] = tdbp->tdb_hnext;
> } else {
> for (tdbpp = tdbh[hashval]; tdbpp != NULL;
> tdbpp = tdbpp->tdb_hnext) {
> if (tdbpp->tdb_hnext == tdbp) {
> tdbpp->tdb_hnext = tdbp->tdb_hnext;
> break;
> }
> }
> }
>
> tdbp->tdb_hnext = NULL;
>
> hashval = tdb_hash(tdbp->tdb_rdomain, 0, &tdbp->tdb_dst,
> tdbp->tdb_sproto);
>
> - if (tdbaddr[hashval] == tdbp) {
> - tdbaddr[hashval] = tdbp->tdb_anext;
> + if (tdbdst[hashval] == tdbp) {
> + tdbdst[hashval] = tdbp->tdb_dnext;
> } else {
> - for (tdbpp = tdbaddr[hashval]; tdbpp != NULL;
> - tdbpp = tdbpp->tdb_anext) {
> - if (tdbpp->tdb_anext == tdbp) {
> - tdbpp->tdb_anext = tdbp->tdb_anext;
> + for (tdbpp = tdbdst[hashval]; tdbpp != NULL;
> + tdbpp = tdbpp->tdb_dnext) {
> + if (tdbpp->tdb_dnext == tdbp) {
> + tdbpp->tdb_dnext = tdbp->tdb_dnext;
> break;
> }
> }
> }
>
> + tdbp->tdb_dnext = NULL;
> +
> hashval = tdb_hash(tdbp->tdb_rdomain, 0, &tdbp->tdb_src,
> tdbp->tdb_sproto);
>
> if (tdbsrc[hashval] == tdbp) {
> tdbsrc[hashval] = tdbp->tdb_snext;
> }
> else {
> for (tdbpp = tdbsrc[hashval]; tdbpp != NULL;
> tdbpp = tdbpp->tdb_snext) {
> if (tdbpp->tdb_snext == tdbp) {
> diff --git sys/netinet/ip_ipsp.h sys/netinet/ip_ipsp.h
> index 8c24fc1..47a5670 100644
> --- sys/netinet/ip_ipsp.h
> +++ sys/netinet/ip_ipsp.h
> @@ -264,21 +264,21 @@ struct ipsec_policy {
> struct tdb { /* tunnel descriptor block */
> /*
> * Each TDB is on three hash tables: one keyed on dst/spi/sproto,
> * one keyed on dst/sproto, and one keyed on src/sproto. The first
> * is used for finding a specific TDB, the second for finding TDBs
> * for outgoing policy matching, and the third for incoming
> * policy matching. The following three fields maintain the hash
> * queues in those three tables.
> */
> struct tdb *tdb_hnext; /* dst/spi/sproto table */
> - struct tdb *tdb_anext; /* dst/sproto table */
> + struct tdb *tdb_dnext; /* dst/sproto table */
> struct tdb *tdb_snext; /* src/sproto table */
> struct tdb *tdb_inext;
> struct tdb *tdb_onext;
>
> struct xformsw *tdb_xform; /* Transform to use */
> struct enc_xform *tdb_encalgxform; /* Enc algorithm */
> struct auth_hash *tdb_authalgxform; /* Auth algorithm */
> struct comp_algo *tdb_compalgxform; /* Compression algo */
>
> #define TDBF_UNIQUE 0x00001 /* This should not be used by
> others */
> @@ -497,21 +497,21 @@ do {
> \
> uint8_t get_sa_require(struct inpcb *);
> #ifdef ENCDEBUG
> const char *ipsp_address(union sockaddr_union, char *, socklen_t);
> #endif /* ENCDEBUG */
>
> /* TDB management routines */
> void tdb_add_inp(struct tdb *, struct inpcb *, int);
> uint32_t reserve_spi(u_int, u_int32_t, u_int32_t, union sockaddr_union *,
> union sockaddr_union *, u_int8_t, int *);
> struct tdb *gettdb(u_int, u_int32_t, union sockaddr_union *, u_int8_t);
> -struct tdb *gettdbbyaddr(u_int, union sockaddr_union *, u_int8_t,
> +struct tdb *gettdbbydst(u_int, union sockaddr_union *, u_int8_t,
> struct ipsec_ref *, struct ipsec_ref *, struct ipsec_ref *,
> struct sockaddr_encap *, struct sockaddr_encap *);
> struct tdb *gettdbbysrc(u_int, union sockaddr_union *, u_int8_t,
> struct ipsec_ref *, struct ipsec_ref *,
> struct sockaddr_encap *, struct sockaddr_encap *);
> struct tdb *gettdbbysrcdst(u_int, u_int32_t, union sockaddr_union *,
> union sockaddr_union *, u_int8_t);
> void puttdb(struct tdb *);
> void tdb_delete(struct tdb *);
> struct tdb *tdb_alloc(u_int);
> diff --git sys/netinet/ip_spd.c sys/netinet/ip_spd.c
> index 3287e2f..81e22da 100644
> --- sys/netinet/ip_spd.c
> +++ sys/netinet/ip_spd.c
> @@ -393,21 +393,21 @@ ipsp_spd_lookup(struct mbuf *m, int af, int hlen, int
> *error, int direction,
> * destinations exist but are not used, possibly leading to an
> * explosion in the number of acquired SAs).
> */
> if (ipo->ipo_last_searched <= ipsec_last_added) {
> /* "Touch" the entry. */
> if (dignore == 0)
> ipo->ipo_last_searched = time_second;
>
> /* Find an appropriate SA from the existing ones. */
> ipo->ipo_tdb =
> - gettdbbyaddr(rdomain,
> + gettdbbydst(rdomain,
> dignore ? &sdst : &ipo->ipo_dst,
> ipo->ipo_sproto,
> srcid ? srcid : ipo->ipo_srcid,
> dstid ? dstid : ipo->ipo_dstid,
> ipo->ipo_local_cred, &ipo->ipo_addr,
> &ipo->ipo_mask);
> if (ipo->ipo_tdb) {
>
> TAILQ_INSERT_TAIL(&ipo->ipo_tdb->tdb_policy_head,
> ipo, ipo_tdb_next);
> *error = 0;
> @@ -1056,38 +1056,38 @@ ipsp_spd_inp(struct mbuf *m, int af, int hlen, int
> *error, int direction,
> /* XXX Only support one policy/protocol for now. */
> if (inp->inp_ipo != NULL) {
> if (inp->inp_ipo->ipo_last_searched <=
> ipsec_last_added) {
> inp->inp_ipo->ipo_last_searched = time_second;
>
> /* Update, just in case. */
> ipsec_update_policy(inp, inp->inp_ipo, af,
> IPSP_DIRECTION_OUT);
>
> - tdb = gettdbbyaddr(rtable_l2(inp->inp_rtableid),
> + tdb = gettdbbydst(rtable_l2(inp->inp_rtableid),
> &inp->inp_ipo->ipo_dst,
> inp->inp_ipo->ipo_sproto,
> inp->inp_ipo->ipo_srcid,
> inp->inp_ipo->ipo_dstid,
> inp->inp_ipo->ipo_local_cred,
> &inp->inp_ipo->ipo_addr,
> &inp->inp_ipo->ipo_mask);
> }
> } else {
> /*
> * Construct a pseudo-policy, with just the necessary
> * fields.
> */
> ipsec_update_policy(inp, &sipon, af,
> IPSP_DIRECTION_OUT);
>
> - tdb = gettdbbyaddr(rtable_l2(inp->inp_rtableid),
> + tdb = gettdbbydst(rtable_l2(inp->inp_rtableid),
> &sipon.ipo_dst, IPPROTO_ESP, NULL,
> NULL, NULL, &sipon.ipo_addr, &sipon.ipo_mask);
> }
>
> /* If we found an appropriate SA... */
> if (tdb != NULL) {
> tdb_add_inp(tdb, inp, 0); /* Latch onto PCB. */
>
> if (ipo != NULL && ipo->ipo_tdb != NULL &&
> ipo->ipo_tdb != inp->inp_tdb_out && m != NULL)
> --
> 2.3.4
>
