Re: When did PCs stop using ISA Timer 1?
Jonathan Gray wrote: > On Fri, Aug 26, 2022 at 10:21:32PM -0500, Scott Cheloha wrote: > > I noticed that on non-LAPIC systems we program channel 0 in periodic > > mode with an initial count of 11932 to effect a 100hz clock interrupt. > > And then we also use that same channel to count time, but because we > > aren't using the full 16-bit range we need to do all this checking and > > incrementing to handle premature overflow to make it appear as though > > the full counter is being used. > > > > And I had this whimsical idea: gee, wouldn't it be so much easier to > > use channel 0 for clock interrupts and a different channel for > > counting time? > > > > But then I started reading and saw that channel 1 had a dedicated > > purpose in the bad old days. > > > > So I was left wondering when channel 1 stopped performing that task, > > and whether those systems (a) predate the APIC and (b) can even run > > OpenBSD at all. > > Attempting to use counter 1 would be more trouble than it is worth. I would not be surprised if there existed machines with broken counter 1 (in some subtle way).
Re: When did PCs stop using ISA Timer 1?
On Fri, Aug 26, 2022 at 10:21:32PM -0500, Scott Cheloha wrote: > I noticed that on non-LAPIC systems we program channel 0 in periodic > mode with an initial count of 11932 to effect a 100hz clock interrupt. > And then we also use that same channel to count time, but because we > aren't using the full 16-bit range we need to do all this checking and > incrementing to handle premature overflow to make it appear as though > the full counter is being used. > > And I had this whimsical idea: gee, wouldn't it be so much easier to > use channel 0 for clock interrupts and a different channel for > counting time? > > But then I started reading and saw that channel 1 had a dedicated > purpose in the bad old days. > > So I was left wondering when channel 1 stopped performing that task, > and whether those systems (a) predate the APIC and (b) can even run > OpenBSD at all. Attempting to use counter 1 would be more trouble than it is worth. > > What is the minimum chipset? 486? 586? You've been doing some > sprucing, so I am unsure. I know the 80386 is out. We have a minimum architecture level, not chipset. Our toolchain defaults to 586. This ensures that 64-bit atomic builtins are available. 586 includes cr4, cpuid, rdtsc, rdmsr etc As to when the APIC was introduced vol 3b of Intel's x86 SDM has: "The Advanced Programmable Interrupt Controller (APIC), referred to in this book as the local APIC, was introduced into the IA-32 processors with the Pentium processor (beginning with the 735/90 and 815/100 models) and is included in the Pentium 4, Intel Xeon, and P6 family processors. The features and functions of the local APIC are derived from the Intel 82489DX external APIC, which was used with the Intel486 and early Pentium processors. Additional refinements of the local APIC architecture were incorporated in the Pentium 4 and Intel Xeon processors." > > > The PCH datasheets from 100 series and later only document counter 0 > > and counter 2. > > > > 9 series and earlier datasheet has > > "The PCH contains three counters that have fixed uses." > > 100 series and later > > "The PCH contains two counters that have fixed uses." > > What does the PCH 9 series and earlier pertain to? What socket would > have it? 9 series is H97/Z97/X99 https://ark.intel.com/content/www/us/en/ark/products/series/98458/intel-9-series-chipsets.html https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/9-series-chipset-pch-datasheet.pdf
Re: When did PCs stop using ISA Timer 1?
On Sat, Aug 27, 2022 at 11:33:58AM +1000, Jonathan Gray wrote: > On Fri, Aug 26, 2022 at 11:09:19AM -0500, Scott Cheloha wrote: > > Hi, > > > > TLDR: > > > > 1. When did PCs stop using ISA Timer 1 to trigger DRAM refresh? > > > > 2. Are any PCs that rely on ISA Timer 1 for DRAM refresh capable of > >running OpenBSD as it exists today? > > > > Long version: > > > > I have a history question for the list. Maybe one of you hardware > > jocks or history buffs can help me out. > > > > So, in the IBM AT/PC and, later, all ISA-compatible systems, the ISA > > timer (an i8253 or compatible clock) has 3 independent 16-bit > > counters. > > > > The first, Timer 0, is available for use by the operating system. > > > > The second, Timer 1, was traditionally programmed by the BIOS at a > > particular rate to trigger DRAM refresh. > > > > The third, Timer 2, is usually wired up to the PC speaker and may be > > used by the operating system to produce primitive sound effects. > > > > I found a more detailed explanation of what Timer 1 actually did in > > this book: > > > > https://ia601901.us.archive.org/12/items/ISA_System_Architecture/ISA_System_Architecture.pdf > > > > > ISA System Architecture Third Edition (1995) > > > Chapter 24: ISA Timers > > > p. 471 > > > > > > Refresh Timer (Timer 1) > > > > > > The refresh timer, or timer 1, is a programmable frequency source. The > > > same 1.19318MHz signal (used by timer 0) provides the refresh timer's > > > clock input. The programmer specifies a divisor to be divided into > > > the input clock to yield the desired output frequency. During the > > > POST, a divisor of 0012h, or a decimal 18, is written to the refresh > > > timer at I/O address 0041h. The input clock frequency of 1.19318MHz is > > > therefore divided by 18 to yield an output frequency of 66287.77Hz, > > > or a pulse every 15.09 microseconds. > > > > > > This is the refresh request signal that triggers the DRAM refresh > > > logic to become bus master once every 15.09 microseconds so it can > > > refresh another row in DRAM memory throughout the system. For more > > > information on DRAM refresh, refer to the chapter entitled "RAM > > > Memory: Theory of Operation." > > > > This is fascinating. > > > > But obviously this is no longer true in modern PCs. The ISA bus is > > still emulated in modern PCs, and DRAM in modern PCs still needs > > refreshing, but they don't rely on the emulated ISA timer to make it > > happen. > > > > So, when did PCs stop using ISA Timer 1 for DRAM refresh? > > > > The IBM AT/PC was built around the 80286. Was it with the advent of > > the 80386 (1985)? The 80486 (1989)? P5 (1993)? P6 (1995)? Later? > > > > Was the change independent of a particular processor generation jump? > > Like, maybe a technological advance in the state of the art in DRAM > > obsoleted the use of ISA Timer 1 for refresh? > > > > And then, more importantly, are any machines that rely on ISA Timer 1 > > for DRAM refresh actually capable of running OpenBSD as it exists > > today? > > What difference does it make? We don't use counter 1. I noticed that on non-LAPIC systems we program channel 0 in periodic mode with an initial count of 11932 to effect a 100hz clock interrupt. And then we also use that same channel to count time, but because we aren't using the full 16-bit range we need to do all this checking and incrementing to handle premature overflow to make it appear as though the full counter is being used. And I had this whimsical idea: gee, wouldn't it be so much easier to use channel 0 for clock interrupts and a different channel for counting time? But then I started reading and saw that channel 1 had a dedicated purpose in the bad old days. So I was left wondering when channel 1 stopped performing that task, and whether those systems (a) predate the APIC and (b) can even run OpenBSD at all. What is the minimum chipset? 486? 586? You've been doing some sprucing, so I am unsure. I know the 80386 is out. > The PCH datasheets from 100 series and later only document counter 0 > and counter 2. > > 9 series and earlier datasheet has > "The PCH contains three counters that have fixed uses." > 100 series and later > "The PCH contains two counters that have fixed uses." What does the PCH 9 series and earlier pertain to? What socket would have it? (I didn't even know Intel had documented this, thanks.)
Re: When did PCs stop using ISA Timer 1?
Jonathan Gray wrote: >> What difference does it make? We don't use counter 1. > > The PCH datasheets from 100 series and later only document counter 0 > and counter 2. > > 9 series and earlier datasheet has > "The PCH contains three counters that have fixed uses." > 100 series and later > "The PCH contains two counters that have fixed uses." In other words: Since that counter was previously used for a well-defined purpose but then stopped being used for that case, there may be chipset vendors who decided to REMOVE or BREAK the hardware... I think you can assume nothing about it. This question can only be answered by finding a body of code which reliably uses it.
Re: When did PCs stop using ISA Timer 1?
On Fri, Aug 26, 2022 at 11:09:19AM -0500, Scott Cheloha wrote: > Hi, > > TLDR: > > 1. When did PCs stop using ISA Timer 1 to trigger DRAM refresh? > > 2. Are any PCs that rely on ISA Timer 1 for DRAM refresh capable of >running OpenBSD as it exists today? > > Long version: > > I have a history question for the list. Maybe one of you hardware > jocks or history buffs can help me out. > > So, in the IBM AT/PC and, later, all ISA-compatible systems, the ISA > timer (an i8253 or compatible clock) has 3 independent 16-bit > counters. > > The first, Timer 0, is available for use by the operating system. > > The second, Timer 1, was traditionally programmed by the BIOS at a > particular rate to trigger DRAM refresh. > > The third, Timer 2, is usually wired up to the PC speaker and may be > used by the operating system to produce primitive sound effects. > > I found a more detailed explanation of what Timer 1 actually did in > this book: > > https://ia601901.us.archive.org/12/items/ISA_System_Architecture/ISA_System_Architecture.pdf > > > ISA System Architecture Third Edition (1995) > > Chapter 24: ISA Timers > > p. 471 > > > > Refresh Timer (Timer 1) > > > > The refresh timer, or timer 1, is a programmable frequency source. The > > same 1.19318MHz signal (used by timer 0) provides the refresh timer's > > clock input. The programmer specifies a divisor to be divided into > > the input clock to yield the desired output frequency. During the > > POST, a divisor of 0012h, or a decimal 18, is written to the refresh > > timer at I/O address 0041h. The input clock frequency of 1.19318MHz is > > therefore divided by 18 to yield an output frequency of 66287.77Hz, > > or a pulse every 15.09 microseconds. > > > > This is the refresh request signal that triggers the DRAM refresh > > logic to become bus master once every 15.09 microseconds so it can > > refresh another row in DRAM memory throughout the system. For more > > information on DRAM refresh, refer to the chapter entitled "RAM > > Memory: Theory of Operation." > > This is fascinating. > > But obviously this is no longer true in modern PCs. The ISA bus is > still emulated in modern PCs, and DRAM in modern PCs still needs > refreshing, but they don't rely on the emulated ISA timer to make it > happen. > > So, when did PCs stop using ISA Timer 1 for DRAM refresh? > > The IBM AT/PC was built around the 80286. Was it with the advent of > the 80386 (1985)? The 80486 (1989)? P5 (1993)? P6 (1995)? Later? > > Was the change independent of a particular processor generation jump? > Like, maybe a technological advance in the state of the art in DRAM > obsoleted the use of ISA Timer 1 for refresh? > > And then, more importantly, are any machines that rely on ISA Timer 1 > for DRAM refresh actually capable of running OpenBSD as it exists > today? What difference does it make? We don't use counter 1. The PCH datasheets from 100 series and later only document counter 0 and counter 2. 9 series and earlier datasheet has "The PCH contains three counters that have fixed uses." 100 series and later "The PCH contains two counters that have fixed uses."
move PRU_SEND request to (*pru_send)()
The former PRU_SEND error path of gre_usrreq() had `control' mbuf(9)
leak. It was fixed in new gre_send().
The former pfkeyv2_send() was renamed to pfkeyv2_dosend().
Index: sys/kern/uipc_usrreq.c
===
RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.175
diff -u -p -r1.175 uipc_usrreq.c
--- sys/kern/uipc_usrreq.c 26 Aug 2022 16:17:39 - 1.175
+++ sys/kern/uipc_usrreq.c 26 Aug 2022 23:00:24 -
@@ -137,6 +137,7 @@ const struct pr_usrreqs uipc_usrreqs = {
.pru_disconnect = uipc_disconnect,
.pru_shutdown = uipc_shutdown,
.pru_rcvd = uipc_rcvd,
+ .pru_send = uipc_send,
};
void
@@ -244,102 +245,6 @@ uipc_usrreq(struct socket *so, int req,
}
break;
- case PRU_SEND:
- if (control) {
- sounlock(so);
- error = unp_internalize(control, p);
- solock(so);
- if (error)
- break;
- }
- switch (so->so_type) {
-
- case SOCK_DGRAM: {
- const struct sockaddr *from;
-
- if (nam) {
- if (unp->unp_conn) {
- error = EISCONN;
- break;
- }
- error = unp_connect(so, nam, p);
- if (error)
- break;
- }
-
- if ((so2 = unp_solock_peer(so)) == NULL) {
- if (nam != NULL)
- error = ECONNREFUSED;
- else
- error = ENOTCONN;
- break;
- }
-
- if (unp->unp_addr)
- from = mtod(unp->unp_addr, struct sockaddr *);
- else
- from = _noname;
- if (sbappendaddr(so2, >so_rcv, from, m, control)) {
- sorwakeup(so2);
- m = NULL;
- control = NULL;
- } else
- error = ENOBUFS;
-
- if (so2 != so)
- sounlock(so2);
-
- if (nam)
- unp_disconnect(unp);
- break;
- }
-
- case SOCK_STREAM:
- case SOCK_SEQPACKET:
- if (so->so_state & SS_CANTSENDMORE) {
- error = EPIPE;
- break;
- }
- if ((so2 = unp_solock_peer(so)) == NULL) {
- error = ENOTCONN;
- break;
- }
-
- /*
-* Send to paired receive port, and then raise
-* send buffer counts to maintain backpressure.
-* Wake up readers.
-*/
- if (control) {
- if (sbappendcontrol(so2, >so_rcv, m,
- control)) {
- control = NULL;
- } else {
- sounlock(so2);
- error = ENOBUFS;
- break;
- }
- } else if (so->so_type == SOCK_SEQPACKET)
- sbappendrecord(so2, >so_rcv, m);
- else
- sbappend(so2, >so_rcv, m);
- so->so_snd.sb_mbcnt = so2->so_rcv.sb_mbcnt;
- so->so_snd.sb_cc = so2->so_rcv.sb_cc;
- if (so2->so_rcv.sb_cc > 0)
- sorwakeup(so2);
-
- sounlock(so2);
- m = NULL;
- break;
-
- default:
- panic("uipc 4");
- }
- /* we need to undo unp_internalize in case of errors */
- if (control && error)
- unp_dispose(control);
- break;
-
case PRU_ABORT:
unp_detach(unp);
sofree(so, 0);
@@ -574,6 +479,115 @@ uipc_rcvd(struct socket *so)
}
return (0);
+}
+
+int
+uipc_send(struct socket *so, struct mbuf *m, struct mbuf *nam,
+struct mbuf *control)
+{
+ struct unpcb *unp = sotounpcb(so);
+ struct socket
Rename global ifnet TAILQ
Naming the list like the struct itself makes for awful grepping.
Distinguish the list name; no functional change.
Builds/runs fine on and64 and sparc64.
Feedback? Objection? OK?
Naming bikeshed (ifnet_tailq, ifnet_list)?
diff --git a/sys/arch/amd64/amd64/autoconf.c b/sys/arch/amd64/amd64/autoconf.c
index 6ae7b28f6d9..979b043c184 100644
--- a/sys/arch/amd64/amd64/autoconf.c
+++ b/sys/arch/amd64/amd64/autoconf.c
@@ -195,7 +195,7 @@ diskconf(void)
if (bios_bootmac) {
struct ifnet *ifp;
- TAILQ_FOREACH(ifp, , if_list) {
+ TAILQ_FOREACH(ifp, , if_list) {
if (ifp->if_type == IFT_ETHER &&
memcmp(bios_bootmac->mac,
((struct arpcom *)ifp)->ac_enaddr,
diff --git a/sys/arch/arm64/arm64/autoconf.c b/sys/arch/arm64/arm64/autoconf.c
index bda3cb3f6b0..dd0449c5eed 100644
--- a/sys/arch/arm64/arm64/autoconf.c
+++ b/sys/arch/arm64/arm64/autoconf.c
@@ -84,7 +84,7 @@ diskconf(void)
if (bootmac) {
struct ifnet *ifp;
- TAILQ_FOREACH(ifp, , if_list) {
+ TAILQ_FOREACH(ifp, , if_list) {
if (ifp->if_type == IFT_ETHER &&
memcmp(bootmac, ((struct arpcom *)ifp)->ac_enaddr,
ETHER_ADDR_LEN) == 0)
diff --git a/sys/arch/armv7/armv7/autoconf.c b/sys/arch/armv7/armv7/autoconf.c
index 00134c44423..0b290eb1db8 100644
--- a/sys/arch/armv7/armv7/autoconf.c
+++ b/sys/arch/armv7/armv7/autoconf.c
@@ -125,7 +125,7 @@ diskconf(void)
if (bootmac) {
struct ifnet *ifp;
- TAILQ_FOREACH(ifp, , if_list) {
+ TAILQ_FOREACH(ifp, , if_list) {
if (ifp->if_type == IFT_ETHER &&
memcmp(bootmac, ((struct arpcom *)ifp)->ac_enaddr,
ETHER_ADDR_LEN) == 0)
diff --git a/sys/arch/i386/i386/autoconf.c b/sys/arch/i386/i386/autoconf.c
index 31e9b42f68f..8b3d6d6908b 100644
--- a/sys/arch/i386/i386/autoconf.c
+++ b/sys/arch/i386/i386/autoconf.c
@@ -232,7 +232,7 @@ diskconf(void)
if (bios_bootmac) {
struct ifnet *ifp;
- TAILQ_FOREACH(ifp, , if_list) {
+ TAILQ_FOREACH(ifp, , if_list) {
if (ifp->if_type == IFT_ETHER &&
memcmp(bios_bootmac->mac,
((struct arpcom *)ifp)->ac_enaddr,
diff --git a/sys/arch/powerpc64/powerpc64/autoconf.c
b/sys/arch/powerpc64/powerpc64/autoconf.c
index ef8e29268f3..13b1fa2c66e 100644
--- a/sys/arch/powerpc64/powerpc64/autoconf.c
+++ b/sys/arch/powerpc64/powerpc64/autoconf.c
@@ -54,7 +54,7 @@ diskconf(void)
if (bootmac) {
struct ifnet *ifp;
- TAILQ_FOREACH(ifp, , if_list) {
+ TAILQ_FOREACH(ifp, , if_list) {
if (ifp->if_type == IFT_ETHER &&
memcmp(bootmac, ((struct arpcom *)ifp)->ac_enaddr,
ETHER_ADDR_LEN) == 0)
diff --git a/sys/arch/riscv64/riscv64/autoconf.c
b/sys/arch/riscv64/riscv64/autoconf.c
index a447667bd51..0bd09cce7d2 100644
--- a/sys/arch/riscv64/riscv64/autoconf.c
+++ b/sys/arch/riscv64/riscv64/autoconf.c
@@ -72,7 +72,7 @@ diskconf(void)
if (bootmac) {
struct ifnet *ifp;
- TAILQ_FOREACH(ifp, , if_list) {
+ TAILQ_FOREACH(ifp, , if_list) {
if (ifp->if_type == IFT_ETHER &&
memcmp(bootmac, ((struct arpcom *)ifp)->ac_enaddr,
ETHER_ADDR_LEN) == 0)
diff --git a/sys/dev/pv/hypervic.c b/sys/dev/pv/hypervic.c
index ad5fc9b0c0b..9c7f70dd96f 100644
--- a/sys/dev/pv/hypervic.c
+++ b/sys/dev/pv/hypervic.c
@@ -873,7 +873,7 @@ kvp_get_ip_info(struct hv_kvp *kvp, const uint8_t *mac,
uint8_t *family,
KERNEL_LOCK();
s = splnet();
- TAILQ_FOREACH(ifp, , if_list) {
+ TAILQ_FOREACH(ifp, , if_list) {
if (!memcmp(LLADDR(ifp->if_sadl), enaddr, ETHER_ADDR_LEN))
break;
}
diff --git a/sys/dev/pv/vmt.c b/sys/dev/pv/vmt.c
index 6e97a46dc16..e604499a814 100644
--- a/sys/dev/pv/vmt.c
+++ b/sys/dev/pv/vmt.c
@@ -869,7 +869,7 @@ vmt_tclo_broadcastip(struct vmt_softc *sc)
/* find first available ipv4 address */
guest_ip = NULL;
- TAILQ_FOREACH(iface, , if_list) {
+ TAILQ_FOREACH(iface, , if_list) {
struct ifaddr *iface_addr;
/* skip loopback */
@@ -1301,7 +1301,7 @@ vmt_xdr_nic_info(char *data)
}
nics = 0;
- TAILQ_FOREACH(iface, , if_list) {
+ TAILQ_FOREACH(iface, , if_list) {
nictotal = vmt_xdr_nic_entry(iface, data);
if (nictotal == 0)
continue;
diff --git a/sys/net/if.c b/sys/net/if.c
index 21ee0a39d14..b270c404ae7 100644
--- a/sys/net/if.c
+++
Re: struct ifnet: remove unused if_switchport member
On Fri, Aug 26, 2022 at 06:15:50PM +0100, Stuart Henderson wrote: > On 2022/08/26 16:50, Klemens Nanni wrote: > > Running the packages.txt files through 'sort -u' and 'comm -12' and > > filtering for ports we actually have leaves us with > > > > aircrack-ng > > firefox > > firefox-esr > > gst-plugins-bad1.0 > > gst-plugins-bad1.0-contrib > > libgtop2 > > libusrsctp > > miniupnpd > > net-snmp > > qt6-webengine > > qtwebengine-opensource-src > > thunderbird > > warzone2100 > > zabbix > > > > I'm sure that some of them are false positives because they > > a) define _KERNEL after including > > b) don't do either of it on OpenBSD > > likely the case for some of them, but checking a couple of likely > candidates I am pretty sure they do use it (e.g. net-snmp, libgtop2, > zabbix, via kvm_read). > > > OK if I just REVISION bump all these and be done with it? That'd be > > this list of PKGPATHs: > > bumps need to be done *after* snapshots with the changes are available > (I would wait at least a day after) otherwise if a ports builder starts > a build after the bump but before they have the new header, the bump > won't help. Yes, of course. I would've waited for an amd64 snap to contain the new if_var.h and then commit the ports bump. > > > security/aircrack-ng > > www/firefox-esr > > www/mozilla-firefox > > multimedia/gstreamer1/plugins-bad > > devel/libgtop2 > > net/usrsctp > > net/miniupnp/miniupnpd > > net/net-snmp,-main > > net/net-snmp,-tkmib > > only REVISION-main for net-snmp, it's used for some mibII things in the > agent, tkmib is just a Perl/Tk-based viewer and doesn't go anywhere near > it so unaffected. > > > net/zabbix,-main > > net/zabbix,-proxy,mysql > > net/zabbix,-proxy,pgsql > > net/zabbix,-proxy,sqlite3 > > net/zabbix,-server,mysql > > net/zabbix,-server,pgsql > > net/zabbix,-web > > same only REVISION-main for zabbix (agent) > > > mail/mozilla-thunderbird > > games/warzone2100 > Full ports diff for completeness. OK to zap the switchport member now, wait, then commit the bumps?
Re: struct ifnet: remove unused if_switchport member
On 2022/08/26 16:50, Klemens Nanni wrote: > Running the packages.txt files through 'sort -u' and 'comm -12' and > filtering for ports we actually have leaves us with > > aircrack-ng > firefox > firefox-esr > gst-plugins-bad1.0 > gst-plugins-bad1.0-contrib > libgtop2 > libusrsctp > miniupnpd > net-snmp > qt6-webengine > qtwebengine-opensource-src > thunderbird > warzone2100 > zabbix > > I'm sure that some of them are false positives because they > a) define _KERNEL after including > b) don't do either of it on OpenBSD likely the case for some of them, but checking a couple of likely candidates I am pretty sure they do use it (e.g. net-snmp, libgtop2, zabbix, via kvm_read). > OK if I just REVISION bump all these and be done with it? That'd be > this list of PKGPATHs: bumps need to be done *after* snapshots with the changes are available (I would wait at least a day after) otherwise if a ports builder starts a build after the bump but before they have the new header, the bump won't help. > security/aircrack-ng > www/firefox-esr > www/mozilla-firefox > multimedia/gstreamer1/plugins-bad > devel/libgtop2 > net/usrsctp > net/miniupnp/miniupnpd > net/net-snmp,-main > net/net-snmp,-tkmib only REVISION-main for net-snmp, it's used for some mibII things in the agent, tkmib is just a Perl/Tk-based viewer and doesn't go anywhere near it so unaffected. > net/zabbix,-main > net/zabbix,-proxy,mysql > net/zabbix,-proxy,pgsql > net/zabbix,-proxy,sqlite3 > net/zabbix,-server,mysql > net/zabbix,-server,pgsql > net/zabbix,-web same only REVISION-main for zabbix (agent) > mail/mozilla-thunderbird > games/warzone2100
Re: struct ifnet: remove unused if_switchport member
On Fri, Aug 26, 2022 at 04:08:34PM +, Klemens Nanni wrote: > On Fri, Aug 26, 2022 at 09:53:35AM -0600, Theo de Raadt wrote: > > Klemens Nanni wrote: > > > > > On Fri, Aug 26, 2022 at 04:15:43PM +0100, Stuart Henderson wrote: > > > > On 2022/08/26 09:49, Klemens Nanni wrote: > > > > > grep and CVS agree that this is a switch(4) left-over. > > > > > > > > > > OK? > > > > > > > > This is exported to userland isn't it? > > > > > > No, everything is under _KERNEL. > > > > But there are a few libraries and programs which #define _KERNEL > > > > So when you see _KERNEL, you need to remain a bit cynical, and check. > > Oh wel... > > https://codesearch.debian.net/search?q=%23%5Cs*define%5Cs%2B_KERNEL%5Cb+filetype%3Ac=0 > > This shows many false positives, but at least libgtop2 does define > _KERNEL and includes so I guess we need to bump that. > > Let me cross-check this list with a search for and come > up with a list of ports to bump. Those define _KERNEL https://codesearch.debian.net/search?q=%23%5Cs*define%5Cs%2B_KERNEL%5Cb+filetype%3Ac=0 https://codesearch.debian.net/results/a710598a524e63cb/packages.txt Those include https://codesearch.debian.net/search?q=%3Cnet%2Fif_var.h%3E+filetype%3Ac=1 curl -s https://codesearch.debian.net/results/3264f4d6c8ec573a/packages.txt You may need to query the search URL to make the packages.txt link available (again). Running the packages.txt files through 'sort -u' and 'comm -12' and filtering for ports we actually have leaves us with aircrack-ng firefox firefox-esr gst-plugins-bad1.0 gst-plugins-bad1.0-contrib libgtop2 libusrsctp miniupnpd net-snmp qt6-webengine qtwebengine-opensource-src thunderbird warzone2100 zabbix I'm sure that some of them are false positives because they a) define _KERNEL after including b) don't do either of it on OpenBSD OK if I just REVISION bump all these and be done with it? That'd be this list of PKGPATHs: security/aircrack-ng www/firefox-esr www/mozilla-firefox multimedia/gstreamer1/plugins-bad devel/libgtop2 net/usrsctp net/miniupnp/miniupnpd net/net-snmp,-main net/net-snmp,-tkmib mail/mozilla-thunderbird games/warzone2100 net/zabbix,-main net/zabbix,-proxy,mysql net/zabbix,-proxy,pgsql net/zabbix,-proxy,sqlite3 net/zabbix,-server,mysql net/zabbix,-server,pgsql net/zabbix,-web
Re: unbound update
On Wed, Aug 24, 2022 at 03:03:01PM +0100, Stuart Henderson wrote: Anyone want to test this? Any OKs? Hello, Seemed to patch OK and built OK with a -current made yesterday, on aarch64. I'm a newbie at building/patching openbsd, so if there's anything you can suggest I test, I'll test. unbound is working. unbound -V still reports Version 1.16.0 though. --
Re: struct ifnet: remove unused if_switchport member
Mark Kettenis wrote: > > From: "Theo de Raadt" > > Date: Fri, 26 Aug 2022 09:53:35 -0600 > > > > Klemens Nanni wrote: > > > > > On Fri, Aug 26, 2022 at 04:15:43PM +0100, Stuart Henderson wrote: > > > > On 2022/08/26 09:49, Klemens Nanni wrote: > > > > > grep and CVS agree that this is a switch(4) left-over. > > > > > > > > > > OK? > > > > > > > > This is exported to userland isn't it? > > > > > > No, everything is under _KERNEL. > > > > But there are a few libraries and programs which #define _KERNEL > > Outside of base? Folks who do that deserve what they get. The upstreams won't really care, but our pkg people have to act upon it. That's why Stuart got involved...
Re: struct ifnet: remove unused if_switchport member
> From: "Theo de Raadt" > Date: Fri, 26 Aug 2022 09:53:35 -0600 > > Klemens Nanni wrote: > > > On Fri, Aug 26, 2022 at 04:15:43PM +0100, Stuart Henderson wrote: > > > On 2022/08/26 09:49, Klemens Nanni wrote: > > > > grep and CVS agree that this is a switch(4) left-over. > > > > > > > > OK? > > > > > > This is exported to userland isn't it? > > > > No, everything is under _KERNEL. > > But there are a few libraries and programs which #define _KERNEL Outside of base? Folks who do that deserve what they get.
When did PCs stop using ISA Timer 1?
Hi, TLDR: 1. When did PCs stop using ISA Timer 1 to trigger DRAM refresh? 2. Are any PCs that rely on ISA Timer 1 for DRAM refresh capable of running OpenBSD as it exists today? Long version: I have a history question for the list. Maybe one of you hardware jocks or history buffs can help me out. So, in the IBM AT/PC and, later, all ISA-compatible systems, the ISA timer (an i8253 or compatible clock) has 3 independent 16-bit counters. The first, Timer 0, is available for use by the operating system. The second, Timer 1, was traditionally programmed by the BIOS at a particular rate to trigger DRAM refresh. The third, Timer 2, is usually wired up to the PC speaker and may be used by the operating system to produce primitive sound effects. I found a more detailed explanation of what Timer 1 actually did in this book: https://ia601901.us.archive.org/12/items/ISA_System_Architecture/ISA_System_Architecture.pdf > ISA System Architecture Third Edition (1995) > Chapter 24: ISA Timers > p. 471 > > Refresh Timer (Timer 1) > > The refresh timer, or timer 1, is a programmable frequency source. The > same 1.19318MHz signal (used by timer 0) provides the refresh timer's > clock input. The programmer specifies a divisor to be divided into > the input clock to yield the desired output frequency. During the > POST, a divisor of 0012h, or a decimal 18, is written to the refresh > timer at I/O address 0041h. The input clock frequency of 1.19318MHz is > therefore divided by 18 to yield an output frequency of 66287.77Hz, > or a pulse every 15.09 microseconds. > > This is the refresh request signal that triggers the DRAM refresh > logic to become bus master once every 15.09 microseconds so it can > refresh another row in DRAM memory throughout the system. For more > information on DRAM refresh, refer to the chapter entitled "RAM > Memory: Theory of Operation." This is fascinating. But obviously this is no longer true in modern PCs. The ISA bus is still emulated in modern PCs, and DRAM in modern PCs still needs refreshing, but they don't rely on the emulated ISA timer to make it happen. So, when did PCs stop using ISA Timer 1 for DRAM refresh? The IBM AT/PC was built around the 80286. Was it with the advent of the 80386 (1985)? The 80486 (1989)? P5 (1993)? P6 (1995)? Later? Was the change independent of a particular processor generation jump? Like, maybe a technological advance in the state of the art in DRAM obsoleted the use of ISA Timer 1 for refresh? And then, more importantly, are any machines that rely on ISA Timer 1 for DRAM refresh actually capable of running OpenBSD as it exists today? -Scott
Re: struct ifnet: remove unused if_switchport member
On Fri, Aug 26, 2022 at 09:53:35AM -0600, Theo de Raadt wrote: > Klemens Nanni wrote: > > > On Fri, Aug 26, 2022 at 04:15:43PM +0100, Stuart Henderson wrote: > > > On 2022/08/26 09:49, Klemens Nanni wrote: > > > > grep and CVS agree that this is a switch(4) left-over. > > > > > > > > OK? > > > > > > This is exported to userland isn't it? > > > > No, everything is under _KERNEL. > > But there are a few libraries and programs which #define _KERNEL > > So when you see _KERNEL, you need to remain a bit cynical, and check. Oh wel... https://codesearch.debian.net/search?q=%23%5Cs*define%5Cs%2B_KERNEL%5Cb+filetype%3Ac=0 This shows many false positives, but at least libgtop2 does define _KERNEL and includes so I guess we need to bump that. Let me cross-check this list with a search for and come up with a list of ports to bump.
Re: [matth...@openbsd.org: Re: xlock don't take my password anymore]
Hi Matthieu,
I'd be inclined to go with a return in the middle and avoid some extra
variables, what do you think about the following (entirely eye-ball tested)?
char*pass;
/* buffer can be in the form style:pass */
if ((pass = strchr(buffer, ':')) != NULL) {
*pass++ = '\0';
if (priv_pw_check(user, buffer, pass))
return 1; /* or whatever success indicator */
*--pass = ':';
}
return priv_pw_check(user, buffer, NULL);
Thanks
Greg
Re: struct ifnet: remove unused if_switchport member
Klemens Nanni wrote: > On Fri, Aug 26, 2022 at 04:15:43PM +0100, Stuart Henderson wrote: > > On 2022/08/26 09:49, Klemens Nanni wrote: > > > grep and CVS agree that this is a switch(4) left-over. > > > > > > OK? > > > > This is exported to userland isn't it? > > No, everything is under _KERNEL. But there are a few libraries and programs which #define _KERNEL So when you see _KERNEL, you need to remain a bit cynical, and check.
Re: struct ifnet: remove unused if_switchport member
On Fri, Aug 26, 2022 at 04:15:43PM +0100, Stuart Henderson wrote: > On 2022/08/26 09:49, Klemens Nanni wrote: > > grep and CVS agree that this is a switch(4) left-over. > > > > OK? > > This is exported to userland isn't it? No, everything is under _KERNEL.
[matth...@openbsd.org: Re: xlock don't take my password anymore]
ok ?
(although I didn't get an answer from Jean-Michel yet, I'm quite sure
the issue is real and the fix correct).
- Forwarded message from Matthieu Herrb -
Date: Tue, 23 Aug 2022 11:08:28 +0200
From: Matthieu Herrb
To: BESSOT Jean-Michel
Cc: b...@openbsd.org
Subject: Re: xlock don't take my password anymore
On Sun, Aug 14, 2022 at 01:05:49PM +0200, BESSOT Jean-Michel wrote:
> Hello
>
> Xlock do not take my password since my last snapshot update (well there is
> time since the one before).
>
> I use a bépo keyboard so kdb is set with fr dvorak
>
Hi,
can you try the patch below (already tested by Denis, who provided a
hint on the issue) ?
(get the xenocara tree, apply in app/xlockmore using patch(1) and
rebuild xlockmore by running the following commands in
/usr/xenocara/app/xlockmore :
doas make -f Makefile.bsd-wrapper obj
doas make -f Makefile.bsd-wrapper build
Index: xlock/passwd.c
===
RCS file: /cvs/OpenBSD/xenocara/app/xlockmore/xlock/passwd.c,v
retrieving revision 1.3
diff -u -r1.3 passwd.c
--- xlock/passwd.c 26 Jun 2022 14:09:51 - 1.3
+++ xlock/passwd.c 22 Aug 2022 21:35:49 -
@@ -1279,16 +1279,19 @@
#ifdef USE_PRIVSEP
char*pass;
char*style;
+ int authok;
/* buffer can be in the form style:pass */
if ((pass = strchr(buffer, ':')) != NULL) {
*pass++ = '\0';
style = buffer;
- } else {
- pass = buffer;
- style = NULL;
- }
- return priv_pw_check(user, pass, style);
+ authok = priv_pw_check(user, style, pass);
+ *--pass = ':';
+ } else
+ authok = 0;
+ pass = buffer;
+ style = NULL;
+ return (authok || priv_pw_check(user, pass, style));
#elif defined(BSD_AUTH)
char *pass;
char *style;
--
Matthieu Herrb
- End forwarded message -
--
Matthieu Herrb
Re: struct ifnet: remove unused if_switchport member
On Fri, Aug 26, 2022 at 05:27:08PM +0200, Claudio Jeker wrote:
> On Fri, Aug 26, 2022 at 04:15:43PM +0100, Stuart Henderson wrote:
> > On 2022/08/26 09:49, Klemens Nanni wrote:
> > > grep and CVS agree that this is a switch(4) left-over.
> > >
> > > OK?
> >
> > This is exported to userland isn't it?
>
> I seariously hope not. All those caddr_t are kernel pointers.
> In general the structs in *var.h are normally only kernel internal.
>
The 'ifnet' structure is not exported to userland. Also it declared
within "#ifdef _KERNEL" block.
This diff is ok by me.
> > If so, I think all ports using it will need a bump.
> >
> >
> > > Index: if_var.h
> > > ===
> > > RCS file: /cvs/src/sys/net/if_var.h,v
> > > retrieving revision 1.114
> > > diff -u -p -r1.114 if_var.h
> > > --- if_var.h 20 Feb 2021 04:55:52 - 1.114
> > > +++ if_var.h 26 Aug 2022 09:47:37 -
> > > @@ -133,7 +133,6 @@ struct ifnet {/* and
> > > the entries */
> > > int if_pcount; /* [N] # of promiscuous listeners */
> > > unsigned int if_bridgeidx; /* [K] used by bridge ports */
> > > caddr_t if_bpf; /* packet filter structure */
> > > - caddr_t if_switchport; /* used by switch ports */
> > > caddr_t if_mcast; /* used by multicast code */
> > > caddr_t if_mcast6; /* used by IPv6 multicast code */
> > > caddr_t if_pf_kif; /* pf interface abstraction */
> > >
> >
>
> --
> :wq Claudio
>
Re: struct ifnet: remove unused if_switchport member
On Fri, Aug 26, 2022 at 04:15:43PM +0100, Stuart Henderson wrote:
> On 2022/08/26 09:49, Klemens Nanni wrote:
> > grep and CVS agree that this is a switch(4) left-over.
> >
> > OK?
>
> This is exported to userland isn't it?
I seariously hope not. All those caddr_t are kernel pointers.
In general the structs in *var.h are normally only kernel internal.
> If so, I think all ports using it will need a bump.
>
>
> > Index: if_var.h
> > ===
> > RCS file: /cvs/src/sys/net/if_var.h,v
> > retrieving revision 1.114
> > diff -u -p -r1.114 if_var.h
> > --- if_var.h20 Feb 2021 04:55:52 - 1.114
> > +++ if_var.h26 Aug 2022 09:47:37 -
> > @@ -133,7 +133,6 @@ struct ifnet { /* and the
> > entries */
> > int if_pcount; /* [N] # of promiscuous listeners */
> > unsigned int if_bridgeidx; /* [K] used by bridge ports */
> > caddr_t if_bpf; /* packet filter structure */
> > - caddr_t if_switchport; /* used by switch ports */
> > caddr_t if_mcast; /* used by multicast code */
> > caddr_t if_mcast6; /* used by IPv6 multicast code */
> > caddr_t if_pf_kif; /* pf interface abstraction */
> >
>
--
:wq Claudio
Re: struct ifnet: remove unused if_switchport member
On 2022/08/26 09:49, Klemens Nanni wrote:
> grep and CVS agree that this is a switch(4) left-over.
>
> OK?
This is exported to userland isn't it?
If so, I think all ports using it will need a bump.
> Index: if_var.h
> ===
> RCS file: /cvs/src/sys/net/if_var.h,v
> retrieving revision 1.114
> diff -u -p -r1.114 if_var.h
> --- if_var.h 20 Feb 2021 04:55:52 - 1.114
> +++ if_var.h 26 Aug 2022 09:47:37 -
> @@ -133,7 +133,6 @@ struct ifnet {/* and the
> entries */
> int if_pcount; /* [N] # of promiscuous listeners */
> unsigned int if_bridgeidx; /* [K] used by bridge ports */
> caddr_t if_bpf; /* packet filter structure */
> - caddr_t if_switchport; /* used by switch ports */
> caddr_t if_mcast; /* used by multicast code */
> caddr_t if_mcast6; /* used by IPv6 multicast code */
> caddr_t if_pf_kif; /* pf interface abstraction */
>
Re: rpki-client: use valid_uri() in load_skiplist()
On Fri, Aug 26, 2022 at 01:48:55PM +0200, Theo Buehler wrote:
> It occurred to me right after committing the previous change that it is
> doing the same thing as valid_uri(). Calling it is simpler and the
> additional "/." check won't hurt.
This is indeed OK. What worries me a bit is that the skiplist is a list
of FQDN nor URI. So using valid_uri() is a bit strange.
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.212
> diff -u -p -r1.212 main.c
> --- main.c26 Aug 2022 11:04:13 - 1.212
> +++ main.c26 Aug 2022 11:47:47 -
> @@ -674,7 +674,7 @@ load_skiplist(const char *slf)
> struct skiplistentry*sle;
> FILE*fp;
> char*line = NULL;
> - size_t linesize = 0, linelen, s;
> + size_t linesize = 0, linelen;
>
> if ((fp = fopen(slf, "r")) == NULL) {
> if (errno == ENOENT && strcmp(slf, DEFAULT_SKIPLIST_FILE) == 0)
> @@ -697,10 +697,8 @@ load_skiplist(const char *slf)
> linelen = strcspn(line, " #\r\n\t");
> line[linelen] = '\0';
>
> - for (s = 0; s < linelen; s++)
> - if (!isalnum((unsigned char)line[s]) &&
> - !ispunct((unsigned char)line[s]))
> - errx(1, "invalid entry in skiplist: %s", line);
> + if (!valid_uri(line, linelen, NULL))
> + errx(1, "invalid entry in skiplist: %s", line);
>
> if ((sle = malloc(sizeof(struct skiplistentry))) == NULL)
> err(1, NULL);
>
--
:wq Claudio
Re: bgpd fix peer signaling bug for busy systems
On Fri, Aug 26, 2022 at 01:42:15PM +0200, Theo Buehler wrote:
> On Fri, Aug 26, 2022 at 10:58:38AM +0200, Claudio Jeker wrote:
> > Noticed on a route collector with >100 full feeds and well 80Mio prefixes.
> > On startup the RDE slurps in a lot of messages and then slowly processes
> > them. Those are mostly IMSG_UDPATE but the current code also queues
> > IMSG_SESSION_DOWN, IMSG_SESSION_UP and the graceful restart imsgs.
> > It does not queue IMSG_SESSION_ADD and this is a problem.
> >
> > If the queue of a neighbor is long then an IMSG_SESSION_DOWN can be
> > delayed after processing the IMSG_SESSION_ADD which is sent when the
> > session is reestablished. The result is that a peer is removed in the
> > RDE that is actually active. Most noticable is that the log is filled
> > with "rde_dispatch: unknown peer id XYZ".
> >
> > To solve this issue IMSG_SESSION_DOWN needs to be called immeditatly.
> > For graceful restart the same is required. In the graceful restart case
> > two messages indicate a session reset (IMSG_SESSION_STALE and the new
> > IMSG_SESSION_NOGRACE). Depending on the AFI/SAFI settings in the GR
> > capability either one or the other is sent when the session is reset.
> >
> > For the above 3 messages the RDE should stop all rib dump runners and more
> > importantly flush the peer imsg queue. All those queued messages no longer
> > matter.
> >
> > IMSG_SESSION_UP could still be handled through the queue since it is the
> > first message sent after IMSG_SESSION_ADD. But I decided to also handle it
> > immediatly.
> >
> > The graceful restart imsgs IMSG_SESSION_FLUSH and IMSG_SESSION_RESTARTED
> > are there to clean up stale routes (either after a timeout or a successful
> > restart). Again it is best to handle those messages immediately and clean
> > up the RIB (even though a backlog of UPDATES may be present).
> > In this case the imsg queue should not be flushed since there may be an
> > active connection. Note: IMSG_SESSION_RESTARTED is sent based on an EoR
> > marker from an IMSG_UPDATE (so it is delayed and in order because of that).
> >
> > With this only IMSG_UPDATE and IMSG_REFRESH are queued.
> >
> > I'm running this diff on the route collector and some test systems and it
> > seems to solve the issue.
>
> This reads fine to me. One comment:
>
> > Index: rde_peer.c
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
> > retrieving revision 1.21
> > diff -u -p -r1.21 rde_peer.c
> > --- rde_peer.c 17 Aug 2022 15:15:26 - 1.21
> > +++ rde_peer.c 26 Aug 2022 08:24:49 -
> > @@ -194,7 +194,7 @@ peer_add(uint32_t id, struct peer_config
> >
> > if ((peer = peer_get(id))) {
> > memcpy(>conf, p_conf, sizeof(struct peer_config));
> > - return (NULL);
> > + return (peer);
>
> In peer_init() there is a fatalx() that ensures that PEER_ID_SELF is
> not present in the peer list. This no longer works with this change.
> I can't tell if this sanity check is important or whether it can go.
Hmm. I had to add it because I use the return value now in the
IMSG_SESSION_ADD case to track the rde_eval_all flag.
I think the check in peer_init() can go it should be impossible to trigger
this case there.
--
:wq Claudio
rpki-client: use valid_uri() in load_skiplist()
It occurred to me right after committing the previous change that it is
doing the same thing as valid_uri(). Calling it is simpler and the
additional "/." check won't hurt.
Index: main.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.212
diff -u -p -r1.212 main.c
--- main.c 26 Aug 2022 11:04:13 - 1.212
+++ main.c 26 Aug 2022 11:47:47 -
@@ -674,7 +674,7 @@ load_skiplist(const char *slf)
struct skiplistentry*sle;
FILE*fp;
char*line = NULL;
- size_t linesize = 0, linelen, s;
+ size_t linesize = 0, linelen;
if ((fp = fopen(slf, "r")) == NULL) {
if (errno == ENOENT && strcmp(slf, DEFAULT_SKIPLIST_FILE) == 0)
@@ -697,10 +697,8 @@ load_skiplist(const char *slf)
linelen = strcspn(line, " #\r\n\t");
line[linelen] = '\0';
- for (s = 0; s < linelen; s++)
- if (!isalnum((unsigned char)line[s]) &&
- !ispunct((unsigned char)line[s]))
- errx(1, "invalid entry in skiplist: %s", line);
+ if (!valid_uri(line, linelen, NULL))
+ errx(1, "invalid entry in skiplist: %s", line);
if ((sle = malloc(sizeof(struct skiplistentry))) == NULL)
err(1, NULL);
Re: bgpd fix peer signaling bug for busy systems
On Fri, Aug 26, 2022 at 10:58:38AM +0200, Claudio Jeker wrote:
> Noticed on a route collector with >100 full feeds and well 80Mio prefixes.
> On startup the RDE slurps in a lot of messages and then slowly processes
> them. Those are mostly IMSG_UDPATE but the current code also queues
> IMSG_SESSION_DOWN, IMSG_SESSION_UP and the graceful restart imsgs.
> It does not queue IMSG_SESSION_ADD and this is a problem.
>
> If the queue of a neighbor is long then an IMSG_SESSION_DOWN can be
> delayed after processing the IMSG_SESSION_ADD which is sent when the
> session is reestablished. The result is that a peer is removed in the
> RDE that is actually active. Most noticable is that the log is filled
> with "rde_dispatch: unknown peer id XYZ".
>
> To solve this issue IMSG_SESSION_DOWN needs to be called immeditatly.
> For graceful restart the same is required. In the graceful restart case
> two messages indicate a session reset (IMSG_SESSION_STALE and the new
> IMSG_SESSION_NOGRACE). Depending on the AFI/SAFI settings in the GR
> capability either one or the other is sent when the session is reset.
>
> For the above 3 messages the RDE should stop all rib dump runners and more
> importantly flush the peer imsg queue. All those queued messages no longer
> matter.
>
> IMSG_SESSION_UP could still be handled through the queue since it is the
> first message sent after IMSG_SESSION_ADD. But I decided to also handle it
> immediatly.
>
> The graceful restart imsgs IMSG_SESSION_FLUSH and IMSG_SESSION_RESTARTED
> are there to clean up stale routes (either after a timeout or a successful
> restart). Again it is best to handle those messages immediately and clean
> up the RIB (even though a backlog of UPDATES may be present).
> In this case the imsg queue should not be flushed since there may be an
> active connection. Note: IMSG_SESSION_RESTARTED is sent based on an EoR
> marker from an IMSG_UPDATE (so it is delayed and in order because of that).
>
> With this only IMSG_UPDATE and IMSG_REFRESH are queued.
>
> I'm running this diff on the route collector and some test systems and it
> seems to solve the issue.
This reads fine to me. One comment:
> Index: rde_peer.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 rde_peer.c
> --- rde_peer.c17 Aug 2022 15:15:26 - 1.21
> +++ rde_peer.c26 Aug 2022 08:24:49 -
> @@ -194,7 +194,7 @@ peer_add(uint32_t id, struct peer_config
>
> if ((peer = peer_get(id))) {
> memcpy(>conf, p_conf, sizeof(struct peer_config));
> - return (NULL);
> + return (peer);
In peer_init() there is a fatalx() that ensures that PEER_ID_SELF is
not present in the peer list. This no longer works with this change.
I can't tell if this sanity check is important or whether it can go.
struct ifnet: remove unused if_switchport member
grep and CVS agree that this is a switch(4) left-over.
OK?
Index: if_var.h
===
RCS file: /cvs/src/sys/net/if_var.h,v
retrieving revision 1.114
diff -u -p -r1.114 if_var.h
--- if_var.h20 Feb 2021 04:55:52 - 1.114
+++ if_var.h26 Aug 2022 09:47:37 -
@@ -133,7 +133,6 @@ struct ifnet { /* and the
entries */
int if_pcount; /* [N] # of promiscuous listeners */
unsigned int if_bridgeidx; /* [K] used by bridge ports */
caddr_t if_bpf; /* packet filter structure */
- caddr_t if_switchport; /* used by switch ports */
caddr_t if_mcast; /* used by multicast code */
caddr_t if_mcast6; /* used by IPv6 multicast code */
caddr_t if_pf_kif; /* pf interface abstraction */
installboot: reflect script failure in exit code
installboot(8) runs newfs_msdos(8) via system(3) but only checks failures
of the function itself, always returning zero no matter what newfs_msdos
returned.
This is bad for regress tests relying on correct return codes.
create_filesystem() itself must not exit as write_filesystem() calls it and
cleans up temporary files upon failure.
Make it return -1 if the script returned non-zero so write_filesystem()
handles it as error, cleans up and makes installboot exit 1.
Stop ignoring create_filesystem()'s return code in md_prepareboot() and
exit the same way.
Here's the change in behaviour on arm64 (newfs_msdos fails because of the
vnd/disklabel race, see "Race in disk_attach_callback?" on tech@):
# installboot -vp $(
#include
#include
+#include
#include
#include
@@ -100,16 +103,11 @@ md_prepareboot(int devfd, char *dev)
warnx("disklabel type unknown");
part = findgptefisys(devfd, );
- if (part != -1) {
- create_filesystem(, (char)part);
- return;
- }
-
- part = findmbrfat(devfd, );
- if (part != -1) {
- create_filesystem(, (char)part);
- return;
- }
+ if (part == -1)
+ part = findmbrfat(devfd, );
+ if (part != -1)
+ if (create_filesystem(, (char)part) == -1)
+ exit(1);
}
void
@@ -177,6 +175,9 @@ create_filesystem(struct disklabel *dl,
warn("system('%s') failed", cmd);
return rslt;
}
+
+ if (WIFEXITED(rslt) && WEXITSTATUS(rslt))
+ return -1;
}
return 0;
bgpd fix peer signaling bug for busy systems
Noticed on a route collector with >100 full feeds and well 80Mio prefixes.
On startup the RDE slurps in a lot of messages and then slowly processes
them. Those are mostly IMSG_UDPATE but the current code also queues
IMSG_SESSION_DOWN, IMSG_SESSION_UP and the graceful restart imsgs.
It does not queue IMSG_SESSION_ADD and this is a problem.
If the queue of a neighbor is long then an IMSG_SESSION_DOWN can be
delayed after processing the IMSG_SESSION_ADD which is sent when the
session is reestablished. The result is that a peer is removed in the
RDE that is actually active. Most noticable is that the log is filled
with "rde_dispatch: unknown peer id XYZ".
To solve this issue IMSG_SESSION_DOWN needs to be called immeditatly.
For graceful restart the same is required. In the graceful restart case
two messages indicate a session reset (IMSG_SESSION_STALE and the new
IMSG_SESSION_NOGRACE). Depending on the AFI/SAFI settings in the GR
capability either one or the other is sent when the session is reset.
For the above 3 messages the RDE should stop all rib dump runners and more
importantly flush the peer imsg queue. All those queued messages no longer
matter.
IMSG_SESSION_UP could still be handled through the queue since it is the
first message sent after IMSG_SESSION_ADD. But I decided to also handle it
immediatly.
The graceful restart imsgs IMSG_SESSION_FLUSH and IMSG_SESSION_RESTARTED
are there to clean up stale routes (either after a timeout or a successful
restart). Again it is best to handle those messages immediately and clean
up the RIB (even though a backlog of UPDATES may be present).
In this case the imsg queue should not be flushed since there may be an
active connection. Note: IMSG_SESSION_RESTARTED is sent based on an EoR
marker from an IMSG_UPDATE (so it is delayed and in order because of that).
With this only IMSG_UPDATE and IMSG_REFRESH are queued.
I'm running this diff on the route collector and some test systems and it
seems to solve the issue.
--
:wq Claudio
Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.449
diff -u -p -r1.449 bgpd.h
--- bgpd.h 10 Aug 2022 14:17:01 - 1.449
+++ bgpd.h 26 Aug 2022 08:24:49 -
@@ -592,6 +592,7 @@ enum imsg_type {
IMSG_SESSION_UP,
IMSG_SESSION_DOWN,
IMSG_SESSION_STALE,
+ IMSG_SESSION_NOGRACE,
IMSG_SESSION_FLUSH,
IMSG_SESSION_RESTARTED,
IMSG_SESSION_DEPENDON,
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.564
diff -u -p -r1.564 rde.c
--- rde.c 17 Aug 2022 15:15:26 - 1.564
+++ rde.c 26 Aug 2022 08:24:49 -
@@ -358,10 +358,10 @@ rde_dispatch_imsg_session(struct imsgbuf
{
struct imsg imsg;
struct peer p;
- struct peer_config pconf;
struct ctl_show_set cset;
struct ctl_show_rib csr;
struct ctl_show_rib_request req;
+ struct session_upsup;
struct rde_peer *peer;
struct rde_aspath *asp;
struct rde_hashstats rdehash;
@@ -373,6 +373,7 @@ rde_dispatch_imsg_session(struct imsgbuf
size_t aslen;
int verbose;
uint16_t len;
+ uint8_t aid;
while (ibuf) {
if ((n = imsg_get(ibuf, )) == -1)
@@ -382,11 +383,6 @@ rde_dispatch_imsg_session(struct imsgbuf
switch (imsg.hdr.type) {
case IMSG_UPDATE:
- case IMSG_SESSION_UP:
- case IMSG_SESSION_DOWN:
- case IMSG_SESSION_STALE:
- case IMSG_SESSION_FLUSH:
- case IMSG_SESSION_RESTARTED:
case IMSG_REFRESH:
if ((peer = peer_get(imsg.hdr.peerid)) == NULL) {
log_warnx("rde_dispatch: unknown peer id %d",
@@ -396,14 +392,71 @@ rde_dispatch_imsg_session(struct imsgbuf
peer_imsg_push(peer, );
break;
case IMSG_SESSION_ADD:
- if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(pconf))
+ if (imsg.hdr.len - IMSG_HEADER_SIZE !=
+ sizeof(struct peer_config))
+ fatalx("incorrect size of session request");
+ peer = peer_add(imsg.hdr.peerid, imsg.data);
+ /* make sure rde_eval_all is on if needed. */
+ if (peer->conf.flags & PEERFLAG_EVALUATE_ALL)
+ rde_eval_all = 1;
+ break;
+ case IMSG_SESSION_UP:
+ if ((peer = peer_get(imsg.hdr.peerid)) == NULL) {
+ log_warnx("%s: unknown peer id
Re: bioctl: sync usage with manual, simplify option list
yep, ok. jmc On 26 August 2022 09:33:00 BST, Klemens Nanni wrote: >On Fri, Aug 26, 2022 at 06:25:56AM +0100, Jason McIntyre wrote: >> On Thu, Aug 25, 2022 at 09:19:09PM +, Klemens Nanni wrote: >> > -l takes chunks as per the manual, not specials. >> > >> > I also think that comma separated lists are marked up overly >> > confusing, so reduce it by one level, i.e. turn >> >-l chunk[,chunk[,...]]] >> > into >> >-l chunk[,...]] >> > >> > Feedback? OK? >> > >> >> hi. >> >> i think the idea of simplifying this is fine. >> >> remember, if you change the formatting in SYNOPSIS you will have to >> mirror that in the actual options list too. > >Of course, thanks. > >> >> personally i would not even attempt to show the optional part and just >> rely on the text description to say how multiple items can be given. >> the author obviously wanted to show that though. > >I also prefer -l chunk[,...] over -l chunks or -l list or so. > >OK? > > >Index: bioctl.8 >=== >RCS file: /cvs/src/sbin/bioctl/bioctl.8,v >retrieving revision 1.109 >diff -u -p -r1.109 bioctl.8 >--- bioctl.8 8 Feb 2021 11:20:03 - 1.109 >+++ bioctl.8 26 Aug 2022 08:31:26 - >@@ -42,10 +42,10 @@ > .Pp > .Nm bioctl > .Op Fl dhiPqsv >-.Op Fl C Ar flag Ns Op Pf , Ar flag Ns Op Pf , Ar ... >+.Op Fl C Ar flag Ns Op Pf , Ar ... > .Op Fl c Ar raidlevel > .Op Fl k Ar keydisk >-.Op Fl l Ar chunk Ns Op Pf , Ar chunk Ns Op Pf , Ar ... >+.Op Fl l Ar chunk Ns Op Pf , Ar ... > .Op Fl O Ar chunk | channel : Ns Ar target Ns Op Pf . Ar lun > .Op Fl p Ar passfile > .Op Fl R Ar chunk | channel : Ns Ar target Ns Op Pf . Ar lun >@@ -183,7 +183,7 @@ the options for > .Xr softraid 4 > devices are as follows: > .Bl -tag -width Ds >-.It Fl C Ar flag Ns Op Pf , Ar flag Ns Op Pf , Ar ... >+.It Fl C Ar flag Ns Op Pf , Ar ... > Pass > .Ar flag > to >@@ -249,7 +249,7 @@ Detach volume specified by > Use special device > .Ar keydisk > as a key disk for a crypto volume. >-.It Fl l Ar chunk Ns Op Pf , Ar chunk Ns Op Pf , Ar ... >+.It Fl l Ar chunk Ns Op Pf , Ar ... > Use the > .Ar chunk > device list to create a new volume within the >Index: bioctl.c >=== >RCS file: /cvs/src/sbin/bioctl/bioctl.c,v >retrieving revision 1.148 >diff -u -p -r1.148 bioctl.c >--- bioctl.c 19 Aug 2022 17:49:10 - 1.148 >+++ bioctl.c 25 Aug 2022 21:16:01 - >@@ -290,8 +290,8 @@ usage(void) > "[-u channel:target[.lun]] " > "device\n" > " %s [-dhiPqsv] " >- "[-C flag[,flag,...]] [-c raidlevel] [-k keydisk]\n" >- "\t[-l special[,special,...]] " >+ "[-C flag[,...]] [-c raidlevel] [-k keydisk]\n" >+ "\t[-l chunk[,...]] " > "[-O device | channel:target[.lun]]\n" > "\t[-p passfile] [-R chunk | channel:target[.lun]]\n" > "\t[-r rounds] " >
Re: bioctl: sync usage with manual, simplify option list
On Fri, Aug 26, 2022 at 06:25:56AM +0100, Jason McIntyre wrote: > On Thu, Aug 25, 2022 at 09:19:09PM +, Klemens Nanni wrote: > > -l takes chunks as per the manual, not specials. > > > > I also think that comma separated lists are marked up overly > > confusing, so reduce it by one level, i.e. turn > > -l chunk[,chunk[,...]]] > > into > > -l chunk[,...]] > > > > Feedback? OK? > > > > hi. > > i think the idea of simplifying this is fine. > > remember, if you change the formatting in SYNOPSIS you will have to > mirror that in the actual options list too. Of course, thanks. > > personally i would not even attempt to show the optional part and just > rely on the text description to say how multiple items can be given. > the author obviously wanted to show that though. I also prefer -l chunk[,...] over -l chunks or -l list or so. OK? Index: bioctl.8 === RCS file: /cvs/src/sbin/bioctl/bioctl.8,v retrieving revision 1.109 diff -u -p -r1.109 bioctl.8 --- bioctl.88 Feb 2021 11:20:03 - 1.109 +++ bioctl.826 Aug 2022 08:31:26 - @@ -42,10 +42,10 @@ .Pp .Nm bioctl .Op Fl dhiPqsv -.Op Fl C Ar flag Ns Op Pf , Ar flag Ns Op Pf , Ar ... +.Op Fl C Ar flag Ns Op Pf , Ar ... .Op Fl c Ar raidlevel .Op Fl k Ar keydisk -.Op Fl l Ar chunk Ns Op Pf , Ar chunk Ns Op Pf , Ar ... +.Op Fl l Ar chunk Ns Op Pf , Ar ... .Op Fl O Ar chunk | channel : Ns Ar target Ns Op Pf . Ar lun .Op Fl p Ar passfile .Op Fl R Ar chunk | channel : Ns Ar target Ns Op Pf . Ar lun @@ -183,7 +183,7 @@ the options for .Xr softraid 4 devices are as follows: .Bl -tag -width Ds -.It Fl C Ar flag Ns Op Pf , Ar flag Ns Op Pf , Ar ... +.It Fl C Ar flag Ns Op Pf , Ar ... Pass .Ar flag to @@ -249,7 +249,7 @@ Detach volume specified by Use special device .Ar keydisk as a key disk for a crypto volume. -.It Fl l Ar chunk Ns Op Pf , Ar chunk Ns Op Pf , Ar ... +.It Fl l Ar chunk Ns Op Pf , Ar ... Use the .Ar chunk device list to create a new volume within the Index: bioctl.c === RCS file: /cvs/src/sbin/bioctl/bioctl.c,v retrieving revision 1.148 diff -u -p -r1.148 bioctl.c --- bioctl.c19 Aug 2022 17:49:10 - 1.148 +++ bioctl.c25 Aug 2022 21:16:01 - @@ -290,8 +290,8 @@ usage(void) "[-u channel:target[.lun]] " "device\n" " %s [-dhiPqsv] " - "[-C flag[,flag,...]] [-c raidlevel] [-k keydisk]\n" - "\t[-l special[,special,...]] " + "[-C flag[,...]] [-c raidlevel] [-k keydisk]\n" + "\t[-l chunk[,...]] " "[-O device | channel:target[.lun]]\n" "\t[-p passfile] [-R chunk | channel:target[.lun]]\n" "\t[-r rounds] "
Re: rpki-client: two skiplist tweaks
On Fri, Aug 26, 2022 at 09:57:19AM +0200, Theo Buehler wrote:
> First, if there's an issue opening the default skip list file other than
> its absence (most likely bad permissions), we should not silently ignore
> it. Also, let's display the error, so use err().
>
> Second, linelen, the return value of getline() is not currently used.
> Since strcspn() already computes the length of the string we're
> interested in, we can save that and use it instead of calling strlen().
OK claudio@
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.209
> diff -u -p -r1.209 main.c
> --- main.c4 Aug 2022 13:44:07 - 1.209
> +++ main.c26 Aug 2022 07:51:06 -
> @@ -674,16 +674,15 @@ load_skiplist(const char *slf)
> struct skiplistentry*sle;
> FILE*fp;
> char*line = NULL;
> - size_t linesize = 0, s;
> - ssize_t linelen;
> + size_t linesize = 0, linelen, s;
>
> if ((fp = fopen(slf, "r")) == NULL) {
> - if (strcmp(slf, DEFAULT_SKIPLIST_FILE) != 0)
> - errx(1, "failed to open skiplist %s", slf);
> - return;
> + if (errno == ENOENT && strcmp(slf, DEFAULT_SKIPLIST_FILE) == 0)
> + return;
> + err(1, "failed to open %s", slf);
> }
>
> - while ((linelen = getline(, , fp)) != -1) {
> + while (getline(, , fp) != -1) {
> /* just eat comment lines or empty lines*/
> if (line[0] == '#' || line[0] == '\n')
> continue;
> @@ -695,9 +694,10 @@ load_skiplist(const char *slf)
>* Ignore anything after comment sign, whitespaces,
>* also chop off LF or CR.
>*/
> - line[strcspn(line, " #\r\n\t")] = 0;
> + linelen = strcspn(line, " #\r\n\t");
> + line[linelen] = '\0';
>
> - for (s = 0; s < strlen(line); s++)
> + for (s = 0; s < linelen; s++)
> if (!isalnum((unsigned char)line[s]) &&
> !ispunct((unsigned char)line[s]))
> errx(1, "invalid entry in skiplist: %s", line);
>
--
:wq Claudio
rpki-client: two skiplist tweaks
First, if there's an issue opening the default skip list file other than
its absence (most likely bad permissions), we should not silently ignore
it. Also, let's display the error, so use err().
Second, linelen, the return value of getline() is not currently used.
Since strcspn() already computes the length of the string we're
interested in, we can save that and use it instead of calling strlen().
Index: main.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.209
diff -u -p -r1.209 main.c
--- main.c 4 Aug 2022 13:44:07 - 1.209
+++ main.c 26 Aug 2022 07:51:06 -
@@ -674,16 +674,15 @@ load_skiplist(const char *slf)
struct skiplistentry*sle;
FILE*fp;
char*line = NULL;
- size_t linesize = 0, s;
- ssize_t linelen;
+ size_t linesize = 0, linelen, s;
if ((fp = fopen(slf, "r")) == NULL) {
- if (strcmp(slf, DEFAULT_SKIPLIST_FILE) != 0)
- errx(1, "failed to open skiplist %s", slf);
- return;
+ if (errno == ENOENT && strcmp(slf, DEFAULT_SKIPLIST_FILE) == 0)
+ return;
+ err(1, "failed to open %s", slf);
}
- while ((linelen = getline(, , fp)) != -1) {
+ while (getline(, , fp) != -1) {
/* just eat comment lines or empty lines*/
if (line[0] == '#' || line[0] == '\n')
continue;
@@ -695,9 +694,10 @@ load_skiplist(const char *slf)
* Ignore anything after comment sign, whitespaces,
* also chop off LF or CR.
*/
- line[strcspn(line, " #\r\n\t")] = 0;
+ linelen = strcspn(line, " #\r\n\t");
+ line[linelen] = '\0';
- for (s = 0; s < strlen(line); s++)
+ for (s = 0; s < linelen; s++)
if (!isalnum((unsigned char)line[s]) &&
!ispunct((unsigned char)line[s]))
errx(1, "invalid entry in skiplist: %s", line);
Re: bioctl: sync usage with manual, simplify option list
On Thu, Aug 25, 2022 at 09:19:09PM +, Klemens Nanni wrote: > -l takes chunks as per the manual, not specials. > > I also think that comma separated lists are marked up overly > confusing, so reduce it by one level, i.e. turn > -l chunk[,chunk[,...]]] > into > -l chunk[,...]] > > Feedback? OK? > hi. i think the idea of simplifying this is fine. remember, if you change the formatting in SYNOPSIS you will have to mirror that in the actual options list too. personally i would not even attempt to show the optional part and just rely on the text description to say how multiple items can be given. the author obviously wanted to show that though. jmc > Index: bioctl.8 > === > RCS file: /cvs/src/sbin/bioctl/bioctl.8,v > retrieving revision 1.109 > diff -u -p -r1.109 bioctl.8 > --- bioctl.8 8 Feb 2021 11:20:03 - 1.109 > +++ bioctl.8 25 Aug 2022 21:11:23 - > @@ -42,10 +42,10 @@ > .Pp > .Nm bioctl > .Op Fl dhiPqsv > -.Op Fl C Ar flag Ns Op Pf , Ar flag Ns Op Pf , Ar ... > +.Op Fl C Ar flag Ns Op Pf , Ar ... > .Op Fl c Ar raidlevel > .Op Fl k Ar keydisk > -.Op Fl l Ar chunk Ns Op Pf , Ar chunk Ns Op Pf , Ar ... > +.Op Fl l Ar chunk Ns Op Pf , Ar ... > .Op Fl O Ar chunk | channel : Ns Ar target Ns Op Pf . Ar lun > .Op Fl p Ar passfile > .Op Fl R Ar chunk | channel : Ns Ar target Ns Op Pf . Ar lun > Index: bioctl.c > === > RCS file: /cvs/src/sbin/bioctl/bioctl.c,v > retrieving revision 1.148 > diff -u -p -r1.148 bioctl.c > --- bioctl.c 19 Aug 2022 17:49:10 - 1.148 > +++ bioctl.c 25 Aug 2022 21:16:01 - > @@ -290,8 +290,8 @@ usage(void) > "[-u channel:target[.lun]] " > "device\n" > " %s [-dhiPqsv] " > - "[-C flag[,flag,...]] [-c raidlevel] [-k keydisk]\n" > - "\t[-l special[,special,...]] " > + "[-C flag[,...]] [-c raidlevel] [-k keydisk]\n" > + "\t[-l chunk[,...]] " > "[-O device | channel:target[.lun]]\n" > "\t[-p passfile] [-R chunk | channel:target[.lun]]\n" > "\t[-r rounds] " >
