On Sat, Nov 26, 2022 at 07:12:12PM +0000, 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.
>
<SNIP>
> > + # 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!
<SNIP>
> >
> > + 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
<SNIP>
> > + elif [[ $_if = ??:??:??:??:??:?? ]]; then
>
> ==, please.
Can do. Too used to writing portable things for /bin/sh I guess.
<SNIP>
> > +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/netstart 5 Nov 2022 12:06:05 -0000 1.229
+++ etc/netstart 26 Nov 2022 21:02:08 -0000
@@ -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: /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 -0000 1.1214
+++ distrib/miniroot/install.sub 26 Nov 2022 21:02:08 -0000
@@ -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.5 26 Jul 2022 00:36:54 -0000 1.79
+++ share/man/man5/hostname.if.5 26 Nov 2022 21:02:08 -0000
@@ -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:90:ab ,
or
.Pa hostname.bridge0 .
+Alternatively the interface name can be replaced with the interface's lladdr,
+provided 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
