Remove the final direct access of struct ifnet members from netlink.
Since only the first address is used, create the iterator and then free,
without fully iterating.
Details
Details
- Reviewers
melifaro bz kp - Commits
- rGb224af946a17: netlink: Don't directly access ifnet members
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 54866 Build 51755: arc lint + arc unit
Event Timeline
| sys/netlink/route/iface.c | ||
|---|---|---|
| 329 | While it does nothing now I'd have to assume that there's some anticipated need for ifa_iter_finish() to either clean up the iterator or to release a lock (or epoch or ..). Wouldn't it belong more after the ifa != NULL check and dump_sa() call? | |
| sys/netlink/route/iface.c | ||
|---|---|---|
| 329 | I think ifa should survive until the epoch exits, ifa_iter_finish() is used to clean up the iterator itself. It's quite likely that I don't understand epoch, and I'm not opposed to moving it before the ifa_iter_finish() if that is the case. That said, on second thought I do see a possibility where ifa might point to just a static internal buffer in the iterator, overwritten each time through the loop and cleaned up at the end. | |
Comment Actions
I don't understand the change. The epoch protection is already right here. We can safely use CK_STAILQ_FIRST.
Comment Actions
It's not about safety, it's because netlink shouldn't know ifnet guts, it's a more generic interface. I'm trying to remove net/if_private.h from if_var.h as soon as possible, and this is the last netlink piece that touches directly.
Comment Actions
Got it. Looks like there is also an assumption here that the first address is always the link level address. Maybe we just need to provide KPI if_getlladdr? May I ask to wait for Alexander melifaro@ to reappear before we proceed forward with that?
P.S. I'm also waiting for him on my netlink reviews.