From 813196a11a12ba7c2de7dc71cb7b01c8cab7ab87 Mon Sep 17 00:00:00 2001 From: Kristof Provost Date: Tue, 4 Oct 2016 19:35:14 +0000 Subject: [PATCH] pf: remove fastroute tag The tag fastroute came from ipf and was removed in OpenBSD in 2011. The code allows to skip the in pfil hooks and completely removes the out pfil invoke, albeit looking up a route that the IP stack will likely find on its own. The code between IPv4 and IPv6 is also inconsistent and marked as "XXX" for years. Submitted by: Franco Fichtner Differential Revision: https://reviews.freebsd.org/D8058 --- sbin/pfctl/parse.y | 5 ++-- sbin/pfctl/pfctl_parser.c | 8 ++--- share/man/man5/pf.conf.5 | 8 ++--- sys/netpfil/pf/pf.c | 63 +++++++++++---------------------------- sys/netpfil/pf/pf_ioctl.c | 4 +-- 5 files changed, 27 insertions(+), 61 deletions(-) diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y index e5ab534b83e4..09f25821208f 100644 --- a/sbin/pfctl/parse.y +++ b/sbin/pfctl/parse.y @@ -2330,7 +2330,7 @@ pfrule : action dir logquick interface route af proto fromto memcpy(&r.rpool.key, $5.key, sizeof(struct pf_poolhashkey)); } - if (r.rt && r.rt != PF_FASTROUTE) { + if (r.rt) { decide_address_family($5.host, &r.af); remove_invalid_hosts(&$5.host, &r.af); if ($5.host == NULL) { @@ -4416,8 +4416,9 @@ route : /* empty */ { $$.pool_opts = 0; } | FASTROUTE { + /* backwards-compat */ $$.host = NULL; - $$.rt = PF_FASTROUTE; + $$.rt = 0; $$.pool_opts = 0; } | ROUTETO routespec pool_opts { diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c index f7a7ad96e703..a47dfd04103c 100644 --- a/sbin/pfctl/pfctl_parser.c +++ b/sbin/pfctl/pfctl_parser.c @@ -778,12 +778,8 @@ print_rule(struct pf_rule *r, const char *anchor_call, int verbose, int numeric) printf(" reply-to"); else if (r->rt == PF_DUPTO) printf(" dup-to"); - else if (r->rt == PF_FASTROUTE) - printf(" fastroute"); - if (r->rt != PF_FASTROUTE) { - printf(" "); - print_pool(&r->rpool, 0, 0, r->af, PF_PASS); - } + printf(" "); + print_pool(&r->rpool, 0, 0, r->af, PF_PASS); } if (r->af) { if (r->af == AF_INET) diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5 index 3ff63bb14c54..96de0ad2abb9 100644 --- a/share/man/man5/pf.conf.5 +++ b/share/man/man5/pf.conf.5 @@ -28,7 +28,7 @@ .\" ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE .\" POSSIBILITY OF SUCH DAMAGE. .\" -.Dd June 9, 2016 +.Dd September 28, 2016 .Dt PF.CONF 5 .Os .Sh NAME @@ -1874,10 +1874,6 @@ route the packet according to the type of route option. When such a rule creates state, the route option is also applied to all packets matching the same connection. .Bl -tag -width xxxx -.It Ar fastroute -The -.Ar fastroute -option does a normal route lookup to find the next hop for the packet. .It Ar route-to The .Ar route-to @@ -2839,7 +2835,7 @@ option = "set" ( [ "timeout" ( timeout | "{" timeout-list "}" ) ] | pf-rule = action [ ( "in" | "out" ) ] [ "log" [ "(" logopts ")"] ] [ "quick" ] - [ "on" ifspec ] [ "fastroute" | route ] [ af ] [ protospec ] + [ "on" ifspec ] [ route ] [ af ] [ protospec ] hosts [ filteropt-list ] logopts = logopt [ "," logopts ] diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 2d32b7a288cc..d760eecd930e 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -3632,7 +3632,7 @@ pf_create_state(struct pf_rule *r, struct pf_rule *nr, struct pf_rule *a, s->timeout = PFTM_OTHER_FIRST_PACKET; } - if (r->rt && r->rt != PF_FASTROUTE) { + if (r->rt) { if (pf_map_addr(pd->af, r, pd->src, &s->rt_addr, NULL, &sn)) { REASON_SET(&reason, PFRES_MAPFAILED); pf_src_tree_remove_state(s); @@ -5439,41 +5439,24 @@ pf_route(struct mbuf **m, struct pf_rule *r, int dir, struct ifnet *oifp, dst.sin_len = sizeof(dst); dst.sin_addr = ip->ip_dst; - if (r->rt == PF_FASTROUTE) { - struct nhop4_basic nh4; - - if (s) - PF_STATE_UNLOCK(s); - - if (fib4_lookup_nh_basic(M_GETFIB(m0), ip->ip_dst, 0, - m0->m_pkthdr.flowid, &nh4) != 0) { - KMOD_IPSTAT_INC(ips_noroute); - error = EHOSTUNREACH; - goto bad; - } - - ifp = nh4.nh_ifp; - dst.sin_addr = nh4.nh_addr; + if (TAILQ_EMPTY(&r->rpool.list)) { + DPFPRINTF(PF_DEBUG_URGENT, + ("%s: TAILQ_EMPTY(&r->rpool.list)\n", __func__)); + goto bad_locked; + } + if (s == NULL) { + pf_map_addr(AF_INET, r, (struct pf_addr *)&ip->ip_src, + &naddr, NULL, &sn); + if (!PF_AZERO(&naddr, AF_INET)) + dst.sin_addr.s_addr = naddr.v4.s_addr; + ifp = r->rpool.cur->kif ? + r->rpool.cur->kif->pfik_ifp : NULL; } else { - if (TAILQ_EMPTY(&r->rpool.list)) { - DPFPRINTF(PF_DEBUG_URGENT, - ("%s: TAILQ_EMPTY(&r->rpool.list)\n", __func__)); - goto bad_locked; - } - if (s == NULL) { - pf_map_addr(AF_INET, r, (struct pf_addr *)&ip->ip_src, - &naddr, NULL, &sn); - if (!PF_AZERO(&naddr, AF_INET)) - dst.sin_addr.s_addr = naddr.v4.s_addr; - ifp = r->rpool.cur->kif ? - r->rpool.cur->kif->pfik_ifp : NULL; - } else { - if (!PF_AZERO(&s->rt_addr, AF_INET)) - dst.sin_addr.s_addr = - s->rt_addr.v4.s_addr; - ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL; - PF_STATE_UNLOCK(s); - } + if (!PF_AZERO(&s->rt_addr, AF_INET)) + dst.sin_addr.s_addr = + s->rt_addr.v4.s_addr; + ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL; + PF_STATE_UNLOCK(s); } if (ifp == NULL) goto bad; @@ -5615,16 +5598,6 @@ pf_route6(struct mbuf **m, struct pf_rule *r, int dir, struct ifnet *oifp, dst.sin6_len = sizeof(dst); dst.sin6_addr = ip6->ip6_dst; - /* Cheat. XXX why only in the v6 case??? */ - if (r->rt == PF_FASTROUTE) { - if (s) - PF_STATE_UNLOCK(s); - m0->m_flags |= M_SKIP_FIREWALL; - ip6_output(m0, NULL, NULL, 0, NULL, NULL, NULL); - *m = NULL; - return; - } - if (TAILQ_EMPTY(&r->rpool.list)) { DPFPRINTF(PF_DEBUG_URGENT, ("%s: TAILQ_EMPTY(&r->rpool.list)\n", __func__)); diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c index cf7f6f26a5b6..73fcb4523b00 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -1267,7 +1267,7 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td pf_mv_pool(&V_pf_pabuf, &rule->rpool.list); if (((((rule->action == PF_NAT) || (rule->action == PF_RDR) || (rule->action == PF_BINAT)) && rule->anchor == NULL) || - (rule->rt > PF_FASTROUTE)) && + (rule->rt > PF_NOPFROUTE)) && (TAILQ_FIRST(&rule->rpool.list) == NULL)) error = EINVAL; @@ -1527,7 +1527,7 @@ DIOCADDRULE_error: if (((((newrule->action == PF_NAT) || (newrule->action == PF_RDR) || (newrule->action == PF_BINAT) || - (newrule->rt > PF_FASTROUTE)) && + (newrule->rt > PF_NOPFROUTE)) && !newrule->anchor)) && (TAILQ_FIRST(&newrule->rpool.list) == NULL)) error = EINVAL;