Re: nd6 change and rc.d/network_ipv6 -> rc.d/netif integration

On Tue, Oct 6, 2009 at 8:37 AM, Dimitry Andric <dimitry_at_andric.com> wrote:
> On 2009-10-05 21:39, Dimitry Andric wrote:
>>>   http://people.freebsd.org/~hrs/ipv6_stable8.20091005.diff
>> Hmm, build bombs out at sbin/ifconfig:
> ...
>> This is because the patch doesn't seem to contain
>> sbin/ifconfig/af_nd6.c.  Can I just use the version from head?
>
> FYI, the patch also misses etc/rc.d/{faith,stf}.  Without these,
> mergemaster fails; again, taking them from head fixes it.

Hiroki --

I also attempted to use your patch against 8/Stable svn, and wanted to
give some feedback.

svn add <filename> before doing the svn diff will ensure that files
you want to add to a patch get noticed.  svn diff will only look at
files under version control.

1)  Patchset is missing examples and defaults for new rc.conf
variables to /etc/defaults/rc.conf. (The defaults/rc.conf has been
updated in -current, although perhaps once everything settles, it
would help to expand the examples in comments)

2)  I really like the changes to ifconfig and kernel for exposing
per-interface flags for "accept_rtadv" and other ndp flags to ifconfig
(and inherently rc.conf).  I previously had to do some hackery to
disable "accept_rtadv" at boot time for just one interface within
rc.conf.

3)  I would prefer that ipv6_enable remain a global flag in rc.conf,
and NOT be obsoleted.  I would also prefer that
ipv6_network_interfaces="auto" as in the past by default.  Again, I
like the logic changes and the flexibility it provides, it is just the
default/obsolete that I am interested in changing.

4) Personal opinion time:   change the "accept_rtadv" token to
"autoconf" in ifconfig and rc.conf, as this it is a better
self-description.  Just one persons opinion.

5) I noticed in the comments that you are considering allowing
autoconf+router in a future advanced configuration.  I completely
agree with adding this functionality to -current (for ipv6 router cpe
needs), we just need a new knob when this gets added.  I had code
around somewhere at one point.  I'll see if I can find it still.
There is a PR (conf/121812) on this as well.

Given the timing, +1 for letting this bake in -current until after 8.0
release.

If you want any early feedback for a future MFC of this code, feel
free to email me and I will gladly test the patch.

Thanks for the changes.

---Dave H