On Wed, Oct 19, 2016 at 11:49:56PM +0200, Alexander Bluhm wrote:
> Hi,
>
> I would like to pass a struct pf_pdesc to pf_route() like it is
> done in the other pf functions. That means less parameters, more
> consistency and later I can call functions that need an pd from
> pf_route().
>
> 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.
>
> ok?
>
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.
> bluhm
>
> 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 -u -p -r1.235 if_pfsync.c
> --- net/if_pfsync.c 4 Oct 2016 13:54:32 -0000 1.235
> +++ net/if_pfsync.c 19 Oct 2016 21:22:27 -0000
> @@ -1732,6 +1732,7 @@ void
> pfsync_undefer(struct pfsync_deferral *pd, int drop)
> {
> struct pfsync_softc *sc = pfsyncif;
> + struct pf_pdesc pdesc;
>
> splsoftassert(IPL_SOFTNET);
>
> @@ -1743,17 +1744,18 @@ pfsync_undefer(struct pfsync_deferral *p
> m_freem(pd->pd_m);
> else {
> if (pd->pd_st->rule.ptr->rt == PF_ROUTETO) {
> + memset(&pdesc, 0, sizeof(pdesc));
> + pdesc.dir = pd->pd_st->direction;
> + pdesc.kif = pd->pd_st->rt_kif;
> 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 */
> }
> Index: net/pf.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.992
> diff -u -p -u -p -r1.992 pf.c
> --- net/pf.c 18 Oct 2016 13:28:01 -0000 1.992
> +++ net/pf.c 19 Oct 2016 21:26:59 -0000
> @@ -5764,7 +5764,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;
> @@ -5777,8 +5777,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) {
> @@ -5791,7 +5790,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;
> }
> @@ -5856,7 +5855,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)
> @@ -5931,7 +5930,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;
> @@ -5944,8 +5943,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) {
> @@ -5958,7 +5956,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;
> }
> @@ -6004,7 +6002,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)
> @@ -6905,9 +6903,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;
> @@ -6921,11 +6919,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/pfvar.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfvar.h,v
> retrieving revision 1.441
> diff -u -p -u -p -r1.441 pfvar.h
> --- net/pfvar.h 18 Oct 2016 13:28:01 -0000 1.441
> +++ net/pfvar.h 19 Oct 2016 21:22:27 -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);
>
--
:wq Claudio
