On Sun, Dec 11, 2022 at 01:05:13PM -0800, Andrew Hewus Fresh wrote:
> This would be the diff to swap priority of name/lladdr as some folks
> wanted to see.  I still don't recommend having both as you can still
> have surprising outcomes with more complex configurations.
> 
> The install.sub includes an "if_name_to_lladdr" function from the
> diff to support creating a hostname.lladdr during install.  If we don't
> decide to support that, I'd probably in-line that as is done in
> netstart.
> 
> I did find that my iwm doesn't like being configured by lladdr at this
> point of the upgrade  because it doesn't have firmware loaded when it's
> looking to see if it's a valid hostname.lladdr, but the iwm's lladdr is
> all zeros still.
> 
> 
> Index: etc/netstart
> ===================================================================
> RCS file: /cvs/src/etc/netstart,v
> retrieving revision 1.230
> diff -u -p -r1.230 netstart
> --- etc/netstart      5 Dec 2022 20:12:00 -0000       1.230
> +++ etc/netstart      11 Dec 2022 20:44:34 -0000

More context for this diff:
138     if [[ $_if == ??:??:??:??:??:?? ]]; then
139             if ! _line=$( ifconfig -M $_if ); then
140                     print -u2 "${0##*/}: $_if is not unique."
141                     return
142             fi

> @@ -144,11 +144,14 @@ ifstart() {
>               [[ -z $_line ]] && return

_line is the interface name "em0", please call it _ifname or _if_name.
Just anything obviously descriptive.

>               _if=$_line
>               _line=

So the MAC in _if is overwritten with the interface name in _line which
is being zeroed, i.e. we looked up the interface via lladdr and proceed
as if the user had placed an .em0 file instead.

With s/_inline/_ifname/ that would be much easier to follow.

> -
> -             if [[ -e /etc/hostname.$_if ]]; then
> -                     print -u2 "${0##*/}: $_hn: /etc/hostname.$_if overrides"
> +     else
> +             _line=$(ifconfig $_if | sed -n 
> 's/^[[:space:]]*lladdr[[:space:]]//p')

This will print "_if: no such interface" for nonexistent interfaces and
must be silenced.

> +             if [[ -n $_line && -n $(ifconfig -M $_line) \

Pretty sure the -M call needs "$_line" since you can hit existent
interfaces without lladdr, so _line is empty and `ifconfig -M' would
print usage while `ifconfig -M ""' is silent as expected.

> +                && -e /etc/hostname.$_line ]]; then

The same sanity check further below uses -f.  Not sure if existence is
enough here or whether we should only switch files if there is an actual
file...

> +                     print -u2 "${0##*/}: $_hn: /etc/hostname.$_line: 
> overrides"
>                       return
>               fi
> +             _line=
>       fi

Otherwise the else block logic makes sense, but now you're using _if as
an interface name before the input validation, which follows immediately
afterwards.

Just merge the two like this:
        if [[ $_if == ??:??:??:??:??:?? ]]; then
                ...
        else if [[ $_if == +([[:alpha:]])+([[:digit:]]) ]]; then
                ...
        else
                return

Diff below to make things clear.
Maybe it'd be better to clean up things a bit and then do the prio swap
separately?

> Index: distrib/miniroot/install.sub

If that looks right, we can adapt the insaller bits accordingly.
I did not go over them now.


Index: netstart
===================================================================
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.230
diff -u -p -r1.230 netstart
--- netstart    5 Dec 2022 20:12:00 -0000       1.230
+++ netstart    15 Dec 2022 06:30:31 -0000
@@ -132,28 +132,29 @@ vifscreate() {
 # Start a single interface.
 # Usage: ifstart if1
 ifstart() {
-       local _if=$1 _hn=/etc/hostname.$1 _cmds _i=0 _line _stat
+       local _if=$1 _ifname _lladdr _hn=/etc/hostname.$1 _hnprio _cmds _i=0 
_line _stat
        set -A _cmds
 
        if [[ $_if == ??:??:??:??:??:?? ]]; then
-               if ! _line=$( ifconfig -M $_if ); then
+               _ifname=$(ifconfig -M $_if)
+               if (($? != 0)); 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
+               [[ -z $_if ]] && return
+               _if=$_ifname
+       elif [[ $_if == +([[:alpha:]])+([[:digit:]]) ]]; then
+               _lladdr=$(ifconfig $_if 2>/dev/null |
+                   awk '$1 == "lladdr" { print $2 }')
+               _hnprio=${_hn%$if}$_lladdr
+               if [[ -n $_lladdr && -e $_hnprio &&
+                   -n $(ifconfig -M "$_lladdr") ]]; then
+                       print -u2 "${0##*/}: $_hn: $_hnprio: overrides"
                fi
+       else
+               return
        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
 
        if [[ ! -f $_hn ]]; then
                print -u2 "${0##*/}: $_hn: No such file or directory."

Reply via email to