ifpromic() change & bridge/switch(4) cleanup

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

ifpromic() change & bridge/switch(4) cleanup

Martin Pieuchot
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);