| Message ID | 20210722170128.223387-1-gnaaman@drivenets.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 227adfb2b1dfbc53dfc53b9dd7a93a6298ff7c56 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: Set true network header for ECN decapsulation | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 88 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
Gilad Naaman <gnaaman@drivenets.com> writes: > In cases where the header straight after the tunnel header was > another ethernet header (TEB), instead of the network header, > the ECN decapsulation code would treat the ethernet header as if > it was an IP header, resulting in mishandling and possible > wrong drops or corruption of the IP header. > > In this case, ECT(1) is sent, so IP_ECN_decapsulate tries to copy it to the > inner IPv4 header, and correct its checksum. > > The offset of the ECT bits in an IPv4 header corresponds to the > lower 2 bits of the second octet of the destination MAC address > in the ethernet header. > The IPv4 checksum corresponds to end of the source address. > > In order to reproduce: > > $ ip netns add A > $ ip netns add B > $ ip -n A link add _v0 type veth peer name _v1 netns B > $ ip -n A link set _v0 up > $ ip -n A addr add dev _v0 10.254.3.1/24 > $ ip -n A route add default dev _v0 scope global > $ ip -n B link set _v1 up > $ ip -n B addr add dev _v1 10.254.1.6/24 > $ ip -n B route add default dev _v1 scope global > $ ip -n B link add gre1 type gretap local 10.254.1.6 remote 10.254.3.1 key 0x49000000 > $ ip -n B link set gre1 up > > # Now send an IPv4/GRE/Eth/IPv4 frame where the outer header has ECT(1), > # and the inner header has no ECT bits set: > > $ cat send_pkt.py > #!/usr/bin/env python3 > from scapy.all import * > > pkt = IP(b'E\x01\x00\xa7\x00\x00\x00\x00@/`%\n\xfe\x03\x01\n\xfe\x01\x06 \x00eXI\x00' > b'\x00\x00\x18\xbe\x92\xa0\xee&\x18\xb0\x92\xa0l&\x08\x00E\x00\x00}\x8b\x85' > b'@\x00\x01\x01\xe4\xf2\x82\x82\x82\x01\x82\x82\x82\x02\x08\x00d\x11\xa6\xeb' > b'3\x1e\x1e\\xf3\\xf7`\x00\x00\x00\x00ZN\x00\x00\x00\x00\x00\x00\x10\x11\x12' > b'\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f !"#$%&\'()*+,-./01234' > b'56789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ') > > send(pkt) > $ sudo ip netns exec B tcpdump -neqlllvi gre1 icmp & ; sleep 1 > $ sudo ip netns exec A python3 send_pkt.py > > In the original packet, the source/destinatio MAC addresses are > dst=18:be:92:a0:ee:26 src=18:b0:92:a0:6c:26 > > In the received packet, they are > dst=18:bd:92:a0:ee:26 src=18:b0:92:a0:6c:27 > > Thanks to Lahav Schlesinger <lschlesinger@drivenets.com> and Isaac Garzon <isaac@speed.io> > for helping me pinpoint the origin. Oops! Thank you for the fix :) Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Thu, 22 Jul 2021 20:01:28 +0300 you wrote: > In cases where the header straight after the tunnel header was > another ethernet header (TEB), instead of the network header, > the ECN decapsulation code would treat the ethernet header as if > it was an IP header, resulting in mishandling and possible > wrong drops or corruption of the IP header. > > In this case, ECT(1) is sent, so IP_ECN_decapsulate tries to copy it to the > inner IPv4 header, and correct its checksum. > > [...] Here is the summary with links: - net: Set true network header for ECN decapsulation https://git.kernel.org/netdev/net/c/227adfb2b1df You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c index 0dca00745ac3..be75b409445c 100644 --- a/net/ipv4/ip_tunnel.c +++ b/net/ipv4/ip_tunnel.c @@ -390,7 +390,7 @@ int ip_tunnel_rcv(struct ip_tunnel *tunnel, struct sk_buff *skb, tunnel->i_seqno = ntohl(tpi->seq) + 1; } - skb_reset_network_header(skb); + skb_set_network_header(skb, (tunnel->dev->type == ARPHRD_ETHER) ? ETH_HLEN : 0); err = IP_ECN_decapsulate(iph, skb); if (unlikely(err)) {
In cases where the header straight after the tunnel header was another ethernet header (TEB), instead of the network header, the ECN decapsulation code would treat the ethernet header as if it was an IP header, resulting in mishandling and possible wrong drops or corruption of the IP header. In this case, ECT(1) is sent, so IP_ECN_decapsulate tries to copy it to the inner IPv4 header, and correct its checksum. The offset of the ECT bits in an IPv4 header corresponds to the lower 2 bits of the second octet of the destination MAC address in the ethernet header. The IPv4 checksum corresponds to end of the source address. In order to reproduce: $ ip netns add A $ ip netns add B $ ip -n A link add _v0 type veth peer name _v1 netns B $ ip -n A link set _v0 up $ ip -n A addr add dev _v0 10.254.3.1/24 $ ip -n A route add default dev _v0 scope global $ ip -n B link set _v1 up $ ip -n B addr add dev _v1 10.254.1.6/24 $ ip -n B route add default dev _v1 scope global $ ip -n B link add gre1 type gretap local 10.254.1.6 remote 10.254.3.1 key 0x49000000 $ ip -n B link set gre1 up # Now send an IPv4/GRE/Eth/IPv4 frame where the outer header has ECT(1), # and the inner header has no ECT bits set: $ cat send_pkt.py #!/usr/bin/env python3 from scapy.all import * pkt = IP(b'E\x01\x00\xa7\x00\x00\x00\x00@/`%\n\xfe\x03\x01\n\xfe\x01\x06 \x00eXI\x00' b'\x00\x00\x18\xbe\x92\xa0\xee&\x18\xb0\x92\xa0l&\x08\x00E\x00\x00}\x8b\x85' b'@\x00\x01\x01\xe4\xf2\x82\x82\x82\x01\x82\x82\x82\x02\x08\x00d\x11\xa6\xeb' b'3\x1e\x1e\\xf3\\xf7`\x00\x00\x00\x00ZN\x00\x00\x00\x00\x00\x00\x10\x11\x12' b'\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f !"#$%&\'()*+,-./01234' b'56789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ') send(pkt) $ sudo ip netns exec B tcpdump -neqlllvi gre1 icmp & ; sleep 1 $ sudo ip netns exec A python3 send_pkt.py In the original packet, the source/destinatio MAC addresses are dst=18:be:92:a0:ee:26 src=18:b0:92:a0:6c:26 In the received packet, they are dst=18:bd:92:a0:ee:26 src=18:b0:92:a0:6c:27 Thanks to Lahav Schlesinger <lschlesinger@drivenets.com> and Isaac Garzon <isaac@speed.io> for helping me pinpoint the origin. Fixes: b723748750ec ("tunnel: Propagate ECT(1) when decapsulating as recommended by RFC6040") Cc: David S. Miller <davem@davemloft.net> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org> Cc: David Ahern <dsahern@kernel.org> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Toke Høiland-Jørgensen <toke@redhat.com> Signed-off-by: Gilad Naaman <gnaaman@drivenets.com> --- net/ipv4/ip_tunnel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)