netinet6: Tear down IPv6 source address selection policies with rest of IPv6.

[bms]

Summary

This may plug minor leaks which no-one has reported. The default IPv6 source
address selection policy list in FreeBSD is usually limited to 9 entries,
and can be readily inspected with ip6addrctl(8). The policy table is
however instantiated for each VNET.

The leak of a pol instance in delete_addrsel_policyent() was already
plugged by @ae in commit-id ecc5c73, so that change has not been merged.

Obtained from: dSBR-ILNP-8 (for diff reduction)
Sponsored by: Cisco Systems, Inc.

[bms]

@ae: Do I have you confused with @ache ? It appears I do.

Needs @glebius or @imp to approve since I am technically still under mentorship since returning.

[pouria]

FYI, Steps to reproduce:

# vmstat -m | grep ifaddr
ifaddr   38 16352   50 16,32,128,256,384,1024,2048,4096
# jail -c persist name=vnet1 mount.devfs devfs_ruleset=5 vnet
path=/
# vmstat -m | grep ifaddr
ifaddr   43 23808   55 16,32,128,256,384,1024,2048,4096
# jexec -l 4 ip6addrctl add 2a01:e140::/32 60 20
# vmstat -m | grep ifaddr
ifaddr   44 23936   56 16,32,128,256,384,1024,2048,4096
# jail -r 4
# vmstat -m | grep ifaddr
ifaddr   40 16736   56 16,32,128,256,384,1024,2048,4096

128B memory leak for each undeleted ip6ctl policy in vnets. + lock leaks

sys/netinet6/in6.h

692

We should somehow avoid these externs in future. But this is not concern of this revision.

sys/netinet6/in6_src.c

117–118

is this "\" really required?
I think it would fit single line without violating the style(9)

[bms]

sys/netinet6/in6_src.c

117–118

DIdn't want to break the tab alignment versus surrounding macros.

[glebius]

This is not correct VNET-wise. A general rule of thumb: you ever want to use IS_DEFAULT_VNET(curvnet) in some stub network module - you are doing something wrong :)

Here is the right way to code VNET constructors and destructors:

1. A destructor registered with VNET_SYSUNINIT() shall work on a single VNET and should apply same actions on any VNET, not singling out the default one or any other. It should free/destroy all of the stuff that had been registered with complementary VNET_SYSINIT() macro and nothing else.
2. A constructor registered with SYSINIT() shall allocate the dynamic stuff that is used by all VNETs. It shall be freed in complementary destructor registered with SYSUNINIT().
3. Alternative method to 2) is MOD_LOAD and MOD_UNLOAD cases with module(9) registered modules.
4. Stuff on bss shall be initialized with static initializers, or if that is not possible with SYSINIT(). This one does not need to have complementary SYSUNINIT().

In this particular case the mistake predates the suggested change. The addrsel_policy_init() which is per-VNET constructor shall not initialize the mutex and the sx which are global. There are two options here:

1. Add MTX_SYSINIT() and SX_SYSINIT() for these two global locks.
2. Add mtx_init() and sx_init() to the global constructor ip6_init().

I prefer the second option - less code after compilation.

The main part of the change: calling destroy_policy_queue() from the VNET destructor is a correct memory leak fix of course! Thanks!

[pouria]

In D55599#1272617, @glebius wrote:

A general rule of thumb: you ever want to use IS_DEFAULT_VNET(curvnet) in some stub network module - you are doing something wrong :)

Noted. Thanks for explaining.

[glebius]

In D55599#1272620, @pouria wrote:

In D55599#1272617, @glebius wrote:

A general rule of thumb: you ever want to use IS_DEFAULT_VNET(curvnet) in some stub network module - you are doing something wrong :)

Noted. Thanks for explaining.

Of course there could be exclusions, but in most cases it is a sign of doing wrong :)

[glebius]

Bruce, to be more specific: this particular change needs to add only destroy_policy_queue() and don't do anything wrt the locks. Then a separate change that moves lock initialization to ip6_init() is possible.