Re: tcp sync cache signed use counter

2023-09-04 Thread Vitaliy Makkoveev
> On 4 Sep 2023, at 23:43, Christian Weisgerber  wrote:
> 
> Alexander Bluhm:
> 
>> Variable scs_use is basically counting packet insertions to syn
>> cache, so I would prefer type long to exclude overflow on fast
>> machines.  With the current limits int should be enough, but long
>> does not hurt.
> 
> But long is the same size as int.  On our 32-bit archs.  Or are
> those inherently so slow that this doesn't matter for them?
> 

Guess this relies to packets count. 32-bit machines are much
limited, so a 32-bit counter is sufficient, while 64-bit machines
can store many more packets.



Re: tcp sync cache signed use counter

2023-09-04 Thread Christian Weisgerber
Alexander Bluhm:

> Variable scs_use is basically counting packet insertions to syn
> cache, so I would prefer type long to exclude overflow on fast
> machines.  With the current limits int should be enough, but long
> does not hurt.

But long is the same size as int.  On our 32-bit archs.  Or are
those inherently so slow that this doesn't matter for them?

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: tcp sync cache signed use counter

2023-09-04 Thread Vitaliy Makkoveev
> On 4 Sep 2023, at 19:52, Alexander Bluhm  wrote:
> 
> On Mon, Sep 04, 2023 at 07:22:03PM +0300, Vitaliy Makkoveev wrote:
>>> On 4 Sep 2023, at 16:19, Alexander Bluhm  wrote:
>>> 
>>> Hi,
>>> 
>>> Variable scs_use is basically counting packet insertions to syn
>>> cache, so I would prefer type long to exclude overflow on fast
>>> machines.  With the current limits int should be enough, but long
>>> does not hurt.
>>> 
>>> It can be negative as it starts at a positive limit and counts
>>> backwards.  After all entries in the current syn cache have been
>>> timed out, it is reset to positive limit.  If timeout takes a while,
>>> it may get well below zero.
>>> 
>>> To prevent netstat output like this
>>>   18446744073709531826 uses of current SYN cache left
>>> make tcps_sc_uses_left signed and print it as long long integer.
>>> 
>>> ok?
>>> 
>> 
>> Does the negative value output makes sense? If not, could you
>> prevent negative value output too?
> 
> Goal is to reseed the hash algorithm from time to time.  This cannot
> be done while there are still entries in the hash table.
> 
> So we have two syn caches, an active and an inactive one.
> 
> They switch roles when the active syn cache has been used for a
> while and the inactive one is empty.  The scs_use counts the packets
> that have been inserted in the active one.
> 
> We start at a limit that has been set by sysctl.  We count down to
> 0, then use the inactive one.  But it may be busy, so we wait until
> all entries have timed out.  While that the active one is still
> used, and the counter gets negative.
> 
> Negative numbers express how long the active syn cache has been
> used above the limit.
> 
> Of course I could move the range to have positive numbers only.
> But then the state "used above limit" would not be obvious anymore.
> 

Thanks for explanation. No objections from me.

> 
>>> Index: sys/netinet/tcp_var.h
>>> ===
>>> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_var.h,v
>>> retrieving revision 1.170
>>> diff -u -p -r1.170 tcp_var.h
>>> --- sys/netinet/tcp_var.h   28 Aug 2023 14:50:02 -  1.170
>>> +++ sys/netinet/tcp_var.h   4 Sep 2023 13:02:40 -
>>> @@ -288,9 +288,9 @@ struct syn_cache_head {
>>> 
>>> struct syn_cache_set {
>>> struct  syn_cache_head *scs_buckethead;
>>> +   longscs_use;
>>> int scs_size;
>>> int scs_count;
>>> -   int scs_use;
>>> u_int32_t   scs_random[5];
>>> };
>>> 
>>> @@ -430,7 +430,7 @@ struct  tcpstat {
>>> u_int64_t tcps_sc_entry_limit;  /* limit of syn cache entries */
>>> u_int64_t tcps_sc_bucket_maxlen;/* maximum # of entries in any bucket */
>>> u_int64_t tcps_sc_bucket_limit; /* limit of syn cache bucket list */
>>> -   u_int64_t tcps_sc_uses_left;/* use counter of current syn cache */
>>> +   int64_t tcps_sc_uses_left;  /* use counter of current syn cache */
>>> 
>>> u_int64_t tcps_conndrained; /* # of connections drained */
>>> 
>>> Index: usr.bin/netstat/inet.c
>>> ===
>>> RCS file: /data/mirror/openbsd/cvs/src/usr.bin/netstat/inet.c,v
>>> retrieving revision 1.178
>>> diff -u -p -r1.178 inet.c
>>> --- usr.bin/netstat/inet.c  7 Jul 2023 09:15:13 -   1.178
>>> +++ usr.bin/netstat/inet.c  4 Sep 2023 12:51:01 -
>>> @@ -498,7 +498,7 @@ tcp_stats(char *name)
>>> "\t%llu entr%s in current SYN cache, limit is %llu\n");
>>> p2b(tcps_sc_bucket_maxlen, tcps_sc_bucket_limit,
>>> "\t%llu longest bucket length in current SYN cache, limit is 
>>> %llu\n");
>>> -   p(tcps_sc_uses_left, "\t%llu use%s of current SYN cache left\n");
>>> +   p(tcps_sc_uses_left, "\t%lld use%s of current SYN cache left\n");
>>> 
>>> p(tcps_sack_recovery_episode, "\t%llu SACK recovery episode%s\n");
>>> p(tcps_sack_rexmits,



Re: tcp sync cache signed use counter

2023-09-04 Thread Alexander Bluhm
On Mon, Sep 04, 2023 at 07:22:03PM +0300, Vitaliy Makkoveev wrote:
> > On 4 Sep 2023, at 16:19, Alexander Bluhm  wrote:
> > 
> > Hi,
> > 
> > Variable scs_use is basically counting packet insertions to syn
> > cache, so I would prefer type long to exclude overflow on fast
> > machines.  With the current limits int should be enough, but long
> > does not hurt.
> > 
> > It can be negative as it starts at a positive limit and counts
> > backwards.  After all entries in the current syn cache have been
> > timed out, it is reset to positive limit.  If timeout takes a while,
> > it may get well below zero.
> > 
> > To prevent netstat output like this
> >18446744073709531826 uses of current SYN cache left
> > make tcps_sc_uses_left signed and print it as long long integer.
> > 
> > ok?
> > 
> 
> Does the negative value output makes sense? If not, could you
> prevent negative value output too?

Goal is to reseed the hash algorithm from time to time.  This cannot
be done while there are still entries in the hash table.

So we have two syn caches, an active and an inactive one.

They switch roles when the active syn cache has been used for a
while and the inactive one is empty.  The scs_use counts the packets
that have been inserted in the active one.

We start at a limit that has been set by sysctl.  We count down to
0, then use the inactive one.  But it may be busy, so we wait until
all entries have timed out.  While that the active one is still
used, and the counter gets negative.

Negative numbers express how long the active syn cache has been
used above the limit.

Of course I could move the range to have positive numbers only.
But then the state "used above limit" would not be obvious anymore.

bluhm

> > Index: sys/netinet/tcp_var.h
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_var.h,v
> > retrieving revision 1.170
> > diff -u -p -r1.170 tcp_var.h
> > --- sys/netinet/tcp_var.h   28 Aug 2023 14:50:02 -  1.170
> > +++ sys/netinet/tcp_var.h   4 Sep 2023 13:02:40 -
> > @@ -288,9 +288,9 @@ struct syn_cache_head {
> > 
> > struct syn_cache_set {
> > struct  syn_cache_head *scs_buckethead;
> > +   longscs_use;
> > int scs_size;
> > int scs_count;
> > -   int scs_use;
> > u_int32_t   scs_random[5];
> > };
> > 
> > @@ -430,7 +430,7 @@ struct  tcpstat {
> > u_int64_t tcps_sc_entry_limit;  /* limit of syn cache entries */
> > u_int64_t tcps_sc_bucket_maxlen;/* maximum # of entries in any bucket */
> > u_int64_t tcps_sc_bucket_limit; /* limit of syn cache bucket list */
> > -   u_int64_t tcps_sc_uses_left;/* use counter of current syn cache */
> > +   int64_t tcps_sc_uses_left;  /* use counter of current syn cache */
> > 
> > u_int64_t tcps_conndrained; /* # of connections drained */
> > 
> > Index: usr.bin/netstat/inet.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/usr.bin/netstat/inet.c,v
> > retrieving revision 1.178
> > diff -u -p -r1.178 inet.c
> > --- usr.bin/netstat/inet.c  7 Jul 2023 09:15:13 -   1.178
> > +++ usr.bin/netstat/inet.c  4 Sep 2023 12:51:01 -
> > @@ -498,7 +498,7 @@ tcp_stats(char *name)
> > "\t%llu entr%s in current SYN cache, limit is %llu\n");
> > p2b(tcps_sc_bucket_maxlen, tcps_sc_bucket_limit,
> > "\t%llu longest bucket length in current SYN cache, limit is 
> > %llu\n");
> > -   p(tcps_sc_uses_left, "\t%llu use%s of current SYN cache left\n");
> > +   p(tcps_sc_uses_left, "\t%lld use%s of current SYN cache left\n");
> > 
> > p(tcps_sack_recovery_episode, "\t%llu SACK recovery episode%s\n");
> > p(tcps_sack_rexmits,
> > 



Re: tcp sync cache signed use counter

2023-09-04 Thread Vitaliy Makkoveev
> On 4 Sep 2023, at 16:19, Alexander Bluhm  wrote:
> 
> Hi,
> 
> Variable scs_use is basically counting packet insertions to syn
> cache, so I would prefer type long to exclude overflow on fast
> machines.  With the current limits int should be enough, but long
> does not hurt.
> 
> It can be negative as it starts at a positive limit and counts
> backwards.  After all entries in the current syn cache have been
> timed out, it is reset to positive limit.  If timeout takes a while,
> it may get well below zero.
> 
> To prevent netstat output like this
>18446744073709531826 uses of current SYN cache left
> make tcps_sc_uses_left signed and print it as long long integer.
> 
> ok?
> 

Does the negative value output makes sense? If not, could you
prevent negative value output too?

> bluhm
> 
> Index: sys/netinet/tcp_var.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_var.h,v
> retrieving revision 1.170
> diff -u -p -r1.170 tcp_var.h
> --- sys/netinet/tcp_var.h 28 Aug 2023 14:50:02 -  1.170
> +++ sys/netinet/tcp_var.h 4 Sep 2023 13:02:40 -
> @@ -288,9 +288,9 @@ struct syn_cache_head {
> 
> struct syn_cache_set {
>   struct  syn_cache_head *scs_buckethead;
> + longscs_use;
>   int scs_size;
>   int scs_count;
> - int scs_use;
>   u_int32_t   scs_random[5];
> };
> 
> @@ -430,7 +430,7 @@ structtcpstat {
>   u_int64_t tcps_sc_entry_limit;  /* limit of syn cache entries */
>   u_int64_t tcps_sc_bucket_maxlen;/* maximum # of entries in any bucket */
>   u_int64_t tcps_sc_bucket_limit; /* limit of syn cache bucket list */
> - u_int64_t tcps_sc_uses_left;/* use counter of current syn cache */
> + int64_t tcps_sc_uses_left;  /* use counter of current syn cache */
> 
>   u_int64_t tcps_conndrained; /* # of connections drained */
> 
> Index: usr.bin/netstat/inet.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.bin/netstat/inet.c,v
> retrieving revision 1.178
> diff -u -p -r1.178 inet.c
> --- usr.bin/netstat/inet.c7 Jul 2023 09:15:13 -   1.178
> +++ usr.bin/netstat/inet.c4 Sep 2023 12:51:01 -
> @@ -498,7 +498,7 @@ tcp_stats(char *name)
>   "\t%llu entr%s in current SYN cache, limit is %llu\n");
>   p2b(tcps_sc_bucket_maxlen, tcps_sc_bucket_limit,
>   "\t%llu longest bucket length in current SYN cache, limit is 
> %llu\n");
> - p(tcps_sc_uses_left, "\t%llu use%s of current SYN cache left\n");
> + p(tcps_sc_uses_left, "\t%lld use%s of current SYN cache left\n");
> 
>   p(tcps_sack_recovery_episode, "\t%llu SACK recovery episode%s\n");
>   p(tcps_sack_rexmits,
>