In order to reduce the number of ifp->if_ioctl() calls, I'd like to simplify ifpromic().
There's no need for the underlying interface to be UP when calling ifpromic(). If the interface is DOWN, set or remove the flag and return. Next time the interface will be brought UP its multicast filters/promiscuous mode will be configured. This allows us to remove duplicated code in bridge/switch(4) to work around the stupid requirement. ok? Index: net/if.c =================================================================== RCS file: /cvs/src/sys/net/if.c,v retrieving revision 1.526 diff -u -p -r1.526 if.c --- net/if.c 12 Nov 2017 14:11:15 -0000 1.526 +++ net/if.c 13 Nov 2017 15:19:44 -0000 @@ -2639,32 +2639,33 @@ int ifpromisc(struct ifnet *ifp, int pswitch) { struct ifreq ifr; + unsigned short oif_flags; + int oif_pcount, error; + oif_flags = ifp->if_flags; + oif_pcount = ifp->if_pcount; if (pswitch) { - /* - * If the device is not configured up, we cannot put it in - * promiscuous mode. - */ - if ((ifp->if_flags & IFF_UP) == 0) - return (ENETDOWN); if (ifp->if_pcount++ != 0) return (0); ifp->if_flags |= IFF_PROMISC; } else { if (--ifp->if_pcount > 0) return (0); - ifp->if_flags &= ~IFF_PROMISC; - /* - * If the device is not configured up, we should not need to - * turn off promiscuous mode (device should have turned it - * off when interface went down; and will look at IFF_PROMISC - * again next time interface comes up). - */ - if ((ifp->if_flags & IFF_UP) == 0) - return (0); + ifp->if_flags &= ~IFF_PROMISC; } + + if ((ifp->if_flags & IFF_UP) == 0) + return (0); + + memset(&ifr, 0, sizeof(ifr)); ifr.ifr_flags = ifp->if_flags; - return ((*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, (caddr_t)&ifr)); + error = ((*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, (caddr_t)&ifr)); + if (error) { + ifp->if_flags = oif_flags; + ifp->if_pcount = oif_pcount; + } + + return (error); } void Index: net/if_bridge.c =================================================================== RCS file: /cvs/src/sys/net/if_bridge.c,v retrieving revision 1.298 diff -u -p -r1.298 if_bridge.c --- net/if_bridge.c 17 Aug 2017 10:14:08 -0000 1.298 +++ net/if_bridge.c 13 Nov 2017 15:21:11 -0000 @@ -299,41 +299,9 @@ bridge_ioctl(struct ifnet *ifp, u_long c } if (ifs->if_type == IFT_ETHER) { - if ((ifs->if_flags & IFF_UP) == 0) { - struct ifreq ifreq; - - /* - * Bring interface up long enough to set - * promiscuous flag, then shut it down again. - */ - strlcpy(ifreq.ifr_name, req->ifbr_ifsname, - IFNAMSIZ); - ifs->if_flags |= IFF_UP; - ifreq.ifr_flags = ifs->if_flags; - error = (*ifs->if_ioctl)(ifs, SIOCSIFFLAGS, - (caddr_t)&ifreq); - if (error != 0) - break; - - error = ifpromisc(ifs, 1); - if (error != 0) - break; - - strlcpy(ifreq.ifr_name, req->ifbr_ifsname, - IFNAMSIZ); - ifs->if_flags &= ~IFF_UP; - ifreq.ifr_flags = ifs->if_flags; - error = (*ifs->if_ioctl)(ifs, SIOCSIFFLAGS, - (caddr_t)&ifreq); - if (error != 0) { - ifpromisc(ifs, 0); - break; - } - } else { - error = ifpromisc(ifs, 1); - if (error != 0) - break; - } + error = ifpromisc(ifs, 1); + if (error != 0) + break; } #if NMPW > 0 else if (ifs->if_type == IFT_MPLSTUNNEL) { Index: net/if_switch.c =================================================================== RCS file: /cvs/src/sys/net/if_switch.c,v retrieving revision 1.20 diff -u -p -r1.20 if_switch.c --- net/if_switch.c 31 May 2017 05:59:09 -0000 1.20 +++ net/if_switch.c 13 Nov 2017 15:21:34 -0000 @@ -488,7 +488,6 @@ switch_port_add(struct switch_softc *sc, { struct ifnet *ifs; struct switch_port *swpo; - struct ifreq ifreq; int error; if ((ifs = ifunit(req->ifbr_ifsname)) == NULL) @@ -506,32 +505,8 @@ switch_port_add(struct switch_softc *sc, } if (ifs->if_type == IFT_ETHER) { - if ((ifs->if_flags & IFF_UP) == 0) { - /* - * Bring interface up long enough to set - * promiscuous flag, then shut it down again. - */ - strlcpy(ifreq.ifr_name, req->ifbr_ifsname, IFNAMSIZ); - ifs->if_flags |= IFF_UP; - ifreq.ifr_flags = ifs->if_flags; - if ((error = (*ifs->if_ioctl)(ifs, SIOCSIFFLAGS, - (caddr_t)&ifreq)) != 0) - return (error); - - if ((error = ifpromisc(ifs, 1)) != 0) - return (error); - strlcpy(ifreq.ifr_name, req->ifbr_ifsname, IFNAMSIZ); - ifs->if_flags &= ~IFF_UP; - ifreq.ifr_flags = ifs->if_flags; - if ((error = (*ifs->if_ioctl)(ifs, SIOCSIFFLAGS, - (caddr_t)&ifreq)) != 0) { - ifpromisc(ifs, 0); - return (error); - } - } else { - if ((error = ifpromisc(ifs, 1)) != 0) - return (error); - } + if ((error = ifpromisc(ifs, 1)) != 0) + return (error); } swpo = malloc(sizeof(*swpo), M_DEVBUF, M_NOWAIT|M_ZERO);