d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

The result of right shifting a negative signed value is implementation

Browse Source 
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 <ralsaadi@swin.edu.au>
MFC after:	1 week
This commit is contained in:
Don Lewis 2017-05-19 01:23:06 +00:00
parent 9ebaf5f802
commit 36fb8be630
2 changed files with 76 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
sys/netpfil/ipfw/dn_aqm_pie.c
View File

@ -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 )
if (p_isneg) {
prob = prob - p;
/* 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;
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 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");
/*
* decay the drop probability exponentially
* and restrict it to range 0 to PIE_MAX_PROB
*/
if (prob < 0) {
prob = 0;
} else {
if (pst->current_qdelay == 0 && pst->qdelay_old == 0) {
/* 0.98 ~= 1- 1/64 */
prob = prob - (prob >> 6);
}
/* make drop probability between 0 and PIE_MAX_PROB*/
if (prob < 0)
prob = 0;
else if (prob > PIE_MAX_PROB)
prob = PIE_MAX_PROB;
if (prob > PIE_MAX_PROB) {
prob = PIE_MAX_PROB;
}
}
pst->drop_prob = prob;

59
sys/netpfil/ipfw/dn_sched_fq_pie.c
View File

@ -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 )
if (p_isneg) {
prob = prob - p;
/* 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;
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 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");
/*
* decay the drop probability exponentially
* and restrict it to range 0 to PIE_MAX_PROB
*/
if (prob < 0) {
prob = 0;
} else {
if (pst->current_qdelay == 0 && pst->qdelay_old == 0) {
/* 0.98 ~= 1- 1/64 */
prob = prob - (prob >> 6);
}
/* make drop probability between 0 and PIE_MAX_PROB*/
if (prob < 0)
prob = 0;
else if (prob > PIE_MAX_PROB)
prob = PIE_MAX_PROB;
if (prob > PIE_MAX_PROB) {
prob = PIE_MAX_PROB;
}
}
pst->drop_prob = prob;