Hi,
This series of 29 small diffs slims pf.o by 2640 bytes and pf.c by
113 non-comment lines.
pf_translate(), in particular, is now much shorter and clearer[0].
I've tested it by running on my home router (alix, i386, inet4) for a
week or so without issue, and by profiling it under stress. I saw no
significant performance impact, and it's, possibly, even a little more
efficient. See below for the profile details.
The patch's length is due to its conservatism; the cumulative patch
is much smaller.
At each step, I attempted a correctness argument and some of these follow
the calculational approach of Dijkstra and others. For background, see
e.g. EWD1300 and EWD1123 (for the interested, I've found A.J.M van
Gasteren's 'On the shape of mathematical arguments' a stellar read, and
also recommend EWD1023). I've found it a useful exercise, particularly
as the effort needed to make the argument indicates the effort required
to review the diff, and several times I was led to take smaller bites.
best,
Richard.
PS. If you, reading this, like the patch, or even if you don't,
please drop me a line to let me know. Thanks!
[0] As pf_translate() is called only on state creation it isn't
terribly performance sensitive, either.
Profiling ---------------------------------------------
Setup: netcat a 415MB file through the profiled router.
[A] -- 192.168.3.2 vr1 --> [router] -- vr0 192.168.1.2 --> [B]
(vr0 is part of a bridge.)
pf.conf contains
match on vr1 scrub (random-id, reassemble tcp)
match out on vr0 inet from !vr0:network nat-to $router
pass
before patch: real 1m18.118s
after patch: real 1m17.282s, 1m18.150s, 1m17.716s
before patch:
1.04 4.92 1303400/1303400 pf_test [9]
[13] 5.9 1.04 4.92 1303400 pf_test_state [13]
0.89 2.10 1301332/1301332 pf_tcp_track_full [19]
0.36 0.69 1303400/1304361 pf_find_state [43]
0.29 0.06 1301273/1951905 m_copyback [63]
0.20 0.00 3257446/5872085 pf_addrcpy [74]
0.08 0.08 650646/650647 pf_change_a [93]
0.10 0.06 650646/650647 pf_change_ap [95]
after patch:
(note, profiler spent less time in the idle loop before the test began,
so the time percentages 5.9, 6.9 aren't comparable between these runs.)
0.96 4.88 1301679/1301679 pf_test [7]
[13] 6.9 0.96 4.88 1301679 pf_test_state [13]
0.89 1.99 1301628/1301628 pf_tcp_track_full [19]
0.39 0.59 1301679/1301680 pf_find_state [44]
0.19 0.16 1301580/1301582 pf_change_a [74]
0.28 0.05 1301576/1952366 m_copyback [64]
0.13 0.06 1301580/1301582 pf_change_16 [87]
0.15 0.00 2603358/5857533 pf_addrcpy [75]
Patch --------------------------------------------
To apply against HEAD (pf.c:1.935, pfvar.h:1.419):
First apply the checksum patch I posted to tech@
"Re: [patch] cleaner checksum modification for pf",
Message-Id: <98bb5cd8-7b74-4a79-a333-70749e3bc...@gmail.com>.
(I'm also happy to regenerate what follows directly against HEAD
if necessary.)
then,
# cd /usr/src/sys
# cat - | patch
- add pf_change_p(), "change port", and use instead of pf_change_ap() when
changing port only.
No functional change: all replaced pf_change_ap() leave address unchanged.
Argument:
(note: afto == (pd->af != nk->af))
All replaced pf_change_ap() executed under afto with arg naf := nk->af
=> { pf_change_ap() }
All replaced executions of pf_change_a() executed
with args af := pd->af, naf := nk->af, under afto which implies af != naf
=> { afto and pf_change_a() guarded with 'if (af != naf) return;' }
All replaced executions of pf_change_a() are no-op
=> { pf_change_ap() }
All replaced pf_change_ap() leave address unchanged
[Context=15]
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -139,30 +139,31 @@ union pf_headers {
struct pool pf_src_tree_pl, pf_rule_pl, pf_queue_pl;
struct pool pf_state_pl, pf_state_key_pl, pf_state_item_pl;
struct pool pf_rule_item_pl, pf_sn_item_pl;
void pf_init_threshold(struct pf_threshold *, u_int32_t,
u_int32_t);
void pf_add_threshold(struct pf_threshold *);
int pf_check_threshold(struct pf_threshold *);
void pf_cksum_fixup(u_int16_t *, u_int16_t, u_int16_t,
u_int8_t);
void pf_cksum_fixup_a(u_int16_t *, const struct pf_addr *,
const struct pf_addr *, sa_family_t, u_int8_t);
void pf_change_32(struct pf_pdesc *, u_int32_t *,
u_int32_t);
+void pf_change_p(struct pf_pdesc *, u_int16_t *, u_int16_t);
void pf_change_a(struct pf_pdesc *, struct pf_addr *,
struct pf_addr *, sa_family_t, sa_family_t);
void pf_change_ap(struct pf_pdesc *, struct pf_addr *,
u_int16_t *, struct pf_addr *, u_int16_t,
sa_family_t);
int pf_modulate_sack(struct pf_pdesc *,
struct pf_state_peer *);
void pf_change_a6(struct pf_pdesc *, struct pf_addr *a,
struct pf_addr *an);
int pf_icmp_mapping(struct pf_pdesc *, u_int8_t, int *,
u_int16_t *, u_int16_t *);
void pf_change_icmp(struct pf_pdesc *, struct pf_addr *,
u_int16_t *, struct pf_addr *, struct pf_addr *,
u_int16_t, sa_family_t);
int pf_change_icmp_af(struct mbuf *, int,
@@ -1824,62 +1825,67 @@ pf_change_16_unaligned(struct pf_pdesc *
pf_change_8(pd, fb++, *vb++, hi);
pf_change_8(pd, fb++, *vb++,!hi);
}
/* pre: *f is 16-bit aligned within its packet */
void
pf_change_32(struct pf_pdesc *pd, u_int32_t *f, u_int32_t v)
{
u_int16_t *pc = pd->pcksum;
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;
}
+void
+pf_change_p(struct pf_pdesc *pd, u_int16_t *p, u_int16_t pn)
+{
+ if (p != NULL) {
+ pf_cksum_fixup(pd->pcksum, *p, pn, pd->proto);
+ *p = pn;
+ }
+}
+
/* pre: *a is 16-bit aligned within its packet */
void
pf_change_a(struct pf_pdesc *pd, struct pf_addr *a, struct pf_addr *an,
sa_family_t af, sa_family_t naf)
{
if (af != naf)
return; /* defer to pf_translate_af() and co. */
switch (pd->proto) {
case IPPROTO_TCP: /* FALLTHROUGH */
case IPPROTO_UDP:
case IPPROTO_ICMPV6:
pf_cksum_fixup_a(pd->pcksum, a, an, af, pd->proto);
break;
case IPPROTO_ICMP: /* ICMPv4 has no pseudo-header */
default:
break;
}
PF_ACPY(a, an, naf);
}
void
pf_change_ap(struct pf_pdesc *pd, struct pf_addr *a, u_int16_t *p,
struct pf_addr *an, u_int16_t pn, sa_family_t naf)
{
- if (p != NULL) {
- pf_cksum_fixup(pd->pcksum, *p, pn, pd->proto);
- *p = pn;
- }
-
+ pf_change_p(pd, p, pn);
pf_change_a(pd, a, an, pd->af, naf);
}
void
pf_change_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;
if (hi && ALIGNED_POINTER(f, u_int32_t)) {
pf_change_32(pd, f, v); /* optimise */
return;
}
pf_change_8(pd, fb++, *vb++, hi);
@@ -5071,36 +5077,36 @@ pf_test_state_icmp(struct pf_pdesc *pd,
if (afto) {
if (pf_translate_icmp_af(pd, nk->af,
pd->hdr.icmp))
return (PF_DROP);
m_copyback(pd->m, pd->off,
sizeof(struct icmp6_hdr),
pd->hdr.icmp6, M_NOWAIT);
if (pf_change_icmp_af(pd->m, ipoff2,
pd, &pd2, &nk->addr[sidx],
&nk->addr[didx], pd->af, nk->af))
return (PF_DROP);
if (nk->af == AF_INET)
pd->proto = IPPROTO_ICMP;
else
pd->proto = IPPROTO_ICMPV6;
- pf_change_ap(pd, pd2.src, &th.th_sport,
- &nk->addr[pd2.sidx], nk->port[sidx],
- nk->af);
- pf_change_ap(pd, pd2.dst, &th.th_dport,
- &nk->addr[pd2.didx], nk->port[didx],
- nk->af);
+
+ pf_change_p(pd,
+ &th.th_sport, nk->port[sidx]);
+ pf_change_p(pd,
+ &th.th_dport, nk->port[didx]);
+
m_copyback(pd2.m, pd2.off, 8, &th,
M_NOWAIT);
pd->m->m_pkthdr.ph_rtableid =
nk->rdomain;
pd->destchg = 1;
PF_ACPY(&pd->nsaddr,
&nk->addr[pd2.sidx], nk->af);
PF_ACPY(&pd->ndaddr,
&nk->addr[pd2.didx], nk->af);
pd->naf = nk->af;
return (PF_AFRT);
}
#endif /* INET6 */
if (PF_ANEQ(pd2.src,
&nk->addr[pd2.sidx], pd2.af) ||
@@ -5186,36 +5192,36 @@ pf_test_state_icmp(struct pf_pdesc *pd,
if (afto) {
if (pf_translate_icmp_af(pd, nk->af,
pd->hdr.icmp))
return (PF_DROP);
m_copyback(pd->m, pd->off,
sizeof(struct icmp6_hdr),
pd->hdr.icmp6, M_NOWAIT);
if (pf_change_icmp_af(pd->m, ipoff2,
pd, &pd2, &nk->addr[sidx],
&nk->addr[didx], pd->af, nk->af))
return (PF_DROP);
if (nk->af == AF_INET)
pd->proto = IPPROTO_ICMP;
else
pd->proto = IPPROTO_ICMPV6;
- pf_change_ap(pd, pd2.src, &uh.uh_sport,
- &nk->addr[pd2.sidx], nk->port[sidx],
- nk->af);
- pf_change_ap(pd, pd2.dst, &uh.uh_dport,
- &nk->addr[pd2.didx], nk->port[didx],
- nk->af);
+
+ pf_change_p(pd,
+ &uh.uh_sport, nk->port[sidx]);
+ pf_change_p(pd,
+ &uh.uh_dport, nk->port[didx]);
+
m_copyback(pd2.m, pd2.off, sizeof(uh),
&uh, M_NOWAIT);
pd->m->m_pkthdr.ph_rtableid =
nk->rdomain;
pd->destchg = 1;
PF_ACPY(&pd->nsaddr,
&nk->addr[pd2.sidx], nk->af);
PF_ACPY(&pd->ndaddr,
&nk->addr[pd2.didx], nk->af);
pd->naf = nk->af;
return (PF_AFRT);
}
#endif /* INET6 */
if (PF_ANEQ(pd2.src,
- normalise pf_change_ap() argument with naf := pd->naf
Via: hoist 'if (afto)' block, then substitute nk->af := pd->naf below it.
No functional change:
- intervening code did not reference the four
variables altered by the afto block,
action, pd->naf, pd->nsaddr, pd->ndaddr
[Context=4]
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -4673,31 +4673,29 @@ pf_test_state(struct pf_pdesc *pd, struc
afto = pd->af != nk->af;
sidx = afto ? pd->didx : pd->sidx;
didx = afto ? pd->sidx : pd->didx;
-
+#ifdef INET6
+ if (afto) {
+ PF_ACPY(&pd->nsaddr, &nk->addr[sidx], nk->af);
+ PF_ACPY(&pd->ndaddr, &nk->addr[didx], nk->af);
+ pd->naf = nk->af;
+ action = PF_AFRT;
+ }
+#endif /* INET6 */
if (afto || PF_ANEQ(pd->src, &nk->addr[sidx], pd->af) ||
nk->port[sidx] != pd->osport)
pf_change_ap(pd, pd->src, pd->sport,
- &nk->addr[sidx], nk->port[sidx], nk->af);
+ &nk->addr[sidx], nk->port[sidx], pd->naf);
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)
pf_change_ap(pd, pd->dst, pd->dport,
- &nk->addr[didx], nk->port[didx], nk->af);
-
-#ifdef INET6
- if (afto) {
- PF_ACPY(&pd->nsaddr, &nk->addr[sidx], nk->af);
- PF_ACPY(&pd->ndaddr, &nk->addr[didx], nk->af);
- pd->naf = nk->af;
- action = PF_AFRT;
- }
-#endif /* INET6 */
+ &nk->addr[didx], nk->port[didx], pd->naf);
pd->m->m_pkthdr.ph_rtableid = nk->rdomain;
copyback = 1;
}
- remove new-address-family arg from pf_change_ap(), use pd->naf instead.
No functional change:
- all calls to pf_change_ap() pass arg naf := pd->naf
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -155,8 +155,7 @@ void pf_change_p(struct pf_pdesc *,
u
void pf_change_a(struct pf_pdesc *, struct pf_addr *,
struct pf_addr *, sa_family_t, sa_family_t);
void pf_change_ap(struct pf_pdesc *, struct pf_addr *,
- u_int16_t *, struct pf_addr *, u_int16_t,
- sa_family_t);
+ u_int16_t *, struct pf_addr *, u_int16_t);
int pf_modulate_sack(struct pf_pdesc *,
struct pf_state_peer *);
void pf_change_a6(struct pf_pdesc *, struct pf_addr *a,
@@ -1871,10 +1870,10 @@ pf_change_a(struct pf_pdesc *pd, struct
void
pf_change_ap(struct pf_pdesc *pd, struct pf_addr *a, u_int16_t *p,
- struct pf_addr *an, u_int16_t pn, sa_family_t naf)
+ struct pf_addr *an, u_int16_t pn)
{
pf_change_p(pd, p, pn);
- pf_change_a(pd, a, an, pd->af, naf);
+ pf_change_a(pd, a, an, pd->af, pd->naf);
}
void
@@ -3946,14 +3945,12 @@ pf_translate(struct pf_pdesc *pd, struct
case IPPROTO_TCP:
if (afto || PF_ANEQ(saddr, pd->src, pd->af) ||
*pd->sport != sport) {
- pf_change_ap(pd, pd->src, pd->sport, saddr, sport,
- pd->naf);
+ pf_change_ap(pd, pd->src, pd->sport, saddr, sport);
rewrite = 1;
}
if (afto || PF_ANEQ(daddr, pd->dst, pd->af) ||
*pd->dport != dport) {
- pf_change_ap(pd, pd->dst, pd->dport, daddr, dport,
- pd->naf);
+ pf_change_ap(pd, pd->dst, pd->dport, daddr, dport);
rewrite = 1;
}
break;
@@ -3961,14 +3958,12 @@ pf_translate(struct pf_pdesc *pd, struct
case IPPROTO_UDP:
if (afto || PF_ANEQ(saddr, pd->src, pd->af) ||
*pd->sport != sport) {
- pf_change_ap(pd, pd->src, pd->sport, saddr, sport,
- pd->naf);
+ pf_change_ap(pd, pd->src, pd->sport, saddr, sport);
rewrite = 1;
}
if (afto || PF_ANEQ(daddr, pd->dst, pd->af) ||
*pd->dport != dport) {
- pf_change_ap(pd, pd->dst, pd->dport, daddr, dport,
- pd->naf);
+ pf_change_ap(pd, pd->dst, pd->dport, daddr, dport);
rewrite = 1;
}
break;
@@ -4685,7 +4680,7 @@ pf_test_state(struct pf_pdesc *pd, struc
if (afto || PF_ANEQ(pd->src, &nk->addr[sidx], pd->af) ||
nk->port[sidx] != pd->osport)
pf_change_ap(pd, pd->src, pd->sport,
- &nk->addr[sidx], nk->port[sidx], pd->naf);
+ &nk->addr[sidx], nk->port[sidx]);
if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af) ||
pd->rdomain != nk->rdomain)
@@ -4694,7 +4689,7 @@ pf_test_state(struct pf_pdesc *pd, struc
if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af) ||
nk->port[didx] != pd->odport)
pf_change_ap(pd, pd->dst, pd->dport,
- &nk->addr[didx], nk->port[didx], pd->naf);
+ &nk->addr[didx], nk->port[didx]);
pd->m->m_pkthdr.ph_rtableid = nk->rdomain;
copyback = 1;
- normalize pf_change_icmp() argument with af := pd->af
No functional change:
- All existing calls have arg af == pd->af
af := pd2.af
== { pd2 constant after init to pd->af }
af := pd->af (0)
af := AF_INET
== { guarded by (pd2.af == AF_INET) }
af := pd2.af
== { (0) }
af := pd->af
af := AF_INET6
== { guarded by (pd2.af == AF_INET6) }
af := pd2.af
== { (0) }
af := pd->af
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -5107,7 +5107,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
pf_change_icmp(pd, pd2.src,
&th.th_sport, daddr,
&nk->addr[pd2.sidx],
- nk->port[pd2.sidx], pd2.af);
+ nk->port[pd2.sidx], pd->af);
if (PF_ANEQ(pd2.dst, &nk->addr[pd2.didx],
pd2.af) || pd2.rdomain != nk->rdomain)
@@ -5120,7 +5120,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
pf_change_icmp(pd, pd2.dst,
&th.th_dport, saddr,
&nk->addr[pd2.didx],
- nk->port[pd2.didx], pd2.af);
+ nk->port[pd2.didx], pd->af);
copyback = 1;
}
@@ -5223,7 +5223,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
pf_change_icmp(pd, pd2.src,
&uh.uh_sport, daddr,
&nk->addr[pd2.sidx],
- nk->port[pd2.sidx], pd2.af);
+ nk->port[pd2.sidx], pd->af);
if (PF_ANEQ(pd2.dst, &nk->addr[pd2.didx],
pd2.af) || pd2.rdomain != nk->rdomain)
@@ -5236,7 +5236,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
pf_change_icmp(pd, pd2.dst,
&uh.uh_dport, saddr,
&nk->addr[pd2.didx],
- nk->port[pd2.didx], pd2.af);
+ nk->port[pd2.didx], pd->af);
switch (pd2.af) {
case AF_INET:
@@ -5349,7 +5349,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
&iih.icmp_id : NULL,
daddr, &nk->addr[pd2.sidx],
(virtual_type == htons(ICMP_ECHO)) ?
- nk->port[iidx] : 0, AF_INET);
+ nk->port[iidx] : 0, pd->af);
if (PF_ANEQ(pd2.dst, &nk->addr[pd2.didx],
pd2.af) || pd2.rdomain != nk->rdomain)
@@ -5360,7 +5360,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
&nk->addr[pd2.didx], pd2.af))
pf_change_icmp(pd, pd2.dst, NULL,
saddr, &nk->addr[pd2.didx], 0,
- AF_INET);
+ pd->af);
m_copyback(pd->m, pd->off, ICMP_MINLEN,
pd->hdr.icmp, M_NOWAIT);
@@ -5464,7 +5464,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
daddr, &nk->addr[pd2.sidx],
(virtual_type ==
htons(ICMP6_ECHO_REQUEST))
- ? nk->port[iidx] : 0, AF_INET6);
+ ? nk->port[iidx] : 0, pd->af);
if (PF_ANEQ(pd2.dst, &nk->addr[pd2.didx],
pd2.af) || pd2.rdomain != nk->rdomain)
@@ -5475,7 +5475,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
&nk->addr[pd2.didx], pd2.af))
pf_change_icmp(pd, pd2.dst, NULL,
saddr, &nk->addr[pd2.didx], 0,
- AF_INET6);
+ pd->af);
m_copyback(pd->m, pd->off,
sizeof(struct icmp6_hdr), pd->hdr.icmp6,
@@ -5509,7 +5509,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
&nk->addr[pd2.sidx], pd2.af))
pf_change_icmp(pd, pd2.src, NULL,
daddr, &nk->addr[pd2.sidx], 0,
- pd2.af);
+ pd->af);
if (PF_ANEQ(pd2.dst, &nk->addr[pd2.didx],
pd2.af) || pd2.rdomain != nk->rdomain)
@@ -5520,7 +5520,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
&nk->addr[pd2.didx], pd2.af))
pf_change_icmp(pd, pd2.dst, NULL,
saddr, &nk->addr[pd2.didx], 0,
- pd2.af);
+ pd->af);
switch (pd2.af) {
case AF_INET:
- remove arg af from pf_change_icmp(), use pd->af instead
No functional change:
- all calls to pf_change_icmp() pass arg af := pd->af
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -164,7 +164,7 @@ int pf_icmp_mapping(struct pf_pdesc *
u_int16_t *, u_int16_t *);
void pf_change_icmp(struct pf_pdesc *, struct pf_addr *,
u_int16_t *, struct pf_addr *, struct pf_addr *,
- u_int16_t, sa_family_t);
+ u_int16_t);
int pf_change_icmp_af(struct mbuf *, int,
struct pf_pdesc *, struct pf_pdesc *,
struct pf_addr *, struct pf_addr *, sa_family_t,
@@ -2094,7 +2094,7 @@ pf_icmp_mapping(struct pf_pdesc *pd, u_i
/* pf_change_ap_icmp, allow for af/ ? */
void
pf_change_icmp(struct pf_pdesc *pd, struct pf_addr *ia, u_int16_t *ip,
- struct pf_addr *oa, struct pf_addr *na, u_int16_t np, sa_family_t af)
+ struct pf_addr *oa, struct pf_addr *na, u_int16_t np)
{
/* note: doesn't trouble to fixup quoted checksums, if any */
@@ -2105,12 +2105,12 @@ pf_change_icmp(struct pf_pdesc *pd, stru
}
/* change quoted ip address */
- pf_cksum_fixup_a(pd->pcksum, ia, na, af, pd->proto);
- PF_ACPY(ia, na, af);
+ pf_cksum_fixup_a(pd->pcksum, ia, na, pd->af, pd->proto);
+ PF_ACPY(ia, na, pd->af);
/* change outer ip address */
if (oa) {
- pf_change_a(pd, oa, na, af, af);
+ pf_change_a(pd, oa, na, pd->af, pd->af);
}
}
@@ -5107,7 +5107,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
pf_change_icmp(pd, pd2.src,
&th.th_sport, daddr,
&nk->addr[pd2.sidx],
- nk->port[pd2.sidx], pd->af);
+ nk->port[pd2.sidx]);
if (PF_ANEQ(pd2.dst, &nk->addr[pd2.didx],
pd2.af) || pd2.rdomain != nk->rdomain)
@@ -5120,7 +5120,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
pf_change_icmp(pd, pd2.dst,
&th.th_dport, saddr,
&nk->addr[pd2.didx],
- nk->port[pd2.didx], pd->af);
+ nk->port[pd2.didx]);
copyback = 1;
}
@@ -5223,7 +5223,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
pf_change_icmp(pd, pd2.src,
&uh.uh_sport, daddr,
&nk->addr[pd2.sidx],
- nk->port[pd2.sidx], pd->af);
+ nk->port[pd2.sidx]);
if (PF_ANEQ(pd2.dst, &nk->addr[pd2.didx],
pd2.af) || pd2.rdomain != nk->rdomain)
@@ -5236,7 +5236,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
pf_change_icmp(pd, pd2.dst,
&uh.uh_dport, saddr,
&nk->addr[pd2.didx],
- nk->port[pd2.didx], pd->af);
+ nk->port[pd2.didx]);
switch (pd2.af) {
case AF_INET:
@@ -5349,7 +5349,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
&iih.icmp_id : NULL,
daddr, &nk->addr[pd2.sidx],
(virtual_type == htons(ICMP_ECHO)) ?
- nk->port[iidx] : 0, pd->af);
+ nk->port[iidx] : 0);
if (PF_ANEQ(pd2.dst, &nk->addr[pd2.didx],
pd2.af) || pd2.rdomain != nk->rdomain)
@@ -5359,8 +5359,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
if (PF_ANEQ(pd2.dst,
&nk->addr[pd2.didx], pd2.af))
pf_change_icmp(pd, pd2.dst, NULL,
- saddr, &nk->addr[pd2.didx], 0,
- pd->af);
+ saddr, &nk->addr[pd2.didx], 0);
m_copyback(pd->m, pd->off, ICMP_MINLEN,
pd->hdr.icmp, M_NOWAIT);
@@ -5464,7 +5463,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
daddr, &nk->addr[pd2.sidx],
(virtual_type ==
htons(ICMP6_ECHO_REQUEST))
- ? nk->port[iidx] : 0, pd->af);
+ ? nk->port[iidx] : 0);
if (PF_ANEQ(pd2.dst, &nk->addr[pd2.didx],
pd2.af) || pd2.rdomain != nk->rdomain)
@@ -5474,8 +5473,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
if (PF_ANEQ(pd2.dst,
&nk->addr[pd2.didx], pd2.af))
pf_change_icmp(pd, pd2.dst, NULL,
- saddr, &nk->addr[pd2.didx], 0,
- pd->af);
+ saddr, &nk->addr[pd2.didx], 0);
m_copyback(pd->m, pd->off,
sizeof(struct icmp6_hdr), pd->hdr.icmp6,
@@ -5508,8 +5506,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
if (PF_ANEQ(pd2.src,
&nk->addr[pd2.sidx], pd2.af))
pf_change_icmp(pd, pd2.src, NULL,
- daddr, &nk->addr[pd2.sidx], 0,
- pd->af);
+ daddr, &nk->addr[pd2.sidx], 0);
if (PF_ANEQ(pd2.dst, &nk->addr[pd2.didx],
pd2.af) || pd2.rdomain != nk->rdomain)
@@ -5519,8 +5516,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
if (PF_ANEQ(pd2.dst,
&nk->addr[pd2.didx], pd2.af))
pf_change_icmp(pd, pd2.dst, NULL,
- saddr, &nk->addr[pd2.didx], 0,
- pd->af);
+ saddr, &nk->addr[pd2.didx], 0);
switch (pd2.af) {
case AF_INET:
- remove pf_change_a6(), replace with pf_change_a(..., AF_INET6, AF_INET6)
No functional change:
- replaced with it's current definition
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -158,8 +158,6 @@ void pf_change_ap(struct pf_pdesc *,
u_int16_t *, struct pf_addr *, u_int16_t);
int pf_modulate_sack(struct pf_pdesc *,
struct pf_state_peer *);
-void pf_change_a6(struct pf_pdesc *, struct pf_addr *a,
- struct pf_addr *an);
int pf_icmp_mapping(struct pf_pdesc *, u_int8_t, int *,
u_int16_t *, u_int16_t *);
void pf_change_icmp(struct pf_pdesc *, struct pf_addr *,
@@ -1893,14 +1891,6 @@ pf_change_32_unaligned(struct pf_pdesc *
pf_change_8(pd, fb++, *vb++,!hi);
}
-#ifdef INET6
-void
-pf_change_a6(struct pf_pdesc *pd, struct pf_addr *a, struct pf_addr *an)
-{
- pf_change_a(pd, a, an, AF_INET6, AF_INET6);
-}
-#endif /* INET6 */
-
int
pf_icmp_mapping(struct pf_pdesc *pd, u_int8_t type, int *icmp_dir,
u_int16_t *virtual_id, u_int16_t *virtual_type)
@@ -4016,11 +4006,13 @@ pf_translate(struct pf_pdesc *pd, struct
rewrite = 1;
} else {
if (PF_ANEQ(saddr, pd->src, pd->af)) {
- pf_change_a6(pd, pd->src, saddr);
+ pf_change_a(pd, pd->src, saddr,
+ AF_INET6, AF_INET6);
rewrite = 1;
}
if (PF_ANEQ(daddr, pd->dst, pd->af)) {
- pf_change_a6(pd, pd->dst, daddr);
+ pf_change_a(pd, pd->dst, daddr,
+ AF_INET6, AF_INET6);
rewrite = 1;
}
}
@@ -4053,11 +4045,13 @@ pf_translate(struct pf_pdesc *pd, struct
#ifdef INET6
case AF_INET6:
if (!afto && PF_ANEQ(saddr, pd->src, pd->af)) {
- pf_change_a6(pd, pd->src, saddr);
+ pf_change_a(pd, pd->src, saddr,
+ AF_INET6, AF_INET6);
rewrite = 1;
}
if (!afto && PF_ANEQ(daddr, pd->dst, pd->af)) {
- pf_change_a6(pd, pd->dst, daddr);
+ pf_change_a(pd, pd->dst, daddr,
+ AF_INET6, AF_INET6);
rewrite = 1;
}
break;
@@ -4855,13 +4849,15 @@ pf_test_state_icmp(struct pf_pdesc *pd,
}
if (!afto && PF_ANEQ(pd->src,
&nk->addr[sidx], AF_INET6))
- pf_change_a6(pd, saddr,
- &nk->addr[sidx]);
+ pf_change_a(pd, saddr,
+ &nk->addr[sidx],
+ AF_INET6, AF_INET6);
if (!afto && PF_ANEQ(pd->dst,
&nk->addr[didx], AF_INET6)) {
- pf_change_a6(pd, daddr,
- &nk->addr[didx]);
+ pf_change_a(pd, daddr,
+ &nk->addr[didx],
+ AF_INET6, AF_INET6);
pd->destchg = 1;
}
- Hoist pf_change_a() guard into pf_change_ap() and pf_change_icmp()
No functional change:
- pf_change_a() now has duplicate guards (af == naf)
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -1871,7 +1871,9 @@ pf_change_ap(struct pf_pdesc *pd, struct
struct pf_addr *an, u_int16_t pn)
{
pf_change_p(pd, p, pn);
- pf_change_a(pd, a, an, pd->af, pd->naf);
+
+ if (pd->af == pd->naf)
+ pf_change_a(pd, a, an, pd->af, pd->af);
}
void
@@ -2100,7 +2102,8 @@ pf_change_icmp(struct pf_pdesc *pd, stru
/* change outer ip address */
if (oa) {
- pf_change_a(pd, oa, na, pd->af, pd->af);
+ if (pd->af == pd->naf)
+ pf_change_a(pd, oa, na, pd->af, pd->af);
}
}
- normalise pf_change_a() arguments with af := pd->af, naf := pd->af
No functional change:
- All existing calls have arg af == pd->af, arg naf == pd->af
af := AF_INET6
== { guarded by (pd->af == AF_INET6) }
af := pd->af
[Context=11]
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -4002,28 +4002,28 @@ pf_translate(struct pf_pdesc *pd, struct
if (pd->af != AF_INET6)
return (0);
if (afto) {
if (pf_translate_icmp_af(pd, AF_INET, pd->hdr.icmp6))
return (0);
pd->proto = IPPROTO_ICMP;
rewrite = 1;
} else {
if (PF_ANEQ(saddr, pd->src, pd->af)) {
pf_change_a(pd, pd->src, saddr,
- AF_INET6, AF_INET6);
+ pd->af, pd->af);
rewrite = 1;
}
if (PF_ANEQ(daddr, pd->dst, pd->af)) {
pf_change_a(pd, pd->dst, daddr,
- AF_INET6, AF_INET6);
+ pd->af, pd->af);
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_change_16(pd,
&pd->hdr.icmp6->icmp6_id, icmpid);
rewrite = 1;
}
@@ -4041,28 +4041,28 @@ pf_translate(struct pf_pdesc *pd, struct
}
if (!afto && PF_ANEQ(daddr, pd->dst, pd->af)) {
pf_change_a(pd, pd->dst, daddr,
pd->af, pd->af);
rewrite = 1;
}
break;
#ifdef INET6
case AF_INET6:
if (!afto && PF_ANEQ(saddr, pd->src, pd->af)) {
pf_change_a(pd, pd->src, saddr,
- AF_INET6, AF_INET6);
+ pd->af, pd->af);
rewrite = 1;
}
if (!afto && PF_ANEQ(daddr, pd->dst, pd->af)) {
pf_change_a(pd, pd->dst, daddr,
- AF_INET6, AF_INET6);
+ pd->af, pd->af);
rewrite = 1;
}
break;
#endif /* INET6 */
}
}
return (rewrite);
}
int
pf_tcp_track_full(struct pf_pdesc *pd, struct pf_state_peer *src,
@@ -4846,29 +4846,29 @@ pf_test_state_icmp(struct pf_pdesc *pd,
case AF_INET6:
if (afto) {
if (pf_translate_icmp_af(pd, AF_INET,
pd->hdr.icmp6))
return (PF_DROP);
pd->proto = IPPROTO_ICMP;
}
if (!afto && PF_ANEQ(pd->src,
&nk->addr[sidx], AF_INET6))
pf_change_a(pd, saddr,
&nk->addr[sidx],
- AF_INET6, AF_INET6);
+ pd->af, pd->af);
if (!afto && PF_ANEQ(pd->dst,
&nk->addr[didx], AF_INET6)) {
pf_change_a(pd, daddr,
&nk->addr[didx],
- AF_INET6, AF_INET6);
+ pd->af, pd->af);
pd->destchg = 1;
}
if (nk->port[iidx] != pd->hdr.icmp6->icmp6_id) {
pf_change_16(pd,
&pd->hdr.icmp6->icmp6_id,
nk->port[iidx]);
}
m_copyback(pd->m, pd->off,
sizeof(struct icmp6_hdr), pd->hdr.icmp6,
- 0) remove args af, naf from pf_change_a(), use pd->af, pd->naf instead.
- 1) remove guard (af == naf) from pf_change_a()
No functional change:
0) all calls to pf_change_a() pass args af := pd->af, naf := pd->af
=>
1) pf_change_a() guard (af == naf) always true, can remove
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -153,7 +153,7 @@ void pf_change_32(struct pf_pdesc *,
u_int32_t);
void pf_change_p(struct pf_pdesc *, u_int16_t *, u_int16_t);
void pf_change_a(struct pf_pdesc *, struct pf_addr *,
- struct pf_addr *, sa_family_t, sa_family_t);
+ struct pf_addr *);
void pf_change_ap(struct pf_pdesc *, struct pf_addr *,
u_int16_t *, struct pf_addr *, u_int16_t);
int pf_modulate_sack(struct pf_pdesc *,
@@ -1845,17 +1845,13 @@ pf_change_p(struct pf_pdesc *pd, u_int16
/* pre: *a is 16-bit aligned within its packet */
void
-pf_change_a(struct pf_pdesc *pd, struct pf_addr *a, struct pf_addr *an,
- sa_family_t af, sa_family_t naf)
+pf_change_a(struct pf_pdesc *pd, struct pf_addr *a, struct pf_addr *an)
{
- if (af != naf)
- return; /* defer to pf_translate_af() and co. */
-
switch (pd->proto) {
case IPPROTO_TCP: /* FALLTHROUGH */
case IPPROTO_UDP:
- case IPPROTO_ICMPV6:
- pf_cksum_fixup_a(pd->pcksum, a, an, af, pd->proto);
+ case IPPROTO_ICMPV6:
+ pf_cksum_fixup_a(pd->pcksum, a, an, pd->af, pd->proto);
break;
case IPPROTO_ICMP: /* ICMPv4 has no pseudo-header */
@@ -1863,7 +1859,7 @@ pf_change_a(struct pf_pdesc *pd, struct
break;
}
- PF_ACPY(a, an, naf);
+ PF_ACPY(a, an, pd->af);
}
void
@@ -1873,7 +1869,7 @@ pf_change_ap(struct pf_pdesc *pd, struct
pf_change_p(pd, p, pn);
if (pd->af == pd->naf)
- pf_change_a(pd, a, an, pd->af, pd->af);
+ pf_change_a(pd, a, an);
}
void
@@ -2103,7 +2099,7 @@ pf_change_icmp(struct pf_pdesc *pd, stru
/* change outer ip address */
if (oa) {
if (pd->af == pd->naf)
- pf_change_a(pd, oa, na, pd->af, pd->af);
+ pf_change_a(pd, oa, na);
}
}
@@ -3975,13 +3971,11 @@ pf_translate(struct pf_pdesc *pd, struct
#endif /* INET6 */
} else {
if (PF_ANEQ(saddr, pd->src, pd->af)) {
- pf_change_a(pd, pd->src, saddr,
- pd->af, pd->af);
+ pf_change_a(pd, pd->src, saddr);
rewrite = 1;
}
if (PF_ANEQ(daddr, pd->dst, pd->af)) {
- pf_change_a(pd, pd->dst, daddr,
- pd->af, pd->af);
+ pf_change_a(pd, pd->dst, daddr);
rewrite = 1;
}
}
@@ -4009,13 +4003,11 @@ pf_translate(struct pf_pdesc *pd, struct
rewrite = 1;
} else {
if (PF_ANEQ(saddr, pd->src, pd->af)) {
- pf_change_a(pd, pd->src, saddr,
- pd->af, pd->af);
+ pf_change_a(pd, pd->src, saddr);
rewrite = 1;
}
if (PF_ANEQ(daddr, pd->dst, pd->af)) {
- pf_change_a(pd, pd->dst, daddr,
- pd->af, pd->af);
+ pf_change_a(pd, pd->dst, daddr);
rewrite = 1;
}
}
@@ -4035,26 +4027,22 @@ pf_translate(struct pf_pdesc *pd, struct
switch (pd->af) {
case AF_INET:
if (!afto && PF_ANEQ(saddr, pd->src, pd->af)) {
- pf_change_a(pd, pd->src, saddr,
- pd->af, pd->af);
+ pf_change_a(pd, pd->src, saddr);
rewrite = 1;
}
if (!afto && PF_ANEQ(daddr, pd->dst, pd->af)) {
- pf_change_a(pd, pd->dst, daddr,
- pd->af, pd->af);
+ pf_change_a(pd, pd->dst, daddr);
rewrite = 1;
}
break;
#ifdef INET6
case AF_INET6:
if (!afto && PF_ANEQ(saddr, pd->src, pd->af)) {
- pf_change_a(pd, pd->src, saddr,
- pd->af, pd->af);
+ pf_change_a(pd, pd->src, saddr);
rewrite = 1;
}
if (!afto && PF_ANEQ(daddr, pd->dst, pd->af)) {
- pf_change_a(pd, pd->dst, daddr,
- pd->af, pd->af);
+ pf_change_a(pd, pd->dst, daddr);
rewrite = 1;
}
break;
@@ -4823,13 +4811,11 @@ pf_test_state_icmp(struct pf_pdesc *pd,
#endif /* INET6 */
if (!afto && PF_ANEQ(pd->src,
&nk->addr[sidx], AF_INET))
- pf_change_a(pd, saddr, &nk->addr[sidx],
- pd->af, pd->af);
+ pf_change_a(pd, saddr, &nk->addr[sidx]);
if (!afto && PF_ANEQ(pd->dst,
&nk->addr[didx], AF_INET)) {
- pf_change_a(pd, daddr, &nk->addr[didx],
- pd->af, pd->af);
+ pf_change_a(pd, daddr, &nk->addr[didx]);
pd->destchg = 1;
}
@@ -4853,14 +4839,12 @@ pf_test_state_icmp(struct pf_pdesc *pd,
if (!afto && PF_ANEQ(pd->src,
&nk->addr[sidx], AF_INET6))
pf_change_a(pd, saddr,
- &nk->addr[sidx],
- pd->af, pd->af);
+ &nk->addr[sidx]);
if (!afto && PF_ANEQ(pd->dst,
&nk->addr[didx], AF_INET6)) {
pf_change_a(pd, daddr,
- &nk->addr[didx],
- pd->af, pd->af);
+ &nk->addr[didx]);
pd->destchg = 1;
}
- Remove pf_change_ap(), substitute it's definition.
No functional change.
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -154,8 +154,6 @@ void pf_change_32(struct pf_pdesc *,
void pf_change_p(struct pf_pdesc *, u_int16_t *, u_int16_t);
void pf_change_a(struct pf_pdesc *, struct pf_addr *,
struct pf_addr *);
-void pf_change_ap(struct pf_pdesc *, struct pf_addr *,
- u_int16_t *, struct pf_addr *, u_int16_t);
int pf_modulate_sack(struct pf_pdesc *,
struct pf_state_peer *);
int pf_icmp_mapping(struct pf_pdesc *, u_int8_t, int *,
@@ -1863,16 +1861,6 @@ pf_change_a(struct pf_pdesc *pd, struct
}
void
-pf_change_ap(struct pf_pdesc *pd, struct pf_addr *a, u_int16_t *p,
- struct pf_addr *an, u_int16_t pn)
-{
- pf_change_p(pd, p, pn);
-
- if (pd->af == pd->naf)
- pf_change_a(pd, a, an);
-}
-
-void
pf_change_32_unaligned(struct pf_pdesc *pd, void *f, u_int32_t v, bool hi)
{
u_int8_t *fb = (u_int8_t*)f;
@@ -3934,12 +3922,16 @@ pf_translate(struct pf_pdesc *pd, struct
case IPPROTO_TCP:
if (afto || PF_ANEQ(saddr, pd->src, pd->af) ||
*pd->sport != sport) {
- pf_change_ap(pd, pd->src, pd->sport, saddr, sport);
+ if (pd->af == pd->naf)
+ pf_change_a(pd, pd->src, saddr);
+ pf_change_p(pd, pd->sport, sport);
rewrite = 1;
}
if (afto || PF_ANEQ(daddr, pd->dst, pd->af) ||
*pd->dport != dport) {
- pf_change_ap(pd, pd->dst, pd->dport, daddr, dport);
+ if (pd->af == pd->naf)
+ pf_change_a(pd, pd->dst, daddr);
+ pf_change_p(pd, pd->dport, dport);
rewrite = 1;
}
break;
@@ -3947,12 +3939,16 @@ pf_translate(struct pf_pdesc *pd, struct
case IPPROTO_UDP:
if (afto || PF_ANEQ(saddr, pd->src, pd->af) ||
*pd->sport != sport) {
- pf_change_ap(pd, pd->src, pd->sport, saddr, sport);
+ if (pd->af == pd->naf)
+ pf_change_a(pd, pd->src, saddr);
+ pf_change_p(pd, pd->sport, sport);
rewrite = 1;
}
if (afto || PF_ANEQ(daddr, pd->dst, pd->af) ||
*pd->dport != dport) {
- pf_change_ap(pd, pd->dst, pd->dport, daddr, dport);
+ if(pd->af == pd->naf)
+ pf_change_a(pd, pd->dst, daddr);
+ pf_change_p(pd, pd->dport, dport);
rewrite = 1;
}
break;
@@ -4663,18 +4659,23 @@ pf_test_state(struct pf_pdesc *pd, struc
}
#endif /* INET6 */
if (afto || PF_ANEQ(pd->src, &nk->addr[sidx], pd->af) ||
- nk->port[sidx] != pd->osport)
- pf_change_ap(pd, pd->src, pd->sport,
- &nk->addr[sidx], nk->port[sidx]);
+ nk->port[sidx] != pd->osport) {
+ if (pd->af == pd->naf)
+ pf_change_a(pd, pd->src, &nk->addr[sidx]);
+ pf_change_p(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)
- pf_change_ap(pd, pd->dst, pd->dport,
- &nk->addr[didx], nk->port[didx]);
+ nk->port[didx] != pd->odport) {
+ if (pd->af == pd->naf)
+ pf_change_a(pd, pd->dst, &nk->addr[didx]);
+ pf_change_p(pd, pd->dport, nk->port[didx]);
+ }
pd->m->m_pkthdr.ph_rtableid = nk->rdomain;
copyback = 1;
- normalize equivalent guards on pf_change_a()
No functional change:
within pf_translate(),
!afto
== { pf_translate() }
!(pd->af != pd->naf)
== { logic }
pd->af == pd->naf
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -3922,14 +3922,14 @@ pf_translate(struct pf_pdesc *pd, struct
case IPPROTO_TCP:
if (afto || PF_ANEQ(saddr, pd->src, pd->af) ||
*pd->sport != sport) {
- if (pd->af == pd->naf)
+ if (!afto)
pf_change_a(pd, pd->src, saddr);
pf_change_p(pd, pd->sport, sport);
rewrite = 1;
}
if (afto || PF_ANEQ(daddr, pd->dst, pd->af) ||
*pd->dport != dport) {
- if (pd->af == pd->naf)
+ if (!afto)
pf_change_a(pd, pd->dst, daddr);
pf_change_p(pd, pd->dport, dport);
rewrite = 1;
@@ -3939,14 +3939,14 @@ pf_translate(struct pf_pdesc *pd, struct
case IPPROTO_UDP:
if (afto || PF_ANEQ(saddr, pd->src, pd->af) ||
*pd->sport != sport) {
- if (pd->af == pd->naf)
+ if (!afto)
pf_change_a(pd, pd->src, saddr);
pf_change_p(pd, pd->sport, sport);
rewrite = 1;
}
if (afto || PF_ANEQ(daddr, pd->dst, pd->af) ||
*pd->dport != dport) {
- if(pd->af == pd->naf)
+ if (!afto)
pf_change_a(pd, pd->dst, daddr);
pf_change_p(pd, pd->dport, dport);
rewrite = 1;
- 0) pf_change_p(), pf_change_a() now return rewrite status
Altered function. No impact on any existing code as new, advisory, return value
is ignored by default.
- 1) utilise rewrite status in pf_translate() via 'rewrite += pf_change_*()'
in place of (stronger) 'rewrite = 1'.
No functional change:
- pf_translate() now establishes weaker 'rewrite >= 0':
- 'rewrite' is initialised to 0, and pf_change_*() >= 0
- but this suffices
- all code relies on at most this strength:
- pf_translate() is called only by pf_test_rule(),
via rewrite += pf_translate(...)
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -151,8 +151,8 @@ void pf_cksum_fixup_a(u_int16_t *,
co
const struct pf_addr *, sa_family_t, u_int8_t);
void pf_change_32(struct pf_pdesc *, u_int32_t *,
u_int32_t);
-void pf_change_p(struct pf_pdesc *, u_int16_t *, u_int16_t);
-void pf_change_a(struct pf_pdesc *, struct pf_addr *,
+int pf_change_p(struct pf_pdesc *, u_int16_t *, u_int16_t);
+int pf_change_a(struct pf_pdesc *, struct pf_addr *,
struct pf_addr *);
int pf_modulate_sack(struct pf_pdesc *,
struct pf_state_peer *);
@@ -1832,19 +1832,26 @@ pf_change_32(struct pf_pdesc *pd, u_int3
*f = v;
}
-void
+int
pf_change_p(struct pf_pdesc *pd, u_int16_t *p, u_int16_t pn)
{
+ int rewrite = 0;
+
if (p != NULL) {
pf_cksum_fixup(pd->pcksum, *p, pn, pd->proto);
*p = pn;
+ rewrite = 1;
}
+
+ return (rewrite);
}
/* pre: *a is 16-bit aligned within its packet */
-void
+int
pf_change_a(struct pf_pdesc *pd, struct pf_addr *a, struct pf_addr *an)
{
+ int rewrite = 0;
+
switch (pd->proto) {
case IPPROTO_TCP: /* FALLTHROUGH */
case IPPROTO_UDP:
@@ -1858,6 +1865,9 @@ pf_change_a(struct pf_pdesc *pd, struct
}
PF_ACPY(a, an, pd->af);
+ rewrite = 1;
+
+ return (rewrite);
}
void
@@ -3923,16 +3933,14 @@ pf_translate(struct pf_pdesc *pd, struct
if (afto || PF_ANEQ(saddr, pd->src, pd->af) ||
*pd->sport != sport) {
if (!afto)
- pf_change_a(pd, pd->src, saddr);
- pf_change_p(pd, pd->sport, sport);
- rewrite = 1;
+ rewrite += pf_change_a(pd, pd->src, saddr);
+ rewrite += pf_change_p(pd, pd->sport, sport);
}
if (afto || PF_ANEQ(daddr, pd->dst, pd->af) ||
*pd->dport != dport) {
if (!afto)
- pf_change_a(pd, pd->dst, daddr);
- pf_change_p(pd, pd->dport, dport);
- rewrite = 1;
+ rewrite += pf_change_a(pd, pd->dst, daddr);
+ rewrite += pf_change_p(pd, pd->dport, dport);
}
break;
@@ -3940,16 +3948,14 @@ pf_translate(struct pf_pdesc *pd, struct
if (afto || PF_ANEQ(saddr, pd->src, pd->af) ||
*pd->sport != sport) {
if (!afto)
- pf_change_a(pd, pd->src, saddr);
- pf_change_p(pd, pd->sport, sport);
- rewrite = 1;
+ rewrite += pf_change_a(pd, pd->src, saddr);
+ rewrite += pf_change_p(pd, pd->sport, sport);
}
if (afto || PF_ANEQ(daddr, pd->dst, pd->af) ||
*pd->dport != dport) {
if (!afto)
- pf_change_a(pd, pd->dst, daddr);
- pf_change_p(pd, pd->dport, dport);
- rewrite = 1;
+ rewrite += pf_change_a(pd, pd->dst, daddr);
+ rewrite += pf_change_p(pd, pd->dport, dport);
}
break;
@@ -3967,12 +3973,10 @@ pf_translate(struct pf_pdesc *pd, struct
#endif /* INET6 */
} else {
if (PF_ANEQ(saddr, pd->src, pd->af)) {
- pf_change_a(pd, pd->src, saddr);
- rewrite = 1;
+ rewrite += pf_change_a(pd, pd->src, saddr);
}
if (PF_ANEQ(daddr, pd->dst, pd->af)) {
- pf_change_a(pd, pd->dst, daddr);
- rewrite = 1;
+ rewrite += pf_change_a(pd, pd->dst, daddr);
}
}
if (virtual_type == htons(ICMP_ECHO)) {
@@ -3999,12 +4003,10 @@ pf_translate(struct pf_pdesc *pd, struct
rewrite = 1;
} else {
if (PF_ANEQ(saddr, pd->src, pd->af)) {
- pf_change_a(pd, pd->src, saddr);
- rewrite = 1;
+ rewrite += pf_change_a(pd, pd->src, saddr);
}
if (PF_ANEQ(daddr, pd->dst, pd->af)) {
- pf_change_a(pd, pd->dst, daddr);
- rewrite = 1;
+ rewrite += pf_change_a(pd, pd->dst, daddr);
}
}
if (virtual_type == htons(ICMP6_ECHO_REQUEST)) {
@@ -4023,23 +4025,19 @@ pf_translate(struct pf_pdesc *pd, struct
switch (pd->af) {
case AF_INET:
if (!afto && PF_ANEQ(saddr, pd->src, pd->af)) {
- pf_change_a(pd, pd->src, saddr);
- rewrite = 1;
+ rewrite += pf_change_a(pd, pd->src, saddr);
}
if (!afto && PF_ANEQ(daddr, pd->dst, pd->af)) {
- pf_change_a(pd, pd->dst, daddr);
- rewrite = 1;
+ rewrite += pf_change_a(pd, pd->dst, daddr);
}
break;
#ifdef INET6
case AF_INET6:
if (!afto && PF_ANEQ(saddr, pd->src, pd->af)) {
- pf_change_a(pd, pd->src, saddr);
- rewrite = 1;
+ rewrite += pf_change_a(pd, pd->src, saddr);
}
if (!afto && PF_ANEQ(daddr, pd->dst, pd->af)) {
- pf_change_a(pd, pd->dst, daddr);
- rewrite = 1;
+ rewrite += pf_change_a(pd, pd->dst, daddr);
}
break;
#endif /* INET6 */
@@ -4839,13 +4837,11 @@ pf_test_state_icmp(struct pf_pdesc *pd,
}
if (!afto && PF_ANEQ(pd->src,
&nk->addr[sidx], AF_INET6))
- pf_change_a(pd, saddr,
- &nk->addr[sidx]);
+ pf_change_a(pd, saddr, &nk->addr[sidx]);
if (!afto && PF_ANEQ(pd->dst,
&nk->addr[didx], AF_INET6)) {
- pf_change_a(pd, daddr,
- &nk->addr[didx]);
+ pf_change_a(pd, daddr, &nk->addr[didx]);
pd->destchg = 1;
}
- Normalize pf_test_state_icmp() guards on calls to pf_change_a():
0) ... subtitute pd->af for { AF_INET, AF_INET6 }
No functional change:
in pf_test_state_icmp():
AF_INET
== { guarded by 'switch (pd->af) case AF_INET:' }
pd->af
AF_INET6
== { guarded by 'switch (pd->af) case AF_INET6:' }
pd->af
1) ... substitute pd->src, pd->dst for saddr, daddr arguments,
respectively.
No functional change:
in pf_test_state_icmp():
saddr
== { struct pf_addr *saddr = pd->src, and never changed }
pd->src
daddr
== { struct pf_addr *daddr = pd->dst, and never changed }
pd->dst
[Context=10]
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -4802,26 +4802,26 @@ pf_test_state_icmp(struct pf_pdesc *pd,
case AF_INET:
#ifdef INET6
if (afto) {
if (pf_translate_icmp_af(pd, AF_INET6,
pd->hdr.icmp))
return (PF_DROP);
pd->proto = IPPROTO_ICMPV6;
}
#endif /* INET6 */
if (!afto && PF_ANEQ(pd->src,
- &nk->addr[sidx], AF_INET))
- pf_change_a(pd, saddr, &nk->addr[sidx]);
+ &nk->addr[sidx], pd->af))
+ pf_change_a(pd, pd->src,
&nk->addr[sidx]);
if (!afto && PF_ANEQ(pd->dst,
- &nk->addr[didx], AF_INET)) {
- pf_change_a(pd, daddr, &nk->addr[didx]);
+ &nk->addr[didx], pd->af)) {
+ pf_change_a(pd, pd->dst,
&nk->addr[didx]);
pd->destchg = 1;
}
if (nk->port[iidx] != pd->hdr.icmp->icmp_id) {
pf_change_16(pd, &pd->hdr.icmp->icmp_id,
nk->port[iidx]);
}
m_copyback(pd->m, pd->off, ICMP_MINLEN,
pd->hdr.icmp, M_NOWAIT);
@@ -4829,26 +4829,26 @@ pf_test_state_icmp(struct pf_pdesc *pd,
break;
#ifdef INET6
case AF_INET6:
if (afto) {
if (pf_translate_icmp_af(pd, AF_INET,
pd->hdr.icmp6))
return (PF_DROP);
pd->proto = IPPROTO_ICMP;
}
if (!afto && PF_ANEQ(pd->src,
- &nk->addr[sidx], AF_INET6))
- pf_change_a(pd, saddr, &nk->addr[sidx]);
+ &nk->addr[sidx], pd->af))
+ pf_change_a(pd, pd->src,
&nk->addr[sidx]);
if (!afto && PF_ANEQ(pd->dst,
- &nk->addr[didx], AF_INET6)) {
- pf_change_a(pd, daddr, &nk->addr[didx]);
+ &nk->addr[didx], pd->af)) {
+ pf_change_a(pd, pd->dst,
&nk->addr[didx]);
pd->destchg = 1;
}
if (nk->port[iidx] != pd->hdr.icmp6->icmp6_id) {
pf_change_16(pd,
&pd->hdr.icmp6->icmp6_id,
nk->port[iidx]);
}
m_copyback(pd->m, pd->off,
- push guard PF_ANEQ(a, an, pd->af) into pf_change_a()
No functional change: the guard is an optimisation:
- we show !guard == postcondition.pf_change_a()
Def. PF_ANEQ.(a, an, pd->af) == known.(pd->af) /\ a != an
!guard
== { def }
!PF_ANEQ.(a, an, pd->af)
== { def }
!(known.(pd->af) /\ a != an)
== { de morgan }
!known.(pd->af) \/ a == an
== { known.(pd->af) as
pf_setup_pdesc() otherwise panics }
false \/ a == an
== { logic }
a == an
==
postcondition.pf_change_a()
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -1852,6 +1852,10 @@ pf_change_a(struct pf_pdesc *pd, struct
{
int rewrite = 0;
+ /* warning: !PF_ANEQ != PF_AEQ */
+ if (!PF_ANEQ(a, an, pd->af))
+ return (0);
+
switch (pd->proto) {
case IPPROTO_TCP: /* FALLTHROUGH */
case IPPROTO_UDP:
@@ -3972,12 +3976,8 @@ pf_translate(struct pf_pdesc *pd, struct
rewrite = 1;
#endif /* INET6 */
} else {
- if (PF_ANEQ(saddr, pd->src, pd->af)) {
- rewrite += pf_change_a(pd, pd->src, saddr);
- }
- if (PF_ANEQ(daddr, pd->dst, pd->af)) {
- rewrite += pf_change_a(pd, pd->dst, daddr);
- }
+ rewrite += pf_change_a(pd, pd->src, saddr);
+ rewrite += pf_change_a(pd, pd->dst, daddr);
}
if (virtual_type == htons(ICMP_ECHO)) {
u_int16_t icmpid = (icmp_dir == PF_IN) ? sport : dport;
@@ -4002,12 +4002,8 @@ pf_translate(struct pf_pdesc *pd, struct
pd->proto = IPPROTO_ICMP;
rewrite = 1;
} else {
- if (PF_ANEQ(saddr, pd->src, pd->af)) {
- rewrite += pf_change_a(pd, pd->src, saddr);
- }
- if (PF_ANEQ(daddr, pd->dst, pd->af)) {
- rewrite += pf_change_a(pd, pd->dst, daddr);
- }
+ rewrite += pf_change_a(pd, pd->src, saddr);
+ rewrite += pf_change_a(pd, pd->dst, daddr);
}
if (virtual_type == htons(ICMP6_ECHO_REQUEST)) {
u_int16_t icmpid = (icmp_dir == PF_IN) ? sport : dport;
@@ -4024,19 +4020,15 @@ pf_translate(struct pf_pdesc *pd, struct
default:
switch (pd->af) {
case AF_INET:
- if (!afto && PF_ANEQ(saddr, pd->src, pd->af)) {
+ if (!afto) {
rewrite += pf_change_a(pd, pd->src, saddr);
- }
- if (!afto && PF_ANEQ(daddr, pd->dst, pd->af)) {
rewrite += pf_change_a(pd, pd->dst, daddr);
}
break;
#ifdef INET6
case AF_INET6:
- if (!afto && PF_ANEQ(saddr, pd->src, pd->af)) {
+ if (!afto) {
rewrite += pf_change_a(pd, pd->src, saddr);
- }
- if (!afto && PF_ANEQ(daddr, pd->dst, pd->af)) {
rewrite += pf_change_a(pd, pd->dst, daddr);
}
break;
@@ -4808,13 +4800,13 @@ pf_test_state_icmp(struct pf_pdesc *pd,
pd->proto = IPPROTO_ICMPV6;
}
#endif /* INET6 */
- if (!afto && PF_ANEQ(pd->src,
- &nk->addr[sidx], pd->af))
+ if (!afto) {
pf_change_a(pd, pd->src,
&nk->addr[sidx]);
+ pf_change_a(pd, pd->dst,
&nk->addr[didx]);
+ }
if (!afto && PF_ANEQ(pd->dst,
&nk->addr[didx], pd->af)) {
- pf_change_a(pd, pd->dst,
&nk->addr[didx]);
pd->destchg = 1;
}
@@ -4835,13 +4827,13 @@ pf_test_state_icmp(struct pf_pdesc *pd,
return (PF_DROP);
pd->proto = IPPROTO_ICMP;
}
- if (!afto && PF_ANEQ(pd->src,
- &nk->addr[sidx], pd->af))
+ if (!afto) {
pf_change_a(pd, pd->src,
&nk->addr[sidx]);
+ pf_change_a(pd, pd->dst,
&nk->addr[didx]);
+ }
if (!afto && PF_ANEQ(pd->dst,
&nk->addr[didx], pd->af)) {
- pf_change_a(pd, pd->dst,
&nk->addr[didx]);
pd->destchg = 1;
}
- consolidate redundant pf_change_a() calls by removing guard
No functional change:
- Show guard always true:
guard
== { def }
pd->af == AF_INET \/ pd->af == AF_INET6
== { covers all known af }
known.(pd->af)
== { known.(pd->af) as pf_setup_pdesc() otherwise panics }
true
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -4018,21 +4018,9 @@ pf_translate(struct pf_pdesc *pd, struct
#endif /* INET6 */
default:
- switch (pd->af) {
- case AF_INET:
- if (!afto) {
- rewrite += pf_change_a(pd, pd->src, saddr);
- rewrite += pf_change_a(pd, pd->dst, daddr);
- }
- break;
-#ifdef INET6
- case AF_INET6:
- if (!afto) {
- rewrite += pf_change_a(pd, pd->src, saddr);
- rewrite += pf_change_a(pd, pd->dst, daddr);
- }
- break;
-#endif /* INET6 */
+ if (!afto) {
+ rewrite += pf_change_a(pd, pd->src, saddr);
+ rewrite += pf_change_a(pd, pd->dst, daddr);
}
}
return (rewrite);
- normalize pf_change_p() guard:
replace (nk->port[sidx] != pd->osport)
with (nk->port[sidx] != (pd->sport ? *pd->sport : 0))
No functional change: replaced terms are equivalent
At the pf_test_state() guard, pd->osport is unchanged since init (0)
pd->osport is altered only in pf_test_rule(), which is never
called before pf_test_state().
At the pf_test_state() guard, pd->sport is unchanged since init (1)
All reassignments to pd->sport have not occured at the guard:
a) pf_test_state() has not altered pd->sport.
b) pf_translate(), which alters pd->sport, has not been called.
pf_test_state() is always called before pf_test_rule(),
=> { only pf_test_rule() calls pf_translate() }
pf_test_state() is always called before pf_translate()
=> { pf_test_state() contains the guard }
at the guard, pf_translate() has not been called
Now, after initialization, in pf_setup_pdesc(),
pd->osport
== { initialisation }
pd->sport ? *pd->sport : pd->osport;
== { pd initialised to zero }
pd->sport ? *pd->sport : 0;
And this equivalence holds at the guard, due to (0) and (1) above.
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -4637,7 +4637,7 @@ pf_test_state(struct pf_pdesc *pd, struc
}
#endif /* INET6 */
if (afto || PF_ANEQ(pd->src, &nk->addr[sidx], pd->af) ||
- nk->port[sidx] != pd->osport) {
+ nk->port[sidx] != (pd->sport ? *pd->sport : 0)) {
if (pd->af == pd->naf)
pf_change_a(pd, pd->src, &nk->addr[sidx]);
pf_change_p(pd, pd->sport, nk->port[sidx]);
@@ -4649,7 +4649,7 @@ pf_test_state(struct pf_pdesc *pd, struc
pd->destchg = 1;
if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af) ||
- nk->port[didx] != pd->odport) {
+ nk->port[didx] != (pd->dport ? *pd->dport : 0)) {
if (pd->af == pd->naf)
pf_change_a(pd, pd->dst, &nk->addr[didx]);
pf_change_p(pd, pd->dport, nk->port[didx]);
- normalize guard on pf_change_a(), substitute !afto for (pd->af == pd->naf)
No functional change: the substitute is equivalent.
Note: pf_setup_pdesc() establishes pd->naf == pd->af and this remains
unchanged under !afto, i.e. !afto => pd->naf == pd->af (0)
Th. !afto => (pd->naf == nk->af) (1)
Proof:
!afto
== { (0) }
!afto => pd->naf == pd->af /\ !afto
== { (a => b) /\ a == b /\ a }
pd->naf == pd->af /\ !afto
== { def !afto }
pd->naf == pd->af /\ pd->af == nk->af
=> { transitivity == }
pd->naf == nk->af
Th. pd->naf == nk->af after 'if (afto)' block. (2)
Proof:
{ 'if (afto)' block /\ (1) }
afto => pd->naf == nk->af /\
!afto => pd->naf == nk->af
== { excluded middle }
pd->naf == nk->af
Th. !afto == (pd->naf == pd->af) after the 'if (afto)' block.
Proof:
pd->naf == pd->af
== { (2) }
pd->nk == pd->af
== { def !afto }
!afto
[Context=10]
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -4631,33 +4631,33 @@ pf_test_state(struct pf_pdesc *pd, struc
#ifdef INET6
if (afto) {
PF_ACPY(&pd->nsaddr, &nk->addr[sidx], nk->af);
PF_ACPY(&pd->ndaddr, &nk->addr[didx], nk->af);
pd->naf = nk->af;
action = PF_AFRT;
}
#endif /* INET6 */
if (afto || PF_ANEQ(pd->src, &nk->addr[sidx], pd->af) ||
nk->port[sidx] != (pd->sport ? *pd->sport : 0)) {
- if (pd->af == pd->naf)
+ if (!afto)
pf_change_a(pd, pd->src, &nk->addr[sidx]);
pf_change_p(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->dport ? *pd->dport : 0)) {
- if (pd->af == pd->naf)
+ if (!afto)
pf_change_a(pd, pd->dst, &nk->addr[didx]);
pf_change_p(pd, pd->dport, nk->port[didx]);
}
pd->m->m_pkthdr.ph_rtableid = nk->rdomain;
copyback = 1;
}
if (copyback && pd->hdrlen > 0) {
m_copyback(pd->m, pd->off, pd->hdrlen, pd->hdr.any, M_NOWAIT);
- split guard on pf_change_a(), pf_change_p()
No functional change:
- The existing pf_change_a() guard is invariant under (*pd->sport != sport)
existing pf_change_a() guard
== { three 'if' statements, one inside pf_change_a() }
(afto \/ PF_ANEQ \/ *pd->sport != sport) /\ !afto /\ PF_ANEQ
== { rearrange }
(PF_ANEQ /\ (PF_ANEQ \/ afto \/ pd->sport != sport)) /\ !afto
== { a /\ (a \/ b) == a }
PF_ANEQ /\ !afto
- Stregthening pf_change_p() guard to *pd->sport != sport leaves the
pf_change_p() block's postcondition unchanged: the guard is an optimisation.
Th. new_guard is stronger: new_guard => old_guard
Proof:
new_guard
==
*pd->sport != sport
=> { b => a \/ b }
afto || PF_ANEQ(saddr, pd->src, pd->af) || *pd->sport != sport
==
old_guard
Th. new_guard is an optimisation: !new_guard == postcondition.pf_change_p()
Proof:
!new_guard
== { def }
!(*pd->sport != sport)
== { logic }
*pd->sport == sport
==
postcondition.pf_change_p()
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -3934,33 +3934,33 @@ pf_translate(struct pf_pdesc *pd, struct
switch (pd->proto) {
case IPPROTO_TCP:
- if (afto || PF_ANEQ(saddr, pd->src, pd->af) ||
- *pd->sport != sport) {
- if (!afto)
- rewrite += pf_change_a(pd, pd->src, saddr);
+ if (!afto && PF_ANEQ(saddr, pd->src, pd->af))
+ rewrite += pf_change_a(pd, pd->src, saddr);
+
+ if (*pd->sport != sport)
rewrite += pf_change_p(pd, pd->sport, sport);
- }
- if (afto || PF_ANEQ(daddr, pd->dst, pd->af) ||
- *pd->dport != dport) {
- if (!afto)
- rewrite += pf_change_a(pd, pd->dst, daddr);
+
+ if (!afto && PF_ANEQ(daddr, pd->dst, pd->af))
+ rewrite += pf_change_a(pd, pd->dst, daddr);
+
+ if (*pd->dport != dport)
rewrite += pf_change_p(pd, pd->dport, dport);
- }
+
break;
case IPPROTO_UDP:
- if (afto || PF_ANEQ(saddr, pd->src, pd->af) ||
- *pd->sport != sport) {
- if (!afto)
- rewrite += pf_change_a(pd, pd->src, saddr);
+ if (!afto && PF_ANEQ(saddr, pd->src, pd->af))
+ rewrite += pf_change_a(pd, pd->src, saddr);
+
+ if (*pd->sport != sport)
rewrite += pf_change_p(pd, pd->sport, sport);
- }
- if (afto || PF_ANEQ(daddr, pd->dst, pd->af) ||
- *pd->dport != dport) {
- if (!afto)
- rewrite += pf_change_a(pd, pd->dst, daddr);
+
+ if (!afto && PF_ANEQ(daddr, pd->dst, pd->af))
+ rewrite += pf_change_a(pd, pd->dst, daddr);
+
+ if (*pd->dport != dport)
rewrite += pf_change_p(pd, pd->dport, dport);
- }
+
break;
case IPPROTO_ICMP:
@@ -4636,25 +4636,24 @@ pf_test_state(struct pf_pdesc *pd, struc
action = PF_AFRT;
}
#endif /* INET6 */
- if (afto || PF_ANEQ(pd->src, &nk->addr[sidx], pd->af) ||
- nk->port[sidx] != (pd->sport ? *pd->sport : 0)) {
+ if (afto || PF_ANEQ(pd->src, &nk->addr[sidx], pd->af))
if (!afto)
pf_change_a(pd, pd->src, &nk->addr[sidx]);
- pf_change_p(pd, pd->sport, nk->port[sidx]);
- }
+ if (nk->port[sidx] != (pd->sport ? *pd->sport : 0))
+ pf_change_p(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->dport ? *pd->dport : 0)) {
+ if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af))
if (!afto)
pf_change_a(pd, pd->dst, &nk->addr[didx]);
+
+ if (nk->port[didx] != (pd->dport ? *pd->dport : 0))
pf_change_p(pd, pd->dport, nk->port[didx]);
- }
-
+
pd->m->m_pkthdr.ph_rtableid = nk->rdomain;
copyback = 1;
}
- reduce pf_change_a() guards to '!afto && PF_ANEQ(...)'
No functional change: replaced guards are equivalent
(afto \/ PF_ANEQ(...)) /\ !afto
== { /\ over \/ }
!afto /\ afto \/ !afto /\ PF_ANEQ(...)
== { a \/ false == a }
!afto /\ PF_ANEQ(...)
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -4636,9 +4636,8 @@ pf_test_state(struct pf_pdesc *pd, struc
action = PF_AFRT;
}
#endif /* INET6 */
- if (afto || PF_ANEQ(pd->src, &nk->addr[sidx], pd->af))
- if (!afto)
- pf_change_a(pd, pd->src, &nk->addr[sidx]);
+ if (!afto && PF_ANEQ(pd->src, &nk->addr[sidx], pd->af))
+ pf_change_a(pd, pd->src, &nk->addr[sidx]);
if (nk->port[sidx] != (pd->sport ? *pd->sport : 0))
pf_change_p(pd, pd->sport, nk->port[sidx]);
@@ -4647,9 +4646,8 @@ pf_test_state(struct pf_pdesc *pd, struc
pd->rdomain != nk->rdomain)
pd->destchg = 1;
- if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af))
- if (!afto)
- pf_change_a(pd, pd->dst, &nk->addr[didx]);
+ if (!afto && PF_ANEQ(pd->dst, &nk->addr[didx], pd->af))
+ pf_change_a(pd, pd->dst, &nk->addr[didx]);
if (nk->port[didx] != (pd->dport ? *pd->dport : 0))
pf_change_p(pd, pd->dport, nk->port[didx]);
- Strengthen pf_change_p() guards
nk->port != (port ? *port : 0)
== { C language }
port /\ nk->port != *port \/ !port /\ nk->port != 0
<= { logic }
port /\ nk->port != *port
No functional change:
The deleted guard term (!port /\ nk->port != 0) had no effect as
!port => pf_change_p() is a no-op.
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -4639,7 +4639,7 @@ pf_test_state(struct pf_pdesc *pd, struc
if (!afto && PF_ANEQ(pd->src, &nk->addr[sidx], pd->af))
pf_change_a(pd, pd->src, &nk->addr[sidx]);
- if (nk->port[sidx] != (pd->sport ? *pd->sport : 0))
+ if (pd->sport != NULL && nk->port[sidx] != *pd->sport)
pf_change_p(pd, pd->sport, nk->port[sidx]);
if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af) ||
@@ -4649,7 +4649,7 @@ pf_test_state(struct pf_pdesc *pd, struc
if (!afto && PF_ANEQ(pd->dst, &nk->addr[didx], pd->af))
pf_change_a(pd, pd->dst, &nk->addr[didx]);
- if (nk->port[didx] != (pd->dport ? *pd->dport : 0))
+ if (pd->dport != NULL && nk->port[didx] != *pd->dport)
pf_change_p(pd, pd->dport, nk->port[didx]);
pd->m->m_pkthdr.ph_rtableid = nk->rdomain;
- Remove redundant guard term PF_ANEQ(...)
No functional change: show term is redundant:
existing pf_change_a() guard
== { two 'if' statements, one inside pf_change_a() }
!afto && PF_ANEQ(...) && PF_ANEQ(...)
== { a /\ a == a }
!afto && PF_ANEQ(...)
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -3934,30 +3934,28 @@ pf_translate(struct pf_pdesc *pd, struct
switch (pd->proto) {
case IPPROTO_TCP:
- if (!afto && PF_ANEQ(saddr, pd->src, pd->af))
+ if (!afto) {
rewrite += pf_change_a(pd, pd->src, saddr);
+ rewrite += pf_change_a(pd, pd->dst, daddr);
+ }
if (*pd->sport != sport)
rewrite += pf_change_p(pd, pd->sport, sport);
- if (!afto && PF_ANEQ(daddr, pd->dst, pd->af))
- rewrite += pf_change_a(pd, pd->dst, daddr);
-
if (*pd->dport != dport)
rewrite += pf_change_p(pd, pd->dport, dport);
break;
case IPPROTO_UDP:
- if (!afto && PF_ANEQ(saddr, pd->src, pd->af))
+ if (!afto) {
rewrite += pf_change_a(pd, pd->src, saddr);
+ rewrite += pf_change_a(pd, pd->dst, daddr);
+ }
if (*pd->sport != sport)
rewrite += pf_change_p(pd, pd->sport, sport);
- if (!afto && PF_ANEQ(daddr, pd->dst, pd->af))
- rewrite += pf_change_a(pd, pd->dst, daddr);
-
if (*pd->dport != dport)
rewrite += pf_change_p(pd, pd->dport, dport);
@@ -4636,8 +4634,10 @@ pf_test_state(struct pf_pdesc *pd, struc
action = PF_AFRT;
}
#endif /* INET6 */
- if (!afto && PF_ANEQ(pd->src, &nk->addr[sidx], pd->af))
+ if (!afto) {
pf_change_a(pd, pd->src, &nk->addr[sidx]);
+ pf_change_a(pd, pd->dst, &nk->addr[didx]);
+ }
if (pd->sport != NULL && nk->port[sidx] != *pd->sport)
pf_change_p(pd, pd->sport, nk->port[sidx]);
@@ -4646,9 +4646,6 @@ pf_test_state(struct pf_pdesc *pd, struc
pd->rdomain != nk->rdomain)
pd->destchg = 1;
- if (!afto && PF_ANEQ(pd->dst, &nk->addr[didx], pd->af))
- pf_change_a(pd, pd->dst, &nk->addr[didx]);
-
if (pd->dport != NULL && nk->port[didx] != *pd->dport)
pf_change_p(pd, pd->dport, nk->port[didx]);
- push guard (*p != pn) into pf_change_p()
No functional change: (*p != pn) is an optimisation:
- we show !guard == postcondition.pf_change_p()
!guard
== { def }
!(*p != pn)
== { logic }
*p == pn
== { def }
postcondition.pf_change_p()
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -1837,7 +1837,7 @@ pf_change_p(struct pf_pdesc *pd, u_int16
{
int rewrite = 0;
- if (p != NULL) {
+ if (p != NULL && *p != pn) {
pf_cksum_fixup(pd->pcksum, *p, pn, pd->proto);
*p = pn;
rewrite = 1;
@@ -3939,11 +3939,8 @@ pf_translate(struct pf_pdesc *pd, struct
rewrite += pf_change_a(pd, pd->dst, daddr);
}
- if (*pd->sport != sport)
- rewrite += pf_change_p(pd, pd->sport, sport);
-
- if (*pd->dport != dport)
- rewrite += pf_change_p(pd, pd->dport, dport);
+ rewrite += pf_change_p(pd, pd->sport, sport);
+ rewrite += pf_change_p(pd, pd->dport, dport);
break;
@@ -3953,11 +3950,8 @@ pf_translate(struct pf_pdesc *pd, struct
rewrite += pf_change_a(pd, pd->dst, daddr);
}
- if (*pd->sport != sport)
- rewrite += pf_change_p(pd, pd->sport, sport);
-
- if (*pd->dport != dport)
- rewrite += pf_change_p(pd, pd->dport, dport);
+ rewrite += pf_change_p(pd, pd->sport, sport);
+ rewrite += pf_change_p(pd, pd->dport, dport);
break;
@@ -4639,16 +4633,15 @@ pf_test_state(struct pf_pdesc *pd, struc
pf_change_a(pd, pd->dst, &nk->addr[didx]);
}
- if (pd->sport != NULL && nk->port[sidx] != *pd->sport)
+ if (pd->sport != NULL)
pf_change_p(pd, pd->sport, nk->port[sidx]);
+ if (pd->dport != NULL)
+ pf_change_p(pd, pd->dport, nk->port[didx]);
if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af) ||
pd->rdomain != nk->rdomain)
pd->destchg = 1;
- if (pd->dport != NULL && nk->port[didx] != *pd->dport)
- pf_change_p(pd, pd->dport, nk->port[didx]);
-
pd->m->m_pkthdr.ph_rtableid = nk->rdomain;
copyback = 1;
}
- Factor pf_change_a() calls
No functional change:
- same code appears in every code path.
- pf_change_a() is independent of the code intervening
between the shifted blocks:
- pf_change_p()
- altering pd->hdr.icmp->icmp_id
- altering pd->hdr.icmp6->icmp6_id
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -3934,22 +3934,12 @@ pf_translate(struct pf_pdesc *pd, struct
switch (pd->proto) {
case IPPROTO_TCP:
- if (!afto) {
- rewrite += pf_change_a(pd, pd->src, saddr);
- rewrite += pf_change_a(pd, pd->dst, daddr);
- }
-
rewrite += pf_change_p(pd, pd->sport, sport);
rewrite += pf_change_p(pd, pd->dport, dport);
break;
case IPPROTO_UDP:
- if (!afto) {
- rewrite += pf_change_a(pd, pd->src, saddr);
- rewrite += pf_change_a(pd, pd->dst, daddr);
- }
-
rewrite += pf_change_p(pd, pd->sport, sport);
rewrite += pf_change_p(pd, pd->dport, dport);
@@ -3959,18 +3949,15 @@ pf_translate(struct pf_pdesc *pd, struct
/* pf_translate() is also used when logging invalid packets */
if (pd->af != AF_INET)
return (0);
-
- if (afto) {
#ifdef INET6
+ if (afto) {
if (pf_translate_icmp_af(pd, AF_INET6, pd->hdr.icmp))
return (0);
pd->proto = IPPROTO_ICMPV6;
rewrite = 1;
-#endif /* INET6 */
- } else {
- rewrite += pf_change_a(pd, pd->src, saddr);
- rewrite += pf_change_a(pd, pd->dst, daddr);
}
+#endif /* INET6 */
+
if (virtual_type == htons(ICMP_ECHO)) {
u_int16_t icmpid = (icmp_dir == PF_IN) ? sport : dport;
@@ -3981,7 +3968,6 @@ pf_translate(struct pf_pdesc *pd, struct
}
}
break;
-
#ifdef INET6
case IPPROTO_ICMPV6:
/* pf_translate() is also used when logging invalid packets */
@@ -3993,10 +3979,8 @@ pf_translate(struct pf_pdesc *pd, struct
return (0);
pd->proto = IPPROTO_ICMP;
rewrite = 1;
- } else {
- rewrite += pf_change_a(pd, pd->src, saddr);
- rewrite += pf_change_a(pd, pd->dst, daddr);
}
+
if (virtual_type == htons(ICMP6_ECHO_REQUEST)) {
u_int16_t icmpid = (icmp_dir == PF_IN) ? sport : dport;
@@ -4008,13 +3992,13 @@ pf_translate(struct pf_pdesc *pd, struct
}
break;
#endif /* INET6 */
+ }
- default:
- if (!afto) {
- rewrite += pf_change_a(pd, pd->src, saddr);
- rewrite += pf_change_a(pd, pd->dst, daddr);
- }
+ if (!afto) {
+ rewrite += pf_change_a(pd, pd->src, saddr);
+ rewrite += pf_change_a(pd, pd->dst, daddr);
}
+
return (rewrite);
}
- Factor pf_change_p() calls
No functional change: switch cases are identical.
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -3933,16 +3933,10 @@ pf_translate(struct pf_pdesc *pd, struct
pd->destchg = 1;
switch (pd->proto) {
- case IPPROTO_TCP:
- rewrite += pf_change_p(pd, pd->sport, sport);
- rewrite += pf_change_p(pd, pd->dport, dport);
-
- break;
-
+ case IPPROTO_TCP: /* FALLTHROUGH */
case IPPROTO_UDP:
rewrite += pf_change_p(pd, pd->sport, sport);
rewrite += pf_change_p(pd, pd->dport, dport);
-
break;
case IPPROTO_ICMP:
- pf_change_*() now return 'rewrite'
Altered function. No impact on existing code as new return value is ignored.
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -149,7 +149,7 @@ void pf_cksum_fixup(u_int16_t *,
u_in
u_int8_t);
void pf_cksum_fixup_a(u_int16_t *, const struct pf_addr *,
const struct pf_addr *, sa_family_t, u_int8_t);
-void pf_change_32(struct pf_pdesc *, u_int32_t *,
+int pf_change_32(struct pf_pdesc *, u_int32_t *,
u_int32_t);
int pf_change_p(struct pf_pdesc *, u_int16_t *, u_int16_t);
int pf_change_a(struct pf_pdesc *, struct pf_addr *,
@@ -1788,48 +1788,69 @@ pf_cksum_fixup_a(u_int16_t *cksum, const
*cksum = (u_int16_t)(l);
}
-void
+int
pf_change_8(struct pf_pdesc *pd, u_int8_t *f, u_int8_t v, bool hi)
{
- u_int16_t new = hi ? ( v << 8) : v;
- u_int16_t old = hi ? (*f << 8) : *f;
+ int rewrite = 0;
- pf_cksum_fixup(pd->pcksum, htons(old), htons(new), pd->proto);
- *f = v;
+ if (*f != v) {
+ u_int16_t new = htons(hi ? ( v << 8) : v);
+ u_int16_t old = htons(hi ? (*f << 8) : *f);
+
+ 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_change_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_change_16_unaligned(struct pf_pdesc *pd, void *f, u_int16_t v, bool hi)
{
+ 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_change_16(pd, f, v); /* optimise */
- return;
+ return (pf_change_16(pd, f, v)); /* optimise */
}
- pf_change_8(pd, fb++, *vb++, hi);
- pf_change_8(pd, fb++, *vb++,!hi);
+ rewrite += pf_change_8(pd, fb++, *vb++, hi);
+ rewrite += pf_change_8(pd, fb++, *vb++,!hi);
+
+ return (rewrite);
}
/* pre: *f is 16-bit aligned within its packet */
-void
+int
pf_change_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;
+ /* optimise: skip *f != v guard; true for all use-cases */
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);
}
int
@@ -1874,21 +1895,23 @@ pf_change_a(struct pf_pdesc *pd, struct
return (rewrite);
}
-void
+int
pf_change_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_change_32(pd, f, v); /* optimise */
- return;
+ return (pf_change_32(pd, f, v)); /* optimise */
}
- pf_change_8(pd, fb++, *vb++, hi);
- pf_change_8(pd, fb++, *vb++,!hi);
- pf_change_8(pd, fb++, *vb++, hi);
- pf_change_8(pd, fb++, *vb++,!hi);
+ rewrite += pf_change_8(pd, fb++, *vb++, hi);
+ rewrite += pf_change_8(pd, fb++, *vb++,!hi);
+ rewrite += pf_change_8(pd, fb++, *vb++, hi);
+ rewrite += pf_change_8(pd, fb++, *vb++,!hi);
+
+ return (rewrite);
}
int
Index: net/pfvar.h
===================================================================
--- net.orig/pfvar.h
+++ net/pfvar.h
@@ -1722,10 +1722,10 @@ void *pf_pull_hdr(struct mbuf *, int,
#define PF_HI (true)
#define PF_LO (!PF_HI)
#define PF_ALGNMNT(off) (((off) % 2) == 0 ? PF_HI : PF_LO)
-void pf_change_8(struct pf_pdesc *, u_int8_t *, u_int8_t, bool);
-void pf_change_16(struct pf_pdesc *, u_int16_t *, u_int16_t);
-void pf_change_16_unaligned(struct pf_pdesc *, void *, u_int16_t, bool);
-void pf_change_32_unaligned(struct pf_pdesc *, void *, u_int32_t, bool);
+int pf_change_8(struct pf_pdesc *, u_int8_t *, u_int8_t, bool);
+int pf_change_16(struct pf_pdesc *, u_int16_t *, u_int16_t);
+int pf_change_16_unaligned(struct pf_pdesc *, void *, u_int16_t, bool);
+int pf_change_32_unaligned(struct pf_pdesc *, void *, u_int32_t, bool);
int pf_check_proto_cksum(struct pf_pdesc *, int, int, u_int8_t,
sa_family_t);
int pflog_packet(struct pf_pdesc *, u_int8_t, struct pf_rule *,
- Remove pf_change_p(), use pf_change_16() instead.
No functional change: the functions are identical apart from p != NULL guard,
which already appears where necessary.
- Note: the p != NULL guard is unnecessary when:
p := pd->sport /\ pd->proto in { IPPROTO_UDP, IPPROTO_TCP }
=> { pf_setup_pdesc() }
p != NULL
p := pd->dport /\ pd->proto in { IPPROTO_UDP, IPPROTO_TCP }
=> { pf_setup_pdesc() }
p != NULL
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -151,7 +151,6 @@ void pf_cksum_fixup_a(u_int16_t *,
co
const struct pf_addr *, sa_family_t, u_int8_t);
int pf_change_32(struct pf_pdesc *, u_int32_t *,
u_int32_t);
-int pf_change_p(struct pf_pdesc *, u_int16_t *, u_int16_t);
int pf_change_a(struct pf_pdesc *, struct pf_addr *,
struct pf_addr *);
int pf_modulate_sack(struct pf_pdesc *,
@@ -1853,20 +1852,6 @@ pf_change_32(struct pf_pdesc *pd, u_int3
return (rewrite);
}
-int
-pf_change_p(struct pf_pdesc *pd, u_int16_t *p, u_int16_t pn)
-{
- int rewrite = 0;
-
- if (p != NULL && *p != pn) {
- pf_cksum_fixup(pd->pcksum, *p, pn, pd->proto);
- *p = pn;
- rewrite = 1;
- }
-
- return (rewrite);
-}
-
/* pre: *a is 16-bit aligned within its packet */
int
pf_change_a(struct pf_pdesc *pd, struct pf_addr *a, struct pf_addr *an)
@@ -3958,8 +3943,8 @@ pf_translate(struct pf_pdesc *pd, struct
switch (pd->proto) {
case IPPROTO_TCP: /* FALLTHROUGH */
case IPPROTO_UDP:
- rewrite += pf_change_p(pd, pd->sport, sport);
- rewrite += pf_change_p(pd, pd->dport, dport);
+ rewrite += pf_change_16(pd, pd->sport, sport);
+ rewrite += pf_change_16(pd, pd->dport, dport);
break;
case IPPROTO_ICMP:
@@ -4635,9 +4620,9 @@ pf_test_state(struct pf_pdesc *pd, struc
}
if (pd->sport != NULL)
- pf_change_p(pd, pd->sport, nk->port[sidx]);
+ pf_change_16(pd, pd->sport, nk->port[sidx]);
if (pd->dport != NULL)
- pf_change_p(pd, pd->dport, nk->port[didx]);
+ pf_change_16(pd, pd->dport, nk->port[didx]);
if (afto || PF_ANEQ(pd->dst, &nk->addr[didx], pd->af) ||
pd->rdomain != nk->rdomain)
@@ -5031,9 +5016,9 @@ pf_test_state_icmp(struct pf_pdesc *pd,
else
pd->proto = IPPROTO_ICMPV6;
- pf_change_p(pd,
+ pf_change_16(pd,
&th.th_sport, nk->port[sidx]);
- pf_change_p(pd,
+ pf_change_16(pd,
&th.th_dport, nk->port[didx]);
m_copyback(pd2.m, pd2.off, 8, &th,
@@ -5146,9 +5131,9 @@ pf_test_state_icmp(struct pf_pdesc *pd,
else
pd->proto = IPPROTO_ICMPV6;
- pf_change_p(pd,
+ pf_change_16(pd,
&uh.uh_sport, nk->port[sidx]);
- pf_change_p(pd,
+ pf_change_16(pd,
&uh.uh_dport, nk->port[didx]);
m_copyback(pd2.m, pd2.off, sizeof(uh),
- push 'value-altered' guards into pf_change_16()
No functional change: pf_change_16() contains the same guard.
note: pf_translate() establishes 'rewrite >= 0' and this is
preserved as 'pf_change_16() >= 0'.
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -3962,12 +3962,8 @@ pf_translate(struct pf_pdesc *pd, struct
if (virtual_type == htons(ICMP_ECHO)) {
u_int16_t icmpid = (icmp_dir == PF_IN) ? sport : dport;
-
- if (icmpid != pd->hdr.icmp->icmp_id) {
- pf_change_16(pd,
- &pd->hdr.icmp->icmp_id, icmpid);
- rewrite = 1;
- }
+ rewrite += pf_change_16(pd,
+ &pd->hdr.icmp->icmp_id, icmpid);
}
break;
#ifdef INET6
@@ -3985,12 +3981,8 @@ pf_translate(struct pf_pdesc *pd, struct
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_change_16(pd,
- &pd->hdr.icmp6->icmp6_id, icmpid);
- rewrite = 1;
- }
+ rewrite += pf_change_16(pd,
+ &pd->hdr.icmp6->icmp6_id, icmpid);
}
break;
#endif /* INET6 */
@@ -4771,10 +4763,8 @@ pf_test_state_icmp(struct pf_pdesc *pd,
pd->destchg = 1;
}
- if (nk->port[iidx] != pd->hdr.icmp->icmp_id) {
- pf_change_16(pd, &pd->hdr.icmp->icmp_id,
- nk->port[iidx]);
- }
+ pf_change_16(pd,
+ &pd->hdr.icmp->icmp_id, nk->port[iidx]);
m_copyback(pd->m, pd->off, ICMP_MINLEN,
pd->hdr.icmp, M_NOWAIT);
@@ -4798,11 +4788,8 @@ pf_test_state_icmp(struct pf_pdesc *pd,
pd->destchg = 1;
}
- if (nk->port[iidx] != pd->hdr.icmp6->icmp6_id) {
- pf_change_16(pd,
- &pd->hdr.icmp6->icmp6_id,
- nk->port[iidx]);
- }
+ pf_change_16(pd,
+ &pd->hdr.icmp6->icmp6_id, nk->port[iidx]);
m_copyback(pd->m, pd->off,
sizeof(struct icmp6_hdr), pd->hdr.icmp6,
- factor duplicate code
No functional change:
- no intervening coding between shifted (!afto) blocks
- pd->destchg tested only in pf_test(); it's value has
no effect on intervening calls to pf_translate_icmp_af()
or pf_change_a()
[Context=7]
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -4737,36 +4737,34 @@ pf_test_state_icmp(struct pf_pdesc *pd,
afto = pd->af != nk->af;
sidx = afto ? pd->didx : pd->sidx;
didx = afto ? pd->sidx : pd->didx;
iidx = afto ? !iidx : iidx;
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;
+ if (!afto) {
+ pf_change_a(pd, pd->src, &nk->addr[sidx]);
+ pf_change_a(pd, pd->dst, &nk->addr[didx]);
+ }
+
switch (pd->af) {
case AF_INET:
#ifdef INET6
if (afto) {
if (pf_translate_icmp_af(pd, AF_INET6,
pd->hdr.icmp))
return (PF_DROP);
pd->proto = IPPROTO_ICMPV6;
}
#endif /* INET6 */
- if (!afto) {
- pf_change_a(pd, pd->src,
&nk->addr[sidx]);
- pf_change_a(pd, pd->dst,
&nk->addr[didx]);
- }
-
- if (!afto && PF_ANEQ(pd->dst,
- &nk->addr[didx], pd->af)) {
- pd->destchg = 1;
- }
-
pf_change_16(pd,
&pd->hdr.icmp->icmp_id, nk->port[iidx]);
m_copyback(pd->m, pd->off, ICMP_MINLEN,
pd->hdr.icmp, M_NOWAIT);
copyback = 1;
break;
@@ -4774,23 +4772,14 @@ pf_test_state_icmp(struct pf_pdesc *pd,
case AF_INET6:
if (afto) {
if (pf_translate_icmp_af(pd, AF_INET,
pd->hdr.icmp6))
return (PF_DROP);
pd->proto = IPPROTO_ICMP;
}
- if (!afto) {
- pf_change_a(pd, pd->src,
&nk->addr[sidx]);
- pf_change_a(pd, pd->dst,
&nk->addr[didx]);
- }
-
- if (!afto && PF_ANEQ(pd->dst,
- &nk->addr[didx], pd->af)) {
- pd->destchg = 1;
- }
pf_change_16(pd,
&pd->hdr.icmp6->icmp6_id, nk->port[iidx]);
m_copyback(pd->m, pd->off,
sizeof(struct icmp6_hdr), pd->hdr.icmp6,
M_NOWAIT);
- simplify expression
No functional change: substitute return expression is equivalent.
Th. (x => y /\ y => x) == (x == y) (0)
return expression
==
((match != 0) => ((n != 0) => false /\ (n == 0) => true)) /\
((match == 0) => ((n != 0) => true /\ (n == 0) => false))
== { contrapositive, twice }
((match != 0) => (true => (n == 0) /\ (n == 0) => true)) /\
((match == 0) => (false => (n == 0) /\ (n == 0) => false))
== { (0), twice }
((match != 0) => ((n == 0) == true) /\
((match == 0) => ((n == 0) == false))
== { !n == (n == false), n == (n == true) }
((match != 0) => (n == 0)) /\
((match == 0) => (n != 0)))
== { contrapositive }
((match != 0) => (n == 0)) /\
((match != 0) <= (n == 0))
== { (0) }
(match != 0) == (n == 0)
== { match in { 0, 1 } and C language }
match == (n == 0)
We then avoid an unnecessary comparison by eliminating the
match variable, comparing n as soon as we know the match value.
Index: net/pf.c
===================================================================
--- net.orig/pf.c
+++ net/pf.c
@@ -2733,21 +2733,18 @@ pf_send_icmp(struct mbuf *m, u_int8_t ty
}
/*
- * 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 (match == (n == 0)), where match == (a == b (with mask m)).
+ * Note: n != 0 => returns (!match)
*/
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:
@@ -2759,21 +2756,12 @@ pf_match_addr(u_int8_t n, struct pf_addr
(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);
}
/*
