netlink: Bypass refcounting when setting promiscuity
ClosedPublic
Actions

Authored by obiwac on Aug 20 2025, 10:38 PM.

Details

Reviewers

melifaro
saheed
kp
mckusick

Commits

rG431856c868de: netlink: Bypass refcounting when setting promiscuity

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

obiwac created this revision.Aug 20 2025, 10:38 PM

Herald added a reviewer: melifaro. · View Herald TranscriptAug 20 2025, 10:38 PM

Herald added subscribers: glebius, melifaro, ae, imp. · View Herald Transcript

obiwac requested review of this revision.Aug 20 2025, 10:38 PM

Harbormaster completed remote builds in B66388: Diff 160692.Aug 20 2025, 10:38 PM

obiwac edited the test plan for this revision. (Show Details)Aug 20 2025, 10:39 PM

obiwac added reviewers: saheed, kp.

obiwac mentioned this in D52047: ifpromisc: Ignore "off" requests when refcount is zero.

saheed added inline comments.Aug 21 2025, 10:48 PM

sys/net/if.c
4459	Shouldn't it be `((ifp->if_flags ^ new_flags) & IFF_PPROMISC)`?

obiwac added inline comments.Aug 21 2025, 11:00 PM

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.