Fix renaming epair and devd(8) triggered interfaces and auto-NOAUTO the latter by default
Needs ReviewPublic
Actions

Authored by hs.freebsd_id.omnilan.de on Apr 7 2025, 2:42 PM.

Details

Reviewers

cy
emaste
markj

Group Reviewers

network

Summary

The patch allows interface renaming (by rc.conf(5)) for epair (cloned) interfaces and also for devd(8) configured interfaces.

'ifconfing description' is changed to be always applied (if defined), whether or not the NOAUTO keywork was found.

It also fixes non-functional NOAUTO config keyword for targeted start_interface commands (by devd(8)).

And in addition makes NOAUTO the default for devd(8) configured interfaces if rc.conf(5) lacks a corresponding interface configuration.

Test Plan

Tested with ng_eiface(4) and if_epair(4) interfaces (the former created by custom start_if script, the latter by cloned_interfaces="epair1 epair2 epair3" in rc.conf.
Sample excerpt which works with this patch and fails without:
ifconfig_ngeth5_name="vngXunl0"
ifconfig_vngXunl0="NOAUTO"
ifconfig_vngXunl0_descr="bridge-to-bridge downlink: brPTP (jail apiary) to brUNL (host UnscreenedNativeLAN)."

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

hs.freebsd_id.omnilan.de requested review of this revision.Apr 7 2025, 2:42 PM

hs.freebsd_id.omnilan.de created this revision.

Better 'ifconfig description' handling - previous hack wasn't limited to ifn_start condition.

hs.freebsd_id.omnilan.de edited the summary of this revision. (Show Details)Apr 7 2025, 6:41 PM

Replaced PoC diff with real tested diff - sorry for the mess.
This version corresponds to what the summary and the bug report (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=285947) describe.

Removed local _ifconfig_descr leftover from previously corrected autoif() hack

hs.freebsd_id.omnilan.de edited the test plan for this revision. (Show Details)Apr 8 2025, 3:16 PM

hs.freebsd_id.omnilan.de added reviewers: cy, emaste, markj.

Could you please explain what the diff is actually changing? It is quite hard to understand, for someone not very familiar with this code.

libexec/rc/network.subr
1610	Please add a comment explaining what this function does.
libexec/rc/rc.d/netif
196	Why not just write `if [ "$_func" = "ifn_start" ]; then`?
210	Same here.

markj added a reviewer: network.Apr 15 2025, 3:56 PM

Hi Mark, thanks for your review.

I'm not familiar with phabricator, I hope my inline comments are the correct way to answer.

Shall I re-integrate list_expanded_epair() without preserving previous method?

libexec/rc/network.subr
1610	This was previously part of netif_common() in rc.d/netif (lines 185-198) and used to modifiy $_cooked_list to contain the epairNa and epairNb interfaces. The function simply 'prints' epairNa and epairNb if 1st. argument is epair[0-9] without trailing [ab]. If 1st argument (beginning with epair[0-9]) already ends with [a\|b] it's printed unmodified in order to build up $_cmdifn (i.e netif stop epair3 stops epair3a and epair3b). Previously it was processed for the auto-start case too, where 'ifconfig list' populated $_cooked_list, which was superfluous. It's only needed it interfaces are passed as arguments to rc.d/netif - but since I wasn't not sure (especially for the wifi setup) about all possible/valid calls, I wanted to keep as much of the logic as it was designed.
libexec/rc/rc.d/netif
196	Old habit - usually I adopt the existing style, which clearly uses double quotes for variable expansion in the existing scripts.
210	Happy to change that too