Hide Forgot
The provided example is broken in two ways. A patch fixing both has been sent upstream, but not accepted yet: Author: Phil Sutter <phil@nwl.cc> Date: Fri Jan 27 12:08:29 2017 +0100 man: tc-csum.8: Fix example This fixes two issues with the provided example: - Add missing 'dev' keyword to second command. - Use a real IPv4 address instead of a bogus hex value since that will be rejected by get_addr_ipv4(). Fixes: dbfb17a67f9c7 ("man: tc-csum.8: Add an example") Reported-by: Davide Caratti <dcaratti@redhat.com>
Upstream accepted the patch: commit 6bbe5e6290db5cf3e78d6a1a179c76535a2eea3f Author: Phil Sutter <phil@nwl.cc> Date: Sat Jan 28 12:59:10 2017 +0100 man: tc-csum.8: Fix example This fixes two issues with the provided example: - Add missing 'dev' keyword to second command. - Use a real IPv4 address instead of a bogus hex value since that will be rejected by get_addr_ipv4(). Fixes: dbfb17a67f9c7 ("man: tc-csum.8: Add an example") Reported-by: Davide Caratti <dcaratti@redhat.com> Signed-off-by: Phil Sutter <phil@nwl.cc>
Nit pick - after 6bbe5e6290db ("man: tc-csum.8: Fix example") description is now out of sync with the command example: EXAMPLES The following performs stateless NAT for incoming packets from 192.168.1.100 to new destination 18.52.86.120 (0x12345678 in hex). Assuming these are UDP packets, both IP and UDP checksums have to be recalculated: # tc qdisc add dev eth0 ingress handle ffff: # tc filter add dev eth0 prio 1 protocol ip parent ffff: \ u32 match ip src 192.0.2.100/32 flowid :1 \ action pedit munge ip dst set 198.51.100.1 pipe \ csum ip and udp Might be worth shooting a patch upstream.
(In reply to Jakub Sitnicki from comment #2) > Nit pick - after 6bbe5e6290db ("man: tc-csum.8: Fix example") ... > Might be worth shooting a patch upstream. sure, and maybe there is also a grammar fix to be upstreamed (I forgot mentioning it to Phil when we talked about this, sorry). 'action to fix for then incorrect checksums' I think 'then' is a 'the' , and maybe the whole sentence can be reworded as follows: diff --git a/man/man8/tc-csum.8 b/man/man8/tc-csum.8 index 718301d..bee06d2 100644 --- a/man/man8/tc-csum.8 +++ b/man/man8/tc-csum.8 @@ -29,9 +29,9 @@ csum - checksum update action The .B csum action triggers checksum recalculation of specified packet headers. It is -commonly used after packet editing using the +commonly used to fix incorrect checksums after the .B pedit -action to fix for then incorrect checksums. +action has modified the packet contents. .SH OPTIONS .TP .I TARGET
Fixed upstream in b09515553fded944713955815a3f1cc855384abd
Is it really fixed? I don't see Phil's patch in the last build, only typo patch, Example seems to be still broken - different ips in the text and in the commands. The following performs stateless NAT for incoming packets from 192.168.1.100 to new destination 18.52.86.120 (0x12345678 in hex). Assuming these are UDP packets, both IP and UDP checksums have to be recalculated: # tc qdisc add dev eth0 ingress handle ffff: # tc filter add dev eth0 prio 1 protocol ip parent ffff: \ u32 match ip src 192.0.2.100/32 flowid :1 \ action pedit munge ip dst set 198.51.100.1 pipe \ csum ip and udp
Hi Jaroslav, (In reply to Jaroslav Aster from comment #6) > Is it really fixed? I don't see Phil's patch in the last build, only typo > patch, Example seems to be still broken - different ips in the text and in > the commands. The patch I mentioned in comment 1 got pulled in implicitly with the rebase. Inconsistency between example and it's description regarding IP addresses is consistent with upstream. I'll prepare a patch, feel free to set QA_FAILED here or create a new ticket for RHEL7.6 if you think it's not worth waiting for upstream. Thanks, Phil
Patch sent upstream: https://marc.info/?l=linux-netdev&m=151197686415592&w=2
Hi Phil, Thanks. For me, this issue is not enough reason for the respin, but if your patch will be accepted by upstream and there will be a respin for some other reason, we will fix it.
Hi Jaroslav, (In reply to Jaroslav Aster from comment #9) > Thanks. For me, this issue is not enough reason for the respin, but if your > patch will be accepted by upstream and there will be a respin for some other > reason, we will fix it. There will be, due to bug 1459600. :) Cheers, Phil
I know, but has your patch been already accepted by upstream? This is the second condition. If you think, it will take only couple of days, we can wait.
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-2018:0815