svn commit: r356554 - in head: sbin/ifconfig sys/net

[Mark Johnston]

Author: markj
Date: Thu Jan  9 14:58:41 2020
New Revision: 356554
URL: https://svnweb.freebsd.org/changeset/base/356554

Log:
  lagg: Further cleanup of the rr_limit option.
  
  Add an option flag so that arbitrary updates to a lagg's configuration
  do not clear sc_stride.  Preseve compatibility for old ifconfig
  binaries.  Update ifconfig to use the new flag and improve the casting
  used when parsing the option parameter.
  
  Modify the RR transmit function to avoid locklessly reading sc_stride
  twice.  Ensure that sc_stride is always 1 or greater.
  
  Reviewed by:	hselasky
  MFC after:	1 week
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D23092

Modified:
  head/sbin/ifconfig/iflagg.c
  head/sys/net/if_lagg.c
  head/sys/net/if_lagg.h

Modified: head/sbin/ifconfig/iflagg.c
==============================================================================
--- head/sbin/ifconfig/iflagg.c	Thu Jan  9 14:12:57 2020	(r356553)
+++ head/sbin/ifconfig/iflagg.c	Thu Jan  9 14:58:41 2020	(r356554)
@@ -114,10 +114,13 @@ setlaggrr_limit(const char *val, int d, int s, const s
 	
 	bzero(&ro, sizeof(ro));
 	strlcpy(ro.ro_ifname, name, sizeof(ro.ro_ifname));
-	ro.ro_bkt = (int)strtol(val, NULL, 10);
+	ro.ro_opts = LAGG_OPT_RR_LIMIT;
+	ro.ro_bkt = (uint32_t)strtoul(val, NULL, 10);
+	if (ro.ro_bkt == 0)
+		errx(1, "Invalid round-robin stride: %s", val);
 
 	if (ioctl(s, SIOCSLAGGOPTS, &ro) != 0)
-		err(1, "SIOCSLAGG");
+		err(1, "SIOCSLAGGOPTS");
 }
 
 static void

Modified: head/sys/net/if_lagg.c
==============================================================================
--- head/sys/net/if_lagg.c	Thu Jan  9 14:12:57 2020	(r356553)
+++ head/sys/net/if_lagg.c	Thu Jan  9 14:58:41 2020	(r356554)
@@ -1257,21 +1257,18 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data
 
 		/*
 		 * The stride option was added without defining a corresponding
-		 * LAGG_OPT flag, so we must handle it before processing any
-		 * remaining options.
+		 * LAGG_OPT flag, so handle a non-zero value before checking
+		 * anything else to preserve compatibility.
 		 */
 		LAGG_XLOCK(sc);
-		if (ro->ro_bkt != 0) {
+		if (ro->ro_opts == 0 && ro->ro_bkt != 0) {
 			if (sc->sc_proto != LAGG_PROTO_ROUNDROBIN) {
 				LAGG_XUNLOCK(sc);
 				error = EINVAL;
 				break;
 			}
 			sc->sc_stride = ro->ro_bkt;
-		} else {
-			sc->sc_stride = 0;
 		}
-
 		if (ro->ro_opts == 0) {
 			LAGG_XUNLOCK(sc);
 			break;
@@ -1289,6 +1286,7 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data
 		case LAGG_OPT_USE_NUMA:
 		case -LAGG_OPT_USE_NUMA:
 		case LAGG_OPT_FLOWIDSHIFT:
+		case LAGG_OPT_RR_LIMIT:
 			valid = 1;
 			lacp = 0;
 			break;
@@ -1314,14 +1312,23 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data
 			LAGG_XUNLOCK(sc);
 			break;	/* Return from SIOCSLAGGOPTS. */ 
 		}
+
 		/*
 		 * Store new options into sc->sc_opts except for
-		 * FLOWIDSHIFT and LACP options.
+		 * FLOWIDSHIFT, RR and LACP options.
 		 */
 		if (lacp == 0) {
 			if (ro->ro_opts == LAGG_OPT_FLOWIDSHIFT)
 				sc->flowid_shift = ro->ro_flowid_shift;
-			else if (ro->ro_opts > 0)
+			else if (ro->ro_opts == LAGG_OPT_RR_LIMIT) {
+				if (sc->sc_proto != LAGG_PROTO_ROUNDROBIN ||
+				    ro->ro_bkt == 0) {
+					error = EINVAL;
+					LAGG_XUNLOCK(sc);
+					break;
+				}
+				sc->sc_stride = ro->ro_bkt;
+			} else if (ro->ro_opts > 0)
 				sc->sc_opts |= ro->ro_opts;
 			else
 				sc->sc_opts &= ~ro->ro_opts;
@@ -2046,6 +2053,7 @@ static void
 lagg_rr_attach(struct lagg_softc *sc)
 {
 	sc->sc_seq = 0;
+	sc->sc_stride = 1;
 }
 
 static int
@@ -2055,9 +2063,7 @@ lagg_rr_start(struct lagg_softc *sc, struct mbuf *m)
 	uint32_t p;
 
 	p = atomic_fetchadd_32(&sc->sc_seq, 1);
-	if (sc->sc_stride > 0)
-		p /= sc->sc_stride;
-
+	p /= sc->sc_stride;
 	p %= sc->sc_count;
 	lp = CK_SLIST_FIRST(&sc->sc_ports);
 

Modified: head/sys/net/if_lagg.h
==============================================================================
--- head/sys/net/if_lagg.h	Thu Jan  9 14:12:57 2020	(r356553)
+++ head/sys/net/if_lagg.h	Thu Jan  9 14:58:41 2020	(r356554)
@@ -63,11 +63,11 @@ struct lagg_protos {
 
 #define	LAGG_PROTO_DEFAULT	LAGG_PROTO_FAILOVER
 #define LAGG_PROTOS	{						\
-	{ "failover",		LAGG_PROTO_FAILOVER },		\
+	{ "failover",		LAGG_PROTO_FAILOVER },			\
 	{ "lacp",		LAGG_PROTO_LACP },			\
 	{ "loadbalance",	LAGG_PROTO_LOADBALANCE },		\
-	{ "roundrobin",	LAGG_PROTO_ROUNDROBIN },		\
-	{ "broadcast",	LAGG_PROTO_BROADCAST },		\
+	{ "roundrobin",		LAGG_PROTO_ROUNDROBIN },		\
+	{ "broadcast",		LAGG_PROTO_BROADCAST },			\
 	{ "none",		LAGG_PROTO_NONE },			\
 	{ "default",		LAGG_PROTO_DEFAULT }			\
 }
@@ -149,6 +149,7 @@ struct lagg_reqopts {
 #define	LAGG_OPT_LACP_TXTEST		0x20		/* LACP debug: txtest */
 #define	LAGG_OPT_LACP_RXTEST		0x40		/* LACP debug: rxtest */
 #define	LAGG_OPT_LACP_TIMEOUT		0x80		/* LACP timeout */
+#define	LAGG_OPT_RR_LIMIT		0x100		/* RR stride */
 	u_int			ro_count;		/* number of ports */
 	u_int			ro_active;		/* active port count */
 	u_int			ro_flapping;		/* number of flapping */