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

Authored by bms on Feb 28 2026, 11:52 PM.

Details

Reviewers

glebius
imp
pouria
ae

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.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 71103
Build 67986: arc lint + arc unit

Event Timeline

bms created this revision.Feb 28 2026, 11:52 PM

Herald added subscribers: melifaro, ae. · View Herald TranscriptFeb 28 2026, 11:52 PM

bms requested review of this revision.Feb 28 2026, 11:52 PM

Harbormaster completed remote builds in B71103: Diff 172949.Feb 28 2026, 11:52 PM

bms edited the summary of this revision. (Show Details)Feb 28 2026, 11:54 PM

ae accepted this revision.Mar 1 2026, 8:09 AM

This revision is now accepted and ready to land.Mar 1 2026, 8:09 AM

@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.

bms edited the summary of this revision. (Show Details)Mar 1 2026, 9:52 AM

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 `extern`s 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 added inline comments.Mar 1 2026, 4:10 PM

sys/netinet6/in6_src.c
117–118	DIdn't want to break the tab alignment versus surrounding macros.

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:

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.
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().
Alternative method to 2) is MOD_LOAD and MOD_UNLOAD cases with module(9) registered modules.
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:

Add MTX_SYSINIT() and SX_SYSINIT() for these two global locks.
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!

This revision now requires changes to proceed.Mar 2 2026, 4:38 PM

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.

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 :)

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.