tohojo / sqm-scripts

SQM scripts traffic shaper
http://www.bufferbloat.net/projects/cerowrt/wiki/Smart_Queue_Management
236 stars 63 forks source link

do_modules: module - existence checking and loading #13

Closed braveheartleo closed 9 years ago

braveheartleo commented 9 years ago

In function do_modules() in /usr/lib/sqm/functions.sh, there is a TODO note to check first whether the modules exist and only load then. How does this implementation look? Is it good enough?

do_modules() {
    KVER=$(uname -r)
    for i in act_ipt sch_$QDISC sch_ingress act_mirred cls_fw cls_u32 sch_htb sch_hfsc
    do
        [ -f /lib/modules/$KVER/$i.ko ] && insmod $i
    done
}

In case of a built-in module, then there's no need to insmod such module, because it is already part of the running kernel. This also avoids errors such as failed to find a module named sch_fq_codel because sch_fq_codel is a built-in module, and insmod sch_fq_codel results in such error.

Cheers, ianp

moeller0 commented 9 years ago

HI ianp,

On Aug 30, 2015, at 05:18 , braveheartleo notifications@github.com wrote:

In function do_modules() in /usr/lib/sqm/functions.sh, there is a TODO note to check first whether the modules exist and only load then. How does this implementation look? Is it good enough?

do_modules() { KVER=$(uname -r) for i in actipt sch$QDISC sch_ingress act_mirred cls_fw cls_u32 sch_htb sch_hfsc do [ -f /lib/modules/$KVER/$i.ko ] && insmod $i done }

In case of a built-in module, then there's no need to insmod such module, because it is already part of the running kernel. This also avoids errors such as failed to find a module named sch_fq_codel because sch_fq_codel is a built-in module, and insmod sch_fq_codel results in such error.

This function could really use more smarts, I fully agree. The way I see it we have 3 separate issues to solve: 1) making sure modules are loaded before (first) use, as some distributions are not set up for module autoloading (to sure which distributions are affected, so this might be a wring assumption); 2) we actually should check whether a module is loaded already before loading it, because re-loading an already loaded module will produce an error message if I recall correctly; 3) which of the required functionality is requiring a module or is build-in the current kernel. 

2) and 3) are related in that we should actually check for feature availability before trying to locate and load modules. Do to not having a solution for 2) I punted and just accepted the occasional error messages from do_module, given that on my cerowrt system these are benign as the module/functionality is known to exist�

The current code makes sure that 1) is solved, accepting the cost of false negatives (error messages) for in-builts, after the function is run the failure mode, module "available but not loaded" should be settled. Module not available or module already loaded are not well-handled. You are correct that it is a good idea to avoid going through the motion of module loading if no module is available.

What would really be helpful would be a way to check for 3). We do this for sch_${QDSIC} and we might be able to do this for sch_hfsc, sch_htb as well and maybe sch_ingress. If you find a way to check for act_ipt, cls_fw, and cls_u32 functionality that would be great. I guess we need to do something similar as for the qdiscs, namely try to use the functionality and see whether that errors out...

I also believe that our scripts now have differing module/functionality requirements and hence the actual scripts should supply the list of functions/modules they require. So, unless one uses the one script using hfsc thee is no real need to load that module, but if we actually use that script we should error out if we do not have hfsc available�

I realize most of this is out of scope of your proposed improvement though ;). So regarding your proposal, I believe Toke had some code to detect modules that not only worked for openwrt, but also for other distributions, like Arch and OpenSUSE, I believe. Maybe we should use that for the module checks. 

Regarding already loaded modules, see 2) above:

root@nacktmulle:~# modprobe sch_hfsc root@nacktmulle:~# cat /sys/module/sch_hfsc/initstate live root@nacktmulle:~# rmmod sch_hfsc root@nacktmulle:~# cat /sys/module/sch_hfsc/initstate cat: can't open '/sys/module/sch_hfsc/initstate': No such file or directory root@nacktmulle:~#

That might be a way to detect loaded modules�

Best Regards Sebastian

Cheers, ianp

� Reply to this email directly or view it on GitHub.

moeller0 commented 9 years ago

So to be more productive the do_modules() function should basically do something like the following:

1) check whether module is loaded, if not 2) check whether module is available, if not return error message otherwise 3) load module

Note that after we checked 1) we can ignore 2) and just check wether 3) returns an error... But then we can simply do 3) and check the output: root@nacktmulle:~# rmmod sch_hfsc root@nacktmulle:~# modprobe sch_hfsc9 kmod: failed to find a module named sch_hfsc9 root@nacktmulle:~# modprobe sch_hfsc root@nacktmulle:~# modprobe sch_hfsc kmod: sch_hfsc is already loaded

In other words do_module should just lean to check mod probes output (note openwrt and opensuse have different error messages here :( )

Since that will not catch the in-builts we probably also need a check_prerequisits():

1) check whether desired functionality is available, if not 2) do_module() (ewe actually need slightly less than do_modules, as we know the module should not be loaded, but let's keep it simple ;) ) 3) check desired functionality again, error out if not available

I guess I will have a look at check_prerequisites somewhen in the future, unless someone beats me to it...

Best Regards Sebastian

tohojo commented 9 years ago

Might I suggest trying to figure out to what extent we can actually rely on module autoloading these days? We might be able to avoid a lot of complexity if we can actually trust the operating system to do the right thing :)

moeller0 commented 9 years ago

Hmm, Toke you a point; I somehow like to keep this as an additional line of defense. If someone else has proof that for all our target distributions module autoloading works great even for self-compiled modules we can always remove it later. Until then I would prefer not to open another can of worms ;)

Since this is basically a "belt and suspenders" approach; so what about removing the comment and silencing the modprobe calls like so: do_modules() { insmod actipt > /dev/null 2>&1 insmod sch$QDISC > /dev/null 2>&1 insmod sch_ingress > /dev/null 2>&1 insmod act_mirred > /dev/null 2>&1 insmod cls_fw > /dev/null 2>&1 insmod cls_flow > /dev/null 2>&1 insmod cls_u32 > /dev/null 2>&1 insmod sch_htb > /dev/null 2>&1 insmod sch_hfsc > /dev/null 2>&1 }

A test shows that this is totally quiet now (sch_test, obviously does not exist): root@nacktmulle:~# modprobe sch_test kmod: failed to find a module named sch_test root@nacktmulle:~# modprobe sch_test > /dev/null 2>&1 root@nacktmulle:~# rmmod sch_hfsc root@nacktmulle:~# modprobe sch_hfsc > /dev/null 2>&1 root@nacktmulle:~# modprobe sch_hfsc > /dev/null 2>&1 root@nacktmulle:~# modprobe sch_hfsc > /dev/null 2>&1 root@nacktmulle:~# modprobe sch_hfsc9 > /dev/null 2>&1 root@nacktmulle:~# modprobe sch_fqcodel > /dev/null 2>&1 root@nacktmulle:~# modprobe sch_fq_codel > /dev/null 2>&1

This will also make the harmless, cosmetic "failed to find a module named sch_fq_codel" messages go away.

We then can and should still tackle the "is all required functionality actually available" at a higher level. Most users do not care whether something is built-in or available as a module (nor should they), all they care for from our vintage point is: does sqm work as advertised. It would be great if we could report missing prerequisites to the user so they can remedy that. But that is a different issue.

@toke please let me know whether you would take this as a slight improvement or whether you want to completely get rid of this, thanks in advance.

Best Regards Sebastian

moeller0 commented 9 years ago

Scrape this, while fine on the command line it does not work yet for the script... Redirecting the stderr makes it disappear for sqm_logger but nor syslog reports it. This might be good enough, at least there is no smoking gun (SQM tag) pointing to us as the cause for the message ;)

braveheartleo commented 9 years ago

Hi Sebastian,

This function could really use more smarts, I fully agree. The way I see it we have 3 separate issues to solve: 1) making sure modules are loaded before (first) use, as some distributions are not set up for module autoloading (to sure which distributions are affected, so this might be a wring assumption); 2) we actually should check whether a module is loaded already before loading it, because re-loading an already loaded module will produce an error message if I recall correctly; 3) which of the required functionality is requiring a module or is build-in the current kernel.

I was under the impression that this repo is for OpenWRT. But if that isn't the case then you're right in that the scope of this function is much broader than my what current understanding of it is.

Regarding 2), might I suggest a check like this [ -d /sys/module/$KMOD ] to determine if $KMOD is already loaded? Running modules export sysfs entries, and perhaps we can use that to check if a module is already loaded.

Now I'm not sure if the same check can be done for built-in modules (whether ALL built-in modules export sysfs entries). This check should prevent the blind assumption that just because a module isn't found at /lib/modules/$KVER/$KMOD means that is present and built-in. It might be the case the module is not built for (part of) the kernel, therefore missing and not bult-in.

As for 3), if I come up with an idea I'll be sure to share it here. ;)

I realize most of this is out of scope of your proposed improvement though ;). So regarding your proposal, I believe Toke had some code to detect modules that not only worked for openwrt, but also for other distributions, like Arch and OpenSUSE, I believe. Maybe we should use that for the module checks.

Then I defer to you guys on this matter.

Cheers, ianp

moeller0 commented 9 years ago

Hi Ianp,

On Aug 31, 2015, at 02:38 , braveheartleo notifications@github.com wrote:

Hi Sebastian,

This function could really use more smarts, I fully agree. The way I see it we have 3 separate issues to solve: 1) making sure modules are loaded before (first) use, as some distributions are not set up for module autoloading (to sure which distributions are affected, so this might be a wring assumption); 2) we actually should check whether a module is loaded already before loading it, because re-loading an already loaded module will produce an error message if I recall correctly; 3) which of the required functionality is requiring a module or is build-in the current kernel.

I was under the impression that this repo is for OpenWRT. But if that isn't the case then you're right in that the scope of this function is much broader than my what current understanding of it is.

Regarding 2), might I suggest a check like this [ -d /sys/module/$KMOD ] to determine if $KMOD is already loaded? Running modules export sysfs entries, and perhaps we can use that to check if a module is already loaded.

Now I'm not sure if the same check can be done for built-in modules (whether ALL built-in modules export sysfs entries). This check should prevent the assumption that just because a module isn't found at /lib/modules/$KVER/$KMOD, doesn't mean that is present and built-in.

The way it works, I believe, is that a given �piece of code� is either built into the monolithic kernel or built as a loadable module (or not built at all). Only if it is build as a loadable module and actually modprobe�d it will show an entry in /sys/module, so unfortunately this will not allow us to figure out in-builts.
Personally I believe the best thing to do is not fiddle around with heuristics about modules and such, but try to implement better error handling when we actually use functionality. With a bit of cleverness I am confident we can deduce a reasonable theory of why a certain step failed and give helpful information back to the user.
Note I already implemented the functionality checks for htb and hfsc into the scripts using them, as I think it is preferable not to disable sqm in total if only some of the script will not work. (Heck for the whole ingress dance we could resort to no ingress shaping if some of the prerequisites are missing, better to limp along than fail completely. We also should use CAPITALIZATION to confer this degraded loudly state back to the user, obviously, but I digress).

As for 3), if I come up with an idea I'll be sure to share it here. ;)

I realize most of this is out of scope of your proposed improvement though ;). So regarding your proposal, I believe Toke had some code to detect modules that not only worked for openwrt, but also for other distributions, like Arch and OpenSUSE, I believe. Maybe we should use that for the module checks.

Please have a look at the current do_modules, maybe you can find a way of silencing it completely, then do_modules really just is the �belt and suspenders� thing I believe it should be. We could also leave it as is though and just ignore the log-messages it creates...

Then I defer to you guys on this matter.

As much as I would like to make a decision, it is Toke�s project, hence his decision which color to paint this bike shed ;)

Best Regards Sebastian

Cheers, ianp

� Reply to this email directly or view it on GitHub.

moeller0 commented 9 years ago

Okay,

I just tested root@nacktmulle:~# /etc/init.d/sqm stop SQM: /usr/lib/sqm/stop-sqm: Stopping pppoe-ge00 SQM: ifb associated with interface pppoe-ge00: ifb4pppoe-ge00 SQM: /usr/lib/sqm/stop-sqm: ifb4pppoe-ge00 shaper deleted SQM: /usr/lib/sqm/stop-sqm: ifb4pppoe-ge00 interface deleted root@nacktmulle:~# rmmod sch_htb root@nacktmulle:~# /etc/init.d/sqm start SQM: /usr/lib/sqm/start-sqm: Starting pppoe-ge00 SQM: /usr/lib/sqm/start-sqm: Queue Setup Script: simple.qos SQM: QDISC htb is NOT useable.

Note the check for htb availability is performed before do_module is called, so this effectively shows, that cerowrt 3.10.50-1' s module auto-loading feature can not be relied upon.

It would be great if someone could repeat this test with a more modern version of openwrt and or different other distributions. I believe we could probably ignore cerowrt (even though I assume that this means openwrt AA also is afflicted)

Best Regards Sebastian

braveheartleo commented 9 years ago

Hi Sebastian,

It would be great if someone could repeat this test with a more modern version of openwrt and or different other distributions. I believe we could probably ignore cerowrt (even though I assume that this means openwrt AA also is afflicted)

Here's what happens on CHAOS CALMER (Bleeding Edge, r46734)

~# /etc/init.d/sqm stop
SQM: /usr/lib/sqm/stop-sqm: Stopping eth0.2
SQM: ifb associated with interface eth0.2:
SQM: /usr/lib/sqm/stop-sqm: ifb4eth0.2 shaper deleted
SQM: /usr/lib/sqm/stop-sqm: ifb4eth0.2 interface deleted
SQM: /usr/lib/sqm/stop-sqm: Stopping eth1
SQM: ifb associated with interface eth1:
SQM: /usr/lib/sqm/stop-sqm: ifb4eth1 shaper deleted
SQM: /usr/lib/sqm/stop-sqm: ifb4eth1 interface deleted
~# rmmod sch_htb
~# /etc/init.d/sqm start
SQM: /usr/lib/sqm/start-sqm: Starting eth1
SQM: /usr/lib/sqm/start-sqm: Queue Setup Script: simple.qos
SQM: QDISC fq_codel is useable.
SQM: Starting simple.qos
SQM: ifb associated with interface eth1: 
SQM: Squashing differentiated services code points (DSCP) from ingress.
SQM: STAB: stab mtu 2047 tsize 512 mpu 0 overhead 44 linklayer atm
SQM: get_limit:  CURLIMIT: 1001
SQM: cur_target: auto cur_bandwidth: 500
SQM: get_target defaulting to auto.
SQM: get_limit:  CURLIMIT: 1001
SQM: cur_target: auto cur_bandwidth: 500
SQM: get_target defaulting to auto.
SQM: get_limit:  CURLIMIT: 1001
SQM: cur_target: auto cur_bandwidth: 500
SQM: get_target defaulting to auto.
SQM: egress shaping activated
SQM: ingress shaping deactivated
SQM: /usr/lib/sqm/start-sqm: Starting eth0.2
SQM: /usr/lib/sqm/start-sqm: Queue Setup Script: simple.qos
SQM: QDISC fq_codel is useable.
SQM: Starting simple.qos
SQM: ifb associated with interface eth0.2: 
SQM: Squashing differentiated services code points (DSCP) from ingress.
SQM: STAB: stab mtu 2047 tsize 512 mpu 0 overhead 44 linklayer atm
SQM: get_limit:  CURLIMIT: 1001
SQM: cur_target: auto cur_bandwidth: 500
SQM: get_target defaulting to auto.
SQM: get_limit:  CURLIMIT: 1001
SQM: cur_target: auto cur_bandwidth: 500
SQM: get_target defaulting to auto.
SQM: get_limit:  CURLIMIT: 1001
SQM: cur_target: auto cur_bandwidth: 500
SQM: get_target defaulting to auto.
SQM: egress shaping activated
SQM: ingress shaping deactivated

Cheers, ianp

moeller0 commented 9 years ago

Hi Ianp,

sorry, I forgot to mention tat I needed to modify the script before the rmmod sch_htb test could work. To make up for this I will make a small test script, but probably only later today.

On Sep 1, 2015, at 00:06 , braveheartleo notifications@github.com wrote:

Hi Sebastian,

It would be great if someone could repeat this test with a more modern version of openwrt and or different other distributions. I believe we could probably ignore cerowrt (even though I assume that this means openwrt AA also is afflicted)

Here's what happens on CHAOS CALMER (Bleeding Edge, r46734)

~# /etc/init.d/sqm stop SQM: /usr/lib/sqm/stop-sqm: Stopping eth0.2 SQM: ifb associated with interface eth0.2: ifb4eth0.2 SQM: /usr/lib/sqm/stop-sqm: ifb4eth0.2 shaper deleted SQM: /usr/lib/sqm/stop-sqm: ifb4eth0.2 interface deleted SQM: /usr/lib/sqm/stop-sqm: Stopping eth1 SQM: ifb associated with interface eth1: ifb4eth1 SQM: /usr/lib/sqm/stop-sqm: ifb4eth1 shaper deleted SQM: /usr/lib/sqm/stop-sqm: ifb4eth1 interface deleted ~# rmmod sch_htb ~# /etc/init.d/sqm start SQM: /usr/lib/sqm/start-sqm: Starting eth1 SQM: /usr/lib/sqm/start-sqm: Queue Setup Script: simple.qos SQM: QDISC fq_codel is useable.

Here the following is missing in the simplpe.qos script before do_modules:

verify_qdisc "htb" || return 1

In reality do_modules needs to be called before all verify_qdisc invocations to be effective, but this test aims at checking whether do_modules actually is required. The script you tested only verifies fq_codel, but fq_codel is built-in for openwrt, so can not be rrmod�ed.

Many thanks again and best regards

Sebastian

SQM: Starting simple.qos SQM: ifb associated with interface eth1: SQM: Squashing differentiated services code points (DSCP) from ingress. SQM: STAB: stab mtu 2047 tsize 512 mpu 0 overhead 44 linklayer atm SQM: get_limit: CURLIMIT: 1001 SQM: cur_target: auto cur_bandwidth: 500 SQM: get_target defaulting to auto.

SQM: get_limit: CURLIMIT: 1001 SQM: cur_target: auto cur_bandwidth: 500 SQM: get_target defaulting to auto. SQM: get_limit: CURLIMIT: 1001 SQM: cur_target: auto cur_bandwidth: 500 SQM: get_target defaulting to auto. SQM: egress shaping activated SQM: ingress shaping deactivated SQM: /usr/lib/sqm/start-sqm: Starting eth0.2 SQM: /usr/lib/sqm/start-sqm: Queue Setup Script: simple.qos SQM: QDISC fq_codel is useable. SQM: Starting simple.qos SQM: ifb associated with interface eth0.2: SQM: Squashing differentiated services code points (DSCP) from ingress. SQM: STAB: stab mtu 2047 tsize 512 mpu 0 overhead 44 linklayer atm SQM: get_limit: CURLIMIT: 1001 SQM: cur_target: auto cur_bandwidth: 500 SQM: get_target defaulting to auto. SQM: get_limit: CURLIMIT: 1001 SQM: cur_target: auto cur_bandwidth: 500 SQM: get_target defaulting to auto. SQM: get_limit: CURLIMIT: 1001 SQM: cur_target: auto cur_bandwidth: 500 SQM: get_target defaulting to auto. SQM: egress shaping activated SQM: ingress shaping deactivated

Cheers, ianp

� Reply to this email directly or view it on GitHub.

moeller0 commented 9 years ago

Hi Ianp,

I believe I have a simple test for module auto loading that can be run from ssh into the router:

1) Stop sqm if running: "/etc/init.d/sqm stop" root@cerowrt3.10.50-1:/# /etc/init.d/sqm stop 2) source sqm's functions: ". /usr/lib/sqm/functions.sh" root@cerowrt3.10.50-1:/usr/lib/sqm# . /usr/lib/sqm/functions.sh 3) source sqm's default values: ". /usr/lib/sqm/defaults.sh" root@cerowrt3.10.50-1:/usr/lib/sqm# . /usr/lib/sqm/defaults.sh 4) remove a modular qdisc: "rmmod sch_htb" root@cerowrt3.10.50-1:/# rmmod sch_htb 5) check whether htb is useable: "verify_qdisc htb" Ideally this should trigger auto loading as verify_qdisc tries to use sch_htb root@cerowrt3.10.50-1:/# verify_qdisc htb ash: bad number QDISC htb is NOT useable. 6) load all required modules including htb: "do_modules" Note that the silencing with >/dev/null 2>&1 works on the shell. root@cerowrt3.10.50-1:/# do_modules 7) now test ht.'s usability again: "verify_qdisc htb" root@cerowrt3.10.50-1:/# verify_qdisc htb ash: bad number QDISC htb is useable.

Please ignore the ash: bad number issue as it does not seem to affect the actual functionality tested.

If CC behaves like cerowrt 3.10.50-1 we really should improve do_modules. And your idea will work well to avoid warnings for potential built-ins (it will also hide really missing modules a bit deeper), to avoid the "kmod: sch_htb is already loaded" messages we could also implement your proposal for detecting already loaded modules.

Best Regards Sebastian

moeller0 commented 9 years ago

OpenWRT CC-RC3: root@OpenWrt:~# cat /etc/banner


| |.-----.-----.-----.| | | |.----.| | | - || | -| || | | || || | |_____|| |___||||____||| |__| |__| W I R E L E S S F R E E D O M


CHAOS CALMER (15.05-rc3, r46163)


moeller0 commented 9 years ago

OpenWRT CC Bleding Edge: root@OpenWrt:~# cat /etc/banner


| |.-----.-----.-----.| | | |.----.| | | - || | -| || | | || || | |_____|| |___||||____||| |__| |__| W I R E L E S S F R E E D O M


CHAOS CALMER (Bleeding Edge, r46760)


tohojo commented 9 years ago

Mangled as well, but I think this means module autoloading doesn't work on either?

moeller0 commented 9 years ago

@tohojo @braveheartleo we still need do_modules for openwrt's at least. Since silencing it completely did not work; I believe the next simplest fix would be to check whether modules are loaded already and skip loading those again. The error message "module already loaded" is not really adding anything interesting at all. Missing modules are still worth reporting in my book, as we have no guaranteed way of generically checking whether a module was built-in on openwrt, we really should report those, the user than needs to consider whether they are missing or or whether that is a false alarm...

Best Regards Sebastian

tohojo commented 9 years ago

wait, why did the silencing thing not work?

moeller0 commented 9 years ago

Hi Toke,

On Sep 2, 2015, at 23:02 , Toke H�iland-J�rgensen notifications@github.com wrote:

Mangled as well, but I think this means module autoloading doesn't work on either?

Sorry for the mangled pastes, I should in vestige and fix that on my side�

You are right at least with my test it looks like modules are not (reliably) auto-loaded under openwrt/cerowrt. I also believe that messages about already loaded modules are fluff and should go, but missing modules should be reported sincere we have no sure-fire way of figuring out whether the functionality is available in the kernel. Other distributions use a file modules.builtin to list built in stuff that also could be modularized but openwrt does have this under /lib/modules/3.18.20 (example from the bleeding edge version). Nor does openwrt have /proc/config.gz were the same information could be scraped off� So I believe it is better to accept a few false negatives compared to not warn at all...

Best Regards Sebastian

� Reply to this email directly or view it on GitHub.

tohojo commented 9 years ago

As far as I'm concerned just firing off the module loading and forgetting about it is fine (i.e. just suppress any errors). Then do the functionality checking and fail if that doesn't work. If you want to get really fancy, save the fact that module loading failed and output that as part of the error message when failing the functionality check.

A quick test indicates that something like this may work:

silence()
{
    if [ "$DEBUG" -eq "1" ]; then
        "$@"
    else
        "$@" >/dev/null 2>&1
    fi
}

Which could then be prepended to all the commands we don't want to see the output of, like

 silence modprobe sch_fq_codel

(assuming we define DEBUG somewhere, of course). Then we have the option of seeing everything when running in debug mode, while being nice an silent in normal operation.

moeller0 commented 9 years ago

Hi Toke,

On Sep 2, 2015, at 23:19 , Toke H�iland-J�rgensen notifications@github.com wrote:

As far as I'm concerned just firing off the module loading and forgetting about it is fine (i.e. just suppress any errors). Then do the functionality checking and fail if that doesn't work. If you want to get really fancy, save the fact that module loading failed and output that as part of the error message when failing the functionality check.

A quick test indicates that something like this may work:

silence() { if [ "$DEBUG" -eq "1" ]; then "$@" else "$@" >/dev/null 2>&1 fi }

I believe I tried adding � >/dev/null 2>&1� to the mod probe invocations, and the messages do not show up if /etc/init.d/sqm start is called on the command line, but they still show up in the log, from running /etc/init.d/sqm stop ; /etc/init.d/sqm start:

Wed Sep 2 21:22:17 2015 daemon.err modprobe: act_ipt is already loaded Wed Sep 2 21:22:17 2015 daemon.err modprobe: failed to find a module named sch_fq_codel Wed Sep 2 21:22:17 2015 daemon.err modprobe: sch_ingress is already loaded Wed Sep 2 21:22:17 2015 daemon.err modprobe: act_mirred is already loaded Wed Sep 2 21:22:17 2015 daemon.err modprobe: cls_fw is already loaded Wed Sep 2 21:22:17 2015 daemon.err modprobe: cls_flow is already loaded Wed Sep 2 21:22:17 2015 daemon.err modprobe: cls_u32 is already loaded Wed Sep 2 21:22:17 2015 daemon.err modprobe: sch_htb is already loaded Wed Sep 2 21:22:17 2015 daemon.err modprobe: sch_hfsc is already loaded

If there is a simple way of shutting up deamon.err I agree that would be the easiest and simplest,but I have not found one. do_modules for this test looked like: do_modules() {

sm: module autoloading should have taken care of this, but better safe than sorry.

#sm: unfortunatelly syslog still reports $INSMOD's output to the log
${INSMOD} act_ipt > /dev/null 2>&1
${INSMOD} sch_$QDISC > /dev/null 2>&1
${INSMOD} sch_ingress > /dev/null 2>&1
${INSMOD} act_mirred > /dev/null 2>&1
${INSMOD} cls_fw > /dev/null 2>&1
${INSMOD} cls_flow > /dev/null 2>&1
${INSMOD} cls_u32 > /dev/null 2>&1
${INSMOD} sch_htb > /dev/null 2>&1
${INSMOD} sch_hfsc > /dev/null 2>&1

}

Any ideas?

Which could then be prepended to all the commands we don't want to see the output of, like

silence modprobe sch_fq_codel

(assuming we define DEBUG somewhere, of course). Then we have the option of seeing everything when running in debug mode, while being nice an silent in normal operation.

Oh, I like that a lot, we have a few places where we �> /dev/null 2>&1� that could be interesting during debugging�

Best Regards Sebastian

� Reply to this email directly or view it on GitHub.

tohojo commented 9 years ago

Ah, missed the 'modprobe outputs directly to syslog' thing... damn.

tohojo commented 9 years ago

Playing around with it a bit more, it seems to be possible to silence modprobe by piping only stderr to /dev/null but not stdout.

So running

modprobe sch_fq_codel 2>/dev/null

gives no output and the log stays clean.

...on trunk, but not on the old cerowrt. Damn!

moeller0 commented 9 years ago

HI Toke,

On Sep 2, 2015, at 23:42 , Toke H�iland-J�rgensen notifications@github.com wrote:

Playing around with it a bit more, it seems to be possible to silence modprobe by piping only stderr to /dev/null but not stdout.

So running

modprobe sch_fq_codel 2>/dev/null

gives no output and the log stays clean.

...on trunk, but not on the old cerowrt. Damn!

I believe if BB, CC and beyond can be silenced, cerowrt can stay as loud as it wants (as long as it still loads those modules obviously). No one in the cerowrt crowd ever complained about sqm�s verbosity ;)

Best Regards Sebastian

� Reply to this email directly or view it on GitHub.

tohojo commented 9 years ago

Well, it seems modprobe directly opens a socket to /dev/log on cerowrt, so I don't think there's anything to do about that, short of moving /dev/log out of the way before calling modprobe; and I hardly think we want to do that... :)

No idea about BB and CC; don't have any of those around for ready testing...

moeller0 commented 9 years ago

Hi Toke,

On Sep 2, 2015, at 23:51 , Toke H�iland-J�rgensen notifications@github.com wrote:

Well, it seems modprobe directly opens a socket to /dev/log on cerowrt, so I don't think there's anything to do about that, short of moving /dev/log out of the way before calling modprobe; and I hardly think we want to do that... :)

Nah, I just tested your proposed change it works under CC-RC3 and bleeding edge, it fails under cerowrt, but failure here just means same verbosity as before. I do not have access to a BB VM and I really don�t want to set one up ;). Let�s just push your change and have less log to crawl through. (We do loose the information about genuinely missing modules, but no one acted upon that anyway, so no regression)

Best Regards Sebastian

No idea about BB and CC; don't have any of those around for ready testing...

� Reply to this email directly or view it on GitHub.

tohojo commented 9 years ago

moeller0 notifications@github.com writes:

Nah, I just tested your proposed change it works under CC-RC3 and bleeding edge, it fails under cerowrt, but failure here just means same verbosity as before. I do not have access to a BB VM and I really don�t want to set one up ;). Let�s just push your change and have less log to crawl through. (We do loose the information about genuinely missing modules, but no one acted upon that anyway, so no regression)

Cool. Piping just stderr to /dev/null seems to make modprobe shut up on my Arch system as well, so we're probably fine with that :)

-Toke

tohojo commented 9 years ago

Couldn't resist the temptation to move the module list to defaults.sh while I was at it; this should allow the .qos scripts to modify the list if they want. Didn't specifically test that part, but hoping I didn't goof... :P

moeller0 commented 9 years ago

Hi Toke,

On Sep 3, 2015, at 00:04 , Toke H�iland-J�rgensen notifications@github.com wrote:

Couldn't resist the temptation to move the module list to defaults.sh while I was at it;

Yeah, saw you change and liked it ;0

this should allow the .qos scripts to modify the list if they want. Didn't specifically test that part, but hoping I didn't goof... :P

Cerowrt 3.10.50-1 still reports the errors, but neither CC-RC3 nor postCC report errors, so that should be fine.

I just realized that Dave had his bases covered already see:

insmod() { lsmod | grep -q ^$1 || $INSMOD $1 }

in functions.sh. I had overlooked this when sitting from what I believed to be an open coded call to insmod� I do believe the new code is nicer as adding new modules is more natural. And I do not believe that gripping lsmod output is the best way of getting at which modules are loaded.

Best Regards Sebastian

� Reply to this email directly or view it on GitHub.

moeller0 commented 9 years ago

Hi Toke,

On Sep 3, 2015, at 00:16 , Sebastian Moeller moeller0@gmx.de wrote:

I just realized that Dave had his bases covered already see:

insmod() { lsmod | grep -q ^$1 || $INSMOD $1 }

This function can be removed, do you agree?

Best Regards Sebastian

tohojo commented 9 years ago

You, agree. Nuked it