Re: more consistently use "st" as the name for struct pf_state variables

2023-01-05 Thread Alexandr Nedvedicky 
On Thu, Jan 05, 2023 at 08:32:44PM +1000, David Gwynne wrote:
> 
> 
> > On 5 Jan 2023, at 18:56, Alexandr Nedvedicky  wrote:
> > 
> > Hello,
> > 
> > On Wed, Jan 04, 2023 at 09:36:38PM +1000, David Gwynne wrote:
> >> and "stp" for pf_state ** variables.
> >> 
> >I agree with established naming conventions.
> > 
> >I'm also fine with keeping some exceptions such as `a` and `b`
> >in pf_state_compare_id(), local variables `tail`, `head`
> >in pf_states_{clr, get}() and pf_purge_expired_states().
> >I'm also fine with leaving static variable `cur` unchanged.
> > 
> > is there any reason we still keep `pf_state **sm` argument
> > in pf_test_rule()? the same in pf_create_state(). Is it intended?
> 
> there were a bunch of other arguments that ended with m. happy to change it
> to **stp though. we can always do another sweep for other types.
> 

I would change those occurrences to stp. my point is that we don't
expect pf_test_rule() to return matching state.

with those tweaks diff is OK

thanks and
regards
sashan

Re: more consistently use "st" as the name for struct pf_state variables

2023-01-05 Thread David Gwynne 



> On 5 Jan 2023, at 18:56, Alexandr Nedvedicky  wrote:
> 
> Hello,
> 
> On Wed, Jan 04, 2023 at 09:36:38PM +1000, David Gwynne wrote:
>> and "stp" for pf_state ** variables.
>> 
>I agree with established naming conventions.
> 
>I'm also fine with keeping some exceptions such as `a` and `b`
>in pf_state_compare_id(), local variables `tail`, `head`
>in pf_states_{clr, get}() and pf_purge_expired_states().
>I'm also fine with leaving static variable `cur` unchanged.
> 
> is there any reason we still keep `pf_state **sm` argument
> in pf_test_rule()? the same in pf_create_state(). Is it intended?

there were a bunch of other arguments that ended with m. happy to change it to 
**stp though. we can always do another sweep for other types.

> 
> otherwise diff reads OK to me.
> 
> thanks and
> regards
> sashan

Re: more consistently use "st" as the name for struct pf_state variables

2023-01-05 Thread Alexandr Nedvedicky 
Hello,

On Wed, Jan 04, 2023 at 09:36:38PM +1000, David Gwynne wrote:
> and "stp" for pf_state ** variables.
> 
I agree with established naming conventions.

I'm also fine with keeping some exceptions such as `a` and `b`
in pf_state_compare_id(), local variables `tail`, `head`
in pf_states_{clr, get}() and pf_purge_expired_states().
I'm also fine with leaving static variable `cur` unchanged.

is there any reason we still keep `pf_state **sm` argument
in pf_test_rule()? the same in pf_create_state(). Is it intended?

otherwise diff reads OK to me.

thanks and
regards
sashan

more consistently use "st" as the name for struct pf_state variables

2023-01-04 Thread David Gwynne 
and "stp" for pf_state ** variables.

there should be no functional change here.

ok?

Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1167
diff -u -p -r1.1167 pf.c
--- pf.c4 Jan 2023 10:31:55 -   1.1167
+++ pf.c4 Jan 2023 11:34:30 -
@@ -376,23 +376,23 @@ pf_src_compare(struct pf_src_node *a, st
 }
 
 static __inline void
-pf_set_protostate(struct pf_state *s, int which, u_int8_t newstate)
+pf_set_protostate(struct pf_state *st, int which, u_int8_t newstate)
 {
if (which == PF_PEER_DST || which == PF_PEER_BOTH)
-   s->dst.state = newstate;
+   st->dst.state = newstate;
if (which == PF_PEER_DST)
return;
 
-   if (s->src.state == newstate)
+   if (st->src.state == newstate)
return;
-   if (s->creatorid == pf_status.hostid &&
-   s->key[PF_SK_STACK]->proto == IPPROTO_TCP &&
-   !(TCPS_HAVEESTABLISHED(s->src.state) ||
-   s->src.state == TCPS_CLOSED) &&
+   if (st->creatorid == pf_status.hostid &&
+   st->key[PF_SK_STACK]->proto == IPPROTO_TCP &&
+   !(TCPS_HAVEESTABLISHED(st->src.state) ||
+   st->src.state == TCPS_CLOSED) &&
(TCPS_HAVEESTABLISHED(newstate) || newstate == TCPS_CLOSED))
pf_status.states_halfopen--;
 
-   s->src.state = newstate;
+   st->src.state = newstate;
 }
 
 void
@@ -477,25 +477,25 @@ pf_state_list_remove(struct pf_state_lis
 }
 
 int
-pf_src_connlimit(struct pf_state **state)
+pf_src_connlimit(struct pf_state **stp)
 {
int  bad = 0;
struct pf_src_node  *sn;
 
-   if ((sn = pf_get_src_node((*state), PF_SN_NONE)) == NULL)
+   if ((sn = pf_get_src_node((*stp), PF_SN_NONE)) == NULL)
return (0);
 
sn->conn++;
-   (*state)->src.tcp_est = 1;
+   (*stp)->src.tcp_est = 1;
pf_add_threshold(>conn_rate);
 
-   if ((*state)->rule.ptr->max_src_conn &&
-   (*state)->rule.ptr->max_src_conn < sn->conn) {
+   if ((*stp)->rule.ptr->max_src_conn &&
+   (*stp)->rule.ptr->max_src_conn < sn->conn) {
pf_status.lcounters[LCNT_SRCCONN]++;
bad++;
}
 
-   if ((*state)->rule.ptr->max_src_conn_rate.limit &&
+   if ((*stp)->rule.ptr->max_src_conn_rate.limit &&
pf_check_threshold(>conn_rate)) {
pf_status.lcounters[LCNT_SRCCONNRATE]++;
bad++;
@@ -504,7 +504,7 @@ pf_src_connlimit(struct pf_state **state
if (!bad)
return (0);
 
-   if ((*state)->rule.ptr->overload_tbl) {
+   if ((*stp)->rule.ptr->overload_tbl) {
struct pfr_addr p;
u_int32_t   killed = 0;
 
@@ -513,12 +513,12 @@ pf_src_connlimit(struct pf_state **state
log(LOG_NOTICE,
"pf: pf_src_connlimit: blocking address ");
pf_print_host(>addr, 0,
-   (*state)->key[PF_SK_WIRE]->af);
+   (*stp)->key[PF_SK_WIRE]->af);
}
 
memset(, 0, sizeof(p));
-   p.pfra_af = (*state)->key[PF_SK_WIRE]->af;
-   switch ((*state)->key[PF_SK_WIRE]->af) {
+   p.pfra_af = (*stp)->key[PF_SK_WIRE]->af;
+   switch ((*stp)->key[PF_SK_WIRE]->af) {
case AF_INET:
p.pfra_net = 32;
p.pfra_ip4addr = sn->addr.v4;
@@ -531,11 +531,11 @@ pf_src_connlimit(struct pf_state **state
 #endif /* INET6 */
}
 
-   pfr_insert_kentry((*state)->rule.ptr->overload_tbl,
+   pfr_insert_kentry((*stp)->rule.ptr->overload_tbl,
, gettime());
 
/* kill existing states if that's required. */
-   if ((*state)->rule.ptr->flush) {
+   if ((*stp)->rule.ptr->flush) {
struct pf_state_key *sk;
struct pf_state *st;
 
@@ -548,14 +548,14 @@ pf_src_connlimit(struct pf_state **state
 * set)
 */
if (sk->af ==
-   (*state)->key[PF_SK_WIRE]->af &&
-   (((*state)->direction == PF_OUT &&
+   (*stp)->key[PF_SK_WIRE]->af &&
+   (((*stp)->direction == PF_OUT &&
PF_AEQ(>addr, >addr[1], sk->af)) ||
-   ((*state)->direction == PF_IN &&
+   ((*stp)->direction == PF_IN &&
PF_AEQ(>addr, >addr[0], sk->af))) &&
-   ((*state)->rule.ptr->flush &
+   ((*stp)->rule.ptr->flush &