[altq 1451] Re: Another source of HFSC-ALTQ unfairness

[Kenjiro Cho <kjc@csl.sony.co.jp>]

Oleg Cherevko wrote:
> What is written below is rather low-level and is intended
> mostly for those who knows HFSC internals rather well.
> 

[snip]

> Well, what about solutions?
> Currently I see 2 possible paths for solutions:
> 
> 1) Make sure vt values never decrease.
>    This is probably not good, becuase vt (that may increase much
>    faster than the real time if class gets lot of excess bandwidth)
>    might overflow.
> 
> 2) Instead of doing rtsc_min() simply do rtsc_init() when
>    appropriate (I'm not sure right now *when exactly* this is
>    appropriate). This also requires more investigation as it may
>    introduce some other unwanted effects.
> 
> So right now I have no patch ready to fix this behaviuor and I'm
> still thinking. 

I don't have enough time to look into the issue carefully, but you
seem to be correct.  It isn't right to reset vt and leave the runtime
service curve of the previous period.

What do you think about the following patch for init_v()?
(I'll take a closer look tomorrow.)

--- altq_hfsc.c.orig	Fri Apr 26 00:43:21 2002
+++ altq_hfsc.c	Fri Apr 26 01:07:10 2002
@@ -861,6 +861,17 @@
 			/* already active */
 			break;
 
+		/*
+		 * if parent became idle while this class was idle,
+		 * reset vt and the runtime service curve.
+		 */
+		if (cl->cl_parent->cl_nactive == 0 ||
+		    cl->cl_parent->cl_vtperiod != cl->cl_parentperiod) {
+			cl->cl_vt = 0;
+			rtsc_init(&cl->cl_virtual, cl->cl_fsc,
+				  0, cl->cl_total);
+		}
+
 		min_cl = actlist_first(cl->cl_parent->cl_actc);
 		if (min_cl != NULL) {
 			u_int64_t vt;
@@ -875,14 +886,10 @@
 			if (cl->cl_parent->cl_vtperiod != cl->cl_parentperiod
 			    || vt > cl->cl_vt)
 				cl->cl_vt = vt;
-		} else {
-			/* no packet is backlogged.  set vt to 0 */
-			cl->cl_vt = 0;
 		}
 
 		/* update the virtual curve */
-		rtsc_min(&cl->cl_virtual, cl->cl_fsc,
-			 cl->cl_vt, cl->cl_total);
+		rtsc_min(&cl->cl_virtual, cl->cl_fsc, cl->cl_vt, cl->cl_total);
 
 		cl->cl_vtperiod++;  /* increment vt period */
 		cl->cl_parentperiod = cl->cl_parent->cl_vtperiod;


> It would be nice to look at other HFSC implementations to see how
> they handle init_v(). Perhaps, somebody could help me with sources
> of other HFSCs? What I have found so far is some older CMU 
> implementation from here:
>   http://www-2.cs.cmu.edu/afs/cs.cmu.edu/project/cmcl-darwin/kernel/kernel.common/hfsc/src/

This is the only source that the CMU guys were using for both
simulations and experiments.

> But they seem to do something very different in their
> changeSessionStatus() function that I'm still unable to relate
> neither to HFSC-ALTQ nor to the original HFSC paper.

That's why I decided to rewrite the code from scratch :)

-Kenjiro