Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] cake plugins #941

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
052a824
cake: add two simple plugins
Sep 10, 2018
467720a
rename cake.cake_tin_ cake.cake_tin_throughput_
Sep 11, 2018
f202a4d
mv cake.cake_tin_throughput_ to cake.cake_tin_pps_; optimizing
Sep 11, 2018
507d7f1
cake: add cake_tin_delay_ plugin
Sep 11, 2018
dd3f63f
cake: add cake_tin_ecn_ plugin
Sep 11, 2018
12c4695
cake: fix description of cake_tin_pps_'s graph
Sep 11, 2018
af12747
switch vom ms to seconds, to avoid values like 10m ms
Sep 11, 2018
d7f9176
change scientific numbers to ordinary floats; add ns range
Sep 11, 2018
5d31b60
cake_tin_delay_ optimize headline
Sep 11, 2018
8e2aac4
cake_tin_ecn_ optimize headline
Sep 11, 2018
a7bd7e3
cake_ optimize headline
Sep 11, 2018
e336264
cake: add plugin cake_tin_throughput_
Sep 11, 2018
afb969f
cake: update all plugins to add automatic ingress-device detection
Sep 12, 2018
b0c1e96
cake: fix errounous graphs in cake_tin_throughput_
Sep 12, 2018
491bfc1
cake: add a hint, that backlog in cake.cake_ is not pps but absolute
Sep 12, 2018
5e74931
cake: error message fix
Sep 12, 2018
86df21b
remove unnecessary end line delimiter
Sep 13, 2018
d95aa75
cake: chmod 775
Sep 13, 2018
455effd
cake: add static colors
Sep 16, 2018
749d302
cake: shellcheck
Sep 16, 2018
34d81f7
cake: fix bug on suggest function returning just one devices
Sep 16, 2018
234db28
cake: whitespace fixing
Sep 16, 2018
fc8ca5b
cake: add error-message to non-supported cake-configs (best effort) f…
Sep 16, 2018
7540eb3
cake: change cake_tin_pps_ to logarithmic scale
Sep 16, 2018
00888cd
cake: cake.cake_ swap behavoir
Sep 17, 2018
6760871
cake: add double quotation marks on integers arithmetics by request
Sep 18, 2018
8cc759a
fix shellcheck commit
Sep 19, 2018
0ea29f8
cake: remove logarithmic scale, since it cannot be used with negative…
Sep 28, 2018
b75fa5c
cake: suggest was just returning one dev, fixing
Sep 30, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
202 changes: 202 additions & 0 deletions plugins/cake/cake_
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
#!/bin/bash
# -*- sh -*-

: << =cut

=pod

=encoding UTF-8

=head1 NAME

cake_ - Plugin to monitor cake's backlog, dropped, overlimits and requeues

=head1 CONFIGURATION

None needed.

=head1 INTERPRETATION

Cake, also known as sch_cake is a modern bandwidth limiter, which eliminates
buffer bloat over slow links. It's also capable to give flows, hosts and each
flow of each host a fair part of the avaible bandwidth.

This plugin allows for a monitor of the pressure on the qdisc, by monitoring key
values.

=head1 SEE ALSO

Take a look at "man cake" to get more information about cake.

=head1 MAGIC MARKERS

#%# family=auto
#%# capabilities=autoconf suggest

=head1 AUTHORS

RubenKelevra <[email protected]>

work based on the tc plugin, authors:
Steve Schnepp <[email protected]>,
Samuel Smith <[email protected]>,
Nye Liu <[email protected]>

=head1 LICENSE

GPLv2 or later

=cut

DEVICE=${0##*/cake_}

tc_get_ifb_dev() {
dev="$(/sbin/tc filter show dev $DEVICE parent ffff: protocol all | grep "mirred" | grep "Egress Redirect to device")"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use local for variables that are only used in functions.
Afterwards you will need to split the assignment from the local line in order to not hide the exit code of the command substitution.

Please quote variables (e.g. $DEVICE). shellcheck is your friend :)

grep "mirrored": -w would be more specific here. Or maybe join both grep calls into one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads up, haven't thought about that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To the regex idea: I'm not firm enought to use it here - I don't use stuff I don't fully understand.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess grep "Egress Redirect to device.*mirrored" would work here.
(I cannot tell, since my machine does not seem to use cake by default)
Anyway: the current state is also fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sumpfralle I doubt that this would work, since the line reads like:

action order 1: mirred (Egress Redirect to device ifb1) stolen

mirrored isn't contained ;)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure, if you meant this serious, but I will just follow your invitation for spoon feeding :)

grep "mirred.*Egress Redirect to device"

Additionally this would be a great place for adding a comment with this expected example output line. This will ease the life of future readers of the code.

And I have to admit, that this line of output looks quite human-centric and thus will really change somewhen. json would be your friend :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

;)

And good idea :)

[ $? -eq 1 ] && return 1

echo "$dev" | sed -n 's/.* device \([^ ]*\).*/\1/p' | tr ')' ' ' | awk '{ print $1 }'
}

tc_cake_sent() {
/sbin/tc -s qdisc show dev "$1" | grep -E "^ Sent" | tr ',' ' ' | tr ')' ' '
}
tc_cake_backlog() {
/sbin/tc -s qdisc show dev "$1" | grep -E "^ backlog" | tr 'p' ' '
}

DEVICE_IN="$(tc_get_ifb_dev)"
has_if_in=$?

case "$1" in
autoconf)
if [ -r /proc/net/dev ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there really no specific test for the actual use of cake on the system?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in all cases. You could check if the sch_cake module is loaded, but if it's compiled as a built-in module that won't catch it. Also, the module can be loaded without being active.

The only sure way to see if cake is active is to do a tc qdisc and loop through all interfaces to try to find it. You could probably do tc -j qdisc and construct a query with jq that will spit out all cake instances...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this one?

tc -j qdisc | jq -r '.[] | select(.kind == "cake").dev' | sort | uniq

Copy link
Contributor Author

@RubenKelevra RubenKelevra Sep 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thanks!

But do we really want to get rid of the filter with packets received?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... I'll leave it the way it it. If you want to replace this function, feel free to suggest one which covers all features.

Also make sure to add a simple test, if jq is installed with an error-output for autoconf - for example it isn't installed on my system - never had the need for it - so an output about this dependency would be nice.

Copy link
Collaborator

@sumpfralle sumpfralle Sep 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But do we really want to get rid of the filter with packets received?

I am not sure, what this means. How would you detect this situation? I would be happy to suggest a suitable filter.

Regarding the check of for jq - this is simply part of "autoconf", too:

if ! which jq >/dev/null; then
    echo "no (missing 'jq' executable)"
elif [ ! -r /proc/net/dev ]; then
...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the if (($2 > 0) || ($10 > 0))-part of line 26. This was part of the tc_ plugin. :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - I see.
Maybe we have different perceptions regarding the purpose of the "autoconf" test.
Here the plugin needs to answer two questions:

  • can this plugin run at all? (i.e. are all dependencies satisfied?)
  • is this plugin useful for this host?

The "autoconf" call should return "yes" only if both questions are affirmed.

Your current implementation of "autoconf" only cares (partly) for the first question.
Additionally it should check two more details:

  • is tc available? (see my example for jq above)
  • is "cake" in use on this system?

The latter can probably be tested like this:

[ -n "$(tc -j qdisc | jq -r '.[] | select(.kind == "cake").dev')" ]

echo yes
exit 0
else
echo "no (/proc/net/dev not found)"
exit 1
fi
;;
suggest)
if [ -r /proc/net/dev ]; then
ifs="$(awk '
/^ *(eth|tap|bond|wlan|ath|ra|sw|eno|ens|enp|wlp|wl)[0-9]*/ {
split($0, a, /: */);
gsub(/^ +/,"",a[1]);
if (($2 > 0) || ($10 > 0)) print a[1]; }' /proc/net/dev)"
cake_ifs=()
for if in $ifs; do
qdisc="$(/sbin/tc -s qdisc show dev "$if" | head -n1 | awk '{ print $2 }')"
if [ "$qdisc" == "cake" ]; then
cake_ifs+=("$if")
fi
done
echo "$cake_ifs"
fi
exit 0
;;
config)

if [ $has_if_in -eq 0 ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please quote variables. We are striving for shellcheck cleanliness in the plugins.
See the hints for new plugins.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, shellcheck isn't complaining about this line, what exactly is wrong with it?

integer comparisons AFAIK cannot be double quoted and since it's not an input variable I don't see any point changing this to a string, just to make any sanitizer happy.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, shellcheck isn't complaining about this line, what exactly is wrong with it?

Correct: in this case it does not complain, since you are assigning only fixed strings to this variable. Overly simple static parsers (e.g.: humans like me) will repeatedly stumble over this, nevertheless :)

integer comparisons AFAIK cannot be double quoted

Please rest assured, that the shell parser will never forbid you to double-quote a variable :)
(double quotes only prevent word splitting for various expansions - this is usually what you want)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well no, I assigned integers to the variables, that's why it's probably not complaining. You're sure that openwrt's test is working with integer arithmetic AND double quoted variables? I think I stopped doing this for a reason. :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hehe - yes, that works. I am writing way too much shell code for busybox's ash. All the usual features of dash are supported there.

echo "graph_title $DEVICE ($DEVICE_IN) cake queuing"
else
echo "graph_title $DEVICE cake queuing"
fi
echo 'graph_args --base 1000'
if [ $has_if_in -eq 0 ]; then
echo 'graph_vlabel pps in (-) / out (+)'
else
echo 'graph_vlabel packets per second'
fi
echo 'graph_category network'
if [ $has_if_in -eq 0 ]; then
echo "graph_info This graph shows the general packet handling status of egress+ingress traffic of the $DEVICE ($DEVICE_IN) network interface."
else
echo "graph_info This graph shows the general packet handling status of egress traffic of the $DEVICE network interface."
fi

if [ $has_if_in -eq 0 ]; then
echo "backlog_in.label backlog (absolute)"
echo "backlog_in.draw LINE2"
echo "backlog_in.min 0"
echo "backlog_in.info amount of packets currently buffered"
echo "backlog_in.graph no"
fi
echo "backlog.label backlog (absolute)"
echo "backlog.draw LINE2"
echo "backlog.min 0"
echo "backlog.info amount of packets currently buffered"
[ $has_if_in -eq 0 ] && echo "backlog.negative backlog_in"

if [ $has_if_in -eq 0 ]; then
echo "dropped_in.label dropped"
echo "dropped_in.draw AREA"
echo "dropped_in.type DERIVE"
echo "dropped_in.min 0"
echo "dropped_in.info dropped packets in queue"
echo "dropped_in.graph no"
fi
echo "dropped.label dropped"
echo "dropped.draw AREA"
echo "dropped.type DERIVE"
echo "dropped.min 0"
echo "dropped.info dropped packets in queue"
[ $has_if_in -eq 0 ] && echo "dropped.negative dropped_in"

if [ $has_if_in -eq 0 ]; then
echo "overlimits_in.label overlimits"
echo "overlimits_in.draw STACK"
echo "overlimits_in.type DERIVE"
echo "overlimits_in.min 0"
echo "overlimits_in.info packets exeeded a limit"
echo "overlimits_in.graph no"
fi
echo "overlimits.label overlimits"
echo "overlimits.draw STACK"
echo "overlimits.type DERIVE"
echo "overlimits.min 0"
echo "overlimits.info packets exeeded a limit"
[ $has_if_in -eq 0 ] && echo "overlimits.negative overlimits_in"

if [ $has_if_in -eq 0 ]; then
echo "requeues_in.label requeues"
echo "requeues_in.draw STACK"
echo "requeues_in.type DERIVE"
echo "requeues_in.min 0"
echo "requeues_in.info packets requeued in queue"
echo "requeues_in.graph no"
fi
echo "requeues.label requeues"
echo "requeues.draw STACK"
echo "requeues.type DERIVE"
echo "requeues.min 0"
echo "requeues.info packets requeued in queue"
[ $has_if_in -eq 0 ] && echo "requeues.negative requeues_in"

exit 0
;;
esac

tc_cake_sent "$DEVICE" | awk '{
print "dropped.value " $7
print "overlimits.value " $9
print "requeues.value " $11
}'

tc_cake_backlog "$DEVICE" | awk '{
print "backlog.value " $3
}'

if [ $has_if_in -eq 1 ]; then
RubenKelevra marked this conversation as resolved.
Show resolved Hide resolved
exit 0
fi

tc_cake_sent "$DEVICE_IN" | awk '{
print "dropped_in.value " $7
print "overlimits_in.value " $9
print "requeues_in.value " $11
}'

tc_cake_backlog "$DEVICE_IN" | awk '{
print "backlog_in.value " $3
}'
Loading