Page MenuHomeFreeBSD

Enhance IPv6 autoconf startup scripts
ClosedPublic

Authored by bz on Mar 6 2019, 11:56 PM.

Details

Summary

This change improves IPv6 configuration for FreeBSD 13 and later
by running some IPv6 configuration before IPv4 (rtsol now run
unconditionally on rtsold if we accept_rtadv and before dhcp).

MFC after: never

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bz created this revision.Mar 6 2019, 11:56 PM
hrs added a comment.Mar 7 2019, 1:08 AM

(1) looks good to me. For (2), I think the old version unconditionally runs rtsol (w/o "d") when rtsold_enable="NO", and does not when rtsold_enable="YES" because rtsold (w/ "d") will be invoked later by rc.d/rtsold should handle it. In short, the condition which determines if rtsol is invoked or not depends only on "accept_rtadv" flag. Isn't it enough?

bz added a comment.Mar 7 2019, 10:36 AM
In D19488#417164, @hrs wrote:

For (2), I think the old version unconditionally runs rtsol (w/o "d") when rtsold_enable="NO", and does not when rtsold_enable="YES" because rtsold (w/ "d") will be invoked later by rc.d/rtsold should handle it. In short, the condition which determines if rtsol is invoked or not depends only on "accept_rtadv" flag. Isn't it enough?

Hmm, are you sure? Looking at the logic I changed above we invoked rtsol only if rtsold was disabled and otherwise rtsold was started later and done it (so far we agree); I think that again meant that dhcp was run before the first RS/RA cycle happened hence me needing a way to trigger that early. Hence adding the extra interface flag. I think my description above was a cache miss and is wrong (I did this change months ago). I am almost tempted to remove the extra logic and run rtsol (no d) unconditionally if "accept_rtadv" and then start rtsold later or not depending on whether the user wants long-term stability and DNS options from RAs or not.

What do you think?

hrs added a comment.Mar 7 2019, 6:44 PM
In D19488#417207, @bz wrote:
In D19488#417164, @hrs wrote:

For (2), I think the old version unconditionally runs rtsol (w/o "d") when rtsold_enable="NO", and does not when rtsold_enable="YES" because rtsold (w/ "d") will be invoked later by rc.d/rtsold should handle it. In short, the condition which determines if rtsol is invoked or not depends only on "accept_rtadv" flag. Isn't it enough?

Hmm, are you sure? Looking at the logic I changed above we invoked rtsol only if rtsold was disabled and otherwise rtsold was started later and done it (so far we agree); I think that again meant that dhcp was run before the first RS/RA cycle happened hence me needing a way to trigger that early. Hence adding the extra interface flag. I think my description above was a cache miss and is wrong (I did this change months ago). I am almost tempted to remove the extra logic and run rtsol (no d) unconditionally if "accept_rtadv" and then start rtsold later or not depending on whether the user wants long-term stability and DNS options from RAs or not.
What do you think?

Ah, I misunderstood the current code and I could not follow the reason why a late run of rtsold was not enough from the summary line, but now understand the need to send an RS before invoking rc.d/rtsold and the current code prevents it when rtsold_enable="YES". So I prefer to remove both of the $rtsold_enable check and the extra logic RTSOL in your patch, and simply run rtsol unconditionally on each interface with accept_rtadv at an earlier stage. It should be harmless even in the case that rtsold runs later while I do not remember why I added the check of $rtsold_enable.

bz updated this revision to Diff 54825.Mar 7 2019, 11:14 PM
bz edited the summary of this revision. (Show Details)

Remove the RTSOL interface pseudo-option and run rtsol unconditionally on accept_rtadv (if it is installed).

hrs accepted this revision.Mar 12 2019, 5:47 AM
This revision is now accepted and ready to land.Mar 12 2019, 5:47 AM
This revision was automatically updated to reflect the committed changes.

This breaks ifconfig_DEFAULT="DHCP" somehow. It works with either SYNCDHCP or this commit reverted.

bz added a comment.Mar 16 2019, 3:27 PM

This breaks ifconfig_DEFAULT="DHCP" somehow. It works with either SYNCDHCP or this commit reverted.

Do you have an error message from dmesg -a maybe? Which interface is this? Anything from dhclient in logfiles?
Just changing the order should not do anything with regards to dhcp. What was your last good configuration? Did you have all the recent dhclient changes already?

Feel free to also send it by email to bz@ or here.

% cat /etc/rc.conf
clear_tmp_enable="YES"
syslogd_flags="-ss"
sendmail_enable="NONE"
hostname="devel2"
ifconfig_vtnet0="DHCP"
sshd_enable="YES"
ntpd_enable="YES"
dumpdev="AUTO"
zfs_enable="YES"

This is regular head with mergemaster being run after installworlds. Are there any specific files I should take a look at that can cause this?

Here are dumps of dmesg -a with rc_debug="YES" of system without this change and with it respectively.

bz added a comment.Mar 17 2019, 12:29 AM

I think I see how this can happen; I'll send another patch later today for testing.

bz added a comment.EditedMar 17 2019, 12:36 AM
In D19488#419791, @bz wrote:

I think I see how this can happen; I'll send another patch later today for testing.

I haven't tested this; can you give it a try?

Index: libexec/rc/network.subr
===================================================================
--- libexec/rc/network.subr     (revision 345088)
+++ libexec/rc/network.subr     (working copy)
@@ -230,8 +230,7 @@ ifconfig_up()
        fi

        if ! noafif $1 && afexists inet6; then
-               ipv6_accept_rtadv_up $1
-               _cfg=0
+               ipv6_accept_rtadv_up $1 && _cfg=0
        fi

        if dhcpif $1; then
@@ -1205,7 +1204,9 @@ ipv6_accept_rtadv_up()
                if [ -x /sbin/rtsol ]; then
                        /sbin/rtsol ${rtsol_flags} $1
                fi
+               return 0
        fi
+       return 1
 }

 # ipv6_accept_rtadv_down if
'''

I confirm that it works.