net/if: Fix ifa memory leak during vnet removal

[pouria]

Summary

Clean up memory allocations after removal of a vnet jail.

MFC after: 2 weeks

Without patch:
Before running sys/net kyua tests:

[root@ftsr1] [~] # vmstat -m | grep ifaddr
ifaddr   38 17392   38 16,32,128,256,384,1024,2048,4096

After running sys/net kyua tests:

[root@ftsr1] [~] # vmstat -m | grep ifaddr
ifaddr 1418 383152 19838 16,32,128,256,384,1024,2048,4096

With patch:
Before running sys/net kyua tests:

[root@ftsr1] [~] # vmstat -m | grep ifaddr
ifaddr   39 17776   39 16,32,128,256,384,1024,2048,4096

After running sys/net kyua tests:

[root@ftsr1] [~] # vmstat -m | grep ifaddr
ifaddr  140 55984 19010 16,32,128,256,384,1024,2048,4096

Test Plan

vmstat -m | grep ifaddr
jail -c name=leak persist
jail -r leak
vmstat -m | grep ifaddr

To increase leak size:

ifconfig -j leak lo create

[markj]

But where is the leak? Shouldn't each interface be responsible for removing its addresses when it's destroyed?

[pouria]

In D59387#1362564, @markj wrote:

But where is the leak? Shouldn't each interface be responsible for removing its addresses when it's destroyed?

Hi!, See the comments.

sys/net/if.c

877–880

4. We allocate an ifa for loopback interfaces here, but we don't free it after jail removal.

960

2.if we do 1, we can't call in6_purge_proxy_ndp, due to its LLTABLE lock.
even if we make it conditional by using !VNET_IS_SHUTTING_DOWN. Then (see third comment)

967

3. we skip AF_LINKs here anyway, and we can't include AF_LINK in this function. (too soon to free it when !shutdown)

1137

1. We can't call this function under if (shutdown), but even if we do it. (see second comment.)

[glebius]

You definitely found the problem, but the fix doesn't seem correct. There is asymmetry in allocation/freeing. It could close the leak at some configurations without regressions, of course. But IMHO not at all configurations.

First, the stack allocates single address, but frees the entire list. Can this be made symmetrical and only ifp->if_addr is freed by the stack?
Second, allocation happens always, but freeing is under #ifdef VIMAGE.

[glebius]

P.S. Can't resist to put my standard sidenote here. This all is so complicated due to if_vmove() :)

[pouria]

In D59387#1362617, @glebius wrote:

You definitely found the problem, but the fix doesn't seem correct. There is asymmetry in allocation/freeing. It could close the leak at some configurations without regressions, of course. But IMHO not at all configurations.

First, the stack allocates single address, but frees the entire list. Can this be made symmetrical and only ifp->if_addr is freed by the stack?
Second, allocation happens always, but freeing is under #ifdef VIMAGE.

IMHO, this is what we already do under the !vmove condition, and my change also looks similar to !vmove.
I understand the problem statement, but I think it needs more work than simply resolving the leak here, therefore I'm not sure this revision is the right place to fix the overall asymmetry issue. (see comment)

In D59387#1362621, @glebius wrote:

P.S. Can't resist to put my standard sidenote here. This all is so complicated due to if_vmove() :)

was waiting for this one :D

sys/net/if.c

1144–1162

Normally, without the shutdown condition, this code runs and removes any remaining ifa, especially AF_LINK, since if_purgeaddrs() above it frees the rest.
We already do that. I could create a separate dedicated function for both !vmove and shutdown condition, but I'd rather not, because the code under the shutdown doesn't actually require removing each ifa from ifp->if_addrhead, so it won't need its lock.

[glebius]

I vaguely remember that December 2025 when I was refactoring the attach/detach of domains/interfaces 0d469d23715d690b863787ebfa51529e1f6a9092, I found out that there are at least 2 places where we free addresses and multicast addresses. And apparently we mishandle that as there is a leak left. And I left it for later. Maybe good time for another take on that mess.