ifpromisc: Ignore "off" requests when refcount is zero

[saheed]

Summary

Adds checks to ensure that the refcount (number of listeners) of
IFF_PROMISC (ifp->if_pcount) is non-zero before clearing.
This avoids kernel panic on assert failure in if_setflag() on clearing
IFF_PROMISC using ifpromisc(9) if the flag's refcount is zero.
ifpromisc(9) internally calls if_setflag() which assumes the refcount
to be positive when clearing a flag.
This fixes https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=288967

Sponsored by: Google LLC (GSoC)

[obiwac]

I don't think this is the right solution - there is a bit of an issue with the way we set/unset promiscuity with netlink on FreeBSD.

if we are to take the netlink doc to the letter and set ifi_change = 0xFFFFFFFF all the time, then any time we end up calling _nl_modify_ifp_generic we will decrement the promiscuity refcount. This also goes for if we were to use the IFLA_PROMISCUITY attribute (which we don't support yet).

on linux my understanding is that the refcounting (which they do with dev->promiscuity) is bypassed when using dev_change_flags instead of dev_set_promiscuity; if we do a no promisc -> promisc transition we increment, and only when we do a promisc -> no promisc transition do we decrement. But in between if we are already promisc and we do a promisc flag set -> promisc flag set transition we don't touch anything (see the (flags ^ dev->gflags) & IFF_PROMISC check).

I think we should implement something similar for setting promiscuity through netlink. Or maybe just interpreting IFF_PROMISC as IFF_PPROMISC would be an option.

@melifaro @kp do you guys have any input on this? who else can I ping for netlink-related stuff?

[obiwac]

In D52047#1189001, @obiwac wrote:

Or maybe just interpreting IFF_PROMISC as IFF_PPROMISC would be an option.

I have tried this out in D52056

[saheed]

This revision shall be closed in favor of D52056 :)