[ovs-dev] [PATCH ovn] Fix tcp_reset action handling
numans at ovn.org
numans at ovn.org
Thu Feb 6 06:45:36 UTC 2020
From: Numan Siddique <numans at ovn.org>
The current handling of tcp_reset, has few issues.
1. The IP and TCP checksum of the packet injected by ovn-controller is wrong.
2. The ack tcp flag is not set.
3. If there are load balancers configured, the tcp reset packet is sent to the
conntrack, which results in packet getting dropped in the subsequent upcalls
by ovs-vswitchd.
This patch fixes these issues. In ovn-northd lflows are added to skip sending
the tcp reset packets to conntrack.
Reported-by: Tim Rozet <trozet at redhat.com>
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1795790
Signed-off-by: Numan Siddique <numans at ovn.org>
---
controller/pinctrl.c | 78 +++++++++++++++++------------------
northd/ovn-northd.c | 12 ++++--
tests/ovn.at | 20 ++++-----
tests/system-ovn.at | 98 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 153 insertions(+), 55 deletions(-)
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index a35c73af4..09b0f7bf7 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -300,6 +300,19 @@ static void svc_monitors_run(struct rconn *swconn,
OVS_REQUIRES(pinctrl_mutex);
static void svc_monitors_wait(long long int svc_monitors_next_run_time);
+static void pinctrl_compose_ipv4(struct dp_packet *packet,
+ struct eth_addr eth_src,
+ struct eth_addr eth_dst, ovs_be32 ipv4_src,
+ ovs_be32 ipv4_dst, uint8_t ip_proto,
+ uint8_t ttl, uint16_t ip_payload_len);
+static void pinctrl_compose_ipv6(struct dp_packet *packet,
+ struct eth_addr eth_src,
+ struct eth_addr eth_dst,
+ struct in6_addr *ipv6_src,
+ struct in6_addr *ipv6_dst,
+ uint8_t ip_proto, uint8_t ttl,
+ uint16_t ip_payload_len);
+
COVERAGE_DEFINE(pinctrl_drop_put_mac_binding);
COVERAGE_DEFINE(pinctrl_drop_buffered_packets_map);
COVERAGE_DEFINE(pinctrl_drop_controller_event);
@@ -911,57 +924,40 @@ pinctrl_handle_tcp_reset(struct rconn *swconn, const struct flow *ip_flow,
struct dp_packet packet;
dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
- dp_packet_clear(&packet);
packet.packet_type = htonl(PT_ETH);
-
- struct eth_header *eh = dp_packet_put_zeros(&packet, sizeof *eh);
- eh->eth_dst = ip_flow->dl_dst;
- eh->eth_src = ip_flow->dl_src;
-
if (get_dl_type(ip_flow) == htons(ETH_TYPE_IPV6)) {
- struct ip6_hdr *nh = dp_packet_put_zeros(&packet, sizeof *nh);
-
- eh->eth_type = htons(ETH_TYPE_IPV6);
- dp_packet_set_l3(&packet, nh);
- nh->ip6_vfc = 0x60;
- nh->ip6_nxt = IPPROTO_TCP;
- nh->ip6_plen = htons(TCP_HEADER_LEN);
- packet_set_ipv6(&packet, &ip_flow->ipv6_src, &ip_flow->ipv6_dst,
- ip_flow->nw_tos, ip_flow->ipv6_label, 255);
+ pinctrl_compose_ipv6(&packet, ip_flow->dl_src, ip_flow->dl_dst,
+ (struct in6_addr *)&ip_flow->ipv6_src,
+ (struct in6_addr *)&ip_flow->ipv6_dst,
+ IPPROTO_TCP, 63, TCP_HEADER_LEN);
} else {
- struct ip_header *nh = dp_packet_put_zeros(&packet, sizeof *nh);
-
- eh->eth_type = htons(ETH_TYPE_IP);
- dp_packet_set_l3(&packet, nh);
- nh->ip_ihl_ver = IP_IHL_VER(5, 4);
- nh->ip_tot_len = htons(IP_HEADER_LEN + TCP_HEADER_LEN);
- nh->ip_proto = IPPROTO_TCP;
- nh->ip_frag_off = htons(IP_DF);
- packet_set_ipv4(&packet, ip_flow->nw_src, ip_flow->nw_dst,
- ip_flow->nw_tos, 255);
+ pinctrl_compose_ipv4(&packet, ip_flow->dl_src, ip_flow->dl_dst,
+ ip_flow->nw_src, ip_flow->nw_dst,
+ IPPROTO_TCP, 63, TCP_HEADER_LEN);
}
struct tcp_header *th = dp_packet_put_zeros(&packet, sizeof *th);
- struct tcp_header *tcp_in = dp_packet_l4(pkt_in);
dp_packet_set_l4(&packet, th);
- th->tcp_ctl = TCP_CTL(TCP_RST, 5);
- if (ip_flow->tcp_flags & htons(TCP_ACK)) {
- th->tcp_seq = tcp_in->tcp_ack;
- } else {
- uint32_t tcp_seq, ack_seq, tcp_len;
+ th->tcp_dst = ip_flow->tp_src;
+ th->tcp_src = ip_flow->tp_dst;
- tcp_seq = ntohl(get_16aligned_be32(&tcp_in->tcp_seq));
- tcp_len = TCP_OFFSET(tcp_in->tcp_ctl) * 4;
- ack_seq = tcp_seq + dp_packet_l4_size(pkt_in) - tcp_len;
- put_16aligned_be32(&th->tcp_ack, htonl(ack_seq));
- put_16aligned_be32(&th->tcp_seq, 0);
- }
- packet_set_tcp_port(&packet, ip_flow->tp_dst, ip_flow->tp_src);
+ th->tcp_ctl = htons((5 << 12) | TCP_RST | TCP_ACK);
+ put_16aligned_be32(&th->tcp_seq, 0);
- if (ip_flow->vlans[0].tci & htons(VLAN_CFI)) {
- eth_push_vlan(&packet, htons(ETH_TYPE_VLAN_8021Q),
- ip_flow->vlans[0].tci);
+ struct tcp_header *tcp_in = dp_packet_l4(pkt_in);
+ uint32_t tcp_seq = ntohl(get_16aligned_be32(&tcp_in->tcp_seq)) + 1;
+ put_16aligned_be32(&th->tcp_ack, htonl(tcp_seq));
+
+ uint32_t csum;
+ if (get_dl_type(ip_flow) == htons(ETH_TYPE_IPV6)) {
+ csum = packet_csum_pseudoheader6(dp_packet_l3(&packet));
+ } else {
+ csum = packet_csum_pseudoheader(dp_packet_l3(&packet));
}
+ csum = csum_continue(csum, th, dp_packet_size(&packet) -
+ ((const unsigned char *)th -
+ (const unsigned char *)dp_packet_eth(&packet)));
+ th->tcp_csum = csum_finish(csum);
set_actions_and_enqueue_msg(swconn, &packet, md, userdata);
dp_packet_uninit(&packet);
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 6e113b35c..42ae5c61f 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -4638,11 +4638,11 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows)
* unreachable packets. */
ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
"nd || nd_rs || nd_ra || icmp4.type == 3 || "
- "icmp6.type == 1 || (tcp && tcp.flags == 4)",
+ "icmp6.type == 1 || (tcp && tcp.flags == 20)",
"next;");
ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
"nd || nd_rs || nd_ra || icmp4.type == 3 || "
- "icmp6.type == 1 || (tcp && tcp.flags == 4)",
+ "icmp6.type == 1 || (tcp && tcp.flags == 20)",
"next;");
/* Ingress and Egress Pre-ACL Table (Priority 100).
@@ -4750,9 +4750,13 @@ build_pre_lb(struct ovn_datapath *od, struct hmap *lflows,
{
/* Do not send ND packets to conntrack */
ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
- "nd || nd_rs || nd_ra", "next;");
+ "nd || nd_rs || nd_ra || icmp4.type == 3 ||"
+ "icmp6.type == 1 || (tcp && tcp.flags == 20)",
+ "next;");
ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
- "nd || nd_rs || nd_ra", "next;");
+ "nd || nd_rs || nd_ra || icmp4.type == 3 ||"
+ "icmp6.type == 1 || (tcp && tcp.flags == 20)",
+ "next;");
/* Do not send service monitor packets to conntrack. */
char *svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac);
diff --git a/tests/ovn.at b/tests/ovn.at
index 091351c0e..8e18b24ec 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -11681,8 +11681,8 @@ test_tcp_syn_packet() {
local ip_ttl=ff
local packet=${eth_dst}${eth_src}08004500002800004000${ip_ttl}06${ip_chksum}${ipv4_src}${ipv4_dst}${tcp_sport}${tcp_dport}000000010000000050027210${tcp_chksum}0000
- local tcp_rst_ttl=ff
- local reply=${eth_src}${eth_dst}08004500002800004000${tcp_rst_ttl}06${exp_ip_chksum}${ipv4_dst}${ipv4_src}${tcp_dport}${tcp_sport}000000000000000150040000${exp_tcp_rst_chksum}0000
+ local tcp_rst_ttl=3f
+ local reply=${eth_src}${eth_dst}08004500002800004000${tcp_rst_ttl}06${exp_ip_chksum}${ipv4_dst}${ipv4_src}${tcp_dport}${tcp_sport}000000000000000250140000${exp_tcp_rst_chksum}0000
echo $reply >> vif$inport.expected
as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet
@@ -11738,9 +11738,9 @@ test_ip_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 168 1 31) $(ip_to_
test_ipv6_packet 11 1 000000000011 000000000021 fe80000000000000020001fffe000001 fe80000000000000020001fffe000002 6183
-test_tcp_syn_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168 1 11) $(ip_to_hex 192 168 1 21) 0000 8b40 3039 0000 7d8d 4486
-test_tcp_syn_packet 21 2 000000000021 000000000011 $(ip_to_hex 192 168 1 21) $(ip_to_hex 192 168 1 11) 0000 8b40 3039 0000 7d8d 4486
-test_tcp_syn_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 168 1 31) $(ip_to_hex 192 168 1 12) 0000 8b40 3039 0000 7d82 4486
+test_tcp_syn_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168 1 11) $(ip_to_hex 192 168 1 21) 0000 8b40 3039 0000 b85f 70e4
+test_tcp_syn_packet 21 2 000000000021 000000000011 $(ip_to_hex 192 168 1 21) $(ip_to_hex 192 168 1 11) 0000 8b40 3039 0000 b85f 70e4
+test_tcp_syn_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 168 1 31) $(ip_to_hex 192 168 1 12) 0000 8b40 3039 0000 b854 70d9
for i in 1 2 3; do
OVN_CHECK_PACKETS([hv$i/vif${i}1-tx.pcap], [vif${i}1.expected])
@@ -12814,8 +12814,8 @@ test_tcp_syn_packet() {
local ip_ttl=ff
local packet=${eth_dst}${eth_src}08004500002800004000${ip_ttl}06${ip_chksum}${ipv4_src}${ip_router}${tcp_sport}${tcp_dport}000000010000000050027210${tcp_chksum}0000
- local tcp_rst_ttl=fe
- local reply=${eth_src}${eth_dst}08004500002800004000${tcp_rst_ttl}06${exp_ip_chksum}${ip_router}${ipv4_src}${tcp_dport}${tcp_sport}000000000000000150040000${exp_tcp_rst_chksum}0000
+ local tcp_rst_ttl=3e
+ local reply=${eth_src}${eth_dst}08004500002800004000${tcp_rst_ttl}06${exp_ip_chksum}${ip_router}${ipv4_src}${tcp_dport}${tcp_sport}000000000000000250140000${exp_tcp_rst_chksum}0000
echo $reply >> vif$inport.expected
as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet
@@ -12835,7 +12835,7 @@ test_tcp6_packet() {
local ip6_hdr=60000000001406ff${ipv6_src}${ipv6_router}
local packet=${eth_dst}${eth_src}86dd${ip6_hdr}${tcp_sport}${tcp_dport}000000010000000050027210${tcp_chksum}0000
- local reply=${eth_src}${eth_dst}86dd60000000001406fe${ipv6_router}${ipv6_src}${tcp_dport}${tcp_sport}000000000000000150040000${exp_tcp_rst_chksum}0000
+ local reply=${eth_src}${eth_dst}86dd600000000014063e${ipv6_router}${ipv6_src}${tcp_dport}${tcp_sport}000000000000000250140000${exp_tcp_rst_chksum}0000
echo $reply >> vif$inport.expected
as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet
@@ -12901,9 +12901,9 @@ test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_he
test_ip6_packet 1 1 000000000001 00000000ff01 20010db8000100000000000000000011 20010db8000100000000000000000001 11 0015 dbb8303900155bac6b646f65206676676e6d66720a 0104 d570
OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected])
-test_tcp_syn_packet 2 2 000000000002 00000000ff02 $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 2 254) 0000 8b40 3039 0000 7bae 4486
+test_tcp_syn_packet 2 2 000000000002 00000000ff02 $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 2 254) 0000 8b40 3039 0000 b680 6e05
test_ip6_packet 2 2 000000000002 00000000ff02 20010db8000200000000000000000011 20010db8000200000000000000000001 84 0004 01020304 0103 627e
-test_tcp6_packet 2 2 000000000002 00000000ff02 20010db8000200000000000000000011 20010db8000200000000000000000001 8b40 3039 0000 4486
+test_tcp6_packet 2 2 000000000002 00000000ff02 20010db8000200000000000000000011 20010db8000200000000000000000001 8b40 3039 0000 98cd
OVN_CHECK_PACKETS([hv2/vif2-tx.pcap], [vif2.expected])
OVN_CLEANUP([hv1], [hv2])
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 662d71251..53da910cb 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -3623,3 +3623,101 @@ as
OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
/connection dropped.*/d"])
AT_CLEANUP
+
+
+AT_SETUP([ovn -- ACL reject - TCP reset])
+AT_SKIP_IF([test $HAVE_NC = no])
+AT_KEYWORDS([lb])
+
+ovn_start
+
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-int])
+
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+ -- set Open_vSwitch . external-ids:system-id=hv1 \
+ -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+ -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+ -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+ -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+# Start ovn-controller
+start_daemon ovn-controller
+
+ovn-nbctl ls-add sw0
+
+ovn-nbctl lsp-add sw0 sw0-p1
+ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3 aef0::3"
+ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3 aef0::3"
+
+ovn-nbctl lsp-add sw0 sw0-p2
+ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4 aef0::4"
+ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4 aef0::4"
+
+ovn-nbctl --log acl-add sw0 from-lport 1000 "inport == \"sw0-p1\" && tcp && tcp.dst == 80" reject
+ovn-nbctl --log acl-add sw0 from-lport 1000 "inport == \"sw0-p2\" && ip6 && tcp && tcp.dst == 80" reject
+
+OVN_POPULATE_ARP
+ovn-nbctl --wait=hv sync
+
+ADD_NAMESPACES(sw0-p1)
+ADD_VETH(sw0-p1, sw0-p1, br-int, "10.0.0.3/24", "50:54:00:00:00:03", \
+ "10.0.0.1")
+
+ADD_NAMESPACES(sw0-p2)
+ADD_VETH(sw0-p2, sw0-p2, br-int, "10.0.0.4/24", "50:54:00:00:00:04", \
+ "10.0.0.1")
+
+NS_CHECK_EXEC([sw0-p1], [ip a a aef0::3/64 dev sw0-p1], [0])
+NS_CHECK_EXEC([sw0-p2], [ip a a aef0::4/64 dev sw0-p2], [0])
+
+# Capture packets in sw0-p1.
+NS_CHECK_EXEC([sw0-p1], [tcpdump -n -c 2 -i sw0-p1 tcp port 80 > sw0-p1-ip4.pcap &], [0])
+sleep 1
+
+NS_CHECK_EXEC([sw0-p1], [nc 10.0.0.4 80], [1], [],
+[dnl
+Ncat: Connection refused.
+])
+
+OVS_WAIT_UNTIL([
+ total=`cat sw0-p1-ip4.pcap | wc -l`
+ echo "total = $total"
+ test "${total}" = "2"
+])
+
+# Without this sleep, test case fails intermittently.
+sleep 3
+
+NS_CHECK_EXEC([sw0-p2], [tcpdump -n -c 2 -i sw0-p2 tcp port 80 > sw0-p2-ip6.pcap &], [0])
+
+sleep 1
+
+NS_CHECK_EXEC([sw0-p2], [nc -6 aef0::3 80], [1], [],
+[dnl
+Ncat: Connection refused.
+])
+
+OVS_WAIT_UNTIL([
+ total=`cat sw0-p2-ip6.pcap | wc -l`
+ echo "total = $total"
+ test "${total}" = "2"
+])
+
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([ovn-northd])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+
+AT_CLEANUP
--
2.24.1
More information about the dev
mailing list