Author: truckman
Date: Fri May 19 01:23:06 2017
New Revision: 318511
URL: https://svnweb.freebsd.org/changeset/base/318511
Log:
The result of right shifting a negative signed value is implementation
defined. On machines without arithmetic shift instructions, zero bits
may be shifted in from the left, giving a large positive result instead
of the desired divide-by power-of-2. Fix this by operating on the
absolute value and compensating for the possible negation later.
Reverse the order of the underflow/overflow tests and the exponential
decay calculation to avoid the possibility of an erroneous overflow
detection if p is a sufficiently small non-negative value. Also
check for negative values of prob before doing the exponential decay
to avoid another instance of of right shifting a negative value.
Tested by: Rasool Al-Saadi <ralsa...@swin.edu.au>
MFC after: 1 week
Modified:
head/sys/netpfil/ipfw/dn_aqm_pie.c
head/sys/netpfil/ipfw/dn_sched_fq_pie.c
Modified: head/sys/netpfil/ipfw/dn_aqm_pie.c
==============================================================================
--- head/sys/netpfil/ipfw/dn_aqm_pie.c Fri May 19 00:43:49 2017
(r318510)
+++ head/sys/netpfil/ipfw/dn_aqm_pie.c Fri May 19 01:23:06 2017
(r318511)
@@ -206,6 +206,7 @@ calculate_drop_prob(void *x)
int64_t p, prob, oldprob;
struct dn_aqm_pie_parms *pprms;
struct pie_status *pst = (struct pie_status *) x;
+ int p_isneg;
pprms = pst->parms;
prob = pst->drop_prob;
@@ -221,6 +222,12 @@ calculate_drop_prob(void *x)
((int64_t)pst->current_qdelay - (int64_t)pprms->qdelay_ref);
p +=(int64_t) pprms->beta *
((int64_t)pst->current_qdelay - (int64_t)pst->qdelay_old);
+
+ /* take absolute value so right shift result is well defined */
+ p_isneg = p < 0;
+ if (p_isneg) {
+ p = -p;
+ }
/* We PIE_MAX_PROB shift by 12-bits to increase the division precision
*/
p *= (PIE_MAX_PROB << 12) / AQM_TIME_1S;
@@ -243,37 +250,47 @@ calculate_drop_prob(void *x)
oldprob = prob;
- /* Cap Drop adjustment */
- if ((pprms->flags & PIE_CAPDROP_ENABLED) && prob >= PIE_MAX_PROB / 10
- && p > PIE_MAX_PROB / 50 )
- p = PIE_MAX_PROB / 50;
+ if (p_isneg) {
+ prob = prob - p;
- prob = prob + p;
-
- /* decay the drop probability exponentially */
- if (pst->current_qdelay == 0 && pst->qdelay_old == 0)
- /* 0.98 ~= 1- 1/64 */
- prob = prob - (prob >> 6);
+ /* check for multiplication underflow */
+ if (prob > oldprob) {
+ prob= 0;
+ D("underflow");
+ }
+ } else {
+ /* Cap Drop adjustment */
+ if ((pprms->flags & PIE_CAPDROP_ENABLED) &&
+ prob >= PIE_MAX_PROB / 10 &&
+ p > PIE_MAX_PROB / 50 ) {
+ p = PIE_MAX_PROB / 50;
+ }
+ prob = prob + p;
- /* check for multiplication overflow/underflow */
- if (p>0) {
+ /* check for multiplication overflow */
if (prob<oldprob) {
D("overflow");
prob= PIE_MAX_PROB;
}
}
- else
- if (prob>oldprob) {
- prob= 0;
- D("underflow");
- }
- /* make drop probability between 0 and PIE_MAX_PROB*/
- if (prob < 0)
+ /*
+ * decay the drop probability exponentially
+ * and restrict it to range 0 to PIE_MAX_PROB
+ */
+ if (prob < 0) {
prob = 0;
- else if (prob > PIE_MAX_PROB)
- prob = PIE_MAX_PROB;
+ } else {
+ if (pst->current_qdelay == 0 && pst->qdelay_old == 0) {
+ /* 0.98 ~= 1- 1/64 */
+ prob = prob - (prob >> 6);
+ }
+
+ if (prob > PIE_MAX_PROB) {
+ prob = PIE_MAX_PROB;
+ }
+ }
pst->drop_prob = prob;
Modified: head/sys/netpfil/ipfw/dn_sched_fq_pie.c
==============================================================================
--- head/sys/netpfil/ipfw/dn_sched_fq_pie.c Fri May 19 00:43:49 2017
(r318510)
+++ head/sys/netpfil/ipfw/dn_sched_fq_pie.c Fri May 19 01:23:06 2017
(r318511)
@@ -377,6 +377,7 @@ fq_calculate_drop_prob(void *x)
struct dn_aqm_pie_parms *pprms;
int64_t p, prob, oldprob;
aqm_time_t now;
+ int p_isneg;
now = AQM_UNOW;
pprms = pst->parms;
@@ -393,6 +394,12 @@ fq_calculate_drop_prob(void *x)
((int64_t)pst->current_qdelay - (int64_t)pprms->qdelay_ref);
p +=(int64_t) pprms->beta *
((int64_t)pst->current_qdelay - (int64_t)pst->qdelay_old);
+
+ /* take absolute value so right shift result is well defined */
+ p_isneg = p < 0;
+ if (p_isneg) {
+ p = -p;
+ }
/* We PIE_MAX_PROB shift by 12-bits to increase the division precision
*/
p *= (PIE_MAX_PROB << 12) / AQM_TIME_1S;
@@ -415,37 +422,47 @@ fq_calculate_drop_prob(void *x)
oldprob = prob;
- /* Cap Drop adjustment */
- if ((pprms->flags & PIE_CAPDROP_ENABLED) && prob >= PIE_MAX_PROB / 10
- && p > PIE_MAX_PROB / 50 )
- p = PIE_MAX_PROB / 50;
+ if (p_isneg) {
+ prob = prob - p;
- prob = prob + p;
-
- /* decay the drop probability exponentially */
- if (pst->current_qdelay == 0 && pst->qdelay_old == 0)
- /* 0.98 ~= 1- 1/64 */
- prob = prob - (prob >> 6);
+ /* check for multiplication underflow */
+ if (prob > oldprob) {
+ prob= 0;
+ D("underflow");
+ }
+ } else {
+ /* Cap Drop adjustment */
+ if ((pprms->flags & PIE_CAPDROP_ENABLED) &&
+ prob >= PIE_MAX_PROB / 10 &&
+ p > PIE_MAX_PROB / 50 ) {
+ p = PIE_MAX_PROB / 50;
+ }
+ prob = prob + p;
- /* check for multiplication over/under flow */
- if (p>0) {
+ /* check for multiplication overflow */
if (prob<oldprob) {
D("overflow");
prob= PIE_MAX_PROB;
}
}
- else
- if (prob>oldprob) {
- prob= 0;
- D("underflow");
- }
- /* make drop probability between 0 and PIE_MAX_PROB*/
- if (prob < 0)
+ /*
+ * decay the drop probability exponentially
+ * and restrict it to range 0 to PIE_MAX_PROB
+ */
+ if (prob < 0) {
prob = 0;
- else if (prob > PIE_MAX_PROB)
- prob = PIE_MAX_PROB;
+ } else {
+ if (pst->current_qdelay == 0 && pst->qdelay_old == 0) {
+ /* 0.98 ~= 1- 1/64 */
+ prob = prob - (prob >> 6);
+ }
+
+ if (prob > PIE_MAX_PROB) {
+ prob = PIE_MAX_PROB;
+ }
+ }
pst->drop_prob = prob;
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
