Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 11 Feb 2019 22:58:43 +0000 (UTC)
From:      Patrick Kelsey <pkelsey@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r344025 - head/sbin/pfctl
Message-ID:  <201902112258.x1BMwhMB075098@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: pkelsey
Date: Mon Feb 11 22:58:43 2019
New Revision: 344025
URL: https://svnweb.freebsd.org/changeset/base/344025

Log:
  Fix the fix added in r343287 for spurious HFSC bandwidth check errors
  
  The logic added in r343287 to avoid false-positive
  sum-of-child-bandwidth check errors for HFSC queues has a bug in it
  that causes the upperlimit service curve of an HFSC queue to be pulled
  down to its parent's linkshare service curve if it happens to be above
  it.
  
  Upon further inspection/reflection, this generic
  sum-of-child-bandwidths check does not need to be fixed for HFSC - it
  needs to be skipped.  For HFSC, the equivalent check is to ensure the
  sum of child linkshare service curves are at or below the parent's
  linkshare service curve, and this check is already being performed by
  eval_pfqueue_hfsc().
  
  This commit reverts the affected parts of r343287 and adds new logic
  to skip the generic sum-of-child-bandwidths check for HFSC.
  
  MFC after:	1 day
  Sponsored by:	RG Nets
  Differential Revision:	https://reviews.freebsd.org/D19124

Modified:
  head/sbin/pfctl/pfctl_altq.c

Modified: head/sbin/pfctl/pfctl_altq.c
==============================================================================
--- head/sbin/pfctl/pfctl_altq.c	Mon Feb 11 22:09:26 2019	(r344024)
+++ head/sbin/pfctl/pfctl_altq.c	Mon Feb 11 22:58:43 2019	(r344025)
@@ -429,34 +429,25 @@ eval_pfqueue(struct pfctl *pf, struct pf_altq *pa, str
 	if (pa->qlimit == 0)
 		pa->qlimit = DEFAULT_QLIMIT;
 
-	if (eval_queue_opts(pa, opts,
-		parent == NULL ? pa->ifbandwidth : parent->pa.bandwidth))
-		return (1);
-
 	if (pa->scheduler == ALTQT_CBQ || pa->scheduler == ALTQT_HFSC ||
 		pa->scheduler == ALTQT_FAIRQ) {
 		pa->bandwidth = eval_bwspec(bw,
 		    parent == NULL ? pa->ifbandwidth : parent->pa.bandwidth);
 
-		/*
-		 * For HFSC, if the linkshare service curve m2 parameter is
-		 * set, it overrides the provided queue bandwidth parameter,
-		 * so adjust the queue bandwidth parameter accordingly here
-		 * to avoid false positives in the total child bandwidth
-		 * check below.
-		 */
-		if ((pa->scheduler == ALTQT_HFSC) &&
-		    (pa->pq_u.hfsc_opts.lssc_m2 != 0)) {
-			pa->bandwidth = pa->pq_u.hfsc_opts.lssc_m2;
-		}
-
 		if (pa->bandwidth > pa->ifbandwidth) {
 			fprintf(stderr, "bandwidth for %s higher than "
 			    "interface\n", pa->qname);
 			return (1);
 		}
-		/* check the sum of the child bandwidth is under parent's */
-		if (parent != NULL) {
+		/*
+		 * If not HFSC, then check that the sum of the child
+		 * bandwidths is less than the parent's bandwidth.  For
+		 * HFSC, the equivalent concept is to check that the sum of
+		 * the child linkshare service curves are under the parent's
+		 * linkshare service curve, and that check is performed by
+		 * eval_pfqueue_hfsc().
+		 */
+		if ((parent != NULL) && (pa->scheduler != ALTQT_HFSC)) {
 			if (pa->bandwidth > parent->pa.bandwidth) {
 				warnx("bandwidth for %s higher than parent",
 				    pa->qname);
@@ -471,6 +462,10 @@ eval_pfqueue(struct pfctl *pf, struct pf_altq *pa, str
 			}
 		}
 	}
+
+	if (eval_queue_opts(pa, opts,
+		parent == NULL ? pa->ifbandwidth : parent->pa.bandwidth))
+		return (1);
 
 	if (parent != NULL)
 		parent->meta.children++;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201902112258.x1BMwhMB075098>