From: Klemens Nanni <kn@openbsd.org>
Subject: Re: Use dmesg(8) in fw_update(8) instead of dmesg.boot
To: Andrew Hewus Fresh <andrew@afresh1.com>, tech@openbsd.org, Theo de Raadt <deraadt@openbsd.org>
Date: Fri, 26 Dec 2025 16:59:53 +0000

26.12.2025 00:03, Andrew Hewus Fresh пишет:
> On Thu, Dec 25, 2025 at 12:55:11PM -0700, Theo de Raadt wrote:
>> Klemens Nanni <kn@openbsd.org> wrote:
>>
>>> 25.12.2025 19:07, Theo de Raadt пишет:
>>>> Hmm.
>>>>
>>>> I think we chose dmesg.boot for a reason before, but I'm not sure I
>>>> remember the reason.
>>>>
>>>> Maybe we thought $(dmesg) could have rolled over with noisy debug messages,
>>>> and thus hidden names of devices we want to identify, and therefore
>>>> dmesg.boot was more reliable?
>>>>
>>>> Here's a thought:  Should we look at both $(dmesg) and dmesg.boot ??
>>>
>>> I certainly have machines where dmesg gets flooded with log spam,
>>> so any $(dmesg) with a few hours of uptime won't have a single device.
>>>
>>> If fw_update stops looking at dmesg.boot, there won't be any chance
>>> for it to update existing firmware, right?
>>
>> Look at my suggestion.
> 
> Or what I said on this:
> 
> 
> On Thu, Dec 25, 2025 at 08:01:28AM -0800, Andrew Hewus Fresh wrote:
>> The previous implementation looked at both,  I assume in case anything
>> had "fallen off" the dmesg filling up.  I think it is OK not to make
>> that more intrusive change because we would have checked at boot and we
>> also look for updates to already installed firmware.  The only failure
>> point I see is if dmesg had filled up past dmesg.boot, new firmware came
>> to be that didn't previously exist, and someone wanted to install it
>> without rebooting.
>>
>> https://github.com/openbsd/src/blob/master/usr.sbin/pkg_add/OpenBSD/FwUpdate.pm#L158-L175
> 
> In any case, here is a patch to scan both.  I haven't tried my alpha to
> see how much it slows things down, but folks are showing up to visit so
> I won't have too much time for revisions today.

Overall reads fine to me.

OK kn with the fix inline.

> 
> Index: fw_update.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/fw_update/fw_update.8,v
> diff -u -p -r1.8 fw_update.8
> --- fw_update.8	22 Mar 2025 19:51:29 -0000	1.8
> +++ fw_update.8	25 Dec 2025 20:56:42 -0000
> @@ -59,11 +59,11 @@ If used in conjunction with
>  .Fl a ,
>  delete firmware for all drivers.
>  .It Fl D Ar path
> -Use the
> -.Xr dmesg 8
> -output from
> +Use the content of
>  .Ar path
> -rather than
> +rather than output from 
> +.Xr dmesg 8
> +and
>  .Pa /var/run/dmesg.boot
>  to determine which firmware are needed.
>  .It Fl F
> Index: fw_update.sh
> ===================================================================
> RCS file: /cvs/src/usr.sbin/fw_update/fw_update.sh,v
> diff -u -p -r1.65 fw_update.sh
> --- fw_update.sh	12 May 2025 23:48:12 -0000	1.65
> +++ fw_update.sh	25 Dec 2025 20:56:42 -0000
> @@ -239,12 +239,25 @@ verify_existing() {
>  }
>  
>  devices_in_dmesg() {
> +	if [ "${DMESG:-}" ]; then
> +		_devices_in_dmesg "$DMESG"
> +		return
> +	fi
> +
> +	dmesg > "$FD_DIR/dmesg"
> +	
> +	_devices_in_dmesg /var/run/dmesg.boot
> +	_devices_in_dmesg "$FD_DIR/dmesg"
> +}
> +
> +_devices_in_dmesg() {
> +	local _dmesg=$1
>  	local IFS
>  	local _d _m _dmesgtail _last='' _nl='
>  '
>  
>  	# The dmesg can contain multiple boots, only look in the last one
> -	_dmesgtail="$( echo ; sed -n 'H;/^OpenBSD/h;${g;p;}' "$DMESG" )"
> +	_dmesgtail="$( echo ; sed -n 'H;/^OpenBSD/h;${g;p;}' "$_dmesg" )"
>  
>  	grep -v '^[[:space:]]*#' "$FWPATTERNS" |
>  	    while read -r _d _m; do
> @@ -489,7 +502,7 @@ set_fw_paths() {
>  	if [ ! "$_version" ]; then
>  		_version=$(sed -nE \
>  		    '/^OpenBSD ([0-9]+\.[0-9][^ ]*) .*/{s//\1/;h;};${g;p;}' \
> -		    "$DMESG")
> +		    "${DMESG:-/var/run/dmesg.boot")

} missing before ")

>  	
>  		# If VNAME was set in the environment instead of the DMESG,
>  		# looking in the DMESG for "current" is wrong.
> @@ -514,7 +527,6 @@ usage() {
>  
>  ALL=false
>  LIST=false
> -DMESG=/var/run/dmesg.boot
>  
>  while getopts :adD:Flnp:v name
>  do
> @@ -561,7 +573,7 @@ if [ "${FWURL:-}" ] && ! "$INSTALL" ; th
>  	usage
>  fi
>  
> -if [ ! -s "$DMESG" ]; then
> +if [ "${DMESG:-}" ] && [ ! -s "$DMESG" ]; then
>  	warn "${0##*/}: $DMESG: No such file or directory"
>  	exit 1
>  fi
>