Re: sigsuspend(2): sleep on channel?
On Thu, Nov 11, 2021 at 02:13:26PM -0600, Scott Cheloha wrote:
> On Thu, Nov 11, 2021 at 08:53:20PM +0100, Mark Kettenis wrote:
> > > Date: Thu, 11 Nov 2021 13:30:04 -0600
> > > From: Scott Cheloha
> > >
> > > My understanding of sigsuspend(2) is that it only returns if a signal
> > > is delivered to the calling thread. However, in sys_sigsuspend() we
> > > pass >p_p->ps_sigacts as the wakeup channel to tsleep_nsec(9).
> > >
> > > Are we actually waiting for a wakeup on that channel? Or can we sleep
> > > on here? Patch attached. Note that we don't need to loop
> > > here anymore: we can't receieve wakeup so tsleep_nsec(9) will never
> > > return zero.
> >
> > I don't think we expect a wakeup on that channel. But the loop is
> > still needed as a spurious wakeup may happen.
>
> I'm not quite sure how you'd get a spurious wakeup in this context,
> but I'll take your word for it.
>
> So use to indicate we're not expecting a wakeup, but keep the
> loop.
>
> Index: kern_sig.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sig.c,v
> retrieving revision 1.287
> diff -u -p -r1.287 kern_sig.c
> --- kern_sig.c24 Oct 2021 00:02:25 - 1.287
> +++ kern_sig.c11 Nov 2021 20:10:51 -
> @@ -509,7 +509,7 @@ dosigsuspend(struct proc *p, sigset_t ne
> }
>
> /*
> - * Suspend process until signal, providing mask to be set
> + * Suspend thread until signal, providing mask to be set
> * in the meantime. Note nonstandard calling convention:
> * libc stub passes mask, not pointer, to save a copyin.
> */
> @@ -519,12 +519,10 @@ sys_sigsuspend(struct proc *p, void *v,
> struct sys_sigsuspend_args /* {
> syscallarg(int) mask;
> } */ *uap = v;
> - struct process *pr = p->p_p;
> - struct sigacts *ps = pr->ps_sigacts;
>
> dosigsuspend(p, SCARG(uap, mask) &~ sigcantmask);
> - while (tsleep_nsec(ps, PPAUSE|PCATCH, "sigsusp", INFSLP) == 0)
> - /* void */;
> + while (tsleep_nsec(, PPAUSE|PCATCH, "sigsusp", INFSLP) == 0)
> + continue;
> /* always return EINTR rather than ERESTART... */
> return (EINTR);
> }
>
OK claudio@
--
:wq Claudio
Re: add 802.11n 40MHz support to iwn(4)
On Thu, Nov 11, 2021 at 02:42:39PM +0100, Stefan Sperling wrote: > Testing on 4965 by jsg@ revealed an unrelated issue on those devices. > A fix for this problem has just been committed. > This new version of the 40MHz patch applies on top of that fix. Working fine on my "Intel Centrino Advanced-N 6205" rev 0x34: msi, MIMO 2T2R. No perceivable change to the previous patch revision.
Re: uhidev: claim multiple report ids [3/N]
Anton Lindqvist writes: > On Thu, Nov 11, 2021 at 03:29:15PM +0100, Anton Lindqvist wrote: >> Hi, >> The second attempt to solve the uhidev claim multiple report ids >> conflict didn't work out either as it broke fido(4). Signalling claim >> multiple report ids to the match routines using the report id does not >> work as all 256 values already have semantic meaning. I instead want to >> use `uha->claimed != NULL' to signal that multiple report ids can be >> claimed. Before doing so, refactor in order to make an upcoming diff >> with the actual fix significantly smaller. >> >> No intended^W functional change. >> >> Comments? OK? > > ... and here's the actual fix applied on top of the previous diff. The pair of diffs seems to work for me (fido remains operational unlike the previous iteration). There's a minor change in dmesg output which is not otherwise consequential: OpenBSD 7.0-current (GENERIC.MP) #14: Wed Nov 10 22:17:10 PST 2021 ... xhci0 at pci4 dev 0 function 1 "AMD 17h xHCI" rev 0x00: msi, xHCI 1.10 usb0 at xhci0: USB revision 3.0 uhub0 at usb0 configuration 1 interface 0 "AMD xHCI root hub" rev 3.00/1.00 addr 1 ... uhub4 at uhub0 port 1 configuration 1 interface 0 "VIA Labs, Inc. USB2.0 Hub" rev 2.10/b.e0 addr 2 uhidev0 at uhub4 port 1 configuration 1 interface 0 "Kinesis Advantage2 Keyboard" rev 2.00/1.00 addr 3 uhidev0: iclass 3/1 ums0 at uhidev0: 3 buttons, Z dir wsmouse0 at ums0 mux 0 uhidev1 at uhub4 port 1 configuration 1 interface 1 "Kinesis Advantage2 Keyboard" rev 2.00/1.00 addr 3 uhidev1: iclass 3/1 ukbd0 at uhidev1: 8 variable keys, 6 key codes wskbd1 at ukbd0 mux 1 uhidev2 at uhub4 port 1 configuration 1 interface 2 "Kinesis Advantage2 Keyboard" rev 2.00/1.00 addr 3 uhidev2: iclass 3/0, 2 report ids ucc0 at uhidev2 reportid 1: 7 usages, 7 keys, enum wskbd2 at ucc0 mux 1 uhid0 at uhidev2 reportid 2: input=1, output=0, feature=0 uhidev3 at uhub4 port 2 configuration 1 interface 0 "Yubico Yubico Gnubby (gnubby1)" rev 2.00/0.97 addr 4 uhidev3: iclass 3/0 fido0 at uhidev3: input=64, output=64, feature=0 uhidev4 at uhub4 port 4 configuration 1 interface 0 "Microsoft Microsoft 3-Button Mouse with IntelliEye(TM)" rev 1.10/3.00 addr 5 uhidev4: iclass 3/1 ums1 at uhidev4: 3 buttons, Z dir wsmouse1 at ums1 mux 0 uhub5 at uhub0 port 8 configuration 1 interface 0 "VIA Labs, Inc. USB3.0 Hub" rev 3.00/b.e1 addr 6 ... wsdisplay0 at amdgpu0 mux 1: console (std, vt100 emulation), using wskbd0 wskbd1: connecting to wsdisplay0 wskbd2: connecting to wsdisplay0 wsdisplay0: screen 1-5 added (std, vt100 emulation) OpenBSD 7.0-current (GENERIC.MP) #15: Thu Nov 11 16:55:24 PST 2021 ... xhci0 at pci4 dev 0 function 1 "AMD 17h xHCI" rev 0x00: msi, xHCI 1.10 usb0 at xhci0: USB revision 3.0 uhub0 at usb0 configuration 1 interface 0 "AMD xHCI root hub" rev 3.00/1.00 addr 1 ... uhub4 at uhub0 port 1 configuration 1 interface 0 "VIA Labs, Inc. USB2.0 Hub" rev 2.10/b.e0 addr 2 uhidev0 at uhub4 port 1 configuration 1 interface 0 "Kinesis Advantage2 Keyboard" rev 2.00/1.00 addr 3 uhidev0: iclass 3/1 ums0 at uhidev0: 3 buttons, Z dir wsmouse0 at ums0 mux 0 ums1 at uhidev0: 3 buttons, Z dir wsmouse1 at ums1 mux 0 uhidev1 at uhub4 port 1 configuration 1 interface 1 "Kinesis Advantage2 Keyboard" rev 2.00/1.00 addr 3 uhidev1: iclass 3/1 ukbd0 at uhidev1: 8 variable keys, 6 key codes wskbd1 at ukbd0 mux 1 ukbd1 at uhidev1: 8 variable keys, 6 key codes wskbd2 at ukbd1 mux 1 uhidev2 at uhub4 port 1 configuration 1 interface 2 "Kinesis Advantage2 Keyboard" rev 2.00/1.00 addr 3 uhidev2: iclass 3/0, 2 report ids ucc0 at uhidev2 reportid 1: 7 usages, 7 keys, enum wskbd3 at ucc0 mux 1 uhid0 at uhidev2 reportid 2: input=1, output=0, feature=0 uhidev3 at uhub4 port 2 configuration 1 interface 0 "Yubico Yubico Gnubby (gnubby1)" rev 2.00/0.97 addr 4 uhidev3: iclass 3/0 fido0 at uhidev3: input=64, output=64, feature=0 uhidev4 at uhub4 port 4 configuration 1 interface 0 "Microsoft Microsoft 3-Button Mouse with IntelliEye(TM)" rev 1.10/3.00 addr 5 uhidev4: iclass 3/1 ums2 at uhidev4: 3 buttons, Z dir wsmouse2 at ums2 mux 0 ums3 at uhidev4: 3 buttons, Z dir wsmouse3 at ums3 mux 0 uhub5 at uhub0 port 8 configuration 1 interface 0 "VIA Labs, Inc. USB3.0 Hub" rev 3.00/b.e1 addr 6 ... wsdisplay0 at amdgpu0 mux 1: console (std, vt100 emulation), using wskbd0 wskbd1: connecting to wsdisplay0 wskbd2: connecting to wsdisplay0 wskbd3: connecting to wsdisplay0 wsdisplay0: screen 1-5 added (std, vt100 emulation)
UNIX sockets: move garbage collector data out from `unp_lock'
The final step before rework UNIX sockets to fine grained locks. Except
`unp_ino' this leaves only per-socket data protected by `unp_lock'. The
`unp_ino' protection is not the big deal and will be done with mutex(9)
in the future diff.
The `unp_gc_lock' rwlock(9) protects `unp_defer', `unp_gcing',
`unp_gcflags' and `unp_link' list.
We need to simultaneously lock `unp_gc_lock' and `unp_lock'. When we
perform unp_attach() or unp_detach() we link PCB to `unp_link' list with
`unp_lock' held and the lock order should be `unp_lock' ->
`unp_gc_lock'. But when unp_gc() does `unp_link' list walkthrough with
the `unp_gc_lock' lock held it should lock socket while performs
`so_rcv' buffer scan and the lock order should be the opposite.
In the future diff `unp_lock' will be replaced by per-socket `so_lock'
so it's better to enforce `unp_gc_lock' -> `unp_lock' (solock()) lock
order and release `unp_lock' in the unp_attach() and unp_detach() paths.
The previously committed diffs made this safe.
The `unp_df_lock' introduced because the `unp_lock' and `unp_gc_lock'
state are unknown when unp_discard() called. Since it touches only
`ud_link' list the re-lock dances are unwanted in this path. Also this
keeps M_WAITOK allocation outside rwlock(9) when unp_discard() called
from unp_externalize() error path.
The garbage collector flags moved from solock() protected `unp_flags' to
`unp_gc_lock' protected `unp_gcflags'
The regress/sys/kern/ungc could be used to test this diff.
Index: sys/kern/uipc_usrreq.c
===
RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.156
diff -u -p -r1.156 uipc_usrreq.c
--- sys/kern/uipc_usrreq.c 11 Nov 2021 18:36:59 - 1.156
+++ sys/kern/uipc_usrreq.c 12 Nov 2021 00:18:21 -
@@ -59,12 +59,17 @@
/*
* Locks used to protect global data and struct members:
* I immutable after creation
+ * D unp_df_lock
+ * G unp_gc_lock
* U unp_lock
* R unp_rights_mtx
* a atomic
*/
struct rwlock unp_lock = RWLOCK_INITIALIZER("unplock");
+struct rwlock unp_df_lock = RWLOCK_INITIALIZER("unpdflk");
+struct rwlock unp_gc_lock = RWLOCK_INITIALIZER("unpgclk");
+
struct mutex unp_rights_mtx = MUTEX_INITIALIZER(IPL_SOFTNET);
/*
@@ -72,7 +77,7 @@ struct mutex unp_rights_mtx = MUTEX_INIT
* not received and need to be closed.
*/
struct unp_deferral {
- SLIST_ENTRY(unp_deferral) ud_link;/* [U] */
+ SLIST_ENTRY(unp_deferral) ud_link;/* [D] */
int ud_n; /* [I] */
/* followed by ud_n struct fdpass */
struct fdpass ud_fp[];/* [I] */
@@ -97,17 +102,17 @@ struct task unp_gc_task = TASK_INITIALIZ
*/
const struct sockaddr sun_noname = { sizeof(sun_noname), AF_UNIX };
-/* [U] list of all UNIX domain sockets, for unp_gc() */
+/* [G] list of all UNIX domain sockets, for unp_gc() */
LIST_HEAD(unp_head, unpcb) unp_head =
LIST_HEAD_INITIALIZER(unp_head);
-/* [U] list of sets of files that were sent over sockets that are now closed */
+/* [D] list of sets of files that were sent over sockets that are now closed */
SLIST_HEAD(,unp_deferral) unp_deferred =
SLIST_HEAD_INITIALIZER(unp_deferred);
ino_t unp_ino;/* [U] prototype for fake inode numbers */
intunp_rights; /* [R] file descriptors in flight */
-intunp_defer; /* [U] number of deferred fp to close by the GC task */
-intunp_gcing; /* [U] GC task currently running */
+intunp_defer; /* [G] number of deferred fp to close by the GC task */
+intunp_gcing; /* [G] GC task currently running */
void
unp_init(void)
@@ -430,7 +435,21 @@ uipc_attach(struct socket *so, int proto
unp->unp_socket = so;
so->so_pcb = unp;
getnanotime(>unp_ctime);
+
+ /*
+* Enforce `unp_gc_lock' -> `solock()' lock order.
+*/
+ /*
+* We also release the lock on listening socket and on our peer
+* socket when called from unp_connect(). This is safe. The
+* listening socket protected by vnode(9) lock. The peer socket
+* has 'UNP_CONNECTING' flag set.
+*/
+ sounlock(so, SL_LOCKED);
+ rw_enter_write(_gc_lock);
LIST_INSERT_HEAD(_head, unp, unp_link);
+ rw_exit_write(_gc_lock);
+ solock(so);
return (0);
}
@@ -490,28 +509,29 @@ unp_detach(struct unpcb *unp)
rw_assert_wrlock(_lock);
- LIST_REMOVE(unp, unp_link);
-
- if (vp != NULL) {
- unp->unp_vnode = NULL;
+ unp->unp_vnode = NULL;
- /*
-* Enforce `i_lock' -> `unp_lock' because fifo
-* subsystem requires it.
-*/
+ /*
+* Enforce `unp_gc_lock' -> `solock()' lock order.
+* Enforce `i_lock' -> `unp_lock' lock order.
+
Re: make.1: Inconsistent whitespace
On Thu, Nov 11, 2021 at 06:25:46PM +0100, Leon Fischer wrote: > Some keywords in make(1)'s list of conditionals have extra spaces: > > .ifmake [!]target [operator target ...] > Test the target being built. > > .ifnmake [!] target [operator target ...] > Test the target being built. > fixed, thanks. jmc > Index: make.1 > === > RCS file: /cvs/src/usr.bin/make/make.1,v > retrieving revision 1.133 > diff -u -p -r1.133 make.1 > --- make.18 Mar 2021 06:20:50 - 1.133 > +++ make.111 Nov 2021 17:22:18 - > @@ -1082,7 +1082,7 @@ Test the value of a variable. > Test the target being built. > .It Xo > .Ic .ifnmake > -.Oo \&! Oc Ar target > +.Oo \&! Oc Ns Ar target > .Op Ar operator target ... > .Xc > Test the target being built. > @@ -1090,7 +1090,7 @@ Test the target being built. > Reverse the sense of the last conditional. > .It Xo > .Ic .elif > -.Oo \&! Oc Ar expression > +.Oo \&! Oc Ns Ar expression > .Op Ar operator expression ... > .Xc > A combination of >
Re: [PATCH] [src] share/man/man5/hostname.if.5 - nwid -> join
On Thu, Nov 11, 2021 at 08:30:45PM +, Klemens Nanni wrote: > On Thu, Nov 11, 2021 at 08:13:13PM +, Jason McIntyre wrote: > > i don;t think this bit of text works well now because you say it runs > > ifconfig 3 times then ... respectively. but you only list two things > > (even though i understand dynamic config is two). since your text is > > already clear, i'd omit ", respectively". or maybe say "inet4 and inet6 > > dynamic ..." to balance out the three times would be better. > > Thanks, is this better? > > > Index: hostname.if.5 > === > RCS file: /cvs/src/share/man/man5/hostname.if.5,v > retrieving revision 1.77 > diff -u -p -r1.77 hostname.if.5 > --- hostname.if.5 17 Jul 2021 15:28:31 - 1.77 > +++ hostname.if.5 11 Nov 2021 20:30:05 - > @@ -67,21 +67,13 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d > Each line is processed separately and in order. > For example: > .Bd -literal -offset indent > -nwid mynwid wpakey mywpakey > +join mynwid wpakey mywpakey > inet6 autoconf > inet autoconf > .Ed > .Pp > -would run ifconfig three times to set the > -.Cm nwid > -and > -.Cm wpakey > -of the interface, > -the > -.Sy AUTOCONF6 > -flag and the > -.Sy AUTOCONF4 > -flag, respectively. > +would run ifconfig three times to add a wireless network using WPA to the > +join list and enable dynamic address configuration for IPv6 and IPv4. > .Sh STATIC ADDRESS CONFIGURATION > The following packed formats are valid for configuring network > interfaces with static addresses. > bingo! jmc
Re: [PATCH] [src] share/man/man5/hostname.if.5 - nwid -> join
On Thu, Nov 11, 2021 at 08:13:13PM +, Jason McIntyre wrote: > i don;t think this bit of text works well now because you say it runs > ifconfig 3 times then ... respectively. but you only list two things > (even though i understand dynamic config is two). since your text is > already clear, i'd omit ", respectively". or maybe say "inet4 and inet6 > dynamic ..." to balance out the three times would be better. Thanks, is this better? Index: hostname.if.5 === RCS file: /cvs/src/share/man/man5/hostname.if.5,v retrieving revision 1.77 diff -u -p -r1.77 hostname.if.5 --- hostname.if.5 17 Jul 2021 15:28:31 - 1.77 +++ hostname.if.5 11 Nov 2021 20:30:05 - @@ -67,21 +67,13 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d Each line is processed separately and in order. For example: .Bd -literal -offset indent -nwid mynwid wpakey mywpakey +join mynwid wpakey mywpakey inet6 autoconf inet autoconf .Ed .Pp -would run ifconfig three times to set the -.Cm nwid -and -.Cm wpakey -of the interface, -the -.Sy AUTOCONF6 -flag and the -.Sy AUTOCONF4 -flag, respectively. +would run ifconfig three times to add a wireless network using WPA to the +join list and enable dynamic address configuration for IPv6 and IPv4. .Sh STATIC ADDRESS CONFIGURATION The following packed formats are valid for configuring network interfaces with static addresses.
Re: sigsuspend(2): sleep on channel?
On Thu, Nov 11, 2021 at 08:53:20PM +0100, Mark Kettenis wrote:
> > Date: Thu, 11 Nov 2021 13:30:04 -0600
> > From: Scott Cheloha
> >
> > My understanding of sigsuspend(2) is that it only returns if a signal
> > is delivered to the calling thread. However, in sys_sigsuspend() we
> > pass >p_p->ps_sigacts as the wakeup channel to tsleep_nsec(9).
> >
> > Are we actually waiting for a wakeup on that channel? Or can we sleep
> > on here? Patch attached. Note that we don't need to loop
> > here anymore: we can't receieve wakeup so tsleep_nsec(9) will never
> > return zero.
>
> I don't think we expect a wakeup on that channel. But the loop is
> still needed as a spurious wakeup may happen.
I'm not quite sure how you'd get a spurious wakeup in this context,
but I'll take your word for it.
So use to indicate we're not expecting a wakeup, but keep the
loop.
Index: kern_sig.c
===
RCS file: /cvs/src/sys/kern/kern_sig.c,v
retrieving revision 1.287
diff -u -p -r1.287 kern_sig.c
--- kern_sig.c 24 Oct 2021 00:02:25 - 1.287
+++ kern_sig.c 11 Nov 2021 20:10:51 -
@@ -509,7 +509,7 @@ dosigsuspend(struct proc *p, sigset_t ne
}
/*
- * Suspend process until signal, providing mask to be set
+ * Suspend thread until signal, providing mask to be set
* in the meantime. Note nonstandard calling convention:
* libc stub passes mask, not pointer, to save a copyin.
*/
@@ -519,12 +519,10 @@ sys_sigsuspend(struct proc *p, void *v,
struct sys_sigsuspend_args /* {
syscallarg(int) mask;
} */ *uap = v;
- struct process *pr = p->p_p;
- struct sigacts *ps = pr->ps_sigacts;
dosigsuspend(p, SCARG(uap, mask) &~ sigcantmask);
- while (tsleep_nsec(ps, PPAUSE|PCATCH, "sigsusp", INFSLP) == 0)
- /* void */;
+ while (tsleep_nsec(, PPAUSE|PCATCH, "sigsusp", INFSLP) == 0)
+ continue;
/* always return EINTR rather than ERESTART... */
return (EINTR);
}
Re: [PATCH] [src] share/man/man5/hostname.if.5 - nwid -> join
On Thu, Nov 11, 2021 at 08:01:37PM +, Klemens Nanni wrote: > On Thu, Nov 11, 2021 at 05:48:24PM +, Stuart Henderson wrote: > > On 2021/11/11 16:30, Klemens Nanni wrote: > > > On Thu, Nov 11, 2021 at 04:11:10PM +, Raf Czlonka wrote: > > > > Hello, > > > > > > > > It seems like this has been missed in recent thread[0]. > > > > > > > > Not entirely sure whether the sentence "flows" any longer but here > > > > it goes anyway. > > > > > > > > [0] https://marc.info/?l=openbsd-tech=163507448118443=2 > > > > > > Thanks, I missed that! > > > > > > > Index: share/man/man5/hostname.if.5 > > > > === > > > > RCS file: /cvs/src/share/man/man5/hostname.if.5,v > > > > retrieving revision 1.77 > > > > diff -u -p -r1.77 hostname.if.5 > > > > --- share/man/man5/hostname.if.517 Jul 2021 15:28:31 - > > > > 1.77 > > > > +++ share/man/man5/hostname.if.511 Nov 2021 16:09:33 - > > > > @@ -67,13 +67,13 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d > > > > Each line is processed separately and in order. > > > > For example: > > > > .Bd -literal -offset indent > > > > -nwid mynwid wpakey mywpakey > > > > +join mynwid wpakey mywpakey > > > > inet6 autoconf > > > > inet autoconf > > > > .Ed > > > > .Pp > > > > would run ifconfig three times to set the > > > > -.Cm nwid > > > > +.Cm join > > > > and > > > > .Cm wpakey > > > > of the interface, > > > > > > Maybe this reads better in general? > > > > > > would run ifconfig three times to join a wireless network using WPA > > > and > > > to set the AUTOCONF6 and AUTOCONF4 flags, respectively. > > > > It is more "add a network to the join list", join implies that it's > > connecting directly to that ssid (i.e. what "nwid" did). > > Good point. > Also no markup as per jmc. > > OK? > > > Index: hostname.if.5 > === > RCS file: /cvs/src/share/man/man5/hostname.if.5,v > retrieving revision 1.77 > diff -u -p -r1.77 hostname.if.5 > --- hostname.if.5 17 Jul 2021 15:28:31 - 1.77 > +++ hostname.if.5 11 Nov 2021 20:00:45 - > @@ -67,21 +67,13 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d > Each line is processed separately and in order. > For example: > .Bd -literal -offset indent > -nwid mynwid wpakey mywpakey > +join mynwid wpakey mywpakey > inet6 autoconf > inet autoconf > .Ed > .Pp > -would run ifconfig three times to set the > -.Cm nwid > -and > -.Cm wpakey > -of the interface, > -the > -.Sy AUTOCONF6 > -flag and the > -.Sy AUTOCONF4 > -flag, respectively. > +would run ifconfig three times to add a wireless network using WPA to the > +join list and enable dynamic address configuration, respectively. > .Sh STATIC ADDRESS CONFIGURATION > The following packed formats are valid for configuring network > interfaces with static addresses. > i don;t think this bit of text works well now because you say it runs ifconfig 3 times then ... respectively. but you only list two things (even though i understand dynamic config is two). since your text is already clear, i'd omit ", respectively". or maybe say "inet4 and inet6 dynamic ..." to balance out the three times would be better. jmc
Re: [PATCH] [src] share/man/man5/hostname.if.5 - nwid -> join
On Thu, Nov 11, 2021 at 05:48:24PM +, Stuart Henderson wrote: > On 2021/11/11 16:30, Klemens Nanni wrote: > > On Thu, Nov 11, 2021 at 04:11:10PM +, Raf Czlonka wrote: > > > Hello, > > > > > > It seems like this has been missed in recent thread[0]. > > > > > > Not entirely sure whether the sentence "flows" any longer but here > > > it goes anyway. > > > > > > [0] https://marc.info/?l=openbsd-tech=163507448118443=2 > > > > Thanks, I missed that! > > > > > Index: share/man/man5/hostname.if.5 > > > === > > > RCS file: /cvs/src/share/man/man5/hostname.if.5,v > > > retrieving revision 1.77 > > > diff -u -p -r1.77 hostname.if.5 > > > --- share/man/man5/hostname.if.5 17 Jul 2021 15:28:31 - 1.77 > > > +++ share/man/man5/hostname.if.5 11 Nov 2021 16:09:33 - > > > @@ -67,13 +67,13 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d > > > Each line is processed separately and in order. > > > For example: > > > .Bd -literal -offset indent > > > -nwid mynwid wpakey mywpakey > > > +join mynwid wpakey mywpakey > > > inet6 autoconf > > > inet autoconf > > > .Ed > > > .Pp > > > would run ifconfig three times to set the > > > -.Cm nwid > > > +.Cm join > > > and > > > .Cm wpakey > > > of the interface, > > > > Maybe this reads better in general? > > > > would run ifconfig three times to join a wireless network using WPA and > > to set the AUTOCONF6 and AUTOCONF4 flags, respectively. > > It is more "add a network to the join list", join implies that it's > connecting directly to that ssid (i.e. what "nwid" did). Good point. Also no markup as per jmc. OK? Index: hostname.if.5 === RCS file: /cvs/src/share/man/man5/hostname.if.5,v retrieving revision 1.77 diff -u -p -r1.77 hostname.if.5 --- hostname.if.5 17 Jul 2021 15:28:31 - 1.77 +++ hostname.if.5 11 Nov 2021 20:00:45 - @@ -67,21 +67,13 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d Each line is processed separately and in order. For example: .Bd -literal -offset indent -nwid mynwid wpakey mywpakey +join mynwid wpakey mywpakey inet6 autoconf inet autoconf .Ed .Pp -would run ifconfig three times to set the -.Cm nwid -and -.Cm wpakey -of the interface, -the -.Sy AUTOCONF6 -flag and the -.Sy AUTOCONF4 -flag, respectively. +would run ifconfig three times to add a wireless network using WPA to the +join list and enable dynamic address configuration, respectively. .Sh STATIC ADDRESS CONFIGURATION The following packed formats are valid for configuring network interfaces with static addresses.
Re: amd4/bsd.rd: make "config -e" work
The growth is not terrible, considering what media this is.
Fine with me.
Klemens Nanni wrote:
> On Thu, Nov 04, 2021 at 10:49:46PM +, Klemens Nanni wrote:
> > amd64, alpha, i386 and macppc strip all symbols off the ramdisk bsd.rd
> > (before gzipping it) and thus break config(8)'s modification feature:
> >
> > $ gzcat bsd.rd > bsd.rd.raw
> > $ config -e bsd.rd.raw
> > WARNING no output file specified
> > WARNING this kernel doesn't contain all information needed!
> > WARNING the commands add and change might not work.
> > WARNING this kernel doesn't support pseudo devices.
> > WARNING this kernel doesn't support modification of NKMEMPAGES.
> > config: failed to get first cfdata
> >
> > Needing 'disable xhci' on arm64/Pinebook Pro right now, I looked into
> > all of bsd.re-config(5), UKC and how we build bsd.rd for all platforms.
>
> Correction: needed it for the Pi 4, the PBP had other problems...
>
> > That's how I noticed modifying a ramdisk kernel on these for platforms
> > wouldn't work in the first place.
> >
> > Needing it to fix or upgrade systems can be crucial, so I figured it's
> > worth addressing.
> >
> > While `boot /bsd.rd -c' does work on amd64 and macppc regardless of
> > symbols, its changes do not persist and `config -e' is more convenient.
> >
> > So I propose to strip less symbols such to make this work.
> >
> > On amd64, this accounts for 200K growth in bsd.gz according to `du -k':
>
> New diff now that amd64 RAMDISK_CD contains igc(4):
>
> -9168obj/bsd.strip
> +9872obj/bsd.strip
> -4272obj/bsd.gz
> +4480obj/bsd.gz
>
> > I cranked FSSIZE to the smallest possible volume that would build.
> > The resulting miniroot70.img boots fine in vmm(4) and on a X250.
>
> In 512-byte block counts (`du obj/bsd.{strip,gz}`):
>
> -18336 obj/bsd.strip
> +19744 obj/bsd.strip
> -8544obj/bsd.gz
> +8960obj/bsd.gz
>
> So I first cranked FSSIZE by 8960 - 8544 = 416, from 10240 to 10686.
> That built, but is more than required, so I bisected it to the minimal
> working value of 10368.
>
> amd64 builds, boots and works as expected.
>
> > macppc is still building; I have no alpha or i386.
>
> macppc builds, boots and works as expected without cranking FSSIZE.
>
>
> Feedback? Objections? OK?
>
> Index: amd64/ramdisk_cd/Makefile
> ===
> RCS file: /cvs/src/distrib/amd64/ramdisk_cd/Makefile,v
> retrieving revision 1.32
> diff -u -p -r1.32 Makefile
> --- amd64/ramdisk_cd/Makefile 7 Nov 2021 15:50:15 - 1.32
> +++ amd64/ramdisk_cd/Makefile 11 Nov 2021 19:43:56 -
> @@ -1,7 +1,7 @@
> #$OpenBSD: Makefile,v 1.32 2021/11/07 15:50:15 deraadt Exp $
>
> FS= miniroot${OSrev}.img
> -FSSIZE= 10240
> +FSSIZE= 10368
> FSDISKTYPE= mini34
> CDROM= cd${OSrev}.iso
> MOUNT_POINT= /mnt
> @@ -56,7 +56,7 @@ MRDISKTYPE= rdrootb
> MRMAKEFSARGS=-o disklabel=${MRDISKTYPE},minfree=0,density=4096
>
> bsd.gz: bsd.rd
> - objcopy -S -R .comment -R .SUNW_ctf \
> + objcopy -g -x -R .comment -R .SUNW_ctf \
> -K rd_root_size -K rd_root_image \
> bsd.rd bsd.strip
> gzip -9cn bsd.strip > bsd.gz
> Index: macppc/ramdisk/Makefile
> ===
> RCS file: /cvs/src/distrib/macppc/ramdisk/Makefile,v
> retrieving revision 1.51
> diff -u -p -r1.51 Makefile
> --- macppc/ramdisk/Makefile 26 Jul 2021 12:47:46 - 1.51
> +++ macppc/ramdisk/Makefile 11 Nov 2021 19:44:14 -
> @@ -35,7 +35,7 @@ bsd.gz: bsd.rd
> gzip -9cn bsd.rd > bsd.gz
>
> bsd.rd: mr.fs bsd
> - objcopy -S -R .comment -R .SUNW_ctf \
> + objcopy -g -x -R .comment -R .SUNW_ctf \
> -K rd_root_size -K rd_root_image \
> bsd bsd.rd
> rdsetroot bsd.rd mr.fs
>
Re: sigsuspend(2): sleep on channel?
> Date: Thu, 11 Nov 2021 13:30:04 -0600
> From: Scott Cheloha
>
> My understanding of sigsuspend(2) is that it only returns if a signal
> is delivered to the calling thread. However, in sys_sigsuspend() we
> pass >p_p->ps_sigacts as the wakeup channel to tsleep_nsec(9).
>
> Are we actually waiting for a wakeup on that channel? Or can we sleep
> on here? Patch attached. Note that we don't need to loop
> here anymore: we can't receieve wakeup so tsleep_nsec(9) will never
> return zero.
I don't think we expect a wakeup on that channel. But the loop is
still needed as a spurious wakeup may happen.
> I can't find any wakeups in the kernel sent to any ps_sigacts, but
> maybe I missed something.
>
> If not, ok?
>
> Index: kern_sig.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sig.c,v
> retrieving revision 1.287
> diff -u -p -r1.287 kern_sig.c
> --- kern_sig.c24 Oct 2021 00:02:25 - 1.287
> +++ kern_sig.c11 Nov 2021 19:24:49 -
> @@ -509,7 +509,7 @@ dosigsuspend(struct proc *p, sigset_t ne
> }
>
> /*
> - * Suspend process until signal, providing mask to be set
> + * Suspend thread until signal, providing mask to be set
> * in the meantime. Note nonstandard calling convention:
> * libc stub passes mask, not pointer, to save a copyin.
> */
> @@ -519,12 +519,9 @@ sys_sigsuspend(struct proc *p, void *v,
> struct sys_sigsuspend_args /* {
> syscallarg(int) mask;
> } */ *uap = v;
> - struct process *pr = p->p_p;
> - struct sigacts *ps = pr->ps_sigacts;
>
> dosigsuspend(p, SCARG(uap, mask) &~ sigcantmask);
> - while (tsleep_nsec(ps, PPAUSE|PCATCH, "sigsusp", INFSLP) == 0)
> - /* void */;
> + tsleep_nsec(, PPAUSE|PCATCH, "sigsusp", INFSLP);
> /* always return EINTR rather than ERESTART... */
> return (EINTR);
> }
>
>
Re: amd4/bsd.rd: make "config -e" work
On Thu, Nov 04, 2021 at 10:49:46PM +, Klemens Nanni wrote:
> amd64, alpha, i386 and macppc strip all symbols off the ramdisk bsd.rd
> (before gzipping it) and thus break config(8)'s modification feature:
>
> $ gzcat bsd.rd > bsd.rd.raw
> $ config -e bsd.rd.raw
> WARNING no output file specified
> WARNING this kernel doesn't contain all information needed!
> WARNING the commands add and change might not work.
> WARNING this kernel doesn't support pseudo devices.
> WARNING this kernel doesn't support modification of NKMEMPAGES.
> config: failed to get first cfdata
>
> Needing 'disable xhci' on arm64/Pinebook Pro right now, I looked into
> all of bsd.re-config(5), UKC and how we build bsd.rd for all platforms.
Correction: needed it for the Pi 4, the PBP had other problems...
> That's how I noticed modifying a ramdisk kernel on these for platforms
> wouldn't work in the first place.
>
> Needing it to fix or upgrade systems can be crucial, so I figured it's
> worth addressing.
>
> While `boot /bsd.rd -c' does work on amd64 and macppc regardless of
> symbols, its changes do not persist and `config -e' is more convenient.
>
> So I propose to strip less symbols such to make this work.
>
> On amd64, this accounts for 200K growth in bsd.gz according to `du -k':
New diff now that amd64 RAMDISK_CD contains igc(4):
-9168obj/bsd.strip
+9872obj/bsd.strip
-4272obj/bsd.gz
+4480obj/bsd.gz
> I cranked FSSIZE to the smallest possible volume that would build.
> The resulting miniroot70.img boots fine in vmm(4) and on a X250.
In 512-byte block counts (`du obj/bsd.{strip,gz}`):
-18336 obj/bsd.strip
+19744 obj/bsd.strip
-8544obj/bsd.gz
+8960obj/bsd.gz
So I first cranked FSSIZE by 8960 - 8544 = 416, from 10240 to 10686.
That built, but is more than required, so I bisected it to the minimal
working value of 10368.
amd64 builds, boots and works as expected.
> macppc is still building; I have no alpha or i386.
macppc builds, boots and works as expected without cranking FSSIZE.
Feedback? Objections? OK?
Index: amd64/ramdisk_cd/Makefile
===
RCS file: /cvs/src/distrib/amd64/ramdisk_cd/Makefile,v
retrieving revision 1.32
diff -u -p -r1.32 Makefile
--- amd64/ramdisk_cd/Makefile 7 Nov 2021 15:50:15 - 1.32
+++ amd64/ramdisk_cd/Makefile 11 Nov 2021 19:43:56 -
@@ -1,7 +1,7 @@
# $OpenBSD: Makefile,v 1.32 2021/11/07 15:50:15 deraadt Exp $
FS=miniroot${OSrev}.img
-FSSIZE=10240
+FSSIZE=10368
FSDISKTYPE=mini34
CDROM= cd${OSrev}.iso
MOUNT_POINT= /mnt
@@ -56,7 +56,7 @@ MRDISKTYPE= rdrootb
MRMAKEFSARGS= -o disklabel=${MRDISKTYPE},minfree=0,density=4096
bsd.gz: bsd.rd
- objcopy -S -R .comment -R .SUNW_ctf \
+ objcopy -g -x -R .comment -R .SUNW_ctf \
-K rd_root_size -K rd_root_image \
bsd.rd bsd.strip
gzip -9cn bsd.strip > bsd.gz
Index: macppc/ramdisk/Makefile
===
RCS file: /cvs/src/distrib/macppc/ramdisk/Makefile,v
retrieving revision 1.51
diff -u -p -r1.51 Makefile
--- macppc/ramdisk/Makefile 26 Jul 2021 12:47:46 - 1.51
+++ macppc/ramdisk/Makefile 11 Nov 2021 19:44:14 -
@@ -35,7 +35,7 @@ bsd.gz: bsd.rd
gzip -9cn bsd.rd > bsd.gz
bsd.rd: mr.fs bsd
- objcopy -S -R .comment -R .SUNW_ctf \
+ objcopy -g -x -R .comment -R .SUNW_ctf \
-K rd_root_size -K rd_root_image \
bsd bsd.rd
rdsetroot bsd.rd mr.fs
sigsuspend(2): sleep on channel?
My understanding of sigsuspend(2) is that it only returns if a signal
is delivered to the calling thread. However, in sys_sigsuspend() we
pass >p_p->ps_sigacts as the wakeup channel to tsleep_nsec(9).
Are we actually waiting for a wakeup on that channel? Or can we sleep
on here? Patch attached. Note that we don't need to loop
here anymore: we can't receieve wakeup so tsleep_nsec(9) will never
return zero.
I can't find any wakeups in the kernel sent to any ps_sigacts, but
maybe I missed something.
If not, ok?
Index: kern_sig.c
===
RCS file: /cvs/src/sys/kern/kern_sig.c,v
retrieving revision 1.287
diff -u -p -r1.287 kern_sig.c
--- kern_sig.c 24 Oct 2021 00:02:25 - 1.287
+++ kern_sig.c 11 Nov 2021 19:24:49 -
@@ -509,7 +509,7 @@ dosigsuspend(struct proc *p, sigset_t ne
}
/*
- * Suspend process until signal, providing mask to be set
+ * Suspend thread until signal, providing mask to be set
* in the meantime. Note nonstandard calling convention:
* libc stub passes mask, not pointer, to save a copyin.
*/
@@ -519,12 +519,9 @@ sys_sigsuspend(struct proc *p, void *v,
struct sys_sigsuspend_args /* {
syscallarg(int) mask;
} */ *uap = v;
- struct process *pr = p->p_p;
- struct sigacts *ps = pr->ps_sigacts;
dosigsuspend(p, SCARG(uap, mask) &~ sigcantmask);
- while (tsleep_nsec(ps, PPAUSE|PCATCH, "sigsusp", INFSLP) == 0)
- /* void */;
+ tsleep_nsec(, PPAUSE|PCATCH, "sigsusp", INFSLP);
/* always return EINTR rather than ERESTART... */
return (EINTR);
}
Re: New hw.perfpolicy behavior
Nicola Dell'Uomo wrote: > Hi, > > I tested my laptop using different -CURRENT releases (#67, #71, #76) and the > problem persists. > First of all I disabled apmd as Theo suggested and my laptop did not auto > suspended anymore. > This is the output of systat sens -1 when my system fails to read battery > status (with last output from a working setup, just for comparison): > > > acpibat0.volt015.40 V DC voltage > acpibat0.volt1 0.00 V DC unknown current voltage > acpibat0.power0 0.00 W unknown rate > acpibat0.watthour0 51.99 Wh last full capacity > acpibat0.watthour1 2.60 Wh warning capacity > acpibat0.watthour2 0.20 Wh low capacity > acpibat0.watthour3 0.00 Wh unknown remaining capacity > acpibat0.watthour4 51.00 Wh design capacity > acpibat0.raw0 1 raw unknown battery unknown > acpibat0.raw1335 raw discharge cycles I don't think this is a software problem.
Re: [PATCH] [src] share/man/man5/hostname.if.5 - nwid -> join
On 2021/11/11 16:30, Klemens Nanni wrote: > On Thu, Nov 11, 2021 at 04:11:10PM +, Raf Czlonka wrote: > > Hello, > > > > It seems like this has been missed in recent thread[0]. > > > > Not entirely sure whether the sentence "flows" any longer but here > > it goes anyway. > > > > [0] https://marc.info/?l=openbsd-tech=163507448118443=2 > > Thanks, I missed that! > > > Index: share/man/man5/hostname.if.5 > > === > > RCS file: /cvs/src/share/man/man5/hostname.if.5,v > > retrieving revision 1.77 > > diff -u -p -r1.77 hostname.if.5 > > --- share/man/man5/hostname.if.517 Jul 2021 15:28:31 - 1.77 > > +++ share/man/man5/hostname.if.511 Nov 2021 16:09:33 - > > @@ -67,13 +67,13 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d > > Each line is processed separately and in order. > > For example: > > .Bd -literal -offset indent > > -nwid mynwid wpakey mywpakey > > +join mynwid wpakey mywpakey > > inet6 autoconf > > inet autoconf > > .Ed > > .Pp > > would run ifconfig three times to set the > > -.Cm nwid > > +.Cm join > > and > > .Cm wpakey > > of the interface, > > Maybe this reads better in general? > > would run ifconfig three times to join a wireless network using WPA and > to set the AUTOCONF6 and AUTOCONF4 flags, respectively. It is more "add a network to the join list", join implies that it's connecting directly to that ssid (i.e. what "nwid" did). > No need to explain lines command by command. Keep it simple; users > must read ifconfig(8) anyway. > > One might as well say to > > ... and > to enable automatic address configuration for IPv6 and IPv4, respectively. > > Then we don't have the weird mix of conceptual words and technical bits, > i.e. "connect to wifi" vs. "set if flags [they're picked up by network > daemons, but we don't say which ones]". > > DYNAMIC ADDRESS CONFIGURATION down below repeats all this, anyway. > > Feedback? OK? > > > Index: hostname.if.5 > === > RCS file: /cvs/src/share/man/man5/hostname.if.5,v > retrieving revision 1.77 > diff -u -p -r1.77 hostname.if.5 > --- hostname.if.5 17 Jul 2021 15:28:31 - 1.77 > +++ hostname.if.5 11 Nov 2021 16:30:00 - > @@ -67,21 +67,15 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d > Each line is processed separately and in order. > For example: > .Bd -literal -offset indent > -nwid mynwid wpakey mywpakey > +join mynwid wpakey mywpakey > inet6 autoconf > inet autoconf > .Ed > .Pp > -would run ifconfig three times to set the > -.Cm nwid > -and > -.Cm wpakey > -of the interface, > -the > -.Sy AUTOCONF6 > -flag and the > -.Sy AUTOCONF4 > -flag, respectively. > +would run ifconfig three times to > +.Cm join > +a wireless network using WPA > +and enable dynamic address configuration, respectively. > .Sh STATIC ADDRESS CONFIGURATION > The following packed formats are valid for configuring network > interfaces with static addresses. >
Re: [PATCH] [src] share/man/man5/hostname.if.5 - nwid -> join
On Thu, Nov 11, 2021 at 04:30:15PM +, Klemens Nanni wrote: > On Thu, Nov 11, 2021 at 04:11:10PM +, Raf Czlonka wrote: > > Hello, > > > > It seems like this has been missed in recent thread[0]. > > > > Not entirely sure whether the sentence "flows" any longer but here > > it goes anyway. > > > > [0] https://marc.info/?l=openbsd-tech=163507448118443=2 > > Thanks, I missed that! > > > Index: share/man/man5/hostname.if.5 > > === > > RCS file: /cvs/src/share/man/man5/hostname.if.5,v > > retrieving revision 1.77 > > diff -u -p -r1.77 hostname.if.5 > > --- share/man/man5/hostname.if.517 Jul 2021 15:28:31 - 1.77 > > +++ share/man/man5/hostname.if.511 Nov 2021 16:09:33 - > > @@ -67,13 +67,13 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d > > Each line is processed separately and in order. > > For example: > > .Bd -literal -offset indent > > -nwid mynwid wpakey mywpakey > > +join mynwid wpakey mywpakey > > inet6 autoconf > > inet autoconf > > .Ed > > .Pp > > would run ifconfig three times to set the > > -.Cm nwid > > +.Cm join > > and > > .Cm wpakey > > of the interface, > > Maybe this reads better in general? > > would run ifconfig three times to join a wireless network using WPA and > to set the AUTOCONF6 and AUTOCONF4 flags, respectively. > > No need to explain lines command by command. Keep it simple; users > must read ifconfig(8) anyway. > > One might as well say to > > ... and > to enable automatic address configuration for IPv6 and IPv4, respectively. > > Then we don't have the weird mix of conceptual words and technical bits, > i.e. "connect to wifi" vs. "set if flags [they're picked up by network > daemons, but we don't say which ones]". > > DYNAMIC ADDRESS CONFIGURATION down below repeats all this, anyway. > > Feedback? OK? > > > Index: hostname.if.5 > === > RCS file: /cvs/src/share/man/man5/hostname.if.5,v > retrieving revision 1.77 > diff -u -p -r1.77 hostname.if.5 > --- hostname.if.5 17 Jul 2021 15:28:31 - 1.77 > +++ hostname.if.5 11 Nov 2021 16:30:00 - > @@ -67,21 +67,15 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d > Each line is processed separately and in order. > For example: > .Bd -literal -offset indent > -nwid mynwid wpakey mywpakey > +join mynwid wpakey mywpakey > inet6 autoconf > inet autoconf > .Ed > .Pp > -would run ifconfig three times to set the > -.Cm nwid > -and > -.Cm wpakey > -of the interface, > -the > -.Sy AUTOCONF6 > -flag and the > -.Sy AUTOCONF4 > -flag, respectively. > +would run ifconfig three times to > +.Cm join > +a wireless network using WPA > +and enable dynamic address configuration, respectively. > .Sh STATIC ADDRESS CONFIGURATION > The following packed formats are valid for configuring network > interfaces with static addresses. > hi. i think this reads fine. i would not mark up "join" myself, but whatever. jmc
Re: [PATCH] [src] share/man/man5/hostname.if.5 - nwid -> join
On Thu, Nov 11, 2021 at 04:30:15PM GMT, Klemens Nanni wrote: > > Maybe this reads better in general? > > would run ifconfig three times to join a wireless network using WPA and > to set the AUTOCONF6 and AUTOCONF4 flags, respectively. > > [...] > > Feedback? OK? FWIW, this looks much better to me. Raf > > Index: hostname.if.5 > === > RCS file: /cvs/src/share/man/man5/hostname.if.5,v > retrieving revision 1.77 > diff -u -p -r1.77 hostname.if.5 > --- hostname.if.5 17 Jul 2021 15:28:31 - 1.77 > +++ hostname.if.5 11 Nov 2021 16:30:00 - > @@ -67,21 +67,15 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d > Each line is processed separately and in order. > For example: > .Bd -literal -offset indent > -nwid mynwid wpakey mywpakey > +join mynwid wpakey mywpakey > inet6 autoconf > inet autoconf > .Ed > .Pp > -would run ifconfig three times to set the > -.Cm nwid > -and > -.Cm wpakey > -of the interface, > -the > -.Sy AUTOCONF6 > -flag and the > -.Sy AUTOCONF4 > -flag, respectively. > +would run ifconfig three times to > +.Cm join > +a wireless network using WPA > +and enable dynamic address configuration, respectively. > .Sh STATIC ADDRESS CONFIGURATION > The following packed formats are valid for configuring network > interfaces with static addresses.
Re: quotaon(8): small improvements
Friendly ping
On Fri, Oct 29, 2021 at 10:06:44AM +0200, Martin Vahlensieck wrote:
> Hi
>
> Here are some small changes to quotaon(8). If you want I can split
> them up, but since they are small I am sending one diff. Here is
> a list of changes roughly in the order they appear in the diff:
>
> - Constify some function arguments
>
> - Use __progname instead of separate whoami variable + small KNF
>where the line is touched anyways.
>
> - Order letters in getopt(3) call
>
> - Convert a fprintf(3) + perror(3) to warn(3). warn(3) is
>already used in this function for similar purposes.
>
> - Replace strtok(3) with strsep(3). Is there a more
>elegant way than my solution? (I took the freedom to collect all
>the char * variables into one line).
>
> - Set cp to NULL at the end of the while loop scanning the mount
>options. Otherwise if the last mount option contains a '=' the
>part after it is used as the quota file (of course only if the
>desired userquota/groupquota option isn't found).
>
> - Use strncmp(3) instead of memcmp(3). Looking at the kernel code
>it seems that it is zero terminated. The other code nearby uses
>strcmp(3) already.
>
> - Invert an if condition to prevent an empty body.
>
> Happy to hear feedback, also feel free to only commit parts of it.
>
> Best,
>
> Martin
>
> P.S.: If the prototypes are touched anyways, the names of the
> arguments might be removed as well to comply with KNF. Let me
> know if you want that.
>
> Index: quotaon.c
> ===
> RCS file: /cvs/src/usr.sbin/quotaon/quotaon.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 quotaon.c
> --- quotaon.c 26 Apr 2018 12:42:51 - 1.27
> +++ quotaon.c 29 Oct 2021 07:46:25 -
> @@ -44,6 +44,8 @@
> #include
> #include
>
> +extern char *__progname;
> +
> char *qfname = QUOTAFILENAME;
> char *qfextension[] = INITQFNAMES;
>
> @@ -52,34 +54,31 @@ int gflag; /* operate on group quotas *
> int uflag; /* operate on user quotas */
> int vflag; /* verbose */
>
> -void usage(char *whoami);
> -int hasquota(struct fstab *fs, int type, char **qfnamep, int force);
> -int quotaonoff(struct fstab *fs, int offmode, int type, char *qfpathname);
> -int oneof(char *target, char *list[], int cnt);
> -int readonly(struct fstab *fs);
> +void usage();
> +int hasquota(const struct fstab *fs, int type, char **qfnamep, int force);
> +int quotaonoff(const struct fstab *fs, int offmode, int type, char
> *qfpathname);
> +int oneof(const char *target, char *list[], int cnt);
> +int readonly(const struct fstab *fs);
>
>
> int
> main(int argc, char *argv[])
> {
> struct fstab *fs;
> - char *qfnp, *whoami;
> + char *qfnp;
> long argnum, done = 0;
> int i, offmode = 0, errs = 0;
> extern int optind;
> int ch;
>
> - whoami = strrchr(*argv, '/') + 1;
> - if (whoami == (char *)1)
> - whoami = *argv;
> - if (strcmp(whoami, "quotaoff") == 0)
> + if (strcmp(__progname, "quotaoff") == 0)
> offmode = 1;
> - else if (strcmp(whoami, "quotaon") != 0) {
> + else if (strcmp(__progname, "quotaon") != 0) {
> fprintf(stderr, "Name must be quotaon or quotaoff not %s\n",
> - whoami);
> + __progname);
> exit(1);
> }
> - while ((ch = getopt(argc, argv, "avug")) != -1) {
> + while ((ch = getopt(argc, argv, "aguv")) != -1) {
> switch (ch) {
> case 'a':
> aflag = 1;
> @@ -94,13 +93,13 @@ main(int argc, char *argv[])
> vflag = 1;
> break;
> default:
> - usage(whoami);
> + usage();
> }
> }
> argc -= optind;
> argv += optind;
> if (argc <= 0 && !aflag)
> - usage(whoami);
> + usage();
> if (!gflag && !uflag) {
> gflag = 1;
> uflag = 1;
> @@ -142,22 +141,20 @@ main(int argc, char *argv[])
> }
>
> void
> -usage(char *whoami)
> +usage()
> {
> -
> - fprintf(stderr, "usage: %s [-aguv] filesystem ...\n", whoami);
> + fprintf(stderr, "usage: %s [-aguv] filesystem ...\n", __progname);
> exit(1);
> }
>
> int
> -quotaonoff(struct fstab *fs, int offmode, int type, char *qfpathname)
> +quotaonoff(const struct fstab *fs, int offmode, int type, char *qfpathname)
> {
> if (strcmp(fs->fs_file, "/") && readonly(fs))
> return (1);
> if (offmode) {
> if (quotactl(fs->fs_file, QCMD(Q_QUOTAOFF, type), 0, 0) < 0) {
> - fprintf(stderr, "quotaoff: ");
> - perror(fs->fs_file);
> + warn("%s", fs->fs_file);
> return (1);
> }
> if (vflag)
> @@ -180,7
Re: [PATCH] [src] share/man/man5/hostname.if.5 - nwid -> join
On Thu, Nov 11, 2021 at 04:11:10PM +, Raf Czlonka wrote: > Hello, > > It seems like this has been missed in recent thread[0]. > > Not entirely sure whether the sentence "flows" any longer but here > it goes anyway. > > [0] https://marc.info/?l=openbsd-tech=163507448118443=2 Thanks, I missed that! > Index: share/man/man5/hostname.if.5 > === > RCS file: /cvs/src/share/man/man5/hostname.if.5,v > retrieving revision 1.77 > diff -u -p -r1.77 hostname.if.5 > --- share/man/man5/hostname.if.5 17 Jul 2021 15:28:31 - 1.77 > +++ share/man/man5/hostname.if.5 11 Nov 2021 16:09:33 - > @@ -67,13 +67,13 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d > Each line is processed separately and in order. > For example: > .Bd -literal -offset indent > -nwid mynwid wpakey mywpakey > +join mynwid wpakey mywpakey > inet6 autoconf > inet autoconf > .Ed > .Pp > would run ifconfig three times to set the > -.Cm nwid > +.Cm join > and > .Cm wpakey > of the interface, Maybe this reads better in general? would run ifconfig three times to join a wireless network using WPA and to set the AUTOCONF6 and AUTOCONF4 flags, respectively. No need to explain lines command by command. Keep it simple; users must read ifconfig(8) anyway. One might as well say to ... and to enable automatic address configuration for IPv6 and IPv4, respectively. Then we don't have the weird mix of conceptual words and technical bits, i.e. "connect to wifi" vs. "set if flags [they're picked up by network daemons, but we don't say which ones]". DYNAMIC ADDRESS CONFIGURATION down below repeats all this, anyway. Feedback? OK? Index: hostname.if.5 === RCS file: /cvs/src/share/man/man5/hostname.if.5,v retrieving revision 1.77 diff -u -p -r1.77 hostname.if.5 --- hostname.if.5 17 Jul 2021 15:28:31 - 1.77 +++ hostname.if.5 11 Nov 2021 16:30:00 - @@ -67,21 +67,15 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d Each line is processed separately and in order. For example: .Bd -literal -offset indent -nwid mynwid wpakey mywpakey +join mynwid wpakey mywpakey inet6 autoconf inet autoconf .Ed .Pp -would run ifconfig three times to set the -.Cm nwid -and -.Cm wpakey -of the interface, -the -.Sy AUTOCONF6 -flag and the -.Sy AUTOCONF4 -flag, respectively. +would run ifconfig three times to +.Cm join +a wireless network using WPA +and enable dynamic address configuration, respectively. .Sh STATIC ADDRESS CONFIGURATION The following packed formats are valid for configuring network interfaces with static addresses.
Let filt_fileattach() run without kernel lock
This patch moves kernel locking a step deeper when attaching file event
filters, letting filt_fileattach() run without the kernel lock.
filt_fileattach() calls fp->f_ops->fo_kqfilter(). The f_ops field is
immutable, and the pointed fileops structs are constant.
There are five instances of fo_kqfilter: dmabuf_kqfilter,
kqueue_kqfilter, pipe_kqfilter, soo_kqfilter, and vn_kqfilter. Of these,
dmabuf_kqfilter has no side effects, and vn_kqfilter needs the kernel
lock to call the vnode operation. The rest should be MP-safe.
OK?
Index: kern/kern_event.c
===
RCS file: src/sys/kern/kern_event.c,v
retrieving revision 1.170
diff -u -p -r1.170 kern_event.c
--- kern/kern_event.c 6 Nov 2021 05:48:47 - 1.170
+++ kern/kern_event.c 11 Nov 2021 16:00:55 -
@@ -159,7 +160,7 @@ const struct filterops proc_filtops = {
};
const struct filterops file_filtops = {
- .f_flags= FILTEROP_ISFD,
+ .f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE,
.f_attach = filt_fileattach,
.f_detach = NULL,
.f_event= NULL,
Index: kern/vfs_vnops.c
===
RCS file: src/sys/kern/vfs_vnops.c,v
retrieving revision 1.118
diff -u -p -r1.118 vfs_vnops.c
--- kern/vfs_vnops.c25 Oct 2021 10:24:54 - 1.118
+++ kern/vfs_vnops.c11 Nov 2021 16:00:55 -
@@ -629,7 +629,12 @@ vn_closefile(struct file *fp, struct pro
int
vn_kqfilter(struct file *fp, struct knote *kn)
{
- return (VOP_KQFILTER(fp->f_data, fp->f_flag, kn));
+ int error;
+
+ KERNEL_LOCK();
+ error = VOP_KQFILTER(fp->f_data, fp->f_flag, kn);
+ KERNEL_UNLOCK();
+ return (error);
}
int
[PATCH] [src] share/man/man5/hostname.if.5 - nwid -> join
Hello, It seems like this has been missed in recent thread[0]. Not entirely sure whether the sentence "flows" any longer but here it goes anyway. [0] https://marc.info/?l=openbsd-tech=163507448118443=2 Cheers, Raf Index: share/man/man5/hostname.if.5 === RCS file: /cvs/src/share/man/man5/hostname.if.5,v retrieving revision 1.77 diff -u -p -r1.77 hostname.if.5 --- share/man/man5/hostname.if.517 Jul 2021 15:28:31 - 1.77 +++ share/man/man5/hostname.if.511 Nov 2021 16:09:33 - @@ -67,13 +67,13 @@ inet 10.0.0.1 255.255.255.0 10.0.0.255 d Each line is processed separately and in order. For example: .Bd -literal -offset indent -nwid mynwid wpakey mywpakey +join mynwid wpakey mywpakey inet6 autoconf inet autoconf .Ed .Pp would run ifconfig three times to set the -.Cm nwid +.Cm join and .Cm wpakey of the interface,
Re: uhidev: claim multiple report ids [3/N]
On Thu, Nov 11, 2021 at 03:29:15PM +0100, Anton Lindqvist wrote:
> Hi,
> The second attempt to solve the uhidev claim multiple report ids
> conflict didn't work out either as it broke fido(4). Signalling claim
> multiple report ids to the match routines using the report id does not
> work as all 256 values already have semantic meaning. I instead want to
> use `uha->claimed != NULL' to signal that multiple report ids can be
> claimed. Before doing so, refactor in order to make an upcoming diff
> with the actual fix significantly smaller.
>
> No intended^W functional change.
>
> Comments? OK?
... and here's the actual fix applied on top of the previous diff.
diff --git sys/dev/usb/uhidev.c sys/dev/usb/uhidev.c
index 60d1874322a..eb9bc927670 100644
--- sys/dev/usb/uhidev.c
+++ sys/dev/usb/uhidev.c
@@ -248,7 +248,7 @@ uhidev_attach(struct device *parent, struct device *self,
void *aux)
uha.uaa = uaa;
uha.parent = sc;
- uha.reportid = __UHIDEV_CLAIM_MULTIPLE_REPORTID;
+ uha.reportid = 0;
uha.nreports = nrepid;
uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
@@ -360,7 +360,7 @@ uhidevprint(void *aux, const char *pnp)
if (pnp)
printf("uhid at %s", pnp);
- if (uha->reportid != 0 && uha->reportid !=
__UHIDEV_CLAIM_MULTIPLE_REPORTID)
+ if (uha->reportid != 0)
printf(" reportid %d", uha->reportid);
return (UNCONF);
}
diff --git sys/dev/usb/uhidev.h sys/dev/usb/uhidev.h
index 86217fb8880..fda59b2a139 100644
--- sys/dev/usb/uhidev.h
+++ sys/dev/usb/uhidev.h
@@ -75,13 +75,11 @@ struct uhidev_attach_arg {
struct usb_attach_arg *uaa;
struct uhidev_softc *parent;
uint8_t reportid;
- uint8_t nreports;
+ u_intnreports;
uint8_t *claimed;
};
-#define UHIDEV_CLAIM_MULTIPLE_REPORTID(u) \
- ((u)->reportid == __UHIDEV_CLAIM_MULTIPLE_REPORTID)
-#define__UHIDEV_CLAIM_MULTIPLE_REPORTID255 /* XXX */
+#define UHIDEV_CLAIM_MULTIPLE_REPORTID(u) ((u)->claimed != NULL)
int uhidev_report_type_conv(int);
void uhidev_get_report_desc(struct uhidev_softc *, void **, int *);
Re: UNIX sockets: use vnode(9) lock to protect `v_socket' dereference
On Thu, Nov 11, 2021 at 03:21:45PM +0100, Alexander Bluhm wrote:
> On Thu, Nov 11, 2021 at 12:09:41PM +0300, Vitaliy Makkoveev wrote:
> > Can I propose to commit this diff before? It reorders soclose() to
> > destroy PCB before `so_q0' and `so_q' cleanup.
>
> OK bluhm@
>
> > Also the tool to test the diff:
>
> What happens when you run the tool with the current code? Does the
> kernel crash? Are there some inconsistencies that userland could
> detect?
>
> If yes, we could convert it to a regress test.
>
All fine with the current code. I wrote this to check that nothing
changes with the diff.
> bluhm
>
> > #include
> > #include
> > #include
> > #include
> > #include
> > #include
> > #include
> > #include
> > #include
> >
> > #include
> > #include
> >
> > #define NTHR (10)
> > #define NCONNECT (10)
> > #define NACCEPT ((NTHR * NCONNECT) / 2)
> >
> > static void *
> > thr_conn(void *arg)
> > {
> > struct sockaddr_storage *ss = arg;
> > int s[NCONNECT];
> > size_t hd = 0, tl = 0;
> >
> > while (1) {
> > if ((s[hd] = socket(ss->ss_family, SOCK_STREAM, 0)) < 0)
> > continue;
> >
> > if (connect(s[hd], (struct sockaddr *)ss, ss->ss_len) < 0) {
> > close(s[hd]);
> > continue;
> > }
> >
> > if ((hd = (hd + 1) % NCONNECT) == tl) {
> > close(s[tl]);
> > tl = (tl + 1) % NCONNECT;
> > }
> > }
> >
> > return NULL;
> > }
> >
> > static void
> > usage(void)
> > {
> > extern char *__progname;
> >
> > fprintf(stderr, "Usage %s: unix | inet | inet6\n", __progname);
> > exit(1);
> > }
> >
> > int
> > main(int argc, char *argv[])
> > {
> > struct sockaddr_storage ss;
> > size_t i;
> > int s;
> >
> > if (argc != 2) {
> > usage();
> > }
> >
> > memset(, 0, sizeof(ss));
> >
> > if (strcmp(argv[1], "unix") == 0) {
> > struct sockaddr_un *sun;
> >
> > sun = (struct sockaddr_un *)
> > sun->sun_len = sizeof(*sun);
> > sun->sun_family = AF_UNIX;
> > snprintf(sun->sun_path, sizeof(sun->sun_path) - 1,
> > "/tmp/socket%d", getpid());
> > } else if (strcmp(argv[1], "inet") == 0) {
> > struct sockaddr_in *sin;
> >
> > sin = (struct sockaddr_in *)
> > sin->sin_len = sizeof(*sin);
> > sin->sin_family = AF_INET;
> > sin->sin_port = htons(1+(getpid()%1));
> > if (inet_pton(AF_INET, "127.0.0.1", >sin_addr) <= 0)
> > errx(1, "inet_pton()");
> > } else if (strcmp(argv[1], "inet6") == 0) {
> > struct sockaddr_in6 *sin6;
> >
> > sin6 = (struct sockaddr_in6 *)
> > sin6->sin6_len = sizeof(*sin6);
> > sin6->sin6_family = AF_INET6;
> > sin6->sin6_port = htons(1+(getpid()%1));
> > if (inet_pton(AF_INET6, "::1", >sin6_addr) <= 0)
> > errx(1, "inet_pton()");
> > } else
> > usage();
> >
> > for (i = 0; i < NTHR; ++i) {
> > pthread_t thr;
> > int error;
> >
> > if ((error = pthread_create(, NULL, thr_conn, )))
> > errc(1, error, "pthread_create()");
> > }
> >
> > while(1) {
> > int conns[NACCEPT];
> >
> > if ((s = socket(ss.ss_family, SOCK_STREAM, 0)) < 0)
> > err(1, "socket()");
> >
> > if (ss.ss_family == AF_UNIX) {
> > struct sockaddr_un *sun = (struct sockaddr_un *)
> > unlink(sun->sun_path);
> > } else {
> > int opt = 1;
> >
> > if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR,
> > , sizeof(opt)) < 0)
> > err(1, "setsockopt()");
> > }
> >
> > if (bind(s, (struct sockaddr *), ss.ss_len) < 0)
> > err(1, "bind()");
> >
> > if (listen(s, 10) < 0)
> > err(1, "listen()");
> >
> > for(i=0; i < NACCEPT; ++i) {
> > if ((conns[i] = accept(s, NULL, NULL)) < 0)
> > err(1, "accept()");
> > }
> >
> > close(s);
> >
> > for(i=0; i < NACCEPT; ++i) {
> > close(conns[i]);
> > }
> > }
> >
> > return 0;
> > }
> >
> > Index: sys/kern/uipc_socket.c
> > ===
> > RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> > retrieving revision 1.267
> > diff -u -p -r1.267 uipc_socket.c
> > --- sys/kern/uipc_socket.c 24 Oct 2021 07:02:47 - 1.267
> > +++ sys/kern/uipc_socket.c 11 Nov 2021 08:56:17 -
> > @@ -322,19 +322,9 @@ soclose(struct socket *so, int flags)
> > s
uhidev: claim multiple report ids [3/N]
Hi,
The second attempt to solve the uhidev claim multiple report ids
conflict didn't work out either as it broke fido(4). Signalling claim
multiple report ids to the match routines using the report id does not
work as all 256 values already have semantic meaning. I instead want to
use `uha->claimed != NULL' to signal that multiple report ids can be
claimed. Before doing so, refactor in order to make an upcoming diff
with the actual fix significantly smaller.
No intended^W functional change.
Comments? OK?
diff --git sys/dev/usb/fido.c sys/dev/usb/fido.c
index c6d846aaa84..0232694b55a 100644
--- sys/dev/usb/fido.c
+++ sys/dev/usb/fido.c
@@ -63,7 +63,7 @@ fido_match(struct device *parent, void *match, void *aux)
void *desc;
int ret = UMATCH_NONE;
- if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
+ if (UHIDEV_CLAIM_MULTIPLE_REPORTID(uha))
return (ret);
/* Find the FIDO usage page and U2F collection */
diff --git sys/dev/usb/ucycom.c sys/dev/usb/ucycom.c
index ca8636f0a7f..621f6af2d5d 100644
--- sys/dev/usb/ucycom.c
+++ sys/dev/usb/ucycom.c
@@ -165,7 +165,7 @@ ucycom_match(struct device *parent, void *match, void *aux)
{
struct uhidev_attach_arg *uha = aux;
- if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
+ if (UHIDEV_CLAIM_MULTIPLE_REPORTID(uha))
return (UMATCH_NONE);
return (usb_lookup(ucycom_devs, uha->uaa->vendor, uha->uaa->product) !=
NULL ?
diff --git sys/dev/usb/ugold.c sys/dev/usb/ugold.c
index b83d8edd8e8..aafaa1712cc 100644
--- sys/dev/usb/ugold.c
+++ sys/dev/usb/ugold.c
@@ -113,7 +113,7 @@ ugold_match(struct device *parent, void *match, void *aux)
int size;
void *desc;
- if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
+ if (UHIDEV_CLAIM_MULTIPLE_REPORTID(uha))
return (UMATCH_NONE);
if (usb_lookup(ugold_devs, uha->uaa->vendor, uha->uaa->product) == NULL)
diff --git sys/dev/usb/uhid.c sys/dev/usb/uhid.c
index 90a75d878e3..c2f97dd833a 100644
--- sys/dev/usb/uhid.c
+++ sys/dev/usb/uhid.c
@@ -115,7 +115,7 @@ uhid_match(struct device *parent, void *match, void *aux)
{
struct uhidev_attach_arg *uha = aux;
- if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
+ if (UHIDEV_CLAIM_MULTIPLE_REPORTID(uha))
return (UMATCH_NONE);
return (UMATCH_IFACECLASS_GENERIC);
diff --git sys/dev/usb/uhidev.c sys/dev/usb/uhidev.c
index 3ce1684893b..60d1874322a 100644
--- sys/dev/usb/uhidev.c
+++ sys/dev/usb/uhidev.c
@@ -248,7 +248,7 @@ uhidev_attach(struct device *parent, struct device *self,
void *aux)
uha.uaa = uaa;
uha.parent = sc;
- uha.reportid = UHIDEV_CLAIM_MULTIPLE_REPORTID;
+ uha.reportid = __UHIDEV_CLAIM_MULTIPLE_REPORTID;
uha.nreports = nrepid;
uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
@@ -360,7 +360,7 @@ uhidevprint(void *aux, const char *pnp)
if (pnp)
printf("uhid at %s", pnp);
- if (uha->reportid != 0 && uha->reportid !=
UHIDEV_CLAIM_MULTIPLE_REPORTID)
+ if (uha->reportid != 0 && uha->reportid !=
__UHIDEV_CLAIM_MULTIPLE_REPORTID)
printf(" reportid %d", uha->reportid);
return (UNCONF);
}
diff --git sys/dev/usb/uhidev.h sys/dev/usb/uhidev.h
index b2b7608b083..86217fb8880 100644
--- sys/dev/usb/uhidev.h
+++ sys/dev/usb/uhidev.h
@@ -75,11 +75,14 @@ struct uhidev_attach_arg {
struct usb_attach_arg *uaa;
struct uhidev_softc *parent;
uint8_t reportid;
-#defineUHIDEV_CLAIM_MULTIPLE_REPORTID 255
uint8_t nreports;
uint8_t *claimed;
};
+#define UHIDEV_CLAIM_MULTIPLE_REPORTID(u) \
+ ((u)->reportid == __UHIDEV_CLAIM_MULTIPLE_REPORTID)
+#define__UHIDEV_CLAIM_MULTIPLE_REPORTID255 /* XXX */
+
int uhidev_report_type_conv(int);
void uhidev_get_report_desc(struct uhidev_softc *, void **, int *);
int uhidev_open(struct uhidev *);
diff --git sys/dev/usb/ujoy.c sys/dev/usb/ujoy.c
index ca461e5969a..11d77f9daba 100644
--- sys/dev/usb/ujoy.c
+++ sys/dev/usb/ujoy.c
@@ -104,7 +104,7 @@ ujoy_match(struct device *parent, void *match, void *aux)
void *desc;
int ret = UMATCH_NONE;
- if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
+ if (UHIDEV_CLAIM_MULTIPLE_REPORTID(uha))
return (ret);
/* Find the general usage page and gamecontroller collections */
diff --git sys/dev/usb/umt.c sys/dev/usb/umt.c
index 3aa45bf298a..0915b38de5c 100644
--- sys/dev/usb/umt.c
+++ sys/dev/usb/umt.c
@@ -90,7 +90,7 @@ umt_match(struct device *parent, void *match, void *aux)
int size;
void *desc;
- if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID) {
+ if (UHIDEV_CLAIM_MULTIPLE_REPORTID(uha)) {
Re: UNIX sockets: use vnode(9) lock to protect `v_socket' dereference
On Thu, Nov 11, 2021 at 12:42:36AM +0300, Vitaliy Makkoveev wrote:
> What about 'SO_DYING' flag? Anyway I want to remove it with the next
> diff.
The so_options are for user flags. The so_state is for kernel
transitions. So a SS_DYING would be better, see sys/socketvar.h.
But as we commit the other soclose() diff first, it does not matter
anymore.
> No problem, but I made "if (vp)" without NULL for the consistency with
> "if (unp->unp_vnode)" in this function. Should they have the same
> notation?
mpi@ and I converted the network stack gradually to the pointer ==
NULL or pointer != NULL idiom. If you change something, go into
this direction. But there is no need to make bulk conversions.
> OK for updated diff?
OK bluhm@ without the SO_DYING part.
> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.268
> diff -u -p -r1.268 uipc_socket.c
> --- sys/kern/uipc_socket.c6 Nov 2021 05:26:33 - 1.268
> +++ sys/kern/uipc_socket.c10 Nov 2021 21:33:01 -
> @@ -334,6 +334,8 @@ soclose(struct socket *so, int flags)
> /* Revoke async IO early. There is a final revocation in sofree(). */
> sigio_free(>so_sigio);
> if (so->so_options & SO_ACCEPTCONN) {
> + so->so_options |= SO_DYING;
> +
> while ((so2 = TAILQ_FIRST(>so_q0)) != NULL) {
> (void) soqremque(so2, 0);
> (void) soabort(so2);
> Index: sys/kern/uipc_usrreq.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> retrieving revision 1.154
> diff -u -p -r1.154 uipc_usrreq.c
> --- sys/kern/uipc_usrreq.c6 Nov 2021 17:35:14 - 1.154
> +++ sys/kern/uipc_usrreq.c10 Nov 2021 21:33:01 -
> @@ -485,20 +485,30 @@ void
> unp_detach(struct unpcb *unp)
> {
> struct socket *so = unp->unp_socket;
> - struct vnode *vp = NULL;
> + struct vnode *vp = unp->unp_vnode;
>
> rw_assert_wrlock(_lock);
>
> LIST_REMOVE(unp, unp_link);
> - if (unp->unp_vnode) {
> +
> + if (vp != NULL) {
> + unp->unp_vnode = NULL;
> +
> /*
> - * `v_socket' is only read in unp_connect and
> - * unplock prevents concurrent access.
> + * Enforce `i_lock' -> `unp_lock' because fifo
> + * subsystem requires it.
>*/
>
> - unp->unp_vnode->v_socket = NULL;
> - vp = unp->unp_vnode;
> - unp->unp_vnode = NULL;
> + sounlock(so, SL_LOCKED);
> +
> + VOP_LOCK(vp, LK_EXCLUSIVE);
> + vp->v_socket = NULL;
> +
> + KERNEL_LOCK();
> + vput(vp);
> + KERNEL_UNLOCK();
> +
> + solock(so);
> }
>
> if (unp->unp_conn)
> @@ -511,21 +521,6 @@ unp_detach(struct unpcb *unp)
> pool_put(_pool, unp);
> if (unp_rights)
> task_add(systqmp, _gc_task);
> -
> - if (vp != NULL) {
> - /*
> - * Enforce `i_lock' -> `unplock' because fifo subsystem
> - * requires it. The socket can't be closed concurrently
> - * because the file descriptor reference is
> - * still hold.
> - */
> -
> - sounlock(so, SL_LOCKED);
> - KERNEL_LOCK();
> - vrele(vp);
> - KERNEL_UNLOCK();
> - solock(so);
> - }
> }
>
> int
> @@ -670,7 +665,8 @@ unp_connect(struct socket *so, struct mb
> goto put_locked;
> }
> if (so->so_proto->pr_flags & PR_CONNREQUIRED) {
> - if ((so2->so_options & SO_ACCEPTCONN) == 0 ||
> + if ((so2->so_options & SO_DYING) != 0 ||
> + (so2->so_options & SO_ACCEPTCONN) == 0 ||
> (so3 = sonewconn(so2, 0)) == NULL) {
> error = ECONNREFUSED;
> goto put_locked;
> Index: sys/sys/socket.h
> ===
> RCS file: /cvs/src/sys/sys/socket.h,v
> retrieving revision 1.101
> diff -u -p -r1.101 socket.h
> --- sys/sys/socket.h 7 Nov 2021 12:05:28 - 1.101
> +++ sys/sys/socket.h 10 Nov 2021 21:33:01 -
> @@ -97,6 +97,7 @@ typedef __sa_family_t sa_family_t;/* so
> #define SO_TIMESTAMP 0x0800 /* timestamp received dgram traffic */
> #define SO_BINDANY 0x1000 /* allow bind to any address */
> #define SO_ZEROIZE 0x2000 /* zero out all mbufs sent over socket
> */
> +#define SO_DYING 0x4000 /* socket is dying */
>
> /*
> * Additional options, not kept in so_options.
Re: UNIX sockets: use vnode(9) lock to protect `v_socket' dereference
On Thu, Nov 11, 2021 at 12:09:41PM +0300, Vitaliy Makkoveev wrote:
> Can I propose to commit this diff before? It reorders soclose() to
> destroy PCB before `so_q0' and `so_q' cleanup.
OK bluhm@
> Also the tool to test the diff:
What happens when you run the tool with the current code? Does the
kernel crash? Are there some inconsistencies that userland could
detect?
If yes, we could convert it to a regress test.
bluhm
> #include
> #include
> #include
> #include
> #include
> #include
> #include
> #include
> #include
>
> #include
> #include
>
> #define NTHR (10)
> #define NCONNECT (10)
> #define NACCEPT ((NTHR * NCONNECT) / 2)
>
> static void *
> thr_conn(void *arg)
> {
> struct sockaddr_storage *ss = arg;
> int s[NCONNECT];
> size_t hd = 0, tl = 0;
>
> while (1) {
> if ((s[hd] = socket(ss->ss_family, SOCK_STREAM, 0)) < 0)
> continue;
>
> if (connect(s[hd], (struct sockaddr *)ss, ss->ss_len) < 0) {
> close(s[hd]);
> continue;
> }
>
> if ((hd = (hd + 1) % NCONNECT) == tl) {
> close(s[tl]);
> tl = (tl + 1) % NCONNECT;
> }
> }
>
> return NULL;
> }
>
> static void
> usage(void)
> {
> extern char *__progname;
>
> fprintf(stderr, "Usage %s: unix | inet | inet6\n", __progname);
> exit(1);
> }
>
> int
> main(int argc, char *argv[])
> {
> struct sockaddr_storage ss;
> size_t i;
> int s;
>
> if (argc != 2) {
> usage();
> }
>
> memset(, 0, sizeof(ss));
>
> if (strcmp(argv[1], "unix") == 0) {
> struct sockaddr_un *sun;
>
> sun = (struct sockaddr_un *)
> sun->sun_len = sizeof(*sun);
> sun->sun_family = AF_UNIX;
> snprintf(sun->sun_path, sizeof(sun->sun_path) - 1,
> "/tmp/socket%d", getpid());
> } else if (strcmp(argv[1], "inet") == 0) {
> struct sockaddr_in *sin;
>
> sin = (struct sockaddr_in *)
> sin->sin_len = sizeof(*sin);
> sin->sin_family = AF_INET;
> sin->sin_port = htons(1+(getpid()%1));
> if (inet_pton(AF_INET, "127.0.0.1", >sin_addr) <= 0)
> errx(1, "inet_pton()");
> } else if (strcmp(argv[1], "inet6") == 0) {
> struct sockaddr_in6 *sin6;
>
> sin6 = (struct sockaddr_in6 *)
> sin6->sin6_len = sizeof(*sin6);
> sin6->sin6_family = AF_INET6;
> sin6->sin6_port = htons(1+(getpid()%1));
> if (inet_pton(AF_INET6, "::1", >sin6_addr) <= 0)
> errx(1, "inet_pton()");
> } else
> usage();
>
> for (i = 0; i < NTHR; ++i) {
> pthread_t thr;
> int error;
>
> if ((error = pthread_create(, NULL, thr_conn, )))
> errc(1, error, "pthread_create()");
> }
>
> while(1) {
> int conns[NACCEPT];
>
> if ((s = socket(ss.ss_family, SOCK_STREAM, 0)) < 0)
> err(1, "socket()");
>
> if (ss.ss_family == AF_UNIX) {
> struct sockaddr_un *sun = (struct sockaddr_un *)
> unlink(sun->sun_path);
> } else {
> int opt = 1;
>
> if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR,
> , sizeof(opt)) < 0)
> err(1, "setsockopt()");
> }
>
> if (bind(s, (struct sockaddr *), ss.ss_len) < 0)
> err(1, "bind()");
>
> if (listen(s, 10) < 0)
> err(1, "listen()");
>
> for(i=0; i < NACCEPT; ++i) {
> if ((conns[i] = accept(s, NULL, NULL)) < 0)
> err(1, "accept()");
> }
>
> close(s);
>
> for(i=0; i < NACCEPT; ++i) {
> close(conns[i]);
> }
> }
>
> return 0;
> }
>
> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.267
> diff -u -p -r1.267 uipc_socket.c
> --- sys/kern/uipc_socket.c24 Oct 2021 07:02:47 - 1.267
> +++ sys/kern/uipc_socket.c11 Nov 2021 08:56:17 -
> @@ -322,19 +322,9 @@ soclose(struct socket *so, int flags)
> s = solock(so);
> /* Revoke async IO early. There is a final revocation in sofree(). */
> sigio_free(>so_sigio);
> - if (so->so_options & SO_ACCEPTCONN) {
> - while ((so2 = TAILQ_FIRST(>so_q0)) != NULL) {
> - (void) soqremque(so2, 0);
> - (void)
Re: Teach manpages of resolv(8) and unwindctl(8) about sppp(4)
On Thu, Nov 11, 2021 at 07:01:42AM +0100, Bjorn Ketelaars wrote: > On Wed 10/11/2021 21:20, Klemens Nanni wrote: > > I think only unwind(8) should list all the inputs and unwindctl(8) > > should just say "Show learned nameservers". > > > > unwind(8) is already incomplete regardless of sppp(4) and unwindctl(8) > > is a poor duplicate of it. > > I agree with unwindctl(8). For unwind(8) i would like to take it one > step further and use resolvd(8) as single source of truth. What do you > think of the diff below? I'm not convinced: users shouldn't read manuals of tools they don't use in order to learn how the tools they do use are working. > diff --git sbin/unwind/unwind.8 sbin/unwind/unwind.8 > index afa7d0ad2c1..38f7d1aaea8 100644 > --- sbin/unwind/unwind.8 > +++ sbin/unwind/unwind.8 > @@ -33,11 +33,8 @@ It is intended to run on client machines like workstations > or laptops and only > listens on localhost. > .Nm > sends DNS queries to nameservers to answer queries and switches to resolvers > -learned from > -.Xr dhclient 8 , > -.Xr dhcpleased 8 > -or > -.Xr slaacd 8 > +learned by > +.Xr resolvd 8 > if it detects that DNS queries are blocked by the local network. > It periodically probes if DNS is no longer blocked and switches back to > querying nameservers itself. This is wrong. Stop resolvd and start unwind, you'll see that `unwindctl status autoconf' shows learned nameservers. > @@ -104,9 +101,7 @@ socket used for communication with > .El > .Sh SEE ALSO > .Xr unwind.conf 5 , > -.Xr dhclient 8 , > -.Xr dhcpleased 8 , > -.Xr slaacd 8 , > +.Xr resolvd 8 , > .Xr unbound 8 , > .Xr unwindctl 8 > .Sh STANDARDS > diff --git usr.sbin/unwindctl/unwindctl.8 usr.sbin/unwindctl/unwindctl.8 > index 1b1be4451e7..b8289a82bd9 100644 > --- usr.sbin/unwindctl/unwindctl.8 > +++ usr.sbin/unwindctl/unwindctl.8 > @@ -56,11 +56,7 @@ Reload the configuration file. > .It Cm status > Show a status summary. > .It Cm status autoconf > -Show nameservers learned from > -.Xr dhclient 8 , > -.Xr dhcpleased 8 > -or > -.Xr slaacd 8 . > +Show learned nameservers. > .It Cm status memory > Show memory consumption. > .El That's what I have in my WIP diff already.
Re: add 802.11n 40MHz support to iwn(4)
On Tue, Nov 09, 2021 at 02:23:09PM +0100, Stefan Sperling wrote:
> On Mon, Nov 01, 2021 at 12:56:26PM +0100, Stefan Sperling wrote:
> > I have tested on a 6205 device. More tests are needed, especially on
> > the old 4965AGN generation because those chips require the driver to
> > do specific calibration work which newer chips perform in firmware.
> > I suspect you will find 4965 devices in older thinkpads which first
> > introduced 11n wifi to these laptops (the x60/x61 generation probably).
>
> Does nobody have a 4965AGN device anymore?
>
> The patch is in snaps, so a simple upgrade to the latest snapshot
> would be sufficient to test this. Thanks!
Testing on 4965 by jsg@ revealed an unrelated issue on those devices.
A fix for this problem has just been committed.
This new version of the 40MHz patch applies on top of that fix.
diff refs/heads/iwn-rxon refs/heads/40mhz
blob - 41b2623db022d54e532ab5a1cd06f7a3adc9a69e
blob + 1d6668e5484e58465a874e4c51c943fe4c2ba5d3
--- sys/dev/pci/if_iwn.c
+++ sys/dev/pci/if_iwn.c
@@ -241,12 +241,16 @@ uint16_t iwn_get_passive_dwell_time(struct iwn_softc *
intiwn_scan(struct iwn_softc *, uint16_t, int);
void iwn_scan_abort(struct iwn_softc *);
intiwn_bgscan(struct ieee80211com *);
+void iwn_rxon_configure_ht40(struct ieee80211com *,
+ struct ieee80211_node *);
+intiwn_rxon_ht40_enabled(struct iwn_softc *);
intiwn_auth(struct iwn_softc *, int);
intiwn_run(struct iwn_softc *);
intiwn_set_key(struct ieee80211com *, struct ieee80211_node *,
struct ieee80211_key *);
void iwn_delete_key(struct ieee80211com *, struct ieee80211_node *,
struct ieee80211_key *);
+void iwn_updatechan(struct ieee80211com *);
void iwn_updateprot(struct ieee80211com *);
void iwn_updateslot(struct ieee80211com *);
void iwn_update_rxon_restore_power(struct iwn_softc *);
@@ -491,13 +495,15 @@ iwn_attach(struct device *parent, struct device *self,
ic->ic_caps |= (IEEE80211_C_QOS | IEEE80211_C_TX_AMPDU);
/* Set HT capabilities. */
ic->ic_htcaps = IEEE80211_HTCAP_SGI20;
+ /* 6200 devices have issues with SGI40 for some reason. */
+ if ((sc->sc_flags & IWN_FLAG_INTERNAL_PA) == 0)
+ ic->ic_htcaps |= IEEE80211_HTCAP_SGI40;
+ ic->ic_htcaps |= IEEE80211_HTCAP_CBW20_40;
#ifdef notyet
ic->ic_htcaps |=
#if IWN_RBUF_SIZE == 8192
IEEE80211_HTCAP_AMSDU7935 |
#endif
- IEEE80211_HTCAP_CBW20_40 |
- IEEE80211_HTCAP_SGI40;
if (sc->hw_type != IWN_HW_REV_TYPE_4965)
ic->ic_htcaps |= IEEE80211_HTCAP_GF;
if (sc->hw_type == IWN_HW_REV_TYPE_6050)
@@ -543,6 +549,7 @@ iwn_attach(struct device *parent, struct device *self,
ic->ic_updateedca = iwn_updateedca;
ic->ic_set_key = iwn_set_key;
ic->ic_delete_key = iwn_delete_key;
+ ic->ic_updatechan = iwn_updatechan;
ic->ic_updateprot = iwn_updateprot;
ic->ic_updateslot = iwn_updateslot;
ic->ic_ampdu_rx_start = iwn_ampdu_rx_start;
@@ -1477,8 +1484,8 @@ iwn4965_read_eeprom(struct iwn_softc *sc)
/* Read regulatory domain (4 ASCII characters). */
iwn_read_prom_data(sc, IWN4965_EEPROM_DOMAIN, sc->eeprom_domain, 4);
- /* Read the list of authorized channels (20MHz ones only). */
- for (i = 0; i < 5; i++) {
+ /* Read the list of authorized channels. */
+ for (i = 0; i < 7; i++) {
addr = iwn4965_regulatory_bands[i];
iwn_read_eeprom_channels(sc, i, addr);
}
@@ -1562,8 +1569,8 @@ iwn5000_read_eeprom(struct iwn_softc *sc)
iwn_read_prom_data(sc, base + IWN5000_EEPROM_DOMAIN,
sc->eeprom_domain, 4);
- /* Read the list of authorized channels (20MHz ones only). */
- for (i = 0; i < 5; i++) {
+ /* Read the list of authorized channels. */
+ for (i = 0; i < 7; i++) {
addr = base + iwn5000_regulatory_bands[i];
iwn_read_eeprom_channels(sc, i, addr);
}
@@ -1633,7 +1640,7 @@ iwn_read_eeprom_channels(struct iwn_softc *sc, int n,
IEEE80211_CHAN_CCK | IEEE80211_CHAN_OFDM |
IEEE80211_CHAN_DYN | IEEE80211_CHAN_2GHZ;
- } else {/* 5GHz band */
+ } else if (n < 5) { /* 5GHz band */
/*
* Some adapters support channels 7, 8, 11 and 12
* both in the 2GHz and 4.9GHz bands.
@@ -1648,22 +1655,29 @@ iwn_read_eeprom_channels(struct iwn_softc *sc, int n,
ic->ic_channels[chan].ic_flags = IEEE80211_CHAN_A;
/* We have at least one valid 5GHz
lib/Makefile: libfido2 requires libz
Hi, Recently I tried to crossbuild for arm64 on amd64. I found "TARGET=arm64 make cross-tools" stops when building libfido2. libfido2 requires libz but this is not built yet at that time. lib/Makefile needs to tweak like this. openbsd-current-vm# diff -uNpr Makefile~ Makefile --- Makefile~ Sun Jan 3 05:04:36 2021 +++ MakefileThu Nov 11 19:47:42 2021 @@ -2,10 +2,10 @@ # $NetBSD: Makefile,v 1.20.4.1 1996/06/14 17:22:38 cgd Exp $ SUBDIR=csu libagentx libarch libc libcbor libcrypto libcurses \ - libedit libelf libevent libexpat \ + libedit libelf libevent libexpat libz \ libfido2 libform libfuse libkeynote libkvm libl libm libmenu \ libossaudio libpanel libpcap libradius librthread \ librpcsvc libskey libsndio libssl libtls libusbhid \ - libutil liby libz + libutil liby .include openbsd-current-vm# It works good, but I think there is better build order. -- SASANO Takayoshi (JG1UAA)
Re: UNIX sockets: use vnode(9) lock to protect `v_socket' dereference
On Thu, Nov 11, 2021 at 12:42:36AM +0300, Vitaliy Makkoveev wrote:
> On Wed, Nov 10, 2021 at 10:03:48PM +0100, Alexander Bluhm wrote:
> > On Tue, Oct 26, 2021 at 02:12:36PM +0300, Vitaliy Makkoveev wrote:
> > > --- sys/kern/uipc_socket.c14 Oct 2021 23:05:10 - 1.265
> > > +++ sys/kern/uipc_socket.c26 Oct 2021 11:05:59 -
> > > @@ -315,6 +315,8 @@ soclose(struct socket *so, int flags)
> > > /* Revoke async IO early. There is a final revocation in sofree(). */
> > > sigio_free(>so_sigio);
> > > if (so->so_options & SO_ACCEPTCONN) {
> > > + so->so_options &= ~SO_ACCEPTCONN;
> > > +
> > > while ((so2 = TAILQ_FIRST(>so_q0)) != NULL) {
> > > (void) soqremque(so2, 0);
> > > (void) soabort(so2);
> >
> > I don't like this. A listening socket can never be converted to a
> > non-listening socket. Whatever this transition means in th TCP
> > layer. If you want to mark a listening socket as closing to avoid
> > accepting connections, use a approriate flag. Do not convert it
> > to an non-listening socket that may have races elsewhere.
> >
> > I would say some so_state bits may be suitable if we really need
> > this feature.
> >
>
> This is temporary solution. I want to reorder soclose() to destroy PCB
> before `so_q0' and `so_q' cleanup, so this 'SO_ACCEPTCONN' modification
> will be removed. This is required because the relock dances will
> appeared in socose() and unp_detach() for the per-socket locking.
>
> We don't release netlock in the inet sockets case, so they are not
> affected.
>
> What about 'SO_DYING' flag? Anyway I want to remove it with the next
> diff.
>
> > > - if (unp->unp_vnode) {
> > > +
> > > + if (vp) {
> >
> > Please use (vp != NULL) for consistency.
> >
>
> No problem, but I made "if (vp)" without NULL for the consistency with
> "if (unp->unp_vnode)" in this function. Should they have the same
> notation?
>
> > > + unp->unp_vnode = NULL;
> > > +
> > > /*
> > > - * `v_socket' is only read in unp_connect and
> > > - * unplock prevents concurrent access.
> > > + * Enforce `i_lock' -> `unp_lock' because fifo
> > > + * subsystem requires it.
> > >*/
> > >
> > > - unp->unp_vnode->v_socket = NULL;
> > > - vp = unp->unp_vnode;
> > > - unp->unp_vnode = NULL;
> > > + sounlock(so, SL_LOCKED);
> > > +
> > > + VOP_LOCK(vp, LK_EXCLUSIVE);
> > > + vp->v_socket = NULL;
> > > +
> > > + KERNEL_LOCK();
> > > + vput(vp);
> > > + KERNEL_UNLOCK();
> > > +
> > > + solock(so);
> > > }
> >
> > This might work, we should try it.
> >
> > > @@ -500,21 +510,6 @@ unp_detach(struct unpcb *unp)
> > > pool_put(_pool, unp);
> > > if (unp_rights)
> > > task_add(systqmp, _gc_task);
> > > -
> > > - if (vp != NULL) {
> > > - /*
> > > - * Enforce `i_lock' -> `unplock' because fifo subsystem
> > > - * requires it. The socket can't be closed concurrently
> > > - * because the file descriptor reference is
> > > - * still hold.
> > > - */
> > > -
> > > - sounlock(so, SL_LOCKED);
> > > - KERNEL_LOCK();
> > > - vrele(vp);
> > > - KERNEL_UNLOCK();
> > > - solock(so);
> > > - }
> > > }
> >
> > Why did you move the lock dance to the beginning of the function?
> > Does it matter?
> >
>
> Yes, I want to disconnect dying socket from the file system before start
> it's destruction. This does matter because in some cases we need to
> release the solock() before acquire solock() for two sockets to keep
> lock order between them. With listening socket disconnected from the
> file system we can't have concurrent threads performing connect to dying
> socket.
>
> Also this is the reason I want to reorder listening socket destruction
> in the soclose().
>
> OK for updated diff?
>
Can I propose to commit this diff before? It reorders soclose() to
destroy PCB before `so_q0' and `so_q' cleanup.
The dying socket is already unlinked from the file descriptor layer, but
still accessible from the stack or from the file system layer. When we
release solock() while processing `so_q0' or `so_q' queues or when
performing (*pr_detach)(), concurrent thread could perform connection.
This reorder prevents this. The reorder will be required for the
upcoming fine grained locking diffs.
According the current code we assume the `so_q0' or `so_q' queues
cleanup could be done with destroyed PCB so this diff doesn't modify
existing logic. With this diff committed I don't need to modify
`so_state' in the unp_detach() reorder diff which started this thread.
Also the tool to test the diff:
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#define NTHR (10)
#define NCONNECT (10)
#define NACCEPT ((NTHR * NCONNECT) / 2)
static void *
thr_conn(void *arg)
