While looking into Alexandr's report I made a few nits that I consider worth getting in. This is the first one.
multi is just a flag these days (for better or worse), so having a enum and a store in pf_icmp_mapping is pointless since the usage is always the same: try looking up an ICMP state using real addresses and then if that fails and we happen to be looking up an ICMPv6 state, try multicast. No need for fancy names here. It used to be a triple value toggle, but that was changed a few years ago. OK? diff --git sys/net/pf.c sys/net/pf.c index 5bd5864..d4cb67c 100644 --- sys/net/pf.c +++ sys/net/pf.c @@ -147,11 +147,11 @@ void pf_change_ap(struct pf_pdesc *, struct pf_addr *, int pf_modulate_sack(struct pf_pdesc *, struct pf_state_peer *); void pf_change_a6(struct pf_pdesc *, struct pf_addr *a, struct pf_addr *an); int pf_icmp_mapping(struct pf_pdesc *, u_int8_t, int *, - int *, u_int16_t *, u_int16_t *); + u_int16_t *, u_int16_t *); void pf_change_icmp(struct pf_pdesc *, struct pf_addr *, u_int16_t *, struct pf_addr *, struct pf_addr *, u_int16_t, sa_family_t); int pf_change_icmp_af(struct mbuf *, int, struct pf_pdesc *, struct pf_pdesc *, @@ -242,13 +242,10 @@ struct pf_pool_limit pf_pool_limits[PF_LIMIT_MAX] = { { &pf_frent_pl, PFFRAG_FRENT_HIWAT, PFFRAG_FRENT_HIWAT }, { &pfr_ktable_pl, PFR_KTABLE_HIWAT, PFR_KTABLE_HIWAT }, { &pfr_kentry_pl, PFR_KENTRY_HIWAT, PFR_KENTRY_HIWAT } }; -enum { PF_ICMP_MULTI_NONE, PF_ICMP_MULTI_LINK }; - - #define STATE_LOOKUP(i, k, d, s, m) \ do { \ s = pf_find_state(i, k, d, m); \ if (s == NULL || (s)->timeout == PFTM_PURGE) \ return (PF_DROP); \ @@ -817,11 +814,11 @@ pf_state_key_addr_setup(struct pf_pdesc *pd, void *arg, int sidx, key->addr[didx].addr32[3] = 0; daddr = NULL; /* overwritten */ } break; default: - if (multi == PF_ICMP_MULTI_LINK) { + if (multi) { key->addr[sidx].addr32[0] = __IPV6_ADDR_INT32_MLL; key->addr[sidx].addr32[1] = 0; key->addr[sidx].addr32[2] = 0; key->addr[sidx].addr32[3] = __IPV6_ADDR_INT32_ONE; saddr = NULL; /* overwritten */ @@ -1687,20 +1684,19 @@ pf_change_a6(struct pf_pdesc *pd, struct pf_addr *a, struct pf_addr *an) PF_ACPY(a, an, AF_INET6); } #endif /* INET6 */ int -pf_icmp_mapping(struct pf_pdesc *pd, u_int8_t type, int *icmp_dir, int *multi, +pf_icmp_mapping(struct pf_pdesc *pd, u_int8_t type, int *icmp_dir, u_int16_t *virtual_id, u_int16_t *virtual_type) { /* * ICMP types marked with PF_OUT are typically responses to * PF_IN, and will match states in the opposite direction. * PF_IN ICMP types need to match a state with that type. */ *icmp_dir = PF_OUT; - *multi = PF_ICMP_MULTI_LINK; /* Queries (and responses) */ switch (pd->af) { case AF_INET: switch (type) { @@ -3079,11 +3075,11 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm, struct pf_state **sm, u_short reason; int rewrite = 0; int tag = -1; int asd = 0; int match = 0; - int state_icmp = 0, icmp_dir, multi; + int state_icmp = 0, icmp_dir; u_int16_t virtual_type, virtual_id; u_int8_t icmptype = 0, icmpcode = 0; bzero(&act, sizeof(act)); bzero(sns, sizeof(sns)); @@ -3098,11 +3094,11 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm, struct pf_state **sm, switch (pd->virtual_proto) { case IPPROTO_ICMP: icmptype = pd->hdr.icmp->icmp_type; icmpcode = pd->hdr.icmp->icmp_code; state_icmp = pf_icmp_mapping(pd, icmptype, - &icmp_dir, &multi, &virtual_id, &virtual_type); + &icmp_dir, &virtual_id, &virtual_type); if (icmp_dir == PF_IN) { pd->osport = pd->nsport = virtual_id; pd->odport = pd->ndport = virtual_type; } else { pd->osport = pd->nsport = virtual_type; @@ -3112,11 +3108,11 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm, struct pf_state **sm, #ifdef INET6 case IPPROTO_ICMPV6: icmptype = pd->hdr.icmp6->icmp6_type; icmpcode = pd->hdr.icmp6->icmp6_code; state_icmp = pf_icmp_mapping(pd, icmptype, - &icmp_dir, &multi, &virtual_id, &virtual_type); + &icmp_dir, &virtual_id, &virtual_type); if (icmp_dir == PF_IN) { pd->osport = pd->nsport = virtual_id; pd->odport = pd->ndport = virtual_type; } else { pd->osport = pd->nsport = virtual_type; @@ -4468,11 +4464,11 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, u_short *reason) { struct pf_addr *saddr = pd->src, *daddr = pd->dst; u_int16_t virtual_id, virtual_type; u_int8_t icmptype; - int icmp_dir, iidx, ret, multi, copyback = 0; + int icmp_dir, iidx, ret, copyback = 0; struct pf_state_key_cmp key; switch (pd->proto) { case IPPROTO_ICMP: @@ -4483,25 +4479,25 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, icmptype = pd->hdr.icmp6->icmp6_type; break; #endif /* INET6 */ } - if (pf_icmp_mapping(pd, icmptype, &icmp_dir, &multi, - &virtual_id, &virtual_type) == 0) { + if (pf_icmp_mapping(pd, icmptype, &icmp_dir, &virtual_id, + &virtual_type) == 0) { /* * ICMP query/reply message not related to a TCP/UDP packet. * Search for an ICMP state. */ ret = pf_icmp_state_lookup(pd, &key, state, virtual_id, virtual_type, icmp_dir, &iidx, - PF_ICMP_MULTI_NONE, 0); + 0, 0); if (ret >= 0) { if (ret == PF_DROP && pd->af == AF_INET6 && icmp_dir == PF_OUT) { ret = pf_icmp_state_lookup(pd, &key, state, virtual_id, virtual_type, icmp_dir, &iidx, - multi, 0); + 1, 0); if (ret >= 0) return (ret); } else return (ret); } @@ -4995,15 +4991,14 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, return (PF_DROP); } pd2.hdr.icmp = &iih; pf_icmp_mapping(&pd2, iih.icmp_type, - &icmp_dir, &multi, &virtual_id, &virtual_type); + &icmp_dir, &virtual_id, &virtual_type); ret = pf_icmp_state_lookup(&pd2, &key, state, - virtual_id, virtual_type, icmp_dir, &iidx, - PF_ICMP_MULTI_NONE, 1); + virtual_id, virtual_type, icmp_dir, &iidx, 0, 1); if (ret >= 0) return (ret); /* translate source/destination address, if necessary */ if ((*state)->key[PF_SK_WIRE] != @@ -5103,20 +5098,19 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, return (PF_DROP); } pd2.hdr.icmp6 = &iih; pf_icmp_mapping(&pd2, iih.icmp6_type, - &icmp_dir, &multi, &virtual_id, &virtual_type); + &icmp_dir, &virtual_id, &virtual_type); ret = pf_icmp_state_lookup(&pd2, &key, state, - virtual_id, virtual_type, icmp_dir, &iidx, - PF_ICMP_MULTI_NONE, 1); + virtual_id, virtual_type, icmp_dir, &iidx, 0, 1); if (ret >= 0) { if (ret == PF_DROP && pd2.af == AF_INET6 && icmp_dir == PF_OUT) { ret = pf_icmp_state_lookup(&pd2, &key, state, virtual_id, virtual_type, - icmp_dir, &iidx, multi, 1); + icmp_dir, &iidx, 1, 1); if (ret >= 0) return (ret); } else return (ret); }