Page MenuHomeFreeBSD

Add if_try_ref() to simplify refcount handling inside epoch

Authored by melifaro on Sun, Feb 21, 2:00 PM.



More and more code migrates from lock-based protection to the NET_EPOCH umbrella.
It requires some logic changes, including, notably, refcount handling.

When we have an ifp pointer and we're running inside epoch we're guaranteed that this pointer will not be freed.
However, the following case can still happen:

  • in thread 1 we drop to 0 refcount for ifp and schedule its deletion.
  • in thread 2 we use this ifp and reference it
  • destroy callout kicks in
  • unhappy user reports bug

This can happen with the current implementation of ifnet_byindex_ref(), as we're not holding any locks preventing ifnet deletion by a parallel thread.

To address it, if_try_ref() function is added, allowing to return failure when we try to reference ifp with 0 refcount.
Additionally, existing if_ref() is enforced with KASSERT to provide a cleaner error in such scenarios.

Finally, fix ifnet_byindex_ref() by using if_try_ref() and returning NULL if the latter fails.

This is pretty much the same as D28639.

Diff Detail

R10 FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

melifaro retitled this revision from Add if_try_ref() failing when current refcount=0. Fix ifnet_byindex_ref() by using if_try_ref() instead of if_ref(). to Add if_try_ref() to simplify refcount handling inside epoch.Sun, Feb 21, 2:08 PM
melifaro edited the summary of this revision. (Show Details)
melifaro added a reviewer: network.
This revision was not accepted when it landed; it landed in state Needs Review.Mon, Feb 22, 11:38 PM
This revision was automatically updated to reflect the committed changes.