Hi, On Tue, 28 Jul 2020 18:54:48 +0200 Alexandr Nedvedicky <alexandr.nedvedi...@oracle.com> wrote: > On Wed, Jul 29, 2020 at 01:22:48AM +0900, YASUOKA Masahiko wrote: >> Previous commit has a wrong part.. >> >> ok? >> >> Fix previous commit which referred wrong address. > > would it make sense to move the block, you've introduced earler > under the !PF_AZERO() branch just couple lines below. something > like this: > > --------8<---------------8<---------------8<------------------8<-------- > diff --git a/sys/net/pf_lb.c b/sys/net/pf_lb.c > index 510795a4d0b..f77d96a99ec 100644 > --- a/sys/net/pf_lb.c > +++ b/sys/net/pf_lb.c > @@ -322,13 +322,13 @@ pf_map_addr_sticky(sa_family_t af, struct pf_rule *r, > struct pf_addr *saddr, > return (-1); > } > > - if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES) { > - if (pf_map_addr_states_increase(af, rpool, naddr) == -1) > + if (!PF_AZERO(cached, af)) { > + pf_addrcpy(naddr, cached, af); > + if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES) > && > + ((pf_map_addr_states_increase(af, rpool, cached) == -1)) > return (-1); > } > > - if (!PF_AZERO(cached, af)) > - pf_addrcpy(naddr, cached, af); > if (pf_status.debug >= LOG_DEBUG) { > log(LOG_DEBUG, "pf: pf_map_addr: " > "src tracking (%u) maps ", type); > > --------8<---------------8<---------------8<------------------8<-------- > > It seems to me it would be better to bump number of states if and only if we > actually find some address in pool.
Yes, I agree. ok? Fix previous commit which referred wrong address and returned wrong value. Index: sys/net/pf_lb.c =================================================================== RCS file: /cvs/src/sys/net/pf_lb.c,v retrieving revision 1.66 diff -u -p -r1.66 pf_lb.c --- sys/net/pf_lb.c 28 Jul 2020 16:47:41 -0000 1.66 +++ sys/net/pf_lb.c 28 Jul 2020 17:01:34 -0000 @@ -322,13 +322,13 @@ pf_map_addr_sticky(sa_family_t af, struc return (-1); } - if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES) { - if (pf_map_addr_states_increase(af, rpool, naddr) == -1) - return (-1); - } - if (!PF_AZERO(cached, af)) + if (!PF_AZERO(cached, af)) { pf_addrcpy(naddr, cached, af); + if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES && + pf_map_addr_states_increase(af, rpool, cached) == -1) + return (-1); + } if (pf_status.debug >= LOG_DEBUG) { log(LOG_DEBUG, "pf: pf_map_addr: " "src tracking (%u) maps ", type); @@ -651,7 +651,7 @@ pf_map_addr_states_increase(sa_family_t pf_print_host(naddr, 0, af); addlog(". Failed to increase count!\n"); } - return (1); + return (-1); } } else if (rpool->addr.type == PF_ADDR_DYNIFTL) { if (pfr_states_increase(rpool->addr.p.dyn->pfid_kt, @@ -663,7 +663,7 @@ pf_map_addr_states_increase(sa_family_t pf_print_host(naddr, 0, af); addlog(". Failed to increase count!\n"); } - return (1); + return (-1); } } return (0);