From 7f3e6f164ebcff9c09678029baee66ef3cf7b718 Mon Sep 17 00:00:00 2001 From: procter Date: Sat, 20 Aug 2016 08:34:30 +0000 Subject: [PATCH] Push 'field changed' guards into 'change field' functions; optimise pf_patch_32(); simplify pf_match_addr() OK mikeb@ --- sys/net/pf.c | 298 ++++++++++++++++++------------------------------ sys/net/pfvar.h | 12 +- 2 files changed, 118 insertions(+), 192 deletions(-) diff --git a/sys/net/pf.c b/sys/net/pf.c index 58934deb82e..b105a420c82 100644 --- a/sys/net/pf.c +++ b/sys/net/pf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf.c,v 1.981 2016/08/20 08:31:36 procter Exp $ */ +/* $OpenBSD: pf.c,v 1.982 2016/08/20 08:34:30 procter Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -164,7 +164,7 @@ int pf_change_icmp_af(struct mbuf *, int, struct pf_pdesc *, struct pf_pdesc *, struct pf_addr *, struct pf_addr *, sa_family_t, sa_family_t); -void pf_translate_a(struct pf_pdesc *, struct pf_addr *, +int pf_translate_a(struct pf_pdesc *, struct pf_addr *, struct pf_addr *); void pf_translate_icmp(struct pf_pdesc *, struct pf_addr *, u_int16_t *, struct pf_addr *, struct pf_addr *, @@ -1863,65 +1863,95 @@ pf_cksum_fixup_a(u_int16_t *cksum, const struct pf_addr *a, *cksum = (u_int16_t)(x); } -void +int pf_patch_8(struct pf_pdesc *pd, u_int8_t *f, u_int8_t v, bool hi) { - u_int16_t new = htons(hi ? ( v << 8) : v); - u_int16_t old = htons(hi ? (*f << 8) : *f); + int rewrite = 0; - pf_cksum_fixup(pd->pcksum, old, new, pd->proto); - *f = v; + if (*f != v) { + u_int16_t old = htons(hi ? (*f << 8) : *f); + u_int16_t new = htons(hi ? ( v << 8) : v); + + pf_cksum_fixup(pd->pcksum, old, new, pd->proto); + *f = v; + rewrite = 1; + } + + return (rewrite); } /* pre: *f is 16-bit aligned within its packet */ -void +int pf_patch_16(struct pf_pdesc *pd, u_int16_t *f, u_int16_t v) { - pf_cksum_fixup(pd->pcksum, *f, v, pd->proto); - *f = v; + int rewrite = 0; + + if (*f != v) { + pf_cksum_fixup(pd->pcksum, *f, v, pd->proto); + *f = v; + rewrite = 1; + } + + return (rewrite); } -void +int pf_patch_16_unaligned(struct pf_pdesc *pd, void *f, u_int16_t v, bool hi) { - u_int8_t *fb = (u_int8_t*)f; - u_int8_t *vb = (u_int8_t*)&v; + int rewrite = 0; + u_int8_t *fb = (u_int8_t*)f; + u_int8_t *vb = (u_int8_t*)&v; if (hi && ALIGNED_POINTER(f, u_int16_t)) { - pf_patch_16(pd, f, v); /* optimise */ - return; + return (pf_patch_16(pd, f, v)); /* optimise */ } - pf_patch_8(pd, fb++, *vb++, hi); - pf_patch_8(pd, fb++, *vb++,!hi); + rewrite += pf_patch_8(pd, fb++, *vb++, hi); + rewrite += pf_patch_8(pd, fb++, *vb++,!hi); + + return (rewrite); } /* pre: *f is 16-bit aligned within its packet */ -void +/* pre: pd->proto != IPPROTO_UDP */ +int pf_patch_32(struct pf_pdesc *pd, u_int32_t *f, u_int32_t v) { - u_int16_t *pc = pd->pcksum; + int rewrite = 0; + u_int16_t *pc = pd->pcksum; + u_int8_t proto = pd->proto; + + /* optimise: inline udp fixup code is unused; let compiler scrub it */ + if (proto == IPPROTO_UDP) + panic("pf_patch_32: udp"); + + /* optimise: skip *f != v guard; true for all use-cases */ + pf_cksum_fixup(pc, *f / (1 << 16), v / (1 << 16), proto); + pf_cksum_fixup(pc, *f % (1 << 16), v % (1 << 16), proto); - pf_cksum_fixup(pc, *f / (1 << 16), v / (1 << 16), pd->proto); - pf_cksum_fixup(pc, *f % (1 << 16), v % (1 << 16), pd->proto); *f = v; + rewrite = 1; + + return (rewrite); } -void +int pf_patch_32_unaligned(struct pf_pdesc *pd, void *f, u_int32_t v, bool hi) { - u_int8_t *fb = (u_int8_t*)f; - u_int8_t *vb = (u_int8_t*)&v; + int rewrite = 0; + u_int8_t *fb = (u_int8_t*)f; + u_int8_t *vb = (u_int8_t*)&v; if (hi && ALIGNED_POINTER(f, u_int32_t)) { - pf_patch_32(pd, f, v); /* optimise */ - return; + return (pf_patch_32(pd, f, v)); /* optimise */ } - pf_patch_8(pd, fb++, *vb++, hi); - pf_patch_8(pd, fb++, *vb++,!hi); - pf_patch_8(pd, fb++, *vb++, hi); - pf_patch_8(pd, fb++, *vb++,!hi); + rewrite += pf_patch_8(pd, fb++, *vb++, hi); + rewrite += pf_patch_8(pd, fb++, *vb++,!hi); + rewrite += pf_patch_8(pd, fb++, *vb++, hi); + rewrite += pf_patch_8(pd, fb++, *vb++,!hi); + + return (rewrite); } int @@ -2135,9 +2165,15 @@ pf_translate_icmp(struct pf_pdesc *pd, struct pf_addr *qa, u_int16_t *qp, /* pre: *a is 16-bit aligned within its packet */ /* *a is a network header src/dst address */ -void +int pf_translate_a(struct pf_pdesc *pd, struct pf_addr *a, struct pf_addr *an) { + int rewrite = 0; + + /* warning: !PF_ANEQ != PF_AEQ */ + if (!PF_ANEQ(a, an, pd->af)) + return (0); + /* fixup transport pseudo-header, if any */ switch (pd->proto) { case IPPROTO_TCP: /* FALLTHROUGH */ @@ -2150,6 +2186,9 @@ pf_translate_a(struct pf_pdesc *pd, struct pf_addr *a, struct pf_addr *an) } PF_ACPY(a, an, pd->af); + rewrite = 1; + + return (rewrite); } #if INET6 @@ -2778,21 +2817,18 @@ pf_send_icmp(struct mbuf *m, u_int8_t type, u_int8_t code, sa_family_t af, } /* - * Return 1 if the addresses a and b match (with mask m), otherwise return 0. - * If n is 0, they match if they are equal. If n is != 0, they match if they - * are different. + * Return ((n = 0) == (a = b [with mask m])) + * Note: n != 0 => returns (a != b [with mask m]) */ int pf_match_addr(u_int8_t n, struct pf_addr *a, struct pf_addr *m, struct pf_addr *b, sa_family_t af) { - int match = 0; - switch (af) { case AF_INET: if ((a->addr32[0] & m->addr32[0]) == (b->addr32[0] & m->addr32[0])) - match++; + return (n == 0); break; #ifdef INET6 case AF_INET6: @@ -2804,21 +2840,12 @@ pf_match_addr(u_int8_t n, struct pf_addr *a, struct pf_addr *m, (b->addr32[2] & m->addr32[2])) && ((a->addr32[3] & m->addr32[3]) == (b->addr32[3] & m->addr32[3]))) - match++; + return (n == 0); break; #endif /* INET6 */ } - if (match) { - if (n) - return (0); - else - return (1); - } else { - if (n) - return (1); - else - return (0); - } + + return (n != 0); } /* @@ -3994,38 +4021,10 @@ pf_translate(struct pf_pdesc *pd, struct pf_addr *saddr, u_int16_t sport, pd->destchg = 1; switch (pd->proto) { - case IPPROTO_TCP: - if (afto || PF_ANEQ(saddr, pd->src, pd->af) || - *pd->sport != sport) { - if (pd->af == pd->naf) - pf_translate_a(pd, pd->src, saddr); - pf_patch_16(pd, pd->sport, sport); - rewrite = 1; - } - if (afto || PF_ANEQ(daddr, pd->dst, pd->af) || - *pd->dport != dport) { - if (pd->af == pd->naf) - pf_translate_a(pd, pd->dst, daddr); - pf_patch_16(pd, pd->dport, dport); - rewrite = 1; - } - break; - + case IPPROTO_TCP: /* FALLTHROUGH */ case IPPROTO_UDP: - if (afto || PF_ANEQ(saddr, pd->src, pd->af) || - *pd->sport != sport) { - if (pd->af == pd->naf) - pf_translate_a(pd, pd->src, saddr); - pf_patch_16(pd, pd->sport, sport); - rewrite = 1; - } - if (afto || PF_ANEQ(daddr, pd->dst, pd->af) || - *pd->dport != dport) { - if (pd->af == pd->naf) - pf_translate_a(pd, pd->dst, daddr); - pf_patch_16(pd, pd->dport, dport); - rewrite = 1; - } + rewrite += pf_patch_16(pd, pd->sport, sport); + rewrite += pf_patch_16(pd, pd->dport, dport); break; case IPPROTO_ICMP: @@ -4040,24 +4039,11 @@ pf_translate(struct pf_pdesc *pd, struct pf_addr *saddr, u_int16_t sport, pd->proto = IPPROTO_ICMPV6; rewrite = 1; #endif /* INET6 */ - } else { - if (PF_ANEQ(saddr, pd->src, pd->af)) { - pf_translate_a(pd, pd->src, saddr); - rewrite = 1; - } - if (PF_ANEQ(daddr, pd->dst, pd->af)) { - pf_translate_a(pd, pd->dst, daddr); - rewrite = 1; - } } if (virtual_type == htons(ICMP_ECHO)) { u_int16_t icmpid = (icmp_dir == PF_IN) ? sport : dport; - - if (icmpid != pd->hdr.icmp->icmp_id) { - pf_patch_16(pd, - &pd->hdr.icmp->icmp_id, icmpid); - rewrite = 1; - } + rewrite += pf_patch_16(pd, + &pd->hdr.icmp->icmp_id, icmpid); } break; @@ -4072,54 +4058,21 @@ pf_translate(struct pf_pdesc *pd, struct pf_addr *saddr, u_int16_t sport, return (0); pd->proto = IPPROTO_ICMP; rewrite = 1; - } else { - if (PF_ANEQ(saddr, pd->src, pd->af)) { - pf_translate_a(pd, pd->src, saddr); - rewrite = 1; - } - if (PF_ANEQ(daddr, pd->dst, pd->af)) { - pf_translate_a(pd, pd->dst, daddr); - rewrite = 1; - } } if (virtual_type == htons(ICMP6_ECHO_REQUEST)) { u_int16_t icmpid = (icmp_dir == PF_IN) ? sport : dport; - - if (icmpid != pd->hdr.icmp6->icmp6_id) { - pf_patch_16(pd, - &pd->hdr.icmp6->icmp6_id, icmpid); - rewrite = 1; - } + rewrite += pf_patch_16(pd, + &pd->hdr.icmp6->icmp6_id, icmpid); } break; #endif /* INET6 */ + } - default: - switch (pd->af) { - case AF_INET: - if (!afto && PF_ANEQ(saddr, pd->src, pd->af)) { - pf_translate_a(pd, pd->src, saddr); - rewrite = 1; - } - if (!afto && PF_ANEQ(daddr, pd->dst, pd->af)) { - pf_translate_a(pd, pd->dst, daddr); - rewrite = 1; - } - break; -#ifdef INET6 - case AF_INET6: - if (!afto && PF_ANEQ(saddr, pd->src, pd->af)) { - pf_translate_a(pd, pd->src, saddr); - rewrite = 1; - } - if (!afto && PF_ANEQ(daddr, pd->dst, pd->af)) { - pf_translate_a(pd, pd->dst, daddr); - rewrite = 1; - } - break; -#endif /* INET6 */ - } + if (!afto) { + rewrite += pf_translate_a(pd, pd->src, saddr); + rewrite += pf_translate_a(pd, pd->dst, daddr); } + return (rewrite); } @@ -4733,25 +4686,21 @@ pf_test_state(struct pf_pdesc *pd, struct pf_state **state, u_short *reason) } #endif /* INET6 */ - if (afto || PF_ANEQ(pd->src, &nk->addr[sidx], pd->af) || - nk->port[sidx] != pd->osport) { - if (pd->af == pd->naf) - pf_translate_a(pd, pd->src, &nk->addr[sidx]); - if (pd->sport != NULL) - pf_patch_16(pd, pd->sport, nk->port[sidx]); - } + if (!afto) + pf_translate_a(pd, pd->src, &nk->addr[sidx]); + + if (pd->sport != NULL) + pf_patch_16(pd, pd->sport, nk->port[sidx]); if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af) || pd->rdomain != nk->rdomain) pd->destchg = 1; - if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af) || - nk->port[didx] != pd->odport) { - if (pd->af == pd->naf) - pf_translate_a(pd, pd->dst, &nk->addr[didx]); - if (pd->dport != NULL) - pf_patch_16(pd, pd->dport, nk->port[didx]); - } + if (!afto) + pf_translate_a(pd, pd->dst, &nk->addr[didx]); + + if (pd->dport != NULL) + pf_patch_16(pd, pd->dport, nk->port[didx]); pd->m->m_pkthdr.ph_rtableid = nk->rdomain; copyback = 1; @@ -4877,8 +4826,16 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, pd->naf = nk->af; } #endif /* INET6 */ + if (!afto) { + pf_translate_a(pd, pd->src, &nk->addr[sidx]); + pf_translate_a(pd, pd->dst, &nk->addr[didx]); + } + if (pd->rdomain != nk->rdomain) pd->destchg = 1; + if (!afto && PF_ANEQ(pd->dst, + &nk->addr[didx], pd->af)) + pd->destchg = 1; pd->m->m_pkthdr.ph_rtableid = nk->rdomain; switch (pd->af) { @@ -4891,22 +4848,8 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, pd->proto = IPPROTO_ICMPV6; } #endif /* INET6 */ - if (!afto && PF_ANEQ(pd->src, - &nk->addr[sidx], AF_INET)) - pf_translate_a(pd, - pd->src, &nk->addr[sidx]); - - if (!afto && PF_ANEQ(pd->dst, - &nk->addr[didx], AF_INET)) { - pf_translate_a(pd, - pd->dst, &nk->addr[didx]); - pd->destchg = 1; - } - - if (nk->port[iidx] != pd->hdr.icmp->icmp_id) { - pf_patch_16(pd, &pd->hdr.icmp->icmp_id, - nk->port[iidx]); - } + pf_patch_16(pd, + &pd->hdr.icmp->icmp_id, nk->port[iidx]); m_copyback(pd->m, pd->off, ICMP_MINLEN, pd->hdr.icmp, M_NOWAIT); @@ -4920,24 +4863,9 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, return (PF_DROP); pd->proto = IPPROTO_ICMP; } - if (!afto && PF_ANEQ(pd->src, - &nk->addr[sidx], AF_INET6)) { - pf_translate_a(pd, - pd->src, &nk->addr[sidx]); - } - if (!afto && PF_ANEQ(pd->dst, - &nk->addr[didx], AF_INET6)) { - pf_translate_a(pd, - pd->dst, &nk->addr[didx]); - pd->destchg = 1; - } - - if (nk->port[iidx] != pd->hdr.icmp6->icmp6_id) { - pf_patch_16(pd, - &pd->hdr.icmp6->icmp6_id, - nk->port[iidx]); - } + pf_patch_16(pd, + &pd->hdr.icmp6->icmp6_id, nk->port[iidx]); m_copyback(pd->m, pd->off, sizeof(struct icmp6_hdr), pd->hdr.icmp6, @@ -5389,8 +5317,7 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, if (pf_translate_icmp_af(pd, nk->af, &iih)) return (PF_DROP); - if (virtual_type == htons(ICMP_ECHO) && - nk->port[iidx] != iih.icmp_id) + if (virtual_type == htons(ICMP_ECHO)) pf_patch_16(pd, &iih.icmp_id, nk->port[iidx]); m_copyback(pd2.m, pd2.off, ICMP_MINLEN, @@ -5502,8 +5429,7 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, nk->af, &iih)) return (PF_DROP); if (virtual_type == - htons(ICMP6_ECHO_REQUEST) && - nk->port[iidx] != iih.icmp6_id) + htons(ICMP6_ECHO_REQUEST)) pf_patch_16(pd, &iih.icmp6_id, nk->port[iidx]); m_copyback(pd2.m, pd2.off, diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index 43c91df13b8..3193e8ab3a8 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -1,4 +1,4 @@ -/* $OpenBSD: pfvar.h,v 1.435 2016/08/17 03:24:12 procter Exp $ */ +/* $OpenBSD: pfvar.h,v 1.436 2016/08/20 08:34:30 procter Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -1714,11 +1714,11 @@ void *pf_pull_hdr(struct mbuf *, int, void *, int, u_short *, u_short *, #define PF_HI (true) #define PF_LO (!PF_HI) #define PF_ALGNMNT(off) (((off) % 2) == 0 ? PF_HI : PF_LO) -void pf_patch_8(struct pf_pdesc *, u_int8_t *, u_int8_t, bool); -void pf_patch_16(struct pf_pdesc *, u_int16_t *, u_int16_t); -void pf_patch_16_unaligned(struct pf_pdesc *, void *, u_int16_t, bool); -void pf_patch_32(struct pf_pdesc *, u_int32_t *, u_int32_t); -void pf_patch_32_unaligned(struct pf_pdesc *, void *, u_int32_t, bool); +int pf_patch_8(struct pf_pdesc *, u_int8_t *, u_int8_t, bool); +int pf_patch_16(struct pf_pdesc *, u_int16_t *, u_int16_t); +int pf_patch_16_unaligned(struct pf_pdesc *, void *, u_int16_t, bool); +int pf_patch_32(struct pf_pdesc *, u_int32_t *, u_int32_t); +int pf_patch_32_unaligned(struct pf_pdesc *, void *, u_int32_t, bool); int pflog_packet(struct pf_pdesc *, u_int8_t, struct pf_rule *, struct pf_rule *, struct pf_ruleset *, struct pf_rule *); void pf_send_deferred_syn(struct pf_state *); -- 2.20.1