(Feel myself like a spammer today... :-\ )
And now with fixed rule reference counting in if_pfsync.c (modelled
after pf_test_rule()):
if (rtr != NULL && rtr != r) {
/*
* We need to keep the routing rule in case of changes,
* so add a reference.
*
* XXX: Need accounting for other match rules?
*/
SLIST_INIT(&st->match_rules);
if ((ri = pool_get(&pf_rule_item_pl, pool_flags)) == NULL)
goto cleanup;
ri->r = rtr;
SLIST_INSERT_HEAD(&st->match_rules, ri, entry);
/* no need for pf_get_transaddr() because no NAT is done here */
}
Also, state count is decremented for rtr on error now too. I found
no more places needing intervention.
One problem discovered: "match" rules does not have accounting when
their states being imported through pfsync. If this should be fixed
then it's probably better to not use padding bytes for now, and
think about exchanging match rules lists for states over pfsync?
I have plans for allowing specifing more than one routing option.
And this will require either saving to more 2 rule IDs in
structure pfsync_state, or pushing match rules lists over the wire.
Last one looks like will require pfsync version bump anyway. :-(
Index: sys/net/if_pflog.c
===================================================================
RCS file: /cvs/src/sys/net/if_pflog.c,v
retrieving revision 1.32
diff -u -p -r1.32 if_pflog.c
--- sys/net/if_pflog.c 21 Sep 2010 22:49:14 -0000 1.32
+++ sys/net/if_pflog.c 16 Nov 2010 12:49:11 -0000
@@ -212,7 +212,7 @@ pflogioctl(struct ifnet *ifp, u_long cmd
int
pflog_packet(struct pfi_kif *kif, struct mbuf *m, sa_family_t af, u_int8_t dir,
u_int8_t reason, struct pf_rule *rm, struct pf_rule *am,
- struct pf_ruleset *ruleset, struct pf_pdesc *pd)
+ struct pf_rule *rtr, struct pf_ruleset *ruleset, struct pf_pdesc *pd)
{
#if NBPFILTER > 0
struct ifnet *ifn;
@@ -241,6 +241,10 @@ pflog_packet(struct pfi_kif *kif, struct
strlcpy(hdr.ruleset, ruleset->anchor->name,
sizeof(hdr.ruleset));
}
+ if (rtr == NULL)
+ hdr.rt_rulenr = htonl(-1);
+ else
+ hdr.rt_rulenr = htonl(rtr->nr);
if (rm->log & PF_LOG_SOCKET_LOOKUP && !pd->lookup.done)
pd->lookup.done = pf_socket_lookup(dir, pd);
if (pd->lookup.done > 0) {
Index: sys/net/if_pflog.h
===================================================================
RCS file: /cvs/src/sys/net/if_pflog.h,v
retrieving revision 1.17
diff -u -p -r1.17 if_pflog.h
--- sys/net/if_pflog.h 21 Sep 2010 11:05:10 -0000 1.17
+++ sys/net/if_pflog.h 16 Nov 2010 12:49:11 -0000
@@ -59,6 +59,7 @@ struct pfloghdr {
struct pf_addr daddr;
u_int16_t sport;
u_int16_t dport;
+ u_int32_t rt_rulenr;
};
#define PFLOG_HDRLEN sizeof(struct pfloghdr)
@@ -70,9 +71,9 @@ struct pfloghdr {
void pflog_bpfcopy(const void *, void *, size_t);
#if NPFLOG > 0
-#define PFLOG_PACKET(i,x,a,b,c,d,e,f,g,h)
pflog_packet(i,a,b,c,d,e,f,g,h)
+#define PFLOG_PACKET(i,x,a,b,c,d,e,f,g,h,j)
pflog_packet(i,a,b,c,d,e,f,g,h,j)
#else
-#define PFLOG_PACKET(i,x,a,b,c,d,e,f,g,h) ((void)0)
+#define PFLOG_PACKET(i,x,a,b,c,d,e,f,g,h,j) ((void)0)
#endif /* NPFLOG > 0 */
#endif /* _KERNEL */
#endif /* _NET_IF_PFLOG_H_ */
Index: sys/net/if_pfsync.c
===================================================================
RCS file: /cvs/src/sys/net/if_pfsync.c,v
retrieving revision 1.156
diff -u -p -r1.156 if_pfsync.c
--- sys/net/if_pfsync.c 27 Sep 2010 23:45:48 -0000 1.156
+++ sys/net/if_pfsync.c 16 Nov 2010 12:49:11 -0000
@@ -466,6 +466,15 @@ pfsync_state_export(struct pfsync_state
sp->anchor = htonl(st->anchor.ptr->nr);
sp->nat_rule = htonl(-1); /* left for compat, nat_rule is gone */
+ /*
+ * Make pfsync with and without support of routing rules co-exist: leave
+ * 0 instead -1 and do the decrement dance in pfsync_state_import().
+ */
+ if (st->rt_rule == NULL)
+ sp->rt_rule = 0;
+ else
+ sp->rt_rule = htonl(st->rt_rule->nr + 1);
+
pf_state_counter_hton(st->packets[0], sp->packets[0]);
pf_state_counter_hton(st->packets[1], sp->packets[1]);
pf_state_counter_hton(st->bytes[0], sp->bytes[0]);
@@ -479,12 +488,12 @@ pfsync_state_export(struct pfsync_state
int
pfsync_state_import(struct pfsync_state *sp, u_int8_t flags)
{
- struct pf_state *st = NULL;
- struct pf_state_key *skw = NULL, *sks = NULL;
- struct pf_rule *r = NULL;
- struct pfi_kif *kif;
- int pool_flags;
- int error;
+ struct pf_state *st = NULL;
+ struct pf_state_key *skw = NULL, *sks = NULL;
+ struct pf_rule *r = NULL, *rtr = NULL;
+ struct pf_rule_item *ri;
+ struct pfi_kif *kif;
+ int error, pool_flags;
if (sp->creatorid == 0) {
DPFPRINTF(LOG_NOTICE, "pfsync_state_import: "
@@ -504,12 +513,18 @@ pfsync_state_import(struct pfsync_state
* If the ruleset checksums match or the state is coming from the ioctl,
* it's safe to associate the state with the rule of that number.
*/
- if (sp->rule != htonl(-1) && sp->anchor == htonl(-1) &&
- (flags & (PFSYNC_SI_IOCTL | PFSYNC_SI_CKSUM)) && ntohl(sp->rule) <
- pf_main_ruleset.rules.active.rcount)
- r = pf_main_ruleset.rules.active.ptr_array[ntohl(sp->rule)];
- else
- r = &pf_default_rule;
+ if ((flags & (PFSYNC_SI_IOCTL | PFSYNC_SI_CKSUM))) {
+ if (sp->rule != htonl(-1) && sp->anchor == htonl(-1) &&
+ ntohl(sp->rule) < pf_main_ruleset.rules.active.rcount)
+ r =
pf_main_ruleset.rules.active.ptr_array[ntohl(sp->rule)];
+ else
+ r = &pf_default_rule;
+
+ /* 0 means either "no routing rule" or "routing rules not
supported" */
+ if (sp->rt_rule != 0 &&
+ ntohl(sp->rt_rule) - 1 <
pf_main_ruleset.rules.active.rcount)
+ rtr =
pf_main_ruleset.rules.active.ptr_array[ntohl(sp->rt_rule) - 1];
+ }
if ((r->max_states && r->states_cur >= r->max_states))
goto cleanup;
@@ -588,6 +603,23 @@ pfsync_state_import(struct pfsync_state
st->rule.ptr = r;
st->anchor.ptr = NULL;
st->rt_kif = NULL;
+ st->rt_rule = rtr;
+
+ if (rtr != NULL && rtr != r) {
+ /*
+ * We need to keep the routing rule in case of changes,
+ * so add a reference.
+ *
+ * XXX: Need accounting for other match rules?
+ */
+
+ SLIST_INIT(&st->match_rules);
+ if ((ri = pool_get(&pf_rule_item_pl, pool_flags)) == NULL)
+ goto cleanup;
+ ri->r = rtr;
+ SLIST_INSERT_HEAD(&st->match_rules, ri, entry);
+ /* no need for pf_get_transaddr() because no NAT is done here */
+ }
st->pfsync_time = time_uptime;
st->sync_state = PFSYNC_S_NONE;
@@ -595,6 +627,8 @@ pfsync_state_import(struct pfsync_state
/* XXX when we have anchors, use STATE_INC_COUNTERS */
r->states_cur++;
r->states_tot++;
+ if (rtr != NULL)
+ rtr->states_cur++;
if (!ISSET(flags, PFSYNC_SI_IOCTL))
SET(st->state_flags, PFSTATE_NOSYNC);
@@ -602,6 +636,8 @@ pfsync_state_import(struct pfsync_state
if (pf_state_insert(kif, skw, sks, st) != 0) {
/* XXX when we have anchors, use STATE_DEC_COUNTERS */
r->states_cur--;
+ if (rtr != NULL)
+ rtr->states_cur--;
error = EEXIST;
goto cleanup_state;
}
Index: sys/net/pf.c
===================================================================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.713
diff -u -p -r1.713 pf.c
--- sys/net/pf.c 24 Sep 2010 02:28:10 -0000 1.713
+++ sys/net/pf.c 16 Nov 2010 12:49:12 -0000
@@ -263,11 +263,11 @@ enum { PF_ICMP_MULTI_NONE, PF_ICMP_MULTI
s = pf_find_state(i, k, d, m); \
if (s == NULL || (s)->timeout == PFTM_PURGE) \
return (PF_DROP); \
- if (d == PF_OUT && \
- (((s)->rule.ptr->rt == PF_ROUTETO && \
- (s)->rule.ptr->direction == PF_OUT) || \
- ((s)->rule.ptr->rt == PF_REPLYTO && \
- (s)->rule.ptr->direction == PF_IN)) && \
+ if (d == PF_OUT && (s)->rt_rule != NULL && \
+ (((s)->rt_rule->rt == PF_ROUTETO && \
+ (s)->rt_rule->direction == PF_OUT) || \
+ ((s)->rt_rule->rt == PF_REPLYTO && \
+ (s)->rt_rule->direction == PF_IN)) && \
(s)->rt_kif != NULL && \
(s)->rt_kif != i) \
return (PF_PASS); \
@@ -2695,11 +2695,11 @@ pf_calc_mss(struct pf_addr *addr, sa_fam
void
pf_set_rt_ifp(struct pf_state *s, struct pf_addr *saddr)
{
- struct pf_rule *r = s->rule.ptr;
+ struct pf_rule *r = s->rt_rule;
struct pf_src_node *sn = NULL;
s->rt_kif = NULL;
- if (!r->rt || r->rt == PF_FASTROUTE)
+ if (r == NULL || !r->rt || r->rt == PF_FASTROUTE)
return;
switch (s->key[PF_SK_WIRE]->af) {
#ifdef INET
@@ -2764,6 +2764,8 @@ pf_rule_to_actions(struct pf_rule *r, st
a->min_ttl = r->min_ttl;
if (r->max_mss)
a->max_mss = r->max_mss;
+ if (r->rt)
+ a->rt_rule = r;
a->flags |= (r->scrub_flags & (PFSTATE_NODF|PFSTATE_RANDOMID|
PFSTATE_SETTOS|PFSTATE_SCRUB_TCP));
}
@@ -2944,7 +2946,7 @@ pf_test_rule(struct pf_rule **rm, struct
if (r->log || act.log & PF_LOG_MATCHES)
PFLOG_PACKET(kif, h, m, af,
direction, reason, r,
- a, ruleset, pd);
+ a, act.rt_rule, ruleset,
pd);
} else {
match = 1;
*rm = r;
@@ -2953,7 +2955,7 @@ pf_test_rule(struct pf_rule **rm, struct
if (act.log & PF_LOG_MATCHES)
PFLOG_PACKET(kif, h, m, af,
direction, reason, r,
- a, ruleset, pd);
+ a, act.rt_rule, ruleset,
pd);
}
if ((*rm)->quick)
@@ -2981,7 +2983,7 @@ pf_test_rule(struct pf_rule **rm, struct
if (r->log || act.log & PF_LOG_MATCHES)
PFLOG_PACKET(kif, h, m, af, direction, reason,
- r, a, ruleset, pd);
+ r, a, act.rt_rule, ruleset, pd);
if ((r->action == PF_DROP) &&
((r->rule_flag & PFRULE_RETURNRST) ||
@@ -3147,6 +3149,7 @@ pf_create_state(struct pf_rule *r, struc
s->min_ttl = act->min_ttl;
s->set_tos = act->set_tos;
s->max_mss = act->max_mss;
+ s->rt_rule = act->rt_rule;
s->state_flags |= act->flags;
s->sync_state = PFSYNC_S_NONE;
switch (pd->proto) {
@@ -3419,7 +3422,7 @@ pf_test_fragment(struct pf_rule **rm, in
struct mbuf *m, void *h, struct pf_pdesc *pd, struct pf_rule **am,
struct pf_ruleset **rsm)
{
- struct pf_rule *r, *a = NULL;
+ struct pf_rule *r, *a = NULL, *rtr = NULL;
struct pf_ruleset *ruleset = NULL;
sa_family_t af = pd->af;
u_short reason;
@@ -3464,6 +3467,7 @@ pf_test_fragment(struct pf_rule **rm, in
else if (r->match_tag && !pf_match_tag(m, r, &tag))
r = TAILQ_NEXT(r, entries);
else {
+ /* XXX: No handling of "match" rules? */
if (r->anchor == NULL) {
match = 1;
*rm = r;
@@ -3483,12 +3487,14 @@ pf_test_fragment(struct pf_rule **rm, in
r = *rm;
a = *am;
ruleset = *rsm;
+ if (r->rt)
+ rtr = r;
REASON_SET(&reason, PFRES_MATCH);
if (r->log)
- PFLOG_PACKET(kif, h, m, af, direction, reason, r, a, ruleset,
- pd);
+ PFLOG_PACKET(kif, h, m, af, direction, reason, r, a, rtr,
+ ruleset, pd);
if (r->action == PF_DROP)
return (PF_DROP);
@@ -5830,7 +5836,7 @@ pf_test(int dir, struct ifnet *ifp, stru
u_short action, reason = 0, pflog = 0;
struct mbuf *m = *m0;
struct ip *h;
- struct pf_rule *a = NULL, *r = &pf_default_rule;
+ struct pf_rule *a = NULL, *r = &pf_default_rule, *rtr = NULL;
struct pf_state *s = NULL;
struct pf_ruleset *ruleset = NULL;
struct pf_pdesc pd;
@@ -5998,12 +6004,15 @@ done:
qid = s->pqid;
else
qid = s->qid;
+ rtr = s->rt_rule;
} else {
pf_scrub_ip(&m, r->scrub_flags, r->min_ttl, r->set_tos);
if (pqid || (pd.tos & IPTOS_LOWDELAY))
qid = r->pqid;
else
qid = r->qid;
+ if (r->rt)
+ rtr = r;
}
if (dir == PF_IN && s && s->key[PF_SK_STACK])
@@ -6052,12 +6061,13 @@ done:
if (pflog & PF_LOG_FORCE || r->log & PF_LOG_ALL)
PFLOG_PACKET(kif, h, m, AF_INET, dir, reason, r, a,
- ruleset, &pd);
+ rtr, ruleset, &pd);
if (s) {
SLIST_FOREACH(ri, &s->match_rules, entry)
if (ri->r->log & PF_LOG_ALL)
PFLOG_PACKET(kif, h, m, AF_INET, dir,
- reason, ri->r, a, ruleset, &pd);
+ reason, ri->r, a, rtr, ruleset,
+ &pd);
}
}
@@ -6077,8 +6087,8 @@ done:
break;
default:
/* pf_route can free the mbuf causing *m0 to become NULL */
- if (r->rt)
- pf_route(m0, r, dir, kif->pfik_ifp, s, &pd);
+ if (rtr != NULL)
+ pf_route(m0, rtr, dir, kif->pfik_ifp, s, &pd);
break;
}
@@ -6095,7 +6105,7 @@ pf_test6(int dir, struct ifnet *ifp, str
u_short action, reason = 0, pflog = 0;
struct mbuf *m = *m0, *n = NULL;
struct ip6_hdr *h;
- struct pf_rule *a = NULL, *r = &pf_default_rule;
+ struct pf_rule *a = NULL, *r = &pf_default_rule, *rtr = NULL;
struct pf_state *s = NULL;
struct pf_ruleset *ruleset = NULL;
struct pf_pdesc pd;
@@ -6267,10 +6277,14 @@ done:
"pf: dropping packet with dangerous v6 headers");
}
- if (s)
+ if (s) {
pf_scrub_ip6(&m, s->min_ttl);
- else
+ rtr = s->rt_rule;
+ } else {
pf_scrub_ip6(&m, r->min_ttl);
+ if (r->rt)
+ rtr = r;
+ }
if (s && s->tag)
pf_tag_packet(m, s ? s->tag : 0, s->rtableid[pd.didx]);
@@ -6320,12 +6334,13 @@ done:
if (pflog & PF_LOG_FORCE || r->log & PF_LOG_ALL)
PFLOG_PACKET(kif, h, m, AF_INET6, dir, reason, r, a,
- ruleset, &pd);
+ rtr, ruleset, &pd);
if (s) {
SLIST_FOREACH(ri, &s->match_rules, entry)
if (ri->r->log & PF_LOG_ALL)
PFLOG_PACKET(kif, h, m, AF_INET6, dir,
- reason, ri->r, a, ruleset, &pd);
+ reason, ri->r, a, rtr, ruleset,
+ &pd);
}
}
@@ -6345,8 +6360,8 @@ done:
break;
default:
/* pf_route6 can free the mbuf causing *m0 to become NULL */
- if (r->rt)
- pf_route6(m0, r, dir, kif->pfik_ifp, s, &pd);
+ if (rtr != NULL)
+ pf_route6(m0, rtr, dir, kif->pfik_ifp, s, &pd);
break;
}
Index: sys/net/pfvar.h
===================================================================
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.318
diff -u -p -r1.318 pfvar.h
--- sys/net/pfvar.h 23 Oct 2010 15:38:18 -0000 1.318
+++ sys/net/pfvar.h 16 Nov 2010 12:49:13 -0000
@@ -520,15 +520,16 @@ struct pf_osfp_ioctl {
};
struct pf_rule_actions {
- int rtableid;
- u_int16_t qid;
- u_int16_t pqid;
- u_int16_t max_mss;
- u_int8_t log;
- u_int8_t set_tos;
- u_int8_t min_ttl;
- u_int8_t pad[1];
- u_int16_t flags;
+ struct pf_rule *rt_rule;
+ int rtableid;
+ u_int16_t qid;
+ u_int16_t pqid;
+ u_int16_t max_mss;
+ u_int8_t log;
+ u_int8_t set_tos;
+ u_int8_t min_ttl;
+ u_int8_t pad[1];
+ u_int16_t flags;
};
union pf_rule_ptr {
@@ -807,6 +808,7 @@ struct pf_state {
struct pf_state_key *key[2]; /* addresses stack and wire */
struct pfi_kif *kif;
struct pfi_kif *rt_kif;
+ struct pf_rule *rt_rule;
u_int64_t packets[2];
u_int64_t bytes[2];
u_int32_t creation;
@@ -896,7 +898,7 @@ struct pfsync_state {
u_int8_t updates;
u_int8_t min_ttl;
u_int8_t set_tos;
- u_int8_t pad[4];
+ u_int32_t rt_rule;
} __packed;
#define PFSYNC_FLAG_SRCNODE 0x04
@@ -1758,8 +1760,8 @@ void *pf_pull_hdr(struct mbuf *, int,
sa_family_t);
void pf_change_a(void *, u_int16_t *, u_int32_t, u_int8_t);
int pflog_packet(struct pfi_kif *, struct mbuf *, sa_family_t, u_int8_t,
- u_int8_t, struct pf_rule *, struct pf_rule *, struct pf_ruleset *,
- struct pf_pdesc *);
+ u_int8_t, struct pf_rule *, struct pf_rule *, struct pf_rule *,
+ struct pf_ruleset *, struct pf_pdesc *);
void pf_send_deferred_syn(struct pf_state *);
int pf_match_addr(u_int8_t, struct pf_addr *, struct pf_addr *,
struct pf_addr *, sa_family_t);
Index: sbin/pfctl/parse.y
===================================================================
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.594
diff -u -p -r1.594 parse.y
--- sbin/pfctl/parse.y 24 Sep 2010 09:17:46 -0000 1.594
+++ sbin/pfctl/parse.y 16 Nov 2010 12:49:13 -0000
@@ -4025,11 +4025,6 @@ rule_consistent(struct pf_rule *r, int a
yyerror("divert is not supported on match rules");
problems++;
}
- if (r->rt) {
- yyerror("route-to, reply-to, dup-to and fastroute "
- "must not be used on match rules");
- problems++;
- }
}
return (-problems);
}
Index: usr.sbin/tcpdump/print-pflog.c
===================================================================
RCS file: /cvs/src/usr.sbin/tcpdump/print-pflog.c,v
retrieving revision 1.23
diff -u -p -r1.23 print-pflog.c
--- usr.sbin/tcpdump/print-pflog.c 9 Oct 2010 08:22:26 -0000 1.23
+++ usr.sbin/tcpdump/print-pflog.c 16 Nov 2010 12:49:13 -0000
@@ -171,6 +171,8 @@ pflog_if_print(u_char *user, const struc
printf("dst %s:%u] ", buf,
ntohs(hdr->dport));
}
+ if (vflag && ntohl(hdr->rt_rulenr) != (u_int32_t) -1)
+ printf("[rtrule %u] ", ntohl(hdr->rt_rulenr));
}
af = hdr->af;
length -= hdrlen;
