Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 26 Jan 2016 04:48:25 +0000 (UTC)
From:      Luigi Rizzo <luigi@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r294761 - head/sys/netpfil/ipfw
Message-ID:  <201601260448.u0Q4mPW2074250@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: luigi
Date: Tue Jan 26 04:48:24 2016
New Revision: 294761
URL: https://svnweb.freebsd.org/changeset/base/294761

Log:
  Revert one chunk from commit 285362, which introduced an off-by-one error
  in computing a shift index. The error was due to the use of mixed
  fls() / __fls() functions in another implementation of qfq.
  To avoid that the problem occurs again, properly document which
  incarnation of the function we need.
  Note that the bug only affects QFQ in FreeBSD head from last july, as
  the patch was not merged to other versions.

Modified:
  head/sys/netpfil/ipfw/dn_sched_qfq.c

Modified: head/sys/netpfil/ipfw/dn_sched_qfq.c
==============================================================================
--- head/sys/netpfil/ipfw/dn_sched_qfq.c	Tue Jan 26 04:41:18 2016	(r294760)
+++ head/sys/netpfil/ipfw/dn_sched_qfq.c	Tue Jan 26 04:48:24 2016	(r294761)
@@ -60,6 +60,10 @@ typedef	unsigned long	bitmap;
 /*
  * bitmaps ops are critical. Some linux versions have __fls
  * and the bitmap ops. Some machines have ffs
+ * NOTE: fls() returns 1 for the least significant bit,
+ *       __fls() returns 0 for the same case.
+ * We use the base-0 version __fls() to match the description in
+ * the ToN QFQ paper
  */
 #if defined(_WIN32) || (defined(__MIPSEL__) && defined(LINUX_24))
 int fls(unsigned int n)
@@ -409,8 +413,8 @@ qfq_make_eligible(struct qfq_sched *q, u
 	old_vslot = old_V >> QFQ_MIN_SLOT_SHIFT;
 
 	if (vslot != old_vslot) {
-		/* should be 1ULL not 2ULL */
-		mask = (1ULL << (__fls(vslot ^ old_vslot))) - 1;
+		/* must be 2ULL, see ToN QFQ article fig.5, we use base-0 fls */
+		mask = (2ULL << (__fls(vslot ^ old_vslot))) - 1;
 		qfq_move_groups(q, mask, IR, ER);
 		qfq_move_groups(q, mask, IB, EB);
 	}



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