Page MenuHomeFreeBSD

factor out remove logic from if_delgroup and if_delgroups
Needs ReviewPublic

Authored by jacob.e.keller_intel.com on Fri, Nov 1, 5:23 PM.

Details

Reviewers
jhb
gallatin
erj
Group Reviewers
Intel Networking
Summary

The if_delgroups function failed to free the ifg_list structure pointer
memory. This results in a small leak on M_TEMP for each ifp group every
time an interface is detached.

This was caused because if_delgroup and if_delgroups duplicate the logic
for releasing the group memory.

Fix this by factoring out an if_freegroup function which can be called
by both if_delgroup and if_delgroups.

This results in slightly funky code, because we must drop the
IFNET_WLOCK before calling the event handler. However, we want the
IFNET_WLOCK around the group list search in if_delgroup and in the group
list iteration in if_delgroups. Further, if_delgroups must re-take the
lock after calling the new if_freegroups.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 27309
Build 25567: arc lint + arc unit

Event Timeline

This is likely the better approach to solving the leak described in https://reviews.freebsd.org/D22215

I don't know 100% though, because the fact that the function releases the lock smells weird and like it could be confusing to look at in the future. That's why I posted both potential fixes.

jhb accepted this revision.Fri, Nov 1, 6:08 PM

I think the refactor is fine.

sys/net/if.c
1504

Better to add an IFNET_WLOCK_ASSERT() or some such than to just document it in a comment.

This revision is now accepted and ready to land.Fri, Nov 1, 6:08 PM

add an IFNET_WLOCK_ASSERT in if_freegroup

This revision now requires review to proceed.Fri, Nov 1, 6:15 PM