Hello Yasuok,
On Thu, Jul 23, 2020 at 08:01:18PM +0900, YASUOKA Masahiko wrote:
> Hi,
>
> Last month, I fixed the problem "route-to least-state" in an anchor
> didn't work.
>
> https://marc.info/?t=159117457800002&r=1&w=2
>
> I noticed the same problem happens on "random" and "srchash" as well.
>
> ok?
change looks good. I have just one nit-pick comment. I leave decision
whether it's worth to adjust your diff or commit as-is up to you.
see in-line further below.
OK sashan@
>
> Use the table on root always if current table is not active.
>
> Index: sys/net/pf_lb.c
> ===================================================================
> RCS file: /disk/cvs/openbsd/src/sys/net/pf_lb.c,v
> retrieving revision 1.64
> diff -u -p -r1.64 pf_lb.c
> --- sys/net/pf_lb.c 2 Jul 2019 09:04:53 -0000 1.64
> +++ sys/net/pf_lb.c 23 Jul 2020 10:45:48 -0000
> @@ -345,6 +345,7 @@ pf_map_addr(sa_family_t af, struct pf_ru
> struct pf_addr faddr;
> struct pf_addr *raddr = &rpool->addr.v.a.addr;
> struct pf_addr *rmask = &rpool->addr.v.a.mask;
> + struct pfr_ktable *kt;
> u_int64_t states;
> u_int16_t weight;
> u_int64_t load;
> @@ -396,18 +397,17 @@ pf_map_addr(sa_family_t af, struct pf_ru
> pf_poolmask(naddr, raddr, rmask, saddr, af);
> break;
> case PF_POOL_RANDOM:
> - if (rpool->addr.type == PF_ADDR_TABLE) {
> - cnt = rpool->addr.p.tbl->pfrkt_cnt;
> - if (cnt == 0)
> - rpool->tblidx = 0;
> + if (rpool->addr.type == PF_ADDR_TABLE ||
> + rpool->addr.type == PF_ADDR_DYNIFTL) {
> + if (rpool->addr.type == PF_ADDR_TABLE)
> + kt = rpool->addr.p.tbl;
> else
> - rpool->tblidx = (int)arc4random_uniform(cnt);
> - memset(&rpool->counter, 0, sizeof(rpool->counter));
> - if (pfr_pool_get(rpool, &raddr, &rmask, af))
> + kt = rpool->addr.p.dyn->pfid_kt;
> + kt = pfr_ktable_select_active(kt);
> + if (!kt)
^^^^^^^^
I would prefer to use 'kt == NULL', so it becomes
clear we deal with pointer.
</snip>
> @@ -453,18 +453,18 @@ pf_map_addr(sa_family_t af, struct pf_ru
> case PF_POOL_SRCHASH:
> hashidx =
> pf_hash(saddr, (struct pf_addr *)&hash, &rpool->key, af);
> - if (rpool->addr.type == PF_ADDR_TABLE) {
> - cnt = rpool->addr.p.tbl->pfrkt_cnt;
> - if (cnt == 0)
> - rpool->tblidx = 0;
> +
> + if (rpool->addr.type == PF_ADDR_TABLE ||
> + rpool->addr.type == PF_ADDR_DYNIFTL) {
> + if (rpool->addr.type == PF_ADDR_TABLE)
> + kt = rpool->addr.p.tbl;
> else
> - rpool->tblidx = (int)(hashidx % cnt);
> - memset(&rpool->counter, 0, sizeof(rpool->counter));
> - if (pfr_pool_get(rpool, &raddr, &rmask, af))
> + kt = rpool->addr.p.dyn->pfid_kt;
> + kt = pfr_ktable_select_active(kt);
> + if (!kt)
^^^^^^^^
same here.
</snip>
> ===================================================================
> RCS file: /disk/cvs/openbsd/src/sys/net/pf_table.c,v
> retrieving revision 1.133
> diff -u -p -r1.133 pf_table.c
> --- sys/net/pf_table.c 24 Jun 2020 22:03:43 -0000 1.133
> +++ sys/net/pf_table.c 23 Jul 2020 10:45:48 -0000
> @@ -2108,9 +2108,8 @@ pfr_kentry_byaddr(struct pfr_ktable *kt,
> struct sockaddr_in6 tmp6;
> #endif /* INET6 */
>
> - if (!(kt->pfrkt_flags & PFR_TFLAG_ACTIVE) && kt->pfrkt_root != NULL)
> - kt = kt->pfrkt_root;
> - if (!(kt->pfrkt_flags & PFR_TFLAG_ACTIVE))
> + kt = pfr_ktable_select_active(kt);
> + if (!kt)
^^^^^^^
and here too.
</snip>
> @@ -2308,9 +2306,8 @@ pfr_pool_get(struct pf_pool *rpool, stru
> kt = rpool->addr.p.dyn->pfid_kt;
> else
> return (-1);
> - if (!(kt->pfrkt_flags & PFR_TFLAG_ACTIVE) && kt->pfrkt_root != NULL)
> - kt = kt->pfrkt_root;
> - if (!(kt->pfrkt_flags & PFR_TFLAG_ACTIVE))
> + kt = pfr_ktable_select_active(kt);
> + if (!kt)
^^^^^^^^
here as well
I like the introduction of pfr_ktable_select_active(), it reads far better
than current code in tree.
thanks and
regards
sashan
