Re: uhidpp(4): add support for Bolt receivers and Unified Battery feature
On Sat, Nov 26, 2022 at 1:47 AM Anton Lindqvist wrote:
> Hi,
> This diff adds supports for the following to uhidpp:
>
> 1. Bolt receivers, they use a different set of registers for querying
>paired devices.
>
> 2. The Unified Battery feature, this is a more competent feature
>function used to report battery status compared to the Battery
>feature which is already supported by uhidpp.
>
> Makes battery sensors appear for my Logitech Lift mouse and Paul de
> Weerd's MX Anywhere 3 mouse.
>
> If you have a Logitech HID++ device currently lacking sensors under
> sysctl hw.sensors, please give this diff a try. Send me your dmesg and
> output of sysctl hw.sensors after moving the mouse to ensure a connect
> interrupt has fired off. Note that the diff must be applied on top of
> revision 1.37 of sys/dev/usb/uhidpp.c.
>
> Comments? OK?
>
>
> @@ -193,17 +202,20 @@ struct uhidpp_device {
> uint8_t d_features;
> #define UHIDPP_DEVICE_FEATURE_ROOT 0x01
> #define UHIDPP_DEVICE_FEATURE_BATTERY 0x02
> +#define UHIDPP_DEVICE_FEATURE_UNIFIIED_BATTERY 0x04
>
>
>
Do you actually mean "unifiied" with an extra "i" here?
--david
Re: dhcpleased, resolvd, slaacd: accurate lock error message
Yes, that is understandable and suggests "no need to panic".
> On Sat, 26 Nov 2022 11:09:58 -0700, "Theo de Raadt" wrote:
>
> > But "Resource temporarily unavailable" has absolutely no meaning to
> > anyone who will try to do this by hand, it is better for the message
> > to interpret the situation so that someone can understand.
>
> Agreed, it would be friendlier to fo something like:
>
> if (lockfd == -1) {
> if (errno == EAGAIN)
> errx(1, "already running");
> err(1, "%s", _PATH_LOCKFILE);
> }
>
> - todd
Re: dhcpleased, resolvd, slaacd: accurate lock error message
On Sat, 26 Nov 2022 11:09:58 -0700, "Theo de Raadt" wrote:
> But "Resource temporarily unavailable" has absolutely no meaning to
> anyone who will try to do this by hand, it is better for the message
> to interpret the situation so that someone can understand.
Agreed, it would be friendlier to fo something like:
if (lockfd == -1) {
if (errno == EAGAIN)
errx(1, "already running");
err(1, "%s", _PATH_LOCKFILE);
}
- todd
Re: pfsync panic after pf_purge backout
Hello,
On Sat, Nov 26, 2022 at 08:33:28PM +0100, Hrvoje Popovski wrote:
> I just need to say that with all pf, pfsync and with pf_purge diffs
> after hackaton + this diff on tech@
> https://www.mail-archive.com/tech@openbsd.org/msg72582.html
> my production firewall seems stable and it wasn't without that diff
this diff still waits for OK. it makes pfsync to use
state mutex to safely dereference keys.
>
> I'm not sure if we have same diffs but even Josmar Pierri on bugs@
> https://www.mail-archive.com/bugs@openbsd.org/msg18994.html
> who had panics quite regularly with that diff on tech@ seems to have
> stable firewall now.
>
>
>
> r620-1# uvm_fault(0x82374288, 0x17, 0, 2) -> e
> kernel: page fault trap, code=0
> Stopped at pfsync_q_del+0x96: movq%rdx,0x8(%rax)
> TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
> *192892 19920 0 0x14000 0x2005K softnet
> pfsync_q_del(fd82e8a4ce20) at pfsync_q_del+0x96
> pf_remove_state(fd82e8a4ce20) at pf_remove_state+0x14b
> pfsync_in_del_c(fd8006d843b8,c,79,0) at pfsync_in_del_c+0x6f
> pfsync_input(800022d60ad8,800022d60ae4,f0,2) at pfsync_input+0x33c
> ip_deliver(800022d60ad8,800022d60ae4,f0,2) at ip_deliver+0x113
> ipintr() at ipintr+0x69
> if_netisr(0) at if_netisr+0xea
> taskq_thread(8003) at taskq_thread+0x100
> end trace frame: 0x0, count: 7
> https://www.openbsd.org/ddb.html describes the minimum info required in
> bug reports. Insufficient info makes it difficult to find and fix bugs.
> ddb{5}>
>
those panics are causing me headaches. this got most-likely uncovered
by diff which adds a mutex. The mutex makes pfsync stable enough
so you can trigger unknown bugs.
thanks and
regards
sashan
Re: lladdr support for netstart/hostname.if
On Sat, Nov 26, 2022 at 03:23:03PM -0800, Andrew Hewus Fresh wrote:
> On Sat, Nov 26, 2022 at 07:12:12PM +, Klemens Nanni wrote:
> > On Sat, Nov 26, 2022 at 10:44:05AM -0800, Andrew Hewus Fresh wrote:
> > > I think this is the complete diff, with the man page bits mostly from
> > > Martijn. Now that the ifconfig -M bits are in, this should be easier
> > > to test. I was able to test the installer bits, and they worked for me.
> >
> > This reads straight forward, but see inline.
> >
>
> > > + # If the _if looks like an lladdr and look it up.
> >
> > That sentence is not proper english.
> > "Look up the interface if _if seems like an lladdr."
> > or
> > "Obtain the interface if _if is a MAC"
> > ?
>
> I realized the comment wasn't particularly useful, so I deleted it.
>
>
> > > + if [[ $_if = ??:??:??:??:??:?? ]]; then
> >
> > Please use == for comparison; I know = works but it reads off.
> > Base should use == consistently, imho.
>
> Somehow I thought the right side being a glob match only worked with "="
> not "==", but turns out I was wrong. Fixed.
>
> >
> > > + if ! _line="$( ifconfig -M "$_if" )"; then
> >
> > Here you quote the subshell and the variable...
>
> That is a heck of a habit I have. Apparently I can't avoid typing
> those. Fixed.
>
> >
> > > + print -u2 "${0##*/}: unique if for lladdr $_if not
> > > found."
> >
> > Maybe just "$_if is not unique"?
>
> I like it! Thanks!
>
>
> > >
> > > + if [[ $_if = ??:??:??:??:??:?? ]]; then
> > > + _if=$(ifconfig -M $_if)
> >
> > ... here you quote neither subshell nor variable.
>
> I was trying to match the style like this, but not quoting things is
> hard for me to do.
>
> > I think that's fine, both install.sub and netstart already use unquoted
> > $_if and the subshell does not have to be quoted, anyway, since word
> > spliting does not occur:
> >
> > $ v=$(echo 1 2 3)
> > $ echo $v
> > 1 2 3
>
> It does split though, echo just re-assembles it back the way it started:
>
> $ v=$(printf "%s\n" 1 2 3 )
> $ # without the quotes , it gets tokenized again and the newlines disappear
> $ echo "$v"
> 1
> 2
> 3
Yes sorry, my example was flawed. You still don't have to quote the
subshell during the variable assignment, though, that's what I was
trying to clarify.
>
>
>
> > > + elif [[ $_if = ??:??:??:??:??:?? ]]; then
> >
> > ==, please.
>
> Can do. Too used to writing portable things for /bin/sh I guess.
>
>
> > > +Additionally the interface name can be replaced with the interface's
> > > lladdr,
> > > +given that the lladdr is unique across the system and no
> > > +.Nm
> > > +exists for the interface name.
> >
> > Isn't that too vague?
> >
> > .Nm is hostname.if but you meant to say that .fxp0 should not exist when
> > .MAC
> > already exists, no?
>
> I'm not sure, it makes sense to me. Hopefully jmc provides an excellent
> option, but I'll think on wording.
>
> Here's another revision, not really any changes to the man page, but
> your suggestions got applied, I think.
Looking great so far, thanks.
One nit inline, no need to resend the diff.
>
>
> Index: etc/netstart
> ===
> RCS file: /cvs/src/etc/netstart,v
> retrieving revision 1.229
> diff -u -p -r1.229 netstart
> --- etc/netstart 5 Nov 2022 12:06:05 - 1.229
> +++ etc/netstart 26 Nov 2022 21:02:08 -
> @@ -135,6 +135,22 @@ ifstart() {
> local _if=$1 _hn=/etc/hostname.$1 _cmds _i=0 _line _stat
> set -A _cmds
>
> + if [[ $_if == ??:??:??:??:??:?? ]]; then
> + if ! _line=$( ifconfig -M $_if ); then
> + print -u2 "${0##*/}: $_if is not unique."
> + return
> + fi
> +
> + [[ -z "$_line" ]] && return
These quotes aren't needed, either; [[ doesn't do word splitting.
Base consistently omits quotes unless they're needed to join variables,
e.g. [[ -z "${foo} spaces.. ${bar}" ]]
> + _if=$_line
> + _line=
> +
> + if [[ -e /etc/hostname.$_if ]]; then
> + print -u2 "${0##*/}: $_hn: /etc/hostname.$_if overrides"
> + return
> + fi
> + fi
> +
> # Interface names must be alphanumeric only. We check to avoid
> # configuring backup or temp files, and to catch the "*" case.
> [[ $_if != +([[:alpha:]])+([[:digit:]]) ]] && return
> @@ -183,14 +199,16 @@ ifmstart() {
> local _sifs=$1 _xifs=$2 _hn _if _sif _xif
>
> for _sif in ${_sifs:-ALL}; do
> - for _hn in /etc/hostname.+([[:alpha:]])+([[:digit:]]); do
> + for _hn in
> /etc/hostname.@(+([[:alpha:]])+([[:digit:]])|??:??:??:??:??:??); do
> [[ -f $_hn ]] || continue
> _if=${_hn#/etc/hostname.}
>
> - # Skip unwanted ifs.
> - for _xif in $_xifs; do
> -
Re: lladdr support for netstart/hostname.if
On Sat, Nov 26, 2022 at 07:12:12PM +, Klemens Nanni wrote:
> On Sat, Nov 26, 2022 at 10:44:05AM -0800, Andrew Hewus Fresh wrote:
> > I think this is the complete diff, with the man page bits mostly from
> > Martijn. Now that the ifconfig -M bits are in, this should be easier
> > to test. I was able to test the installer bits, and they worked for me.
>
> This reads straight forward, but see inline.
>
> > + # If the _if looks like an lladdr and look it up.
>
> That sentence is not proper english.
> "Look up the interface if _if seems like an lladdr."
> or
> "Obtain the interface if _if is a MAC"
> ?
I realized the comment wasn't particularly useful, so I deleted it.
> > + if [[ $_if = ??:??:??:??:??:?? ]]; then
>
> Please use == for comparison; I know = works but it reads off.
> Base should use == consistently, imho.
Somehow I thought the right side being a glob match only worked with "="
not "==", but turns out I was wrong. Fixed.
>
> > + if ! _line="$( ifconfig -M "$_if" )"; then
>
> Here you quote the subshell and the variable...
That is a heck of a habit I have. Apparently I can't avoid typing
those. Fixed.
>
> > + print -u2 "${0##*/}: unique if for lladdr $_if not
> > found."
>
> Maybe just "$_if is not unique"?
I like it! Thanks!
> >
> > + if [[ $_if = ??:??:??:??:??:?? ]]; then
> > + _if=$(ifconfig -M $_if)
>
> ... here you quote neither subshell nor variable.
I was trying to match the style like this, but not quoting things is
hard for me to do.
> I think that's fine, both install.sub and netstart already use unquoted
> $_if and the subshell does not have to be quoted, anyway, since word
> spliting does not occur:
>
> $ v=$(echo 1 2 3)
> $ echo $v
> 1 2 3
It does split though, echo just re-assembles it back the way it started:
$ v=$(printf "%s\n" 1 2 3 )
$ # without the quotes , it gets tokenized again and the newlines disappear
$ echo "$v"
1
2
3
> > + elif [[ $_if = ??:??:??:??:??:?? ]]; then
>
> ==, please.
Can do. Too used to writing portable things for /bin/sh I guess.
> > +Additionally the interface name can be replaced with the interface's
> > lladdr,
> > +given that the lladdr is unique across the system and no
> > +.Nm
> > +exists for the interface name.
>
> Isn't that too vague?
>
> .Nm is hostname.if but you meant to say that .fxp0 should not exist when .MAC
> already exists, no?
I'm not sure, it makes sense to me. Hopefully jmc provides an excellent
option, but I'll think on wording.
Here's another revision, not really any changes to the man page, but
your suggestions got applied, I think.
Index: etc/netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.229
diff -u -p -r1.229 netstart
--- etc/netstart5 Nov 2022 12:06:05 - 1.229
+++ etc/netstart26 Nov 2022 21:02:08 -
@@ -135,6 +135,22 @@ ifstart() {
local _if=$1 _hn=/etc/hostname.$1 _cmds _i=0 _line _stat
set -A _cmds
+ if [[ $_if == ??:??:??:??:??:?? ]]; then
+ if ! _line=$( ifconfig -M $_if ); then
+ print -u2 "${0##*/}: $_if is not unique."
+ return
+ fi
+
+ [[ -z "$_line" ]] && return
+ _if=$_line
+ _line=
+
+ if [[ -e /etc/hostname.$_if ]]; then
+ print -u2 "${0##*/}: $_hn: /etc/hostname.$_if overrides"
+ return
+ fi
+ fi
+
# Interface names must be alphanumeric only. We check to avoid
# configuring backup or temp files, and to catch the "*" case.
[[ $_if != +([[:alpha:]])+([[:digit:]]) ]] && return
@@ -183,14 +199,16 @@ ifmstart() {
local _sifs=$1 _xifs=$2 _hn _if _sif _xif
for _sif in ${_sifs:-ALL}; do
- for _hn in /etc/hostname.+([[:alpha:]])+([[:digit:]]); do
+ for _hn in
/etc/hostname.@(+([[:alpha:]])+([[:digit:]])|??:??:??:??:??:??); do
[[ -f $_hn ]] || continue
_if=${_hn#/etc/hostname.}
- # Skip unwanted ifs.
- for _xif in $_xifs; do
- [[ $_xif == ${_if%%[0-9]*} ]] && continue 2
- done
+ if [[ $_if == +([[:alpha:]])+([[:digit:]]) ]]; then
+ # Skip unwanted ifs.
+ for _xif in $_xifs; do
+ [[ $_xif == ${_if%%[0-9]*} ]] &&
continue 2
+ done
+ fi
# Start wanted ifs.
[[ $_sif == @(ALL|${_if%%[0-9]*}) ]] && ifstart $_if
Index: distrib/miniroot/install.sub
===
RCS file:
pfsync panic after pf_purge backout
Hi all,
sashan@ and dlg@ I'm sorry if your eyes bleeds from pfsync panics. I
just wanted to send panic log here after pf_purge backout.
I'm testing pfsync with NET_TASKQ=6 because I have 6 core boxes and that
way panics are faster to trigger.
In this test I sure that pf is hitting state limit quite hard and I'm
only generating traffic and nothing more.
I'm generating traffic with cisco t-rex so traffic is real as what you
can get in lab environment.
Now I'm trying to trigger panic with WITNESS but it seems that it hard.
I just need to say that with all pf, pfsync and with pf_purge diffs
after hackaton + this diff on tech@
https://www.mail-archive.com/tech@openbsd.org/msg72582.html
my production firewall seems stable and it wasn't without that diff
I'm not sure if we have same diffs but even Josmar Pierri on bugs@
https://www.mail-archive.com/bugs@openbsd.org/msg18994.html
who had panics quite regularly with that diff on tech@ seems to have
stable firewall now.
r620-1# uvm_fault(0x82374288, 0x17, 0, 2) -> e
kernel: page fault trap, code=0
Stopped at pfsync_q_del+0x96: movq%rdx,0x8(%rax)
TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
*192892 19920 0 0x14000 0x2005K softnet
pfsync_q_del(fd82e8a4ce20) at pfsync_q_del+0x96
pf_remove_state(fd82e8a4ce20) at pf_remove_state+0x14b
pfsync_in_del_c(fd8006d843b8,c,79,0) at pfsync_in_del_c+0x6f
pfsync_input(800022d60ad8,800022d60ae4,f0,2) at pfsync_input+0x33c
ip_deliver(800022d60ad8,800022d60ae4,f0,2) at ip_deliver+0x113
ipintr() at ipintr+0x69
if_netisr(0) at if_netisr+0xea
taskq_thread(8003) at taskq_thread+0x100
end trace frame: 0x0, count: 7
https://www.openbsd.org/ddb.html describes the minimum info required in
bug reports. Insufficient info makes it difficult to find and fix bugs.
ddb{5}>
ddb{5}> show panic
*cpu5: uvm_fault(0x82374288, 0x17, 0, 2) -> e
ddb{5}>
ddb{5}> show reg
rdi0
rsi 0xf
rbp 0x800022d60920
rbx0x3cc
rdx 0x
rcx 0x10
rax 0xf
r80xfd82b9c8a8d0
r90xfd82cd8efe38
r10 0xfd82cd8efe38
r11 0x56c3d350aa4ea3d2
r12 0x808c4000
r13 0x28
r14 0xfd82e8a4ce20
r15 0x808c4720
rip 0x81c78626pfsync_q_del+0x96
cs 0x8
rflags 0x10286__ALIGN_SIZE+0xf286
rsp 0x800022d608f0
ss 0x10
pfsync_q_del+0x96: movq%rdx,0x8(%rax)
ddb{5}>
ddb{5}> ps /o
TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
*192892 19920 0 0x14000 0x2005K softnet
ddb{5}> trace /t 0t 192892
sleep_finish(8272dea0,1) at sleep_finish+0xfe
tsleep(823738f0,4,81f880db,0) at tsleep+0xb2
main(0) at main+0x775
end trace frame: 0x0, count: -3
ddb{5}> ps
PID TID PPIDUID S FLAGS WAIT COMMAND
4058 502031 1 0 30x100083 ttyin ksh
48707 291695 1 0 30x100098 kqreadcron
37722 219650 96592 95 3 0x1100092 kqreadsmtpd
38110 501136 96592103 3 0x1100092 kqreadsmtpd
6191 274846 96592 95 3 0x1100092 kqreadsmtpd
93001 113813 96592 95 30x100092 kqreadsmtpd
48938 178445 96592 95 3 0x1100092 kqreadsmtpd
60938 255136 96592 95 3 0x1100092 kqreadsmtpd
96592 417634 1 0 30x100080 kqreadsmtpd
37963 127625 1 0 30x88 kqreadsshd
32077 367571 1 0 30x100080 kqreadntpd
47614 473827 5112 83 30x100092 kqreadntpd
5112 74565 1 83 3 0x1100012 netlock ntpd
88022 357328 31934 74 3 0x1100092 bpf pflogd
31934 430624 1 0 30x80 netio pflogd
88174 57576 57658 73 3 0x1100090 kqreadsyslogd
576585426 1 0 30x100082 netio syslogd
93915 271655 0 0 3 0x14200 bored smr
60765 222732 0 0 3 0x14200 pgzerozerothread
75107 75302 0 0 3 0x14200 aiodoned aiodoned
26715 517414 0 0 3 0x14200 syncerupdate
30007 265724 0 0 3 0x14200 cleaner cleaner
53800 389208 0 0 3 0x14200 reaperreaper
19611 77488 0 0 3 0x14200 pgdaemon pagedaemon
38998 273899 0 0 3 0x14200 usbtskusbtask
92490 66823 0 0 3 0x14200 usbatsk usbatsk
37775 10602 0 0 3 0x40014200 acpi0 acpi0
70481
Re: lladdr support for netstart/hostname.if
On Sat, Nov 26, 2022 at 10:44:05AM -0800, Andrew Hewus Fresh wrote:
> I think this is the complete diff, with the man page bits mostly from
> Martijn. Now that the ifconfig -M bits are in, this should be easier
> to test. I was able to test the installer bits, and they worked for me.
This reads straight forward, but see inline.
>
> Index: etc/netstart
> ===
> RCS file: /cvs/src/etc/netstart,v
> retrieving revision 1.229
> diff -u -p -r1.229 netstart
> --- etc/netstart 5 Nov 2022 12:06:05 - 1.229
> +++ etc/netstart 26 Nov 2022 17:21:38 -
> @@ -135,6 +135,23 @@ ifstart() {
> local _if=$1 _hn=/etc/hostname.$1 _cmds _i=0 _line _stat
> set -A _cmds
>
> + # If the _if looks like an lladdr and look it up.
That sentence is not proper english.
"Look up the interface if _if seems like an lladdr."
or
"Obtain the interface if _if is a MAC"
?
> + if [[ $_if = ??:??:??:??:??:?? ]]; then
Please use == for comparison; I know = works but it reads off.
Base should use == consistently, imho.
> + if ! _line="$( ifconfig -M "$_if" )"; then
Here you quote the subshell and the variable...
> + print -u2 "${0##*/}: unique if for lladdr $_if not
> found."
Maybe just "$_if is not unique"?
> + return
> + fi
> +
> + [[ -z "$_line" ]] && return
> + _if=$_line
> + _line=
> +
> + if [[ -e /etc/hostname.$_if ]]; then
> + print -u2 "${0##*/}: $_hn: /etc/hostname.$_if overrides"
> + return
> + fi
> + fi
> +
> # Interface names must be alphanumeric only. We check to avoid
> # configuring backup or temp files, and to catch the "*" case.
> [[ $_if != +([[:alpha:]])+([[:digit:]]) ]] && return
> @@ -183,14 +200,16 @@ ifmstart() {
> local _sifs=$1 _xifs=$2 _hn _if _sif _xif
>
> for _sif in ${_sifs:-ALL}; do
> - for _hn in /etc/hostname.+([[:alpha:]])+([[:digit:]]); do
> + for _hn in
> /etc/hostname.@(+([[:alpha:]])+([[:digit:]])|??:??:??:??:??:??); do
> [[ -f $_hn ]] || continue
> _if=${_hn#/etc/hostname.}
>
> - # Skip unwanted ifs.
> - for _xif in $_xifs; do
> - [[ $_xif == ${_if%%[0-9]*} ]] && continue 2
> - done
> + if [[ $_if = +([[:alpha:]])+([[:digit:]]) ]]; then
> + # Skip unwanted ifs.
> + for _xif in $_xifs; do
> + [[ $_xif == ${_if%%[0-9]*} ]] &&
> continue 2
> + done
> + fi
>
> # Start wanted ifs.
> [[ $_sif == @(ALL|${_if%%[0-9]*}) ]] && ifstart $_if
> Index: distrib/miniroot/install.sub
> ===
> RCS file: /cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.1214
> diff -u -p -r1.1214 install.sub
> --- distrib/miniroot/install.sub 6 Nov 2022 21:32:54 - 1.1214
> +++ distrib/miniroot/install.sub 26 Nov 2022 17:21:38 -
> @@ -2431,6 +2431,12 @@ ifstart() {
> local _if=$1 _hn=/mnt/etc/hostname.$1 _cmds _i=0 _line
> set -A _cmds
>
> + if [[ $_if = ??:??:??:??:??:?? ]]; then
> + _if=$(ifconfig -M $_if)
... here you quote neither subshell nor variable.
I think that's fine, both install.sub and netstart already use unquoted
$_if and the subshell does not have to be quoted, anyway, since word
spliting does not occur:
$ v=$(echo 1 2 3)
$ echo $v
1 2 3
> + [[ -z $_if ]] && return # invalid interface
> + [[ -e /mnt/etc/hostname.$_if ]] && return # duplicate config
> + fi
> +
> # Create interface if it does not yet exist.
> { ifconfig $_if || ifconfig $_if create; } >/dev/null 2>&1 || return
>
> @@ -2472,6 +2478,9 @@ enable_ifs() {
> svlan) _svlans="$_svlans $_if";;
> vlan) _vlans="$_vlans $_if";;
> esac
> + elif [[ $_if = ??:??:??:??:??:?? ]]; then
==, please.
> + # start by lladdr
> + ifstart $_if
> else
> # 'Real' interfaces (if available) are done now.
> ifconfig $_if >/dev/null 2>&1 && ifstart $_if
> Index: share/man/man5/hostname.if.5
> ===
> RCS file: /cvs/src/share/man/man5/hostname.if.5,v
> retrieving revision 1.79
> diff -u -p -r1.79 hostname.if.5
> --- share/man/man5/hostname.if.5 26 Jul 2022 00:36:54 - 1.79
> +++ share/man/man5/hostname.if.5 26 Nov 2022 17:21:38 -
> @@ -41,9 +41,14
Re: lladdr support for netstart/hostname.if
I think this is the complete diff, with the man page bits mostly from
Martijn. Now that the ifconfig -M bits are in, this should be easier
to test. I was able to test the installer bits, and they worked for me.
Index: etc/netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.229
diff -u -p -r1.229 netstart
--- etc/netstart5 Nov 2022 12:06:05 - 1.229
+++ etc/netstart26 Nov 2022 17:21:38 -
@@ -135,6 +135,23 @@ ifstart() {
local _if=$1 _hn=/etc/hostname.$1 _cmds _i=0 _line _stat
set -A _cmds
+ # If the _if looks like an lladdr and look it up.
+ if [[ $_if = ??:??:??:??:??:?? ]]; then
+ if ! _line="$( ifconfig -M "$_if" )"; then
+ print -u2 "${0##*/}: unique if for lladdr $_if not
found."
+ return
+ fi
+
+ [[ -z "$_line" ]] && return
+ _if=$_line
+ _line=
+
+ if [[ -e /etc/hostname.$_if ]]; then
+ print -u2 "${0##*/}: $_hn: /etc/hostname.$_if overrides"
+ return
+ fi
+ fi
+
# Interface names must be alphanumeric only. We check to avoid
# configuring backup or temp files, and to catch the "*" case.
[[ $_if != +([[:alpha:]])+([[:digit:]]) ]] && return
@@ -183,14 +200,16 @@ ifmstart() {
local _sifs=$1 _xifs=$2 _hn _if _sif _xif
for _sif in ${_sifs:-ALL}; do
- for _hn in /etc/hostname.+([[:alpha:]])+([[:digit:]]); do
+ for _hn in
/etc/hostname.@(+([[:alpha:]])+([[:digit:]])|??:??:??:??:??:??); do
[[ -f $_hn ]] || continue
_if=${_hn#/etc/hostname.}
- # Skip unwanted ifs.
- for _xif in $_xifs; do
- [[ $_xif == ${_if%%[0-9]*} ]] && continue 2
- done
+ if [[ $_if = +([[:alpha:]])+([[:digit:]]) ]]; then
+ # Skip unwanted ifs.
+ for _xif in $_xifs; do
+ [[ $_xif == ${_if%%[0-9]*} ]] &&
continue 2
+ done
+ fi
# Start wanted ifs.
[[ $_sif == @(ALL|${_if%%[0-9]*}) ]] && ifstart $_if
Index: distrib/miniroot/install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1214
diff -u -p -r1.1214 install.sub
--- distrib/miniroot/install.sub6 Nov 2022 21:32:54 - 1.1214
+++ distrib/miniroot/install.sub26 Nov 2022 17:21:38 -
@@ -2431,6 +2431,12 @@ ifstart() {
local _if=$1 _hn=/mnt/etc/hostname.$1 _cmds _i=0 _line
set -A _cmds
+ if [[ $_if = ??:??:??:??:??:?? ]]; then
+ _if=$(ifconfig -M $_if)
+ [[ -z $_if ]] && return # invalid interface
+ [[ -e /mnt/etc/hostname.$_if ]] && return # duplicate config
+ fi
+
# Create interface if it does not yet exist.
{ ifconfig $_if || ifconfig $_if create; } >/dev/null 2>&1 || return
@@ -2472,6 +2478,9 @@ enable_ifs() {
svlan) _svlans="$_svlans $_if";;
vlan) _vlans="$_vlans $_if";;
esac
+ elif [[ $_if = ??:??:??:??:??:?? ]]; then
+ # start by lladdr
+ ifstart $_if
else
# 'Real' interfaces (if available) are done now.
ifconfig $_if >/dev/null 2>&1 && ifstart $_if
Index: share/man/man5/hostname.if.5
===
RCS file: /cvs/src/share/man/man5/hostname.if.5,v
retrieving revision 1.79
diff -u -p -r1.79 hostname.if.5
--- share/man/man5/hostname.if.526 Jul 2022 00:36:54 - 1.79
+++ share/man/man5/hostname.if.526 Nov 2022 17:21:38 -
@@ -41,9 +41,14 @@ The
.Nm hostname.*\&
files contain information regarding the configuration of each network
interface.
One file should exist for each interface that is to be configured, such as
-.Pa hostname.fxp0
+.Pa hostname.fxp0 ,
+.Pa hostname.12:34:56:78:ab ,
or
.Pa hostname.bridge0 .
+Additionally the interface name can be replaced with the interface's lladdr,
+given that the lladdr is unique across the system and no
+.Nm
+exists for the interface name.
A configuration file is not needed for lo0.
.Pp
The configuration information is expressed in a line-by-line packed format
Re: dhcpleased, resolvd, slaacd: accurate lock error message
But "Resource temporarily unavailable" has absolutely no meaning to anyone who will try to do this by hand, it is better for the message to interpret the situation so that someone can understand. I think your proposal makes it worse. Klemens Nanni wrote: > To debug things, I started slaacd manually from single-user: > > boot> b /bsd -s > # slaacd -dv > slaacd: already running > > We added the lock and this message to prevent multiple instances in the > installer, where this is indeed the only error case that can happen. > > But hitting it outside the installer (which entirely discards stderr) > makes me think that the error message it too much of an assumption and > ignores the valuable errno for no reason. > > /dev/slaacd.lock couldn't be aquired because / is still read-only, so > the following would've told me that immediately: > > # slaacd -dv > slaacd: lock: Read-only file system > > If you actually have it running already in multi-user and start a copy: > > # obj/slaacd -dv > slaacd: lock: Resource temporarily unavailable > > > The lock was introduced to fix duplicate startups in the installer; > there the error message is entirely discarded anyway, as the only error > should be indeed an already running instance. > > Should "lock" be _PATH_LOCKFILE, i.e. > slaacd: /dev/slaacd.lock: Read-only file system > > Feedback? Objection? OK? > > Index: slaacd/slaacd.c > === > RCS file: /cvs/src/sbin/slaacd/slaacd.c,v > retrieving revision 1.66 > diff -u -p -r1.66 slaacd.c > --- slaacd/slaacd.c 15 Sep 2022 07:59:59 - 1.66 > +++ slaacd/slaacd.c 26 Nov 2022 17:02:51 - > @@ -180,7 +180,7 @@ main(int argc, char *argv[]) > > lockfd = open(_PATH_LOCKFILE, O_CREAT|O_RDWR|O_EXLOCK|O_NONBLOCK, 0600); > if (lockfd == -1) > - errx(1, "already running"); > + err(1, "lock"); > > /* Check for assigned daemon user */ > if (getpwnam(SLAACD_USER) == NULL) > Index: dhcpleased/dhcpleased.c > === > RCS file: /cvs/src/sbin/dhcpleased/dhcpleased.c,v > retrieving revision 1.26 > diff -u -p -r1.26 dhcpleased.c > --- dhcpleased/dhcpleased.c 23 Jul 2022 09:33:18 - 1.26 > +++ dhcpleased/dhcpleased.c 26 Nov 2022 17:02:51 - > @@ -222,7 +222,7 @@ main(int argc, char *argv[]) > > lockfd = open(_PATH_LOCKFILE, O_CREAT|O_RDWR|O_EXLOCK|O_NONBLOCK, 0600); > if (lockfd == -1) > - errx(1, "already running"); > + err(1, "lock"); > > /* Check for assigned daemon user */ > if (getpwnam(DHCPLEASED_USER) == NULL) > Index: resolvd/resolvd.c > === > RCS file: /cvs/src/sbin/resolvd/resolvd.c,v > retrieving revision 1.29 > diff -u -p -r1.29 resolvd.c > --- resolvd/resolvd.c 14 Nov 2022 13:57:46 - 1.29 > +++ resolvd/resolvd.c 26 Nov 2022 17:02:50 - > @@ -194,7 +194,7 @@ main(int argc, char *argv[]) > > lockfd = open(_PATH_LOCKFILE, O_CREAT|O_RDWR|O_EXLOCK|O_NONBLOCK, 0600); > if (lockfd == -1) > - errx(1, "already running"); > + err(1, "lock"); > > if (!debug) > daemon(0, 0); >
dhcpleased, resolvd, slaacd: accurate lock error message
To debug things, I started slaacd manually from single-user: boot> b /bsd -s # slaacd -dv slaacd: already running We added the lock and this message to prevent multiple instances in the installer, where this is indeed the only error case that can happen. But hitting it outside the installer (which entirely discards stderr) makes me think that the error message it too much of an assumption and ignores the valuable errno for no reason. /dev/slaacd.lock couldn't be aquired because / is still read-only, so the following would've told me that immediately: # slaacd -dv slaacd: lock: Read-only file system If you actually have it running already in multi-user and start a copy: # obj/slaacd -dv slaacd: lock: Resource temporarily unavailable The lock was introduced to fix duplicate startups in the installer; there the error message is entirely discarded anyway, as the only error should be indeed an already running instance. Should "lock" be _PATH_LOCKFILE, i.e. slaacd: /dev/slaacd.lock: Read-only file system Feedback? Objection? OK? Index: slaacd/slaacd.c === RCS file: /cvs/src/sbin/slaacd/slaacd.c,v retrieving revision 1.66 diff -u -p -r1.66 slaacd.c --- slaacd/slaacd.c 15 Sep 2022 07:59:59 - 1.66 +++ slaacd/slaacd.c 26 Nov 2022 17:02:51 - @@ -180,7 +180,7 @@ main(int argc, char *argv[]) lockfd = open(_PATH_LOCKFILE, O_CREAT|O_RDWR|O_EXLOCK|O_NONBLOCK, 0600); if (lockfd == -1) - errx(1, "already running"); + err(1, "lock"); /* Check for assigned daemon user */ if (getpwnam(SLAACD_USER) == NULL) Index: dhcpleased/dhcpleased.c === RCS file: /cvs/src/sbin/dhcpleased/dhcpleased.c,v retrieving revision 1.26 diff -u -p -r1.26 dhcpleased.c --- dhcpleased/dhcpleased.c 23 Jul 2022 09:33:18 - 1.26 +++ dhcpleased/dhcpleased.c 26 Nov 2022 17:02:51 - @@ -222,7 +222,7 @@ main(int argc, char *argv[]) lockfd = open(_PATH_LOCKFILE, O_CREAT|O_RDWR|O_EXLOCK|O_NONBLOCK, 0600); if (lockfd == -1) - errx(1, "already running"); + err(1, "lock"); /* Check for assigned daemon user */ if (getpwnam(DHCPLEASED_USER) == NULL) Index: resolvd/resolvd.c === RCS file: /cvs/src/sbin/resolvd/resolvd.c,v retrieving revision 1.29 diff -u -p -r1.29 resolvd.c --- resolvd/resolvd.c 14 Nov 2022 13:57:46 - 1.29 +++ resolvd/resolvd.c 26 Nov 2022 17:02:50 - @@ -194,7 +194,7 @@ main(int argc, char *argv[]) lockfd = open(_PATH_LOCKFILE, O_CREAT|O_RDWR|O_EXLOCK|O_NONBLOCK, 0600); if (lockfd == -1) - errx(1, "already running"); + err(1, "lock"); if (!debug) daemon(0, 0);
Re: uhidpp(4): add support for Bolt receivers and Unified Battery feature
On Sat, Nov 26, 2022 at 02:39:48PM +, Klemens Nanni wrote: > On Sat, Nov 26, 2022 at 07:46:08AM +0100, Anton Lindqvist wrote: > > Hi, > > This diff adds supports for the following to uhidpp: > > > > 1. Bolt receivers, they use a different set of registers for querying > >paired devices. > > > > 2. The Unified Battery feature, this is a more competent feature > >function used to report battery status compared to the Battery > >feature which is already supported by uhidpp. > > > > Makes battery sensors appear for my Logitech Lift mouse and Paul de > > Weerd's MX Anywhere 3 mouse. > > > > If you have a Logitech HID++ device currently lacking sensors under > > sysctl hw.sensors, please give this diff a try. Send me your dmesg and > > output of sysctl hw.sensors after moving the mouse to ensure a connect > > interrupt has fired off. Note that the diff must be applied on top of > > revision 1.37 of sys/dev/usb/uhidpp.c. > > No idea what Bolt or Logitech HID++ is, but my MX Ergo doesn't report > any new sensors with this diff. Should it do so? Nevermind, it just took a while for the device to send battery reports: hw.sensors.uhidpp0.indicator0=Off (charger) hw.sensors.uhidpp0.raw0=4 (number of battery levels) hw.sensors.uhidpp0.percent0=50.00% (battery level), OK So all working as intended, I just didn't wait long enough after attach to mail out my findings. These lines/values are logged every thirty seconds, I guess those are the actual battery reports? hidpp_send_report: 11 01 08 01 [00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00] uhidpp_intr: 11 01 08 01 [32 14 00 00 00 00 00 00 00 00 00 00 00 00 00 00] Thanks!
Re: uhidpp(4): add support for Bolt receivers and Unified Battery feature
On Sat, Nov 26, 2022 at 07:46:08AM +0100, Anton Lindqvist wrote:
> Hi,
> This diff adds supports for the following to uhidpp:
>
> 1. Bolt receivers, they use a different set of registers for querying
>paired devices.
>
> 2. The Unified Battery feature, this is a more competent feature
>function used to report battery status compared to the Battery
>feature which is already supported by uhidpp.
>
> Makes battery sensors appear for my Logitech Lift mouse and Paul de
> Weerd's MX Anywhere 3 mouse.
>
> If you have a Logitech HID++ device currently lacking sensors under
> sysctl hw.sensors, please give this diff a try. Send me your dmesg and
> output of sysctl hw.sensors after moving the mouse to ensure a connect
> interrupt has fired off. Note that the diff must be applied on top of
> revision 1.37 of sys/dev/usb/uhidpp.c.
No idea what Bolt or Logitech HID++ is, but my MX Ergo doesn't report
any new sensors with this diff. Should it do so?
The trackball keeps working, though.
Here's the dmesg from when I attach the USB receiver with your diff:
uhidev0 at uhub3 port 2 configuration 1 interface 0 "Logitech USB Receiver" rev
2.00/12.03 addr 3
uhidev0: iclass 3/1
ukbd0 at uhidev0: 8 variable keys, 6 key codes
wskbd1 at ukbd0 mux 1
wskbd1: connecting to wsdisplay0
uhidev1 at uhub3 port 2 configuration 1 interface 1 "Logitech USB Receiver" rev
2.00/12.03 addr 3
uhidev1: iclass 3/1, 8 report ids
ums0 at uhidev1 reportid 2: 16 buttons, Z and W dir
wsmouse2 at ums0 mux 0
ucc0 at uhidev1 reportid 3: 767 usages, 20 keys, array
wskbd2 at ucc0 mux 1
wskbd2: connecting to wsdisplay0
uhid0 at uhidev1 reportid 4: input=1, output=0, feature=0
uhid1 at uhidev1 reportid 8: input=1, output=0, feature=0
uhidev2 at uhub3 port 2 configuration 1 interface 2 "Logitech USB Receiver" rev
2.00/12.03 addr 3
uhidev2: iclass 3/0, 33 report ids
uhidpp0 at uhidev2hidpp_send_report: 10 ff 83 b5 [20 00 00]
uhidpp_intr: 11 ff 83 b5 [20 0b 08 40 6f 04 02 08 07 00 00 00 00 00 00 00]
hidpp_send_report: 10 ff 83 b5 [40 00 00]
uhidpp_intr: 11 ff 83 b5 [40 07 4d 58 20 45 72 67 6f 00 00 00 00 00 00 00]
device 1 trackball "MX Ergo"hidpp_send_report: 10 ff 83 b5 [21 00 00]
uhidpp_intr: 10 ff 8f 83 [b5 03 00]
hidpp_send_report: 10 ff 83 b5 [22 00 00]
uhidpp_intr: 10 ff 8f 83 [b5 03 00]
hidpp_send_report: 10 ff 83 b5 [23 00 00]
uhidpp_intr: 10 ff 8f 83 [b5 03 00]
hidpp_send_report: 10 ff 83 b5 [24 00 00]
uhidpp_intr: 10 ff 8f 83 [b5 03 00]
hidpp_send_report: 10 ff 83 b5 [25 00 00]
uhidpp_intr: 10 ff 8f 83 [b5 03 00]
hidpp_send_report: 10 ff 80 00 [10 09 00]
uhidpp_intr: 10 ff 80 00 [00 00 00]
uhidpp_intr: 10 ff 80 00 [00 00 00]
uhid2 at uhidev2 reportid 32: input=14, output=14, feature=0
uhid3 at uhidev2 reportid 33: input=31, output=31, feature=0
>
> Comments? OK?
>
> diff --git sys/dev/usb/uhidpp.c sys/dev/usb/uhidpp.c
> index 83e58d56fc9..5b9ef198948 100644
> --- sys/dev/usb/uhidpp.c
> +++ sys/dev/usb/uhidpp.c
> @@ -29,7 +29,7 @@
> #include
> #include
>
> -/* #define UHIDPP_DEBUG */
> +#define UHIDPP_DEBUG
> #ifdef UHIDPP_DEBUG
>
> #define DPRINTF(x...) do { \
> @@ -125,6 +125,15 @@ int uhidpp_debug = 1;
> #define HIDPP20_FEAT_BATTERY_CAPABILITY_FUNC 0x0001
> #define HIDPP20_CAPABILITY_RECHARGEABLE 0x0004
>
> +#define HIDPP20_FEAT_UNIFIED_BATTERY_ID 0x1004
> +#define HIDPP20_FEAT_UNIFIED_BATTERY_CAPABILITIES_FUNC 0x
> +#define HIDPP20_CAPABILITES_RECHARGEABLE0x0002
> +#define HIDPP20_FEAT_UNIFIED_BATTERY_STATUS_FUNC 0x0001
> +#define HIDPP20_BATTERY_STATUS_CRITICAL 0x0001
> +#define HIDPP20_BATTERY_STATUS_LOW 0x0002
> +#define HIDPP20_BATTERY_STATUS_GOOD 0x0004
> +#define HIDPP20_BATTERY_STATUS_FULL 0x0008
> +
> /* HID++ 2.0 error codes. */
> #define HIDPP20_ERROR0xff
> #define HIDPP20_ERROR_NO_ERROR 0x00
> @@ -193,17 +202,20 @@ struct uhidpp_device {
> uint8_t d_features;
> #define UHIDPP_DEVICE_FEATURE_ROOT 0x01
> #define UHIDPP_DEVICE_FEATURE_BATTERY0x02
> +#define UHIDPP_DEVICE_FEATURE_UNIFIIED_BATTERY 0x04
>
> struct {
> struct ksensor sens[UHIDPP_NSENSORS];
> uint8_t feature_idx;
> uint8_t nlevels;
> + uint8_t unified_level_mask;
> uint8_t rechargeable;
> } d_battery;
> };
>
> /*
> * Locking:
> + * [I] immutable
> * [m] sc_mtx
> */
> struct uhidpp_softc {
> @@ -226,6 +238,11 @@ struct uhidpp_softc {
> struct uhidpp_report *sc_req; /* [m] synchronous request buffer */
> struct uhidpp_report *sc_resp; /* [m] synchronous response buffer */
> u_int sc_resp_state;/* [m] synchronous response state */
> +
> + enum {
> + UHIDPP_RECEIVER_UNIFYING,
> + UHIDPP_RECEIVER_BOLT,
> + } sc_receiver;
