Re: [Cake] [sch_cake] Merge branch 'exposeinterval' (5541271)

Re: [Cake] [sch_cake] Merge branch 'exposeinterval' (5541271)

[Sebastian Moeller]

Hi Kevin,

On Oct 12, 2015, at 10:55 , Kevin Darbyshire-Bryant <notifications@github.com> wrote:

> It's worse/better than that - if you look at cake_set_rate it actually sets the interval to the max of target(5ms) * 8 or the supplied interval parameter, so actually as it currently stands the lowest you can ACTUALLY set interval to is 40ms.

	Good to know, data center people might not appreciate that given that for fq_codel a target value of 500µsec seems to be recommended (now whether fq_codel/cake really are applicable for data centers is another question). But since I have no data center available I would not know how to test this, so others need to chime in.
	(Note to self, it might make sense to also expose the target-parameter, potentially not in raw time units, but as a percentage of interval, as specified in the codel RFC draft section "4.4.  The target Setpoint,”, see https://datatracker.ietf.org/doc/draft-ietf-aqm-codel/?include_text=1 ).

> 
> I put those limits in purely as an anti hack/tc stupidity/pointer gone wild - defensive programming thing. Take 'em out/tweak/whatever.

	Oh, I am all for sanity and playing it safe, this is why I wondered whether the actual limits will work for the targeted audience. It would be sweet if the initial limits of the upstream commit could survive some time, so sqm-scripts does not need any gymnastics to inform users about changed behavior (so I am asking for purely selfish reasons) ;)
	Loganaden, since you have experience, how long are your RTTs  from Mauritius to France and both US coasts (as a proxy for “typical” traffic destinations); so we can taylor the limits to allow that.

Best Regards
	Sebastian

> 
> —
> Reply to this email directly or view it on GitHub.
> 

Re: [Cake] [sch_cake] Merge branch 'exposeinterval' (5541271)

[Dave Taht]

On Mon, Oct 12, 2015 at 11:08 AM, Sebastian Moeller <moeller0@gmx.de> wrote:
> Hi Kevin,
>
> On Oct 12, 2015, at 10:55 , Kevin Darbyshire-Bryant <notifications@github.com> wrote:
>
>> It's worse/better than that - if you look at cake_set_rate it actually sets the interval to the max of target(5ms) * 8 or the supplied interval parameter, so actually as it currently stands the lowest you can ACTUALLY set interval to is 40ms.

This is incorrect for the DC use case. We should be able to have the
target fall below at least 250us.

>         Good to know, data center people might not appreciate that given that for fq_codel a target value of 500µsec seems to be recommended (now whether fq_codel/cake really are applicable for data centers is another question). But since I have no data center available I would not know how to test this, so others need to chime in.
>         (Note to self, it might make sense to also expose the target-parameter, potentially not in raw time units, but as a percentage of interval, as specified in the codel RFC draft section "4.4.  The target Setpoint,”, see https://datatracker.ietf.org/doc/draft-ietf-aqm-codel/?include_text=1 ).
>
>>
>> I put those limits in purely as an anti hack/tc stupidity/pointer gone wild - defensive programming thing. Take 'em out/tweak/whatever.
>
>         Oh, I am all for sanity and playing it safe, this is why I wondered whether the actual limits will work for the targeted audience. It would be sweet if the initial limits of the upstream commit could survive some time, so sqm-scripts does not need any gymnastics to inform users about changed behavior (so I am asking for purely selfish reasons) ;)
>         Loganaden, since you have experience, how long are your RTTs  from Mauritius to France and both US coasts (as a proxy for “typical” traffic destinations); so we can taylor the limits to allow that.
>
> Best Regards
>         Sebastian
>
>>
>> —
>> Reply to this email directly or view it on GitHub.
>>
>



-- 
Dave Täht
Do you want faster, better, wifi? https://www.patreon.com/dtaht

Re: [Cake] [sch_cake] Merge branch 'exposeinterval' (5541271)

[Jonathan Morton]

> On 12 Oct, 2015, at 12:24, Dave Taht <dave.taht@gmail.com> wrote:
> 
> This is incorrect for the DC use case. We should be able to have the
> target fall below at least 250us.

Hey, I’m trying to work on this myself, but the code keeps changing under me.

 - Jonathan Morton

Re: [Cake] [sch_cake] Merge branch 'exposeinterval' (5541271)

[Dave Taht]

On Mon, Oct 12, 2015 at 11:28 AM, Jonathan Morton <chromatix99@gmail.com> wrote:
>
>> On 12 Oct, 2015, at 12:24, Dave Taht <dave.taht@gmail.com> wrote:
>>
>> This is incorrect for the DC use case. We should be able to have the
>> target fall below at least 250us.
>
> Hey, I’m trying to work on this myself, but the code keeps changing under me.

Tough. Deal with it. :) Once the rtt option works right toke and I can
finally go test the
count/2 idea out properly in the testbed, like, today. You otherwise
have the pen,
as far as I am concerned.

>
>  - Jonathan Morton
>



-- 
Dave Täht
Do you want faster, better, wifi? https://www.patreon.com/dtaht

Re: [Cake] [sch_cake] Merge branch 'exposeinterval' (5541271)

[Dave Taht]

um, no, interval needs to be kept in usec, and converted more like I did.

On Mon, Oct 12, 2015 at 2:36 AM, Dave Taht <dave.taht@gmail.com> wrote:
> On Mon, Oct 12, 2015 at 11:28 AM, Jonathan Morton <chromatix99@gmail.com> wrote:
>>
>>> On 12 Oct, 2015, at 12:24, Dave Taht <dave.taht@gmail.com> wrote:
>>>
>>> This is incorrect for the DC use case. We should be able to have the
>>> target fall below at least 250us.
>>
>> Hey, I’m trying to work on this myself, but the code keeps changing under me.
>
> Tough. Deal with it. :) Once the rtt option works right toke and I can
> finally go test the
> count/2 idea out properly in the testbed, like, today. You otherwise
> have the pen,
> as far as I am concerned.
>
>>
>>  - Jonathan Morton
>>
>
>
>
> --
> Dave Täht
> Do you want faster, better, wifi? https://www.patreon.com/dtaht



-- 
Dave Täht
Do you want faster, better, wifi? https://www.patreon.com/dtaht

Re: [Cake] [sch_cake] Merge branch 'exposeinterval' (5541271)

[Sebastian Moeller]

Hi Dave,


On Oct 12, 2015, at 13:16 , Dave Taht <dave.taht@gmail.com> wrote:

> um, no, interval needs to be kept in usec, and converted more like I did.

	I have not looked at your code yet. In tc (tc_utils.h or so) there is get_time() that knows how to deal with the default time specifiers like sec ms us… I believe from the tc side this should be used to parse the user input, just like Kevin’s code does. Now this needs to be passed to sch_cake.c, where I believe Kevin’s version expects milliseconds (MS2TIME(q->interval)), which does not do the right thing if the user specified “rtf 123usecs” on tic’s cli… So just getting rid of the MS2TIME called should do the trick, no?

Best Regards
	Sebastian
	
> 
> On Mon, Oct 12, 2015 at 2:36 AM, Dave Taht <dave.taht@gmail.com> wrote:
>> On Mon, Oct 12, 2015 at 11:28 AM, Jonathan Morton <chromatix99@gmail.com> wrote:
>>> 
>>>> On 12 Oct, 2015, at 12:24, Dave Taht <dave.taht@gmail.com> wrote:
>>>> 
>>>> This is incorrect for the DC use case. We should be able to have the
>>>> target fall below at least 250us.
>>> 
>>> Hey, I’m trying to work on this myself, but the code keeps changing under me.
>> 
>> Tough. Deal with it. :) Once the rtt option works right toke and I can
>> finally go test the
>> count/2 idea out properly in the testbed, like, today. You otherwise
>> have the pen,
>> as far as I am concerned.
>> 
>>> 
>>> - Jonathan Morton
>>> 
>> 
>> 
>> 
>> --
>> Dave Täht
>> Do you want faster, better, wifi? https://www.patreon.com/dtaht
> 
> 
> 
> -- 
> Dave Täht
> Do you want faster, better, wifi? https://www.patreon.com/dtaht

Re: [Cake] [sch_cake] Merge branch 'exposeinterval' (5541271)

[Dave Taht]

sigh.

In looking this code over again, interval is a 64 bit value in nsec in
our version of codel, and needs no conversion into usec. It also seems
to imply that all the conversion values are wrong throughout the code,
and the api should expose a 64 bit value.

On Mon, Oct 12, 2015 at 1:49 PM, Sebastian Moeller <moeller0@gmx.de> wrote:
> Hi Dave,
>
>
> On Oct 12, 2015, at 13:16 , Dave Taht <dave.taht@gmail.com> wrote:
>
>> um, no, interval needs to be kept in usec, and converted more like I did.
>
>         I have not looked at your code yet. In tc (tc_utils.h or so) there is get_time() that knows how to deal with the default time specifiers like sec ms us… I believe from the tc side this should be used to parse the user input, just like Kevin’s code does. Now this needs to be passed to sch_cake.c, where I believe Kevin’s version expects milliseconds (MS2TIME(q->interval)), which does not do the right thing if the user specified “rtf 123usecs” on tic’s cli… So just getting rid of the MS2TIME called should do the trick, no?
>
> Best Regards
>         Sebastian
>
>>
>> On Mon, Oct 12, 2015 at 2:36 AM, Dave Taht <dave.taht@gmail.com> wrote:
>>> On Mon, Oct 12, 2015 at 11:28 AM, Jonathan Morton <chromatix99@gmail.com> wrote:
>>>>
>>>>> On 12 Oct, 2015, at 12:24, Dave Taht <dave.taht@gmail.com> wrote:
>>>>>
>>>>> This is incorrect for the DC use case. We should be able to have the
>>>>> target fall below at least 250us.
>>>>
>>>> Hey, I’m trying to work on this myself, but the code keeps changing under me.
>>>
>>> Tough. Deal with it. :) Once the rtt option works right toke and I can
>>> finally go test the
>>> count/2 idea out properly in the testbed, like, today. You otherwise
>>> have the pen,
>>> as far as I am concerned.
>>>
>>>>
>>>> - Jonathan Morton
>>>>
>>>
>>>
>>>
>>> --
>>> Dave Täht
>>> Do you want faster, better, wifi? https://www.patreon.com/dtaht
>>
>>
>>
>> --
>> Dave Täht
>> Do you want faster, better, wifi? https://www.patreon.com/dtaht
>



-- 
Dave Täht
Do you want faster, better, wifi? https://www.patreon.com/dtaht

Re: [Cake] [sch_cake] Merge branch 'exposeinterval' (5541271)

[Sebastian Moeller]

Hi Dave,

On Oct 12, 2015, at 14:04 , Dave Taht <dave.taht@gmail.com> wrote:

> sigh.
> 
> In looking this code over again, interval is a 64 bit value in nsec in
> our version of codel, and needs no conversion into usec. It also seems
> to imply that all the conversion values are wrong throughout the code,
> and the api should expose a 64 bit value.

	But tc will only pass in an unsigned worth of microseconds, so there needs to be a conversion to nsecs, no? Isn’t MS2TIME doing that conversion for us, so maybe we need US2TIME to handle time values passed from tc (so interval and potentially target).

Best Regards
	Sebastian

> 
> On Mon, Oct 12, 2015 at 1:49 PM, Sebastian Moeller <moeller0@gmx.de> wrote:
>> Hi Dave,
>> 
>> 
>> On Oct 12, 2015, at 13:16 , Dave Taht <dave.taht@gmail.com> wrote:
>> 
>>> um, no, interval needs to be kept in usec, and converted more like I did.
>> 
>>        I have not looked at your code yet. In tc (tc_utils.h or so) there is get_time() that knows how to deal with the default time specifiers like sec ms us… I believe from the tc side this should be used to parse the user input, just like Kevin’s code does. Now this needs to be passed to sch_cake.c, where I believe Kevin’s version expects milliseconds (MS2TIME(q->interval)), which does not do the right thing if the user specified “rtf 123usecs” on tic’s cli… So just getting rid of the MS2TIME called should do the trick, no?
>> 
>> Best Regards
>>        Sebastian
>> 
>>> 
>>> On Mon, Oct 12, 2015 at 2:36 AM, Dave Taht <dave.taht@gmail.com> wrote:
>>>> On Mon, Oct 12, 2015 at 11:28 AM, Jonathan Morton <chromatix99@gmail.com> wrote:
>>>>> 
>>>>>> On 12 Oct, 2015, at 12:24, Dave Taht <dave.taht@gmail.com> wrote:
>>>>>> 
>>>>>> This is incorrect for the DC use case. We should be able to have the
>>>>>> target fall below at least 250us.
>>>>> 
>>>>> Hey, I’m trying to work on this myself, but the code keeps changing under me.
>>>> 
>>>> Tough. Deal with it. :) Once the rtt option works right toke and I can
>>>> finally go test the
>>>> count/2 idea out properly in the testbed, like, today. You otherwise
>>>> have the pen,
>>>> as far as I am concerned.
>>>> 
>>>>> 
>>>>> - Jonathan Morton
>>>>> 
>>>> 
>>>> 
>>>> 
>>>> --
>>>> Dave Täht
>>>> Do you want faster, better, wifi? https://www.patreon.com/dtaht
>>> 
>>> 
>>> 
>>> --
>>> Dave Täht
>>> Do you want faster, better, wifi? https://www.patreon.com/dtaht
>> 
> 
> 
> 
> -- 
> Dave Täht
> Do you want faster, better, wifi? https://www.patreon.com/dtaht