rpki-client: decrease how long to wait for the remote peer to send IO

2022-08-09 Thread Job Snijders
Dear all,

I like to run rpki-client very often, and not be bogged down with
non-responsive respositories. If a repository is uncommunicative,
rpki-client as-is will try other transports, or come back later (because
of a next crontab invocation).

In rpki-client, RSYNC & HTTPS timeouts now are unified:

- if the initial connection can't be established within MAX_CONN_TIMEOUT
  seconds, try one of: Another Address Family (such as IPv6), or Another
  Transport Protocol (like RSYNC).

- If the remote peer at the other side of the HTTPS or RSYNC connection
  doens't send us any data for the duration of MAX_IO_TIMEOUT, try one
  of Another Address Family (such as IPv6), or Another Transport
  Protocol (like RSYNC).

I propose that if the remote side fails to send any new data for the
duration of 30 seconds, we should give up, try something else (which
includes locally cached data from the previous invocation).

I've tested with MAX_IO_TIMEOUT set to 15 seconds; and would feel
comfortable committing that; but some in our community expressed
hesitance, hence the below proposal for 30.

OK?

Kind regards,

Job

Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.146
diff -u -p -r1.146 extern.h
--- extern.h9 Aug 2022 09:02:26 -   1.146
+++ extern.h10 Aug 2022 02:01:45 -
@@ -731,7 +731,7 @@ int mkpathat(int, const char *);
 #define MAX_CONN_TIMEOUT   15
 
 /* How long to wait for IO from a remote server. */
-#define MAX_IO_TIMEOUT 180
+#define MAX_IO_TIMEOUT 30
 
 /* Maximum allowd repositories per tal */
 #define MAX_REPO_PER_TAL   1000



Re: alpha: remove misaligned access emulation code

2022-08-09 Thread Theo de Raadt
There have to be some strict-aligned architectures which don't emulate
unaligned access, because there will always be architectures which have
a high emulation cost, and I'm ok with alpha joining that group.


Miod Vallat  wrote:

> The alpha part contains code in the kernel to handle unaligned memory
> accesses from userland programs, to prevent them from dying in horrible
> SIGBUS.
> 
> This made sense in the '90s, but since then people have learned to work
> with strict-alignment architectures, and this code has been less and
> less triggered - in my own experience I don't remember seeing it
> triggered in the last 20 years.
> 
> I think it's time to send it to the Attic for a well-deserved
> retirement, and let these ill-behaved userland programs catch the SIGBUS
> they deserve.
> 
> Index: etc/etc.alpha/sysctl.conf
> ===
> RCS file: /OpenBSD/src/etc/etc.alpha/sysctl.conf,v
> retrieving revision 1.8
> diff -u -p -r1.8 sysctl.conf
> --- etc/etc.alpha/sysctl.conf 2 Mar 2013 22:53:10 -   1.8
> +++ etc/etc.alpha/sysctl.conf 9 Aug 2022 06:16:34 -
> @@ -1,5 +1,2 @@
> -#machdep.unaligned_print=0   # 0 - disable printing of unaligned access
> -#machdep.unaligned_fix=0 # 0 - disable fixup of unaligned access
> -#machdep.unaligned_sigbus=0  # 0 - don't sigbus on unaligned access
>  #machdep.allowaperture=1 # see xf86(4)
>  #machdep.led_blink=1 # blink chassis leds on DEC 3000
> Index: sys/arch/alpha/alpha/machdep.c
> ===
> RCS file: /OpenBSD/src/sys/arch/alpha/alpha/machdep.c,v
> retrieving revision 1.196
> diff -u -p -r1.196 machdep.c
> --- sys/arch/alpha/alpha/machdep.c6 Oct 2021 15:46:03 -   1.196
> +++ sys/arch/alpha/alpha/machdep.c9 Aug 2022 06:16:34 -
> @@ -180,9 +180,6 @@ u_int8_t  dec_3000_scsiid[2], dec_3000_sc
>  struct platform platform;
>  
>  /* for cpu_sysctl() */
> -int  alpha_unaligned_print = 1;  /* warn about unaligned accesses */
> -int  alpha_unaligned_fix = 1;/* fix up unaligned accesses */
> -int  alpha_unaligned_sigbus = 1; /* SIGBUS on fixed-up accesses */
>  #ifndef NO_IEEE
>  int  alpha_fp_sync_complete = 0; /* fp fixup if sync even without /s */
>  #endif
> @@ -1555,18 +1552,6 @@ cpu_sysctl(int *name, u_int namelen, voi
>   sizeof consdev));
>  
>  #ifndef SMALL_KERNEL
> - case CPU_UNALIGNED_PRINT:
> - return (sysctl_int(oldp, oldlenp, newp, newlen,
> - _unaligned_print));
> -
> - case CPU_UNALIGNED_FIX:
> - return (sysctl_int(oldp, oldlenp, newp, newlen,
> - _unaligned_fix));
> -
> - case CPU_UNALIGNED_SIGBUS:
> - return (sysctl_int(oldp, oldlenp, newp, newlen,
> - _unaligned_sigbus));
> -
>   case CPU_BOOTED_KERNEL:
>   return (sysctl_rdstring(oldp, oldlenp, newp,
>   bootinfo.booted_kernel));
> Index: sys/arch/alpha/alpha/trap.c
> ===
> RCS file: /OpenBSD/src/sys/arch/alpha/alpha/trap.c,v
> retrieving revision 1.100
> diff -u -p -r1.100 trap.c
> --- sys/arch/alpha/alpha/trap.c   9 Dec 2021 00:26:11 -   1.100
> +++ sys/arch/alpha/alpha/trap.c   9 Aug 2022 06:16:35 -
> @@ -110,21 +110,6 @@
>  #endif
>  #include 
>  
> -#ifndef SMALL_KERNEL
> -
> -unsigned longSfloat_to_reg(unsigned int);
> -unsigned int reg_to_Sfloat(unsigned long);
> -unsigned longTfloat_reg_cvt(unsigned long);
> -#ifdef FIX_UNALIGNED_VAX_FP
> -unsigned longFfloat_to_reg(unsigned int);
> -unsigned int reg_to_Ffloat(unsigned long);
> -unsigned longGfloat_reg_cvt(unsigned long);
> -#endif
> -
> -int  unaligned_fixup(unsigned long, unsigned long,
> - unsigned long, struct proc *);
> -#endif   /* SMALL_KERNEL */
> -
>  int  handle_opdec(struct proc *p, u_int64_t *ucodep);
>  
>  #ifndef NO_IEEE
> @@ -249,19 +234,10 @@ trap(a0, a1, a2, entry, framep)
>   switch (entry) {
>   case ALPHA_KENTRY_UNA:
>   /*
> -  * If user-land, do whatever fixups, printing, and
> -  * signalling is appropriate (based on system-wide
> -  * and per-process unaligned-access-handling flags).
> +  * If user-land, deliver SIGBUS unconditionally.
>*/
>   if (user) {
> -#ifndef SMALL_KERNEL
> - KERNEL_LOCK();
> - i = unaligned_fixup(a0, a1, a2, p);
> - KERNEL_UNLOCK();
> - if (i == 0)
> - goto out;
> -#endif
> -
> + i = SIGBUS;
>   ucode = ILL_ILLADR;
>   v = (caddr_t)a0;
>   break;
> @@ -716,11 +692,6 @@ ast(framep)
>   userret(p);
>  }
>  
> -/*
> - * Unaligned access handler.  

Re: alpha: remove misaligned access emulation code

2022-08-09 Thread Todd C . Miller
On Tue, 09 Aug 2022 19:39:31 -, Miod Vallat wrote:

> The alpha part contains code in the kernel to handle unaligned memory
> accesses from userland programs, to prevent them from dying in horrible
> SIGBUS.
>
> This made sense in the '90s, but since then people have learned to work
> with strict-alignment architectures, and this code has been less and
> less triggered - in my own experience I don't remember seeing it
> triggered in the last 20 years.
>
> I think it's time to send it to the Attic for a well-deserved
> retirement, and let these ill-behaved userland programs catch the SIGBUS
> they deserve.

Makes sense to me.  There's no reason alpha needs to suport unaligned
memory accesses when we have other strict alignment architectures
that don't do the fixup.

 - todd



Re: alpha: remove misaligned access emulation code

2022-08-09 Thread Jonathan Gray
On Tue, Aug 09, 2022 at 07:39:31PM +, Miod Vallat wrote:
> The alpha part contains code in the kernel to handle unaligned memory
> accesses from userland programs, to prevent them from dying in horrible
> SIGBUS.
> 
> This made sense in the '90s, but since then people have learned to work
> with strict-alignment architectures, and this code has been less and
> less triggered - in my own experience I don't remember seeing it
> triggered in the last 20 years.
> 
> I think it's time to send it to the Attic for a well-deserved
> retirement, and let these ill-behaved userland programs catch the SIGBUS
> they deserve.

I like the direction and am ok with this being committed but don't have
a working alpha to test.

Having the same behaviour on SMALL_KERNEL and GENERIC is an added bonus.



Re: mips64: trigger deferred timer interrupt from splx(9)

2022-08-09 Thread Scott Cheloha
On Tue, Aug 09, 2022 at 06:02:10PM +, Miod Vallat wrote:
> > Other platforms (architectures?) (powerpc, powerpc64, arm64, riscv64)
> > multiplex their singular interrupt clock to schedule both a
> > fixed-period hardclock and a pseudorandom statclock.
> > 
> > This is the direction I intend to take every platform, mips64
> > included, after the next release.
> > 
> > In that context, would there be any reason to prefer glxclk to
> > CP0.count?
> 
> No. The cop0 timer is supposed to be the most reliable timer available.
> (although one may argue that, on sgi, the xbow timer on some systems is
> even better quality)

Alright, got it.  If glxclk provides no other utility aside from an
interrupt clock on loongson, then you and I can coordinate unhooking
it when we switch loongson to the new clockintr code in the Fall.

If I'm missing something and it does other work, then nevermind.

Does the latest patch work on any loongson machines you have?

I didn't see any other splx(9) implementations aside from bonito and
the one for loongson3.

Index: mips64/mips64/clock.c
===
RCS file: /cvs/src/sys/arch/mips64/mips64/clock.c,v
retrieving revision 1.45
diff -u -p -r1.45 clock.c
--- mips64/mips64/clock.c   6 Apr 2022 18:59:26 -   1.45
+++ mips64/mips64/clock.c   9 Aug 2022 14:48:47 -
@@ -60,6 +60,7 @@ const struct cfattach clock_ca = {
 };
 
 void   cp0_startclock(struct cpu_info *);
+void   cp0_trigger_int5(void);
 uint32_t cp0_int5(uint32_t, struct trapframe *);
 
 int
@@ -86,19 +87,20 @@ clockattach(struct device *parent, struc
cp0_set_compare(cp0_get_count() - 1);
 
md_startclock = cp0_startclock;
+   md_triggerclock = cp0_trigger_int5;
 }
 
 /*
  *  Interrupt handler for targets using the internal count register
  *  as interval clock. Normally the system is run with the clock
  *  interrupt always enabled. Masking is done here and if the clock
- *  can not be run the tick is just counted and handled later when
- *  the clock is logically unmasked again.
+ *  cannot be run the tick is handled later when the clock is logically
+ *  unmasked again.
  */
 uint32_t
 cp0_int5(uint32_t mask, struct trapframe *tf)
 {
-   u_int32_t clkdiff;
+   u_int32_t clkdiff, pendingticks = 0;
struct cpu_info *ci = curcpu();
 
/*
@@ -113,15 +115,26 @@ cp0_int5(uint32_t mask, struct trapframe
}
 
/*
+* If the clock interrupt is masked, defer any work until it
+* is unmasked from splx(9).
+*/
+   if (tf->ipl >= IPL_CLOCK) {
+   ci->ci_clock_deferred = 1;
+   cp0_set_compare(cp0_get_count() - 1);
+   return CR_INT_5;
+   }
+   ci->ci_clock_deferred = 0;
+
+   /*
 * Count how many ticks have passed since the last clock interrupt...
 */
clkdiff = cp0_get_count() - ci->ci_cpu_counter_last;
while (clkdiff >= ci->ci_cpu_counter_interval) {
ci->ci_cpu_counter_last += ci->ci_cpu_counter_interval;
clkdiff = cp0_get_count() - ci->ci_cpu_counter_last;
-   ci->ci_pendingticks++;
+   pendingticks++;
}
-   ci->ci_pendingticks++;
+   pendingticks++;
ci->ci_cpu_counter_last += ci->ci_cpu_counter_interval;
 
/*
@@ -132,32 +145,64 @@ cp0_int5(uint32_t mask, struct trapframe
clkdiff = cp0_get_count() - ci->ci_cpu_counter_last;
if ((int)clkdiff >= 0) {
ci->ci_cpu_counter_last += ci->ci_cpu_counter_interval;
-   ci->ci_pendingticks++;
+   pendingticks++;
cp0_set_compare(ci->ci_cpu_counter_last);
}
 
/*
-* Process clock interrupt unless it is currently masked.
+* Process clock interrupt.
 */
-   if (tf->ipl < IPL_CLOCK) {
 #ifdef MULTIPROCESSOR
-   register_t sr;
+   register_t sr;
 
-   sr = getsr();
-   ENABLEIPI();
+   sr = getsr();
+   ENABLEIPI();
 #endif
-   while (ci->ci_pendingticks) {
-   atomic_inc_long(
-   (unsigned long *)_clock_count.ec_count);
-   hardclock(tf);
-   ci->ci_pendingticks--;
-   }
+   while (pendingticks) {
+   atomic_inc_long((unsigned long *)_clock_count.ec_count);
+   hardclock(tf);
+   pendingticks--;
+   }
 #ifdef MULTIPROCESSOR
-   setsr(sr);
+   setsr(sr);
 #endif
-   }
 
return CR_INT_5;/* Clock is always on 5 */
+}
+
+unsigned long cp0_raise_calls, cp0_raise_miss;
+
+/*
+ * Trigger the clock interrupt.
+ * 
+ * We need to spin until either (a) INT5 is pending or (b) the compare
+ * register leads the count register, i.e. we know INT5 will be pending
+ * very soon.
+ *
+ * To ensure we don't spin forever, double the compensatory offset
+ 

httpd: output 'meta viewport' with autogenerated pages

2022-08-09 Thread Matthias Balk
Hi,

httpd's autogenerated pages (file index and error page) should use the
"viewport" meta tag, because they would be better displayed on mobile
devices in my opinion.

-- 
matthias


diff --git usr.sbin/httpd/server_file.c usr.sbin/httpd/server_file.c
index 786f3677422..a09a5264e62 100644
--- usr.sbin/httpd/server_file.c
+++ usr.sbin/httpd/server_file.c
@@ -531,6 +531,7 @@ server_file_index(struct httpd *env, struct client *clt)
"\n"
"Index of %s\n"
"\n"
+   "\n"
"\n"
"\n"
"Index of %s\n"
diff --git usr.sbin/httpd/server_http.c usr.sbin/httpd/server_http.c
index 63c91e8d075..c611a416d87 100644
--- usr.sbin/httpd/server_http.c
+++ usr.sbin/httpd/server_http.c
@@ -1005,6 +1005,7 @@ server_abort_http(struct client *clt, unsigned int code, 
const char *msg)
"\n"
"%03d %s\n"
"\n"
+   "\n"
"\n"
"\n"
"%03d %s\n"



alpha: remove misaligned access emulation code

2022-08-09 Thread Miod Vallat
The alpha part contains code in the kernel to handle unaligned memory
accesses from userland programs, to prevent them from dying in horrible
SIGBUS.

This made sense in the '90s, but since then people have learned to work
with strict-alignment architectures, and this code has been less and
less triggered - in my own experience I don't remember seeing it
triggered in the last 20 years.

I think it's time to send it to the Attic for a well-deserved
retirement, and let these ill-behaved userland programs catch the SIGBUS
they deserve.

Index: etc/etc.alpha/sysctl.conf
===
RCS file: /OpenBSD/src/etc/etc.alpha/sysctl.conf,v
retrieving revision 1.8
diff -u -p -r1.8 sysctl.conf
--- etc/etc.alpha/sysctl.conf   2 Mar 2013 22:53:10 -   1.8
+++ etc/etc.alpha/sysctl.conf   9 Aug 2022 06:16:34 -
@@ -1,5 +1,2 @@
-#machdep.unaligned_print=0 # 0 - disable printing of unaligned access
-#machdep.unaligned_fix=0   # 0 - disable fixup of unaligned access
-#machdep.unaligned_sigbus=0# 0 - don't sigbus on unaligned access
 #machdep.allowaperture=1   # see xf86(4)
 #machdep.led_blink=1   # blink chassis leds on DEC 3000
Index: sys/arch/alpha/alpha/machdep.c
===
RCS file: /OpenBSD/src/sys/arch/alpha/alpha/machdep.c,v
retrieving revision 1.196
diff -u -p -r1.196 machdep.c
--- sys/arch/alpha/alpha/machdep.c  6 Oct 2021 15:46:03 -   1.196
+++ sys/arch/alpha/alpha/machdep.c  9 Aug 2022 06:16:34 -
@@ -180,9 +180,6 @@ u_int8_tdec_3000_scsiid[2], dec_3000_sc
 struct platform platform;
 
 /* for cpu_sysctl() */
-intalpha_unaligned_print = 1;  /* warn about unaligned accesses */
-intalpha_unaligned_fix = 1;/* fix up unaligned accesses */
-intalpha_unaligned_sigbus = 1; /* SIGBUS on fixed-up accesses */
 #ifndef NO_IEEE
 intalpha_fp_sync_complete = 0; /* fp fixup if sync even without /s */
 #endif
@@ -1555,18 +1552,6 @@ cpu_sysctl(int *name, u_int namelen, voi
sizeof consdev));
 
 #ifndef SMALL_KERNEL
-   case CPU_UNALIGNED_PRINT:
-   return (sysctl_int(oldp, oldlenp, newp, newlen,
-   _unaligned_print));
-
-   case CPU_UNALIGNED_FIX:
-   return (sysctl_int(oldp, oldlenp, newp, newlen,
-   _unaligned_fix));
-
-   case CPU_UNALIGNED_SIGBUS:
-   return (sysctl_int(oldp, oldlenp, newp, newlen,
-   _unaligned_sigbus));
-
case CPU_BOOTED_KERNEL:
return (sysctl_rdstring(oldp, oldlenp, newp,
bootinfo.booted_kernel));
Index: sys/arch/alpha/alpha/trap.c
===
RCS file: /OpenBSD/src/sys/arch/alpha/alpha/trap.c,v
retrieving revision 1.100
diff -u -p -r1.100 trap.c
--- sys/arch/alpha/alpha/trap.c 9 Dec 2021 00:26:11 -   1.100
+++ sys/arch/alpha/alpha/trap.c 9 Aug 2022 06:16:35 -
@@ -110,21 +110,6 @@
 #endif
 #include 
 
-#ifndef SMALL_KERNEL
-
-unsigned long  Sfloat_to_reg(unsigned int);
-unsigned int   reg_to_Sfloat(unsigned long);
-unsigned long  Tfloat_reg_cvt(unsigned long);
-#ifdef FIX_UNALIGNED_VAX_FP
-unsigned long  Ffloat_to_reg(unsigned int);
-unsigned int   reg_to_Ffloat(unsigned long);
-unsigned long  Gfloat_reg_cvt(unsigned long);
-#endif
-
-intunaligned_fixup(unsigned long, unsigned long,
-   unsigned long, struct proc *);
-#endif /* SMALL_KERNEL */
-
 inthandle_opdec(struct proc *p, u_int64_t *ucodep);
 
 #ifndef NO_IEEE
@@ -249,19 +234,10 @@ trap(a0, a1, a2, entry, framep)
switch (entry) {
case ALPHA_KENTRY_UNA:
/*
-* If user-land, do whatever fixups, printing, and
-* signalling is appropriate (based on system-wide
-* and per-process unaligned-access-handling flags).
+* If user-land, deliver SIGBUS unconditionally.
 */
if (user) {
-#ifndef SMALL_KERNEL
-   KERNEL_LOCK();
-   i = unaligned_fixup(a0, a1, a2, p);
-   KERNEL_UNLOCK();
-   if (i == 0)
-   goto out;
-#endif
-
+   i = SIGBUS;
ucode = ILL_ILLADR;
v = (caddr_t)a0;
break;
@@ -716,11 +692,6 @@ ast(framep)
userret(p);
 }
 
-/*
- * Unaligned access handler.  It's not clear that this can get much slower...
- *
- */
-
 const static int reg_to_framereg[32] = {
FRAME_V0,   FRAME_T0,   FRAME_T1,   FRAME_T2,
FRAME_T3,   FRAME_T4,   FRAME_T5,   FRAME_T6,
@@ -736,356 +707,6 @@ const static int reg_to_framereg[32] = {
((reg_to_framereg[(reg)] == -1) ? NULL :\

ctfdump: fix a compiler warning or two

2022-08-09 Thread Theo Buehler
The revert of zlib.h r1.7 led to one or two compiler warnings in
ctfdump, depending on the architecture:

/usr/src/usr.bin/ctfdump/ctfdump.c:704:7: warning: format specifies type 
'unsigned long long' but the argument has type 'uLong' (aka 'unsigned long') 
[-Wformat]
stream.total_out, len);
^~~~

This one has an obvious fix. The format specifier for len is also wrong,
but -Wformat doesn't care.

/usr/src/usr.bin/ctfdump/ctfdump.c:702:23: warning: comparison of integers of 
different signs: 'uLong' (aka 'unsigned long') and 'off_t' (aka 'long long') 
[-Wsign-compare]
if (stream.total_out != len) {
 ^  ~~~

This one a bit less so.

z_off_t (aka 'long long') was changed to uLong (aka 'unsigned long'), so
it changed from signed to unsigned.

uLong will have to be converted to off_t due to the integer conversion
rank of 'long long' being higher than the one of long. This conversion
is possible on our ILP32 architectures. On our LP64 architectures, the
conversion of values > LONG_MAX is a priori implementation-defined.

So I think the correct fix is to cast them both to an unsigned type
wider than both of them. I chose uintmax_t since that cast is already
used in this file. The len < 0 check is only there for stylistic
reasons. As a sum of two uint32_t, len can't be < 0.

A similar fix will be needed in db_ctf_decompress() in sys/ddb/db_ctf.c
once we revert zlib.h r1.7 in the kernel.

Index: ctfdump.c
===
RCS file: /cvs/src/usr.bin/ctfdump/ctfdump.c,v
retrieving revision 1.25
diff -u -p -r1.25 ctfdump.c
--- ctfdump.c   10 Feb 2022 23:40:09 -  1.25
+++ ctfdump.c   9 Aug 2022 17:55:43 -
@@ -699,8 +699,8 @@ decompress(const char *buf, size_t size,
goto exit;
}
 
-   if (stream.total_out != len) {
-   warnx("decompression failed: %llu != %llu",
+   if (len < 0 || (uintmax_t)stream.total_out != (uintmax_t)len) {
+   warnx("decompression failed: %lu != %lld",
stream.total_out, len);
goto exit;
}



Re: mips64: trigger deferred timer interrupt from splx(9)

2022-08-09 Thread Miod Vallat
> Other platforms (architectures?) (powerpc, powerpc64, arm64, riscv64)
> multiplex their singular interrupt clock to schedule both a
> fixed-period hardclock and a pseudorandom statclock.
> 
> This is the direction I intend to take every platform, mips64
> included, after the next release.
> 
> In that context, would there be any reason to prefer glxclk to
> CP0.count?

No. The cop0 timer is supposed to be the most reliable timer available.
(although one may argue that, on sgi, the xbow timer on some systems is
even better quality)



Re: splassert: vlan_ioctl: want 2 have 0

2022-08-09 Thread Vitaliy Makkoveev
On Tue, Aug 09, 2022 at 06:58:05PM +0200, Hrvoje Popovski wrote:
> Hi all,
> 
> I've sysupgrade firewall from
> OpenBSD 7.2-beta (GENERIC.MP) #651: Tue Jul 26 23:11:26 MDT 2022
> to
> OpenBSD 7.2-beta (GENERIC.MP) #677: Mon Aug  8 18:58:49 MDT 2022
> 
> and on console there was lot's of spalassert like below. I'm having ix,
> aggr and vlan on that firewall
> 
> 
> splassert: vlan_ioctl: want 2 have 0
> Starting stack trace...
> vlan_ioctl(80d55800,c0406938,800020ccfe20) at vlan_ioctl+0x64
> ifioctl(fd887d7fdc78,c0406938,800020ccfe20,800020cac550) at
> ifioctl+0xc40
> soo_ioctl(fd887c7dd7f8,c0406938,800020ccfe20,800020cac550)
> at soo_ioctl+0x161
> sys_ioctl(800020cac550,800020ccff30,800020ccff90) at
> sys_ioctl+0x2c4
> syscall(800020cd) at syscall+0x384
> Xsyscall() at Xsyscall+0x128
> end of kernel
> end trace frame: 0x7f7c3da0, count: 251
> End of stack trace.
> 

Now (*if_ioctl)() called without netlock for SIOC{G.S}IFMEDIA commands.

So remove the assertion and be like all other pseudo interfaces?

Index: sys/net/if_vlan.c
===
RCS file: /cvs/src/sys/net/if_vlan.c,v
retrieving revision 1.209
diff -u -p -r1.209 if_vlan.c
--- sys/net/if_vlan.c   30 Aug 2021 14:44:39 -  1.209
+++ sys/net/if_vlan.c   9 Aug 2022 17:47:48 -
@@ -676,7 +676,6 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd
uint16_t tag;
int error = 0;
 
-   NET_ASSERT_LOCKED();
if (sc->sc_dead)
return (ENXIO);
 

Or be conservative, and drop it for SIOC{S,G}IFMEDIA only? I
intentionally left SIOCSIFMEDIA case in the original switch(cmd) block.

Index: sys/net/if_vlan.c
===
RCS file: /cvs/src/sys/net/if_vlan.c,v
retrieving revision 1.209
diff -u -p -r1.209 if_vlan.c
--- sys/net/if_vlan.c   30 Aug 2021 14:44:39 -  1.209
+++ sys/net/if_vlan.c   9 Aug 2022 17:40:12 -
@@ -676,7 +676,15 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd
uint16_t tag;
int error = 0;
 
-   NET_ASSERT_LOCKED();
+   switch (cmd) {
+   case SIOCGIFMEDIA:
+   case SIOCSIFMEDIA:
+   break;
+   default:
+   NET_ASSERT_LOCKED();
+   break;
+   }
+
if (sc->sc_dead)
return (ENXIO);
 



splassert: vlan_ioctl: want 2 have 0

2022-08-09 Thread Hrvoje Popovski
Hi all,

I've sysupgrade firewall from
OpenBSD 7.2-beta (GENERIC.MP) #651: Tue Jul 26 23:11:26 MDT 2022
to
OpenBSD 7.2-beta (GENERIC.MP) #677: Mon Aug  8 18:58:49 MDT 2022

and on console there was lot's of spalassert like below. I'm having ix,
aggr and vlan on that firewall


splassert: vlan_ioctl: want 2 have 0
Starting stack trace...
vlan_ioctl(80d55800,c0406938,800020ccfe20) at vlan_ioctl+0x64
ifioctl(fd887d7fdc78,c0406938,800020ccfe20,800020cac550) at
ifioctl+0xc40
soo_ioctl(fd887c7dd7f8,c0406938,800020ccfe20,800020cac550)
at soo_ioctl+0x161
sys_ioctl(800020cac550,800020ccff30,800020ccff90) at
sys_ioctl+0x2c4
syscall(800020cd) at syscall+0x384
Xsyscall() at Xsyscall+0x128
end of kernel
end trace frame: 0x7f7c3da0, count: 251
End of stack trace.



Re: mips64: trigger deferred timer interrupt from splx(9)

2022-08-09 Thread Scott Cheloha
On Tue, Aug 09, 2022 at 02:56:54PM +, Miod Vallat wrote:
> > Do those machines not have Coprocessor 0?  If they do, why would you
> > prefer glxclk over CP0?
> 
> cop0 only provides one timer, from which both the scheduling clock and
> statclk are derived. glxclk allows two timers to be used, and thus can
> provide a more reliable statclk (see the Torek paper, etc - it is even
> mentioned in the glxclk manual page).

Other platforms (architectures?) (powerpc, powerpc64, arm64, riscv64)
multiplex their singular interrupt clock to schedule both a
fixed-period hardclock and a pseudorandom statclock.

This is the direction I intend to take every platform, mips64
included, after the next release.

In that context, would there be any reason to prefer glxclk to
CP0.count?



Re: mips64: trigger deferred timer interrupt from splx(9)

2022-08-09 Thread Miod Vallat
> Do those machines not have Coprocessor 0?  If they do, why would you
> prefer glxclk over CP0?

cop0 only provides one timer, from which both the scheduling clock and
statclk are derived. glxclk allows two timers to be used, and thus can
provide a more reliable statclk (see the Torek paper, etc - it is even
mentioned in the glxclk manual page).



Re: mips64: trigger deferred timer interrupt from splx(9)

2022-08-09 Thread Scott Cheloha
On Tue, Aug 09, 2022 at 02:03:31PM +, Visa Hankala wrote:
> On Mon, Aug 08, 2022 at 02:52:37AM -0500, Scott Cheloha wrote:
> > One thing I'm still uncertain about is how glxclk fits into the
> > loongson picture.  It's an interrupt clock that runs hardclock() and
> > statclock(), but the code doesn't do any logical masking, so I don't
> > know whether or not I need to adjust anything in that code or account
> > for it at all.  If there's no logical masking there's no deferral, so
> > it would never call need to call md_triggerclock() from splx(9).
> 
> I think the masking of glxclk interrupts are handled by the ISA
> interrupt code.

Do those machines not have Coprocessor 0?  If they do, why would you
prefer glxclk over CP0?

> The patch misses md_triggerclock definition in mips64_machdep.c.

Whoops, forgot that file.  Fuller patch below.

> I have put this to the test on the mips64 ports builder machines.

Cool, thank you for testing.

Index: mips64/mips64/clock.c
===
RCS file: /cvs/src/sys/arch/mips64/mips64/clock.c,v
retrieving revision 1.45
diff -u -p -r1.45 clock.c
--- mips64/mips64/clock.c   6 Apr 2022 18:59:26 -   1.45
+++ mips64/mips64/clock.c   9 Aug 2022 14:48:47 -
@@ -60,6 +60,7 @@ const struct cfattach clock_ca = {
 };
 
 void   cp0_startclock(struct cpu_info *);
+void   cp0_trigger_int5(void);
 uint32_t cp0_int5(uint32_t, struct trapframe *);
 
 int
@@ -86,19 +87,20 @@ clockattach(struct device *parent, struc
cp0_set_compare(cp0_get_count() - 1);
 
md_startclock = cp0_startclock;
+   md_triggerclock = cp0_trigger_int5;
 }
 
 /*
  *  Interrupt handler for targets using the internal count register
  *  as interval clock. Normally the system is run with the clock
  *  interrupt always enabled. Masking is done here and if the clock
- *  can not be run the tick is just counted and handled later when
- *  the clock is logically unmasked again.
+ *  cannot be run the tick is handled later when the clock is logically
+ *  unmasked again.
  */
 uint32_t
 cp0_int5(uint32_t mask, struct trapframe *tf)
 {
-   u_int32_t clkdiff;
+   u_int32_t clkdiff, pendingticks = 0;
struct cpu_info *ci = curcpu();
 
/*
@@ -113,15 +115,26 @@ cp0_int5(uint32_t mask, struct trapframe
}
 
/*
+* If the clock interrupt is masked, defer any work until it
+* is unmasked from splx(9).
+*/
+   if (tf->ipl >= IPL_CLOCK) {
+   ci->ci_clock_deferred = 1;
+   cp0_set_compare(cp0_get_count() - 1);
+   return CR_INT_5;
+   }
+   ci->ci_clock_deferred = 0;
+
+   /*
 * Count how many ticks have passed since the last clock interrupt...
 */
clkdiff = cp0_get_count() - ci->ci_cpu_counter_last;
while (clkdiff >= ci->ci_cpu_counter_interval) {
ci->ci_cpu_counter_last += ci->ci_cpu_counter_interval;
clkdiff = cp0_get_count() - ci->ci_cpu_counter_last;
-   ci->ci_pendingticks++;
+   pendingticks++;
}
-   ci->ci_pendingticks++;
+   pendingticks++;
ci->ci_cpu_counter_last += ci->ci_cpu_counter_interval;
 
/*
@@ -132,32 +145,64 @@ cp0_int5(uint32_t mask, struct trapframe
clkdiff = cp0_get_count() - ci->ci_cpu_counter_last;
if ((int)clkdiff >= 0) {
ci->ci_cpu_counter_last += ci->ci_cpu_counter_interval;
-   ci->ci_pendingticks++;
+   pendingticks++;
cp0_set_compare(ci->ci_cpu_counter_last);
}
 
/*
-* Process clock interrupt unless it is currently masked.
+* Process clock interrupt.
 */
-   if (tf->ipl < IPL_CLOCK) {
 #ifdef MULTIPROCESSOR
-   register_t sr;
+   register_t sr;
 
-   sr = getsr();
-   ENABLEIPI();
+   sr = getsr();
+   ENABLEIPI();
 #endif
-   while (ci->ci_pendingticks) {
-   atomic_inc_long(
-   (unsigned long *)_clock_count.ec_count);
-   hardclock(tf);
-   ci->ci_pendingticks--;
-   }
+   while (pendingticks) {
+   atomic_inc_long((unsigned long *)_clock_count.ec_count);
+   hardclock(tf);
+   pendingticks--;
+   }
 #ifdef MULTIPROCESSOR
-   setsr(sr);
+   setsr(sr);
 #endif
-   }
 
return CR_INT_5;/* Clock is always on 5 */
+}
+
+unsigned long cp0_raise_calls, cp0_raise_miss;
+
+/*
+ * Trigger the clock interrupt.
+ * 
+ * We need to spin until either (a) INT5 is pending or (b) the compare
+ * register leads the count register, i.e. we know INT5 will be pending
+ * very soon.
+ *
+ * To ensure we don't spin forever, double the compensatory offset
+ * added to the compare value every time we miss the count register.
+ */
+void

Re: mips64: trigger deferred timer interrupt from splx(9)

2022-08-09 Thread Visa Hankala
On Mon, Aug 08, 2022 at 02:52:37AM -0500, Scott Cheloha wrote:
> One thing I'm still uncertain about is how glxclk fits into the
> loongson picture.  It's an interrupt clock that runs hardclock() and
> statclock(), but the code doesn't do any logical masking, so I don't
> know whether or not I need to adjust anything in that code or account
> for it at all.  If there's no logical masking there's no deferral, so
> it would never call need to call md_triggerclock() from splx(9).

I think the masking of glxclk interrupts are handled by the ISA
interrupt code.

The patch misses md_triggerclock definition in mips64_machdep.c.

I have put this to the test on the mips64 ports builder machines.



Re: nd6: Constify sockaddr pointer arguments in nd6_*() functions

2022-08-09 Thread Claudio Jeker
On Tue, Aug 09, 2022 at 08:42:37AM +, Klemens Nanni wrote:
> All of them are passed to inspect/copy out fields, none of the functions
> writes to the struct.
> 
> This makes it easier to argue about code (in MP context).
> 
> For this to work, ifa_ifwithaddr(), ifa_ifwithdstaddr() and
> ifaof_ifpforaddr() need const arguments as well (matching the behaviour).
> 
> Expand three satosin6() calls and use a local variable to be able to add
> const and reduce duplication.
> 
> Builds and runs fine on sparc64.
> Feedback? OK?

I'm not in favor of this move. The introduction of const just spreads like
a virus and so does the amount of dubious casts. I don't think it makes it
that much easier to argue about MP. The same can be achieved using other
methods.


> diff --git a/sys/net/if.c b/sys/net/if.c
> index cdb43e83d15..d71927b9aa9 100644
> --- a/sys/net/if.c
> +++ b/sys/net/if.c
> @@ -1414,7 +1414,7 @@ if_congested(void)
>   * Locate an interface based on a complete address.
>   */
>  struct ifaddr *
> -ifa_ifwithaddr(struct sockaddr *addr, u_int rtableid)
> +ifa_ifwithaddr(const struct sockaddr *addr, u_int rtableid)
>  {
>   struct ifnet *ifp;
>   struct ifaddr *ifa;
> @@ -1444,7 +1444,7 @@ ifa_ifwithaddr(struct sockaddr *addr, u_int rtableid)
>   * Locate the point to point interface with a given destination address.
>   */
>  struct ifaddr *
> -ifa_ifwithdstaddr(struct sockaddr *addr, u_int rdomain)
> +ifa_ifwithdstaddr(const struct sockaddr *addr, u_int rdomain)
>  {
>   struct ifnet *ifp;
>   struct ifaddr *ifa;
> @@ -1475,11 +1475,11 @@ ifa_ifwithdstaddr(struct sockaddr *addr, u_int 
> rdomain)
>   * a given address.
>   */
>  struct ifaddr *
> -ifaof_ifpforaddr(struct sockaddr *addr, struct ifnet *ifp)
> +ifaof_ifpforaddr(const struct sockaddr *addr, struct ifnet *ifp)
>  {
>   struct ifaddr *ifa;
> - char *cp, *cp2, *cp3;
> - char *cplim;
> + const char *cp, *cp2, *cp3;
> + const char *cplim;
>   struct ifaddr *ifa_maybe = NULL;
>   u_int af = addr->sa_family;
>  
> diff --git a/sys/net/if_var.h b/sys/net/if_var.h
> index 400afd319b0..f511af7dfbd 100644
> --- a/sys/net/if_var.h
> +++ b/sys/net/if_var.h
> @@ -330,9 +330,9 @@ void  p2p_rtrequest(struct ifnet *, int, struct 
> rtentry *);
>  void p2p_input(struct ifnet *, struct mbuf *);
>  int  p2p_bpf_mtap(caddr_t, const struct mbuf *, u_int);
>  
> -struct   ifaddr *ifa_ifwithaddr(struct sockaddr *, u_int);
> -struct   ifaddr *ifa_ifwithdstaddr(struct sockaddr *, u_int);
> -struct   ifaddr *ifaof_ifpforaddr(struct sockaddr *, struct ifnet *);
> +struct   ifaddr *ifa_ifwithaddr(const struct sockaddr *, u_int);
> +struct   ifaddr *ifa_ifwithdstaddr(const struct sockaddr *, u_int);
> +struct   ifaddr *ifaof_ifpforaddr(const struct sockaddr *, struct ifnet 
> *);
>  void ifafree(struct ifaddr *);
>  
>  int  if_isconnected(const struct ifnet *, unsigned int);
> diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
> index d4aedb92187..5259b0790c7 100644
> --- a/sys/netinet6/nd6.c
> +++ b/sys/netinet6/nd6.c
> @@ -1329,14 +1329,15 @@ nd6_slowtimo(void *ignored_arg)
>  
>  int
>  nd6_resolve(struct ifnet *ifp, struct rtentry *rt0, struct mbuf *m,
> -struct sockaddr *dst, u_char *desten)
> +const struct sockaddr *dst, u_char *desten)
>  {
>   struct sockaddr_dl *sdl;
>   struct rtentry *rt;
>   struct llinfo_nd6 *ln = NULL;
> + const struct in6_addr *in6 = &((struct sockaddr_in6 *)dst)->sin6_addr;
>  
>   if (m->m_flags & M_MCAST) {
> - ETHER_MAP_IPV6_MULTICAST((dst)->sin6_addr, desten);
> + ETHER_MAP_IPV6_MULTICAST(in6, desten);
>   return (0);
>   }
>  
> @@ -1404,8 +1405,7 @@ nd6_resolve(struct ifnet *ifp, struct rtentry *rt0, 
> struct mbuf *m,
>   char addr[INET6_ADDRSTRLEN];
>   log(LOG_DEBUG, "%s: %s: incorrect nd6 information\n",
>   __func__,
> - inet_ntop(AF_INET6, (dst)->sin6_addr,
> - addr, sizeof(addr)));
> + inet_ntop(AF_INET6, in6, addr, sizeof(addr)));
>   m_freem(m);
>   return (EINVAL);
>   }
> @@ -1431,7 +1431,7 @@ nd6_resolve(struct ifnet *ifp, struct rtentry *rt0, 
> struct mbuf *m,
>   if (!ND6_LLINFO_PERMANENT(ln) && ln->ln_asked == 0) {
>   ln->ln_asked++;
>   nd6_llinfo_settimer(ln, ND_IFINFO(ifp)->retrans / 1000);
> - nd6_ns_output(ifp, NULL, (dst)->sin6_addr, ln, 0);
> + nd6_ns_output(ifp, NULL, in6, ln, 0);
>   }
>   return (EAGAIN);
>  }
> diff --git a/sys/netinet6/nd6.h b/sys/netinet6/nd6.h
> index 980524f16ea..254a613ba42 100644
> --- a/sys/netinet6/nd6.h
> +++ b/sys/netinet6/nd6.h
> @@ -171,12 +171,12 @@ int nd6_ioctl(u_long, caddr_t, struct ifnet *);
>  void nd6_cache_lladdr(struct ifnet *, const struct in6_addr *, char *,
>  int, int, 

nd6: Constify sockaddr pointer arguments in nd6_*() functions

2022-08-09 Thread Klemens Nanni
All of them are passed to inspect/copy out fields, none of the functions
writes to the struct.

This makes it easier to argue about code (in MP context).

For this to work, ifa_ifwithaddr(), ifa_ifwithdstaddr() and
ifaof_ifpforaddr() need const arguments as well (matching the behaviour).

Expand three satosin6() calls and use a local variable to be able to add
const and reduce duplication.

Builds and runs fine on sparc64.
Feedback? OK?
diff --git a/sys/net/if.c b/sys/net/if.c
index cdb43e83d15..d71927b9aa9 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1414,7 +1414,7 @@ if_congested(void)
  * Locate an interface based on a complete address.
  */
 struct ifaddr *
-ifa_ifwithaddr(struct sockaddr *addr, u_int rtableid)
+ifa_ifwithaddr(const struct sockaddr *addr, u_int rtableid)
 {
struct ifnet *ifp;
struct ifaddr *ifa;
@@ -1444,7 +1444,7 @@ ifa_ifwithaddr(struct sockaddr *addr, u_int rtableid)
  * Locate the point to point interface with a given destination address.
  */
 struct ifaddr *
-ifa_ifwithdstaddr(struct sockaddr *addr, u_int rdomain)
+ifa_ifwithdstaddr(const struct sockaddr *addr, u_int rdomain)
 {
struct ifnet *ifp;
struct ifaddr *ifa;
@@ -1475,11 +1475,11 @@ ifa_ifwithdstaddr(struct sockaddr *addr, u_int rdomain)
  * a given address.
  */
 struct ifaddr *
-ifaof_ifpforaddr(struct sockaddr *addr, struct ifnet *ifp)
+ifaof_ifpforaddr(const struct sockaddr *addr, struct ifnet *ifp)
 {
struct ifaddr *ifa;
-   char *cp, *cp2, *cp3;
-   char *cplim;
+   const char *cp, *cp2, *cp3;
+   const char *cplim;
struct ifaddr *ifa_maybe = NULL;
u_int af = addr->sa_family;
 
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 400afd319b0..f511af7dfbd 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -330,9 +330,9 @@ voidp2p_rtrequest(struct ifnet *, int, struct 
rtentry *);
 void   p2p_input(struct ifnet *, struct mbuf *);
 intp2p_bpf_mtap(caddr_t, const struct mbuf *, u_int);
 
-struct ifaddr *ifa_ifwithaddr(struct sockaddr *, u_int);
-struct ifaddr *ifa_ifwithdstaddr(struct sockaddr *, u_int);
-struct ifaddr *ifaof_ifpforaddr(struct sockaddr *, struct ifnet *);
+struct ifaddr *ifa_ifwithaddr(const struct sockaddr *, u_int);
+struct ifaddr *ifa_ifwithdstaddr(const struct sockaddr *, u_int);
+struct ifaddr *ifaof_ifpforaddr(const struct sockaddr *, struct ifnet *);
 void   ifafree(struct ifaddr *);
 
 intif_isconnected(const struct ifnet *, unsigned int);
diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
index d4aedb92187..5259b0790c7 100644
--- a/sys/netinet6/nd6.c
+++ b/sys/netinet6/nd6.c
@@ -1329,14 +1329,15 @@ nd6_slowtimo(void *ignored_arg)
 
 int
 nd6_resolve(struct ifnet *ifp, struct rtentry *rt0, struct mbuf *m,
-struct sockaddr *dst, u_char *desten)
+const struct sockaddr *dst, u_char *desten)
 {
struct sockaddr_dl *sdl;
struct rtentry *rt;
struct llinfo_nd6 *ln = NULL;
+   const struct in6_addr *in6 = &((struct sockaddr_in6 *)dst)->sin6_addr;
 
if (m->m_flags & M_MCAST) {
-   ETHER_MAP_IPV6_MULTICAST((dst)->sin6_addr, desten);
+   ETHER_MAP_IPV6_MULTICAST(in6, desten);
return (0);
}
 
@@ -1404,8 +1405,7 @@ nd6_resolve(struct ifnet *ifp, struct rtentry *rt0, 
struct mbuf *m,
char addr[INET6_ADDRSTRLEN];
log(LOG_DEBUG, "%s: %s: incorrect nd6 information\n",
__func__,
-   inet_ntop(AF_INET6, (dst)->sin6_addr,
-   addr, sizeof(addr)));
+   inet_ntop(AF_INET6, in6, addr, sizeof(addr)));
m_freem(m);
return (EINVAL);
}
@@ -1431,7 +1431,7 @@ nd6_resolve(struct ifnet *ifp, struct rtentry *rt0, 
struct mbuf *m,
if (!ND6_LLINFO_PERMANENT(ln) && ln->ln_asked == 0) {
ln->ln_asked++;
nd6_llinfo_settimer(ln, ND_IFINFO(ifp)->retrans / 1000);
-   nd6_ns_output(ifp, NULL, (dst)->sin6_addr, ln, 0);
+   nd6_ns_output(ifp, NULL, in6, ln, 0);
}
return (EAGAIN);
 }
diff --git a/sys/netinet6/nd6.h b/sys/netinet6/nd6.h
index 980524f16ea..254a613ba42 100644
--- a/sys/netinet6/nd6.h
+++ b/sys/netinet6/nd6.h
@@ -171,12 +171,12 @@ int nd6_ioctl(u_long, caddr_t, struct ifnet *);
 void nd6_cache_lladdr(struct ifnet *, const struct in6_addr *, char *,
 int, int, int);
 int nd6_resolve(struct ifnet *, struct rtentry *, struct mbuf *,
-struct sockaddr *, u_char *);
+const struct sockaddr *, u_char *);
 int nd6_need_cache(struct ifnet *);
 
 void nd6_na_input(struct mbuf *, int, int);
 void nd6_na_output(struct ifnet *, const struct in6_addr *,
-   const struct in6_addr *, u_long, int, struct sockaddr *);
+   const struct in6_addr *, u_long, int, const struct sockaddr *);
 void nd6_ns_input(struct mbuf *, int, int);
 void