Bug 1322406
| Summary: | TestOnly: Backport recent pedit action fixes from upstream | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 7 | Reporter: | Phil Sutter <psutter> |
| Component: | iproute | Assignee: | Andrea Claudi <aclaudi> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | BaseOS QE Security Team <qe-baseos-security> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | 7.3 | CC: | aclaudi, atragler, jaster, mleitner, omoris, sukulkar, tredaelli |
| Target Milestone: | rc | Keywords: | TestOnly |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | iproute-4.11.0-4.el7 | Doc Type: | If docs needed, set a value |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2020-03-02 17:14:53 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: | |||
| Bug Depends On: | 1435647 | ||
| Bug Blocks: | |||
Re-adding devel_ack+ since I still think it should be fixed. Backported all the requested commits:
commit ec0ceeec4954b1a5439ec3684460e8385454de90
commit f440e9d8c2f6f0eb2e9fccd8f4d7c42c11ba4979
commit 338b003bcc22a62c98b84dbe5e491cae84dbb03c
commit 77bed404d03f39adf2842ee4aefb53ba8a68f087
commit a33786b582a29a940cfe1eff826ff5a0548a6d81
Plus one dependent commit:
commit 0bbca0422f9779cc4eeaf70aa01dcad10d6ab076
Author: Maciej Żenczykowski <maze>
Date: Thu Jun 25 02:03:02 2015 -0700
iproute2: tc/m_pedit.c - remove dead code
The initializers are simply not needed.
These if-blocks are outright dead code, because '0 > unsigned' is always
false, so only else clause triggers and regardless of which clause triggers
it only updates 'ind' which is later unconditionally written to before
being used anyway.
Otherwise we get errors from clang:
m_pedit.c:166:8: error: comparison of 0 > unsigned expression is always false [-Werror,-Wtautological-compare]
if (0 > tkey->off) {
~ ^ ~~~~~~~~~
m_pedit.c:209:8: error: comparison of 0 > unsigned expression is always false [-Werror,-Wtautological-compare]
if (0 > tkey->off) {
~ ^ ~~~~~~~~~
2 errors generated.
Change-Id: I3c9e9092915088fc56f992e5df736851541a4458
Hi Dalibor, this ticket is a TestOnly one, which means no action required from devel side (apart from providing devel_ack+) but testing by QA is still missing. So whenever they get capacity for it, they should provide qa_ack+ and it will make it into the next errata and is tested for. Therefore we want to keep this ticket, otherwise the feature is there but we can't (officially) be sure it's working as intended. I'll anyway move this to rhel-7.8. |
The pedit tc action is severely broken in RHEL, as it was upstream until recently. The following fixes should be beackported: commit ec0ceeec4954b1a5439ec3684460e8385454de90 Author: Phil Sutter <phil> Date: Wed Mar 2 12:20:29 2016 +0100 tc: pedit: Fix layered op parsing After lookup of the layered op submodule, pedit would pass argv and argc including the layered op identifier at first position which confused the submodule parser. Fix this by calling NEXT_ARG() before calling the parse_peopt() callback. Signed-off-by: Phil Sutter <phil> commit f440e9d8c2f6f0eb2e9fccd8f4d7c42c11ba4979 Author: Phil Sutter <phil> Date: Wed Mar 2 12:20:30 2016 +0100 tc: pedit: Fix parse_cmd() This was horribly broken: * pack_key8() and pack_key16() ... * missed to invert retain value when applying it to the mask, * did not sanitize val by ANDing it with retain, * and ignored the mask which is necessary for 'invert' command. * pack_key16() did not convert mask to network byte order. * Changing the retain value for 'invert' or 'retain' operation seems just plain wrong. * While here, also got rid of unnecessary offset sanitization in pack_key32(). * Simplify code a bit by always assigning the local mask variable to tkey->mask before calling any of the pack_key*() variants. Signed-off-by: Phil Sutter <phil> commit 338b003bcc22a62c98b84dbe5e491cae84dbb03c Author: Phil Sutter <phil> Date: Wed Mar 2 12:20:31 2016 +0100 tc: pedit: Fix retain value for ihl adjustments Since the IP Header Length field is just half a byte, adjust retain to only match these bits so the Version field is not overwritten by accident. The whole concept is actually broken due to dependency on endianness which pedit ignores. Signed-off-by: Phil Sutter <phil> commit 77bed404d03f39adf2842ee4aefb53ba8a68f087 Author: Phil Sutter <phil> Date: Tue Mar 22 15:16:22 2016 +0100 tc: pedit: Fix for big-endian systems This was tricky to get right: - The 'stride' value used for 8 and 16 bit values must behave inverse to the value's intra word offset to work correctly with big-endian data act_pedit is editing. - The 'm' array's values are in host byte order, so they have to be converted as well (and the ordering was just inverse, for some reason). - The only sane way of getting this right is to manipulate value/mask in host byte order and convert the output. - TIPV4 (i.e. 'munge ip src/dst') had it's own pitfall: the address parser converts to network byte order automatically. This patch fixes this by converting it back before calling pack_key32, which is a hack but at least does not require to implement a completely separate code flow. Signed-off-by: Phil Sutter <phil> Signed-off-by: Stephen Hemminger <stephen> commit a33786b582a29a940cfe1eff826ff5a0548a6d81 Author: Phil Sutter <phil> Date: Tue Mar 22 15:16:23 2016 +0100 tc: pedit: Fix raw op The retain value was wrong for u16 and u8 types. Signed-off-by: Phil Sutter <phil> Signed-off-by: Stephen Hemminger <stephen>