On Sat, Jul 02, 2022 at 08:12:29PM +0000, Klemens Nanni wrote:
> On Sat, Jul 02, 2022 at 03:00:00PM +0200, Alexander Hall wrote:
> > On Thu, Jun 30, 2022 at 03:35:05PM +0000, Klemens Nanni wrote:
> > > On Tue, Dec 07, 2021 at 08:15:41PM +0000, Klemens Nanni wrote:
> > > > On Tue, Nov 23, 2021 at 01:17:14AM +0000, Klemens Nanni wrote:
> > > > > On Tue, Nov 16, 2021 at 11:09:40PM +0000, Klemens Nanni wrote:
> > > > > > Run on boot without arguments, netstart(8) creates all virtual
> > > > > > interfaces *for which hostname.if files exist* before configuring
> > > > > > them.
> > > > > >
> > > > > > This prevents ordering problems with bridges and its members, as
> > > > > > dlg's
> > > > > > commit message from 2018 reminds us.
> > > > > >
> > > > > > But it also helps interface types like pair(4) which pair one
> > > > > > another
> > > > > > in whatever way the user says:
> > > > > >
> > > > > > $ cat /etc/hostname.pair1
> > > > > > patch pair2
> > > > > > $ cat /etc/hostname.pair2
> > > > > > rdomain 1
> > > > > >
> > > > > > On boot this works, but `sh /etc/netstart pair1 pair2' won't work
> > > > > > because pair2 does not exist a creation time of pair1 because
> > > > > > netstart
> > > > > > does not create virtual interfaces upfront.
> > > > > >
> > > > > > I just hit this exact use case when setting up gelatod(8) (see
> > > > > > ports@).
> > > > > >
> > > > > > To fix this, pass the list of interfaces to vifscreate() and make it
> > > > > > create only those iff given.
> > > > > >
> > > > > > Regular boot, i.e. `sh /etc/netstart', stays uneffected by this and
> > > > > > selective runs as shown work as expected without requring users to
> > > > > > know
> > > > > > the order in which netstart creates/configures interfaces.
> > > > > >
> > > > > > The installer's internal version of netstart doesn't need this at
> > > > > > all;
> > > > > > neither does it have the selective semantic nor does vifscreate()
> > > > > > exist.
> > > > >
> > > > > Anyone?
> > > > >
> > > > > It seems only logical to treat subsets of interfaces the same way as
> > > > > a full `sh /etc/netstart'.
> > > > >
> > > > > A pair of pair(4) is one example, I'm certain there are more scenarios
> > > > > where you craft interfaces with `ifconfig ...' in the shell, then set
> > > > > up
> > > > > the hostname.* files and test them with `sh /etc/netstart bridge0 ...'
> > > > > where pseudo interfaces are involved.
> > > >
> > > > Anyone?
> > > >
> > > > This is really practical and fixes things at least for me when I destroy
> > > > interfaces, reconfigure and recreate them together, for example like so:
> > > >
> > > > # ifconfig pair2 destroy
> > > > # ifconfig pair1 destroy
> > > > ... edit hostname.*
> > > > # sh /etc/netstart pair1 pair2
> > > > ifconfig: patch pair2: No such file or directory
> > > > add net default: gateway 192.0.0.1
> > > >
> > > > (redoing it because who knows what failed due to the order problem and
> > > > what didn't...)
> > > >
> > > > # ifconfig pair2 destroy
> > > > # ifconfig pair1 destroy
> > > > # sh /usr/src/etc/netstart pair1 pair2
> > > > add net default: gateway 192.0.0.1
> > > >
> > > > Feedback? Objection? OK?
> > >
> > > One last ping with the same diff on top of -CURRENT.
> > >
> > >
> > > Index: etc/netstart
> > > ===================================================================
> > > RCS file: /cvs/src/etc/netstart,v
> > > retrieving revision 1.218
> > > diff -u -p -r1.218 netstart
> > > --- etc/netstart 26 Jun 2022 09:36:13 -0000 1.218
> > > +++ etc/netstart 30 Jun 2022 14:48:46 -0000
> > > @@ -94,9 +94,11 @@ ifcreate() {
> > > }
> > >
> > > # Create interfaces for network pseudo-devices referred to by
> > > hostname.if files.
> > > -# Usage: vifscreate
> > > +# Optionally, limit creation to given interfaces only.
> > > +# Usage: vifscreate [if ...]
> > > vifscreate() {
> > > - local _vif _hn _if
> > > + local _vif _hn _if _ifs
> > > + set -A _ifs -- "$@"
> > >
> > > for _vif in $(ifconfig -C); do
> > > for _hn in /etc/hostname.${_vif}+([[:digit:]]); do
> > > @@ -106,6 +108,9 @@ vifscreate() {
> > > # loopback for routing domain is created by kernel
> > > [[ -n ${_if##lo[1-9]*} ]] || continue
> > >
> > > + ((${#_ifs[*]} > 0)) && [[ ${_ifs[*]} != *${_if}* ]] &&
> > > + continue
> >
> > My gut feeling says this is wrong.
> > I suspect `netstart vlan0` will create an0.
>
> Sorry, I don't follow; how would it chop leading chars?
>
> Maybe you meant that somehow `sh /etc/netstart an0' would attempt
> creating an0 since *an0* would match e.g. "lo0 em0 vlan0" or so?
Yeah, along those lines, but my example was bogus as $_if would never
be "an0" here.
However, `sh /etc/vetstart egre0 # (or even whatevergre0)` would create
gre0 if /etc/hostname.gre0 existed.
Unless I'm mistaken again. Either way, it feels fragile.
>
> If so, my diff would change nothing in this regard and netstart already
> behaves... off for bogus interfaces, with and without my diff:
>
> # echo up > /etc/hostname.vlan0
> # echo up > /etc/hostname.an0
> # sh /etc/netstart -n an0 ; echo $?
> WARNING: /etc/hostname.an0 is insecure, fixing permissions.
> { ifconfig an0 || ifconfig an0 create; }
> ifconfig an0 up
> 0
> # sh /etc/netstart an0 ; echo $?
> 0
>
> > You could probably do
> >
> > ((${#_ifs[*]} > 0)) && [[ " ${_ifs[*]} " != *" ${_if} "* ]] &&
> >
> > but then it starts looking even worse. :-P
>
> Agreed.
>
> Here's a better diff at the end reusing isin() from install.sub.
> This way the purely theoretical false-positive globbing goes away and
> cryptic ksh code is traded for known, mnemonic ksh code.
>
> > > +
> > > if ! ifcreate $_if; then
> > > print -u2 "${0##*/}: create for '$_if' failed."
> > > fi
> > > @@ -314,6 +319,7 @@ $PRINT_ONLY || [[ ! -f /etc/soii.key ]]
> > > # If we were invoked with a list of interface names, just reconfigure
> > > these
> > > # interfaces (or bridges), add default routes and return.
> > > if (($# > 0)); then
> > > + vifscreate "$@"
> > > for _if; do ifstart $_if; done
> > > defaultroute
> > > return
> >
> > Would it be a problem just creating all pinpointed interfaces, be they
> > virtual or not, upfront?
>
> That'll fix my use case but I'd need more time to reason about this
> approach.
>
> ifstart()'s creation comes to mind:
>
> 139 Check for ifconfig'able interface, except if -n option is specified.
> 140 ifcreate $_if || return
>
> Will that trip or can it go?
Given its
{ ifconfig $_if || ifconfig $_if create; } >/dev/null 2>&1
, I'd say it's safe.
> With your diff it'd be purely redundant.
It would only be redundant for the case when you pass specific interface
names to netstart.
However, my loop would attempt to create any specified interface, regardless
of whether a matching hostname.if file existed...
>
> > diff --git a/etc/netstart b/etc/netstart
> > index 33e9689a819..62ca64803d8 100644
> > --- a/etc/netstart
> > +++ b/etc/netstart
> > @@ -314,6 +314,7 @@ $PRINT_ONLY || [[ ! -f /etc/soii.key ]] ||
> > # If we were invoked with a list of interface names, just reconfigure these
> > # interfaces (or bridges), add default routes and return.
> > if (($# > 0)); then
> > + for _if; do ifcreate $_if; done
> > for _if; do ifstart $_if; done
> > defaultroute
> > return
>
>
> Index: netstart
> ===================================================================
> RCS file: /cvs/src/etc/netstart,v
> retrieving revision 1.218
> diff -u -p -r1.218 netstart
> --- netstart 26 Jun 2022 09:36:13 -0000 1.218
> +++ netstart 2 Jul 2022 20:01:16 -0000
> @@ -11,6 +11,17 @@ usage() {
> exit 1
> }
>
> +# Test the first argument against the remaining ones, return success on a
> match.
> +isin() {
> + local _a=$1 _b
> +
> + shift
> + for _b; do
> + [[ $_a == "$_b" ]] && return 0
> + done
> + return 1
> +}
> +
> # Echo file $1 to stdout. Skip comment lines. Strip leading and trailing
> # whitespace if IFS is set.
> # Usage: stripcom /path/to/file
> @@ -94,9 +105,11 @@ ifcreate() {
> }
>
> # Create interfaces for network pseudo-devices referred to by hostname.if
> files.
> -# Usage: vifscreate
> +# Optionally, limit creation to given interfaces only.
> +# Usage: vifscreate [if ...]
> vifscreate() {
> - local _vif _hn _if
> + local _vif _hn _if _ifs
> + set -A _ifs -- "$@"
>
> for _vif in $(ifconfig -C); do
> for _hn in /etc/hostname.${_vif}+([[:digit:]]); do
> @@ -106,6 +119,10 @@ vifscreate() {
> # loopback for routing domain is created by kernel
> [[ -n ${_if##lo[1-9]*} ]] || continue
>
> + if ((${#_ifs[*]} > 0)) && ! isin $_if "${_ifs[@]}"; then
> + continue
> + fi
> +
Ok, I concur, and isin() is way cleaner. I do think the _ifs dance looks
messy though, and would suggest just using $@ as-is;
@@ -94,7 +105,8 @@ ifcreate() {
}
# Create interfaces for network pseudo-devices referred to by hostname.if
files.
-# Usage: vifscreate
+# Optionally, limit creation to given interfaces only.
+# Usage: vifscreate [if ...]
vifscreate() {
local _vif _hn _if
@@ -106,6 +118,11 @@ vifscreate() {
# loopback for routing domain is created by kernel
[[ -n ${_if##lo[1-9]*} ]] || continue
+ # restrict to $@, if any
+ if (($# > 0)) && ! isin $_if "$@"; then
+ continue
+ fi
+
if ! ifcreate $_if; then
print -u2 "${0##*/}: create for '$_if' failed."
fi
/Alexander
> if ! ifcreate $_if; then
> print -u2 "${0##*/}: create for '$_if' failed."
> fi
> @@ -313,7 +330,11 @@ $PRINT_ONLY || [[ ! -f /etc/soii.key ]]
>
> # If we were invoked with a list of interface names, just reconfigure these
> # interfaces (or bridges), add default routes and return.
> +# Create virtual interfaces upfront to make ifconfig commands depending on
> +# other interfaces, e.g. "patch", work regardless of in which order interface
> +# names were specified.
> if (($# > 0)); then
> + vifscreate "$@"
> for _if; do ifstart $_if; done
> defaultroute
> return
