Page MenuHomeFreeBSD

netlink: Bypass refcounting when setting promiscuity
ClosedPublic

Authored by obiwac on Aug 20 2025, 10:38 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Oct 12, 7:27 PM
Unknown Object (File)
Sun, Oct 12, 12:12 PM
Unknown Object (File)
Sun, Oct 12, 1:15 AM
Unknown Object (File)
Sat, Oct 11, 3:45 PM
Unknown Object (File)
Sat, Oct 11, 3:45 PM
Unknown Object (File)
Sat, Oct 11, 3:45 PM
Unknown Object (File)
Sat, Oct 11, 7:16 AM
Unknown Object (File)
Wed, Oct 8, 9:13 PM

Details

Summary

When asking for IFF_PROMISC when modifying interfaces with netlink, set permanent flag instead (IFF_PPROMISC) as netlink interface modification has no way of doing promiscuity reference counting through ifpromisc(). We can't do reference counting because every netlink interface modification necessarily either sets or unsets IFF_PROMISC in ifi_flags, and ifi_change is usually set to 0xFFFFFFFF.

This logic was the same between this and SIOCSIFFLAGS, so factor out if_setppromisc() function.

Test Plan

will test this tomorrow, maybe add a set IFF_PROMISC, then 2* unset test to TestRtNlIface.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/net/if.c
4459

Shouldn't it be ((ifp->if_flags ^ new_flags) & IFF_PPROMISC)?

sys/net/if.c
4459

yes you're right

The commit message needs to answer the "Why?".
Why are we bypassing the reference counting?

I've not tested this, but it does look sane.

sys/net/if.c
4451

I'd use 'bool ppromisc' here. We're not passing flags, it's explicitly IFF_PPROMISC only.

sys/netlink/route/iface_drivers.c
99

I'd keep this out of this commit.

That's an unrelated 'Handle ifhwioct(SIOCSIFMTU) failure' change.

Respond to review comments

obiwac marked 3 inline comments as done.

Remove failure handling change

obiwac edited the summary of this revision. (Show Details)EditedAug 22 2025, 8:24 PM
obiwac edited the summary of this revision. (Show Details)

Thanks for your reviews @saheed and @kp, have updated my commit message :)

Set D52132 as parent because the error variable would be unused if we merged this first.

This revision is now accepted and ready to land.Aug 23 2025, 11:57 AM

I tested it out, it works nicely :)

mckusick added a subscriber: mckusick.

Mentor approval - looks good to go.