On Thu, May 10, 2012 at 09:38:39PM +0200, Henning Brauer wrote: > I'm looking for oks on this diff to commit it.
I think this is not correct. > > @@ -6951,12 +6953,12 @@ done: > > struct pf_rule_item *ri; > > > > if (pd.pflog & PF_LOG_FORCE || r->log & PF_LOG_ALL) > > - PFLOG_PACKET(&pd, reason, r, a, ruleset); > > + PFLOG_PACKET(&pd, reason, action, r, a, ruleset); > > if (s) { > > SLIST_FOREACH(ri, &s->match_rules, entry) > > if (ri->r->log & PF_LOG_ALL) > > - PFLOG_PACKET(&pd, reason, ri->r, a, > > - ruleset); > > + PFLOG_PACKET(&pd, reason, action, > > + ri->r, a, ruleset); > > } > > } The variable action does not hold the final action at this place. It could be PF_SYNPROXY_DROP PF_DEFER PF_DIVERT PF_AFRT, we don't want to log this. It is rewrittren in the switch (action) block below. Moving the logging after the switch is not an option as the mbuf *m0 might get freed there. We could do PFLOG_PACKET(&pd, reason, action == PF_PASS ? PF_PASS : PF_DROP, ri->r, a, ruleset) for most cases. Unfortunately PF_AFRT could be set to PF_DROP in some cases. Any ideas for a better fix? bluhm