From owner-svn-src-head@freebsd.org Mon Feb 11 22:58:44 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 1A91F14E63B8; Mon, 11 Feb 2019 22:58:44 +0000 (UTC) (envelope-from pkelsey@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id B41388DA20; Mon, 11 Feb 2019 22:58:43 +0000 (UTC) (envelope-from pkelsey@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id A13F16BDE; Mon, 11 Feb 2019 22:58:43 +0000 (UTC) (envelope-from pkelsey@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x1BMwhmc075099; Mon, 11 Feb 2019 22:58:43 GMT (envelope-from pkelsey@FreeBSD.org) Received: (from pkelsey@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x1BMwhMB075098; Mon, 11 Feb 2019 22:58:43 GMT (envelope-from pkelsey@FreeBSD.org) Message-Id: <201902112258.x1BMwhMB075098@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: pkelsey set sender to pkelsey@FreeBSD.org using -f From: Patrick Kelsey Date: Mon, 11 Feb 2019 22:58:43 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r344025 - head/sbin/pfctl X-SVN-Group: head X-SVN-Commit-Author: pkelsey X-SVN-Commit-Paths: head/sbin/pfctl X-SVN-Commit-Revision: 344025 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: B41388DA20 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.97 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.995,0]; NEURAL_HAM_LONG(-1.00)[-0.999,0]; NEURAL_HAM_SHORT(-0.97)[-0.971,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Feb 2019 22:58:44 -0000 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++;