Page MenuHomeFreeBSD

iflib: add iflib_deregister to help cleanup on exit
ClosedPublic

Authored by jacob.e.keller_intel.com on Jul 19 2019, 11:52 PM.

Details

Summary

The iflib_register function exists to allocate and setup some common
structures used by both iflib_device_register and iflib_pseudo_register.

There is no associated cleanup function used to undo the steps taken in
this function.

Both iflib_device_deregister and iflib_pseudo_deregister have some of
the necessary steps scattered in their flow. However, most of the
necessary cleanup is not done during the error path of
iflib_device_register and iflib_pseudo_register.

Some examples of missed cleanup include:

  1. the ifp pointer is not free'd during error cleanup
  2. the STATE and CTX locks are not destroyed during error cleanup
  3. the vlan event handlers are not removed during error cleanup
  4. media added to the ifmedia structure is not removed
  5. the kobject reference is never deleted

Additionally, when initializing the kobject class reference counter is
increased even though kobj_init already increases it. This results in
the class never being free'd again because the reference count would
never hit zero even after all driver instances are unloaded.

To aid in proper cleanup, implement an iflib_deregister function that
goes through the reverse steps taken by iflib_register.

Call this function during the error cleanup for iflib_device_register
and iflib_pseudo_register. Additionally call the function in the
iflib_device_deregister and iflib_pseudo_deregister functions near the
end of their flow. This helps reduce code duplication and ensures that
proper steps are taken to cleanup allocations and references in both the
regular and error cleanup flows.

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

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

This is built on top of https://reviews.freebsd.org/D21003

It could probably be backported, but isn't as necessary as the previous patch which is why I kept them separate.

erj added a comment.Jul 23 2019, 6:16 PM

This is built on top of https://reviews.freebsd.org/D21004
It could probably be backported, but isn't as necessary as the previous patch which is why I kept them separate.

Do you mean https://reviews.freebsd.org/D21003?

In D21005#456595, @erj wrote:

This is built on top of https://reviews.freebsd.org/D21004
It could probably be backported, but isn't as necessary as the previous patch which is why I kept them separate.

Do you mean https://reviews.freebsd.org/D21003?

Yep.

Move the kobject refcount removal to a separate patch

This revision is now accepted and ready to land.Aug 14 2019, 6:17 PM
erj accepted this revision.Aug 16 2019, 11:33 PM
This revision was automatically updated to reflect the committed changes.