pf: route-to least-states

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

pf: route-to least-states

YASUOKA Masahiko-3
Hi,

The diff fixes 2 problems of "least-states":

- states whose address is selected by sticky-address is not counted
  for the number of states.
- interface is not selected properly if selected table entry specifies
  an interface.

ok?

Increase state counter for least-states when the address is selected
by sticky-address.  Also fix the problem that the interface which is
specified by the selected table entry is not used properly.

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 11:06:05 -0000
@@ -97,6 +97,8 @@ u_int64_t pf_hash(struct pf_addr *, st
 int pf_get_sport(struct pf_pdesc *, struct pf_rule *,
     struct pf_addr *, u_int16_t *, u_int16_t,
     u_int16_t, struct pf_src_node **);
+int pf_map_addr_states_increase(sa_family_t,
+ struct pf_pool *, struct pf_addr *);
 int pf_get_transaddr_af(struct pf_rule *,
     struct pf_pdesc *, struct pf_src_node **);
 int pf_map_addr_sticky(sa_family_t, struct pf_rule *,
@@ -319,6 +321,12 @@ pf_map_addr_sticky(sa_family_t af, struc
  sns[type] = NULL;
  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))
  pf_addrcpy(naddr, cached, af);
  if (pf_status.debug >= LOG_DEBUG) {
@@ -345,6 +353,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 pfi_kif *kif;
  u_int64_t states;
  u_int16_t weight;
  u_int64_t load;
@@ -539,6 +548,7 @@ pf_map_addr(sa_family_t af, struct pf_ru
 
  states = rpool->states;
  weight = rpool->weight;
+ kif = rpool->kif;
 
  if ((rpool->addr.type == PF_ADDR_TABLE &&
     rpool->addr.p.tbl->pfrkt_refcntcost > 0) ||
@@ -581,6 +591,7 @@ pf_map_addr(sa_family_t af, struct pf_ru
  if (cload < load) {
  states = rpool->states;
  weight = rpool->weight;
+ kif = rpool->kif;
  load = cload;
 
  pf_addrcpy(naddr, &rpool->counter, af);
@@ -591,29 +602,10 @@ pf_map_addr(sa_family_t af, struct pf_ru
  } while (pf_match_addr(1, &faddr, rmask, &rpool->counter, af) &&
     (states > 0));
 
- if (rpool->addr.type == PF_ADDR_TABLE) {
- if (pfr_states_increase(rpool->addr.p.tbl,
-    naddr, af) == -1) {
- if (pf_status.debug >= LOG_DEBUG) {
- log(LOG_DEBUG,"pf: pf_map_addr: "
-    "selected address ");
- pf_print_host(naddr, 0, af);
- addlog(". Failed to increase count!\n");
- }
- return (1);
- }
- } else if (rpool->addr.type == PF_ADDR_DYNIFTL) {
- if (pfr_states_increase(rpool->addr.p.dyn->pfid_kt,
-    naddr, af) == -1) {
- if (pf_status.debug >= LOG_DEBUG) {
- log(LOG_DEBUG, "pf: pf_map_addr: "
-    "selected address ");
- pf_print_host(naddr, 0, af);
- addlog(". Failed to increase count!\n");
- }
- return (1);
- }
- }
+ if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
+ return (1);
+ /* revert the kif which was set by pfr_pool_get() */
+ rpool->kif = kif;
  break;
  }
 
@@ -642,6 +634,38 @@ pf_map_addr(sa_family_t af, struct pf_ru
  addlog("\n");
  }
 
+ return (0);
+}
+
+int
+pf_map_addr_states_increase(sa_family_t af, struct pf_pool *rpool,
+    struct pf_addr *naddr)
+{
+ if (rpool->addr.type == PF_ADDR_TABLE) {
+ if (pfr_states_increase(rpool->addr.p.tbl,
+    naddr, af) == -1) {
+ if (pf_status.debug >= LOG_DEBUG) {
+ log(LOG_DEBUG,
+    "pf: pf_map_addr_states_increase: "
+    "selected address ");
+ pf_print_host(naddr, 0, af);
+ addlog(". Failed to increase count!\n");
+ }
+ return (1);
+ }
+ } else if (rpool->addr.type == PF_ADDR_DYNIFTL) {
+ if (pfr_states_increase(rpool->addr.p.dyn->pfid_kt,
+    naddr, af) == -1) {
+ if (pf_status.debug >= LOG_DEBUG) {
+ log(LOG_DEBUG,
+    "pf: pf_map_addr_states_increase: "
+    "selected address ");
+ pf_print_host(naddr, 0, af);
+ addlog(". Failed to increase count!\n");
+ }
+ return (1);
+ }
+ }
  return (0);
 }
 

Reply | Threaded
Open this post in threaded view
|

Re: pf: route-to least-states

Joerg Jung

> On 23. Jul 2020, at 13:23, YASUOKA Masahiko <[hidden email]> wrote:
>
> The diff fixes 2 problems of "least-states":
>
> - states whose address is selected by sticky-address is not counted
>  for the number of states.
> - interface is not selected properly if selected table entry specifies
>  an interface.
>
> ok?

Good catch, diff looks good to me.

ok jung@


> Increase state counter for least-states when the address is selected
> by sticky-address.  Also fix the problem that the interface which is
> specified by the selected table entry is not used properly.
>
> 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 11:06:05 -0000
> @@ -97,6 +97,8 @@ u_int64_t pf_hash(struct pf_addr *, st
> int pf_get_sport(struct pf_pdesc *, struct pf_rule *,
>    struct pf_addr *, u_int16_t *, u_int16_t,
>    u_int16_t, struct pf_src_node **);
> +int pf_map_addr_states_increase(sa_family_t,
> + struct pf_pool *, struct pf_addr *);
> int pf_get_transaddr_af(struct pf_rule *,
>    struct pf_pdesc *, struct pf_src_node **);
> int pf_map_addr_sticky(sa_family_t, struct pf_rule *,
> @@ -319,6 +321,12 @@ pf_map_addr_sticky(sa_family_t af, struc
> sns[type] = NULL;
> 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))
> pf_addrcpy(naddr, cached, af);
> if (pf_status.debug >= LOG_DEBUG) {
> @@ -345,6 +353,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 pfi_kif *kif;
> u_int64_t states;
> u_int16_t weight;
> u_int64_t load;
> @@ -539,6 +548,7 @@ pf_map_addr(sa_family_t af, struct pf_ru
>
> states = rpool->states;
> weight = rpool->weight;
> + kif = rpool->kif;
>
> if ((rpool->addr.type == PF_ADDR_TABLE &&
>    rpool->addr.p.tbl->pfrkt_refcntcost > 0) ||
> @@ -581,6 +591,7 @@ pf_map_addr(sa_family_t af, struct pf_ru
> if (cload < load) {
> states = rpool->states;
> weight = rpool->weight;
> + kif = rpool->kif;
> load = cload;
>
> pf_addrcpy(naddr, &rpool->counter, af);
> @@ -591,29 +602,10 @@ pf_map_addr(sa_family_t af, struct pf_ru
> } while (pf_match_addr(1, &faddr, rmask, &rpool->counter, af) &&
>    (states > 0));
>
> - if (rpool->addr.type == PF_ADDR_TABLE) {
> - if (pfr_states_increase(rpool->addr.p.tbl,
> -    naddr, af) == -1) {
> - if (pf_status.debug >= LOG_DEBUG) {
> - log(LOG_DEBUG,"pf: pf_map_addr: "
> -    "selected address ");
> - pf_print_host(naddr, 0, af);
> - addlog(". Failed to increase count!\n");
> - }
> - return (1);
> - }
> - } else if (rpool->addr.type == PF_ADDR_DYNIFTL) {
> - if (pfr_states_increase(rpool->addr.p.dyn->pfid_kt,
> -    naddr, af) == -1) {
> - if (pf_status.debug >= LOG_DEBUG) {
> - log(LOG_DEBUG, "pf: pf_map_addr: "
> -    "selected address ");
> - pf_print_host(naddr, 0, af);
> - addlog(". Failed to increase count!\n");
> - }
> - return (1);
> - }
> - }
> + if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
> + return (1);
> + /* revert the kif which was set by pfr_pool_get() */
> + rpool->kif = kif;
> break;
> }
>
> @@ -642,6 +634,38 @@ pf_map_addr(sa_family_t af, struct pf_ru
> addlog("\n");
> }
>
> + return (0);
> +}
> +
> +int
> +pf_map_addr_states_increase(sa_family_t af, struct pf_pool *rpool,
> +    struct pf_addr *naddr)
> +{
> + if (rpool->addr.type == PF_ADDR_TABLE) {
> + if (pfr_states_increase(rpool->addr.p.tbl,
> +    naddr, af) == -1) {
> + if (pf_status.debug >= LOG_DEBUG) {
> + log(LOG_DEBUG,
> +    "pf: pf_map_addr_states_increase: "
> +    "selected address ");
> + pf_print_host(naddr, 0, af);
> + addlog(". Failed to increase count!\n");
> + }
> + return (1);
> + }
> + } else if (rpool->addr.type == PF_ADDR_DYNIFTL) {
> + if (pfr_states_increase(rpool->addr.p.dyn->pfid_kt,
> +    naddr, af) == -1) {
> + if (pf_status.debug >= LOG_DEBUG) {
> + log(LOG_DEBUG,
> +    "pf: pf_map_addr_states_increase: "
> +    "selected address ");
> + pf_print_host(naddr, 0, af);
> + addlog(". Failed to increase count!\n");
> + }
> + return (1);
> + }
> + }
> return (0);
> }
>
>

Reply | Threaded
Open this post in threaded view
|

Re: pf: route-to least-states

Alexandr Nedvedicky
In reply to this post by YASUOKA Masahiko-3
Hello Yasuoka,

</snip>

> - interface is not selected properly if selected table entry specifies
>   an interface.

    to be honest I don't quite understand what's going on here.
    can you share some details of configuration/scenario, which
    triggers the bug your diff is fixing?


    the part of your change, which I'm not able to figure out is
    this single line:

> + if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
> + return (1);
> + /* revert the kif which was set by pfr_pool_get() */
> + rpool->kif = kif;
>   break;
>   }

    your fix changes behavior, which is there since least-state
    option has been introduced. I believe it does not matter
    for case when route-to specifies single interface such as:

        route-to 192.168.1.10@em0 least-states

    I'm not sure what will happen in situation, when there are more interfaces
    specified in combination with sticky-address:
       
        route-to {192.168.1.10@em0, 192.168.1.20@em1} last-states sticky-address

    the resulting code does not look quite right with your diff applied:

602                 } while (pf_match_addr(1, &faddr, rmask, &rpool->counter, af) &&
603                     (states > 0));
604
605                 if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
606                         return (1);
607                 /* revert the kif which was set by pfr_pool_get() */
608                 rpool->kif = kif;
609                 break;
610         }
611
612         if (rpool->opts & PF_POOL_STICKYADDR) {
613                 if (sns[type] != NULL) {
614                         pf_remove_src_node(sns[type]);
615                         sns[type] = NULL;
616                 }
617                 if (pf_insert_src_node(&sns[type], r, type, af, saddr, naddr,
618                     rpool->kif))
619                         return (1);
620         }


    at line 608 new code reverts kif set by pfr_pool_get(). At line 617
    (executed when sticky-address is set) the original code passes kif chosen be
    pfr_pool_get(). You diff changes that behavior. So my question is simple:
        is that intentional change?

thanks and
regards
sashan

Reply | Threaded
Open this post in threaded view
|

Re: pf: route-to least-states

YASUOKA Masahiko-3
Hi,

Thank you for your review.

On Fri, 24 Jul 2020 01:25:42 +0200
Alexandr Nedvedicky <[hidden email]> wrote:
>> - interface is not selected properly if selected table entry specifies
>>   an interface.
>
>     to be honest I don't quite understand what's going on here.
>     can you share some details of configuration/scenario, which
>     triggers the bug your diff is fixing?

You seem to have understood the scenario correctly.

>     the part of your change, which I'm not able to figure out is
>     this single line:
>
>> + if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
>> + return (1);
>> + /* revert the kif which was set by pfr_pool_get() */
>> + rpool->kif = kif;
>>   break;
>>   }
>
>     your fix changes behavior, which is there since least-state
>     option has been introduced. I believe it does not matter
>     for case when route-to specifies single interface such as:
>
> route-to 192.168.1.10@em0 least-states
>
>     I'm not sure what will happen in situation, when there are more interfaces
>     specified in combination with sticky-address:
>
> route-to {192.168.1.10@em0, 192.168.1.20@em1} last-states sticky-address

Yes.  This is a senario.

>     the resulting code does not look quite right with your diff applied:
>
> 602                 } while (pf_match_addr(1, &faddr, rmask, &rpool->counter, af) &&
> 603                     (states > 0));
> 604
> 605                 if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
> 606                         return (1);
> 607                 /* revert the kif which was set by pfr_pool_get() */
> 608                 rpool->kif = kif;
> 609                 break;
> 610         }
> 611
> 612         if (rpool->opts & PF_POOL_STICKYADDR) {
> 613                 if (sns[type] != NULL) {
> 614                         pf_remove_src_node(sns[type]);
> 615                         sns[type] = NULL;
> 616                 }
> 617                 if (pf_insert_src_node(&sns[type], r, type, af, saddr, naddr,
> 618                     rpool->kif))
> 619                         return (1);
> 620         }
>
>
>     at line 608 new code reverts kif set by pfr_pool_get(). At line 617
>     (executed when sticky-address is set) the original code passes kif chosen be
>     pfr_pool_get(). You diff changes that behavior. So my question is simple:
> is that intentional change?

Yes.

Let me simplify the block for "least-states".

535       case PF_POOL_LEASTSTATES:
539           pfr_pool_get(rpool);      // fist entry
 :
561           faddr = rpool->counter;   //record as final
 :
557           load = rpool->states / rpool->weight;
563           naddr = rpool->counter;
 :
571          do {
572              rpool->counter++;
575              pfr_pool_get(rpool);   /* next entry */
 :
585              cload = rpool->states / rpool->weight;
 :
 :               /* find lc minimum */
591              if (cload < load) {
595                 load = cload;
597                 naddr = rpool->counter;
601              }
603           } while (raddr->counter != faddr); // loop until final

the loop #571:606 is to find the minimum (least-states) entry.  If the
last entry is not the minimum, after the loop,

   rpool <= the last entry
   naddr <= the minimum entry

Also see the pfr_pool_get():

2272 int
2273 pfr_pool_get(struct pf_pool *rpool, struct pf_addr **raddr,
2274     struct pf_addr **rmask, sa_family_t af)
2275 {
(snip)
2417                         rpool->states = 0;
2418                         if (ke->pfrke_counters != NULL)
2419                                 rpool->states = ke->pfrke_counters->states;
2420                         switch (ke->pfrke_type) {
2421                         case PFRKE_COST:
2422                                 rpool->weight =
2423                                     ((struct pfr_kentry_cost *)ke)->weight;
2424                                 /* FALLTHROUGH */
2425                         case PFRKE_ROUTE:
2426                                 rpool->kif = ((struct pfr_kentry_route *)ke)->kif;
2427                                 break;
2428                         default:
2429                                 rpool->weight = 1;
2430                                 break;
2431                         }

some fields of rpool (states, weight, kif) are set by the values of
the selected table entry.

So,

|  rpool <= the last entry
|  naddr <= the minimum entry

rpool->kif is the interface of the last entery.  It might be different
than the inteface of the minimum entry.

The diff is to keep kif of the minimum entry,

+               kif = rpool->kif;

revert rpool->kif by it after the loop.

+               /* revert the kif which was set by pfr_pool_get() */
+               rpool->kif = kif;


Reply | Threaded
Open this post in threaded view
|

Re: pf: route-to least-states

Alexandr Nedvedicky
Hello Yasuoka,

>
> Yes.
>
> Let me simplify the block for "least-states".
>
</snip>

    thanks for your explanation. It helped me to understand the code.

    I'm OK with your fix.

thanks and
regards
sashan

Reply | Threaded
Open this post in threaded view
|

Re: pf: route-to least-states

YASUOKA Masahiko-3
Hi,

Previous commit has a wrong part..

ok?

Fix previous commit which referred wrong address.

Index: sys/net/pf_lb.c
===================================================================
RCS file: /cvs/src/sys/net/pf_lb.c,v
retrieving revision 1.65
diff -u -p -r1.65 pf_lb.c
--- sys/net/pf_lb.c 24 Jul 2020 14:06:33 -0000 1.65
+++ sys/net/pf_lb.c 28 Jul 2020 16:15:50 -0000
@@ -323,7 +323,7 @@ pf_map_addr_sticky(sa_family_t af, struc
  }
 
  if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES) {
- if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
+ if (pf_map_addr_states_increase(af, rpool, cached) == -1)
  return (-1);
  }
 

Reply | Threaded
Open this post in threaded view
|

Re: pf: route-to least-states

Alexandr Nedvedicky
Hello Yasuoka,

On Wed, Jul 29, 2020 at 01:22:48AM +0900, YASUOKA Masahiko wrote:
> Hi,
>
> 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.

thanks and
regards
sashan

Reply | Threaded
Open this post in threaded view
|

Re: pf: route-to least-states

YASUOKA Masahiko-3
In reply to this post by YASUOKA Masahiko-3
Hi,

Let me add another fix of previous.

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 16:52:24 -0000
@@ -323,7 +323,7 @@ pf_map_addr_sticky(sa_family_t af, struc
  }
 
  if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES) {
- if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
+ if (pf_map_addr_states_increase(af, rpool, cached) == -1)
  return (-1);
  }
 
@@ -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);

Reply | Threaded
Open this post in threaded view
|

Re: pf: route-to least-states

YASUOKA Masahiko-3
In reply to this post by Alexandr Nedvedicky
Hi,

On Tue, 28 Jul 2020 18:54:48 +0200
Alexandr Nedvedicky <[hidden email]> 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);

Reply | Threaded
Open this post in threaded view
|

Re: pf: route-to least-states

Alexandr Nedvedicky
In reply to this post by YASUOKA Masahiko-3
Hello Yasuoka,


On Wed, Jul 29, 2020 at 01:55:20AM +0900, YASUOKA Masahiko wrote:
> Hi,
>
> Let me add another fix of previous.
>
> ok?

    OK. thanks for taking care of that. I've entirely missed 1 vs. -1 return
    value, when reviewing your change.

regards
sashan