remove cpu var from i386
removed from amd64 in locore.S 1.71 in 2015
the cpuid_level == -1 path can go in a later step
diff --git sys/arch/amd64/include/cpu.h sys/arch/amd64/include/cpu.h
index b8db48f2714..d8b7bc61ede 100644
--- sys/arch/amd64/include/cpu.h
+++ sys/arch/amd64/include/cpu.h
@@ -369,7 +369,6 @@ struct timeval;
/* locore.S */
extern int biosbasemem;
extern int biosextmem;
-extern int cpu;
extern int cpu_feature;
extern int cpu_ebxfeature;
extern int cpu_ecxfeature;
diff --git sys/arch/i386/i386/locore0.S sys/arch/i386/i386/locore0.S
index 8d03facc7b7..cba7edd53ba 100644
--- sys/arch/i386/i386/locore0.S
+++ sys/arch/i386/i386/locore0.S
@@ -131,7 +131,7 @@ start: movw$0x1234,0x472 # warm
boot
testl %eax,%eax
jnz .Ltry586
-.Lis486: movl$CPU_486,RELOC(_C_LABEL(cpu))
+.Lis486:
jmp 2f
diff --git sys/arch/i386/i386/machdep.c sys/arch/i386/i386/machdep.c
index 36f3eef49e5..364132760fc 100644
--- sys/arch/i386/i386/machdep.c
+++ sys/arch/i386/i386/machdep.c
@@ -493,15 +493,6 @@ i386_init_pcb_tss(struct cpu_info *ci)
*/
char cpu_model[120];
-/*
- * Note: these are just the ones that may not have a cpuid instruction.
- * We deal with the rest in a different way.
- */
-const struct cpu_nocpuid_nameclass i386_nocpuid_cpus[] = {
- { CPUVENDOR_INTEL, "Intel", "486DX",CPUCLASS_486,
- NULL}, /* CPU_486 */
-};
-
const char *classnames[] = {
"",
"486",
@@ -1672,18 +1663,13 @@ identifycpu(struct cpu_info *ci)
extern uint32_t cpu_meltdown;
if (cpuid_level == -1) {
-#ifdef DIAGNOSTIC
- if (cpu < 0 || cpu >=
- (sizeof i386_nocpuid_cpus/sizeof(struct
cpu_nocpuid_nameclass)))
- panic("unknown cpu type %d", cpu);
-#endif
- name = i386_nocpuid_cpus[cpu].cpu_name;
- vendor = i386_nocpuid_cpus[cpu].cpu_vendor;
- vendorname = i386_nocpuid_cpus[cpu].cpu_vendorname;
+ name = "486DX";
+ vendor = CPUVENDOR_INTEL;
+ vendorname = "Intel";
model = -1;
step = -1;
- class = i386_nocpuid_cpus[cpu].cpu_class;
- ci->cpu_setup = i386_nocpuid_cpus[cpu].cpu_setup;
+ class = CPUCLASS_486;
+ ci->cpu_setup = NULL;
modifier = "";
token = "";
} else {
diff --git sys/arch/i386/include/cpu.h sys/arch/i386/include/cpu.h
index 5f300710562..e6f2237eafe 100644
--- sys/arch/i386/include/cpu.h
+++ sys/arch/i386/include/cpu.h
@@ -342,7 +342,6 @@ struct cpu_cpuid_feature {
};
/* locore.s */
-extern int cpu;
extern int cpu_id;
extern char cpu_vendor[]; /* note: NOT nul-terminated */
extern char cpu_brandstr[];
diff --git sys/arch/i386/include/cputypes.h sys/arch/i386/include/cputypes.h
index fd7c6b0bd3d..52f1f5318ec 100644
--- sys/arch/i386/include/cputypes.h
+++ sys/arch/i386/include/cputypes.h
@@ -36,12 +36,6 @@
#defineCPUCLASS_5862
#defineCPUCLASS_6863
-/*
- * Kind of Processor.
- */
-
-#defineCPU_486 0 /* Intel 80486DX */
-
/*
* CPU vendors
*/
USB string descriptor requests
I got a StarTech SV431USBDDM KVM switch, and it specifically mangles the
string descriptor responses for my keyboard (which otherwise works just
fine) when probed about the vendor string after plugging in or resetting
the device.
After a ton of messing around, I also found out that this problem just
isn't happening at all if OpenBSD only sends a single string descriptor
request with the maximum possible length, rather than two as it
currently does.
According to the USB 2.0 and 3.2 specs, devices that receive a larger
wLength than the descriptor's actual length /should/ just send the full
descriptor in a short packet. Of course, the current behavior isn't
wrong either, if the wLength is shorter, then only the initial bytes or
length (depending on the spec) /should/ be sent.
(So unless there's some deeper bug in OpenBSD, my KVM switch is totally
dropping the ball either way. I'll probably post about it on Misc later,
unless details are desired here...)
But I'm curious if there's a reason for sending two separate packets
when one should be fine. Do certain USB devices expect it that way?
I suppose it seems Windows does it this way too (the only reason the
switch works there is because they neglect to check the vendor string),
so the answer to that could very well be a hard yes...
This is my first post, so sorry in advance if I did anything dumb. Diff
provided for context.
Index: usb_subr.c
===
RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.158
diff -u -p -r1.158 usb_subr.c
--- usb_subr.c 16 Feb 2022 06:23:42 - 1.158
+++ usb_subr.c 21 Aug 2022 03:33:10 -
@@ -125,20 +125,17 @@ usbd_get_string_desc(struct usbd_device
req.bRequest = UR_GET_DESCRIPTOR;
USETW2(req.wValue, UDESC_STRING, sindex);
USETW(req.wIndex, langid);
- USETW(req.wLength, 2); /* size and descriptor type first */
+ USETW(req.wLength, sizeof(*sdesc)); /* size and descriptor type
first */
+
err = usbd_do_request_flags(dev, , sdesc, USBD_SHORT_XFER_OK,
, USBD_DEFAULT_TIMEOUT);
+
if (err)
return (err);
if (actlen < 2)
return (USBD_SHORT_XFER);
- USETW(req.wLength, sdesc->bLength); /* the whole string */
- err = usbd_do_request_flags(dev, , sdesc, USBD_SHORT_XFER_OK,
- , USBD_DEFAULT_TIMEOUT);
- if (err)
- return (err);
if (actlen != sdesc->bLength) {
DPRINTFN(-1, ("%s: expected %d, got %d\n", __func__,
Re: Consistency and cleanup in /share/misc/airport
I've committed the updates for the German airports but left the metropolitan area airports alone. Thanks!
Re: improve transmeta pci device names
On Sat, Aug 20, 2022 at 09:52:15PM -0400, Daniel Dickman wrote: > On Sat, Aug 20, 2022 at 9:50 PM Jonathan Gray wrote: > > why not just > > > > product TRANSMETA SDRAM 0x0396 SDRAM > > product TRANSMETA BIOS 0x0397 BIOS > > > That works too. ok jsg@ for that version
Re: improve transmeta pci device names
On Sat, Aug 20, 2022 at 9:50 PM Jonathan Gray wrote: > why not just > > product TRANSMETA SDRAM 0x0396 SDRAM > product TRANSMETA BIOS 0x0397 BIOS That works too.
Re: improve transmeta pci device names
On Sat, Aug 20, 2022 at 09:04:23PM -0400, Daniel Dickman wrote: > We have some generic PCI devices names: > > product TRANSMETA MEM1 0x0396 Mem1 > product TRANSMETA MEM2 0x0397 Mem2 > > Likely because these devices both appear as class=5 and subclass=0 (which > indicates a RAM device): > > # pcidump -v > ... > 0:0:1: Transmeta Mem1 > 0x: Vendor ID: 1279, Product ID: 0396 > 0x0004: Command: , Status: > 0x0008: Class: 05 Memory, Subclass: 00 RAM, > > > NetBSD has: > > product TRANSMETA SDRAM 0x0396 SDRAM Controller > product TRANSMETA BIOS_SCRATCH0x0397 BIOS Scratchpad > > The Transmeta BIOS Programmer's Guide appears to confirm that the NetBSD > device names are correct. A copy of this guide is here: > > http://datasheets.chipdb.org/Transmeta/Crusoe/TM5800/TM5800_BIOSGuide_6-14-02.pdf > > Patch below updates the PCI device names for these 2 devices. > > ok? > > > Index: pcidevs > === > RCS file: /cvs/src/sys/dev/pci/pcidevs,v > retrieving revision 1.2001 > diff -u -p -u -r1.2001 pcidevs > --- pcidevs 16 Aug 2022 09:28:45 - 1.2001 > +++ pcidevs 21 Aug 2022 00:52:28 - > @@ -8886,8 +8886,8 @@ product TOSHIBA2 SDCARD 0x0805 SD > /* Transmeta products */ > product TRANSMETA NB 0x0295 Northbridge > product TRANSMETA LONGRUN_NB 0x0395 LongRun Northbridge > -product TRANSMETA MEM1 0x0396 Mem1 > -product TRANSMETA MEM2 0x0397 Mem2 > +product TRANSMETA SDRAM 0x0396 SDRAM Controller > +product TRANSMETA BIOS_SP0x0397 BIOS Scratchpad why not just product TRANSMETA SDRAM 0x0396 SDRAM product TRANSMETA BIOS 0x0397 BIOS
improve transmeta pci device names
We have some generic PCI devices names:
product TRANSMETA MEM1 0x0396 Mem1
product TRANSMETA MEM2 0x0397 Mem2
Likely because these devices both appear as class=5 and subclass=0 (which
indicates a RAM device):
# pcidump -v
...
0:0:1: Transmeta Mem1
0x: Vendor ID: 1279, Product ID: 0396
0x0004: Command: , Status:
0x0008: Class: 05 Memory, Subclass: 00 RAM,
NetBSD has:
product TRANSMETA SDRAM 0x0396 SDRAM Controller
product TRANSMETA BIOS_SCRATCH 0x0397 BIOS Scratchpad
The Transmeta BIOS Programmer's Guide appears to confirm that the NetBSD
device names are correct. A copy of this guide is here:
http://datasheets.chipdb.org/Transmeta/Crusoe/TM5800/TM5800_BIOSGuide_6-14-02.pdf
Patch below updates the PCI device names for these 2 devices.
ok?
Index: pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.2001
diff -u -p -u -r1.2001 pcidevs
--- pcidevs 16 Aug 2022 09:28:45 - 1.2001
+++ pcidevs 21 Aug 2022 00:52:28 -
@@ -8886,8 +8886,8 @@ product TOSHIBA2 SDCARD 0x0805 SD
/* Transmeta products */
product TRANSMETA NB 0x0295 Northbridge
product TRANSMETA LONGRUN_NB 0x0395 LongRun Northbridge
-product TRANSMETA MEM1 0x0396 Mem1
-product TRANSMETA MEM2 0x0397 Mem2
+product TRANSMETA SDRAM0x0396 SDRAM Controller
+product TRANSMETA BIOS_SP 0x0397 BIOS Scratchpad
/* Trident products */
product TRIDENT 4DWAVE_DX 0x2000 4DWAVE DX
Index: pcidevs.h
===
RCS file: /cvs/src/sys/dev/pci/pcidevs.h,v
retrieving revision 1.1995
diff -u -p -u -r1.1995 pcidevs.h
--- pcidevs.h 16 Aug 2022 09:29:21 - 1.1995
+++ pcidevs.h 21 Aug 2022 00:52:29 -
@@ -8891,8 +8891,8 @@
/* Transmeta products */
#definePCI_PRODUCT_TRANSMETA_NB0x0295 /* Northbridge
*/
#definePCI_PRODUCT_TRANSMETA_LONGRUN_NB0x0395 /*
LongRun Northbridge */
-#definePCI_PRODUCT_TRANSMETA_MEM1 0x0396 /* Mem1 */
-#definePCI_PRODUCT_TRANSMETA_MEM2 0x0397 /* Mem2 */
+#definePCI_PRODUCT_TRANSMETA_SDRAM 0x0396 /* SDRAM
Controller */
+#definePCI_PRODUCT_TRANSMETA_BIOS_SP 0x0397 /* BIOS
Scratchpad */
/* Trident products */
#definePCI_PRODUCT_TRIDENT_4DWAVE_DX 0x2000 /* 4DWAVE DX */
Index: pcidevs_data.h
===
RCS file: /cvs/src/sys/dev/pci/pcidevs_data.h,v
retrieving revision 1.1990
diff -u -p -u -r1.1990 pcidevs_data.h
--- pcidevs_data.h 16 Aug 2022 09:29:21 - 1.1990
+++ pcidevs_data.h 21 Aug 2022 00:52:30 -
@@ -31756,12 +31756,12 @@ static const struct pci_known_product pc
"LongRun Northbridge",
},
{
- PCI_VENDOR_TRANSMETA, PCI_PRODUCT_TRANSMETA_MEM1,
- "Mem1",
+ PCI_VENDOR_TRANSMETA, PCI_PRODUCT_TRANSMETA_SDRAM,
+ "SDRAM Controller",
},
{
- PCI_VENDOR_TRANSMETA, PCI_PRODUCT_TRANSMETA_MEM2,
- "Mem2",
+ PCI_VENDOR_TRANSMETA, PCI_PRODUCT_TRANSMETA_BIOS_SP,
+ "BIOS Scratchpad",
},
{
PCI_VENDOR_TRIDENT, PCI_PRODUCT_TRIDENT_4DWAVE_DX,
Re: regress: vmd: disable on i386
Klemens Nanni writes:
> vmd/Makefile filters for amd64 itself but still, no need to enter
> on !amd64.
>
ok dv@.
> Index: ../Makefile
> ===
> RCS file: /cvs/src/regress/usr.sbin/Makefile,v
> retrieving revision 1.26
> diff -u -p -r1.26 Makefile
> --- ../Makefile 11 Nov 2021 10:03:54 - 1.26
> +++ ../Makefile 20 Aug 2022 09:01:56 -
> @@ -16,7 +16,7 @@ SUBDIR += rpki-client
> SUBDIR += snmpd
> SUBDIR += syslogd
>
> -.if ${MACHINE} == "amd64" || ${MACHINE} == "i386"
> +.if ${MACHINE} == "amd64"
> SUBDIR += vmd
> .endif
>
Change soabort() return type to void
We never interesting soabort() return value.
I don't see any reason to change pru_abort() return type right now,
because it calls (*pru_usrreq)() within. I'll modify it with the
upcoming PRU_ABORT split.
Index: sys/kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.283
diff -u -p -r1.283 uipc_socket.c
--- sys/kern/uipc_socket.c 15 Aug 2022 09:11:38 - 1.283
+++ sys/kern/uipc_socket.c 20 Aug 2022 22:50:46 -
@@ -407,7 +407,7 @@ drop:
(void) soqremque(so2, 0);
if (persocket)
sounlock(so);
- (void) soabort(so2);
+ soabort(so2);
if (persocket)
solock(so);
}
@@ -417,7 +417,7 @@ drop:
(void) soqremque(so2, 1);
if (persocket)
sounlock(so);
- (void) soabort(so2);
+ soabort(so2);
if (persocket)
solock(so);
}
@@ -431,12 +431,11 @@ discard:
return (error);
}
-int
+void
soabort(struct socket *so)
{
soassertlocked(so);
-
- return pru_abort(so);
+ pru_abort(so);
}
int
Index: sys/netinet/tcp_input.c
===
RCS file: /cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.377
diff -u -p -r1.377 tcp_input.c
--- sys/netinet/tcp_input.c 11 Aug 2022 09:13:21 - 1.377
+++ sys/netinet/tcp_input.c 20 Aug 2022 22:50:46 -
@@ -3659,7 +3659,7 @@ resetandabort:
abort:
m_freem(m);
if (so != NULL)
- (void) soabort(so);
+ soabort(so);
syn_cache_put(sc);
tcpstat_inc(tcps_sc_aborted);
return ((struct socket *)(-1));
Index: sys/sys/socketvar.h
===
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.107
diff -u -p -r1.107 socketvar.h
--- sys/sys/socketvar.h 13 Aug 2022 21:01:46 - 1.107
+++ sys/sys/socketvar.h 20 Aug 2022 22:50:46 -
@@ -313,7 +313,7 @@ int sbreserve(struct socket *, struct so
intsbwait(struct socket *, struct sockbuf *);
intsb_lock(struct sockbuf *);
void soinit(void);
-intsoabort(struct socket *);
+void soabort(struct socket *);
intsoaccept(struct socket *, struct mbuf *);
intsobind(struct socket *, struct mbuf *, struct proc *);
void socantrcvmore(struct socket *);
Re: pcb mutex for divert input
> On 21 Aug 2022, at 00:31, Alexander Bluhm wrote:
>
> Hi,
>
> A inpcb mutex seems usable to serialize access to socket receive
> buffer. I have tried it for divert and udp input.
>
> As first step I would like to replace the kernel lock with a per
> PCB mutex in divert input.
>
> ok?
>
ok mvs@
> bluhm
>
> Index: netinet/in_pcb.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
> retrieving revision 1.270
> diff -u -p -r1.270 in_pcb.c
> --- netinet/in_pcb.c 8 Aug 2022 12:06:30 - 1.270
> +++ netinet/in_pcb.c 20 Aug 2022 21:21:13 -
> @@ -236,6 +236,7 @@ in_pcballoc(struct socket *so, struct in
> inp->inp_table = table;
> inp->inp_socket = so;
> refcnt_init_trace(>inp_refcnt, DT_REFCNT_IDX_INPCB);
> + mtx_init(>inp_mtx, IPL_SOFTNET);
> inp->inp_seclevel[SL_AUTH] = IPSEC_AUTH_LEVEL_DEFAULT;
> inp->inp_seclevel[SL_ESP_TRANS] = IPSEC_ESP_TRANS_LEVEL_DEFAULT;
> inp->inp_seclevel[SL_ESP_NETWORK] = IPSEC_ESP_NETWORK_LEVEL_DEFAULT;
> Index: netinet/in_pcb.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.h,v
> retrieving revision 1.129
> diff -u -p -r1.129 in_pcb.h
> --- netinet/in_pcb.h 15 May 2022 09:12:20 - 1.129
> +++ netinet/in_pcb.h 20 Aug 2022 21:21:13 -
> @@ -79,6 +79,7 @@
> *I immutable after creation
> *N net lock
> *t inpt_mtxpcb table mutex
> + * p inpcb_mtx pcb mutex
> */
>
> struct pf_state_key;
> @@ -121,6 +122,7 @@ struct inpcb {
> #define inp_route inp_ru.ru_route
> #define inp_route6 inp_ru.ru_route6
> structrefcnt inp_refcnt;/* refcount PCB, delay memory free */
> + structmutex inp_mtx;/* protect PCB and socket members */
> int inp_flags;/* generic IP/datagram flags */
> union { /* Header prototype. */
> struct ip hu_ip;
> Index: netinet/ip_divert.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_divert.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 ip_divert.c
> --- netinet/ip_divert.c 15 Aug 2022 09:11:39 - 1.69
> +++ netinet/ip_divert.c 20 Aug 2022 21:20:19 -
> @@ -227,22 +227,15 @@ divert_packet(struct mbuf *m, int dir, u
> if_put(ifp);
> }
>
> + mtx_enter(>inp_mtx);
> so = inp->inp_socket;
> - /*
> - * XXXSMP sbappendaddr() is not MP safe and this function is called
> - * from pf with shared netlock. To call only one sbappendaddr() from
> - * divert_packet(), protect it with kernel lock. All other places
> - * call sbappendaddr() with exclusive net lock. This blocks
> - * divert_packet() as we have the shared lock.
> - */
> - KERNEL_LOCK();
> if (sbappendaddr(so, >so_rcv, sintosa(), m, NULL) == 0) {
> - KERNEL_UNLOCK();
> + mtx_leave(>inp_mtx);
> divstat_inc(divs_fullsock);
> goto bad;
> }
> - sorwakeup(inp->inp_socket);
> - KERNEL_UNLOCK();
> + mtx_leave(>inp_mtx);
> + sorwakeup(so);
>
> in_pcbunref(inp);
> return;
> Index: netinet6/ip6_divert.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_divert.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 ip6_divert.c
> --- netinet6/ip6_divert.c 15 Aug 2022 09:11:39 - 1.68
> +++ netinet6/ip6_divert.c 20 Aug 2022 21:27:46 -
> @@ -233,22 +233,15 @@ divert6_packet(struct mbuf *m, int dir,
> if_put(ifp);
> }
>
> + mtx_enter(>inp_mtx);
> so = inp->inp_socket;
> - /*
> - * XXXSMP sbappendaddr() is not MP safe and this function is called
> - * from pf with shared netlock. To call only one sbappendaddr() from
> - * divert_packet(), protect it with kernel lock. All other places
> - * call sbappendaddr() with exclusive net lock. This blocks
> - * divert_packet() as we have the shared lock.
> - */
> - KERNEL_LOCK();
> if (sbappendaddr(so, >so_rcv, sin6tosa(), m, NULL) == 0) {
> - KERNEL_UNLOCK();
> + mtx_leave(>inp_mtx);
> div6stat_inc(div6s_fullsock);
> goto bad;
> }
> - sorwakeup(inp->inp_socket);
> - KERNEL_UNLOCK();
> + mtx_leave(>inp_mtx);
> + sorwakeup(so);
>
> in_pcbunref(inp);
> return;
>
pcb mutex for divert input
Hi,
A inpcb mutex seems usable to serialize access to socket receive
buffer. I have tried it for divert and udp input.
As first step I would like to replace the kernel lock with a per
PCB mutex in divert input.
ok?
bluhm
Index: netinet/in_pcb.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.270
diff -u -p -r1.270 in_pcb.c
--- netinet/in_pcb.c8 Aug 2022 12:06:30 - 1.270
+++ netinet/in_pcb.c20 Aug 2022 21:21:13 -
@@ -236,6 +236,7 @@ in_pcballoc(struct socket *so, struct in
inp->inp_table = table;
inp->inp_socket = so;
refcnt_init_trace(>inp_refcnt, DT_REFCNT_IDX_INPCB);
+ mtx_init(>inp_mtx, IPL_SOFTNET);
inp->inp_seclevel[SL_AUTH] = IPSEC_AUTH_LEVEL_DEFAULT;
inp->inp_seclevel[SL_ESP_TRANS] = IPSEC_ESP_TRANS_LEVEL_DEFAULT;
inp->inp_seclevel[SL_ESP_NETWORK] = IPSEC_ESP_NETWORK_LEVEL_DEFAULT;
Index: netinet/in_pcb.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.h,v
retrieving revision 1.129
diff -u -p -r1.129 in_pcb.h
--- netinet/in_pcb.h15 May 2022 09:12:20 - 1.129
+++ netinet/in_pcb.h20 Aug 2022 21:21:13 -
@@ -79,6 +79,7 @@
* I immutable after creation
* N net lock
* t inpt_mtxpcb table mutex
+ * p inpcb_mtx pcb mutex
*/
struct pf_state_key;
@@ -121,6 +122,7 @@ struct inpcb {
#defineinp_route inp_ru.ru_route
#defineinp_route6 inp_ru.ru_route6
structrefcnt inp_refcnt;/* refcount PCB, delay memory free */
+ structmutex inp_mtx;/* protect PCB and socket members */
int inp_flags;/* generic IP/datagram flags */
union { /* Header prototype. */
struct ip hu_ip;
Index: netinet/ip_divert.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.69
diff -u -p -r1.69 ip_divert.c
--- netinet/ip_divert.c 15 Aug 2022 09:11:39 - 1.69
+++ netinet/ip_divert.c 20 Aug 2022 21:20:19 -
@@ -227,22 +227,15 @@ divert_packet(struct mbuf *m, int dir, u
if_put(ifp);
}
+ mtx_enter(>inp_mtx);
so = inp->inp_socket;
- /*
-* XXXSMP sbappendaddr() is not MP safe and this function is called
-* from pf with shared netlock. To call only one sbappendaddr() from
-* divert_packet(), protect it with kernel lock. All other places
-* call sbappendaddr() with exclusive net lock. This blocks
-* divert_packet() as we have the shared lock.
-*/
- KERNEL_LOCK();
if (sbappendaddr(so, >so_rcv, sintosa(), m, NULL) == 0) {
- KERNEL_UNLOCK();
+ mtx_leave(>inp_mtx);
divstat_inc(divs_fullsock);
goto bad;
}
- sorwakeup(inp->inp_socket);
- KERNEL_UNLOCK();
+ mtx_leave(>inp_mtx);
+ sorwakeup(so);
in_pcbunref(inp);
return;
Index: netinet6/ip6_divert.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_divert.c,v
retrieving revision 1.68
diff -u -p -r1.68 ip6_divert.c
--- netinet6/ip6_divert.c 15 Aug 2022 09:11:39 - 1.68
+++ netinet6/ip6_divert.c 20 Aug 2022 21:27:46 -
@@ -233,22 +233,15 @@ divert6_packet(struct mbuf *m, int dir,
if_put(ifp);
}
+ mtx_enter(>inp_mtx);
so = inp->inp_socket;
- /*
-* XXXSMP sbappendaddr() is not MP safe and this function is called
-* from pf with shared netlock. To call only one sbappendaddr() from
-* divert_packet(), protect it with kernel lock. All other places
-* call sbappendaddr() with exclusive net lock. This blocks
-* divert_packet() as we have the shared lock.
-*/
- KERNEL_LOCK();
if (sbappendaddr(so, >so_rcv, sin6tosa(), m, NULL) == 0) {
- KERNEL_UNLOCK();
+ mtx_leave(>inp_mtx);
div6stat_inc(div6s_fullsock);
goto bad;
}
- sorwakeup(inp->inp_socket);
- KERNEL_UNLOCK();
+ mtx_leave(>inp_mtx);
+ sorwakeup(so);
in_pcbunref(inp);
return;
Re: ifconfig, wireguard output less verbose, unless -A or
On Sat, Aug 20, 2022 at 02:25:25PM +0100, Stuart Henderson wrote: > > > > Long output with as an argument, wgpeers section present: > > > > pce-0067# ifconfig.ifaliases wg0 > > wg0: flags=80c3 mtu 1420 > > index 8 priority 0 llprio 3 > > wgport 51820 > > wgpubkey qcb... > > wgpeer klM... > > wgpsk (present) > > wgpka 25 (sec) > > wgendpoint xxx.xxx.xxx.xxx 51820 > > tx: 178764, rx: 65100 > > last handshake: 7 seconds ago > > wgaip fde4:f456:48c2:13c0::/64 > > groups: wg > > inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64 > > > > > > Above long output works with group as an argument (ifconfig wg) and with > > option -A (ifconfig -A), so I think from user experience perspective, > > works as expected. > > > > Manual page changes not provided, as I'm not sure are they needed with > > this diff. > > At least a small change is needed. Maybe some different text would > be more appropriate though. > > Index: ifconfig.8 > === > RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v > retrieving revision 1.384 > diff -u -p -r1.384 ifconfig.8 > --- ifconfig.827 Jun 2022 16:27:03 - 1.384 > +++ ifconfig.820 Aug 2022 13:22:43 - > @@ -75,7 +75,7 @@ Only the superuser may modify the config > The following options are available: > .Bl -tag -width Ds > .It Fl A > -Causes full interface alias information for each interface to > +Causes full interface alias and wgpeer information for each interface to > be displayed. > .It Fl a > Causes > hi. i think this just muddies the simple explanation of -A. i don;t think it's worth it for a single exception. what about amending wg(1) to note that this info is available via -A? then wg users can find it. jmc
Re: regress: installboot: add initial tests, don't hook up yet
On Sat, Aug 20, 2022 at 09:26:03AM +, Klemens Nanni wrote:
> installboot is not trivial, especially when it comes to the matrix of
> platforms, softraid softraid support and boot loader filesystem support.
>
> It is especially annoying to run into issues in the installer where
> debugging capabilities are limited -- I've had that with softraid on
> sparc64 where the issue turns out to be reproducible with vnd(4) in
> multi-user, i.e. a much quicker and safer setup to squash installboot
> bugs.
>
> OK to add the first round of checks?
>
> It'll need some MD bits wrt. vnd disk setup to work on more platforms,
> but I'd like to do that in-tree.
>
> I could then hook it up per-arch as I run and improve the tests on more
> platforms/machines.
>
> Feedback?
Improved version passing on amd64, failing one test on sparc64 and
needing more work on arm64.
I'll commit it eventually, but would appreciate some feedback on it.
These tests really help getting softraid boot/install support into shape
on !amd64.
Index: regress/usr.sbin/installboot/Makefile
===
RCS file: regress/usr.sbin/installboot/Makefile
diff -N regress/usr.sbin/installboot/Makefile
--- /dev/null 1 Jan 1970 00:00:00 -
+++ regress/usr.sbin/installboot/Makefile 20 Aug 2022 17:13:15 -
@@ -0,0 +1,117 @@
+# $OpenBSD: $
+
+INSTALLBOOT ?= /usr/sbin/installboot -v
+DRY_RUN = ${INSTALLBOOT} -n
+REAL_RUN = ${INSTALLBOOT}
+
+# installboot(8) behaviour for multi-chunk softraid(4) differs across platforms
+NCHUNKS ?= 1 2
+CHUNKFILES = ${NCHUNKS:=chunk%.img}
+DEVFILES = ${NCHUNKS:=vnd%.txt}
+SRFILE = sr.txt
+# allow testing with real bootstrap, e.g. for size constraints or formats
+STAGEFILE ?= stage.empty
+MOUNTPOINT ?= /mnt
+
+REGRESS_SETUP_ONCE = copy-bootstrap-to-softraid
+
+create-new-chunks:
+.for n in ${NCHUNKS}
+ dd if=/dev/zero of=chunk${n}.img bs=1m count=0 seek=32 status=none
+ ${SUDO} vnconfig -- chunk${n}.img 1>vnd${n}.txt
+.endfor
+
+# what the installer does, see /usr/src/distrib/$(machine)/ramdisk/install.md
+format-new-chunks: create-new-chunks
+.for devfile in ${DEVFILES}
+.if ${MACHINE} == "amd64"# assume BIOS/MBR
+ ${SUDO} fdisk -iy -- "$$(<${devfile})" 1>/dev/null
+.elif ${MACHINE} == "arm64"
+ ${SUDO} fdisk -g -y -b32768 -- "$$(<${devfile})" 1>/dev/null
+.endif
+ printf 'a\n\n\n\nRAID\nw\nq\n' | \
+ ${SUDO} disklabel -E -- "$$(<${devfile})" 1>/dev/null
+.endfor
+
+create-new-softraid: format-new-chunks
+ ${SUDO} bioctl -l"$$(sed -- s/$$/a/ ${DEVFILES} | paste -sd, -- -)" \
+ -cc -- softraid0 | \
+ awk -- '{ print $$NF }' 1>${SRFILE}
+
+format-new-softraid: create-new-softraid
+ ${SUDO} disklabel -Aw -- "$$(<${SRFILE})"
+ ${SUDO} newfs -- "$$(<${SRFILE})"a
+ ${SUDO} mount -- /dev/"$$(<${SRFILE})"a ${MOUNTPOINT}
+
+copy-bootstrap-to-softraid: format-new-softraid
+ ${SUDO} mkdir -- ${MOUNTPOINT}/usr
+ ${SUDO} cp -r -- /usr/mdec ${MOUNTPOINT}/usr/
+
+
+# most but not all usages rquire the EFI filesystem to be usable
+.if ${MACHINE} == "arm64"
+REGRESS_TARGETS = prepare
+
+# what the installer does, see /usr/src/distrib/$(machine)/ramdisk/install.md
+# do this as regress target and not in format-new-chunks
+# XXX -p is not yet softraid(4) aware
+prepare:
+ ${SUDO} ${REAL_RUN} -p -- "$$(<${SRFILE})"
+.else
+REGRESS_TARGETS = # empty
+.endif
+REGRESS_TARGETS += dry-prepare \
+ dry-default \
+ dry-root \
+ root-installer \
+ root-explicit-stages
+
+dry-prepare:
+ ${SUDO} ${DRY_RUN} -p -- "$$(<${SRFILE})"
+
+dry-default:
+ ${SUDO} ${DRY_RUN} -- "$$(<${SRFILE})"
+
+dry-root:
+ ${SUDO} ${DRY_RUN} -r/ -- "$$(<${SRFILE})"
+
+# what the installer does, see /usr/src/distrib/$(machine)/ramdisk/install.md
+# XXX fails with N > 1 on sparc64, 1 <= N <= 4 works on amd64
+root-installer:
+ ${SUDO} installboot -r /mnt "$$(<${SRFILE})"
+
+
+REGRESS_EXPECTED_FAILURES =dry-prepare-root \
+ dry-prepare-stage \
+ dry-nodisk-stage \
+ dry-toomany
+REGRESS_TARGETS += ${REGRESS_EXPECTED_FAILURES}
+
+dry-prepare-root:
+ ${DRY_RUN} -p -r/ -- "$$(<${SRFILE})"
+
+dry-prepare-stage:
+ touch -- ${STAGEFILE}
+ ${DRY_RUN} -p -- "$$(<${SRFILE})" ${STAGEFILE}
+
+dry-nodisk-stage:
+ touch -- ${STAGEFILE}
+ ${SUDO} ${DRY_RUN} -- ${STAGEFILE}
+
+dry-toomany:
+ touch -- ${STAGEFILE}
+ ${DRY_RUN} -- disk stage1 stage2 too many
+
+
+CLEANFILES = ${CHUNKFILES} ${DEVFILES} ${SRFILE} ${STAGEFILE}
+REGRESS_CLEANUP = cleanup
+
+# allow failure to always cleanup as much as possible
+cleanup:
+ -${SUDO}
Re: move PRU_BIND request to (*pru_bind)() handler
On Sat, Aug 20, 2022 at 05:56:51PM +0300, Vitaliy Makkoveev wrote:
> We have 15 PRU_ requests to split. Is the one request per diff fine?
Yes. OK bluhm@
> Index: sys/kern/uipc_usrreq.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> retrieving revision 1.168
> diff -u -p -r1.168 uipc_usrreq.c
> --- sys/kern/uipc_usrreq.c15 Aug 2022 09:11:38 - 1.168
> +++ sys/kern/uipc_usrreq.c20 Aug 2022 14:42:47 -
> @@ -130,6 +130,7 @@ const struct pr_usrreqs uipc_usrreqs = {
> .pru_usrreq = uipc_usrreq,
> .pru_attach = uipc_attach,
> .pru_detach = uipc_detach,
> + .pru_bind = uipc_bind,
> };
>
> void
> @@ -222,10 +223,6 @@ uipc_usrreq(struct socket *so, int req,
>
> switch (req) {
>
> - case PRU_BIND:
> - error = unp_bind(unp, nam, p);
> - break;
> -
> case PRU_LISTEN:
> if (unp->unp_vnode == NULL)
> error = EINVAL;
> @@ -535,6 +532,14 @@ uipc_detach(struct socket *so)
> unp_detach(unp);
>
> return (0);
> +}
> +
> +int
> +uipc_bind(struct socket *so, struct mbuf *nam, struct proc *p)
> +{
> + struct unpcb *unp = sotounpcb(so);
> +
> + return unp_bind(unp, nam, p);
> }
>
> int
> Index: sys/net/pfkeyv2.c
> ===
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.235
> diff -u -p -r1.235 pfkeyv2.c
> --- sys/net/pfkeyv2.c 15 Aug 2022 09:11:38 - 1.235
> +++ sys/net/pfkeyv2.c 20 Aug 2022 14:42:47 -
> @@ -358,7 +358,6 @@ pfkeyv2_usrreq(struct socket *so, int re
> switch (req) {
> /* no connect, bind, accept. Socket is connected from the start */
> case PRU_CONNECT:
> - case PRU_BIND:
> case PRU_CONNECT2:
> case PRU_LISTEN:
> case PRU_ACCEPT:
> Index: sys/net/rtsock.c
> ===
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.335
> diff -u -p -r1.335 rtsock.c
> --- sys/net/rtsock.c 15 Aug 2022 09:11:38 - 1.335
> +++ sys/net/rtsock.c 20 Aug 2022 14:42:47 -
> @@ -234,7 +234,6 @@ route_usrreq(struct socket *so, int req,
> switch (req) {
> /* no connect, bind, accept. Socket is connected from the start */
> case PRU_CONNECT:
> - case PRU_BIND:
> case PRU_CONNECT2:
> case PRU_LISTEN:
> case PRU_ACCEPT:
> Index: sys/netinet/ip_divert.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_divert.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 ip_divert.c
> --- sys/netinet/ip_divert.c 15 Aug 2022 09:11:39 - 1.69
> +++ sys/netinet/ip_divert.c 20 Aug 2022 14:42:47 -
> @@ -66,6 +66,7 @@ const struct pr_usrreqs divert_usrreqs =
> .pru_usrreq = divert_usrreq,
> .pru_attach = divert_attach,
> .pru_detach = divert_detach,
> + .pru_bind = divert_bind,
> };
>
> int divbhashsize = DIVERTHASHSIZE;
> @@ -274,10 +275,6 @@ divert_usrreq(struct socket *so, int req
> }
> switch (req) {
>
> - case PRU_BIND:
> - error = in_pcbbind(inp, addr, p);
> - break;
> -
> case PRU_SHUTDOWN:
> socantsendmore(so);
> break;
> @@ -362,6 +359,15 @@ divert_detach(struct socket *so)
>
> in_pcbdetach(inp);
> return (0);
> +}
> +
> +int
> +divert_bind(struct socket *so, struct mbuf *addr, struct proc *p)
> +{
> + struct inpcb *inp = sotoinpcb(so);
> +
> + soassertlocked(so);
> + return in_pcbbind(inp, addr, p);
> }
>
> int
> Index: sys/netinet/ip_divert.h
> ===
> RCS file: /cvs/src/sys/netinet/ip_divert.h,v
> retrieving revision 1.16
> diff -u -p -r1.16 ip_divert.h
> --- sys/netinet/ip_divert.h 15 Aug 2022 09:11:39 - 1.16
> +++ sys/netinet/ip_divert.h 20 Aug 2022 14:42:47 -
> @@ -74,5 +74,6 @@ int divert_usrreq(struct socket *,
> int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *);
> int divert_attach(struct socket *, int);
> int divert_detach(struct socket *);
> +int divert_bind(struct socket *, struct mbuf *, struct proc *);
> #endif /* _KERNEL */
> #endif /* _IP_DIVERT_H_ */
> Index: sys/netinet/ip_gre.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_gre.c,v
> retrieving revision 1.75
> diff -u -p -r1.75 ip_gre.c
> --- sys/netinet/ip_gre.c 15 Aug 2022 09:11:39 - 1.75
> +++ sys/netinet/ip_gre.c 20 Aug 2022 14:42:47 -
> @@ -65,6 +65,7 @@ const struct pr_usrreqs gre_usrreqs = {
> .pru_usrreq = gre_usrreq,
> .pru_attach = rip_attach,
> .pru_detach = rip_detach,
> + .pru_bind = rip_bind,
> };
>
> int
> Index:
Re: move PRU_BIND request to (*pru_bind)() handler
On Sat, Aug 20, 2022 at 03:49:08PM +0200, Alexander Bluhm wrote:
> On Fri, Aug 19, 2022 at 04:28:24PM -0900, Philip Guenther wrote:
> > On Fri, Aug 19, 2022 at 12:42 PM Vitaliy Makkoveev wrote:
> >
> > > bluhm@ pointed, that many KASSERT()s are not welcomed, so I didn't
> > > insert them into newly introduced handlers. Anyway except the tcp(4)
> > > protocol, `so_pcb' cant be NULL here. But the socket lock assertion
> > > looks reasonable.
> > >
> > > Some unp_*() functions could be merged with newly introduced uipc_*(),
> > > but I want to do this after (*pru_usrreq)() split finished.
> > >
> >
> > Having multiple PROTO_bind() routines that just return EOPNOTSUPP seems
> > like overkill to me. I think I would tend to just have the pru_bind()
> > inline do a NULL test and return EOPNOTSUPP if it is and leave the callback
> > NULL for all those protocols,
>
> I think a generic return(EOPNOTSUPP) in pru_bind() is good.
>
I also like this way. Since we use pru_() wrappers we don't mess the
code paths with the "if (...->pr_usrreqs->pru_something)" checks.
We have 15 PRU_ requests to split. Is the one request per diff fine?
Index: sys/kern/uipc_usrreq.c
===
RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.168
diff -u -p -r1.168 uipc_usrreq.c
--- sys/kern/uipc_usrreq.c 15 Aug 2022 09:11:38 - 1.168
+++ sys/kern/uipc_usrreq.c 20 Aug 2022 14:42:47 -
@@ -130,6 +130,7 @@ const struct pr_usrreqs uipc_usrreqs = {
.pru_usrreq = uipc_usrreq,
.pru_attach = uipc_attach,
.pru_detach = uipc_detach,
+ .pru_bind = uipc_bind,
};
void
@@ -222,10 +223,6 @@ uipc_usrreq(struct socket *so, int req,
switch (req) {
- case PRU_BIND:
- error = unp_bind(unp, nam, p);
- break;
-
case PRU_LISTEN:
if (unp->unp_vnode == NULL)
error = EINVAL;
@@ -535,6 +532,14 @@ uipc_detach(struct socket *so)
unp_detach(unp);
return (0);
+}
+
+int
+uipc_bind(struct socket *so, struct mbuf *nam, struct proc *p)
+{
+ struct unpcb *unp = sotounpcb(so);
+
+ return unp_bind(unp, nam, p);
}
int
Index: sys/net/pfkeyv2.c
===
RCS file: /cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.235
diff -u -p -r1.235 pfkeyv2.c
--- sys/net/pfkeyv2.c 15 Aug 2022 09:11:38 - 1.235
+++ sys/net/pfkeyv2.c 20 Aug 2022 14:42:47 -
@@ -358,7 +358,6 @@ pfkeyv2_usrreq(struct socket *so, int re
switch (req) {
/* no connect, bind, accept. Socket is connected from the start */
case PRU_CONNECT:
- case PRU_BIND:
case PRU_CONNECT2:
case PRU_LISTEN:
case PRU_ACCEPT:
Index: sys/net/rtsock.c
===
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.335
diff -u -p -r1.335 rtsock.c
--- sys/net/rtsock.c15 Aug 2022 09:11:38 - 1.335
+++ sys/net/rtsock.c20 Aug 2022 14:42:47 -
@@ -234,7 +234,6 @@ route_usrreq(struct socket *so, int req,
switch (req) {
/* no connect, bind, accept. Socket is connected from the start */
case PRU_CONNECT:
- case PRU_BIND:
case PRU_CONNECT2:
case PRU_LISTEN:
case PRU_ACCEPT:
Index: sys/netinet/ip_divert.c
===
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.69
diff -u -p -r1.69 ip_divert.c
--- sys/netinet/ip_divert.c 15 Aug 2022 09:11:39 - 1.69
+++ sys/netinet/ip_divert.c 20 Aug 2022 14:42:47 -
@@ -66,6 +66,7 @@ const struct pr_usrreqs divert_usrreqs =
.pru_usrreq = divert_usrreq,
.pru_attach = divert_attach,
.pru_detach = divert_detach,
+ .pru_bind = divert_bind,
};
int divbhashsize = DIVERTHASHSIZE;
@@ -274,10 +275,6 @@ divert_usrreq(struct socket *so, int req
}
switch (req) {
- case PRU_BIND:
- error = in_pcbbind(inp, addr, p);
- break;
-
case PRU_SHUTDOWN:
socantsendmore(so);
break;
@@ -362,6 +359,15 @@ divert_detach(struct socket *so)
in_pcbdetach(inp);
return (0);
+}
+
+int
+divert_bind(struct socket *so, struct mbuf *addr, struct proc *p)
+{
+ struct inpcb *inp = sotoinpcb(so);
+
+ soassertlocked(so);
+ return in_pcbbind(inp, addr, p);
}
int
Index: sys/netinet/ip_divert.h
===
RCS file: /cvs/src/sys/netinet/ip_divert.h,v
retrieving revision 1.16
diff -u -p -r1.16 ip_divert.h
--- sys/netinet/ip_divert.h 15 Aug 2022 09:11:39 - 1.16
+++ sys/netinet/ip_divert.h 20 Aug 2022 14:42:47 -
@@ -74,5 +74,6 @@ intdivert_usrreq(struct socket *,
Re: move PRU_BIND request to (*pru_bind)() handler
On Fri, Aug 19, 2022 at 04:28:24PM -0900, Philip Guenther wrote: > On Fri, Aug 19, 2022 at 12:42 PM Vitaliy Makkoveev wrote: > > > bluhm@ pointed, that many KASSERT()s are not welcomed, so I didn't > > insert them into newly introduced handlers. Anyway except the tcp(4) > > protocol, `so_pcb' cant be NULL here. But the socket lock assertion > > looks reasonable. > > > > Some unp_*() functions could be merged with newly introduced uipc_*(), > > but I want to do this after (*pru_usrreq)() split finished. > > > > Having multiple PROTO_bind() routines that just return EOPNOTSUPP seems > like overkill to me. I think I would tend to just have the pru_bind() > inline do a NULL test and return EOPNOTSUPP if it is and leave the callback > NULL for all those protocols, I think a generic return(EOPNOTSUPP) in pru_bind() is good. > but I could also see having a single > prubind_eopnotsupp() (or whatever you think is a clear name) > implementation in some sys/kern/ file which could then be used by all the > protocols that don't implement it. I am not a fan of generic stubs. bluhm
Re: ifconfig, wireguard output less verbose, unless -A or
On 2022/07/14 09:37, Mikolaj Kucharski wrote:
> Hi,
>
> Per other thread, Theo expressed dissatisfaction with long ifconfig(8)
> for wg(4) interface. Stuart Henderson pointed me at direction, which
> below diff makes it work.
>
> I guess to questions are:
>
> - Does the behaviour of ifconfig(8) make sense?
> - Does the code which makes above, make sense?
I think so, and the diff works exactly as I would expect it to.
> This is minimal diff, I would appreciate feedback, I did least
> resistance approach. Looking at diff -wu shows even less changes
> as wg_status() is mainly identation with if-statement.
>
>
> Short output by default, only 6 lines, no wgpeers section:
>
> pce-0067# ifconfig.ifaliases -a | tail -n6
> wg0: flags=80c3 mtu 1420
> index 8 priority 0 llprio 3
> wgport 51820
> wgpubkey qcb...
> groups: wg
> inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64
>
>
> Long output with as an argument, wgpeers section present:
>
> pce-0067# ifconfig.ifaliases wg0
> wg0: flags=80c3 mtu 1420
> index 8 priority 0 llprio 3
> wgport 51820
> wgpubkey qcb...
> wgpeer klM...
> wgpsk (present)
> wgpka 25 (sec)
> wgendpoint xxx.xxx.xxx.xxx 51820
> tx: 178764, rx: 65100
> last handshake: 7 seconds ago
> wgaip fde4:f456:48c2:13c0::/64
> groups: wg
> inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64
>
>
> Above long output works with group as an argument (ifconfig wg) and with
> option -A (ifconfig -A), so I think from user experience perspective,
> works as expected.
>
> Manual page changes not provided, as I'm not sure are they needed with
> this diff.
At least a small change is needed. Maybe some different text would
be more appropriate though.
Index: ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.384
diff -u -p -r1.384 ifconfig.8
--- ifconfig.8 27 Jun 2022 16:27:03 - 1.384
+++ ifconfig.8 20 Aug 2022 13:22:43 -
@@ -75,7 +75,7 @@ Only the superuser may modify the config
The following options are available:
.Bl -tag -width Ds
.It Fl A
-Causes full interface alias information for each interface to
+Causes full interface alias and wgpeer information for each interface to
be displayed.
.It Fl a
Causes
> Comments?
>
>
> Index: ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.456
> diff -u -p -u -r1.456 ifconfig.c
> --- ifconfig.c8 Jul 2022 07:04:54 - 1.456
> +++ ifconfig.c14 Jul 2022 09:25:21 -
> @@ -363,7 +363,7 @@ void unsetwgpeer(const char *, int);
> void unsetwgpeerpsk(const char *, int);
> void unsetwgpeerall(const char *, int);
>
> -void wg_status();
> +void wg_status(int);
> #else
> void setignore(const char *, int);
> #endif
> @@ -679,7 +679,7 @@ void printgroupattribs(char *);
> void printif(char *, int);
> void printb_status(unsigned short, unsigned char *);
> const char *get_linkstate(int, int);
> -void status(int, struct sockaddr_dl *, int);
> +void status(int, struct sockaddr_dl *, int, int);
> __dead void usage(void);
> const char *get_string(const char *, const char *, u_int8_t *, int *);
> int len_string(const u_int8_t *, int);
> @@ -1195,7 +1195,7 @@ printif(char *name, int ifaliases)
> continue;
> ifdata = ifa->ifa_data;
> status(1, (struct sockaddr_dl *)ifa->ifa_addr,
> - ifdata->ifi_link_state);
> + ifdata->ifi_link_state, ifaliases);
> count++;
> noinet = 1;
> continue;
> @@ -3316,7 +3316,7 @@ get_linkstate(int mt, int link_state)
> * specified, show it and it only; otherwise, show them all.
> */
> void
> -status(int link, struct sockaddr_dl *sdl, int ls)
> +status(int link, struct sockaddr_dl *sdl, int ls, int ifaliases)
> {
> const struct afswtch *p = afp;
> struct ifmediareq ifmr;
> @@ -3391,7 +3391,7 @@ status(int link, struct sockaddr_dl *sdl
> mpls_status();
> pflow_status();
> umb_status();
> - wg_status();
> + wg_status(ifaliases);
> #endif
> trunk_status();
> getifgroups();
> @@ -5907,7 +5907,7 @@ process_wg_commands(void)
> }
>
> void
> -wg_status(void)
> +wg_status(int ifaliases)
> {
> size_t i, j, last_size;
> struct timespec now;
> @@ -5942,45 +5942,47 @@ wg_status(void)
> printf("\twgpubkey %s\n", key);
> }
>
> - wg_peer = _interface->i_peers[0];
> - for (i = 0; i < wg_interface->i_peers_count; i++) {
> - b64_ntop(wg_peer->p_public, WG_KEY_LEN,
> - key, sizeof(key));
> -
refactor pcb lookup
Hi,
Can we rename the the function in_pcbhashlookup() to in_pcblookup()?
Then we have in_pcblookup() and in_pcblookup_listen() as public PCB
interface. Using a hash table is only an implementation detail.
For internal use I would like to introduce in_pcbhash_insert() and
in_pcbhash_lookup() to avoid code duplication.
Routing domain is unsigned, change the type to u_int.
If the diff is too large for review, I can split these parts.
ok?
bluhm
Index: net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1137
diff -u -p -r1.1137 pf.c
--- net/pf.c8 Aug 2022 12:06:30 - 1.1137
+++ net/pf.c19 Aug 2022 16:22:47 -
@@ -3348,7 +3348,7 @@ pf_socket_lookup(struct pf_pdesc *pd)
* Fails when rtable is changed while evaluating the ruleset
* The socket looked up will not match the one hit in the end.
*/
- inp = in_pcbhashlookup(tb, saddr->v4, sport, daddr->v4, dport,
+ inp = in_pcblookup(tb, saddr->v4, sport, daddr->v4, dport,
pd->rdomain);
if (inp == NULL) {
inp = in_pcblookup_listen(tb, daddr->v4, dport,
@@ -3359,7 +3359,7 @@ pf_socket_lookup(struct pf_pdesc *pd)
break;
#ifdef INET6
case AF_INET6:
- inp = in6_pcbhashlookup(tb, >v6, sport, >v6,
+ inp = in6_pcblookup(tb, >v6, sport, >v6,
dport, pd->rdomain);
if (inp == NULL) {
inp = in6_pcblookup_listen(tb, >v6, dport,
Index: netinet/in_pcb.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.270
diff -u -p -r1.270 in_pcb.c
--- netinet/in_pcb.c8 Aug 2022 12:06:30 - 1.270
+++ netinet/in_pcb.c19 Aug 2022 20:41:23 -
@@ -120,14 +120,16 @@ struct baddynamicports baddynamicports;
struct baddynamicports rootonlyports;
struct pool inpcb_pool;
-void in_pcbrehash_locked(struct inpcb *);
+void in_pcbhash_insert(struct inpcb *);
+struct inpcb *in_pcbhash_lookup(struct inpcbtable *, u_int,
+const struct in_addr *, u_short, const struct in_addr *, u_short);
intin_pcbresize(struct inpcbtable *, int);
#defineINPCBHASH_LOADFACTOR(_x)(((_x) * 3) / 4)
-struct inpcbhead *in_pcbhash(struct inpcbtable *, int,
+struct inpcbhead *in_pcbhash(struct inpcbtable *, u_int,
const struct in_addr *, u_short, const struct in_addr *, u_short);
-struct inpcbhead *in_pcblhash(struct inpcbtable *, int, u_short);
+struct inpcbhead *in_pcblhash(struct inpcbtable *, u_int, u_short);
/*
* in_pcb is used for inet and inet6. in6_pcb only contains special
@@ -141,12 +143,12 @@ in_init(void)
}
struct inpcbhead *
-in_pcbhash(struct inpcbtable *table, int rdom,
+in_pcbhash(struct inpcbtable *table, u_int rdomain,
const struct in_addr *faddr, u_short fport,
const struct in_addr *laddr, u_short lport)
{
SIPHASH_CTX ctx;
- u_int32_t nrdom = htonl(rdom);
+ u_int32_t nrdom = htonl(rdomain);
SipHash24_Init(, >inpt_key);
SipHash24_Update(, , sizeof(nrdom));
@@ -159,10 +161,10 @@ in_pcbhash(struct inpcbtable *table, int
}
struct inpcbhead *
-in_pcblhash(struct inpcbtable *table, int rdom, u_short lport)
+in_pcblhash(struct inpcbtable *table, u_int rdomain, u_short lport)
{
SIPHASH_CTX ctx;
- u_int32_t nrdom = htonl(rdom);
+ u_int32_t nrdom = htonl(rdomain);
SipHash24_Init(, >inpt_lkey);
SipHash24_Update(, , sizeof(nrdom));
@@ -226,9 +228,6 @@ int
in_pcballoc(struct socket *so, struct inpcbtable *table)
{
struct inpcb *inp;
- struct inpcbhead *head;
-
- NET_ASSERT_LOCKED();
inp = pool_get(_pool, PR_NOWAIT|PR_ZERO);
if (inp == NULL)
@@ -257,19 +256,7 @@ in_pcballoc(struct socket *so, struct in
if (table->inpt_count++ > INPCBHASH_LOADFACTOR(table->inpt_size))
(void)in_pcbresize(table, table->inpt_size * 2);
TAILQ_INSERT_HEAD(>inpt_queue, inp, inp_queue);
- head = in_pcblhash(table, inp->inp_rtableid, inp->inp_lport);
- LIST_INSERT_HEAD(head, inp, inp_lhash);
-#ifdef INET6
- if (sotopf(so) == PF_INET6)
- head = in6_pcbhash(table, rtable_l2(inp->inp_rtableid),
- >inp_faddr6, inp->inp_fport,
- >inp_laddr6, inp->inp_lport);
- else
-#endif /* INET6 */
- head = in_pcbhash(table, rtable_l2(inp->inp_rtableid),
- >inp_faddr, inp->inp_fport,
- >inp_laddr, inp->inp_lport);
- LIST_INSERT_HEAD(head, inp, inp_hash);
+ in_pcbhash_insert(inp);
mtx_leave(>inpt_mtx);
so->so_pcb = inp;
@@ -511,7 +498,7 @@ in_pcbconnect(struct inpcb *inp, struct
if (error)
return
Re: random(6): undefined cast and error checking
On Fri, Aug 05, 2022 at 10:55:23PM +0200, Theo Buehler wrote:
> On Fri, Aug 05, 2022 at 02:12:35PM -0500, luci...@bronze.ctrl-c.club wrote:
> > So this is the final verison of the patch solving the following
> > problems:
> >
> > >The program is broken in multiple ways: return value clamping, casting
> > >from double to uint32_t, wrong error checking for putchar, lack of
> > >warnings when compiling.
> >
> > Now the program is more pedantic and complicated, but at least it does
> > what is says in the man page.
> >
> > Ok, deraadt, tb?
>
> Pretty much. Below is my suggestion, based on yours.
>
> > +CFLAGS=-Wall -Wconversion
>
> I dropped these again. I don't think -Wconversion is helpful here. It
> doesn't understand the semantics of arc4random_buf(), so it whines for
> no good reason. I can be convinced to add -Wall.
>
> I pulled Campbell's license and code up to the top, in particular, we
> can avoid prototypes. I switched a few variables from unsigned to int
> since that makes more sense to me. I dropped the error check to
> fprintf().
>
> I dealt with -e differently than in your diff: reject denominators < 1
> since those makes no sense. Document that. If -e is given, make sure
> denom is at most 256, this way arc4random_uniform() will return a value
> between 0 and 255, which exit() will not truncate. The nature of -e is
> that we can't signal an error via return value, so we had better
> succeed.
>
> Other than that, I think this is good to go in. Surely the manual could
> be improved...
Anyone willing to ok this?
Index: random.6
===
RCS file: /cvs/src/games/random/random.6,v
retrieving revision 1.7
diff -u -p -r1.7 random.6
--- random.631 May 2007 19:19:18 - 1.7
+++ random.65 Aug 2022 20:24:45 -
@@ -43,9 +43,10 @@
.Nm
reads lines from the standard input and copies them to the standard
output with a probability of 1/denominator.
-The default value for
+The
.Ar denominator
-is 2.
+must be at least 1,
+its default value is 2.
.Pp
The options are as follows:
.Bl -tag -width Ds
@@ -55,7 +56,7 @@ If the
option is specified,
.Nm
does not read or write anything, and simply exits with a random
-exit value of 0 to
+exit value of 0 to the minimum of 255 and
.Ar denominator
\&- 1, inclusive.
.It Fl r
Index: random.c
===
RCS file: /cvs/src/games/random/random.c,v
retrieving revision 1.20
diff -u -p -r1.20 random.c
--- random.c7 Mar 2016 12:07:56 - 1.20
+++ random.c5 Aug 2022 20:30:53 -
@@ -33,8 +33,35 @@
* SUCH DAMAGE.
*/
+/*-
+ * Copyright (c) 2014 Taylor R. Campbell
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
#include
#include
+#include
#include
#include
#include
@@ -42,6 +69,101 @@
__dead void usage(void);
int
+clz64(uint64_t x)
+{
+ static const uint64_t mask[] = {
+ 0x, 0x, 0xff00, 0xf0, 0xc, 0x2,
+ };
+ static const int zeroes[] = {
+ 32, 16, 8, 4, 2, 1,
+ };
+ int clz = 0;
+ int i;
+
+ if (x == 0)
+ return 64;
+
+ for (i = 0; i < 6; i++) {
+ if ((x & mask[i]) == 0)
+ clz += zeroes[i];
+ else
+ x >>= zeroes[i];
+ }
+
+ return clz;
+}
+
+uint64_t
+random64(void)
+{
+ uint64_t r;
+
+ arc4random_buf(, sizeof(uint64_t));
+
+ return r;
+}
+
+/*
+ * random_real: Generate a stream of bits uniformly at random and
+ * interpret it as the fractional part of the binary expansion of a
+ * number in [0, 1],
Re: remove support for Cyrix 486DLC & Cyrix 6x86
On Fri, Aug 19, 2022 at 10:31:30PM -0400, Daniel Dickman wrote:
> The below diff removes detection code for the Cyrix 486DLC and Cyrix 6x86
> CPUs from OpenBSD/i386.
>
> The Cyrix 486DLC is a 486-class CPU which we no longer support.
>
> The 6x86, also known as the M1, does not support CPUID by default. But it
> can be made to support this instruction if bit 7 in CCR4 is enabled. We
> don't do this in the tree today.
>
> The reason to remove support for the 6x86 is because it doesn't support
> the RDTSC instruction which we we use unconditionally. Therefore I don't
> believe Cyrix CPUs older than the 6x86MX (aka the M2) can run
> OpenBSD/i386.
>
> We keep the "cyrix6x86_cpu_setup" function in machdep because those quirks
> would still be needed on the M2.
>
> After this diff, the CPU detection code on i386 would assume that if the
> ID bit is missing from EFLAGS, we're running on an intel 486, while if we
> have the ID bit then we use the CPUID instruction for the detection logic.
>
> We also make the same change to amd64/include/specialreg.h to remove the
> defines for the Cyrix 486DLC. No Cyrix CPU supports amd64, so these
> defines have never made sense there.
>
ok jsg@
>
> Index: i386/i386/locore0.S
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/locore0.S,v
> retrieving revision 1.7
> diff -u -p -u -r1.7 locore0.S
> --- i386/i386/locore0.S 15 Aug 2022 04:17:50 - 1.7
> +++ i386/i386/locore0.S 20 Aug 2022 02:07:30 -
> @@ -133,56 +133,6 @@ start: movw$0x1234,0x472 # warm
> boot
> jnz .Ltry586
> .Lis486: movl$CPU_486,RELOC(_C_LABEL(cpu))
>
> - /*
> - * Check Cyrix CPU
> - * Cyrix CPUs do not change the undefined flags following
> - * execution of the divide instruction which divides 5 by 2.
> - *
> - * Note: CPUID is enabled on M2, so it passes another way.
> - */
> - pushfl
> - movl$0x, %eax
> - xorl%edx, %edx
> - movl$2, %ecx
> - clc
> - divl%ecx
> - jnc .Ltrycyrix486
> - popfl
> - jmp 2f
> -.Ltrycyrix486:
> - movl$CPU_6x86,RELOC(_C_LABEL(cpu)) # set CPU type
> - /*
> - * Check for Cyrix 486 CPU by seeing if the flags change during a
> - * divide. This is documented in the Cx486SLC/e SMM Programmer's
> - * Guide.
> - */
> - xorl%edx,%edx
> - cmpl%edx,%edx # set flags to known state
> - pushfl
> - popl%ecx# store flags in ecx
> - movl$-1,%eax
> - movl$4,%ebx
> - divl%ebx# do a long division
> - pushfl
> - popl%eax
> - xorl%ecx,%eax # are the flags different?
> - testl $0x8d5,%eax # only check C|PF|AF|Z|N|V
> - jne 2f # yes; must not be Cyrix CPU
> - movl$CPU_486DLC,RELOC(_C_LABEL(cpu))# set CPU type
> -
> - /* Disable caching of the ISA hole only. */
> - invd
> - movb$CCR0,%al # Configuration Register index (CCR0)
> - outb%al,$0x22
> - inb $0x23,%al
> - orb $(CCR0_NC1|CCR0_BARB),%al
> - movb%al,%ah
> - movb$CCR0,%al
> - outb%al,$0x22
> - movb%ah,%al
> - outb%al,$0x23
> - invd
> -
> jmp 2f
>
> .Ltry586:/* Use the `cpuid' instruction. */
> Index: i386/i386/machdep.c
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
> retrieving revision 1.653
> diff -u -p -u -r1.653 machdep.c
> --- i386/i386/machdep.c 18 Aug 2022 13:05:43 - 1.653
> +++ i386/i386/machdep.c 20 Aug 2022 02:07:30 -
> @@ -498,14 +498,8 @@ char cpu_model[120];
> * We deal with the rest in a different way.
> */
> const struct cpu_nocpuid_nameclass i386_nocpuid_cpus[] = {
> - { CPUVENDOR_INTEL, "Intel", "486SX",CPUCLASS_486,
> - NULL}, /* CPU_486SX */
> { CPUVENDOR_INTEL, "Intel", "486DX",CPUCLASS_486,
> NULL}, /* CPU_486 */
> - { CPUVENDOR_CYRIX, "Cyrix", "486DLC", CPUCLASS_486,
> - NULL}, /* CPU_486DLC */
> - { CPUVENDOR_CYRIX, "Cyrix", "6x86", CPUCLASS_486,
> - cyrix6x86_cpu_setup}, /* CPU_6x86 */
> };
>
> const char *classnames[] = {
> @@ -2075,9 +2069,6 @@ identifycpu(struct cpu_info *ci)
> cpu_class = class;
>
> ci->cpu_class = class;
> -
> - if (cpu == CPU_486DLC)
> - printf("WARNING: CYRIX 486DLC CACHE UNCHANGED.\n");
>
> /*
>* Enable ring 0 write protection.
> Index: i386/include/cputypes.h
> ===
> RCS file: /cvs/src/sys/arch/i386/include/cputypes.h,v
> retrieving
regress: installboot: add initial tests, don't hook up yet
installboot is not trivial, especially when it comes to the matrix of
platforms, softraid softraid support and boot loader filesystem support.
It is especially annoying to run into issues in the installer where
debugging capabilities are limited -- I've had that with softraid on
sparc64 where the issue turns out to be reproducible with vnd(4) in
multi-user, i.e. a much quicker and safer setup to squash installboot
bugs.
OK to add the first round of checks?
It'll need some MD bits wrt. vnd disk setup to work on more platforms,
but I'd like to do that in-tree.
I could then hook it up per-arch as I run and improve the tests on more
platforms/machines.
Feedback?
Index: regress/usr.sbin/installboot/Makefile
===
RCS file: regress/usr.sbin/installboot/Makefile
diff -N regress/usr.sbin/installboot/Makefile
--- /dev/null 1 Jan 1970 00:00:00 -
+++ regress/usr.sbin/installboot/Makefile 20 Aug 2022 09:00:13 -
@@ -0,0 +1,83 @@
+# $OpenBSD: $
+
+INSTALLBOOT ?= /usr/sbin/installboot
+DRY_RUN = ${INSTALLBOOT} -n
+
+# dry-r fails with N > 2 on sparc64, N = 4 works on amd64
+N ?= 1 2 3 4
+IMGFILES = ${N:=disk%.img}
+DEVFILES = ${N:=vnd%.txt}
+SRFILE = sr.txt
+# CONCAT takes any positive number of chunks
+SRLEVEL ?= c
+# allow testing with real bootstrap, e.g. for size constraints or formats
+STAGEFILE ?= stage.empty
+
+REGRESS_SETUP_ONCE = create-new-softraid
+
+create-new-disks:
+.for n in ${N}
+ dd if=/dev/zero of=disk${n}.img bs=1m count=0 seek=32 status=none
+ ${SUDO} vnconfig -- disk${n}.img 1>vnd${n}.txt
+.endfor
+
+format-new-disks: create-new-disks
+.for devfile in ${DEVFILES}
+ @# XXX MD (sparc64 does not need fdisk)
+ ${SUDO} fdisk -iy -- "$$(<${devfile})" 1>/dev/null
+ printf 'a\n\n\n\nRAID\nw\nq\n' | \
+ ${SUDO} disklabel -E -- "$$(<${devfile})" 1>/dev/null
+.endfor
+
+create-new-softraid: format-new-disks
+ ${SUDO} bioctl -c${SRLEVEL} \
+ -l"$$(sed s/$$/a/ vnd?.txt | paste -sd, -)" -- softraid0 | \
+ awk '{ print $$NF }' 1>${SRFILE}
+
+
+REGRESS_TARGETS = dry-p \
+ dry-default \
+ dry-r
+
+dry-p:
+ ${SUDO} ${DRY_RUN} -p -- "$$(<${SRFILE})"
+
+dry-default:
+ ${SUDO} ${DRY_RUN} -- "$$(<${SRFILE})"
+
+dry-r:
+ ${SUDO} ${DRY_RUN} -r/ -- "$$(<${SRFILE})"
+
+
+REGRESS_EXPECTED_FAILURES =dry-p-r \
+ dry-p-stage \
+ dry-nodisk-stage \
+ dry-toomany
+REGRESS_TARGETS += ${REGRESS_EXPECTED_FAILURES}
+
+dry-p-r:
+ ${DRY_RUN} -p -r/ -- "$$(<${SRFILE})"
+
+dry-p-stage:
+ touch -- ${STAGEFILE}
+ ${DRY_RUN} -p -- "$$(<${SRFILE})" ${STAGEFILE}
+
+dry-nodisk-stage:
+ touch -- ${STAGEFILE}
+ ${SUDO} ${DRY_RUN} -- ${STAGEFILE}
+
+dry-toomany:
+ touch -- ${STAGEFILE}
+ ${DRY_RUN} -- disk stage1 stage2 too many
+
+
+CLEANFILES = ${IMGFILES} ${DEVFILES} ${SRFILE} ${STAGEFILE}
+REGRESS_CLEANUP = cleanup
+
+cleanup:
+ ${SUDO} bioctl -d -- "$$(<${SRFILE})"
+.for devfile in ${DEVFILES}
+ ${SUDO} vnconfig -u -- "$$(<${devfile})"
+.endfor
+
+.include
regress: vmd: disable on i386
vmd/Makefile filters for amd64 itself but still, no need to enter
on !amd64.
Index: ../Makefile
===
RCS file: /cvs/src/regress/usr.sbin/Makefile,v
retrieving revision 1.26
diff -u -p -r1.26 Makefile
--- ../Makefile 11 Nov 2021 10:03:54 - 1.26
+++ ../Makefile 20 Aug 2022 09:01:56 -
@@ -16,7 +16,7 @@ SUBDIR += rpki-client
SUBDIR += snmpd
SUBDIR += syslogd
-.if ${MACHINE} == "amd64" || ${MACHINE} == "i386"
+.if ${MACHINE} == "amd64"
SUBDIR += vmd
.endif
installboot: fix argc check to prevent installing w/o disk
Just forgot to pass the disk during sparc64 tests and was surprised to
see it treating a file as disk:
# installboot -nv /usr/mdec/bootblk /ofwboot.test
Using / as root
would install bootstrap on ./usr/mdec/bootblk
using first-stage ./ofwboot.test, second-stage /usr/mdec/ofwboot
boot block is 120696 bytes (236 blocks @ 512 bytes = 120832 bytes)
installboot: boot blocks too big (120832 > 7680)
Same on amd64:
$ installboot -nv /usr/mdec/biosboot /usr/mdec/boot
Using / as root
would install bootstrap on /usr/mdec/biosboot
using first-stage /usr/mdec/boot, second-stage /usr/mdec/boot
installboot: /usr/mdec/boot: 2 ELF load sections (only support 1)
Require an exact argc match instead of erroring out too many args alone:
$ ./obj/installboot -nv /usr/mdec/biosboot /usr/mdec/boot
usage: installboot [-nv] [-r root] disk [stage1 [stage2]]
installboot [-nv] -p disk
This problem exists on all platforms, but EFI (armv7, arm64, risc64)
currently suffers from another bug regarding an explicit stage file:
$ installboot -nv sd0 /usr/mdec/BOOTAA64.EFI
usage: installboot [-nv] [-r root] disk [stage1]
installboot [-nv] -p disk
There md_init() does not yet set `stages' (relevant for this argc fix)
and `stage1', so `stage' remains zero-initialised, effectively always
requiring exactly one argument (the disk).
That's stuff for another diff and luckily the argc fix below does not
change behaviour for the only currently working usage on EFI:
# doas installboot -nv sd0
Using / as root
would install bootstrap on /dev/rsd0c
would copy /usr/mdec/BOOTAA64.EFI to
/tmp/installboot.7w8t7zd34s/efi/boot/bootaa64.efi
would write /tmp/installboot.7w8t7zd34s/efi/boot/startup.nsh
# doas ./obj/installboot -nv sd0
Using / as root
would install bootstrap on /dev/rsd0c
would copy /usr/mdec/BOOTAA64.EFI to
/tmp/installboot.OMturEqYaM/efi/boot/bootaa64.efi
would write /tmp/installboot.OMturEqYaM/efi/boot/startup.nsh
The argc checks now read like I would read the synopsis, making it much
easier to read and understand than the current code:
1. -p?
a. no -r?
b. exactly one arg (disk)?
2. else
a. exactly one arg (disk)?
b. exactly one+stages args (disk + stage1 [+ stage2])?
Pull stage1/stage2 assignments into 2. to clarify things further.
This fixes the bogus case I hit and keeps existing valid usages working.
Feedback? OK?
Index: installboot.c
===
RCS file: /cvs/src/usr.sbin/installboot/installboot.c,v
retrieving revision 1.15
diff -u -p -r1.15 installboot.c
--- installboot.c 19 Aug 2022 08:27:48 - 1.15
+++ installboot.c 20 Aug 2022 06:29:49 -
@@ -75,10 +75,13 @@ main(int argc, char **argv)
argc -= optind;
argv += optind;
- if (argc < 1 || argc > stages + 1)
- usage();
- if (prepare && (root != NULL || argc > 1))
- usage();
+ if (prepare) {
+ if (root != NULL || argc != 1)
+ usage();
+ } else {
+ if (argc != 1 && argc != stages + 1)
+ usage();
+ }
dev = argv[0];
if (argc > 1)
