Re: huge pfsync rewrite
Hello,
The new code looks much better. It's huge leap forward. I don't mind if this
big diff will get committed. Hrvoje did test the whole diff. Trying to split it
to smaller changes might bring in code which is not tested enough. We don't
know how individual pieces work independently.
I've got some questions/comments which are perhaps worth to sort out
before commit.
the diff touches those files:
net/if.c
- no objections/comments
net/ifq.c
- no objections/comments
net/netisr.h
- no objections/comments
net/pf_norm.c
- no objections/comments
net/pf_ioctl.c
- no objections/comments
net/pfvar.h
- no objections/comments
net/pfvar_priv.h
- no objections/comments
netinet/ip_ipsp.h
- no objections/comments
netinet/in_proto.c
- just wondering why we rename/introducing pfsync_input4().
is there a plan to make pfsync(4) to work over IPv6?
net/pf.c
state expiration is overhauled, new code looks better.
once new code will settle down we might need to revisit
'set timeout interval' description in pf.conf(5),
because if I understand the new code right, the state
expiration timer runs every second now. We might also
consider to expose `pf_purge_states_collect` as a new
tuning knob to control state expiration. Anyway this
can be always done as a follow up change.
- function pf_state_export() line 1319 comment XXX
1319 if (READ_ONCE(st->sync_defer) != NULL) /* XXX */
1320 sp->state_flags |= htons(PFSTATE_ACK);
what does it stand for?
- function pf_purge_expired_states(): handling of static
variable `cur` is unsafe w.r.t. pf_state_list. However
the same issue is present in current. I think there is
a diff floating around which makes cur member of
pf_state_list structure.
net/if_pfsync.h
new if_pfsycnc changes codes for sync_state. This makes me wonder
if we should also update PFSYNC_VERSION. Just to avoid some
undesired/undefined behavior if only one firewall node in cluster gets
upgraded.
net/if_pfsync.c
the diff currently uses two slices (PFSYNC_NSLICES). is there a plan to
scale it up? the slice can be simply viewed as a kind of task. IMO the
number of slices can be aligned with number of cpu cores. Or is this
too simplified? I'm just trying to get some hints on how to further
tune performance.
I noticed there are few dances with NET_LOCK() (UNLOCK()/LOCK())
in up() and down() operations. I feel this comes from fact we
use NET_LOCK() to also protect state of pfsync(4) itself. I wounder
if we can make our life easier here with dedicated rw-lock similar
to pfioctl_rw we have in pf(4). The dedicated rw-lock would make
sure there is no other ioctl() operation intervening with us.
this can be certainly left for follow up change.
- I think lines 102-108 and 110 can be removed:
102 #if 0
103 #define DPRINTF(_fmt...) do { \
104 printf("%s[%u]: ", __func__, __LINE__); \
105 printf(_fmt); \
106 printf("\n"); \
107 } while (0)
108 #else
- function pfsync_clone_create(), line 384:
381 pool_init(_deferrals_pool,
382 sizeof(struct pfsync_deferral), 0,
383 IPL_MPFLOOR, 0, "pfdefer", NULL);
384 /* pool_cache_init(_deferrals_pool); */
does it mean pool_cache_init() is not ready/well tested yet?
- function pfsync_clone_create(), comments /* grumble ... */
on lines 401, and 405, what is that? Are those related
to line 440: /* stupid NET_LOCK */ is it about using time out
to deal with NET_LOCK()? like avoid recursion on NET_LOCK()?
- function pfsync_sendout() line 1539, comment /* XXX */
what does it stand for?
- function pfsync_clear_states() is it complete? it seems
to me it has no effect (apart from exercise with reference)
I have not spot anything else.
thanks and
regards
sashan
Re: csh(1), ksh(1), time(1): print durations with millisecond precision
Todd C. Miller wrote: > Other implementations of "time -p" (both builtin and standalone) > only display two digits after the radix point. I'm a little concerned > about breaking scripts that consume out the output of "time -p". I share that concern.
Re: csh(1), ksh(1), time(1): print durations with millisecond precision
Other implementations of "time -p" (both builtin and standalone) only display two digits after the radix point. I'm a little concerned about breaking scripts that consume out the output of "time -p". Changing the precission of the non-portable output format is fine. - todd
posix_spawn(3): explain that handling NULL envp is an extension
The manual already describes how posix_spawn(3) behaves when passing it a NULL envp, but does not make it clear that it is an OpenBSD extension: > If envp is NULL, the environment is passed unchanged from the parent > process. That differs from GNU/Linux, for example, where a NULL envp gives the child an empty environment rather than a copy. Index: posix_spawn.3 === RCS file: /cvs/src/lib/libc/gen/posix_spawn.3,v retrieving revision 1.9 diff -u -p -r1.9 posix_spawn.3 --- posix_spawn.3 17 Oct 2017 22:47:58 - 1.9 +++ posix_spawn.3 25 Jun 2023 21:38:47 - @@ -122,6 +122,10 @@ with exit status 127. .Sh STANDARDS Both functions conform to .St -p1003.1-2001 . +.Pp +The handling of NULL +.Fa envp +is an extension to that standard. .Sh HISTORY These functions were ported from .Fx
Re: csh(1), ksh(1), time(1): print durations with millisecond precision
On Tue, Jun 13, 2023 at 10:59:53PM -0500, Scott Cheloha wrote:
> This patch bumps the precision of durations printed by csh(1), ksh(1),
> and time(1) from centiseconds to milliseconds. The csh(1) and ksh(1)
> builtins "time" and "times" are affected.
>
> My thinking is:
>
> - All practical OpenBSD platforms have a timecounter with at least
> millisecond precision.
>
> - It's not uncommon for people to run a custom HZ=1000 kernel.
> At HZ=1000, the profiled user and system durations offered by
> getrusage(2) are (arguably) millisecond precision.
>
> Yes, I know those numbers are profiled and thus not especially
> trustworthy, but it's no different from the situation on a
> HZ=100 kernel.
>
> - The timing commands offered in other shells like bash and dash
> provide (at least) millisecond precision.
>
> - Centiseconds are a bit odd. They don't align with the tidy
> "thousands" separation typical of metric units.
>
> - The POSIX standard for time(1) and the "times" builtin specifies
> that the durations are formatted as a floating point value of
> seconds, i.e. "%f". This means millisecond precision is okay:
>
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/time.html
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#times
One week bump.
Index: bin/csh/time.c
===
RCS file: /cvs/src/bin/csh/time.c,v
retrieving revision 1.18
diff -u -p -r1.18 time.c
--- bin/csh/time.c 8 Mar 2023 04:43:04 - 1.18
+++ bin/csh/time.c 14 Jun 2023 15:30:01 -
@@ -40,6 +40,7 @@
* C Shell - routines handling process timing and niceing
*/
static voidpdeltat(struct timeval *, struct timeval *);
+static voidpdelta_hms(const struct timespec *, const struct timespec *);
void
settimes(void)
@@ -145,7 +146,7 @@ prusage(struct rusage *r0, struct rusage
break;
case 'E': /* elapsed (wall-clock) time */
- pcsecs((long) ms);
+ pdelta_hms(e, b);
break;
case 'P': /* percent time spent running */
@@ -227,8 +228,7 @@ pdeltat(struct timeval *t1, struct timev
struct timeval td;
timersub(t1, t0, );
-(void) fprintf(cshout, "%lld.%01ld", (long long)td.tv_sec,
- td.tv_usec / 10);
+fprintf(cshout, "%lld.%03ld", (long long)td.tv_sec, td.tv_usec / 1000);
}
#define P2DIG(i) (void) fprintf(cshout, "%d%d", (i) / 10, (i) % 10)
@@ -254,23 +254,18 @@ minsec:
}
void
-pcsecs(long l) /* PWP: print mm:ss.dd, l is in sec*100 */
+pdelta_hms(const struct timespec *t1, const struct timespec *t0)
{
-int i;
+struct timespec elapsed;
+long long hours, minutes, seconds;
-i = l / 36;
-if (i) {
- (void) fprintf(cshout, "%d:", i);
- i = (l % 36) / 100;
- P2DIG(i / 60);
- goto minsec;
-}
-i = l / 100;
-(void) fprintf(cshout, "%d", i / 60);
-minsec:
-i %= 60;
-(void) fputc(':', cshout);
-P2DIG(i);
-(void) fputc('.', cshout);
-P2DIG((int) (l % 100));
+timespecsub(t1, t0, );
+hours = elapsed.tv_sec / 3600;
+minutes = (elapsed.tv_sec % 3600) / 60;
+seconds = elapsed.tv_sec % 60;
+if (hours != 0)
+ fprintf(cshout, "%lld:%02lld:", hours, minutes);
+else
+ fprintf(cshout, "%lld:", minutes);
+fprintf(cshout, "%02lld.%03ld", seconds, elapsed.tv_nsec / 100);
}
Index: bin/ksh/c_sh.c
===
RCS file: /cvs/src/bin/ksh/c_sh.c,v
retrieving revision 1.64
diff -u -p -r1.64 c_sh.c
--- bin/ksh/c_sh.c 22 May 2020 07:50:07 - 1.64
+++ bin/ksh/c_sh.c 14 Jun 2023 15:30:01 -
@@ -681,13 +681,13 @@ p_tv(struct shf *shf, int posix, struct
char *suffix)
{
if (posix)
- shf_fprintf(shf, "%s%*lld.%02ld%s", prefix ? prefix : "",
- width, (long long)tv->tv_sec, tv->tv_usec / 1, suffix);
+ shf_fprintf(shf, "%s%*lld.%03ld%s", prefix ? prefix : "",
+ width, (long long)tv->tv_sec, tv->tv_usec / 1000, suffix);
else
- shf_fprintf(shf, "%s%*lldm%02lld.%02lds%s", prefix ? prefix :
"",
+ shf_fprintf(shf, "%s%*lldm%02lld.%03lds%s", prefix ? prefix :
"",
width, (long long)tv->tv_sec / 60,
(long long)tv->tv_sec % 60,
- tv->tv_usec / 1, suffix);
+ tv->tv_usec / 1000, suffix);
}
static void
@@ -695,14 +695,14 @@ p_ts(struct shf *shf, int posix, struct
char *suffix)
{
if (posix)
- shf_fprintf(shf, "%s%*lld.%02ld%s", prefix ? prefix : "",
- width, (long long)ts->tv_sec, ts->tv_nsec / 1000,
+ shf_fprintf(shf, "%s%*lld.%03ld%s", prefix ? prefix : "",
+ width, (long long)ts->tv_sec, ts->tv_nsec / 100,
ports: brz Distributed version control system based on bzr
bzr got deleted from openbsd ports about 2 years ago (died along with Python2). https://cvsweb.openbsd.org/ports/devel/bzr/Attic/Makefile?only_with_tag=HEAD bzr was superseded by brz but brz never made it into the openbsd ports. Who can I nudge to include this port? freebsd port -- https://www.freshports.org/devel/brz/ -alfred
