On Mon, Jun 25, 2012 at 8:20 AM, Eric Dumazet <[email protected]> wrote: > On Mon, 2012-06-25 at 07:50 -0700, Dave Taht wrote: >> On Sun, Jun 24, 2012 at 10:22 PM, Eric Dumazet <[email protected]> >> wrote: >> > On Sun, 2012-06-24 at 22:00 -0700, Dave Täht wrote: >> >> From: Dave Taht <[email protected]> > >> >> codel_Newton_step(vars); >> >> - if (params->ecn && INET_ECN_set_ce(skb)) { >> >> + if (params->ecn && >> >> + params->ecn_target > vars->ldelay && >> > >> > Wrong test ? >> > >> > We want ECN if delay < ecn_target, not if delay > ecn_target >> > >> > (unresponsive flows will make delay being above ecn_target, while >> > responsive ones should make delay more like target ( < en_target) >> > >> > if (params->ecn && >> > vars->ldelay <= params->ecn_target && >> > INET_ECN_set_ce(skb)) { >> >> The orig is the same test, as params->ecn_target > vars->ldelay >> is equivalent to vars->ldelay <= params->ecn_target > > Yes, I must say I dont like the testing of a variable against a LIMIT > using > > if (LIMIT > variable) > > I prefer for readability > > if (variable < LIMIT) > > If you take a look at linux code, your form is very seldom used. > >> My own mental debate was whether to switch ecn from a bool to being a >> codel_time_t, >> and use a value of 0 for noecn and whatever for the ecn target value. > > Well, why not.
It was the way I was leaning until I observed me dropping ecn enabled mosh, ssh, and babel packets , which tend to be small, so I started thinking in terms of dropping ecn packets on a graduated packet size scale after exceeding target. ESPECIALLY in case of overload you want command and control packets to get through. ... So I figured getting another RFC out was a good idea. O brave new world that has such new drop strategies in it! > > -- Dave Täht http://www.bufferbloat.net/projects/cerowrt/wiki - "3.3.8-6 is out with fq_codel!" _______________________________________________ Codel mailing list [email protected] https://lists.bufferbloat.net/listinfo/codel
