On Fri, Oct 21, 2016 at 08:07:21PM +1300, Richard Procter wrote:
> On 21/10/2016, at 2:03 AM, Alexander Bluhm wrote:
> > On Thu, Oct 20, 2016 at 10:53:17AM +0200, Claudio Jeker wrote:
> >>> Unfortunately pf_route() is called from pfsync which has no idea
> >>> of packet descriptors. As I do not want to rewrite pfsync, I create
> >>> a temporary pf_pdesc on the stack.
> >>
> >> I'm OK with the direction but am wondering if setting only that little
> >> information in the pf_pdesc is save in the future when that pd is passed
> >> further down.
> >
> > Henning hat the same concern. Now I fill the pf_pdesc completely
> > with pf_setup_pdesc().
>
> I'm sympathetic to the direction. However the patch raises to three the
> number of definitions of union pf_headers (the others in pf.c and
> if_pflog.c). At minimum the comment on pf_headers in pf.c:130 should be
> updated to reflect this.
We could put the union pf_headers into a separate header file.
Henning, I hope you don't mind that I used a current license.template
with your old pf_headers code.
Do we want to go this way?
bluhm
Index: net/if_pflog.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_pflog.c,v
retrieving revision 1.74
diff -u -p -r1.74 if_pflog.c
--- net/if_pflog.c 29 Apr 2016 08:55:03 -0000 1.74
+++ net/if_pflog.c 21 Oct 2016 17:51:41 -0000
@@ -62,6 +62,7 @@
#endif /* INET6 */
#include <net/pfvar.h>
+#include <net/pfhdr.h>
#include <net/if_pflog.h>
#define PFLOGMTU (32768 + MHLEN + MLEN)
@@ -297,16 +298,7 @@ pflog_bpfcopy(const void *src_arg, void
u_int count;
u_char *dst, *mdst;
int afto, hlen, mlen, off;
- union pf_headers {
- struct tcphdr tcp;
- struct udphdr udp;
- struct icmp icmp;
-#ifdef INET6
- struct icmp6_hdr icmp6;
- struct mld_hdr mld;
- struct nd_neighbor_solicit nd_ns;
-#endif /* INET6 */
- } pdhdrs;
+ union pf_headers pdhdrs;
struct pf_pdesc pd;
struct pf_addr osaddr, odaddr;
Index: net/if_pfsync.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_pfsync.c,v
retrieving revision 1.235
diff -u -p -r1.235 if_pfsync.c
--- net/if_pfsync.c 4 Oct 2016 13:54:32 -0000 1.235
+++ net/if_pfsync.c 21 Oct 2016 17:52:04 -0000
@@ -61,9 +61,13 @@
#include <net/netisr.h>
#include <netinet/in.h>
#include <netinet/if_ether.h>
+#include <netinet/ip.h>
+#include <netinet/ip_icmp.h>
+#include <netinet/icmp6.h>
#include <netinet/tcp.h>
#include <netinet/tcp_seq.h>
#include <netinet/tcp_fsm.h>
+#include <netinet/udp.h>
#include <netinet/in_var.h>
#include <netinet/ip.h>
@@ -87,6 +91,7 @@
#define PF_DEBUGNAME "pfsync: "
#include <net/pfvar.h>
+#include <net/pfhdr.h>
#include <netinet/ip_ipsp.h>
#include <net/if_pfsync.h>
@@ -1732,6 +1737,8 @@ void
pfsync_undefer(struct pfsync_deferral *pd, int drop)
{
struct pfsync_softc *sc = pfsyncif;
+ struct pf_pdesc pdesc;
+ union pf_headers pdhdrs;
splsoftassert(IPL_SOFTNET);
@@ -1743,17 +1750,22 @@ pfsync_undefer(struct pfsync_deferral *p
m_freem(pd->pd_m);
else {
if (pd->pd_st->rule.ptr->rt == PF_ROUTETO) {
+ if (pf_setup_pdesc(&pdesc, &pdhdrs,
+ pd->pd_st->key[PF_SK_WIRE]->af,
+ pd->pd_st->direction, pd->pd_st->rt_kif,
+ pd->pd_m, NULL) != PF_PASS) {
+ m_freem(pd->pd_m);
+ goto out;
+ }
switch (pd->pd_st->key[PF_SK_WIRE]->af) {
case AF_INET:
- pf_route(&pd->pd_m, pd->pd_st->rule.ptr,
- pd->pd_st->direction,
- pd->pd_st->rt_kif->pfik_ifp, pd->pd_st);
+ pf_route(&pd->pd_m, &pdesc,
+ pd->pd_st->rule.ptr, pd->pd_st);
break;
#ifdef INET6
case AF_INET6:
- pf_route6(&pd->pd_m, pd->pd_st->rule.ptr,
- pd->pd_st->direction,
- pd->pd_st->rt_kif->pfik_ifp, pd->pd_st);
+ pf_route6(&pd->pd_m, &pdesc,
+ pd->pd_st->rule.ptr, pd->pd_st);
break;
#endif /* INET6 */
}
@@ -1772,7 +1784,7 @@ pfsync_undefer(struct pfsync_deferral *p
}
}
}
-
+ out:
pool_put(&sc->sc_pool, pd);
}
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.993
diff -u -p -r1.993 pf.c
--- net/pf.c 20 Oct 2016 23:18:43 -0000 1.993
+++ net/pf.c 21 Oct 2016 17:52:26 -0000
@@ -77,6 +77,7 @@
#include <netinet/ip_divert.h>
#include <net/pfvar.h>
+#include <net/pfhdr.h>
#if NPFLOG > 0
#include <net/if_pflog.h>
@@ -126,22 +127,6 @@ struct pf_anchor_stackframe {
struct pf_anchor *child;
} pf_anchor_stack[64];
-/*
- * Cannot fold into pf_pdesc directly, unknown storage size outside pf.c.
- * Keep in sync with union pf_headers in pflog_bpfcopy() in if_pflog.c.
- */
-union pf_headers {
- struct tcphdr tcp;
- struct udphdr udp;
- struct icmp icmp;
-#ifdef INET6
- struct icmp6_hdr icmp6;
- struct mld_hdr mld;
- struct nd_neighbor_solicit nd_ns;
-#endif /* INET6 */
-};
-
-
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;
@@ -5798,7 +5783,7 @@ pf_rtlabel_match(struct pf_addr *addr, s
}
void
-pf_route(struct mbuf **m, struct pf_rule *r, int dir, struct ifnet *oifp,
+pf_route(struct mbuf **m, struct pf_pdesc *pd, struct pf_rule *r,
struct pf_state *s)
{
struct mbuf *m0, *m1;
@@ -5811,8 +5796,7 @@ pf_route(struct mbuf **m, struct pf_rule
int error = 0;
unsigned int rtableid;
- if (m == NULL || *m == NULL || r == NULL ||
- (dir != PF_IN && dir != PF_OUT) || oifp == NULL)
+ if (m == NULL || *m == NULL || r == NULL)
panic("pf_route: invalid parameters");
if ((*m)->m_pkthdr.pf.routed++ > 3) {
@@ -5825,7 +5809,7 @@ pf_route(struct mbuf **m, struct pf_rule
if ((m0 = m_dup_pkt(*m, max_linkhdr, M_NOWAIT)) == NULL)
return;
} else {
- if ((r->rt == PF_REPLYTO) == (r->direction == dir))
+ if ((r->rt == PF_REPLYTO) == (r->direction == pd->dir))
return;
m0 = *m;
}
@@ -5890,7 +5874,7 @@ pf_route(struct mbuf **m, struct pf_rule
goto bad;
- if (oifp != ifp) {
+ if (pd->kif->pfik_ifp != ifp) {
if (pf_test(AF_INET, PF_OUT, ifp, &m0) != PF_PASS)
goto bad;
else if (m0 == NULL)
@@ -5965,7 +5949,7 @@ bad:
#ifdef INET6
void
-pf_route6(struct mbuf **m, struct pf_rule *r, int dir, struct ifnet *oifp,
+pf_route6(struct mbuf **m, struct pf_pdesc *pd, struct pf_rule *r,
struct pf_state *s)
{
struct mbuf *m0;
@@ -5978,8 +5962,7 @@ pf_route6(struct mbuf **m, struct pf_rul
struct m_tag *mtag;
unsigned int rtableid;
- if (m == NULL || *m == NULL || r == NULL ||
- (dir != PF_IN && dir != PF_OUT) || oifp == NULL)
+ if (m == NULL || *m == NULL || r == NULL)
panic("pf_route6: invalid parameters");
if ((*m)->m_pkthdr.pf.routed++ > 3) {
@@ -5992,7 +5975,7 @@ pf_route6(struct mbuf **m, struct pf_rul
if ((m0 = m_dup_pkt(*m, max_linkhdr, M_NOWAIT)) == NULL)
return;
} else {
- if ((r->rt == PF_REPLYTO) == (r->direction == dir))
+ if ((r->rt == PF_REPLYTO) == (r->direction == pd->dir))
return;
m0 = *m;
}
@@ -6038,7 +6021,7 @@ pf_route6(struct mbuf **m, struct pf_rul
if (ifp == NULL)
goto bad;
- if (oifp != ifp) {
+ if (pd->kif->pfik_ifp != ifp) {
if (pf_test(AF_INET6, PF_OUT, ifp, &m0) != PF_PASS)
goto bad;
else if (m0 == NULL)
@@ -6939,9 +6922,9 @@ done:
break;
}
if (pd.naf == AF_INET)
- pf_route(&pd.m, r, dir, kif->pfik_ifp, s);
+ pf_route(&pd.m, &pd, r, s);
if (pd.naf == AF_INET6)
- pf_route6(&pd.m, r, dir, kif->pfik_ifp, s);
+ pf_route6(&pd.m, &pd, r, s);
*m0 = NULL;
action = PF_PASS;
break;
@@ -6955,11 +6938,11 @@ done:
if (r->rt) {
switch (pd.af) {
case AF_INET:
- pf_route(m0, r, pd.dir, pd.kif->pfik_ifp, s);
+ pf_route(m0, &pd, r, s);
break;
#ifdef INET6
case AF_INET6:
- pf_route6(m0, r, pd.dir, pd.kif->pfik_ifp, s);
+ pf_route6(m0, &pd, r, s);
break;
#endif /* INET6 */
}
Index: net/pfhdr.h
===================================================================
RCS file: net/pfhdr.h
diff -N net/pfhdr.h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ net/pfhdr.h 21 Oct 2016 17:47:53 -0000
@@ -0,0 +1,38 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2016 Alexander Bluhm <[email protected]>
+ * Copyright (c) 2010 Henning Brauer <[email protected]>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef _NET_PFHDR_H_
+#define _NET_PFHDR_H_
+
+/*
+ * Cannot fold into pf_pdesc directly, unknown storage size outside pf.c.
+ * Cannot be put into pfvar.h as this is included in too many places.
+ */
+union pf_headers {
+ struct tcphdr tcp;
+ struct udphdr udp;
+ struct icmp icmp;
+#ifdef INET6
+ struct icmp6_hdr icmp6;
+ struct mld_hdr mld;
+ struct nd_neighbor_solicit nd_ns;
+#endif /* INET6 */
+};
+
+#endif /* _NET_PFHDR_H_ */
Index: net/pfvar.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfvar.h,v
retrieving revision 1.441
diff -u -p -r1.441 pfvar.h
--- net/pfvar.h 18 Oct 2016 13:28:01 -0000 1.441
+++ net/pfvar.h 21 Oct 2016 17:32:52 -0000
@@ -1765,10 +1765,10 @@ int pf_state_key_attach(struct pf_state_
int pf_translate(struct pf_pdesc *, struct pf_addr *, u_int16_t,
struct pf_addr *, u_int16_t, u_int16_t, int);
int pf_translate_af(struct pf_pdesc *);
-void pf_route(struct mbuf **, struct pf_rule *, int,
- struct ifnet *, struct pf_state *);
-void pf_route6(struct mbuf **, struct pf_rule *, int,
- struct ifnet *, struct pf_state *);
+void pf_route(struct mbuf **, struct pf_pdesc *, struct pf_rule *,
+ struct pf_state *);
+void pf_route6(struct mbuf **, struct pf_pdesc *, struct pf_rule *,
+ struct pf_state *);
void pfr_initialize(void);
int pfr_match_addr(struct pfr_ktable *, struct pf_addr *, sa_family_t);
