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);
                        }

Reply via email to