Re: rc.d(8) for tcpbench
On Tue, Dec 15, 2020 at 10:59:50PM +, Stuart Henderson wrote: > On 2020/12/15 23:07, Jan Klemkow wrote: > > for frequent performance test it would be nice to just start tcpbench > > as a regular service. tcpbench gets an extra user and group with this > > diff and is already pledged to "stdio". Thus, there should be no > > security risk to do this even in hostile environments. > > "io" is just for client, for server it's: > > $ ps -O pledge -ax|grep [t]cpb > 43696 stdio,inet,unix pf > I+p 0:00.00 tcpbench -s Oh yes, sorry, I missed that by looking over the source too quickly. > > diff -u -p -r1.104 master.passwd > > --- etc/master.passwd 24 Apr 2020 14:57:31 - 1.104 > > +++ etc/master.passwd 15 Dec 2020 21:25:17 - > > @@ -62,4 +62,5 @@ _ftp_proxy:*:109:109::0:0:ftp proxy daem > > _sndiop:*:110:110::0:0:sndio privileged user:/var/empty:/sbin/nologin > > _syspatch:*:112:112::0:0:syspatch unprivileged > > user:/var/empty:/sbin/nologin > > _slaacd:*:115:115::0:0:SLAAC Daemon:/var/empty:/sbin/nologin > > +_tcpbench:*:116:116::0:0:tcpbench unprivileged > > user:/var/empty:/sbin/nologin > > nobody:*:32767:32767::0:0:Unprivileged user:/nonexistent:/sbin/nologin > > no need to grow the "used system uid" space, 111 is available and was > taken for less than a day so it's unlikely anyone would even need to > merge passwd/group files. or maybe a lower one is better, I don't know > if there's a particular numbering plan for these.. Sure, I will fix that, if this thing has a future. > > Index: etc/rc.d/tcpbench > > === > > RCS file: etc/rc.d/tcpbench > > diff -N etc/rc.d/tcpbench > > --- /dev/null 1 Jan 1970 00:00:00 - > > +++ etc/rc.d/tcpbench 15 Dec 2020 20:30:18 - > > @@ -0,0 +1,12 @@ > > +#!/bin/ksh > > + > > +daemon="/usr/bin/tcpbench" > > +daemon_flags="-s" > > +daemon_user=_tcpbench > > + > > +. /etc/rc.d/rc.subr > > + > > +rc_reload=NO > > +rc_bg=YES > > + > > +rc_cmd $1 > > > > I am not a big fan of this to be honest. tcpbench is written more as a > test tool than a network daemon. For starters, the spew on the console > when it's active is not very nice. As I described in answer to Theo, its seems to be easier this way, if you and to script its. Or, you want to test the quality of link to different locations frequently.
Re: rc.d(8) for tcpbench
On Tue, Dec 15, 2020 at 03:43:38PM -0700, Theo de Raadt wrote: > Jan Klemkow wrote: > > > for frequent performance test it would be nice to just start tcpbench > > as a regular service. tcpbench gets an extra user and group with this > > diff and is already pledged to "stdio". Thus, there should be no > > security risk to do this even in hostile environments. > > You're kidding me. If someone starts this in a hostile environment, their > network/host will be flattened. You are right, someone can use this, to flood a link. But, you can flood someones link with traffic anyway, as botnets do it, or? > I find it difficult to believe there is any environment where someone > wants tcpbench running *all the time*. Sure, its not ideal to run this on public interfaces. I just want to say, its unlikely that someone will take over you system via bugs in this daemon, in my opinion. As it has similar mitigation techniques as our other daemons. I run this daemon in permanent tests setups and on links to different locations. Its easier to use rcctl enable/start then to deal with background sessions on remote machines in shell scripts. Do you think its OK to make a port out of this rc-script for this purpose?
rc.d(8) for tcpbench
Hi,
for frequent performance test it would be nice to just start tcpbench
as a regular service. tcpbench gets an extra user and group with this
diff and is already pledged to "stdio". Thus, there should be no
security risk to do this even in hostile environments.
OK?
bye,
Jan
Index: etc/Makefile
===
RCS file: /cvs/src/etc/Makefile,v
retrieving revision 1.480
diff -u -p -r1.480 Makefile
--- etc/Makefile13 Sep 2020 11:29:52 - 1.480
+++ etc/Makefile15 Dec 2020 21:05:07 -
@@ -64,7 +64,7 @@ RCDAEMONS=amd apmd bgpd bootparamd cron
lpd mopd mountd mrouted nfsd npppd nsd ntpd ospf6d ospfd \
pflogd portmap rad radiusd rarpd rbootd relayd ripd route6d \
sasyncd sensorsd slowcgi slaacd smtpd sndiod snmpd spamd \
- spamlogd sshd statd switchd syslogd tftpd tftpproxy unbound \
+ spamlogd sshd statd switchd syslogd tcpbench tftpd tftpproxy unbound \
unwind vmd watchdogd wsmoused xenodm ypbind ypldap ypserv
MISETS=base${OSrev}.tgz comp${OSrev}.tgz man${OSrev}.tgz
game${OSrev}.tgz
Index: etc/group
===
RCS file: /cvs/src/etc/group,v
retrieving revision 1.94
diff -u -p -r1.94 group
--- etc/group 28 Jan 2020 16:51:03 - 1.94
+++ etc/group 15 Dec 2020 20:48:07 -
@@ -79,6 +79,7 @@ _ftp_proxy:*:109:
_sndiop:*:110:
_syspatch:*:112:
_slaacd:*:115:
+_tcpbench:*:116:
dialer:*:117:
nogroup:*:32766:
nobody:*:32767:
Index: etc/master.passwd
===
RCS file: /cvs/src/etc/master.passwd,v
retrieving revision 1.104
diff -u -p -r1.104 master.passwd
--- etc/master.passwd 24 Apr 2020 14:57:31 - 1.104
+++ etc/master.passwd 15 Dec 2020 21:25:17 -
@@ -62,4 +62,5 @@ _ftp_proxy:*:109:109::0:0:ftp proxy daem
_sndiop:*:110:110::0:0:sndio privileged user:/var/empty:/sbin/nologin
_syspatch:*:112:112::0:0:syspatch unprivileged user:/var/empty:/sbin/nologin
_slaacd:*:115:115::0:0:SLAAC Daemon:/var/empty:/sbin/nologin
+_tcpbench:*:116:116::0:0:tcpbench unprivileged user:/var/empty:/sbin/nologin
nobody:*:32767:32767::0:0:Unprivileged user:/nonexistent:/sbin/nologin
Index: etc/rc.conf
===
RCS file: /cvs/src/etc/rc.conf,v
retrieving revision 1.220
diff -u -p -r1.220 rc.conf
--- etc/rc.conf 24 Jan 2020 06:17:37 - 1.220
+++ etc/rc.conf 15 Dec 2020 20:32:46 -
@@ -65,6 +65,7 @@ spamlogd_flags= # use eg. "-i interface
sshd_flags=
switchd_flags=NO
syslogd_flags= # add more flags, e.g. "-u -a /chroot/dev/log"
+tcpbench_flags=NO
tftpd_flags=NO
tftpproxy_flags=NO
unbound_flags=NO
Index: etc/mail/aliases
===
RCS file: /cvs/src/etc/mail/aliases,v
retrieving revision 1.68
diff -u -p -r1.68 aliases
--- etc/mail/aliases24 Jan 2020 06:17:37 - 1.68
+++ etc/mail/aliases15 Dec 2020 20:48:31 -
@@ -79,6 +79,7 @@ _ftp_proxy: /dev/null
_sndiop: /dev/null
_syspatch: /dev/null
_slaacd: /dev/null
+_tcpbench: /dev/null
sshd: /dev/null
# Well-known aliases -- these should be filled in!
Index: etc/rc.d/tcpbench
===
RCS file: etc/rc.d/tcpbench
diff -N etc/rc.d/tcpbench
--- /dev/null 1 Jan 1970 00:00:00 -
+++ etc/rc.d/tcpbench 15 Dec 2020 20:30:18 -
@@ -0,0 +1,12 @@
+#!/bin/ksh
+
+daemon="/usr/bin/tcpbench"
+daemon_flags="-s"
+daemon_user=_tcpbench
+
+. /etc/rc.d/rc.subr
+
+rc_reload=NO
+rc_bg=YES
+
+rc_cmd $1
Re: rc.d(8) for tcpbench
On 2020/12/15 17:19, Theo de Raadt wrote: > Stuart Henderson wrote: > > > On 2020/12/15 16:33, Theo de Raadt wrote: > > > Jan Klemkow wrote: > > > > > > > On Tue, Dec 15, 2020 at 03:43:38PM -0700, Theo de Raadt wrote: > > > > > Jan Klemkow wrote: > > > > > > > > > > > for frequent performance test it would be nice to just start > > > > > > tcpbench > > > > > > as a regular service. tcpbench gets an extra user and group with > > > > > > this > > > > > > diff and is already pledged to "stdio". Thus, there should be no > > > > > > security risk to do this even in hostile environments. > > > > > > > > > > You're kidding me. If someone starts this in a hostile environment, > > > > > their > > > > > network/host will be flattened. > > > > > > > > You are right, someone can use this, to flood a link. But, you can > > > > flood someones link with traffic anyway, as botnets do it, or? > > > > > > It is not the same at all, because tcpbench will attempt to flow maximum > > > traffic in both directions. No other service has that behaviour. > > > > > > > -s just throws the packets away, it does not transmit > > Regardless, I still don't think it makes any sense placing a debugging > feature into the hands of people who don't know what they are doing. > agreed
Re: rc.d(8) for tcpbench
Stuart Henderson wrote: > On 2020/12/15 16:33, Theo de Raadt wrote: > > Jan Klemkow wrote: > > > > > On Tue, Dec 15, 2020 at 03:43:38PM -0700, Theo de Raadt wrote: > > > > Jan Klemkow wrote: > > > > > > > > > for frequent performance test it would be nice to just start tcpbench > > > > > as a regular service. tcpbench gets an extra user and group with this > > > > > diff and is already pledged to "stdio". Thus, there should be no > > > > > security risk to do this even in hostile environments. > > > > > > > > You're kidding me. If someone starts this in a hostile environment, > > > > their > > > > network/host will be flattened. > > > > > > You are right, someone can use this, to flood a link. But, you can > > > flood someones link with traffic anyway, as botnets do it, or? > > > > It is not the same at all, because tcpbench will attempt to flow maximum > > traffic in both directions. No other service has that behaviour. > > > > -s just throws the packets away, it does not transmit Regardless, I still don't think it makes any sense placing a debugging feature into the hands of people who don't know what they are doing.
Re: rc.d(8) for tcpbench
On 2020/12/15 16:33, Theo de Raadt wrote: > Jan Klemkow wrote: > > > On Tue, Dec 15, 2020 at 03:43:38PM -0700, Theo de Raadt wrote: > > > Jan Klemkow wrote: > > > > > > > for frequent performance test it would be nice to just start tcpbench > > > > as a regular service. tcpbench gets an extra user and group with this > > > > diff and is already pledged to "stdio". Thus, there should be no > > > > security risk to do this even in hostile environments. > > > > > > You're kidding me. If someone starts this in a hostile environment, their > > > network/host will be flattened. > > > > You are right, someone can use this, to flood a link. But, you can > > flood someones link with traffic anyway, as botnets do it, or? > > It is not the same at all, because tcpbench will attempt to flow maximum > traffic in both directions. No other service has that behaviour. > -s just throws the packets away, it does not transmit
Re: Kernel panic with i386 on latest snapshot
On Tue, 15 Dec 2020 at 15:03, Mark Kettenis wrote: > > > Thanks. This is committed now. However, there may be other case > where we use uvm_km_valloc() early on that will trip over the kernel > lock assertion that mpi@ added in uvm_km_pgremove(). Ideally we > should get rid of all the uvm_km_free() calls in the kernel. > Thank you and mpi@ for the fix. I wasn't able to test your diff, since my machine doesn't have the src checked out, so thank you to bluhm and Hrvoje for testing it out.
Re: rc.d(8) for tcpbench
Jan Klemkow wrote: > On Tue, Dec 15, 2020 at 03:43:38PM -0700, Theo de Raadt wrote: > > Jan Klemkow wrote: > > > > > for frequent performance test it would be nice to just start tcpbench > > > as a regular service. tcpbench gets an extra user and group with this > > > diff and is already pledged to "stdio". Thus, there should be no > > > security risk to do this even in hostile environments. > > > > You're kidding me. If someone starts this in a hostile environment, their > > network/host will be flattened. > > You are right, someone can use this, to flood a link. But, you can > flood someones link with traffic anyway, as botnets do it, or? It is not the same at all, because tcpbench will attempt to flow maximum traffic in both directions. No other service has that behaviour.
Re: Kernel panic with i386 on latest snapshot
> Date: Tue, 15 Dec 2020 21:21:37 +0100
> From: Alexander Bluhm
>
> On Tue, Dec 15, 2020 at 06:57:03PM +0100, Mark Kettenis wrote:
> > Does the diff below fix this?
>
> I can reproduce the panic and your diff fixes it.
>
> Usually my regress machines do not trigger it as I do not install
> firmware. fw_update and reboot makes it crash.
>
> bluhm
Thanks. This is committed now. However, there may be other case
where we use uvm_km_valloc() early on that will trip over the kernel
lock assertion that mpi@ added in uvm_km_pgremove(). Ideally we
should get rid of all the uvm_km_free() calls in the kernel.
> OpenBSD 6.8-current (GENERIC.MP) #0: Tue Dec 15 20:18:20 CET 2020
> r...@ot2.obsd-lab.genua.de:/usr/src/sys/arch/i386/compile/GENERIC.MP
> real mem = 2146910208 (2047MB)
> avail mem = 2091409408 (1994MB)
> panic: kernel diagnostic assertion "_kernel_lock_held()" failed: file
> "/usr/src/sys/uvm/uvm_km.c", line 246
> Stopped at db_enter+0x4: popl%ebp
> TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
> db_enter(d0e47604,d110ae68,d0e46ec4,d0e47fec,d0e47fec) at db_enter+0x4
> panic(d0bc79e5,d0c2da24,d0c3a8c4,d0c522a9,f6) at panic+0xd3
> __assert(d0c2da24,d0c522a9,f6,d0c3a8c4,d0e46ec4) at __assert+0x19
> uvm_km_pgremove(d0e2da74,2552c000,2552e000) at uvm_km_pgremove+0x119
> uvm_unmap_kill_entry(d0e47fec,d0e46ec4) at uvm_unmap_kill_entry+0x92
> uvm_unmap_remove(d0e47fec,f552c000,f552e000,d110af10,0,1) at
> uvm_unmap_remove+0
> x1cb
> uvm_unmap(d0e47fec,f552c000,f552e000) at uvm_unmap+0x53
> uvm_km_free(d0e47fec,f552c000,2000,2000) at uvm_km_free+0x25
> cpu_ucode_setup(f092c000,f080,efff9000,150a416,1108000) at
> cpu_ucode_setup+
> 0xeb
> cpu_startup(150a416,1108000,1117000,110b000,0) at cpu_startup+0x14a
> main(0,0,0,0,0) at main+0x6b
> https://www.openbsd.org/ddb.html describes the minimum info required in bug
> reports. Insufficient info makes it difficult to find and fix bugs.
> ddb{0}> boot reboot
> rebooting...
>
> OpenBSD 6.8-current (GENERIC.MP) #1: Tue Dec 15 20:34:36 CET 2020
> r...@ot2.obsd-lab.genua.de:/usr/src/sys/arch/i386/compile/GENERIC.MP
> real mem = 2146910208 (2047MB)
> avail mem = 2091417600 (1994MB)
> random: good seed from bootblocks
> mpath0 at root
> scsibus0 at mpath0: 256 targets
> mainbus0 at root
> bios0 at mainbus0: date 10/20/04, BIOS32 rev. 0 @ 0xfdb30, SMBIOS rev. 2.3 @
> 0xf0640 (63 entries)
> bios0: vendor American Megatrends Inc. version "0700xx" date 10/20/2004
> bios0: Supermicro. X5DL8
> acpi0 at bios0: ACPI 1.0
> acpi0: sleep states S0 S1 S4 S5
> acpi0: tables DSDT FACP APIC
> acpi0: wakeup devices SLPB(S1) NRTH(S5) PS2M(S1) PS2K(S1) UAR1(S1) UAR2(S1)
> USB_(S1) PCI1(S5) PCI2(S5) PCI3(S5) PCI4(S5)
> acpitimer0 at acpi0: 3579545 Hz, 32 bits
> acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
> cpu0 at mainbus0: apid 0 (boot processor)
> cpu0: Intel(R) Xeon(TM) CPU 3.06GHz ("GenuineIntel" 686-class) 3.07 GHz,
> 0f-02-05
> cpu0:
> FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,CNXT-ID,xTPR,PERF,MELTDOWN
> mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
> cpu0: apic clock running at 133MHz
> cpu1 at mainbus0: apid 6 (application processor)
> cpu1: Intel(R) Xeon(TM) CPU 3.06GHz ("GenuineIntel" 686-class) 3.07 GHz,
> 0f-02-05
> cpu1:
> FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,CNXT-ID,xTPR,PERF,MELTDOWN
> cpu2 at mainbus0: apid 1 (application processor)
> cpu2: Intel(R) Xeon(TM) CPU 3.06GHz ("GenuineIntel" 686-class) 3.07 GHz,
> 0f-02-05
> cpu2:
> FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,CNXT-ID,xTPR,PERF,MELTDOWN
> cpu3 at mainbus0: apid 7 (application processor)
> cpu3: Intel(R) Xeon(TM) CPU 3.06GHz ("GenuineIntel" 686-class) 3.07 GHz,
> 0f-02-05
> cpu3:
> FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,CNXT-ID,xTPR,PERF,MELTDOWN
> ioapic0 at mainbus0: apid 8 pa 0xfec0, version 11, 16 pins
> ioapic1 at mainbus0: apid 9 pa 0xfec01000, version 11, 16 pins
> ioapic2 at mainbus0: apid 10 pa 0xfec02000, version 11, 16 pins
> acpiprt0 at acpi0: bus 0 (NRTH)
> acpiprt1 at acpi0: bus 1 (PCI1)
> acpiprt2 at acpi0: bus 2 (PCI2)
> acpiprt3 at acpi0: bus 4 (PCI3)
> acpiprt4 at acpi0: bus 5 (PCI4)
> acpibtn0 at acpi0: SLPB
> "PNP0A03" at acpi0 not configured
> acpicmos0 at acpi0
> "PNP0A03" at acpi0 not configured
> "PNP0A03" at acpi0 not configured
> "PNP0A03" at acpi0 not configured
> "PNP0A03" at acpi0 not configured
> acpicpu0 at acpi0: C1(@1 halt!)
> acpicpu1 at acpi0: C1(@1 halt!)
> acpicpu2 at acpi0: C1(@1 halt!)
> acpicpu3 at acpi0: C1(@1 halt!)
> bios0: ROM list: 0xc/0x8000 0xc8000/0x1000 0xc9000/0x1800 0xca800/0x1800
> pci0 at mainbus0 bus 0: configuration mode 1 (bios)
>
Re: rc.d(8) for tcpbench
On 2020/12/15 23:07, Jan Klemkow wrote: > Hi, > > for frequent performance test it would be nice to just start tcpbench > as a regular service. tcpbench gets an extra user and group with this > diff and is already pledged to "stdio". Thus, there should be no > security risk to do this even in hostile environments. "io" is just for client, for server it's: $ ps -O pledge -ax|grep [t]cpb 43696 stdio,inet,unix pf I+p 0:00.00 tcpbench -s > diff -u -p -r1.104 master.passwd > --- etc/master.passwd 24 Apr 2020 14:57:31 - 1.104 > +++ etc/master.passwd 15 Dec 2020 21:25:17 - > @@ -62,4 +62,5 @@ _ftp_proxy:*:109:109::0:0:ftp proxy daem > _sndiop:*:110:110::0:0:sndio privileged user:/var/empty:/sbin/nologin > _syspatch:*:112:112::0:0:syspatch unprivileged user:/var/empty:/sbin/nologin > _slaacd:*:115:115::0:0:SLAAC Daemon:/var/empty:/sbin/nologin > +_tcpbench:*:116:116::0:0:tcpbench unprivileged user:/var/empty:/sbin/nologin > nobody:*:32767:32767::0:0:Unprivileged user:/nonexistent:/sbin/nologin no need to grow the "used system uid" space, 111 is available and was taken for less than a day so it's unlikely anyone would even need to merge passwd/group files. or maybe a lower one is better, I don't know if there's a particular numbering plan for these.. > Index: etc/rc.d/tcpbench > === > RCS file: etc/rc.d/tcpbench > diff -N etc/rc.d/tcpbench > --- /dev/null 1 Jan 1970 00:00:00 - > +++ etc/rc.d/tcpbench 15 Dec 2020 20:30:18 - > @@ -0,0 +1,12 @@ > +#!/bin/ksh > + > +daemon="/usr/bin/tcpbench" > +daemon_flags="-s" > +daemon_user=_tcpbench > + > +. /etc/rc.d/rc.subr > + > +rc_reload=NO > +rc_bg=YES > + > +rc_cmd $1 > I am not a big fan of this to be honest. tcpbench is written more as a test tool than a network daemon. For starters, the spew on the console when it's active is not very nice.
Re: rc.d(8) for tcpbench
Jan Klemkow wrote: > for frequent performance test it would be nice to just start tcpbench > as a regular service. tcpbench gets an extra user and group with this > diff and is already pledged to "stdio". Thus, there should be no > security risk to do this even in hostile environments. You're kidding me. If someone starts this in a hostile environment, their network/host will be flattened. I find it difficult to believe there is any environment where someone wants tcpbench running *all the time*.
httpd: escape double-quotes in server logs
Hi all
When using the 'combined' or 'forwarded' log style, the request,
referrer and user-agent are all wrapped in double-quotes (") like this:
ifconfig.se 127.0.0.1 - - [15/Dec/2020:22:38:54 +0100] "GET / HTTP/1.1" 200
6320 "Referrer" "User-Agent" 10.0.10.5 -
Since all three are provided by and can be modified the client, they can
easily add extra double-quotes. This can be used to make the logs look
weird or somewhat forged. For example, if the requst is sent with the
user-agent 'User-Agent" 1.2.3.4 "', the logs will look like:
ifconfig.se 127.0.0.1 - - [15/Dec/2020:22:38:54 +0100] "GET / HTTP/1.1" 200
6320 "Referrer" "User-Agent" 1.2.3.4 "" 10.0.10.5 -
By using stravis VIS_DQ flag, they will be encoded with a backslash:
ifconfig.se 127.0.0.1 - - [15/Dec/2020:22:38:54 +0100] "GET / HTTP/1.1" 200
6320 "Referrer" "User-Agent\" 1.2.3.4 \"" 10.0.10.5 -
My inital concern was that someone could forge the 'forwarded' IP.
I tried to work around that by changing the order and have it added
before the user-agent, but after reading the lists[1][2] I learned that
it would break the 'combined' log parsing for Webalizer and GoAccess.
I have no idea how my patch would affect those, but we already encode
backslash, newline and tabs the same way so I guess they handle it well.
A more interesting question is, how do they handle double-quotes in the
referrer or user-agent.
Any thoughts?
Yours,
Jesper Wallin
[1] https://marc.info/?t=154201313600048&r=1&w=2
[2] https://marc.info/?t=15517048833&r=1&w=2
Index: httpd.h
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.153
diff -u -p -r1.153 httpd.h
--- httpd.h 29 Oct 2020 12:30:52 - 1.153
+++ httpd.h 15 Dec 2020 21:42:58 -
@@ -57,7 +57,7 @@
#define HTTPD_REALM_MAX255
#define HTTPD_LOCATION_MAX 255
#define HTTPD_DEFAULT_TYPE { "bin", "application", "octet-stream", NULL }
-#define HTTPD_LOGVIS VIS_NL|VIS_TAB|VIS_CSTYLE
+#define HTTPD_LOGVIS VIS_DQ|VIS_NL|VIS_TAB|VIS_CSTYLE
#define HTTPD_TLS_CERT "/etc/ssl/server.crt"
#define HTTPD_TLS_KEY "/etc/ssl/private/server.key"
#define HTTPD_TLS_CONFIG_MAX 511
Re: SIGSEGV in _rthread_tls_destructors()
On 15/12/20(Tue) 16:30, Mark Kettenis wrote:
> > Date: Tue, 15 Dec 2020 12:15:30 -0300
> > From: Martin Pieuchot
> >
> > When the first thread of multimedia/mpv exits after having played a video
> > with the "gpu" (default) output, the programs receives a SIGSEV when it
> > tries to execute one of its destructor:
> >
> > void
> > _rthread_tls_destructors(pthread_t thread)
> > {
> > [...]
> > for (i = 0; i < PTHREAD_DESTRUCTOR_ITERATIONS; i++) {
> > for (rs = thread->local_storage; rs; rs = rs->next) {
> > if (!rs->data)
> > continue;
> > if (rkeys[rs->keyid].destructor) {
> > void (*destructor)(void *) =
> > rkeys[rs->keyid].destructor;
> > void *data = rs->data;
> > rs->data = NULL;
> > _spinunlock(&rkeyslock);
> > destructor(data); <-- HERE
> > _spinlock(&rkeyslock);
> > }
> > }
> > }
> > [...]
> > }
> >
> > This doesn't happen with other outputs and I haven't checked/don't know
> > which piece of code in the "gpu" output calls pthread_key_create().
> >
> > Full backtrace below.
> >
> > $ mpv *.mp4
> > (+) Video --vid=1 (*) (h264 640x352 30.288fps)
> > (+) Audio --aid=1 (*) (aac 2ch 48000Hz)
> > libEGL warning: DRI3: Screen seems not DRI3 capable
> > AO: [sdl] 48000Hz stereo 2ch s32
> >
> > VO: [gpu] 640x352 yuv420p
> >
> > AV: 00:01:38 / 00:01:38 (100%) A-V: 0.000
> >
> >
> >
> >
> > Exiting... (End of file)
> >
> > pthread_mutex_destroy on mutex with waiters!
>
> > Segmentation fault (core dumped)
>
> POSIX says:
>
> "Attempting to destroy a locked mutex, or a mutex that another
> thread is attempting to lock, or a mutex that is being used in a
> pthread_cond_timedwait() or pthread_cond_wait() call by another
> thread, results in undefined behavior."
It is not clear if this is related. If it is, we don't know what changed
to make it happen. Also why this SIGSEV doesn't happen with other
outputs, or when playing music, even if the pthread_mutex_destroy message
is printed?
uvm_fault when setting ddb breakpoint on armv7 -current
Hello,
I am investigating a usb issue on my imx6-based novena, and I tried to
set a breakpoint to inspect the backtrace when the issue occurs. The
problem is, when resuming execution out of ddb, I get a uvm_fault and
then the only way forward is to reboot the system.
Am I missing a step ? or is it a bug ?
-> Kernel config
$ diff -Naur /usr/src/sys/arch/armv7/conf/{GENERIC,USBDEBUG}
--- /usr/src/sys/arch/armv7/conf/GENERICMon Dec 14 09:19:10 2020
+++ /usr/src/sys/arch/armv7/conf/USBDEBUG Sun Dec 13 17:26:09 2020
@@ -26,6 +26,11 @@
option USBVERBOSE
option USER_PCICONF# user-space PCI configuration
+option USB_DEBUG
+option UHUB_DEBUG
+option UMASS_DEBUG
+option EHCI_DEBUG
+
config bsd swap generic
# The main bus device
-> steps to reproduce over serial console
$ doas sysctl ddb.trigger=1
Stopped at db_enter: ldrbr15, [r15, r15, ror r15]!
ddb> break umass_bbb_reset
ddb> c
uvm_fault(0xc08d1260, c0659000, 2, 0) -> e
Fatal kernel mode data abort: 'Permission fault (L1)'
trapframe: 0xd0ccfcf8
DFSR=080d, DFAR=c06595b8, spsr=2013
r0 =00ff, r1 =c06595b8, r2 =, r3 =0002
r4 =c08e5164, r5 =c06595b8, r6 =d0ccfd91, r7 =0003
r8 =c083ee30, r9 =0004, r10=c06595b8, r11=d0ccfd88
r12=000f, ssp=d0ccfd48, slr=1060, pc =c04d69c0
Stopped at db_write_bytes+0x3ac: strbr0, [r5], #0x001
ddb> trace
db_write_bytes+0x3ac
rlv=0xc03973fc rfp=0xd0ccfda0
db_put_value+0x50
rlv=0xc0669cc0 rfp=0xd0ccfdb0
db_set_breakpoints+0x54
rlv=0xc072e670 rfp=0xd0ccfdd8
db_restart_at_pc+0x178
rlv=0xc06731c4 rfp=0xd0ccfe00
db_trap+0x14c
rlv=0xc04d6b18 rfp=0xd0ccfe20
db_trapper+0x88
rlv=0xc06f4734 rfp=0xd0ccfe50
undefinedinstruction+0x114
rlv=0xc05b5a68 rfp=0xd0ccfed8
$a.13
rlv=0xc04b1a18 rfp=0xd0ccff40
sys_sysctl+0x17c
rlv=0xc0427620 rfp=0xd0ccffa8
swi_handler+0x2e0
rlv=0xc05b5898 rfp=0xbffe1460
-> dmesg
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California. All rights reserved.
Copyrght (c) 1995-2020 OpenBSD. All rights reserved. https://www.OpenBS.org
OpenBSD 6.8-current (USBDEBUG) #2: Mon Dec 14 10:18:02 CET 2020
dermi...@derrida.kilob.yt:/usr/src/sys/arch/armv/compile/USBDEBUG
real mem = 3933511680 (3751MB)
avail mem = 3847270400 (3669MB)
random: good seed from bootblocks
mainbus0 at root: Kosagi Novena Dual/Quad
cpu0 at mainbus0 mpidr 0: ARM Cortex-A9 r2p10
cpu0: 32KB 32b/line 4-way L1 VIPT I-cache, 32KB 32b/line 4-way L1 D-cache
cortex0 at mainbus0
amptimer0 at cortex0: tick rate 396000 KHz
armliicc0 at cortex0: rtl 7 waymask: 0x000f
ampintc0 at mainbus0 nirq 160, ncpu 4: "interrupt-controller"
simplebus0 at mainbus0: "soc"
"dma-apbh" at simplebus0 not configured
"gpu" at simplebus0 not configured
"gpu" at simplebus0 not configured
"hdmi" at simplebus0 not configured
"timer" at simplebus0 not configured
"l2-cache" at simplebus0 not configured
"pcie" at simplebus0 not configured
"pmu" at simplebus0 not configured
simplebus1 at simplebus0: "aips-bus"
imxccm0 at simplebus1
imxanatop0 at simplebus1
syscon0 at simplebus1: "snvs"
imxrtc0 at syscon0
imxsrc0 at simplebus1
syscon1 at simplebus1: "iomuxc-gpr"
imxiomuxc0 at simplebus1
simplebus2 at simplebus1: "spba-bus"
"ssi" at simplebus2 not configured
"asrc" at simplebus2 not configured
"vpu" at simplebus1 not configured
"pwm" at simplebus1 not configured
"gpt" at simplebus1 not configured
imxgpio0 at simplebus1
imxgpio1 at simplebus1
imxgpio2 at simplebus1
imxgpio3 at simplebus1
imxgpio4 at simplebus1
imxgpio5 at simplebus1
imxgpio6 at simplebus1
"kpp" at simplebus1 not configured
imxdog0 at simplebus1
imxtemp0 at simplebus1
"usbphy" at simplebus1 not configured
"usbphy" at simplebus1 not configured
imxgpc0 at simplebus1
"ldb" at simplebus1 not configured
"sdma" at simplebus1 not configured
simplebus3 at simplebus0: "aips-bus"
syscon2 at simplebus3: "ocotp"
"caam" at simplebus3 not configured
imxehci0 at simplebus3
usb0 at imxehci0: USB revision 2.0
uhub0 at usb0 configuration 1 interface 0 "i.MX EHCI root hub" rev 2.00/1.00
addr 1
uhub0: 1 port with 1 removable, self powered
imxehci1 at simplebus3
usb1 at imxehci1: USB revision 2.0
uhub1 at usb1 configuration 1 interface 0 "i.MX EHCI root hub" rev 2.00/1.00
addr 1
uhub1: 1 port with 1 removable, self powered
"usbmisc" at simplebus3 not configured
fec0 at simplebus3
fec0: address 00:1f:11:02:17:de
ukphy0 at fec0 phy 7: Generic IEEE 802.3u media interface, rev. 1: OUI
0x000885, model 0x0021
imxesdhc0 at simplebus3
imxesdhc0: 198 MHz base clock
sdmmc0 at imxesdhc0: 4-bit, sd high-speed, mmc high-speed, dma
imxesdhc1 at simplebus3
imxesdhc1: 198 MHz base clock
sdmmc1 at imxesdhc1: 4-bit, sd high-speed, mmc high-speed, dma
imxiic0 at simplebus3
iic0 at imxiic0
"sbs,sbs-battery" at iic0 addr 0xb not configured
"kosagi,senoko" at iic0 addr 0x20 not configured
"st,stmp
Re: Kernel panic with i386 on latest snapshot
On Tue, Dec 15, 2020 at 06:57:03PM +0100, Mark Kettenis wrote:
> Does the diff below fix this?
I can reproduce the panic and your diff fixes it.
Usually my regress machines do not trigger it as I do not install
firmware. fw_update and reboot makes it crash.
bluhm
OpenBSD 6.8-current (GENERIC.MP) #0: Tue Dec 15 20:18:20 CET 2020
r...@ot2.obsd-lab.genua.de:/usr/src/sys/arch/i386/compile/GENERIC.MP
real mem = 2146910208 (2047MB)
avail mem = 2091409408 (1994MB)
panic: kernel diagnostic assertion "_kernel_lock_held()" failed: file
"/usr/src/sys/uvm/uvm_km.c", line 246
Stopped at db_enter+0x4: popl%ebp
TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
db_enter(d0e47604,d110ae68,d0e46ec4,d0e47fec,d0e47fec) at db_enter+0x4
panic(d0bc79e5,d0c2da24,d0c3a8c4,d0c522a9,f6) at panic+0xd3
__assert(d0c2da24,d0c522a9,f6,d0c3a8c4,d0e46ec4) at __assert+0x19
uvm_km_pgremove(d0e2da74,2552c000,2552e000) at uvm_km_pgremove+0x119
uvm_unmap_kill_entry(d0e47fec,d0e46ec4) at uvm_unmap_kill_entry+0x92
uvm_unmap_remove(d0e47fec,f552c000,f552e000,d110af10,0,1) at uvm_unmap_remove+0
x1cb
uvm_unmap(d0e47fec,f552c000,f552e000) at uvm_unmap+0x53
uvm_km_free(d0e47fec,f552c000,2000,2000) at uvm_km_free+0x25
cpu_ucode_setup(f092c000,f080,efff9000,150a416,1108000) at cpu_ucode_setup+
0xeb
cpu_startup(150a416,1108000,1117000,110b000,0) at cpu_startup+0x14a
main(0,0,0,0,0) at main+0x6b
https://www.openbsd.org/ddb.html describes the minimum info required in bug
reports. Insufficient info makes it difficult to find and fix bugs.
ddb{0}> boot reboot
rebooting...
OpenBSD 6.8-current (GENERIC.MP) #1: Tue Dec 15 20:34:36 CET 2020
r...@ot2.obsd-lab.genua.de:/usr/src/sys/arch/i386/compile/GENERIC.MP
real mem = 2146910208 (2047MB)
avail mem = 2091417600 (1994MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: date 10/20/04, BIOS32 rev. 0 @ 0xfdb30, SMBIOS rev. 2.3 @
0xf0640 (63 entries)
bios0: vendor American Megatrends Inc. version "0700xx" date 10/20/2004
bios0: Supermicro. X5DL8
acpi0 at bios0: ACPI 1.0
acpi0: sleep states S0 S1 S4 S5
acpi0: tables DSDT FACP APIC
acpi0: wakeup devices SLPB(S1) NRTH(S5) PS2M(S1) PS2K(S1) UAR1(S1) UAR2(S1)
USB_(S1) PCI1(S5) PCI2(S5) PCI3(S5) PCI4(S5)
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Xeon(TM) CPU 3.06GHz ("GenuineIntel" 686-class) 3.07 GHz,
0f-02-05
cpu0:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,CNXT-ID,xTPR,PERF,MELTDOWN
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 133MHz
cpu1 at mainbus0: apid 6 (application processor)
cpu1: Intel(R) Xeon(TM) CPU 3.06GHz ("GenuineIntel" 686-class) 3.07 GHz,
0f-02-05
cpu1:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,CNXT-ID,xTPR,PERF,MELTDOWN
cpu2 at mainbus0: apid 1 (application processor)
cpu2: Intel(R) Xeon(TM) CPU 3.06GHz ("GenuineIntel" 686-class) 3.07 GHz,
0f-02-05
cpu2:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,CNXT-ID,xTPR,PERF,MELTDOWN
cpu3 at mainbus0: apid 7 (application processor)
cpu3: Intel(R) Xeon(TM) CPU 3.06GHz ("GenuineIntel" 686-class) 3.07 GHz,
0f-02-05
cpu3:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,CNXT-ID,xTPR,PERF,MELTDOWN
ioapic0 at mainbus0: apid 8 pa 0xfec0, version 11, 16 pins
ioapic1 at mainbus0: apid 9 pa 0xfec01000, version 11, 16 pins
ioapic2 at mainbus0: apid 10 pa 0xfec02000, version 11, 16 pins
acpiprt0 at acpi0: bus 0 (NRTH)
acpiprt1 at acpi0: bus 1 (PCI1)
acpiprt2 at acpi0: bus 2 (PCI2)
acpiprt3 at acpi0: bus 4 (PCI3)
acpiprt4 at acpi0: bus 5 (PCI4)
acpibtn0 at acpi0: SLPB
"PNP0A03" at acpi0 not configured
acpicmos0 at acpi0
"PNP0A03" at acpi0 not configured
"PNP0A03" at acpi0 not configured
"PNP0A03" at acpi0 not configured
"PNP0A03" at acpi0 not configured
acpicpu0 at acpi0: C1(@1 halt!)
acpicpu1 at acpi0: C1(@1 halt!)
acpicpu2 at acpi0: C1(@1 halt!)
acpicpu3 at acpi0: C1(@1 halt!)
bios0: ROM list: 0xc/0x8000 0xc8000/0x1000 0xc9000/0x1800 0xca800/0x1800
pci0 at mainbus0 bus 0: configuration mode 1 (bios)
pchb0 at pci0 dev 0 function 0 "ServerWorks CNB20-HE Host" rev 0x33
pchb1 at pci0 dev 0 function 1 "ServerWorks CNB20-HE Host" rev 0x00
pci1 at pchb1 bus 1
em0 at pci1 dev 2 function 0 "Intel 82546GB" rev 0x03: apic 9 int 10, address
00:1b:21:55:eb:f4
em1 at pci1 dev 2 function 1 "Intel 82546GB" rev 0x03: apic 9 int 11, address
00:1b:21:55:eb:f5
bge0 at pci1 dev 3 function 0 "Broadcom BCM5703X" rev 0x02, BCM5702/5703 A2
(0x1002): apic 9 int 15, address 00:30:48:53:90:95
brgphy0 at bge0 phy 1: BCM5703 10/100/1000baseT PHY,
Re: Kernel panic with i386 on latest snapshot
On 15.12.2020. 18:57, Mark Kettenis wrote:
>> From: jungle Boogie
>> Date: Tue, 15 Dec 2020 08:07:04 -0800
>>
>> Hi All,
>>
>> On my i386 Toshiba netbook machine, I am getting a kernel panic with
>> the latest i386 snapshot.
>>
>> I hope this information helps someone with the issue.
>>
>>> show panic
>> kernel diagnostic assertion "_kernel_lock_held()" failed:
>> "/usr/src/sys/uvm/uvm_km.c", line 246
>>
>>> bt
>> db_enter(d0bc6fab,d0c2da31,d0c3a6bb,d0e36b7c,d0e36b7c) at db_enter+0x4
>> panic(d0bc6fab, d0c2da31, d0c3a6bb, d0c51f06, f6) at panic+0xd3
>> _assert(d0c2da31,d0c51f06,f6,d0c3a6bb,d0e71330) at _assert+0x19
>> uvm_km_pgremove(d0e578ec,2552c000,2553000) at uvm_km_pgremove+0x119
>> uvm_umap_kill_entry(d0e36b7c,d0e71330) at uvm_unmap_kill_entry+0x92
>> uvm_unmap(d0e36b7c,f552c000,f553) at uvm_unmap+0x53
>> uvm_km_free(d0e36b7c,f552c000,4000,4000) at uvm_km_free+0x25
>> cpu_ucode_setup(f092c000,f080,efff9000,8d565328,1107000) at
>> cpu_ucode_setup+0xeb
>> cpu_startup(8d565328,1107000,1116000,110a000,0) at cpu_startup+0x14a
>> main(0,0,0,0) at main+0x6b
>
> Does the diff below fix this?
>
Yes, it fixes panic.. tnx ..
OpenBSD 6.8-current (GENERIC.MP) #18: Tue Dec 15 19:36:39 CET 2020
hrv...@r620-2.srce.hr:/sys/arch/i386/compile/GENERIC.MP
real mem = 3441889280 (3282MB)
avail mem = 3362791424 (3207MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: date 12/06/19, BIOS32 rev. 0 @ 0xf14a0, SMBIOS rev.
2.7 @ 0xcf42c000 (99 entries)
bios0: vendor Dell Inc. version "2.9.0" date 12/06/2019
bios0: Dell Inc. PowerEdge R620
acpi0 at bios0: ACPI 3.0
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP APIC SPCR HPET DMAR MCFG WDAT SLIC ERST HEST
BERT EINJ TCPA PC__ SRAT SSDT
acpi0: wakeup devices PCI0(S5)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 4 (boot processor)
cpu0: Intel(R) Xeon(R) CPU E5-2643 v2 @ 3.50GHz ("GenuineIntel"
686-class) 3.61 GHz, 06-3e-04
cpu0:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 100MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.1, IBE
cpu1 at mainbus0: apid 6 (application processor)
cpu1: Intel(R) Xeon(R) CPU E5-2643 v2 @ 3.50GHz ("GenuineIntel"
686-class) 3.51 GHz, 06-3e-04
cpu1:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu2 at mainbus0: apid 8 (application processor)
cpu2: Intel(R) Xeon(R) CPU E5-2643 v2 @ 3.50GHz ("GenuineIntel"
686-class) 3.51 GHz, 06-3e-04
cpu2:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu3 at mainbus0: apid 16 (application processor)
cpu3: Intel(R) Xeon(R) CPU E5-2643 v2 @ 3.50GHz ("GenuineIntel"
686-class) 3.51 GHz, 06-3e-04
cpu3:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu4 at mainbus0: apid 18 (application processor)
cpu4: Intel(R) Xeon(R) CPU E5-2643 v2 @ 3.50GHz ("GenuineIntel"
686-class) 3.51 GHz, 06-3e-04
cpu4:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu5 at mainbus0: apid 20 (application processor)
cpu5: Intel(R) Xeon(R) CPU E5-2643 v2 @ 3.50GHz ("GenuineIntel"
686-class) 3.51 GHz, 06-3e-04
cpu5:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,
Re: Kernel panic with i386 on latest snapshot
> From: jungle Boogie
> Date: Tue, 15 Dec 2020 08:07:04 -0800
>
> Hi All,
>
> On my i386 Toshiba netbook machine, I am getting a kernel panic with
> the latest i386 snapshot.
>
> I hope this information helps someone with the issue.
>
> > show panic
> kernel diagnostic assertion "_kernel_lock_held()" failed:
> "/usr/src/sys/uvm/uvm_km.c", line 246
>
> > bt
> db_enter(d0bc6fab,d0c2da31,d0c3a6bb,d0e36b7c,d0e36b7c) at db_enter+0x4
> panic(d0bc6fab, d0c2da31, d0c3a6bb, d0c51f06, f6) at panic+0xd3
> _assert(d0c2da31,d0c51f06,f6,d0c3a6bb,d0e71330) at _assert+0x19
> uvm_km_pgremove(d0e578ec,2552c000,2553000) at uvm_km_pgremove+0x119
> uvm_umap_kill_entry(d0e36b7c,d0e71330) at uvm_unmap_kill_entry+0x92
> uvm_unmap(d0e36b7c,f552c000,f553) at uvm_unmap+0x53
> uvm_km_free(d0e36b7c,f552c000,4000,4000) at uvm_km_free+0x25
> cpu_ucode_setup(f092c000,f080,efff9000,8d565328,1107000) at
> cpu_ucode_setup+0xeb
> cpu_startup(8d565328,1107000,1116000,110a000,0) at cpu_startup+0x14a
> main(0,0,0,0) at main+0x6b
Does the diff below fix this?
Index: arch/i386/i386/ucode.c
===
RCS file: /cvs/src/sys/arch/i386/i386/ucode.c,v
retrieving revision 1.2
diff -u -p -r1.2 ucode.c
--- arch/i386/i386/ucode.c 28 Jun 2019 21:54:05 - 1.2
+++ arch/i386/i386/ucode.c 15 Dec 2020 17:54:37 -
@@ -102,7 +102,7 @@ cpu_ucode_setup(void)
size = round_page(bios_ucode->uc_size);
npages = size / PAGE_SIZE;
- va = uvm_km_valloc(kernel_map, size);
+ va = (vaddr_t)km_alloc(size, &kv_any, &kp_none, &kd_nowait);
if (va == 0)
return;
for (i = 0; i < npages; i++) {
@@ -119,7 +119,7 @@ cpu_ucode_setup(void)
pmap_remove(pmap_kernel(), va, va + size);
pmap_update(pmap_kernel());
- uvm_km_free(kernel_map, va, size);
+ km_free((void *)va, size, &kv_any, &kp_none);
}
/*
Re: sdmmc(4): sdmmc_io_function_enable(): don't sleep on lbolt
On Tue, Dec 15, 2020 at 01:47:24PM +0100, Mark Kettenis wrote:
> > Date: Tue, 15 Dec 2020 13:32:22 +0100
> > From: Claudio Jeker
> >
> > On Fri, Dec 11, 2020 at 07:07:56PM -0600, Scott Cheloha wrote:
> > > Hi,
> > >
> > > I'd like to remove lbolt from the kernel. I think having it in the
> > > kernel complicates otherwise simple code.
> > >
> > > We can start with sdmmc(4).
> > >
> > > The goal in sdmmc_io_function_enable() is calling
> > > sdmmc_io_function_ready()
> > > up to six times and sleep 1 second between each attempt. Here's rewritten
> > > code that does with without lbolt.
> > >
> > > ok?
> > >
> > > Index: sdmmc_io.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_io.c,v
> > > retrieving revision 1.41
> > > diff -u -p -r1.41 sdmmc_io.c
> > > --- sdmmc_io.c31 Dec 2019 10:05:33 - 1.41
> > > +++ sdmmc_io.c12 Dec 2020 01:04:59 -
> > > @@ -231,8 +231,8 @@ sdmmc_io_function_enable(struct sdmmc_fu
> > > {
> > > struct sdmmc_softc *sc = sf->sc;
> > > struct sdmmc_function *sf0 = sc->sc_fn0;
> > > + int chan, retry = 5;
> > > u_int8_t rv;
> > > - int retry = 5;
> > >
> > > rw_assert_wrlock(&sc->sc_lock);
> > >
> > > @@ -244,7 +244,7 @@ sdmmc_io_function_enable(struct sdmmc_fu
> > > sdmmc_io_write_1(sf0, SD_IO_CCCR_FN_ENABLE, rv);
> > >
> > > while (!sdmmc_io_function_ready(sf) && retry-- > 0)
> > > - tsleep_nsec(&lbolt, PPAUSE, "pause", INFSLP);
> > > + tsleep_nsec(&chan, PPAUSE, "pause", SEC_TO_NSEC(1));
> > > return (retry >= 0) ? 0 : ETIMEDOUT;
> > > }
> > >
> >
> > Why not use &retry as wait channel instead of adding a new variable
> > chan? Result is the same. Would it make sense to allow NULL as wait
> > channel to make the tsleep not wakeable. At least that could be used in a
> > few places where timeouts are implemented with tsleep and would make the
> > intent more obvious.
>
> Or have an appropriately named global variable? Something like "int nowake"?
I like this. Brief aside into other BSDs:
--
FreeBSD and NetBSD call this operation a "pause" instead of a "sleep".
The idea is that a sleeping thread can be woken up with e.g.
wakeup(9) but that a paused thread cannot be awoken in this way.
Paused threads can still be interrupted with a signal.
NetBSD has kpause(9):
https://man.netbsd.org/kpause.9
FreeBSD has a whole bunch of pause interfaces:
https://www.freebsd.org/cgi/man.cgi?query=pause&sektion=9&manpath=FreeBSD+12.2-RELEASE+and+Ports
It kind-of sounds like what we want. From that page:
> The pause() function is a wrapper around tsleep() that suspends
> execution of the current thread for the indicated timeout. The
> thread can not be awakened early by signals or calls to wakeup(),
> wakeup_one() or wakeup_any(). The pause_sig() function is a variant
> of pause() which can > be awakened early by signals.
FreeBSD implements it with a special per-CPU pause channel.
Look at FreeBSD's _sleep():
https://github.com/freebsd/freebsd/blob/d551da60d42039156f003de6644e9e147ed167a3/sys/kern/kern_synch.c#L173
--
So with that in mind, my thought is to start with a global "int pause"
channel that we all collectively agree not to pass to wakeup(9). We
can advance the concept more if need be.
I'm happy to fuss with the name.
int pause_chan?
Re: Lock operations for knote lists
On Tue, Dec 15, 2020 at 09:58:16AM -0300, Martin Pieuchot wrote:
> On 11/12/20(Fri) 17:37, Visa Hankala wrote:
> > Index: kern/kern_event.c
> > ===
> > RCS file: src/sys/kern/kern_event.c,v
> > retrieving revision 1.147
> > diff -u -p -r1.147 kern_event.c
> > --- kern/kern_event.c 9 Dec 2020 18:58:19 - 1.147
> > +++ kern/kern_event.c 11 Dec 2020 17:05:09 -
> > @@ -1539,9 +1576,14 @@ klist_invalidate(struct klist *list)
> > NET_ASSERT_UNLOCKED();
> >
> > s = splhigh();
> > + ls = klist_lock(list);
>
> Isn't splhigh() redundant with klist_lock() now? If a subsystem
> provides its own lock/unlock routine shouldn't it ensure that the
> necessary SPL is used? Or is this protecting something else? Or is
> it just paranoia and we should try to remove it in a later step?
splhigh() is needed by knote_acquire(). The code will change when
a mutex is added to it.
> > while ((kn = SLIST_FIRST(&list->kl_list)) != NULL) {
> > - if (!knote_acquire(kn))
> > + if (!knote_acquire(kn, list, ls)) {
> > + /* knote_acquire() has unlocked list. */
> > + ls = klist_lock(list);
> > continue;
> > + }
> > + klist_unlock(list, ls);
> > splx(s);
> > kn->kn_fop->f_detach(kn);
> > if (kn->kn_fop->f_flags & FILTEROP_ISFD) {
>
Re: Switch select(2) to kqueue-based implementation
On Tue, Dec 15, 2020 at 07:46:01AM -0300, Martin Pieuchot wrote:
> @@ -636,43 +651,59 @@ dopselect(struct proc *p, int nd, fd_set
> if (sigmask)
> dosigsuspend(p, *sigmask &~ sigcantmask);
>
> -retry:
> - ncoll = nselcoll;
> - atomic_setbits_int(&p->p_flag, P_SELECT);
> - error = selscan(p, pibits[0], pobits[0], nd, ni, retval);
> - if (error || *retval)
> + /* Register kqueue events */
> + if ((error = pselregister(p, pibits[0], nd, ni, &nevents) != 0))
> goto done;
The above has parentheses wrong and returns 1 (EPERM) as error.
The lines should read:
if ((error = pselregister(p, pibits[0], nd, ni, &nevents)) != 0)
goto done;
In addition to the above, I noticed that select(2) behaves differently
than before when a file descriptor that is being monitored is closed by
another thread. The old implementation returns EBADF. The new code keeps
on waiting on the underlying object.
The diff below makes kqueue clear kqpoll's fd event registration on
fd close. However, it does not make select(2) return an error, the fd
just will not cause a wakeup any longer. I think I have an idea on how
to correct that but I need to consider it some more.
Index: kern/kern_event.c
===
RCS file: src/sys/kern/kern_event.c,v
retrieving revision 1.148
diff -u -p -r1.148 kern_event.c
--- kern/kern_event.c 15 Dec 2020 04:48:18 - 1.148
+++ kern/kern_event.c 15 Dec 2020 16:45:33 -
@@ -168,12 +168,17 @@ KQREF(struct kqueue *kq)
void
KQRELE(struct kqueue *kq)
{
- struct filedesc *fdp;
-
if (atomic_dec_int_nv(&kq->kq_refs) > 0)
return;
- fdp = kq->kq_fdp;
+ kqueue_free(kq);
+}
+
+void
+kqueue_free(struct kqueue *kq)
+{
+ struct filedesc *fdp = kq->kq_fdp;
+
if (rw_status(&fdp->fd_lock) == RW_WRITE) {
LIST_REMOVE(kq, kq_next);
} else {
@@ -182,12 +187,6 @@ KQRELE(struct kqueue *kq)
fdpunlock(fdp);
}
- kqueue_free(kq);
-}
-
-void
-kqueue_free(struct kqueue *kq)
-{
free(kq->kq_knlist, M_KEVENT, kq->kq_knlistsize *
sizeof(struct knlist));
hashfree(kq->kq_knhash, KN_HASHSIZE, M_KEVENT);
@@ -509,12 +508,17 @@ void
kqpoll_init(void)
{
struct proc *p = curproc;
+ struct filedesc *fdp;
if (p->p_kq != NULL)
return;
p->p_kq = kqueue_alloc(p->p_fd);
p->p_kq_serial = arc4random();
+ fdp = p->p_fd;
+ fdplock(fdp);
+ LIST_INSERT_HEAD(&fdp->fd_kqlist, p->p_kq, kq_next);
+ fdpunlock(fdp);
}
void
Re: Kernel panic with i386 on latest snapshot
On 15.12.2020. 17:07, jungle Boogie wrote:
> Hi All,
>
> On my i386 Toshiba netbook machine, I am getting a kernel panic with
> the latest i386 snapshot.
>
> I hope this information helps someone with the issue.
>
>> show panic
> kernel diagnostic assertion "_kernel_lock_held()" failed:
> "/usr/src/sys/uvm/uvm_km.c", line 246
>
>> bt
> db_enter(d0bc6fab,d0c2da31,d0c3a6bb,d0e36b7c,d0e36b7c) at db_enter+0x4
> panic(d0bc6fab, d0c2da31, d0c3a6bb, d0c51f06, f6) at panic+0xd3
> _assert(d0c2da31,d0c51f06,f6,d0c3a6bb,d0e71330) at _assert+0x19
> uvm_km_pgremove(d0e578ec,2552c000,2553000) at uvm_km_pgremove+0x119
> uvm_umap_kill_entry(d0e36b7c,d0e71330) at uvm_unmap_kill_entry+0x92
> uvm_unmap(d0e36b7c,f552c000,f553) at uvm_unmap+0x53
> uvm_km_free(d0e36b7c,f552c000,4000,4000) at uvm_km_free+0x25
> cpu_ucode_setup(f092c000,f080,efff9000,8d565328,1107000) at
> cpu_ucode_setup+0xeb
> cpu_startup(8d565328,1107000,1116000,110a000,0) at cpu_startup+0x14a
> main(0,0,0,0) at main+0x6b
>
> I've had no panics on amd64.
>
> Thanks!
>
Hi,
i can confirm this panic on i386 on Dell R620
OpenBSD 6.8-current (GENERIC.MP) #561: Tue Dec 15 03:03:30 MST 2020
dera...@i386.openbsd.org:/usr/src/sys/arch/i386/compile/GENERIC.MP
real mem = 3441889280 (3282MB)
avail mem = 3362799616 (3207MB)
panic: kernel diagnostic assertion "_kernel_lock_held()" failed: file
"/usr/src$
sys/uvm/uvm_km.c", line 246
Stopped at db_enter+0x4: popl%ebp
TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
db_enter(d0e4901c,d1109e68,d0e48600,d0e2f5e0,d0e2f5e0) at db_enter+0x4
panic(d0bc86cd,d0c2d366,d0c3a01b,d0c508c6,f6) at panic+0xd3
__assert(d0c2d366,d0c508c6,f6,d0c3a01b,d0e48600) at __assert+0x19
uvm_km_pgremove(d0e2edbc,2552c000,2553) at uvm_km_pgremove+0x119
uvm_unmap_kill_entry(d0e2f5e0,d0e48600) at uvm_unmap_kill_entry+0x92
uvm_unmap_remove(d0e2f5e0,f552c000,f553,d1109f10,0,1) at
uvm_unmap_remove+0x1cb
uvm_unmap(d0e2f5e0,f552c000,f553) at uvm_unmap+0x53
uvm_km_free(d0e2f5e0,f552c000,4000,4000) at uvm_km_free+0x25
cpu_ucode_setup(f092c000,f080,efff9000,68cac0bc,1107000) at
cpu_ucode_setup+0xeb
cpu_startup(68cac0bc,1107000,1116000,110a000,0) at cpu_startup+0x14a
main(0,0,0,0,0) at main+0x6b
https://www.openbsd.org/ddb.html describes the minimum info required in
bug reports. Insufficient info makes it difficult to find and fix bugs.
ddb{0}>
ddb{0}> show panic
kernel diagnostic assertion "_kernel_lock_held()" failed: file
"/usr/src/sys/uvm/uvm_km.c", line 246
ddb{0}> trace
db_enter(d0e4901c,d1109e68,d0e48600,d0e2f5e0,d0e2f5e0) at db_enter+0x4
panic(d0bc86cd,d0c2d366,d0c3a01b,d0c508c6,f6) at panic+0xd3
__assert(d0c2d366,d0c508c6,f6,d0c3a01b,d0e48600) at __assert+0x19
uvm_km_pgremove(d0e2edbc,2552c000,2553) at uvm_km_pgremove+0x119
uvm_unmap_kill_entry(d0e2f5e0,d0e48600) at uvm_unmap_kill_entry+0x92
uvm_unmap_remove(d0e2f5e0,f552c000,f553,d1109f10,0,1) at
uvm_unmap_remove+0x1cb
uvm_unmap(d0e2f5e0,f552c000,f553) at uvm_unmap+0x53
uvm_km_free(d0e2f5e0,f552c000,4000,4000) at uvm_km_free+0x25
cpu_ucode_setup(f092c000,f080,efff9000,68cac0bc,1107000) at
cpu_ucode_setup+0xeb
cpu_startup(68cac0bc,1107000,1116000,110a000,0) at cpu_startup+0x14a
main(0,0,0,0,0) at main+0x6b
ddb{0}>
ddb{0}> show reg
ds 0x10
es 0x10
fs 0x20
gs 0
edi 0xd0bc86cdacx100_txpower_maxim+0xc67c
esi0x100
ebp 0xd1109e40__kernel_bss_end+0x1c8e40
ebx 0xd1109e68__kernel_bss_end+0x1c8e68
edx0x2fd
ecx0
eax 0x1
eip 0xd05c05b4db_enter+0x4
cs 0x8
eflags 0x2
esp 0xd1109e40__kernel_bss_end+0x1c8e40
ss 0x10
db_enter+0x4: popl%ebp
ddb{0}>
Re: acme-client(1) make -F flag use more obvious
On 15 December 2020 14:56:41 CET, Stuart Henderson wrote: >On 2020/12/15 10:18, Solene Rapenne wrote: >> This is a small change to acme-client(1) because I find >> the explanation of -F flag not being obvious that you >> need it when you add/remove an alternative name in your >> domain config. > >This only works directly for adding. For removal you need to rm >the existing certificate. -F only handles forced renewals correctly. That it can be (ab)used to add subject alt names to a cert is an implementation detail. It would be nice if someone™ would fix this properly by acme-client detecting that cert and config do not agree anymore. -- Sent from a mobile device. Please excuse poor formating.
diff: mbuf-chains for tcp/ip/ether_ouput
Hi,
I took on old idea and diff [1] from bluhm and finished it for the IPv4
case.
This change uses mbuf chains in tcp/ip/ether_output instead of single
mbufs. This approach takes lesser processing power per packets, which
we want to send out. In several code path it optimises the
processing because we just have to look at the first mbuf of a chain.
In other cases we process several packets in a row for one step, which
saves context switches between functions.
Thus, we gain an performance improvement of around 36% with relayd(8).
The measurements were made with iperf and with 30 streams:
2823.0 MBit/s current
3844.4 MBit/s current + mbuf-chaining diff below
ip6_output() is not implemented yet. I would do that in a followup diff
later, if this one is successful.
I tested the diff on amd64 in several testing environments without any
problem.
Whats your opinion about this change?
bye,
Jan
[1]: https://github.com/bluhm/sys/tree/tcp-outlist
Index: net/fq_codel.c
===
RCS file: /cvs/src/sys/net/fq_codel.c,v
retrieving revision 1.14
diff -u -p -r1.14 fq_codel.c
--- net/fq_codel.c 10 Dec 2020 06:53:38 - 1.14
+++ net/fq_codel.c 15 Dec 2020 09:37:36 -
@@ -138,6 +138,7 @@ void fqcodel_purge(struct fqcodel *, s
static const struct ifq_ops fqcodel_ops = {
fqcodel_idx,
fqcodel_if_enq,
+ NULL,
fqcodel_if_deq_begin,
fqcodel_if_deq_commit,
fqcodel_if_purge,
Index: net/hfsc.c
===
RCS file: /cvs/src/sys/net/hfsc.c,v
retrieving revision 1.48
diff -u -p -r1.48 hfsc.c
--- net/hfsc.c 22 Oct 2018 23:44:53 - 1.48
+++ net/hfsc.c 15 Dec 2020 09:37:36 -
@@ -270,6 +270,7 @@ void hfsc_free(unsigned int, void *);
const struct ifq_ops hfsc_ops = {
hfsc_idx,
hfsc_enq,
+ NULL,
hfsc_deq_begin,
hfsc_deq_commit,
hfsc_purge,
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.620
diff -u -p -r1.620 if.c
--- net/if.c3 Oct 2020 00:23:55 - 1.620
+++ net/if.c15 Dec 2020 09:37:36 -
@@ -633,6 +633,8 @@ if_attach_common(struct ifnet *ifp)
ifp->if_rtrequest = if_rtrequest_dummy;
if (ifp->if_enqueue == NULL)
ifp->if_enqueue = if_enqueue_ifq;
+ if (ifp->if_enqueue_ml == NULL)
+ ifp->if_enqueue_ml = if_enqueue_ifq_ml;
ifp->if_llprio = IFQ_DEFPRIO;
}
@@ -682,35 +684,72 @@ if_qstart_compat(struct ifqueue *ifq)
int
if_enqueue(struct ifnet *ifp, struct mbuf *m)
{
+ struct mbuf_list ml = MBUF_LIST_INITIALIZER();
+
+ ml_enqueue(&ml, m);
+ return (if_enqueue_ml(ifp, &ml));
+}
+
+int
+if_enqueue_ml(struct ifnet *ifp, struct mbuf_list *ml)
+{
+ int error;
+ struct mbuf *m;
+
+ while ((m = ml_dequeue(ml)) != NULL) {
#if NPF > 0
- if (m->m_pkthdr.pf.delay > 0)
- return (pf_delay_pkt(m, ifp->if_index));
+ if (m->m_pkthdr.pf.delay > 0) {
+ error = pf_delay_pkt(m, ifp->if_index);
+ if (error != 0)
+ goto bad;
+ continue;
+ }
#endif
#if NBRIDGE > 0
- if (ifp->if_bridgeidx && (m->m_flags & M_PROTO1) == 0) {
- int error;
-
- error = bridge_enqueue(ifp, m);
- return (error);
- }
+ if (ifp->if_bridgeidx && (m->m_flags & M_PROTO1) == 0) {
+ error = bridge_enqueue(ifp, m);
+ if (error != 0)
+ goto bad;
+ continue;
+ }
#endif
#if NPF > 0
- pf_pkt_addr_changed(m);
+ pf_pkt_addr_changed(m);
#endif /* NPF > 0 */
- return ((*ifp->if_enqueue)(ifp, m));
+ error = (*ifp->if_enqueue)(ifp, m);
+ if (error != 0)
+ goto bad;
+ }
+
+ return 0;
+ bad:
+ ml_purge(ml);
+ return error;
}
int
if_enqueue_ifq(struct ifnet *ifp, struct mbuf *m)
{
+ struct mbuf_list ml = MBUF_LIST_INITIALIZER();
+
+ ml_enqueue(&ml, m);
+ return (if_enqueue_ifq_ml(ifp, &ml));
+}
+
+int
+if_enqueue_ifq_ml(struct ifnet *ifp, struct mbuf_list *ml)
+{
struct ifqueue *ifq = &ifp->if_snd;
int error;
if (ifp->if_nifqs > 1) {
unsigned int idx;
+ struct mbuf *m;
+
+ m = MBUF_LIST_FIRST(ml);
/*
* use the operations on the first ifq to pick which of
@@ -721,13 +760,16 @@ if_enqueue_ifq(struct ifnet *ifp, struct
ifq = ifp->if_ifqs[idx];
}
- error = ifq_enqueue(ifq, m);
- if (error)
- return
Kernel panic with i386 on latest snapshot
Hi All, On my i386 Toshiba netbook machine, I am getting a kernel panic with the latest i386 snapshot. I hope this information helps someone with the issue. > show panic kernel diagnostic assertion "_kernel_lock_held()" failed: "/usr/src/sys/uvm/uvm_km.c", line 246 > bt db_enter(d0bc6fab,d0c2da31,d0c3a6bb,d0e36b7c,d0e36b7c) at db_enter+0x4 panic(d0bc6fab, d0c2da31, d0c3a6bb, d0c51f06, f6) at panic+0xd3 _assert(d0c2da31,d0c51f06,f6,d0c3a6bb,d0e71330) at _assert+0x19 uvm_km_pgremove(d0e578ec,2552c000,2553000) at uvm_km_pgremove+0x119 uvm_umap_kill_entry(d0e36b7c,d0e71330) at uvm_unmap_kill_entry+0x92 uvm_unmap(d0e36b7c,f552c000,f553) at uvm_unmap+0x53 uvm_km_free(d0e36b7c,f552c000,4000,4000) at uvm_km_free+0x25 cpu_ucode_setup(f092c000,f080,efff9000,8d565328,1107000) at cpu_ucode_setup+0xeb cpu_startup(8d565328,1107000,1116000,110a000,0) at cpu_startup+0x14a main(0,0,0,0) at main+0x6b I've had no panics on amd64. Thanks!
Re: bgpd send side hold timer
On Mon, Dec 14, 2020 at 06:22:09PM +, Job Snijders wrote:
> Hi all,
>
> This patch appears to be a very elegant solution to a thorny subtle
> problem: what to do when a peer is not accepting new routing information
> from you?
One thing I'm unsure about is the value of the SendHold timer. I reused
the hold timer value with the assumption that for dead connections the
regular hold timer expires before the SendHold timer (the send buffer
needs to be full before send starts blocking).
People should look out for cases where the SendHold timer triggered before
either a NOTIFICATION form the peer arrived or where the SendHold timer
triggered before the HoldTimer. Now that may be tricky since both SendHold
and Hold timer trigger the same EVNT_TIMER_HOLDTIME event so they can not
be distinguished easily.
I think that the SendHold timer will almost never trigger and if it does
only for the case where a session only works in one direction.
> I've seen in the wild that some crashed BGP implementations continue to
> be able to generate KEEPALIVE messages, and are able to TCP ACK
> keepalives you are sending, but won't actually accept anything you send
> to such a problematic peer. A red flag is when the peer keeps telling
> you not to send it anything by signalling a TCP Receive Window of 0.
>
> The consequences are quite dire: in situations like these you know for a
> fact that the peer is operating based on stale routing information (you
> have proof they are not accepting your KEEPALIVEs and more importantly
> UPDATEs, as those are all stuck in your OutQ).
>
> If a peer is not progressing new routing information coming from you,
> how can we trust it was processing any updates coming from other
> neighbors? Are the routes the peer told us in the past (what we have in
> Adj-RIB-In) still valid? Seems unlikely to me... it seems safer to
> destroy the BGP-4 session, log an error, and generate WITHDRAW messages
> for all routes pointing towards the broken peer so the network can
> converge to healthier paths.
>
> IDR discussions here
>
> https://mailarchive.ietf.org/arch/msg/idr/L9nWFBpW0Tci0c9DGfMoqC1j_sA/
>
> OK job@
>
> Kind regards,
>
> Job
>
> On Mon, Dec 14, 2020 at 06:45:47PM +0100, Claudio Jeker wrote:
> > The BGP protocol has a keepalive packet which resets the hold timer when a
> > packet is received. The problem is this covers only one side of the
> > transmission. It seems that some BGP implementations fail to process
> > messages in some cases but still send out KEEPALIVE packets. So bgpd
> > thinks everything is fine even though no updates where processed by the
> > other side (including our KEEPALIVE packets). The session is stuck in
> > limbo and with it some prefixes and routes.
> >
> > Because of this I think it makes sense to add a send hold timer that is
> > reset whenever a write call to the socket is made. If a socket does not
> > become writable for holdtime seconds (90s by default) then the session is
> > reset similar to the hold timer expiring because no data was received.
> >
> > This send holdtimer is not part of the BGP spec right now but looking at
> > discussions on the IDR mailing list I assume something like this may be
> > added at one point.
> >
> > I would like to know what other people think and would especially like to
> > know if this diff causes session resets that should not happen.
> >
> > Cheers
> > --
> > :wq Claudio
> >
> >
> > Index: bgpd.h
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> > retrieving revision 1.405
> > diff -u -p -r1.405 bgpd.h
> > --- bgpd.h 5 Nov 2020 11:52:59 - 1.405
> > +++ bgpd.h 13 Dec 2020 14:56:47 -
> > @@ -1467,6 +1467,7 @@ static const char * const timernames[] =
> > "ConnectRetryTimer",
> > "KeepaliveTimer",
> > "HoldTimer",
> > + "SendHoldTimer",
> > "IdleHoldTimer",
> > "IdleHoldResetTimer",
> > "CarpUndemoteTimer",
> > Index: session.c
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> > retrieving revision 1.406
> > diff -u -p -r1.406 session.c
> > --- session.c 11 Dec 2020 12:00:01 - 1.406
> > +++ session.c 13 Dec 2020 14:52:42 -
> > @@ -373,6 +373,7 @@ session_main(int debug, int verbose)
> > if ((pt = timer_nextisdue(&p->timers, now)) != NULL) {
> > switch (pt->type) {
> > case Timer_Hold:
> > + case Timer_HoldSend:
> > bgp_fsm(p, EVNT_TIMER_HOLDTIME);
> > break;
> > case Timer_ConnectRetry:
> > @@ -597,6 +598,7 @@ bgp_fsm(struct peer *peer, enum session_
> > switch (event) {
> > case EVNT_START:
> > timer_stop(&peer->timers, Timer_Hold);
> > + time
Re: SIGSEGV in _rthread_tls_destructors()
> Date: Tue, 15 Dec 2020 12:15:30 -0300
> From: Martin Pieuchot
>
> When the first thread of multimedia/mpv exits after having played a video
> with the "gpu" (default) output, the programs receives a SIGSEV when it
> tries to execute one of its destructor:
>
> void
> _rthread_tls_destructors(pthread_t thread)
> {
> [...]
> for (i = 0; i < PTHREAD_DESTRUCTOR_ITERATIONS; i++) {
> for (rs = thread->local_storage; rs; rs = rs->next) {
> if (!rs->data)
> continue;
> if (rkeys[rs->keyid].destructor) {
> void (*destructor)(void *) =
> rkeys[rs->keyid].destructor;
> void *data = rs->data;
> rs->data = NULL;
> _spinunlock(&rkeyslock);
> destructor(data); <-- HERE
> _spinlock(&rkeyslock);
> }
> }
> }
> [...]
> }
>
> This doesn't happen with other outputs and I haven't checked/don't know
> which piece of code in the "gpu" output calls pthread_key_create().
>
> Full backtrace below.
>
> $ mpv *.mp4
> (+) Video --vid=1 (*) (h264 640x352 30.288fps)
> (+) Audio --aid=1 (*) (aac 2ch 48000Hz)
> libEGL warning: DRI3: Screen seems not DRI3 capable
> AO: [sdl] 48000Hz stereo 2ch s32
>
> VO: [gpu] 640x352 yuv420p
>
> AV: 00:01:38 / 00:01:38 (100%) A-V: 0.000
>
>
>
>
> Exiting... (End of file)
>
> pthread_mutex_destroy on mutex with waiters!
> Segmentation fault (core dumped)
POSIX says:
"Attempting to destroy a locked mutex, or a mutex that another
thread is attempting to lock, or a mutex that is being used in a
pthread_cond_timedwait() or pthread_cond_wait() call by another
thread, results in undefined behavior."
SIGSEGV in _rthread_tls_destructors()
When the first thread of multimedia/mpv exits after having played a video
with the "gpu" (default) output, the programs receives a SIGSEV when it
tries to execute one of its destructor:
void
_rthread_tls_destructors(pthread_t thread)
{
[...]
for (i = 0; i < PTHREAD_DESTRUCTOR_ITERATIONS; i++) {
for (rs = thread->local_storage; rs; rs = rs->next) {
if (!rs->data)
continue;
if (rkeys[rs->keyid].destructor) {
void (*destructor)(void *) =
rkeys[rs->keyid].destructor;
void *data = rs->data;
rs->data = NULL;
_spinunlock(&rkeyslock);
destructor(data); <-- HERE
_spinlock(&rkeyslock);
}
}
}
[...]
}
This doesn't happen with other outputs and I haven't checked/don't know
which piece of code in the "gpu" output calls pthread_key_create().
Full backtrace below.
$ mpv *.mp4
(+) Video --vid=1 (*) (h264 640x352 30.288fps)
(+) Audio --aid=1 (*) (aac 2ch 48000Hz)
libEGL warning: DRI3: Screen seems not DRI3 capable
AO: [sdl] 48000Hz stereo 2ch s32
VO: [gpu] 640x352 yuv420p
AV: 00:01:38 / 00:01:38 (100%) A-V: 0.000
Exiting... (End of file)
pthread_mutex_destroy on mutex with waiters!
Segmentation fault (core dumped)
mpi@oliva $ egdb /usr/local/bin/mpv *.core
GNU gdb (GDB) 7.12.1
Copyright (C) 2017 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-unknown-openbsd6.8".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
Re: acme-client(1) make -F flag use more obvious
On 2020/12/15 10:18, Solene Rapenne wrote: > This is a small change to acme-client(1) because I find > the explanation of -F flag not being obvious that you > need it when you add/remove an alternative name in your > domain config. This only works directly for adding. For removal you need to rm the existing certificate.
Re: acme-client(1) make -F flag use more obvious
On Tue, Dec 15, 2020 at 10:18:41AM +0100, Solene Rapenne wrote: > This is a small change to acme-client(1) because I find > the explanation of -F flag not being obvious that you > need it when you add/remove an alternative name in your > domain config. > > Maybe wording could be better, if a native English > speaker could give it a look. > > ok? > hi. i think your text reads fine. perhaps "domain's alternative names" (with an apostrophe) would be better, but otherwise ok/ jmc > Index: acme-client.1 > === > RCS file: /home/reposync/src/usr.sbin/acme-client/acme-client.1,v > retrieving revision 1.36 > diff -u -p -r1.36 acme-client.1 > --- acme-client.1 4 Nov 2020 10:34:18 - 1.36 > +++ acme-client.1 15 Dec 2020 09:14:07 - > @@ -68,6 +68,9 @@ The options are as follows: > .Bl -tag -width Ds > .It Fl F > Force certificate renewal, even if it's too soon. > +This is required if the domain alternatives names changed > +in > +.Xr acme-client.conf 5 . > .It Fl f Ar configfile > Specify an alternative configuration file. > .It Fl n >
Re: small typo in imsg_init.3
On Sat, Dec 12, 2020 at 09:20:26PM -0500, Aisha Tammy wrote: > Hi, > ?? While creating a portable version of imsg, I noticed > a small typo in the imsg_init.3 man page which says > the returned value is 'len' instead of 'datalen'. > Attached the patch to fix it. > > OK? > fixed, thanks. jmc > Cheers, > Aisha > > diff --git a/lib/libutil/imsg_init.3 b/lib/libutil/imsg_init.3 > index 18720d1d59b..e7dc6ab928d 100644 > --- a/lib/libutil/imsg_init.3 > +++ b/lib/libutil/imsg_init.3 > @@ -186,7 +186,7 @@ appends to > bytes of ancillary data pointed to by > .Fa data . > It returns > -.Fa len > +.Fa datalen > if it succeeds, \-1 otherwise. > .Pp > .Fn imsg_close >
Re: fortune: allow to use symlinks
On Tue, 2020-12-15 at 15:10 +0300, Vadim Zhukov wrote:
> > I very rarely use fortune and I'm not familiar with the codebase, but
> > from some quick testing and code-scanning I found the following:
> > is_fortfile appends .dat to the input file and sees if it's accessible.
> > Adding a symlink to the corresponding .dat file make the thing work
> > perfectly well for me. Your diff seems to work, because you resolve
> > the path, which in turn gives back the correct location for the .dat,
> > not because symlinks are not suported.
>
> Thank you very much for review. Indeed, adding second symlink fixes
> the initial issue, so the patch now seems useless...
>
> > Personally I don't see good reason to add the additional logic which
> > can be solved with a second simple symlink, but if others think it's
> > worth it, comments inline.
>
> ... but following your comments I've discovered that copy() function
> in fortune.c can return NULL, and that value isn't checked.
>
> So I'm posting the whole diff for the sake of completeness, and what
> I really think worths committing is the last chunk, replacing "return
> NULL" with "err(1, NULL)" in copy(). Okay for it?
OK martijn@ for this last bit.
>
> > martijn@
> >
> > On Tue, 2020-12-15 at 01:09 +0300, Vadim Zhukov wrote:
> > > Hello, all.
> > >
> > > This allows fortune(6) to open fortune files via symlinks.
> > > Maybe this is a stupid idea, I dunno, but it seems inconsistent
> > > that I can't put a symlink to my fortunes collection in
> > > /usr/share/games/fortune and then simply call "fortune foo"
> > > instead of "fortune /usr/local/share/games/fortune/ru/foo".
> > >
> > > I decided to call readlink(2) explicitly rather than adding
> > > check of file type being open, since that resulted in less code.
> > >
> > > Ah, and yes, I have a port of Russian fortune files pending,
> > > but that's for another list and another day.
> > >
> > > So... okay/notokay anyone? :)
>
> --
> WBR,
> Vadim Zhukov
>
>
> Index: fortune.c
> ===
> RCS file: /cvs/src/games/fortune/fortune/fortune.c,v
> retrieving revision 1.60
> diff -u -p -r1.60 fortune.c
> --- fortune.c 10 Aug 2017 17:00:08 - 1.60
> +++ fortune.c 15 Dec 2020 11:46:00 -
> @@ -39,6 +39,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -388,8 +389,9 @@ add_file(int percent, char *file, char *
> FILEDESC *parent)
> {
> FILEDESC*fp;
> + ssize_t lnklen;
> int fd;
> - char*path, *offensive;
> + char*path, *offensive, lnkpath[PATH_MAX];
> boolwas_malloc;
> boolisdir;
>
> @@ -420,8 +422,22 @@ add_file(int percent, char *file, char *
> }
>
> DPRINTF(1, (stderr, "adding file \"%s\"\n", path));
> +
> over:
> + lnklen = readlink(path, lnkpath, sizeof(lnkpath));
> + if (lnklen == -1) {
> + if (errno != EINVAL)
> + goto openfailed;
> + } else {
> + lnkpath[lnklen] = '\0';
> + if (was_malloc)
> + free(path);
> + path = copy(lnkpath, NULL);
> + was_malloc = 1;
> + }
> +
> if ((fd = open(path, O_RDONLY)) < 0) {
> +openfailed:
> /*
> * This is a sneak. If the user said -a, and if the
> * file we're given isn't a file, we check to see if
> @@ -718,7 +734,7 @@ copy(char *str, char *suf)
> char*new;
>
> if (asprintf(&new, "%s%s", str, suf ? suf : "") == -1)
> - return NULL;
> + err(1, NULL);
> return new;
> }
>
acme-client(1) make -F flag use more obvious
This is a small change to acme-client(1) because I find the explanation of -F flag not being obvious that you need it when you add/remove an alternative name in your domain config. Maybe wording could be better, if a native English speaker could give it a look. ok? Index: acme-client.1 === RCS file: /home/reposync/src/usr.sbin/acme-client/acme-client.1,v retrieving revision 1.36 diff -u -p -r1.36 acme-client.1 --- acme-client.1 4 Nov 2020 10:34:18 - 1.36 +++ acme-client.1 15 Dec 2020 09:14:07 - @@ -68,6 +68,9 @@ The options are as follows: .Bl -tag -width Ds .It Fl F Force certificate renewal, even if it's too soon. +This is required if the domain alternatives names changed +in +.Xr acme-client.conf 5 . .It Fl f Ar configfile Specify an alternative configuration file. .It Fl n
Re: Lock operations for knote lists
On 11/12/20(Fri) 17:37, Visa Hankala wrote:
> This patch extends struct klist with a callback descriptor and
> an argument. The main purpose of this is to let the kqueue subsystem
> assert when a klist should be locked, and operate the klist lock
> in klist_invalidate().
Lovely!
> Access to a knote list of a kqueue-monitored object has to be
> serialized somehow. Because the object often has a lock for protecting
> its state, and because the object often acquires this lock at the latest
> in its f_event callback functions, I would like to use the same lock
> also for the knote lists. Uses of NOTE_SUBMIT already show a pattern
> arising.
>
> There could be an embedded lock in klist. However, such a lock would be
> redundant in many cases. The code could not rely on a single lock type
> (mutex, rwlock, something else) because the needs of monitored objects
> vary. In addition, an embedded lock would introduce new lock order
> constraints. Note that this patch does not rule out use of dedicated
> klist locks.
Indeed, I'm currently dealing with such different type of lock issue in
UVM :/
> The patch introduces a way to associate lock operations with a klist.
> The caller can provide a custom implementation, or use a ready-made
> interface with a mutex or rwlock.
>
> For compatibility with old code, the new code falls back to using the
> kernel lock if no specific klist initialization has been done. The
> existing code already relies on implicit initialization of klist.
>
> Unfortunately, the size of struct klist will grow threefold.
The growth is unavoidable, it could have been of the size of a lock...
> As the patch gives the code the ability to operate the klist lock,
> the klist API could provide variants of insert and remove actions that
> handle locking internally, for convenience. However, that I would leave
> for another patch because I would prefer to rename the current
> klist_insert() to klist_insert_locked(), and klist_remove() to
> klist_remove_locked().
>
> The patch additionally provides three examples of usage: audio, pipes,
> and sockets. Each of these examples is logically a separate changeset.
One question below.
> Please test and review.
This is in the middle of a build on sparc64, so far so good.
> Index: kern/kern_event.c
> ===
> RCS file: src/sys/kern/kern_event.c,v
> retrieving revision 1.147
> diff -u -p -r1.147 kern_event.c
> --- kern/kern_event.c 9 Dec 2020 18:58:19 - 1.147
> +++ kern/kern_event.c 11 Dec 2020 17:05:09 -
> @@ -1539,9 +1576,14 @@ klist_invalidate(struct klist *list)
> NET_ASSERT_UNLOCKED();
>
> s = splhigh();
> + ls = klist_lock(list);
Isn't splhigh() redundant with klist_lock() now? If a subsystem
provides its own lock/unlock routine shouldn't it ensure that the
necessary SPL is used? Or is this protecting something else? Or is
it just paranoia and we should try to remove it in a later step?
> while ((kn = SLIST_FIRST(&list->kl_list)) != NULL) {
> - if (!knote_acquire(kn))
> + if (!knote_acquire(kn, list, ls)) {
> + /* knote_acquire() has unlocked list. */
> + ls = klist_lock(list);
> continue;
> + }
> + klist_unlock(list, ls);
> splx(s);
> kn->kn_fop->f_detach(kn);
> if (kn->kn_fop->f_flags & FILTEROP_ISFD) {
Re: sdmmc(4): sdmmc_io_function_enable(): don't sleep on lbolt
> Date: Tue, 15 Dec 2020 13:32:22 +0100
> From: Claudio Jeker
>
> On Fri, Dec 11, 2020 at 07:07:56PM -0600, Scott Cheloha wrote:
> > Hi,
> >
> > I'd like to remove lbolt from the kernel. I think having it in the
> > kernel complicates otherwise simple code.
> >
> > We can start with sdmmc(4).
> >
> > The goal in sdmmc_io_function_enable() is calling sdmmc_io_function_ready()
> > up to six times and sleep 1 second between each attempt. Here's rewritten
> > code that does with without lbolt.
> >
> > ok?
> >
> > Index: sdmmc_io.c
> > ===
> > RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_io.c,v
> > retrieving revision 1.41
> > diff -u -p -r1.41 sdmmc_io.c
> > --- sdmmc_io.c 31 Dec 2019 10:05:33 - 1.41
> > +++ sdmmc_io.c 12 Dec 2020 01:04:59 -
> > @@ -231,8 +231,8 @@ sdmmc_io_function_enable(struct sdmmc_fu
> > {
> > struct sdmmc_softc *sc = sf->sc;
> > struct sdmmc_function *sf0 = sc->sc_fn0;
> > + int chan, retry = 5;
> > u_int8_t rv;
> > - int retry = 5;
> >
> > rw_assert_wrlock(&sc->sc_lock);
> >
> > @@ -244,7 +244,7 @@ sdmmc_io_function_enable(struct sdmmc_fu
> > sdmmc_io_write_1(sf0, SD_IO_CCCR_FN_ENABLE, rv);
> >
> > while (!sdmmc_io_function_ready(sf) && retry-- > 0)
> > - tsleep_nsec(&lbolt, PPAUSE, "pause", INFSLP);
> > + tsleep_nsec(&chan, PPAUSE, "pause", SEC_TO_NSEC(1));
> > return (retry >= 0) ? 0 : ETIMEDOUT;
> > }
> >
>
> Why not use &retry as wait channel instead of adding a new variable
> chan? Result is the same. Would it make sense to allow NULL as wait
> channel to make the tsleep not wakeable. At least that could be used in a
> few places where timeouts are implemented with tsleep and would make the
> intent more obvious.
Or have an appropriately named global variable? Something like "int nowake"?
Re: sdmmc(4): sdmmc_io_function_enable(): don't sleep on lbolt
On Fri, Dec 11, 2020 at 07:07:56PM -0600, Scott Cheloha wrote:
> Hi,
>
> I'd like to remove lbolt from the kernel. I think having it in the
> kernel complicates otherwise simple code.
>
> We can start with sdmmc(4).
>
> The goal in sdmmc_io_function_enable() is calling sdmmc_io_function_ready()
> up to six times and sleep 1 second between each attempt. Here's rewritten
> code that does with without lbolt.
>
> ok?
>
> Index: sdmmc_io.c
> ===
> RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_io.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 sdmmc_io.c
> --- sdmmc_io.c31 Dec 2019 10:05:33 - 1.41
> +++ sdmmc_io.c12 Dec 2020 01:04:59 -
> @@ -231,8 +231,8 @@ sdmmc_io_function_enable(struct sdmmc_fu
> {
> struct sdmmc_softc *sc = sf->sc;
> struct sdmmc_function *sf0 = sc->sc_fn0;
> + int chan, retry = 5;
> u_int8_t rv;
> - int retry = 5;
>
> rw_assert_wrlock(&sc->sc_lock);
>
> @@ -244,7 +244,7 @@ sdmmc_io_function_enable(struct sdmmc_fu
> sdmmc_io_write_1(sf0, SD_IO_CCCR_FN_ENABLE, rv);
>
> while (!sdmmc_io_function_ready(sf) && retry-- > 0)
> - tsleep_nsec(&lbolt, PPAUSE, "pause", INFSLP);
> + tsleep_nsec(&chan, PPAUSE, "pause", SEC_TO_NSEC(1));
> return (retry >= 0) ? 0 : ETIMEDOUT;
> }
>
Why not use &retry as wait channel instead of adding a new variable
chan? Result is the same. Would it make sense to allow NULL as wait
channel to make the tsleep not wakeable. At least that could be used in a
few places where timeouts are implemented with tsleep and would make the
intent more obvious.
--
:wq Claudio
Re: i386: apm(4): apm_thread(): sleep without lbolt
On 11/12/20(Fri) 19:17, Scott Cheloha wrote:
> Here's another sleep that doesn't need lbolt.
>
> The idea here is to call apm_periodic_check() once a second.
> We can do that without lbolt.
>
> Is there some other address that would be more appropriate for this
> thread to sleep on? It doesn't look like any apm(4) code calls
> wakeup(9) on lbolt so I've just replaced with with a local channel.
Note sure we want to grow the stack just for that. Any member of `sc',
or even `sc' itself if this doesn't conflict, could be used as wait
channel.
> ok?
>
> Index: apm.c
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/apm.c,v
> retrieving revision 1.125
> diff -u -p -r1.125 apm.c
> --- apm.c 24 Jun 2020 22:03:40 - 1.125
> +++ apm.c 12 Dec 2020 01:17:38 -
> @@ -50,6 +50,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -904,12 +905,13 @@ void
> apm_thread(void *v)
> {
> struct apm_softc *sc = v;
> + int chan;
>
> for (;;) {
> rw_enter_write(&sc->sc_lock);
> (void) apm_periodic_check(sc);
> rw_exit_write(&sc->sc_lock);
> - tsleep_nsec(&lbolt, PWAIT, "apmev", INFSLP);
> + tsleep_nsec(&chan, PWAIT, "apmev", SEC_TO_NSEC(1));
> }
> }
>
>
Re: sdmmc(4): sdmmc_io_function_enable(): don't sleep on lbolt
On 11/12/20(Fri) 19:07, Scott Cheloha wrote:
> I'd like to remove lbolt from the kernel. I think having it in the
> kernel complicates otherwise simple code.
Decoupling code is IMHO a good thing. I like this move.
> We can start with sdmmc(4).
>
> The goal in sdmmc_io_function_enable() is calling sdmmc_io_function_ready()
> up to six times and sleep 1 second between each attempt. Here's rewritten
> code that does with without lbolt.
Ok with me.
> Index: sdmmc_io.c
> ===
> RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_io.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 sdmmc_io.c
> --- sdmmc_io.c31 Dec 2019 10:05:33 - 1.41
> +++ sdmmc_io.c12 Dec 2020 01:04:59 -
> @@ -231,8 +231,8 @@ sdmmc_io_function_enable(struct sdmmc_fu
> {
> struct sdmmc_softc *sc = sf->sc;
> struct sdmmc_function *sf0 = sc->sc_fn0;
> + int chan, retry = 5;
> u_int8_t rv;
> - int retry = 5;
>
> rw_assert_wrlock(&sc->sc_lock);
>
> @@ -244,7 +244,7 @@ sdmmc_io_function_enable(struct sdmmc_fu
> sdmmc_io_write_1(sf0, SD_IO_CCCR_FN_ENABLE, rv);
>
> while (!sdmmc_io_function_ready(sf) && retry-- > 0)
> - tsleep_nsec(&lbolt, PPAUSE, "pause", INFSLP);
> + tsleep_nsec(&chan, PPAUSE, "pause", SEC_TO_NSEC(1));
> return (retry >= 0) ? 0 : ETIMEDOUT;
> }
>
>
Re: fortune: allow to use symlinks
> I very rarely use fortune and I'm not familiar with the codebase, but
> from some quick testing and code-scanning I found the following:
> is_fortfile appends .dat to the input file and sees if it's accessible.
> Adding a symlink to the corresponding .dat file make the thing work
> perfectly well for me. Your diff seems to work, because you resolve
> the path, which in turn gives back the correct location for the .dat,
> not because symlinks are not suported.
Thank you very much for review. Indeed, adding second symlink fixes
the initial issue, so the patch now seems useless...
> Personally I don't see good reason to add the additional logic which
> can be solved with a second simple symlink, but if others think it's
> worth it, comments inline.
... but following your comments I've discovered that copy() function
in fortune.c can return NULL, and that value isn't checked.
So I'm posting the whole diff for the sake of completeness, and what
I really think worths committing is the last chunk, replacing "return
NULL" with "err(1, NULL)" in copy(). Okay for it?
> martijn@
>
> On Tue, 2020-12-15 at 01:09 +0300, Vadim Zhukov wrote:
> > Hello, all.
> >
> > This allows fortune(6) to open fortune files via symlinks.
> > Maybe this is a stupid idea, I dunno, but it seems inconsistent
> > that I can't put a symlink to my fortunes collection in
> > /usr/share/games/fortune and then simply call "fortune foo"
> > instead of "fortune /usr/local/share/games/fortune/ru/foo".
> >
> > I decided to call readlink(2) explicitly rather than adding
> > check of file type being open, since that resulted in less code.
> >
> > Ah, and yes, I have a port of Russian fortune files pending,
> > but that's for another list and another day.
> >
> > So... okay/notokay anyone? :)
--
WBR,
Vadim Zhukov
Index: fortune.c
===
RCS file: /cvs/src/games/fortune/fortune/fortune.c,v
retrieving revision 1.60
diff -u -p -r1.60 fortune.c
--- fortune.c 10 Aug 2017 17:00:08 - 1.60
+++ fortune.c 15 Dec 2020 11:46:00 -
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -388,8 +389,9 @@ add_file(int percent, char *file, char *
FILEDESC *parent)
{
FILEDESC*fp;
+ ssize_t lnklen;
int fd;
- char*path, *offensive;
+ char*path, *offensive, lnkpath[PATH_MAX];
boolwas_malloc;
boolisdir;
@@ -420,8 +422,22 @@ add_file(int percent, char *file, char *
}
DPRINTF(1, (stderr, "adding file \"%s\"\n", path));
+
over:
+ lnklen = readlink(path, lnkpath, sizeof(lnkpath));
+ if (lnklen == -1) {
+ if (errno != EINVAL)
+ goto openfailed;
+ } else {
+ lnkpath[lnklen] = '\0';
+ if (was_malloc)
+ free(path);
+ path = copy(lnkpath, NULL);
+ was_malloc = 1;
+ }
+
if ((fd = open(path, O_RDONLY)) < 0) {
+openfailed:
/*
* This is a sneak. If the user said -a, and if the
* file we're given isn't a file, we check to see if
@@ -718,7 +734,7 @@ copy(char *str, char *suf)
char*new;
if (asprintf(&new, "%s%s", str, suf ? suf : "") == -1)
- return NULL;
+ err(1, NULL);
return new;
}
Re: Switch select(2) to kqueue-based implementation
On 12/12/20(Sat) 11:29, Visa Hankala wrote:
> On Fri, Dec 11, 2020 at 09:35:59AM -0300, Martin Pieuchot wrote:
> > On 10/12/20(Thu) 09:59, Martin Pieuchot wrote:
> > > All previous kqueue refactoring have been committed, here's a final diff
> > > to modify the internal implementation of {p,}select(2) to query kqfilter
> > > handlers instead of poll ones.
> > >
> > > {p,}poll(2) are left untouched to ease the transition.
> > >
> > > Here's what I said in the original mail back in May [0]:
> > >
> > > > The main argument for this proposal is to reduce the amount of code
> > > > executed to notify userland when an event occur. The outcome of this
> > > > diff is that a single notification subsystem needs to be taken out of
> > > > the KERNEL_LOCK(). This simplifies a lot existing locking tentacles.
> > > >
> > > > Using kqueue internally means collision is avoided and there's no need
> > > > to query handlers for fds that aren't ready. This comes at the cost of
> > > > allocating descriptors. A space vs time trade-off. Note that this cost
> > > > can be diminished by doing lazy removal of event descriptors to be able
> > > > to re-use them.
> > >
> > > The logic is as follow:
> > >
> > > - With this change every thread use a "private" kqueue, usable only by
> > > the kernel, to register events for select(2) and later poll(2).
> > >
> > > - Events specified via FD_SET(2) are converted to their kqueue equivalent.
> > > ktrace(1) now also outputs converted events to ease debugging.
> > >
> > > - kqueue_scan() might be called multiple times per syscall, just like with
> > > the last version of kevent(2).
> > >
> > > - At the end of every {p,}select(2) syscall the private kqueue is purged.
> > >
> > > [0] https://marc.info/?l=openbsd-tech&m=158979921322191&w=2
> >
> > Updated diff that adds a FALLTHOUGHT lost in refactoring and do not
> > block in if the timeout is cleared and no events are requested, pointed
> > by cheloha@:
Updated diff below.
> > +/*
> > + * Convert fd_set into kqueue events and register them on the
> > + * per-thread queue.
> > + */
> > int
> > -selscan(struct proc *p, fd_set *ibits, fd_set *obits, int nfd, int ni,
> > -register_t *retval)
> > +pselregister(struct proc *p, fd_set *ibits, int nfd, int ni, int
> > *nregistered)
>
> I think pselregister() should take the bitvector similarly to
> pselcollect(). Then the function could skip some pointer arithmetics
> as well.
>
> int pselregister(struct proc *p, fd_set *pibits[3], int nfd, int
> *nregistered);
I don't mind but did not include it in the diff, this seem like an extra
refactoring to me and I'm trying to change few lines as possible.
> > +/*
> > + * Convert given kqueue event into corresponding select(2) bit.
> > + */
> > +int
> > +pselcollect(struct proc *p, struct kevent *kevp, fd_set *pobits[3])
> > +{
> > +#ifdef DIAGNOSTIC
> > + /* Filter out and lazily delete spurious events */
> > + if ((unsigned long)kevp->udata != p->p_kq_serial) {
> > + DPRINTFN(0, "select fd %u mismatched serial %lu\n",
> > + (int)kevp->ident, p->p_kq_serial);
> > + kevp->flags = EV_DISABLE|EV_DELETE;
> > + kqueue_register(p->p_kq, kevp, p);
> > + return (0);
> > + }
> > +#endif
>
> Shouldn't skipping of spurious events be taken into account in the
> main scan loop? Otherwise they may cause artifacts, such as premature
> timeout expiry when a wakeup is caused solely by a spurious event, or
> return of an incomplete result when spurious events eat capacity from
> nevents.
This sounds like an later improvement to me and I'd like to discuss it
separately.
Index: kern/sys_generic.c
===
RCS file: /cvs/src/sys/kern/sys_generic.c,v
retrieving revision 1.132
diff -u -p -r1.132 sys_generic.c
--- kern/sys_generic.c 2 Oct 2020 15:45:22 - 1.132
+++ kern/sys_generic.c 15 Dec 2020 10:36:28 -
@@ -55,6 +55,7 @@
#include
#include
#include
+#include
#ifdef KTRACE
#include
#endif
@@ -66,8 +67,21 @@
#include
-int selscan(struct proc *, fd_set *, fd_set *, int, int, register_t *);
-void pollscan(struct proc *, struct pollfd *, u_int, register_t *);
+/*
+ * Debug values:
+ * 1 - print implementation errors, things that should not happen.
+ * 2 - print ppoll(2) information, somewhat verbose
+ * 3 - print pselect(2) and ppoll(2) information, very verbose
+ */
+int kqpoll_debug = 0;
+#define DPRINTFN(v, x...) if (kqpoll_debug > v) { \
+ printf("%s(%d): ", curproc->p_p->ps_comm, curproc->p_tid); \
+ printf(x); \
+}
+
+int pselregister(struct proc *, fd_set *, int, int, int *);
+int pselcollect(struct proc *, struct kevent *, fd_set *[]);
+
int pollout(struct pollfd *, struct pollfd *, u_int);
int dopselect(struct proc *, int, fd_set *, fd_set *, fd_set *,
struct timespec *, const sigset_t
Re: fortune: allow to use symlinks
I very rarely use fortune and I'm not familiar with the codebase, but
from some quick testing and code-scanning I found the following:
is_fortfile appends .dat to the input file and sees if it's accessible.
Adding a symlink to the corresponding .dat file make the thing work
perfectly well for me. Your diff seems to work, because you resolve
the path, which in turn gives back the correct location for the .dat,
not because symlinks are not suported.
Personally I don't see good reason to add the additional logic which
can be solved with a second simple symlink, but if others think it's
worth it, comments inline.
martijn@
On Tue, 2020-12-15 at 01:09 +0300, Vadim Zhukov wrote:
> Hello, all.
>
> This allows fortune(6) to open fortune files via symlinks.
> Maybe this is a stupid idea, I dunno, but it seems inconsistent
> that I can't put a symlink to my fortunes collection in
> /usr/share/games/fortune and then simply call "fortune foo"
> instead of "fortune /usr/local/share/games/fortune/ru/foo".
>
> I decided to call readlink(2) explicitly rather than adding
> check of file type being open, since that resulted in less code.
>
> Ah, and yes, I have a port of Russian fortune files pending,
> but that's for another list and another day.
>
> So... okay/notokay anyone? :)
>
> --
> WBR,
> Vadim Zhukov
>
>
> Index: fortune.c
> ===
> RCS file: /cvs/src/games/fortune/fortune/fortune.c,v
> retrieving revision 1.60
> diff -u -p -r1.60 fortune.c
> --- fortune.c 10 Aug 2017 17:00:08 - 1.60
> +++ fortune.c 14 Dec 2020 22:05:33 -
> @@ -39,6 +39,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -388,8 +389,9 @@ add_file(int percent, char *file, char *
> FILEDESC *parent)
> {
> FILEDESC*fp;
> + ssize_t lnklen;
> int fd;
> - char*path, *offensive;
> + char*path, *offensive, lnkpath[PATH_MAX];
> boolwas_malloc;
> boolisdir;
>
> @@ -420,8 +422,22 @@ add_file(int percent, char *file, char *
> }
>
> DPRINTF(1, (stderr, "adding file \"%s\"\n", path));
> +
> over:
> + lnklen = readlink(path, lnkpath, PATH_MAX);
I'd prefer sizeof(lnkpath) instead of PATH_MAX, just to be a little more
robust.
> + if (lnklen == -1) {
> + if (errno != EINVAL)
> + goto openfailed;
> + } else {
> + lnkpath[lnklen] = '\0';
> + if (was_malloc)
> + free(path);
> + path = strdup(lnkpath);
This needs a NULL-check.
> + was_malloc = 1;
> + }
> +
> if ((fd = open(path, O_RDONLY)) < 0) {
> +openfailed:
> /*
> * This is a sneak. If the user said -a, and if the
> * file we're given isn't a file, we check to see if
>
