The branch main has been updated by obiwac:
URL:
https://cgit.FreeBSD.org/src/commit/?id=431856c868de12d7af127cdf7e9aeb1b99d0ac99
commit 431856c868de12d7af127cdf7e9aeb1b99d0ac99
Author: Aymeric Wibo <obi...@freebsd.org>
AuthorDate: 2025-08-25 22:40:16 +0000
Commit: Aymeric Wibo <obi...@freebsd.org>
CommitDate: 2025-08-25 22:40:17 +0000
netlink: Bypass refcounting when setting promiscuity
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.
Reviewed by: melifaro, saheed, kp, mckusick (mentor)
Approved by: melifaro, saheed, mckusick (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D52056
---
sys/net/if.c | 37 +++++++++++++++++++++++++++----------
sys/net/if_var.h | 1 +
sys/netlink/route/iface_drivers.c | 17 ++++++++++-------
3 files changed, 38 insertions(+), 17 deletions(-)
diff --git a/sys/net/if.c b/sys/net/if.c
index 79c883fd4a0a..202be4794f6e 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -2589,16 +2589,7 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data,
struct thread *td)
* flip. They require special handling because in-kernel
* consumers may indepdently toggle them.
*/
- if ((ifp->if_flags ^ new_flags) & IFF_PPROMISC) {
- if (new_flags & IFF_PPROMISC)
- ifp->if_flags |= IFF_PROMISC;
- else if (ifp->if_pcount == 0)
- ifp->if_flags &= ~IFF_PROMISC;
- if (log_promisc_mode_change)
- if_printf(ifp, "permanently promiscuous mode
%s\n",
- ((new_flags & IFF_PPROMISC) ?
- "enabled" : "disabled"));
- }
+ if_setppromisc(ifp, new_flags & IFF_PPROMISC);
if ((ifp->if_flags ^ new_flags) & IFF_PALLMULTI) {
if (new_flags & IFF_PALLMULTI)
ifp->if_flags |= IFF_ALLMULTI;
@@ -4456,6 +4447,32 @@ if_getmtu_family(const if_t ifp, int family)
return (ifp->if_mtu);
}
+void
+if_setppromisc(if_t ifp, bool ppromisc)
+{
+ int new_flags;
+
+ if (ppromisc)
+ new_flags = ifp->if_flags | IFF_PPROMISC;
+ else
+ new_flags = ifp->if_flags & ~IFF_PPROMISC;
+ if ((ifp->if_flags ^ new_flags) & IFF_PPROMISC) {
+ if (new_flags & IFF_PPROMISC)
+ new_flags |= IFF_PROMISC;
+ /*
+ * Only unset IFF_PROMISC if there are no more consumers of
+ * promiscuity, i.e. the ifp->if_pcount refcount is 0.
+ */
+ else if (ifp->if_pcount == 0)
+ new_flags &= ~IFF_PROMISC;
+ if (log_promisc_mode_change)
+ if_printf(ifp, "permanently promiscuous mode %s\n",
+ ((new_flags & IFF_PPROMISC) ?
+ "enabled" : "disabled"));
+ }
+ ifp->if_flags = new_flags;
+}
+
/*
* Methods for drivers to access interface unicast and multicast
* link level addresses. Driver shall not know 'struct ifaddr' neither
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 08435e7bd5f6..f2df612b19c1 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -622,6 +622,7 @@ int if_setmtu(if_t ifp, int mtu);
int if_getmtu(const if_t ifp);
int if_getmtu_family(const if_t ifp, int family);
void if_notifymtu(if_t ifp);
+void if_setppromisc(const if_t ifp, bool ppromisc);
int if_setflagbits(if_t ifp, int set, int clear);
int if_setflags(if_t ifp, int flags);
int if_getflags(const if_t ifp);
diff --git a/sys/netlink/route/iface_drivers.c
b/sys/netlink/route/iface_drivers.c
index 2a75e6abb2d0..4f1540740ead 100644
--- a/sys/netlink/route/iface_drivers.c
+++ b/sys/netlink/route/iface_drivers.c
@@ -105,13 +105,16 @@ _nl_modify_ifp_generic(struct ifnet *ifp, struct
nl_parsed_link *lattrs,
}
}
- if (lattrs->ifi_change & IFF_PROMISC) {
- error = ifpromisc(ifp, lattrs->ifi_flags & IFF_PROMISC);
- if (error != 0) {
- nlmsg_report_err_msg(npt, "unable to set promisc");
- return (error);
- }
- }
+ if ((lattrs->ifi_change & IFF_PROMISC) != 0 ||
+ lattrs->ifi_change == 0)
+ /*
+ * When asking for IFF_PROMISC, set permanent flag instead
+ * (IFF_PPROMISC) as we have no way of doing promiscuity
+ * reference counting through ifpromisc(). Every call to this
+ * function either sets or unsets IFF_PROMISC, and ifi_change
+ * is usually set to 0xFFFFFFFF.
+ */
+ if_setppromisc(ifp, (lattrs->ifi_flags & IFF_PROMISC) != 0);
if (lattrs->ifla_address != NULL) {
if (nlp_has_priv(npt->nlp, PRIV_NET_SETIFMAC)) {
