Date: Wed, 9 Jul 2014 18:52:12 +0000 (UTC) From: "Alexander V. Chernikov" <melifaro@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r268465 - projects/ipfw/sys/netpfil/ipfw Message-ID: <201407091852.s69IqC67056764@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: melifaro Date: Wed Jul 9 18:52:12 2014 New Revision: 268465 URL: http://svnweb.freebsd.org/changeset/base/268465 Log: * Reduce size of ipfw table entries for cidr/iface: Since old structures had _value as the last field, every table match required 3 cache lines instead of 2. Fix this by - using the fact that supplied masks are suplicated inside radix - using lightweigth sa_in6 structure as key for IPv6 Before (amd64): sizeof(table_entry): 136 sizeof(table_xentry): 160 After (amd64): sizeof(radix_cidr_entry): 120 sizeof(radix_cidr_xentry): 128 sizeof(radix_iface): 128 * Fix memory leak for table entry update * Do some more sanity checks while deleting entry * Do not store masks for host routes Sponsored by: Yandex LLC Modified: projects/ipfw/sys/netpfil/ipfw/ip_fw_table_algo.c Modified: projects/ipfw/sys/netpfil/ipfw/ip_fw_table_algo.c ============================================================================== --- projects/ipfw/sys/netpfil/ipfw/ip_fw_table_algo.c Wed Jul 9 18:32:40 2014 (r268464) +++ projects/ipfw/sys/netpfil/ipfw/ip_fw_table_algo.c Wed Jul 9 18:52:12 2014 (r268465) @@ -78,49 +78,49 @@ static MALLOC_DEFINE(M_IPFW_TBL, "ipfw_t * But for portability, let's avoid assumption and make the code explicit */ #define KEY_LEN(v) *((uint8_t *)&(v)) -#define KEY_OFS (8*offsetof(struct sockaddr_in, sin_addr)) /* * Do not require radix to compare more than actual IPv4/IPv6 address */ #define KEY_LEN_INET (offsetof(struct sockaddr_in, sin_addr) + sizeof(in_addr_t)) -#define KEY_LEN_INET6 (offsetof(struct sockaddr_in6, sin6_addr) + sizeof(struct in6_addr)) +#define KEY_LEN_INET6 (offsetof(struct sa_in6, sin6_addr) + sizeof(struct in6_addr)) #define KEY_LEN_IFACE (offsetof(struct xaddr_iface, ifname)) #define OFF_LEN_INET (8 * offsetof(struct sockaddr_in, sin_addr)) -#define OFF_LEN_INET6 (8 * offsetof(struct sockaddr_in6, sin6_addr)) +#define OFF_LEN_INET6 (8 * offsetof(struct sa_in6, sin6_addr)) #define OFF_LEN_IFACE (8 * offsetof(struct xaddr_iface, ifname)) -struct table_entry { +struct radix_cidr_entry { struct radix_node rn[2]; - struct sockaddr_in addr, mask; - u_int32_t value; + struct sockaddr_in addr; + uint32_t value; + uint8_t masklen; }; -struct xaddr_iface { - uint8_t if_len; /* length of this struct */ - uint8_t pad[7]; /* Align name */ - char ifname[IF_NAMESIZE]; /* Interface name */ +struct sa_in6 { + uint8_t sin6_len; + uint8_t sin6_family; + uint8_t pad[2]; + struct in6_addr sin6_addr; }; -struct table_xentry { +struct radix_cidr_xentry { struct radix_node rn[2]; - union { -#ifdef INET6 - struct sockaddr_in6 addr6; -#endif - struct xaddr_iface iface; - } a; - union { -#ifdef INET6 - struct sockaddr_in6 mask6; -#endif - struct xaddr_iface ifmask; - } m; - u_int32_t value; + struct sa_in6 addr6; + uint32_t value; + uint8_t masklen; }; +struct xaddr_iface { + uint8_t if_len; /* length of this struct */ + uint8_t pad[7]; /* Align name */ + char ifname[IF_NAMESIZE]; /* Interface name */ +}; - +struct radix_iface { + struct radix_node rn[2]; + struct xaddr_iface iface; + uint32_t value; +}; /* * CIDR implementation using radix @@ -134,23 +134,23 @@ ta_lookup_radix(struct table_info *ti, v struct radix_node_head *rnh; if (keylen == sizeof(in_addr_t)) { - struct table_entry *ent; + struct radix_cidr_entry *ent; struct sockaddr_in sa; KEY_LEN(sa) = KEY_LEN_INET; sa.sin_addr.s_addr = *((in_addr_t *)key); rnh = (struct radix_node_head *)ti->state; - ent = (struct table_entry *)(rnh->rnh_matchaddr(&sa, rnh)); + ent = (struct radix_cidr_entry *)(rnh->rnh_matchaddr(&sa, rnh)); if (ent != NULL) { *val = ent->value; return (1); } } else { - struct table_xentry *xent; - struct sockaddr_in6 sa6; + struct radix_cidr_xentry *xent; + struct sa_in6 sa6; KEY_LEN(sa6) = KEY_LEN_INET6; memcpy(&sa6.sin6_addr, key, sizeof(struct in6_addr)); rnh = (struct radix_node_head *)ti->xstate; - xent = (struct table_xentry *)(rnh->rnh_matchaddr(&sa6, rnh)); + xent = (struct radix_cidr_xentry *)(rnh->rnh_matchaddr(&sa6, rnh)); if (xent != NULL) { *val = xent->value; return (1); @@ -184,9 +184,9 @@ static int flush_table_entry(struct radix_node *rn, void *arg) { struct radix_node_head * const rnh = arg; - struct table_entry *ent; + struct radix_cidr_entry *ent; - ent = (struct table_entry *) + ent = (struct radix_cidr_entry *) rnh->rnh_deladdr(rn->rn_key, rn->rn_mask, rnh); if (ent != NULL) free(ent, M_IPFW_TBL); @@ -211,33 +211,22 @@ static int ta_dump_radix_tentry(void *ta_state, struct table_info *ti, void *e, ipfw_obj_tentry *tent) { - struct table_entry *n; - struct table_xentry *xn; -#ifdef INET6 - int i; - uint32_t *v; -#endif + struct radix_cidr_entry *n; + struct radix_cidr_xentry *xn; - n = (struct table_entry *)e; + n = (struct radix_cidr_entry *)e; /* Guess IPv4/IPv6 radix by sockaddr family */ if (n->addr.sin_family == AF_INET) { - if (in_nullhost(n->mask.sin_addr)) - tent->masklen = 0; - else - tent->masklen = 33-ffs(ntohl(n->mask.sin_addr.s_addr)); - /* Save IPv4 address as deprecated IPv6 compatible */ tent->k.addr.s_addr = n->addr.sin_addr.s_addr; + tent->masklen = n->masklen; tent->subtype = AF_INET; tent->value = n->value; #ifdef INET6 } else { - xn = (struct table_xentry *)e; - /* Count IPv6 mask */ - v = (uint32_t *)&xn->m.mask6.sin6_addr; - for (i = 0; i < sizeof(struct in6_addr) / 4; i++, v++) - tent->masklen += bitcount32(*v); - memcpy(&tent->k, &xn->a.addr6.sin6_addr, sizeof(struct in6_addr)); + xn = (struct radix_cidr_xentry *)e; + memcpy(&tent->k, &xn->addr6.sin6_addr, sizeof(struct in6_addr)); + tent->masklen = xn->masklen; tent->subtype = AF_INET6; tent->value = xn->value; #endif @@ -261,7 +250,7 @@ ta_find_radix_tentry(void *ta_state, str rnh = (struct radix_node_head *)ti->state; e = rnh->rnh_matchaddr(&sa, rnh); } else { - struct sockaddr_in6 sa6; + struct sa_in6 sa6; KEY_LEN(sa6) = KEY_LEN_INET6; memcpy(&sa6.sin6_addr, key, sizeof(struct in6_addr)); rnh = (struct radix_node_head *)ti->xstate; @@ -301,8 +290,8 @@ struct ta_buf_cidr struct sockaddr_in ma; } a4; struct { - struct sockaddr_in6 sa; - struct sockaddr_in6 ma; + struct sa_in6 sa; + struct sa_in6 ma; } a6; } addr; }; @@ -324,9 +313,11 @@ static int ta_prepare_add_cidr(struct tentry_info *tei, void *ta_buf) { struct ta_buf_cidr *tb; - struct table_entry *ent; - struct table_xentry *xent; + struct radix_cidr_entry *ent; + struct radix_cidr_xentry *xent; in_addr_t addr; + struct sockaddr_in *mask; + struct sa_in6 *mask6; int mlen; tb = (struct ta_buf_cidr *)ta_buf; @@ -336,24 +327,25 @@ ta_prepare_add_cidr(struct tentry_info * if (tei->subtype == AF_INET) { #ifdef INET - /* IPv4 case */ if (mlen > 32) return (EINVAL); - ent = malloc(sizeof(*ent), M_IPFW_TBL, M_WAITOK | M_ZERO); ent->value = tei->value; + mask = &tb->addr.a4.ma; /* Set 'total' structure length */ KEY_LEN(ent->addr) = KEY_LEN_INET; - KEY_LEN(ent->mask) = KEY_LEN_INET; + KEY_LEN(*mask) = KEY_LEN_INET; ent->addr.sin_family = AF_INET; - ent->mask.sin_addr.s_addr = + mask->sin_addr.s_addr = htonl(mlen ? ~((1 << (32 - mlen)) - 1) : 0); addr = *((in_addr_t *)tei->paddr); - ent->addr.sin_addr.s_addr = addr & ent->mask.sin_addr.s_addr; + ent->addr.sin_addr.s_addr = addr & mask->sin_addr.s_addr; + ent->masklen = mlen; /* Set pointers */ tb->ent_ptr = ent; tb->addr_ptr = (struct sockaddr *)&ent->addr; - tb->mask_ptr = (struct sockaddr *)&ent->mask; + if (mlen != 32) + tb->mask_ptr = (struct sockaddr *)mask; #endif #ifdef INET6 } else if (tei->subtype == AF_INET6) { @@ -362,18 +354,21 @@ ta_prepare_add_cidr(struct tentry_info * return (EINVAL); xent = malloc(sizeof(*xent), M_IPFW_TBL, M_WAITOK | M_ZERO); xent->value = tei->value; + mask6 = &tb->addr.a6.ma; /* Set 'total' structure length */ - KEY_LEN(xent->a.addr6) = KEY_LEN_INET6; - KEY_LEN(xent->m.mask6) = KEY_LEN_INET6; - xent->a.addr6.sin6_family = AF_INET6; - ipv6_writemask(&xent->m.mask6.sin6_addr, mlen); - memcpy(&xent->a.addr6.sin6_addr, tei->paddr, + KEY_LEN(xent->addr6) = KEY_LEN_INET6; + KEY_LEN(*mask6) = KEY_LEN_INET6; + xent->addr6.sin6_family = AF_INET6; + ipv6_writemask(&mask6->sin6_addr, mlen); + memcpy(&xent->addr6.sin6_addr, tei->paddr, sizeof(struct in6_addr)); - APPLY_MASK(&xent->a.addr6.sin6_addr, &xent->m.mask6.sin6_addr); + APPLY_MASK(&xent->addr6.sin6_addr, &mask6->sin6_addr); + xent->masklen = mlen; /* Set pointers */ tb->ent_ptr = xent; - tb->addr_ptr = (struct sockaddr *)&xent->a.addr6; - tb->mask_ptr = (struct sockaddr *)&xent->m.mask6; + tb->addr_ptr = (struct sockaddr *)&xent->addr6; + if (mlen != 128) + tb->mask_ptr = (struct sockaddr *)mask6; #endif } else { /* Unknown CIDR type */ @@ -417,16 +412,18 @@ ta_add_cidr(void *ta_state, struct table if (tei->subtype == AF_INET) { /* IPv4. */ - value = ((struct table_entry *)tb->ent_ptr)->value; - ((struct table_entry *)rn)->value = value; + value = ((struct radix_cidr_entry *)tb->ent_ptr)->value; + ((struct radix_cidr_entry *)rn)->value = value; } else { /* IPv6 */ - value = ((struct table_xentry *)tb->ent_ptr)->value; - ((struct table_xentry *)rn)->value = value; + value = ((struct radix_cidr_xentry *)tb->ent_ptr)->value; + ((struct radix_cidr_xentry *)rn)->value = value; } /* Indicate that update has happened instead of addition */ tei->flags |= TEI_FLAGS_UPDATED; + + return (0); } tb->ent_ptr = NULL; @@ -438,6 +435,8 @@ static int ta_prepare_del_cidr(struct tentry_info *tei, void *ta_buf) { struct ta_buf_cidr *tb; + struct sockaddr_in sa, mask; + struct sa_in6 sa6, mask6; in_addr_t addr; int mlen; @@ -447,8 +446,11 @@ ta_prepare_del_cidr(struct tentry_info * mlen = tei->masklen; if (tei->subtype == AF_INET) { + if (mlen > 32) + return (EINVAL); + memset(&sa, 0, sizeof(struct sockaddr_in)); + memset(&mask, 0, sizeof(struct sockaddr_in)); /* Set 'total' structure length */ - struct sockaddr_in sa, mask; KEY_LEN(sa) = KEY_LEN_INET; KEY_LEN(mask) = KEY_LEN_INET; mask.sin_addr.s_addr = htonl(mlen ? ~((1 << (32 - mlen)) - 1) : 0); @@ -457,15 +459,14 @@ ta_prepare_del_cidr(struct tentry_info * tb->addr.a4.sa = sa; tb->addr.a4.ma = mask; tb->addr_ptr = (struct sockaddr *)&tb->addr.a4.sa; - tb->mask_ptr = (struct sockaddr *)&tb->addr.a4.ma; + if (mlen != 32) + tb->mask_ptr = (struct sockaddr *)&tb->addr.a4.ma; #ifdef INET6 } else if (tei->subtype == AF_INET6) { - /* IPv6 case */ if (mlen > 128) return (EINVAL); - struct sockaddr_in6 sa6, mask6; - memset(&sa6, 0, sizeof(struct sockaddr_in6)); - memset(&mask6, 0, sizeof(struct sockaddr_in6)); + memset(&sa6, 0, sizeof(struct sa_in6)); + memset(&mask6, 0, sizeof(struct sa_in6)); /* Set 'total' structure length */ KEY_LEN(sa6) = KEY_LEN_INET6; KEY_LEN(mask6) = KEY_LEN_INET6; @@ -476,7 +477,8 @@ ta_prepare_del_cidr(struct tentry_info * tb->addr.a6.sa = sa6; tb->addr.a6.ma = mask6; tb->addr_ptr = (struct sockaddr *)&tb->addr.a6.sa; - tb->mask_ptr = (struct sockaddr *)&tb->addr.a6.ma; + if (mlen != 128) + tb->mask_ptr = (struct sockaddr *)&tb->addr.a6.ma; #endif } else return (EINVAL); @@ -547,13 +549,13 @@ ta_lookup_iface(struct table_info *ti, v { struct radix_node_head *rnh; struct xaddr_iface iface; - struct table_xentry *xent; + struct radix_iface *xent; KEY_LEN(iface) = KEY_LEN_IFACE + strlcpy(iface.ifname, (char *)key, IF_NAMESIZE) + 1; rnh = (struct radix_node_head *)ti->xstate; - xent = (struct table_xentry *)(rnh->rnh_matchaddr(&iface, rnh)); + xent = (struct radix_iface *)(rnh->rnh_matchaddr(&iface, rnh)); if (xent != NULL) { *val = xent->value; return (1); @@ -566,12 +568,12 @@ static int flush_iface_entry(struct radix_node *rn, void *arg) { struct radix_node_head * const rnh = arg; - struct table_entry *ent; + struct radix_iface *xent; - ent = (struct table_entry *) + xent = (struct radix_iface *) rnh->rnh_deladdr(rn->rn_key, rn->rn_mask, rnh); - if (ent != NULL) - free(ent, M_IPFW_TBL); + if (xent != NULL) + free(xent, M_IPFW_TBL); return (0); } @@ -611,34 +613,29 @@ static int ta_prepare_add_iface(struct tentry_info *tei, void *ta_buf) { struct ta_buf_iface *tb; - struct table_xentry *xent; + struct radix_iface *xent; int mlen; - char c; + char *ifname; tb = (struct ta_buf_iface *)ta_buf; memset(tb, 0, sizeof(struct ta_buf_cidr)); - mlen = tei->masklen; - /* Check if string is terminated */ - c = ((char *)tei->paddr)[IF_NAMESIZE - 1]; - ((char *)tei->paddr)[IF_NAMESIZE - 1] = '\0'; - mlen = strlen((char *)tei->paddr); - if ((mlen == IF_NAMESIZE - 1) && (c != '\0')) + ifname = (char *)tei->paddr; + if (strnlen(ifname, IF_NAMESIZE) == IF_NAMESIZE) return (EINVAL); /* Include last \0 into comparison */ - mlen++; + mlen = strlen(ifname) + 1; xent = malloc(sizeof(*xent), M_IPFW_TBL, M_WAITOK | M_ZERO); xent->value = tei->value; /* Set 'total' structure length */ - KEY_LEN(xent->a.iface) = KEY_LEN_IFACE + mlen; - KEY_LEN(xent->m.ifmask) = KEY_LEN_IFACE + mlen; - memcpy(xent->a.iface.ifname, tei->paddr, mlen); + KEY_LEN(xent->iface) = KEY_LEN_IFACE + mlen; + memcpy(xent->iface.ifname, tei->paddr, mlen); /* Set pointers */ tb->ent_ptr = xent; - tb->addr_ptr = (struct sockaddr *)&xent->a.iface; + tb->addr_ptr = (struct sockaddr *)&xent->iface; /* Assume direct match */ tb->mask_ptr = NULL; @@ -669,11 +666,13 @@ ta_add_iface(void *ta_state, struct tabl return (EINVAL); } - value = ((struct table_xentry *)tb->ent_ptr)->value; - ((struct table_xentry *)rn)->value = value; + value = ((struct radix_iface *)tb->ent_ptr)->value; + ((struct radix_iface *)rn)->value = value; /* Indicate that update has happened instead of addition */ tei->flags |= TEI_FLAGS_UPDATED; + + return (0); } tb->ent_ptr = NULL; @@ -753,11 +752,11 @@ static int ta_dump_iface_tentry(void *ta_state, struct table_info *ti, void *e, ipfw_obj_tentry *tent) { - struct table_xentry *xn; + struct radix_iface *xn; - xn = (struct table_xentry *)e; + xn = (struct radix_iface *)e; tent->masklen = 8 * IF_NAMESIZE; - memcpy(&tent->k, &xn->a.iface.ifname, IF_NAMESIZE); + memcpy(&tent->k, &xn->iface.ifname, IF_NAMESIZE); tent->value = xn->value; return (0);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201407091852.s69IqC67056764>