Bug 1477523
| Summary: | tc filter with action simple can't be added | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 7 | Reporter: | Jaroslav Aster <jaster> |
| Component: | iproute | Assignee: | Phil Sutter <psutter> |
| Status: | CLOSED ERRATA | QA Contact: | Jaroslav Aster <jaster> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | 7.4-Alt | CC: | atragler, psutter |
| Target Milestone: | rc | Keywords: | Regression |
| Target Release: | 7.4-Alt | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | iproute-4.11.0-4.el7a | Doc Type: | If docs needed, set a value |
| Doc Text: | Story Points: | --- | |
| Clone Of: | 1315930 | Environment: | |
| Last Closed: | 2017-11-09 11:25:03 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
Ah, Jamal broke it (intentionally) in release v4.10.0:
commit fdf1bdd0f14db7b465d93ec09e45e5da70b1c582
Author: Jamal Hadi Salim <jhs>
Date: Sun May 8 11:02:06 2016 -0400
tc simple action update and breakage
Brings it closer to more serious actions (adding branching
and allowing for late binding)
Unfortunately this breaks old syntax of the simple action.
But because simple is a pedagogical example unlikely to be used
in production environments (i.e its role is to serve as an example
on how to write actions), then this is ok.
New syntax for simple has new keyword "sdata". Example usage is:
sudo tc actions add action simple sdata "foobar" index 1
or
tc filter add dev $DEV parent ffff: protocol ip prio 1 u32\
match ip dst 17.0.0.1/32 flowid 1:10 action simple sdata "foobar"
Signed-off-by: Jamal Hadi Salim <jhs>
Jaroslav, I think he's right in that this action shouldn't be used in production environments. IIRC, we only added it to RHEL7 for (internal) testing purposes. Reading Shuang's comment from the original Bug 1315930 again, he didn't even use it eventually. Given these circumstances, are you fine with adjusting your testcase (you'd have to add 'sdata' keyword before the string) or should I try to establish backwards compatibility upstream?
Cheers, Phil
Hi Phil,
thanks for the information. I'm ok with that, but information in man page tc-simple does not correspond new syntax. We should fix it.
tc-simple:
...
SYNOPSIS
tc ... action simple STRING
...
hadi@noma1:$tc filter add dev eth0 parent ffff: protocol ip prio 5 \
u32 match ip protocol 1 0xff flowid 1:1 action simple "Incoming ICMP"
...
Fix submitted upstream: http://marc.info/?l=linux-netdev&m=150177248310588&w=2 Upstream accepted, and fixed my patch as well:
commit e2a055dd23f0e7527a987c24687cb6b0b86f0cde
Author: Phil Sutter <phil>
Date: Thu Aug 3 17:00:51 2017 +0200
tc-simple: Fix documentation
- CONTROL has to come last, otherwise 'index' applies to gact and not
simple itself.
- Man page wasn't updated to reflect syntax changes.
Signed-off-by: Phil Sutter <phil>
commit 620fc6696d4f4e9ad540a45892873b0382907739
Author: Stephen Hemminger <stephen>
Date: Thu Aug 3 16:10:18 2017 -0700
tc: fix m_simple usage
Signed-off-by: Stephen Hemminger <stephen>
Unfortunately update of tc-simple man-page is not correct. There is reference to tc-actions man-page, but this man-page does not exist in current iproute. Fix is simple, s/tc-actions/tc-policy/ tc-simple.
# rpm -q iproute
iproute-4.11.0-2.el7a.aarch64
# man tc-simple | col -b | grep tc-actions
Indicate how tc should proceed after executing the action. For a description of the possible CONTROL values, see tc-actions(8).
tc(8) tc-actions(8)
# man tc-actions
No manual entry for tc-actions
Switching to assigned state.
Unfortunately, fix is not complete. There is another mention about tc-actions man-page in tc-simple.
# rpm -q iproute
iproute-4.11.0-3.el7a.x86_64
# man tc-simple|col -b| grep tc-actions
tc(8) tc-actions(8)
Switching it to assigned state.
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHEA-2017:3172 |
This functionality was added in 7.3 and it is not part of rhel-alt. It is a regression against 7.4. iproute-4.11.0-1.el7a # tc filter add dev TestIface protocol ip parent 1:0 basic match 'meta(priority eq 0x1)' action simple 'Priority is set' Usage:... simple [sdata STRING] [CONTROL] [index INDEX] STRING being an arbitrary string CONTROL := reclassify|pipe|drop|continue|ok INDEX := optional index value used bad action parsing parse_action: bad value (2:simple)! Illegal "action" +++ This bug was initially created as a clone of Bug #1315930 +++ Description of problem: tc filter with action simple can't be added. # tc filter add dev eth0 protocol ip parent 1:0 basic match "meta(priority eq 0x1)" action simple "Priority is set" bad action type simple Usage: ... gact <ACTION> [RAND] [INDEX] Where: ACTION := reclassify | drop | continue | pass RAND := random <RANDTYPE> <ACTION> <VAL> RANDTYPE := netrand | determ VAL : = value not exceeding 10000 INDEX := index value used bad action parsing parse_action: bad value (2:simple)! Illegal "action" Version-Release number of selected component (if applicable): I have done the test on these versions: # rpm -q iproute iproute-2.6.32-54.el6.x86_64 (with kernel:2.6.32-621.el6.x86_64) # rpm -q iproute iproute-2.6.32-130.el6ost.netns.3.x86_64 (with kernel:2.6.32-604.el6.x86_64) # rpm -q iproute iproute-3.10.0-54.el7.x86_64 (with kernel:3.10.0-327.el7.x86_64) How reproducible: always Steps to Reproduce: 1. add tc qdisc # tc qdisc add dev eth0 root handle 1: prio bands 4 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 3 2. add tc filter # tc filter add dev eth0 protocol ip parent 1:0 basic match "meta(priority eq 0x1)" action simple "Priority is set" Actual results: tc filter command failed. Expected results: tc filter command succeed. Additional info: --- Additional comment from Phil Sutter on 2016-03-09 06:36:40 EST --- Hi Li Shuang, It appears that the simple action is neither present in RHEL6 nor RHEL7. Sorry for suggesting to use it without having checked in beforehand. Since this would be a new feature, I'm not comfortable with adding it to RHEL6. Luckily you found a workaround for your testing, so there seems no mandatory need for it (please correct me if I'm wrong). We could add the simple action to RHEL7 instead, but since the xt action is supposed to work there, at least you should be fine without. What do you think? Cheers, Phil --- Additional comment from Li Shuang on 2016-03-09 21:50:57 EST --- Hi Phil, I think it will be perfect if simple action is supported at least on RHEL7 because I'm not sure if someone else need to use it in the future. But now it's OK in my tests without simple action both on RHEL6 and RHEL7, so I'm fine without it. Cheers, Shuang --- Additional comment from Phil Sutter on 2016-03-10 06:50:32 EST --- Hi Shuang, OK, I will move this ticket to RHEL7 to start the process of backporting simple action support. Thanks, Phil --- Additional comment from Phil Sutter on 2016-03-10 06:52:08 EST --- Backport requires the following commit (and follow-ups): commit 087f46ee4ebd178a2a8562989fd9a4e02c93f406 Author: Jamal Hadi Salim <jhs> Date: Sun Sep 29 07:33:42 2013 -0400 tc: introduce simple action Simple action is already in the kernel for years now as an example. This complements it with user space control. Signed-off-by: Jamal Hadi Salim <jhs>