| Message ID | 1453434307-16604-1-git-send-email-michael@michaelmarley.com |
|---|---|
| State | Accepted |
| Headers | show |
On 2016-01-22 04:45, Michael Marley wrote: > I apologize, my client mangled my previous attempt at resubmission. > > Here is an updated version with three improvements. The problem with > the rules not being removed (which was not new and actually caused by a > grep command incompatible with musl) was fixed by using an updated grep > command (thanks nbd!). The problems pertaining to the xtables lock > (including "too many links" and "directory not empty") were fixed by > always executing ip[6]tables with the "-w" command-line argument to make > it wait for the xtables lock. Lastly, I fixed a place where I hardcoded > "iptables" and "ip6tables" instead of looping over the array like > everywhere else. > --trim here-- > > This adds IPv6 support to qos-scripts for both tc/qdisc and the > iptables classification rules. The tc/qdisc part is accomplished > by removing "protocol ip" from the tc command line, causing the > rule to be applied to all protocols. The iptables part is > accomplished by adding each rule using both iptables and ip6tables. > > This patch is based on previous work by Ilkka Ollakka and > Dominique Martinet. > > Signed-off-by: Michael Marley <michael@michaelmarley.com> > --- Applied. Next time, please write any comments under the "---" line. That way I don't have to manually edit the commit message. Thanks, - Felix
~Off topic~ So I'm going to guess this means both of you have qos-scripts running on current trunk builds? Have you seen anyone complaining about kernel panic in hfsc? Should I make a trac ticket? ar71xx/TLWDR4300 WARNING: CPU: 0 PID: 0 at net/sched/sch_hfsc.c:1429 0x831f5b2c()
On 2016-01-22 13:12, Weedy wrote: > ~Off topic~ > > So I'm going to guess this means both of you have qos-scripts running > on current trunk builds? > > Have you seen anyone complaining about kernel panic in hfsc? Should I > make a trac ticket? > > ar71xx/TLWDR4300 > WARNING: CPU: 0 PID: 0 at net/sched/sch_hfsc.c:1429 0x831f5b2c() Can you please test if my recent removal of the txqueuelen override makes a difference here? - Felix
On 22 Jan 2016 07:15, "Felix Fietkau" <nbd@openwrt.org> wrote: > > On 2016-01-22 13:12, Weedy wrote: > > ~Off topic~ > > > > So I'm going to guess this means both of you have qos-scripts running > > on current trunk builds? > > > > Have you seen anyone complaining about kernel panic in hfsc? Should I > > make a trac ticket? > > > > ar71xx/TLWDR4300 > > WARNING: CPU: 0 PID: 0 at net/sched/sch_hfsc.c:1429 0x831f5b2c() > Can you please test if my recent removal of the txqueuelen override > makes a difference here? > > - Felix It's running 48424 and I just double checked that I still have the override in place. Is the override likely to cause or mask the problem?
On 2016-01-22 14:10, Weedy wrote: > On 22 Jan 2016 07:15, "Felix Fietkau" <nbd@openwrt.org > <mailto:nbd@openwrt.org>> wrote: >> >> On 2016-01-22 13:12, Weedy wrote: >> > ~Off topic~ >> > >> > So I'm going to guess this means both of you have qos-scripts running >> > on current trunk builds? >> > >> > Have you seen anyone complaining about kernel panic in hfsc? Should I >> > make a trac ticket? >> > >> > ar71xx/TLWDR4300 >> > WARNING: CPU: 0 PID: 0 at net/sched/sch_hfsc.c:1429 0x831f5b2c() >> Can you please test if my recent removal of the txqueuelen override >> makes a difference here? >> >> - Felix > > It's running 48424 and I just double checked that I still have the > override in place. > > Is the override likely to cause or mask the problem? Maybe the override caused the problem or made it more likely, I don't know. It's just a wild guess... - Felix
The problem still happens for me even without the txqueuelen change. Michael On 01/22/16 07:15, Felix Fietkau wrote: > On 2016-01-22 13:12, Weedy wrote: >> ~Off topic~ >> >> So I'm going to guess this means both of you have qos-scripts running >> on current trunk builds? >> >> Have you seen anyone complaining about kernel panic in hfsc? Should I >> make a trac ticket? >> >> ar71xx/TLWDR4300 >> WARNING: CPU: 0 PID: 0 at net/sched/sch_hfsc.c:1429 0x831f5b2c() > Can you please test if my recent removal of the txqueuelen override > makes a difference here? > > - Felix
On Fri, Jan 22, 2016 at 9:08 AM, Michael Marley <michael@michaelmarley.com> wrote: > The problem still happens for me even without the txqueuelen change. > > Michael > > On 01/22/16 07:15, Felix Fietkau wrote: >> On 2016-01-22 13:12, Weedy wrote: >>> ~Off topic~ >>> >>> So I'm going to guess this means both of you have qos-scripts running >>> on current trunk builds? >>> >>> Have you seen anyone complaining about kernel panic in hfsc? Should I >>> make a trac ticket? >>> >>> ar71xx/TLWDR4300 >>> WARNING: CPU: 0 PID: 0 at net/sched/sch_hfsc.c:1429 0x831f5b2c() >> Can you please test if my recent removal of the txqueuelen override >> makes a difference here? >> >> - Felix I've since done a few distcleans and can't reproduce the problem. I guess I should just make a habit to distclean if I'm updating more then 2-3 weeks worth of changes. Now my new problem. I have a pppoe connection, so my WAN interface gets swapped. I get double shaped at boot, things are fine after a qos restart. root@OpenWrt:~# tc qdisc qdisc fq_codel 0: dev eth0 root refcnt 2 limit 1024p flows 1024 quantum 1514 target 5.0ms interval 100.0ms ecn qdisc hfsc 1: dev ifb0 root refcnt 2 default 30 qdisc fq_codel 100: dev ifb0 parent 1:10 limit 800p flows 1024 quantum 300 target 5.0ms interval 100.0ms qdisc fq_codel 200: dev ifb0 parent 1:20 limit 800p flows 1024 quantum 300 target 5.0ms interval 100.0ms qdisc fq_codel 300: dev ifb0 parent 1:30 limit 800p flows 1024 quantum 300 target 5.0ms interval 100.0ms qdisc fq_codel 400: dev ifb0 parent 1:40 limit 800p flows 1024 quantum 300 target 5.0ms interval 100.0ms ## this doesn't get cleaned up ## qdisc hfsc 1: dev eth0.2 root refcnt 2 default 30 qdisc fq_codel 100: dev eth0.2 parent 1:10 limit 800p flows 1024 quantum 300 target 5.0ms interval 100.0ms qdisc fq_codel 200: dev eth0.2 parent 1:20 limit 800p flows 1024 quantum 300 target 5.0ms interval 100.0ms qdisc fq_codel 300: dev eth0.2 parent 1:30 limit 800p flows 1024 quantum 300 target 5.0ms interval 100.0ms qdisc fq_codel 400: dev eth0.2 parent 1:40 limit 800p flows 1024 quantum 300 target 5.0ms interval 100.0ms qdisc ingress ffff: dev eth0.2 parent ffff:fff1 ---------------- qdisc mq 0: dev wlan0 root qdisc fq_codel 0: dev wlan0 parent :1 limit 1024p flows 1024 quantum 1514 target 5.0ms interval 100.0ms ecn qdisc fq_codel 0: dev wlan0 parent :2 limit 1024p flows 1024 quantum 1514 target 5.0ms interval 100.0ms ecn qdisc fq_codel 0: dev wlan0 parent :3 limit 1024p flows 1024 quantum 1514 target 5.0ms interval 100.0ms ecn qdisc fq_codel 0: dev wlan0 parent :4 limit 1024p flows 1024 quantum 1514 target 5.0ms interval 100.0ms ecn qdisc mq 0: dev wlan1 root qdisc fq_codel 0: dev wlan1 parent :1 limit 1024p flows 1024 quantum 1514 target 5.0ms interval 100.0ms ecn qdisc fq_codel 0: dev wlan1 parent :2 limit 1024p flows 1024 quantum 1514 target 5.0ms interval 100.0ms ecn qdisc fq_codel 0: dev wlan1 parent :3 limit 1024p flows 1024 quantum 1514 target 5.0ms interval 100.0ms ecn qdisc fq_codel 0: dev wlan1 parent :4 limit 1024p flows 1024 quantum 1514 target 5.0ms interval 100.0ms ecn ## this is always fine ## qdisc hfsc 1: dev pppoe-wan root refcnt 2 default 30 qdisc fq_codel 100: dev pppoe-wan parent 1:10 limit 800p flows 1024 quantum 300 target 5.0ms interval 100.0ms qdisc fq_codel 200: dev pppoe-wan parent 1:20 limit 800p flows 1024 quantum 300 target 5.0ms interval 100.0ms qdisc fq_codel 300: dev pppoe-wan parent 1:30 limit 800p flows 1024 quantum 300 target 5.0ms interval 100.0ms qdisc fq_codel 400: dev pppoe-wan parent 1:40 limit 800p flows 1024 quantum 300 target 5.0ms interval 100.0ms qdisc ingress ffff: dev pppoe-wan parent ffff:fff1 ----------------
diff --git a/package/network/config/qos-scripts/files/usr/lib/qos/generate.sh b/package/network/config/qos-scripts/files/usr/lib/qos/generate.sh index caa1125..4a39411 100755 --- a/package/network/config/qos-scripts/files/usr/lib/qos/generate.sh +++ b/package/network/config/qos-scripts/files/usr/lib/qos/generate.sh @@ -336,11 +336,11 @@ tc class add dev $dev parent 1: classid 1:1 hfsc sc rate ${rate}kbit ul rate ${r if [ -n "$halfduplex" ]; then export dev_up="tc qdisc del dev $device root >&- 2>&- tc qdisc add dev $device root handle 1: hfsc -tc filter add dev $device parent 1: protocol ip prio 10 u32 match u32 0 0 flowid 1:1 action mirred egress redirect dev ifb$ifbdev" +tc filter add dev $device parent 1: prio 10 u32 match u32 0 0 flowid 1:1 action mirred egress redirect dev ifb$ifbdev" elif [ -n "$download" ]; then append dev_${dir} "tc qdisc del dev $device ingress >&- 2>&- tc qdisc add dev $device ingress -tc filter add dev $device parent ffff: protocol ip prio 1 u32 match u32 0 0 flowid 1:1 action connmark action mirred egress redirect dev ifb$ifbdev" "$N" +tc filter add dev $device parent ffff: prio 1 u32 match u32 0 0 flowid 1:1 action connmark action mirred egress redirect dev ifb$ifbdev" "$N" fi add_insmod cls_fw add_insmod sch_hfsc @@ -397,17 +397,23 @@ start_cg() { local pktrules local sizerules enum_classes "$cg" - add_rules iptrules "$ctrules" "iptables -t mangle -A qos_${cg}_ct" + for command in $iptables; do + add_rules iptrules "$ctrules" "$command -w -t mangle -A qos_${cg}_ct" + done config_get classes "$cg" classes for class in $classes; do config_get mark "$class" classnr config_get maxsize "$class" maxsize [ -z "$maxsize" -o -z "$mark" ] || { add_insmod xt_length - append pktrules "iptables -t mangle -A qos_${cg} -m mark --mark $mark/0x0f -m length --length $maxsize: -j MARK --set-mark 0/0xff" "$N" + for command in $iptables; do + append pktrules "$command -w -t mangle -A qos_${cg} -m mark --mark $mark/0x0f -m length --length $maxsize: -j MARK --set-mark 0/0xff" "$N" + done } done - add_rules pktrules "$rules" "iptables -t mangle -A qos_${cg}" + for command in $iptables; do + add_rules pktrules "$rules" "$command -w -t mangle -A qos_${cg}" + done for iface in $INTERFACES; do config_get classgroup "$iface" classgroup config_get device "$iface" device @@ -416,18 +422,40 @@ start_cg() { config_get download "$iface" download config_get halfduplex "$iface" halfduplex download="${download:-${halfduplex:+$upload}}" - append up "iptables -t mangle -A OUTPUT -o $device -j qos_${cg}" "$N" - append up "iptables -t mangle -A FORWARD -o $device -j qos_${cg}" "$N" + for command in $iptables; do + append up "$command -w -t mangle -A OUTPUT -o $device -j qos_${cg}" "$N" + append up "$command -w -t mangle -A FORWARD -o $device -j qos_${cg}" "$N" + done done cat <<EOF $INSMOD -iptables -t mangle -N qos_${cg} >&- 2>&- -iptables -t mangle -N qos_${cg}_ct >&- 2>&- -${iptrules:+${iptrules}${N}iptables -t mangle -A qos_${cg}_ct -j CONNMARK --save-mark --mask 0xff} -iptables -t mangle -A qos_${cg} -j CONNMARK --restore-mark --mask 0x0f -iptables -t mangle -A qos_${cg} -m mark --mark 0/0x0f -j qos_${cg}_ct +EOF + +for command in $iptables; do + cat <<EOF + $command -w -t mangle -N qos_${cg} + $command -w -t mangle -N qos_${cg}_ct +EOF +done +cat <<EOF + ${iptrules:+${iptrules}${N}} +EOF +for command in $iptables; do + cat <<EOF + $command -w -t mangle -A qos_${cg}_ct -j CONNMARK --save-mark --mask 0xff + $command -w -t mangle -A qos_${cg} -j CONNMARK --restore-mark --mask 0x0f + $command -w -t mangle -A qos_${cg} -m mark --mark 0/0x0f -j qos_${cg}_ct +EOF +done +cat <<EOF $pktrules -${iptrules:+${iptrules}${N}iptables -t mangle -A qos_${cg} -j CONNMARK --save-mark --mask 0xff} +EOF +for command in $iptables; do + cat <<EOF + $command -w -t mangle -A qos_${cg} -j CONNMARK --save-mark --mask 0xff +EOF +done +cat <<EOF $up$N${down:+${down}$N} EOF unset INSMOD @@ -447,20 +475,22 @@ stop_firewall() { # remove rules referring to them, then delete them # Print rules in the mangle table, like iptables-save - iptables -t mangle -S | - # Find rules for the qos_* chains - grep '^-N qos_\|-j qos_' | - # Exclude rules in qos_* chains (inter-qos_* refs) - grep -v '^-A qos_' | - # Replace -N with -X and hold, with -F and print - # Replace -A with -D - # Print held lines at the end (note leading newline) - sed -e '/^-N/{s/^-N/-X/;H;s/^-X/-F/}' \ - -e 's/^-A/-D/' \ - -e '${p;g}' | - # Make into proper iptables calls - # Note: awkward in previous call due to hold space usage - sed -n -e 's/^./iptables -t mangle &/p' + for command in $iptables; do + $command -w -t mangle -S | + # Find rules for the qos_* chains + grep -E '(^-N qos_|-j qos_)' | + # Exclude rules in qos_* chains (inter-qos_* refs) + grep -v '^-A qos_' | + # Replace -N with -X and hold, with -F and print + # Replace -A with -D + # Print held lines at the end (note leading newline) + sed -e '/^-N/{s/^-N/-X/;H;s/^-X/-F/}' \ + -e 's/^-A/-D/' \ + -e '${p;g}' | + # Make into proper iptables calls + # Note: awkward in previous call due to hold space usage + sed -n -e "s/^./${command} -w -t mangle &/p" + done } C="0" @@ -475,6 +505,12 @@ for iface in $INTERFACES; do export C="$(($C + 1))" done +[ -x /usr/sbin/ip6tables ] && { + iptables="ip6tables iptables" +} || { + iptables="iptables" +} + case "$1" in all) start_interfaces "$C" diff --git a/package/network/config/qos-scripts/files/usr/lib/qos/tcrules.awk b/package/network/config/qos-scripts/files/usr/lib/qos/tcrules.awk index 12f94a6..21df391 100644 --- a/package/network/config/qos-scripts/files/usr/lib/qos/tcrules.awk +++ b/package/network/config/qos-scripts/files/usr/lib/qos/tcrules.awk @@ -84,7 +84,7 @@ END { # filter rule for (i = 1; i <= n; i++) { - filter_cmd = "tc filter add dev "device" parent 1: prio %d protocol ip handle %s fw flowid 1:%d0\n"; + filter_cmd = "tc filter add dev "device" parent 1: prio %d handle %s fw flowid 1:%d0\n"; if (direction == "up") { filter_1 = sprintf("0x%x0/0xf0", class[i]) filter_2 = sprintf("0x0%x/0x0f", class[i])
I apologize, my client mangled my previous attempt at resubmission. Here is an updated version with three improvements. The problem with the rules not being removed (which was not new and actually caused by a grep command incompatible with musl) was fixed by using an updated grep command (thanks nbd!). The problems pertaining to the xtables lock (including "too many links" and "directory not empty") were fixed by always executing ip[6]tables with the "-w" command-line argument to make it wait for the xtables lock. Lastly, I fixed a place where I hardcoded "iptables" and "ip6tables" instead of looping over the array like everywhere else. --trim here-- This adds IPv6 support to qos-scripts for both tc/qdisc and the iptables classification rules. The tc/qdisc part is accomplished by removing "protocol ip" from the tc command line, causing the rule to be applied to all protocols. The iptables part is accomplished by adding each rule using both iptables and ip6tables. This patch is based on previous work by Ilkka Ollakka and Dominique Martinet. Signed-off-by: Michael Marley <michael@michaelmarley.com> --- .../qos-scripts/files/usr/lib/qos/generate.sh | 90 +++++++++++++++------- .../qos-scripts/files/usr/lib/qos/tcrules.awk | 2 +- 2 files changed, 64 insertions(+), 28 deletions(-)