Re: lladdr support for netstart/hostname.if
It is time. ok deraadt There is a man page change we've discussed which can also go in, after that I expect jmc will get involved and change it a little. Andrew and I have had another discussion about how the installer might provide a way in the installer to create these files instead (no new question lines can be added, but questions can become more complex), but that's a later thing. 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. > > 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. > + 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.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) > + [[ -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 - 1.79 > +++ share/man/man5/hostname.if.5 26 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 . >
Re: lladdr support for netstart/hostname.if
On Thu, Dec 01, 2022 at 06:53:30PM -0800, Andrew Hewus Fresh wrote: > How about this adjustment to the man page? Reads fine, although it doesn't explain which file type wins. > > 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 2 Dec 2022 02:51:27 - > @@ -40,8 +40,10 @@ > 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 > +One file should exist for each interface that is to be configured. If you say "Exactly one" file, it'd be clear that there must not be both .MAC and .if for one interface. This way, I think, we're good with leaving out the implementation detail of .MAC overwriting .if -- afterall, users are told not to have multiple files per interface. > +The interface can be referenced by name or lladdr, such as > +.Pa hostname.fxp0 , > +.Pa hostname.00:00:5e:00:53:af , > or > .Pa hostname.bridge0 . > A configuration file is not needed for lo0. >
Re: lladdr support for netstart/hostname.if
How about this adjustment to the man page? 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.52 Dec 2022 02:51:27 - @@ -40,8 +40,10 @@ 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 +One file should exist for each interface that is to be configured. +The interface can be referenced by name or lladdr, such as +.Pa hostname.fxp0 , +.Pa hostname.00:00:5e:00:53:af , or .Pa hostname.bridge0 . A configuration file is not needed for lo0.
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:
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: lladdr support for netstart/hostname.if
Andrew spotted a potential issue with the short-circuit ordering, so here is a newer diff. Index: ifconfig.8 === RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v retrieving revision 1.385 diff -u -p -u -r1.385 ifconfig.8 --- ifconfig.8 26 Oct 2022 17:06:31 - 1.385 +++ ifconfig.8 24 Nov 2022 18:18:51 - @@ -40,6 +40,7 @@ .Sh SYNOPSIS .Nm ifconfig .Op Fl AaC +.Op Fl M Ar lladdr .Op Ar interface .Op Ar address_family .Oo @@ -88,6 +89,11 @@ This is the default, if no parameters ar Print the names of all network pseudo-devices that can be created dynamically at runtime using .Nm Cm create . +.It Fl M Ar lladdr +Scan the non-cloned interface list for the MAC address +.Ar lladdr +and print the name of that interface. +If the MAC address is found on multiple interfaces, print nothing. .It Ar interface The .Ar interface Index: ifconfig.c === RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v retrieving revision 1.457 diff -u -p -u -r1.457 ifconfig.c --- ifconfig.c 26 Oct 2022 17:06:31 - 1.457 +++ ifconfig.c 24 Nov 2022 19:19:55 - @@ -368,6 +368,9 @@ voidwg_status(int); void setignore(const char *, int); #endif +struct if_clonereq *get_cloners(void); +intfindmac(const char *); + /* * Media stuff. Whenever a media command is first performed, the * currently select media is grabbed for this interface. If `media' @@ -795,6 +798,11 @@ main(int argc, char *argv[]) Cflag = 1; nomore = 1; break; + case 'M': + if (argv[1] == NULL) + usage(); + exit (findmac(argv[1])); + break; default: usage(); break; @@ -1255,12 +1263,10 @@ clone_destroy(const char *addr, int para err(1, "SIOCIFDESTROY"); } -void -list_cloners(void) +struct if_clonereq * +get_cloners(void) { - struct if_clonereq ifcr; - char *cp, *buf; - int idx; + static struct if_clonereq ifcr; memset(, 0, sizeof(ifcr)); @@ -1269,12 +1275,9 @@ list_cloners(void) if (ioctl(sock, SIOCIFGCLONERS, ) == -1) err(1, "SIOCIFGCLONERS for count"); - buf = calloc(ifcr.ifcr_total, IFNAMSIZ); - if (buf == NULL) + if ((ifcr.ifcr_buffer = calloc(ifcr.ifcr_total, IFNAMSIZ)) == NULL) err(1, "unable to allocate cloner name buffer"); - ifcr.ifcr_count = ifcr.ifcr_total; - ifcr.ifcr_buffer = buf; if (ioctl(sock, SIOCIFGCLONERS, ) == -1) err(1, "SIOCIFGCLONERS for names"); @@ -1285,17 +1288,30 @@ list_cloners(void) if (ifcr.ifcr_count > ifcr.ifcr_total) ifcr.ifcr_count = ifcr.ifcr_total; - qsort(buf, ifcr.ifcr_count, IFNAMSIZ, + return +} + +void +list_cloners(void) +{ + struct if_clonereq *ifcr; + char *cp, *buf; + int idx; + + ifcr = get_cloners(); + buf = ifcr->ifcr_buffer; + + qsort(buf, ifcr->ifcr_count, IFNAMSIZ, (int(*)(const void *, const void *))strcmp); - for (cp = buf, idx = 0; idx < ifcr.ifcr_count; idx++, cp += IFNAMSIZ) { + for (cp = buf, idx = 0; idx < ifcr->ifcr_count; idx++, cp += IFNAMSIZ) { if (idx > 0) putchar(' '); printf("%s", cp); } putchar('\n'); - free(buf); + free(ifcr->ifcr_buffer); } #define RIDADDR 0 @@ -6614,7 +6630,7 @@ __dead void usage(void) { fprintf(stderr, - "usage: ifconfig [-AaC] [interface] [address_family] " + "usage: ifconfig [-AaC] [-M lladdr] [interface] [address_family] " "[address [dest_address]]\n" "\t\t[parameters]\n"); exit(1); @@ -6782,3 +6798,55 @@ setignore(const char *id, int param) /* just digest the command */ } #endif + +int +findmac(const char *mac) +{ + struct ifaddrs *ifap, *ifa; + const char *ifnam = NULL; + struct if_clonereq *ifcr; + + ifcr = get_cloners(); + + if (getifaddrs() != 0) + err(1, "getifaddrs"); + + for (ifa = ifap; ifa; ifa = ifa->ifa_next) { + struct sockaddr_dl *sdl = (struct sockaddr_dl *)ifa->ifa_addr; + + if (sdl != NULL && sdl->sdl_alen && + (sdl->sdl_type == IFT_ETHER || sdl->sdl_type == IFT_CARP)) { + if (strcmp(ether_ntoa((struct ether_addr *)LLADDR(sdl)), + mac) == 0) { + int idx, skip = 0; + char *cp; + size_t len; + + /* MACs
Re: lladdr support for netstart/hostname.if
I think this is the bits the installer needs. I might adjust /etc/netstart to use this glob or this to use *:*, still thinking on it. It's a bit simpler because I figured the installer could be quieter so I could just return instead of printing errors first. I haven't tested this yet. We'll see how my time is today with the US holiday, but too much further work will probably be pushed to the weekend. With the discussion otherwhere in the thread, I assume that `ifconfig -M` is going to start filtering dynamic interfaces so we're safe starting by lladdr early instead of mapping it to a name and putting it into the appropriate dynamic interface list. 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.sub24 Nov 2022 18:49:30 - @@ -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
Re: lladdr support for netstart/hostname.if
Theo de Raadt wrote: > Vitaliy Makkoveev wrote: > > > > On 24 Nov 2022, at 19:20, Theo de Raadt wrote: > > > > > >> I like to exclude pseudo devices. The pseudo device list is immutable, > > >> so we need to get only once during /etc/netstart. > > > > > > Why do we need to excluse them? > > > > > > The users will learn when to use this, and when not to. > > > > > > > So, I can't use hostname.lladdr and usb ethernet cards with vlan? > > Probably not. > > Please answer this question: Did you try the ifconfig change, and the > script, in such a scenario? > > Or are you just musing without trying it? How about this. vlan's and other potential interfaces with this problem have the IFXF_CLONED flag set, and I don't see any IFXF_CLONED we would want a hostname.MAC file to work against. Unfortunately IFXF_CLONED is not visible in an ioctl. But the IFXF_CLONED devices are in ifconfig -C output, so I have abstracted that code into 'get the list' and 'print the list' functions, and reused the 'get the list' function in findmac() so that I can exclude interfaces with that root-name. It is some ugly string mangling code, hope I got it right. My coffee cup is still pretty full... Index: ifconfig.8 === RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v retrieving revision 1.385 diff -u -p -u -r1.385 ifconfig.8 --- ifconfig.8 26 Oct 2022 17:06:31 - 1.385 +++ ifconfig.8 22 Nov 2022 15:04:09 - @@ -40,6 +40,7 @@ .Sh SYNOPSIS .Nm ifconfig .Op Fl AaC +.Op Fl M Ar lladdr .Op Ar interface .Op Ar address_family .Oo @@ -88,6 +89,11 @@ This is the default, if no parameters ar Print the names of all network pseudo-devices that can be created dynamically at runtime using .Nm Cm create . +.It Fl M Ar lladdr +Scan the interface list for the MAC address +.Ar lladdr +and print the name of that interface. +If the MAC address is found on multiple interfaces, print nothing. .It Ar interface The .Ar interface Index: ifconfig.c === RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v retrieving revision 1.457 diff -u -p -u -r1.457 ifconfig.c --- ifconfig.c 26 Oct 2022 17:06:31 - 1.457 +++ ifconfig.c 24 Nov 2022 17:39:40 - @@ -368,6 +368,9 @@ voidwg_status(int); void setignore(const char *, int); #endif +struct if_clonereq *get_cloners(void); +intfindmac(const char *); + /* * Media stuff. Whenever a media command is first performed, the * currently select media is grabbed for this interface. If `media' @@ -795,6 +798,11 @@ main(int argc, char *argv[]) Cflag = 1; nomore = 1; break; + case 'M': + if (argv[1] == NULL) + usage(); + exit (findmac(argv[1])); + break; default: usage(); break; @@ -1255,12 +1263,10 @@ clone_destroy(const char *addr, int para err(1, "SIOCIFDESTROY"); } -void -list_cloners(void) +struct if_clonereq * +get_cloners(void) { - struct if_clonereq ifcr; - char *cp, *buf; - int idx; + static struct if_clonereq ifcr; memset(, 0, sizeof(ifcr)); @@ -1269,12 +1275,9 @@ list_cloners(void) if (ioctl(sock, SIOCIFGCLONERS, ) == -1) err(1, "SIOCIFGCLONERS for count"); - buf = calloc(ifcr.ifcr_total, IFNAMSIZ); - if (buf == NULL) + if ((ifcr.ifcr_buffer = calloc(ifcr.ifcr_total, IFNAMSIZ)) == NULL) err(1, "unable to allocate cloner name buffer"); - ifcr.ifcr_count = ifcr.ifcr_total; - ifcr.ifcr_buffer = buf; if (ioctl(sock, SIOCIFGCLONERS, ) == -1) err(1, "SIOCIFGCLONERS for names"); @@ -1285,17 +1288,30 @@ list_cloners(void) if (ifcr.ifcr_count > ifcr.ifcr_total) ifcr.ifcr_count = ifcr.ifcr_total; - qsort(buf, ifcr.ifcr_count, IFNAMSIZ, + return +} + +void +list_cloners(void) +{ + struct if_clonereq *ifcr; + char *cp, *buf; + int idx; + + ifcr = get_cloners(); + buf = ifcr->ifcr_buffer; + + qsort(buf, ifcr->ifcr_count, IFNAMSIZ, (int(*)(const void *, const void *))strcmp); - for (cp = buf, idx = 0; idx < ifcr.ifcr_count; idx++, cp += IFNAMSIZ) { + for (cp = buf, idx = 0; idx < ifcr->ifcr_count; idx++, cp += IFNAMSIZ) { if (idx > 0) putchar(' '); printf("%s", cp); } putchar('\n'); - free(buf); + free(ifcr->ifcr_buffer); } #define RIDADDR 0 @@ -6614,7 +6630,7 @@ __dead void usage(void) { fprintf(stderr, - "usage: ifconfig [-AaC] [interface]
Re: lladdr support for netstart/hostname.if
How about if ifconfig -M skips devices that are IFXF_CLONED? That lookup might not be atomic (I don't think xflags is in the ifaddrs list), but I don't think atomicity matters. Then the hostname.MAC will always work on the underlying physical device. Or is there another way to identify overlay devices? Does this idea support the script well enough?
Re: lladdr support for netstart/hostname.if
Vitaliy Makkoveev wrote: > > On 24 Nov 2022, at 19:20, Theo de Raadt wrote: > > > >> I like to exclude pseudo devices. The pseudo device list is immutable, > >> so we need to get only once during /etc/netstart. > > > > Why do we need to excluse them? > > > > The users will learn when to use this, and when not to. > > > > So, I can't use hostname.lladdr and usb ethernet cards with vlan? Probably not. Please answer this question: Did you try the ifconfig change, and the script, in such a scenario? Or are you just musing without trying it?
Re: lladdr support for netstart/hostname.if
> On 24 Nov 2022, at 19:20, Theo de Raadt wrote: > >> I like to exclude pseudo devices. The pseudo device list is immutable, >> so we need to get only once during /etc/netstart. > > Why do we need to excluse them? > > The users will learn when to use this, and when not to. > So, I can't use hostname.lladdr and usb ethernet cards with vlan?
Re: lladdr support for netstart/hostname.if
> I like to exclude pseudo devices. The pseudo device list is immutable, > so we need to get only once during /etc/netstart. Why do we need to excluse them? The users will learn when to use this, and when not to.
Re: lladdr support for netstart/hostname.if
On Thu, Nov 24, 2022 at 01:34:30PM +, Stuart Henderson wrote: > On 2022/11/24 14:36, Vitaliy Makkoveev wrote: > > On Wed, Nov 23, 2022 at 09:36:28PM -0700, Theo de Raadt wrote: > > > Theo de Raadt wrote: > > > > > > > > The other, that if both exist, > > > > > /etc/hostname.$if will override /etc/hostname.$lladdr. > > > > > > > > We do need to decide which one is priority, and document that. > > > > > > > > I am still unsure which is better. (I've seen a lot of spurious > > > > comments > > > > from folk, so please think about realistic cases, and don't make stuff > > > > up..) > > > > > > And by that I mean: Actually try andrews's diff. It does it one way (.if > > > is more important). Maybe it needs to be the other way. > > > > > > > Could we check the simultaneous existence of both hostname.if and > > hostname.lladdr corresponding to one interface and print error message > > if so? > > > > I'm also interesting how the following case will be handled. We have > > /etc/hostname.vlan0 and /etc/hostname.08002233ccbb, network devices > > configured as: > > I don't see how this scheme can work with vlans (or any interface which > changes its MAC address, whether that's automatically - vlan, trunk - > randomly - vether, etc - or explicitly with lladdr). > > Another thing with vlans is that, they have a 00:00:00:00:00:00 MAC > address until they're configured, but if you then later re-run netstart, > they then *will* have a MAC address (matching a physical interface), > so netstart's behaviour will be different between running at boot, > and running later. > > That's not really a problem as such, it just limits the scope of where > this functionality could be used. > > (It wouldn't really help vlans anyway - the thing which needs changing > for those is picking which parent interface to use, not picking a > different hostname file for the vlan interface). > I like to exclude pseudo devices. The pseudo device list is immutable, so we need to get only once during /etc/netstart. Also, we could have some physical ethernet devices with the same mac address.
Re: lladdr support for netstart/hostname.if
On 2022/11/24 14:36, Vitaliy Makkoveev wrote: > On Wed, Nov 23, 2022 at 09:36:28PM -0700, Theo de Raadt wrote: > > Theo de Raadt wrote: > > > > > > The other, that if both exist, > > > > /etc/hostname.$if will override /etc/hostname.$lladdr. > > > > > > We do need to decide which one is priority, and document that. > > > > > > I am still unsure which is better. (I've seen a lot of spurious comments > > > from folk, so please think about realistic cases, and don't make stuff > > > up..) > > > > And by that I mean: Actually try andrews's diff. It does it one way (.if > > is more important). Maybe it needs to be the other way. > > > > Could we check the simultaneous existence of both hostname.if and > hostname.lladdr corresponding to one interface and print error message > if so? > > I'm also interesting how the following case will be handled. We have > /etc/hostname.vlan0 and /etc/hostname.08002233ccbb, network devices > configured as: I don't see how this scheme can work with vlans (or any interface which changes its MAC address, whether that's automatically - vlan, trunk - randomly - vether, etc - or explicitly with lladdr). Another thing with vlans is that, they have a 00:00:00:00:00:00 MAC address until they're configured, but if you then later re-run netstart, they then *will* have a MAC address (matching a physical interface), so netstart's behaviour will be different between running at boot, and running later. That's not really a problem as such, it just limits the scope of where this functionality could be used. (It wouldn't really help vlans anyway - the thing which needs changing for those is picking which parent interface to use, not picking a different hostname file for the vlan interface).
Re: lladdr support for netstart/hostname.if
On Wed, Nov 23, 2022 at 09:36:28PM -0700, Theo de Raadt wrote: > Theo de Raadt wrote: > > > > The other, that if both exist, > > > /etc/hostname.$if will override /etc/hostname.$lladdr. > > > > We do need to decide which one is priority, and document that. > > > > I am still unsure which is better. (I've seen a lot of spurious comments > > from folk, so please think about realistic cases, and don't make stuff up..) > > And by that I mean: Actually try andrews's diff. It does it one way (.if > is more important). Maybe it needs to be the other way. > Could we check the simultaneous existence of both hostname.if and hostname.lladdr corresponding to one interface and print error message if so? I'm also interesting how the following case will be handled. We have /etc/hostname.vlan0 and /etc/hostname.08002233ccbb, network devices configured as: vlan0: flags=8002 mtu 1500 lladdr 08:00:23:33:cc:bb index 5 priority 0 llprio 3 encap: vnetid 1 parent em0 txprio packet rxprio outer em0: flags=8843 mtu 1500 lladdr 08:00:23:33:cc:bb and we do /etc/netstart? > By using that script, let's find an actual case where it matters and the > other way is better, and then we discuss that. Maybe it doesn't matter > which way we do it, as long as we document it. >
Re: lladdr support for netstart/hostname.if
Theo de Raadt wrote: > > The other, that if both exist, > > /etc/hostname.$if will override /etc/hostname.$lladdr. > > We do need to decide which one is priority, and document that. > > I am still unsure which is better. (I've seen a lot of spurious comments > from folk, so please think about realistic cases, and don't make stuff up..) And by that I mean: Actually try andrews's diff. It does it one way (.if is more important). Maybe it needs to be the other way. By using that script, let's find an actual case where it matters and the other way is better, and then we discuss that. Maybe it doesn't matter which way we do it, as long as we document it.
Re: lladdr support for netstart/hostname.if
> The other, that if both exist, > /etc/hostname.$if will override /etc/hostname.$lladdr. We do need to decide which one is priority, and document that. I am still unsure which is better. (I've seen a lot of spurious comments from folk, so please think about realistic cases, and don't make stuff up..)
Re: lladdr support for netstart/hostname.if
On Wed, Nov 23, 2022 at 05:28:35PM -0800, Andrew Hewus Fresh wrote: > On Tue, Nov 22, 2022 at 08:09:11AM -0700, Theo de Raadt wrote: > > Florian Obser wrote: > > > ifconfig(8) already knows about these (see -C option). Which made me > > > think, it might be easier to just ask ifconfig(8). > > > > > I've done it as -M > > With `ifconfig -M $lladdr` support, and ignoring the check for both > /etc/hostname.$if and /etc/hostname.$lladdr pointing to the same > interface, this gets pretty simple. > > First, in the case of `netstart $lladdr` we ask `ifconfig -M` to > translate to a $if and if that works, we continue on, using > hostname.$lladdr. > > In the case of netstart without any arguments, it gets a little less > cozy. We have to iterate all hostname.* files, and if they look like > an interface name (as before) we use that directly, and otherwise we > pass the extension as above, hoping it is an lladdr and trusting the > validation there. > > That does mean calling `ifconfig -M` with any random extension we find > on /etc/hostname.*, we found, although we could put back the `LLGLOB` > and do a bit more validation here before calling ifconfig -M. Theo had some suggestions, one obvious one is that we can at least filter for only /etc/hostname.*:* and ignore anything without a :, as it can't be a mac address. The other, that if both exist, /etc/hostname.$if will override /etc/hostname.$lladdr. 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/netstart24 Nov 2022 02:10:51 - @@ -135,6 +135,23 @@ ifstart() { local _if=$1 _hn=/etc/hostname.$1 _cmds _i=0 _line _stat set -A _cmds + # If the _if has a :, assume it is 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
Re: lladdr support for netstart/hostname.if
On Tue, Nov 22, 2022 at 08:09:11AM -0700, Theo de Raadt wrote: > Florian Obser wrote: > > ifconfig(8) already knows about these (see -C option). Which made me > > think, it might be easier to just ask ifconfig(8). > > I've done it as -M With `ifconfig -M $lladdr` support, and ignoring the check for both /etc/hostname.$if and /etc/hostname.$lladdr pointing to the same interface, this gets pretty simple. First, in the case of `netstart $lladdr` we ask `ifconfig -M` to translate to a $if and if that works, we continue on, using hostname.$lladdr. In the case of netstart without any arguments, it gets a little less cozy. We have to iterate all hostname.* files, and if they look like an interface name (as before) we use that directly, and otherwise we pass the extension as above, hoping it is an lladdr and trusting the validation there. That does mean calling `ifconfig -M` with any random extension we find on /etc/hostname.*, we found, although we could put back the `LLGLOB` and do a bit more validation here before calling ifconfig -M. 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/netstart24 Nov 2022 01:20:28 - @@ -135,6 +135,17 @@ ifstart() { local _if=$1 _hn=/etc/hostname.$1 _cmds _i=0 _line _stat set -A _cmds + # If the _if doesn't look like an interface name, + # assume it is an lladdr and look it up. + if [[ $_if != +([[:alpha:]])+([[:digit:]]) ]]; then + if ! _line="$( ifconfig -M "$_if" )"; then + print -u2 "${0##*/}: unique if for lladdr $_if not found." + return + fi + [[ -n "$_line" ]] && _if=$_line + _line= + 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 +194,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.*; 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:]]) ]]; do + # 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
Re: lladdr support for netstart/hostname.if
On Tue, 2022-11-22 at 11:25 +0100, Claudio Jeker wrote: > On Tue, Nov 22, 2022 at 09:25:08AM +, Stuart Henderson wrote: > > Need to query (and set $if, which might be used in route commands etc) I > > think. > > > > I would prefer if people took a step back from configuring interfaces by > MAC address. It feels like a overly specific hack is introduced for > a case that should be handled in a different way. > > Not all interfaces have MAC addresses. E.g. umb(4) would need a > different identifier (most probably the IMEI). Some interfaces inherit > the MAC address from an other interface (vlan, trunk). > > This requires the use of interface groups to 'rename' these interfaces > e.g. as 'green' and 'blue' or 'in' and 'out'. So that you can use these > handles in pf.conf and other commands (rad comes to mind). Not all > commands work with interface groups. route(8) is such an example but there > are more commands needing this. > > Btw. a lot of this can be done already now by using '!' in hostname.if > It wont be pretty but it is also not a common use case. Maybe you're right, but as theo said doing it via !-commands is just horrendous. Best I can think of is letting people write their own script and call it via `!/path/to/script $if`, which is guaranteed going to end messy. Here's a completely other idea that might be more generic and hopefully doesn't cause this same level of confusion. What if instead of adding a new extension we let people do that themselves and add support for including other files? Diff below does this and must still be considered a PoC. And yes I know sed, which isn't allowed. But the command above it does the same thing, so I'm not going to apologise for it in a PoC. One downside in the current parse_hn_line format is that spaces other than a single SP are completely butchered, but I'm not expecting much problems there. In addition to $if which is available for '!'-commands I also added support for $lladdr. $ cat /etc/hostname.em0 . /etc/hostname.$lladdr $ cat /etc/hostname.88\:a4\:c2\:fb\:84\:77 autoconf up $ doas sh ./netstart -n em0 { ifconfig em0 || ifconfig em0 create; } ifconfig em0 autoconf ifconfig em0 up martijn@ Index: netstart === RCS file: /cvs/src/etc/netstart,v retrieving revision 1.229 diff -u -p -r1.229 netstart --- netstart5 Nov 2022 12:06:05 - 1.229 +++ netstart23 Nov 2022 07:22:07 - @@ -35,10 +35,38 @@ stripcom() { done <$_file } +LLGLOB='[[:xdigit:]][[:xdigit:]]:[[:xdigit:]][[:xdigit:]]' +LLGLOB="$LLGLOB:$LLGLOB:$LLGLOB" +set -A LLADDR_MAP -- $( + ifconfig | while IFS= read -- _line; do + if [[ "$_line" = +([[:alpha:]])+([[:digit:]]):* ]]; then + _if=${_line%:*} + elif [[ -n "$_if" +&& "$_line" = +([[:space:]])lladdr\ $LLGLOB ]]; then + print "$_if,${_line#*lladdr }" + _if= + fi + done +) + +# Find lladdr for if +# Usage: if2lladdr if1 +if2lladdr() { + local _if=$1 + for m in "${LLADDR_MAP[@]}"; do + if [[ "$_if" = "${m%,*}" ]]; then + print -- "${m#*,}" + break + fi + done + return 0 +} + # Parse and "unpack" a hostname.if(5) line given as positional parameters. # Fill the _cmds array with the resulting interface configuration commands. parse_hn_line() { - local _af=0 _name=1 _mask=2 _bc=3 _prefix=2 _c _cmd _prev _daddr _dhcp _i + local _af=0 _name=1 _mask=2 _bc=3 _prefix=2 _c _cmd _prev _daddr _dhcp + local _i _file _line set -A _c -- "$@" set -o noglob @@ -84,6 +112,17 @@ parse_hn_line() { ;; '!'*) _cmd=$(print -- "${_c[@]}" | sed 's/\$if/'$_if'/g') _cmds[${#_cmds[*]}]="${_cmd#!}" + ;; + '.')unset _c[_af] + _file="$(print -- "${_c[@]}" | sed -e 's/\$if/'$_if'/g' \ + -e 's/\$lladdr/'$(if2lladdr $_if)'/g')" + if [[ ! -f $_file ]]; then + print -u2 "${0##*/}: $_file: No such file or directory." + return + fi + while IFS= read -- _line; do + parse_hn_line $_line + done<"$_file" ;; *) _cmds[${#_cmds[*]}]="ifconfig $_if ${_c[@]}" ;;
Re: lladdr support for netstart/hostname.if
On Tue, Nov 22, 2022 at 03:37:20PM +, Klemens Nanni wrote: > On Tue, Nov 22, 2022 at 08:09:11AM -0700, Theo de Raadt wrote: > > Florian Obser wrote: > > > ifconfig(8) already knows about these (see -C option). Which made me > > > think, it might be easier to just ask ifconfig(8). > > > > > > $ ifconfig -Q 00:80:41:7b:f3:c3 > > > vio0 > > > > > > Would that be helpful? > > > > I'm unsure about the rest of your proposal, where MAC works in place if > > the IF argument. Let's say we do this in ifcconfig. Do we do it in route? > > Or ten other commands? I think that is the wrong way to go. > > > > But this first idea is valid. We've now seen 3 monster functions trying to > > do this task of convering MAC to IF in shell. Here's code to do it in > > ifconfig. > > > > I've done it as -M > > Better than [Q]uery. > > > > > It fails if the same MAC is on multiple interfaces. Go back to using > > hostname.if# files, you heathen. > > This reads like a viable approach, much cleaner than the netstart > globbing attempts. > > Using ifconfig -M, I can give the shell bits a try later this week. I agree that offloading mapping of the unique identifier to ifconfig makes the shell bits easier. We currently need to be able to go both directions, mostly to check existence of /etc/hostname.ure0 and /etc/hostname.$lladdr when when trying to netstart ure0 so we can complain that there are two configurations for the same interface. We can, of course, use the existing ifconfig parsing to look up the "lladdr ...". Maybe this is a case of letting folks shoot themselves in the foot. I do think that referring to it as -M for MAC lookup makes me expect it to return all interface names with that mac, not error if there is more than one. Unlike it it referred to a "unique identifier" that might be a mac or maybe IMEI. I could be wrong, but it might make swapping umb devices from different providers easer.
Re: lladdr support for netstart/hostname.if
On Tue, Nov 22, 2022 at 08:09:11AM -0700, Theo de Raadt wrote: > Florian Obser wrote: > > > On 2022-11-22 18:06 +10, David Gwynne wrote: > > > > > > There are a few things to keep in mind if we're going to use lladdrs like > > > this. > > > > > > vlan interfaces start with their lladdr as 00:00:00:00:00:00 and then > > > assume the lladdr of the parent interface when that is configured. > > > > > > Clonable devices (eg, egre, vport, aggr, etc) tend to generate random > > > lladdrs when they're created. If you want a consistent lladdr for > > > these you configure that in /etc/hostname.vportX or whatever you're > > > using. > > > > ifconfig(8) already knows about these (see -C option). Which made me > > think, it might be easier to just ask ifconfig(8). > > > > $ ifconfig -Q 00:80:41:7b:f3:c3 > > vio0 > > > > Would that be helpful? > > I'm unsure about the rest of your proposal, where MAC works in place if > the IF argument. Let's say we do this in ifcconfig. Do we do it in route? > Or ten other commands? I think that is the wrong way to go. > > But this first idea is valid. We've now seen 3 monster functions trying to > do this task of convering MAC to IF in shell. Here's code to do it in > ifconfig. > > I've done it as -M Better than [Q]uery. > > It fails if the same MAC is on multiple interfaces. Go back to using > hostname.if# files, you heathen. This reads like a viable approach, much cleaner than the netstart globbing attempts. Using ifconfig -M, I can give the shell bits a try later this week. Nits inline. > > Index: ifconfig.8 > === > RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v > retrieving revision 1.385 > diff -u -p -u -r1.385 ifconfig.8 > --- ifconfig.826 Oct 2022 17:06:31 - 1.385 > +++ ifconfig.822 Nov 2022 15:04:09 - > @@ -40,6 +40,7 @@ > .Sh SYNOPSIS > .Nm ifconfig > .Op Fl AaC > +.Op Fl M Ar lladdr > .Op Ar interface > .Op Ar address_family > .Oo > @@ -88,6 +89,11 @@ This is the default, if no parameters ar > Print the names of all network pseudo-devices that > can be created dynamically at runtime using > .Nm Cm create . > +.It Fl M Ar lladdr > +Scan the interface list for the MAC address > +.Ar lladdr > +and print the name of that interface. > +If the MAC address is found on multiple interfaces, print nothing. > .It Ar interface > The > .Ar interface > Index: ifconfig.c > === > RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v > retrieving revision 1.457 > diff -u -p -u -r1.457 ifconfig.c > --- ifconfig.c26 Oct 2022 17:06:31 - 1.457 > +++ ifconfig.c22 Nov 2022 15:02:20 - > @@ -368,6 +368,8 @@ void wg_status(int); > void setignore(const char *, int); > #endif > > +void findmac(char *); > + > /* > * Media stuff. Whenever a media command is first performed, the > * currently select media is grabbed for this interface. If `media' > @@ -759,6 +761,7 @@ main(int argc, char *argv[]) > int create = 0; > int Cflag = 0; > int gflag = 0; > + int Mflag = 0; > int found_rulefile = 0; > int i; > > @@ -795,6 +798,12 @@ main(int argc, char *argv[]) > Cflag = 1; > nomore = 1; > break; > + case 'M': > + if (argv[1] == NULL) > + usage(); > + findmac(argv[1]); > + exit(1); Above main() already uses return, no need for exit here, I think: return findmac(argv[1]); > + break; > default: > usage(); > break; > @@ -6614,7 +6623,7 @@ __dead void > usage(void) > { > fprintf(stderr, > - "usage: ifconfig [-AaC] [interface] [address_family] " > + "usage: ifconfig [-AaC] [-M lladdr] [interface] [address_family] " > "[address [dest_address]]\n" > "\t\t[parameters]\n"); > exit(1); > @@ -6782,3 +6791,30 @@ setignore(const char *id, int param) > /* just digest the command */ > } > #endif > + > +void > +findmac(char *mac) > +{ > + struct ifaddrs *ifap, *ifa; > + char *ifnam = NULL; Both *mac and *ifnam can be const. > + > + if (getifaddrs() != 0) > + err(1, "getifaddrs"); > + > + for (ifa = ifap; ifa; ifa = ifa->ifa_next) { > + struct sockaddr_dl *sdl = (struct sockaddr_dl *)ifa->ifa_addr; > + > + if (sdl != NULL && sdl->sdl_alen && > + (sdl->sdl_type == IFT_ETHER || sdl->sdl_type == IFT_CARP)) { > + if (strcmp(ether_ntoa((struct ether_addr *)LLADDR(sdl)), > + mac) == 0) { > + if (ifnam) /*
Re: lladdr support for netstart/hostname.if
Florian Obser wrote: > On 2022-11-22 18:06 +10, David Gwynne wrote: > > > > There are a few things to keep in mind if we're going to use lladdrs like > > this. > > > > vlan interfaces start with their lladdr as 00:00:00:00:00:00 and then > > assume the lladdr of the parent interface when that is configured. > > > > Clonable devices (eg, egre, vport, aggr, etc) tend to generate random > > lladdrs when they're created. If you want a consistent lladdr for > > these you configure that in /etc/hostname.vportX or whatever you're > > using. > > ifconfig(8) already knows about these (see -C option). Which made me > think, it might be easier to just ask ifconfig(8). > > $ ifconfig -Q 00:80:41:7b:f3:c3 > vio0 > > Would that be helpful? I'm unsure about the rest of your proposal, where MAC works in place if the IF argument. Let's say we do this in ifcconfig. Do we do it in route? Or ten other commands? I think that is the wrong way to go. But this first idea is valid. We've now seen 3 monster functions trying to do this task of convering MAC to IF in shell. Here's code to do it in ifconfig. I've done it as -M It fails if the same MAC is on multiple interfaces. Go back to using hostname.if# files, you heathen. Index: ifconfig.8 === RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v retrieving revision 1.385 diff -u -p -u -r1.385 ifconfig.8 --- ifconfig.8 26 Oct 2022 17:06:31 - 1.385 +++ ifconfig.8 22 Nov 2022 15:04:09 - @@ -40,6 +40,7 @@ .Sh SYNOPSIS .Nm ifconfig .Op Fl AaC +.Op Fl M Ar lladdr .Op Ar interface .Op Ar address_family .Oo @@ -88,6 +89,11 @@ This is the default, if no parameters ar Print the names of all network pseudo-devices that can be created dynamically at runtime using .Nm Cm create . +.It Fl M Ar lladdr +Scan the interface list for the MAC address +.Ar lladdr +and print the name of that interface. +If the MAC address is found on multiple interfaces, print nothing. .It Ar interface The .Ar interface Index: ifconfig.c === RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v retrieving revision 1.457 diff -u -p -u -r1.457 ifconfig.c --- ifconfig.c 26 Oct 2022 17:06:31 - 1.457 +++ ifconfig.c 22 Nov 2022 15:02:20 - @@ -368,6 +368,8 @@ voidwg_status(int); void setignore(const char *, int); #endif +void findmac(char *); + /* * Media stuff. Whenever a media command is first performed, the * currently select media is grabbed for this interface. If `media' @@ -759,6 +761,7 @@ main(int argc, char *argv[]) int create = 0; int Cflag = 0; int gflag = 0; + int Mflag = 0; int found_rulefile = 0; int i; @@ -795,6 +798,12 @@ main(int argc, char *argv[]) Cflag = 1; nomore = 1; break; + case 'M': + if (argv[1] == NULL) + usage(); + findmac(argv[1]); + exit(1); + break; default: usage(); break; @@ -6614,7 +6623,7 @@ __dead void usage(void) { fprintf(stderr, - "usage: ifconfig [-AaC] [interface] [address_family] " + "usage: ifconfig [-AaC] [-M lladdr] [interface] [address_family] " "[address [dest_address]]\n" "\t\t[parameters]\n"); exit(1); @@ -6782,3 +6791,30 @@ setignore(const char *id, int param) /* just digest the command */ } #endif + +void +findmac(char *mac) +{ + struct ifaddrs *ifap, *ifa; + char *ifnam = NULL; + + if (getifaddrs() != 0) + err(1, "getifaddrs"); + + for (ifa = ifap; ifa; ifa = ifa->ifa_next) { + struct sockaddr_dl *sdl = (struct sockaddr_dl *)ifa->ifa_addr; + + if (sdl != NULL && sdl->sdl_alen && + (sdl->sdl_type == IFT_ETHER || sdl->sdl_type == IFT_CARP)) { + if (strcmp(ether_ntoa((struct ether_addr *)LLADDR(sdl)), + mac) == 0) { + if (ifnam) /* same MAC on multiple ifp */ + exit(1); + ifnam = ifa->ifa_name; + } + } + } + if (ifnam) + printf("%s\n", ifnam); + exit(0); +}
Re: lladdr support for netstart/hostname.if
On Tue, Nov 22, 2022 at 06:28:31AM -0700, Theo de Raadt wrote: > Vitaliy Makkoveev wrote: > > > On Tue, Nov 22, 2022 at 11:25:55AM +0100, Claudio Jeker wrote: > > > On Tue, Nov 22, 2022 at 09:25:08AM +, Stuart Henderson wrote: > > > > Need to query (and set $if, which might be used in route commands etc) > > > > I think. > > > > > > > > > > I would prefer if people took a step back from configuring interfaces by > > > MAC address. It feels like a overly specific hack is introduced for > > > a case that should be handled in a different way. > > > > > > > [skip] > > > > > > > > Btw. a lot of this can be done already now by using '!' in hostname.if > > > It wont be pretty but it is also not a common use case. > > > > Agree. > > > Come on, that's just some incomplete claim. > > I do not think you have tried this. > > Try it. I have actually tried it. The results were so incredibly > disgusting that I gave up and used another machine. > As dlg@ pointed, we have collisions with clonable devices, which inherit lladdr or could generate already existing lladdr. We need to handle this, for example exclude clobable devices from hostname.lladdr logic.
Re: lladdr support for netstart/hostname.if
Vitaliy Makkoveev wrote: > As dlg@ pointed, we have collisions with clonable devices, which inherit > lladdr or could generate already existing lladdr. We need to handle > this, for example exclude clobable devices from hostname.lladdr logic. Noone is proposing deleting the existing support for 'hostname.if'. hostname.mac will be an addition to identify SOME network interfaces. Noone is suggesting that users use only hostname.MAC Specific exclusions could be handled, for instance rejecting 00:00:00:00:00:00 only working on the first or last device with a specific MAC, or rejecting operation if a MAC exists multiple times --> and thus forcing people back to using hostname.IF for those cases But if no such tests were added (to the already bloated) /etc/netstart, the 'number of people harmed' will probably be zero.
Re: lladdr support for netstart/hostname.if
Claudio Jeker wrote: > On Tue, Nov 22, 2022 at 09:25:08AM +, Stuart Henderson wrote: > > Need to query (and set $if, which might be used in route commands etc) I > > think. > > > > I would prefer if people took a step back from configuring interfaces by > MAC address. It feels like a overly specific hack is introduced for > a case that should be handled in a different way. > > Not all interfaces have MAC addresses. E.g. umb(4) would need a > different identifier (most probably the IMEI). Some interfaces inherit > the MAC address from an other interface (vlan, trunk). > > This requires the use of interface groups to 'rename' these interfaces > e.g. as 'green' and 'blue' or 'in' and 'out'. So that you can use these > handles in pf.conf and other commands (rad comes to mind). Not all > commands work with interface groups. route(8) is such an example but there > are more commands needing this. The point of hostname.MAC is you'll be able to 1) set the address 2) set the group and then pf.conf will use the group But that's no different than doing this in a hostname.IF file! We've been encouraging people to use the group egress, as well as predefined groups since they were invented!. In this case, people will start to use those more, but not because it is a MAC, but because the IF name is unstable. They can establish a stable group name, if they know what interface-instance to assign a group to. But otherwise, using groups to identify the specific interface position is completely unrelated to setting the configuration on these interfaces in the first place. Claudio, you really should *SHOW* a working configuration using !, if you believe in it so much. And do it without a helper shell script, because where are you going to slap that, into /etc, come on. What I came up before had 3 lines of ! command that were 70-90 characters long, and each line had to to run ifconfig to COMPARE AT THE MAC, and then run ifconfig to set a configuration; another example did the MAC and then ran ifconfig multipel times but that single line wrapped the screen multiple times. It was a demo of how rough this problem is. So maybe, show how you do ! commands in such files, so that everyone can see it is ugly as sin and the wrong thing.
Re: lladdr support for netstart/hostname.if (was: Re: Locking network card configuration)
Miod Vallat wrote: > I'm a bit late to the thread, but whatever its outcome, things have to > work correctly on older sparc64 hardware, where the default behaviour > for on-board and Sun-branded expansion card interfaces is to use the > same MAC address. > > This hints that hostname. should have priority over > hoshname. for the latter will be ambiguous on these > systems. Don't use hostname.MAC in that case. Noone is proposing removing hostname.IF0 support. Just like noone is proposing deleting sed because awk can do the job.
Re: lladdr support for netstart/hostname.if
Vitaliy Makkoveev wrote: > On Tue, Nov 22, 2022 at 11:25:55AM +0100, Claudio Jeker wrote: > > On Tue, Nov 22, 2022 at 09:25:08AM +, Stuart Henderson wrote: > > > Need to query (and set $if, which might be used in route commands etc) I > > > think. > > > > > > > I would prefer if people took a step back from configuring interfaces by > > MAC address. It feels like a overly specific hack is introduced for > > a case that should be handled in a different way. > > > > [skip] > > > > > Btw. a lot of this can be done already now by using '!' in hostname.if > > It wont be pretty but it is also not a common use case. > > Agree. Come on, that's just some incomplete claim. I do not think you have tried this. Try it. I have actually tried it. The results were so incredibly disgusting that I gave up and used another machine.
Re: lladdr support for netstart/hostname.if
Claudio Jeker wrote: > Not all interfaces have MAC addresses. E.g. umb(4) would need a > different identifier (most probably the IMEI). Some interfaces inherit > the MAC address from an other interface (vlan, trunk). Then don't use hostname.MAC for those interfaces. The other mechanism will remain. And if that is the case, what is the fuss? There is no other solution available for interfaces that attach out of order. Any attempt to solve this in subr_config.c to provide stable device names / interface names is going to be a bigger mess.
Re: lladdr support for netstart/hostname.if (was: Re: Locking network card configuration)
I'm a bit late to the thread, but whatever its outcome, things have to work correctly on older sparc64 hardware, where the default behaviour for on-board and Sun-branded expansion card interfaces is to use the same MAC address. This hints that hostname. should have priority over hoshname. for the latter will be ambiguous on these systems.
Re: lladdr support for netstart/hostname.if
On Tue, Nov 22, 2022 at 11:25:55AM +0100, Claudio Jeker wrote: > On Tue, Nov 22, 2022 at 09:25:08AM +, Stuart Henderson wrote: > > Need to query (and set $if, which might be used in route commands etc) I > > think. > > > > I would prefer if people took a step back from configuring interfaces by > MAC address. It feels like a overly specific hack is introduced for > a case that should be handled in a different way. > [skip] > > Btw. a lot of this can be done already now by using '!' in hostname.if > It wont be pretty but it is also not a common use case. Agree.
Re: lladdr support for netstart/hostname.if
On Tue, Nov 22, 2022 at 09:25:08AM +, Stuart Henderson wrote: > Need to query (and set $if, which might be used in route commands etc) I > think. > I would prefer if people took a step back from configuring interfaces by MAC address. It feels like a overly specific hack is introduced for a case that should be handled in a different way. Not all interfaces have MAC addresses. E.g. umb(4) would need a different identifier (most probably the IMEI). Some interfaces inherit the MAC address from an other interface (vlan, trunk). This requires the use of interface groups to 'rename' these interfaces e.g. as 'green' and 'blue' or 'in' and 'out'. So that you can use these handles in pf.conf and other commands (rad comes to mind). Not all commands work with interface groups. route(8) is such an example but there are more commands needing this. Btw. a lot of this can be done already now by using '!' in hostname.if It wont be pretty but it is also not a common use case. -- :wq Claudio > On 22 November 2022 08:37:05 Florian Obser wrote: > > > On 2022-11-22 18:06 +10, David Gwynne wrote: > > > > > > There are a few things to keep in mind if we're going to use lladdrs like > > > this. > > > > > > vlan interfaces start with their lladdr as 00:00:00:00:00:00 and > > > then assume the lladdr of the parent interface when that is > > > configured. > > > > > > Clonable devices (eg, egre, vport, aggr, etc) tend to generate random > > > lladdrs when they're created. If you want a consistent lladdr for > > > these you configure that in /etc/hostname.vportX or whatever you're > > > using. > > > > ifconfig(8) already knows about these (see -C option). Which made me > > think, it might be easier to just ask ifconfig(8). > > > > $ ifconfig -Q 00:80:41:7b:f3:c3 > > vio0 > > > > Would that be helpful? > > > > Or would you need > > > > # ifconfig 00:80:41:7b:f3:c3 inet 192.0.2.2/24 > > > > to work? > > > > I think you want to query,no? > > > > > > > > "Platform deficient" systems like arm SBCs don't always do a good job > > > of providing lladdrs for their ethernet interfaces. I'm working on one > > > now that has rge(4) and it comes up with an lladdr of > > > 00:00:00:00:00:00. I have another one where the drivers fall back to > > > randomly generating an lladdr if none is set by u-boot/edk/etc. > > > > > > I don't think any of these are showstoppers, but do need to be considered. > > > > > > > -- > > I'm not entirely sure you are real. >
Re: lladdr support for netstart/hostname.if
Need to query (and set $if, which might be used in route commands etc) I think. -- Sent from a phone, apologies for poor formatting. On 22 November 2022 08:37:05 Florian Obser wrote: On 2022-11-22 18:06 +10, David Gwynne wrote: There are a few things to keep in mind if we're going to use lladdrs like this. vlan interfaces start with their lladdr as 00:00:00:00:00:00 and then assume the lladdr of the parent interface when that is configured. Clonable devices (eg, egre, vport, aggr, etc) tend to generate random lladdrs when they're created. If you want a consistent lladdr for these you configure that in /etc/hostname.vportX or whatever you're using. ifconfig(8) already knows about these (see -C option). Which made me think, it might be easier to just ask ifconfig(8). $ ifconfig -Q 00:80:41:7b:f3:c3 vio0 Would that be helpful? Or would you need # ifconfig 00:80:41:7b:f3:c3 inet 192.0.2.2/24 to work? I think you want to query,no? "Platform deficient" systems like arm SBCs don't always do a good job of providing lladdrs for their ethernet interfaces. I'm working on one now that has rge(4) and it comes up with an lladdr of 00:00:00:00:00:00. I have another one where the drivers fall back to randomly generating an lladdr if none is set by u-boot/edk/etc. I don't think any of these are showstoppers, but do need to be considered. -- I'm not entirely sure you are real.
Re: lladdr support for netstart/hostname.if
On 2022-11-22 18:06 +10, David Gwynne wrote: > > There are a few things to keep in mind if we're going to use lladdrs like > this. > > vlan interfaces start with their lladdr as 00:00:00:00:00:00 and then assume > the lladdr of the parent interface when that is configured. > > Clonable devices (eg, egre, vport, aggr, etc) tend to generate random > lladdrs when they're created. If you want a consistent lladdr for > these you configure that in /etc/hostname.vportX or whatever you're > using. ifconfig(8) already knows about these (see -C option). Which made me think, it might be easier to just ask ifconfig(8). $ ifconfig -Q 00:80:41:7b:f3:c3 vio0 Would that be helpful? Or would you need # ifconfig 00:80:41:7b:f3:c3 inet 192.0.2.2/24 to work? I think you want to query,no? > > "Platform deficient" systems like arm SBCs don't always do a good job > of providing lladdrs for their ethernet interfaces. I'm working on one > now that has rge(4) and it comes up with an lladdr of > 00:00:00:00:00:00. I have another one where the drivers fall back to > randomly generating an lladdr if none is set by u-boot/edk/etc. > > I don't think any of these are showstoppers, but do need to be considered. > -- I'm not entirely sure you are real.
Re: lladdr support for netstart/hostname.if (was: Re: Locking network card configuration)
> On 22 Nov 2022, at 16:13, Andrew Hewus Fresh wrote: > > On Mon, Nov 21, 2022 at 04:56:07PM +0100, Martijn van Duren wrote: >> On Sun, 2022-11-20 at 19:35 -0700, Theo de Raadt wrote: >>> Steve Litt wrote: >>> Vitaliy Makkoveev said on Mon, 21 Nov 2022 03:48:21 +0300 >> On 20 Nov 2022, at 18:06, Odd Martin Baanrud >> wrote: >> >> Hello, >> >> I have a Raspberry Pi 4 with 2 USB NIC’s attached. >> One via USB3 (ure0), and the other via USB2 (ure1). >> Since they are connected to different USB interfaces, I thaught they >> would get configured the same way on reboot. But that’s not the case. >> They became swapped on reboot. >> Is there a way to “lock” the configuration I want? >> So the USB3 NIC always become ure0, and the USB2 ure1. >> >> Regards, Martin >> > > You could parse ifconfig(8) output to determine which names network > interfaces received. But unfortunately, you can’t rename interfaces. During your parsing you could assign each one to an environment variable such that, for instance, $lan contains the network card name of the LAN one, and $wan contains the network name of the one going to the Internet. Unfortunately, this would probably mean changing a lot of existing shellscripts, but it's doable. >>> >>> But that is not the problem. >>> >>> hostname.* installs addresses on an interface, based upon the name of that >>> interface. >>> >>> So it is too late for what you suggest. >>> >>> Unless the suggestion is have each hostname.* do a !command to a script >>> which >>> does the assigning. That is pretty crazy. >>> >>> pf.conf is not the problem either, because that can be entirely written >>> using >>> egress and groups. >>> >>> >>> >>> There is a problem with device attachment -> naming a device at that >>> moment -> using that name in netstart.. but I am not sure how we could >>> solve this without creating bigger problems for everyone else in the >>> other non-hot-plug configurations, which is the majority of users with 1 network device. >>> >>> We also hit this problem with disks, and we worked around it with the >>> DUID subsystem. >>> >>> >>> I suppose there is some argument that we should support hostname.MAC >>> files >>> >> I don't have a usecase for this myself, but it seemed like a nice >> exercise and might get the ball rolling. I also don't have much >> experience with our rc/netstart shellcode, so I'm expecting this diff >> should be taken as a starting-point iff we want something like this. >> >> I've chosen to error out on missing lladdr, duplicate lladdr and when >> there's a hostname.if for both the lladdr and the if variant. This means >> that there's smaller chance for order confusion or doubly applied >> configs. Downside is that if someone decided to backup their hostname.if >> to hostname.lladdr that will break their setup. However, I don't expect >> people to backup their config files in this manner, but you never know. >> >> Errors: >> On duplicate lladdr (in this case em0 and iwx0 in trunk0): >> $ doas sh /usr/src/etc/netstart 88:a4:c2:fb:84:77 >> netstart: /etc/hostname.88:a4:c2:fb:84:77: unique if for lladdr not found. >> >> On missing lladdr: >> $ doas sh /usr/src/etc/netstart 88:a4:c2:fb:84:76 >> netstart: /etc/hostname.88:a4:c2:fb:84:76: unique if for lladdr not found. >> >> And having both hostname.if and hostname.lladdr installed: >> $ doas sh ./netstart 00:11:22:33:44:55 >> netstart: /etc/hostname.00:11:22:33:44:55: duplicate config found in >> /etc/hostname.vio0. >> $ doas sh ./netstart vio0 >> netstart: /etc/hostname.vio0: duplicate config found in >> /etc/hostname.00:11:22:33:44:55. >> >> Two omissions I considered but didn't implement: >> 1) I didn't test if the lladdr is owned by one of `ifconfig -C` >> interfaces. Not sure if this is an upside or downside. >> 2) Allowing /etc/netstart if1 and parsing the hostname.lladdr1 and vice >> versa. > > > I got interested in this, and looked at it a bit. My diff is also a bit > preliminary, but a couple of things. > > First, I only parse ifconfig output once and save the LLADDR_MAP to look > up later. This makes the lookup functions a bit simpler. Also, the > glob now uses xdigit, which seems more correct, unless there's something > I am missing about mac addresses. > > I also thought the error message for `netstart $lladdr` when > /etc/hostname.$lladdr doesn't exist, but /etc/hostname.$if does was poor > (it claimed duplicate configs which wasn't true) so I thought the > easiest solution was to implement your #2 there and allow it to start > the $if when you specify the $lladdr. > > Unfortunately, I then looked at the clock and realized it's time for > bed, but I figured you might be interested in another take, even if it's > probably incomplete. In any case, tomorrow is dinner with friends, so > it will be Wednesday before I again have
Re: lladdr support for netstart/hostname.if (was: Re: Locking network card configuration)
On Mon, Nov 21, 2022 at 04:56:07PM +0100, Martijn van Duren wrote: > On Sun, 2022-11-20 at 19:35 -0700, Theo de Raadt wrote: > > Steve Litt wrote: > > > > > Vitaliy Makkoveev said on Mon, 21 Nov 2022 03:48:21 +0300 > > > > > > > > On 20 Nov 2022, at 18:06, Odd Martin Baanrud > > > > > wrote: > > > > > > > > > > Hello, > > > > > > > > > > I have a Raspberry Pi 4 with 2 USB NIC’s attached. > > > > > One via USB3 (ure0), and the other via USB2 (ure1). > > > > > Since they are connected to different USB interfaces, I thaught they > > > > > would get configured the same way on reboot. But that’s not the case. > > > > > They became swapped on reboot. > > > > > Is there a way to “lock” the configuration I want? > > > > > So the USB3 NIC always become ure0, and the USB2 ure1. > > > > > > > > > > Regards, Martin > > > > > > > > > > > > > You could parse ifconfig(8) output to determine which names network > > > > interfaces received. But unfortunately, you can’t rename interfaces. > > > > > > During your parsing you could assign each one to an environment > > > variable such that, for instance, $lan contains the network card name > > > of the LAN one, and $wan contains the network name of the one going to > > > the Internet. Unfortunately, this would probably mean changing a lot of > > > existing shellscripts, but it's doable. > > > > But that is not the problem. > > > > hostname.* installs addresses on an interface, based upon the name of that > > interface. > > > > So it is too late for what you suggest. > > > > Unless the suggestion is have each hostname.* do a !command to a script > > which > > does the assigning. That is pretty crazy. > > > > pf.conf is not the problem either, because that can be entirely written > > using > > egress and groups. > > > > > > > > There is a problem with device attachment -> naming a device at that > > moment -> using that name in netstart.. but I am not sure how we could > > solve this without creating bigger problems for everyone else in the > > other non-hot-plug configurations, which is the majority of users with > > > 1 network device. > > > > We also hit this problem with disks, and we worked around it with the > > DUID subsystem. > > > > > > I suppose there is some argument that we should support hostname.MAC > > files > > > I don't have a usecase for this myself, but it seemed like a nice > exercise and might get the ball rolling. I also don't have much > experience with our rc/netstart shellcode, so I'm expecting this diff > should be taken as a starting-point iff we want something like this. > > I've chosen to error out on missing lladdr, duplicate lladdr and when > there's a hostname.if for both the lladdr and the if variant. This means > that there's smaller chance for order confusion or doubly applied > configs. Downside is that if someone decided to backup their hostname.if > to hostname.lladdr that will break their setup. However, I don't expect > people to backup their config files in this manner, but you never know. > > Errors: > On duplicate lladdr (in this case em0 and iwx0 in trunk0): > $ doas sh /usr/src/etc/netstart 88:a4:c2:fb:84:77 > netstart: /etc/hostname.88:a4:c2:fb:84:77: unique if for lladdr not found. > > On missing lladdr: > $ doas sh /usr/src/etc/netstart 88:a4:c2:fb:84:76 > netstart: /etc/hostname.88:a4:c2:fb:84:76: unique if for lladdr not found. > > And having both hostname.if and hostname.lladdr installed: > $ doas sh ./netstart 00:11:22:33:44:55 > netstart: /etc/hostname.00:11:22:33:44:55: duplicate config found in > /etc/hostname.vio0. > $ doas sh ./netstart vio0 > netstart: /etc/hostname.vio0: duplicate config found in > /etc/hostname.00:11:22:33:44:55. > > Two omissions I considered but didn't implement: > 1) I didn't test if the lladdr is owned by one of `ifconfig -C` >interfaces. Not sure if this is an upside or downside. > 2) Allowing /etc/netstart if1 and parsing the hostname.lladdr1 and vice >versa. I got interested in this, and looked at it a bit. My diff is also a bit preliminary, but a couple of things. First, I only parse ifconfig output once and save the LLADDR_MAP to look up later. This makes the lookup functions a bit simpler. Also, the glob now uses xdigit, which seems more correct, unless there's something I am missing about mac addresses. I also thought the error message for `netstart $lladdr` when /etc/hostname.$lladdr doesn't exist, but /etc/hostname.$if does was poor (it claimed duplicate configs which wasn't true) so I thought the easiest solution was to implement your #2 there and allow it to start the $if when you specify the $lladdr. Unfortunately, I then looked at the clock and realized it's time for bed, but I figured you might be interested in another take, even if it's probably incomplete. In any case, tomorrow is dinner with friends, so it will be Wednesday before I again have a chance to think on this. Index: etc/netstart