[Cerowrt-devel] aqm gui feedback on cerowrt-3.10.24-1 for linklayer adaption

[Sebastian Moeller]

Hi Dave,


On Dec 14, 2013, at 07:26 , Dave Taht <dave.taht at gmail.com> wrote:

> one of the things that makes me happy with all-up testing is that
> occasionally after completely blowing up my own work, I get to
> critique fresh work that isn't mine, in an area with which I have no
> expertise, with gratitude that I don't have to figure out the answer.
> :)
> 
> So I spent some time clicking wildly all over the AQM gui webpage to
> see what I could break.
> 
> 1) the aqm gui code doesn't work due to a bug at line 66.
> sc:depends("advanced", "1").
> sc has to be initialized first, which happens later in the file. Extra
> line removed in ceropackages, committed, pushed, you will need to do a
> pull. Merge failure?

	Worse, process failure, instead of actually committing tested code I manually edited some changes ("that could not break") into the repository and pushed those without actually testing them. Will try to be more careful in the future
I put in an additional "s" so in stead of "sc:depends("advanced", "1")" it should have read "c:depends("advanced", "1")" as the current identifier in that section is c. I pulled your changes, made my edit and puhed it again.

> 
> 2) it's not clear to me we have to support both the stab and
> htb_private methods of fixing htb's linklayer. It was important that
> these be fixed for everyone else that uses htb,
> but is one of these is faster than the other?

	So, tc_stab is generic in that it should work with HFSC, HTB and TBF, while htb_private will only work with HTB (it seems TBF also has a private method but I have no clue whether that actually works, I just noticed that someone from huawei is posting changes to TBF on linux net-dev). My take is that we should just stick to stab so we can keep the same configuration fiels for most scripts people might come up with (thing free.qos without HTB). I have no idea which is faster though.

> I seem to recall one was
> a calculated value in the kernel, the other some sort of table.

	If I recall correctly HTB lost its internal tables to allow higher rates and/or precision; Jesper than fixed the htb_private method to account for the link layer on the fly. So currently HTB is more advanced than tc_stab, since HTB will allow arbitrarily large packets and still do the right thing, while tc_stab will either need humungous tables or will not work with jumbo packets and GRO. I think for shaping on a home router we could not care less. People who can afford such large packets and GRO on the router probably have bandwidths available that cerowrt does not handle well anyway (picture me heavily handwaving here).

> Does
> this choice need to be made by the
> user?

	Well, no the user should not care. We should make that decision for the user; then again unless we are able to constantly check both methods against each other one will bitrott, so maybe we should default to tc_stab and make htb_private an advanced configuration option? Also we should try to drape tc_stab into the future and teach it the same trick htb_private does (and then also fix the fact that tc_stab ignores the information we have about overhead). If no one beats me to it I will try to prepare my first patch to the kernel to fix this sometime next year; but I reallr really have no time for that in the near future, as I have to papers to write and grants to write as well as apply for a new position. (Also I am not too keen of getting a patch into the kernel, I just want this issue fixed; but since it looks this itches me most...)

> The two variants benchmarked? Jesper?
> 
> 3) Clicking "advanced configuration" on and off toggles display of the
> qdisc and qdisc script,

	Did it really? I think with your change only the qdisc
  script should have toggled (but I am no Luci expert).

> and twiddling with the linklayer value brings
> up all the extra DSL detail. Yea!
> 
> ... and I think I was wrong in mentally visualizing the thing
> 
> If these were made tabs [Basic, Queueing Discipline, Linklayer,
> Priorities], there would be more room for explanatory text in
> particular and better alignment with the
> "look and feel" of the rest of the gui.

	I agree, then we would not need to hide anything, and could bring on more options like "interval" and "target" (and/or some of the pie details)

> Note that "priorities" is a
> placeholder for somehow
> bringing out something remotely similar to what openwrt's qos system
> already does
> and what AQM (ceroshaper? some other name is needed) does implicitly
> with optimizing for dns and ntp.

	So a way to place different packets into the different priority bands (in simple.qos)?

> 
> ECN enablement should be brought out in "Queueing discipline" via the
> ALLECN variable. It seems likely ALLECN needs to have 4 states rather
> than 3, which needs to also be fixed in the scripts.

	What about two fields then "ECN inbound" and "ECN outbound", same for states but easier to understand...

> 
> While I'm at it, perhaps having tabs for each physical interface is
> not a horrible idea,
> but I shudder to think of people rate-limiting their wifi in the hope
> that that would help.
> 
> ?

	I would rather propose, I have a look at disabling the ability to add and delete "Queues" sections, one should be enough.

> 
> 5) Adding a second interface shows @ge01 as an option, which isn't a
> real interface, and se00 as an option and not the gw* or sw*
> interfaces.

	If I press the add option I can see all interfaces?

> Adding se00 with the default option
> gives me an error
> 
> One or more required fields have no value!
> One or more required fields have no value!
> One or more required fields have no value!
> One or more required fields have no value!

	I assume you did not fill the bandwidth fields?

> 
> (and I'm pretty sure the aqm-scripts break even if this is correctly
> written to the config file)

	Yeah, I would say, let's try to disable this on more than one interface. This requirement seems advanced enough that requesters can be bothered to script it themselves :)

> 
> 6) feel free to add your copyright to the code.  :)

	Thanks, but basically I am still just only rearranging your code and Toke's so I am not sure there is anything to copy yet :)

> 
> I return now to figuring out why bringing up the wifi is so hosed. I
> will probably be reverting the kernel, netifd, and other things, way,
> way, way back to when they used to work.

	So for me things worked well with 3.10.21-1. I have no idea how much work it is for you to roll a new -2 version, but with a 3.10.21-2 including Sujith's atheros patch I would be happy to see whether the TX DMA failures are gone (assuming that those are connected to the disappearing radios). But just an offer, I would not be unhappy if abler hands than mine would fix this :).


Best Regards
	Sebastian
 

> 
> -- 
> Dave Täht
> 
> Fixing bufferbloat with cerowrt: http://www.teklibre.com/cerowrt/subscribe.html
> _______________________________________________
> Cerowrt-devel mailing list
> Cerowrt-devel at lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cerowrt-devel