From patchwork Thu Feb 9 10:19:08 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xavier Simonart X-Patchwork-Id: 1739867 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=CVLtAFbU; dkim-atps=neutral Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PCCX80yg5z23jH for ; Thu, 9 Feb 2023 21:19:20 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id D6D2640BB9; Thu, 9 Feb 2023 10:19:16 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org D6D2640BB9 Authentication-Results: smtp2.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=CVLtAFbU X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id m_xFLDX4Hb7c; Thu, 9 Feb 2023 10:19:15 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id 8254F404F7; Thu, 9 Feb 2023 10:19:14 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 8254F404F7 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 59070C0032; Thu, 9 Feb 2023 10:19:14 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id BC933C002B for ; Thu, 9 Feb 2023 10:19:12 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 89A5760BE9 for ; Thu, 9 Feb 2023 10:19:12 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 89A5760BE9 Authentication-Results: smtp3.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=CVLtAFbU X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eKtvsHowYuCo for ; Thu, 9 Feb 2023 10:19:11 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 157FF60B29 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp3.osuosl.org (Postfix) with ESMTPS id 157FF60B29 for ; Thu, 9 Feb 2023 10:19:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675937950; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=AtaW3NWne3d+pFz1FD3PEVILvYrbJvFqD76qDHMiYGI=; b=CVLtAFbU32QwYwAqjLunQ9qDMAQLEETX6wHoRrws1222twM7ydX3PQ+vyMDf6lEwK+FkRh krhmFWbVIRj3eYibcjeYmcG8O5IrEjVuY/6W6u8xIcSw8WQ/TLNvb58p3EZxvkJ6kTVDZw rU6gODSOiKAXAHkdELq+K4iq13cRJjE= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-636-KkLl4n0vOfurQVaHOoA6ow-1; Thu, 09 Feb 2023 05:19:09 -0500 X-MC-Unique: KkLl4n0vOfurQVaHOoA6ow-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E55E838060FE for ; Thu, 9 Feb 2023 10:19:08 +0000 (UTC) Received: from wsfd-netdev90.ntdv.lab.eng.bos.redhat.com (wsfd-netdev90.ntdv.lab.eng.bos.redhat.com [10.19.188.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id C6EFD2026D4B; Thu, 9 Feb 2023 10:19:08 +0000 (UTC) From: Xavier Simonart To: xsimonar@redhat.com, dev@openvswitch.org Date: Thu, 9 Feb 2023 05:19:08 -0500 Message-Id: <20230209101908.1020980-1-xsimonar@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn] northd: prevents sending packet to conntrack for router ports X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" As commented in northd.c, we should not use ct() for router ports. When there are no stateful_acl, this patch prevents sending packet to conntrack for router ports. However, this patch does not change the behavior for ACLs such as allow-related: packets are still sent to conntrack, even for router ports. While this does not work if router ports are distributed, allow-related ACLs work today on router ports when those ports are handled on the same chassis for ingress and egress traffic. This patch does not change that behavior. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2062431 Signed-off-by: Xavier Simonart --- northd/northd.c | 30 +++++--- tests/ovn-northd.at | 17 ++--- tests/system-ovn.at | 167 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 196 insertions(+), 18 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 77e105b86..f161ebebc 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -5766,20 +5766,30 @@ skip_port_from_conntrack(struct ovn_datapath *od, struct ovn_port *op, * know about the connection, as the icmp request went through the logical * router on hostA, not hostB. This would only work with distributed * conntrack state across all chassis. */ - struct ds match_in = DS_EMPTY_INITIALIZER; - struct ds match_out = DS_EMPTY_INITIALIZER; - ds_put_format(&match_in, "ip && inport == %s", op->json_key); - ds_put_format(&match_out, "ip && outport == %s", op->json_key); + char *ingress_action, *egress_action; + + if (od->has_stateful_acl) { + ingress_action = xasprintf("next;"); + egress_action = xasprintf("next;"); + } else { + ingress_action = xasprintf("ct_clear;next;"); + egress_action = xasprintf("ct_clear;next;"); + } + char *ingress_match = xasprintf("ip && inport == %s", op->json_key); + char *egress_match = xasprintf("ip && outport == %s", op->json_key); + ovn_lflow_add_with_lport_and_hint(lflows, od, in_stage, priority, - ds_cstr(&match_in), "next;", op->key, - &op->nbsp->header_); + ingress_match, ingress_action, + op->key, &op->nbsp->header_); ovn_lflow_add_with_lport_and_hint(lflows, od, out_stage, priority, - ds_cstr(&match_out), "next;", op->key, - &op->nbsp->header_); + egress_match, egress_action, + op->key, &op->nbsp->header_); - ds_destroy(&match_in); - ds_destroy(&match_out); + free(ingress_action); + free(egress_action); + free(ingress_match); + free(egress_match); } static void diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 3fa02d2b3..2e1788d73 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -4111,15 +4111,16 @@ check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0 check ovn-nbctl --wait=sb sync check_stateful_flows() { + action=$1 ovn-sbctl dump-flows sw0 > sw0flows AT_CAPTURE_FILE([sw0flows]) - AT_CHECK([grep "ls_in_pre_lb" sw0flows | sort | sed 's/table=./table=?/'], [0], [dnl + AT_CHECK_UNQUOTED([grep "ls_in_pre_lb" sw0flows | sort | sed 's/table=./table=?/'], [0], [dnl table=? (ls_in_pre_lb ), priority=0 , match=(1), action=(next;) table=? (ls_in_pre_lb ), priority=100 , match=(ip), action=(reg0[[2]] = 1; next;) - table=? (ls_in_pre_lb ), priority=110 , match=(eth.dst == $svc_monitor_mac), action=(next;) + table=? (ls_in_pre_lb ), priority=110 , match=(eth.dst == \$svc_monitor_mac), action=(next;) table=? (ls_in_pre_lb ), priority=110 , match=(eth.mcast), action=(next;) - table=? (ls_in_pre_lb ), priority=110 , match=(ip && inport == "sw0-lr0"), action=(next;) + table=? (ls_in_pre_lb ), priority=110 , match=(ip && inport == "sw0-lr0"), action=($action) table=? (ls_in_pre_lb ), priority=110 , match=(nd || nd_rs || nd_ra || mldv1 || mldv2), action=(next;) table=? (ls_in_pre_lb ), priority=110 , match=(reg0[[16]] == 1), action=(next;) ]) @@ -4144,12 +4145,12 @@ check_stateful_flows() { table=??(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_mark.blocked = 0; ct_label.label = reg3; }; next;) ]) - AT_CHECK([grep "ls_out_pre_lb" sw0flows | sort], [0], [dnl + AT_CHECK_UNQUOTED([grep "ls_out_pre_lb" sw0flows | sort], [0], [dnl table=1 (ls_out_pre_lb ), priority=0 , match=(1), action=(next;) table=1 (ls_out_pre_lb ), priority=100 , match=(ip), action=(reg0[[2]] = 1; next;) table=1 (ls_out_pre_lb ), priority=110 , match=(eth.mcast), action=(next;) - table=1 (ls_out_pre_lb ), priority=110 , match=(eth.src == $svc_monitor_mac), action=(next;) - table=1 (ls_out_pre_lb ), priority=110 , match=(ip && outport == "sw0-lr0"), action=(next;) + table=1 (ls_out_pre_lb ), priority=110 , match=(eth.src == \$svc_monitor_mac), action=(next;) + table=1 (ls_out_pre_lb ), priority=110 , match=(ip && outport == "sw0-lr0"), action=($action) table=1 (ls_out_pre_lb ), priority=110 , match=(nd || nd_rs || nd_ra || mldv1 || mldv2), action=(next;) table=1 (ls_out_pre_lb ), priority=110 , match=(reg0[[16]] == 1), action=(next;) ]) @@ -4169,13 +4170,13 @@ check_stateful_flows() { ]) } -check_stateful_flows +check_stateful_flows "ct_clear;next;" # Add few ACLs check ovn-nbctl --wait=sb acl-add sw0 from-lport 1002 "ip4 && tcp && tcp.dst == 80" allow-related check ovn-nbctl --wait=sb acl-add sw0 to-lport 1002 "ip4 && tcp && tcp.src == 80" drop -check_stateful_flows +check_stateful_flows "next;" # Remove load balancers from sw0 check ovn-nbctl ls-lb-del sw0 lb0 diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 2ece0f571..1cabf0233 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -10285,3 +10285,170 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d /connection dropped.*/d"]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ACL and commiting to conntrack]) +AT_KEYWORDS([acl]) + +CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() +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_daemon ovn-controller + +check ovn-nbctl lr-add r1 +check ovn-nbctl lrp-add r1 r1_s1 00:de:ad:fe:00:01 173.0.1.1/24 +check ovn-nbctl lrp-add r1 r1_s2 00:de:ad:fe:00:02 173.0.2.1/24 + +check ovn-nbctl ls-add s1 +check ovn-nbctl lsp-add s1 s1_r1 +check ovn-nbctl lsp-set-type s1_r1 router +check ovn-nbctl lsp-set-addresses s1_r1 router +check ovn-nbctl lsp-set-options s1_r1 router-port=r1_s1 + +check ovn-nbctl ls-add s2 +check ovn-nbctl lsp-add s2 s2_r1 +check ovn-nbctl lsp-set-type s2_r1 router +check ovn-nbctl lsp-set-addresses s2_r1 router +check ovn-nbctl lsp-set-options s2_r1 router-port=r1_s2 + +check ovn-nbctl lsp-add s1 vm1 +check ovn-nbctl lsp-set-addresses vm1 "00:de:ad:01:00:01 173.0.1.2" + +check ovn-nbctl lsp-add s2 vm2 +check ovn-nbctl lsp-set-addresses vm2 "00:de:ad:01:00:02 173.0.2.2" + +check ovn-nbctl lsp-add s2 vm3 +check ovn-nbctl lsp-set-addresses vm3 "00:de:ad:01:00:03 173.0.2.3" + +check ovn-nbctl lb-add lb1 30.0.0.1:80 173.0.2.2:80 udp +check ovn-nbctl lb-add lb2 20.0.0.1:80 173.0.1.2:80 udp +check ovn-nbctl lb-add lb1 30.0.0.1 173.0.2.2 +check ovn-nbctl lb-add lb2 173.0.2.250 173.0.1.3 +check ovn-nbctl ls-lb-add s1 lb1 +check ovn-nbctl ls-lb-add s2 lb2 + +ADD_NAMESPACES(vm1) +ADD_VETH(vm1, vm1, br-int, "173.0.1.2/24", "00:de:ad:01:00:01", \ + "173.0.1.1") +ADD_NAMESPACES(vm2) +ADD_VETH(vm2, vm2, br-int, "173.0.2.2/24", "00:de:ad:01:00:02", \ + "173.0.2.1") +ADD_NAMESPACES(vm3) +ADD_VETH(vm3, vm3, br-int, "173.0.2.250/24", "00:de:ad:01:00:03", \ + "173.0.2.1") + +check ovn-nbctl acl-add s1 from-lport 1001 "ip" allow +check ovn-nbctl acl-add s1 to-lport 1002 "ip" allow +check ovn-nbctl acl-add s2 from-lport 1003 "ip" allow +check ovn-nbctl acl-add s2 to-lport 1004 "ip" allow +check ovn-nbctl --wait=hv sync +AS_BOX([initial ping]) + +# Send ping in background. Same ping, same flow throughout the test +on_exit 'kill $(pidof ping)' +NS_EXEC([vm1], [ping -c 10000 -i 0.1 30.0.0.1 > icmp.txt &]) + +# Check for conntrack entries +OVS_WAIT_FOR_OUTPUT([ + ovs-appctl dpctl/dump-conntrack | FORMAT_CT(173.0.1.2) | \ + sed -e 's/zone=[[0-9]]*/zone=/'], [0], [dnl +icmp,orig=(src=173.0.1.2,dst=173.0.2.2,id=,type=8,code=0),reply=(src=173.0.2.2,dst=173.0.1.2,id=,type=0,code=0),zone= +icmp,orig=(src=173.0.1.2,dst=30.0.0.1,id=,type=8,code=0),reply=(src=173.0.2.2,dst=173.0.1.2,id=,type=0,code=0),zone=,mark=2 +]) + +# Now check for multiple ct_commits +ovs-appctl dpctl/dump-flows > dp_flows +zone_id=$(ovn-appctl -t ovn-controller ct-zone-list | grep vm1 | cut -d ' ' -f2) +AT_CHECK([test 1 = `cat dp_flows | grep "commit,zone=$zone_id" | wc -l`]) + +check ovn-nbctl acl-del s1 from-lport 1001 "ip" +check ovn-nbctl acl-del s1 to-lport 1002 "ip" +check ovn-nbctl acl-del s2 from-lport 1003 "ip" +check ovn-nbctl acl-del s2 to-lport 1004 "ip" + +AS_BOX([acl drop echo request]) +check ovn-nbctl --log --severity=alert --name=drop-flow-s1 acl-add s1 to-lport 2001 icmp4 drop +# acl-drop to-lport s1 apply to traffic from s1 to vm1 and s1 to r1. +check ovn-nbctl --wait=hv sync + +# Check that traffic is blocked +# Wait for some packets to hit the rule to avoid potential race conditions. Then count packets. +OVS_WAIT_UNTIL([test `cat ovn-controller.log | grep acl_log | grep -c drop-flow-s1` -gt "0"]) +total_icmp_pkts=$(cat icmp.txt | grep ttl | wc -l) + +# Wait some time and check whether packets went through. In the worse race condition, the sleep is too short +# and this test will still succeed. +sleep 1 +OVS_WAIT_UNTIL([ + total_icmp1_pkts=$(cat icmp.txt | grep ttl | wc -l) + test "${total_icmp1_pkts}" -eq "${total_icmp_pkts}" +]) + +AS_BOX([acl allow-related echo request]) +check ovn-nbctl acl-add s1 to-lport 2002 "icmp4 && ip4.src == 173.0.1.2" allow-related +# This rule has higher priority than to-lport 2001 icmp4 drop. +# So traffic from s1 (w/ src=173.0.1.2) to r1 should be accepted +# (return) traffic from s1 to vm1 should be accepted as return traffic +check ovn-nbctl --wait=hv sync +OVS_WAIT_UNTIL([ + total_icmp1_pkts=$(cat icmp.txt | grep ttl | wc -l) + test "${total_icmp1_pkts}" -gt "${total_icmp_pkts}" +]) + +# Check we did not break handling acl-drop for existing flows +AS_BOX([acl drop echo request in s2]) +check ovn-nbctl acl-del s1 to-lport 2001 icmp4 +check ovn-nbctl --log --severity=alert --name=drop-flow-s2 acl-add s2 to-lport 2001 icmp4 drop +check ovn-nbctl --wait=hv sync + +OVS_WAIT_UNTIL([test `cat ovn-controller.log | grep acl_log | grep -c drop-flow-s2` -gt "0"]) + +OVS_WAIT_FOR_OUTPUT([ + ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.1) | \ + sed -e 's/zone=[[0-9]]*/zone=/' | \ + sed -e 's/mark=[[0-9]]*/mark=/'], [0], [dnl +icmp,orig=(src=173.0.1.2,dst=30.0.0.1,id=,type=8,code=0),reply=(src=173.0.2.2,dst=173.0.1.2,id=,type=0,code=0),zone=,mark= +]) +total_icmp_pkts=$(cat icmp.txt | grep ttl | wc -l) + +# Allow ping again +AS_BOX([acl allow echo request in s2]) +check ovn-nbctl acl-add s2 to-lport 2005 icmp4 allow +check ovn-nbctl --wait=hv sync +OVS_WAIT_FOR_OUTPUT([ + ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.1) | \ + sed -e 's/zone=[[0-9]]*/zone=/'], [0], [dnl +icmp,orig=(src=173.0.1.2,dst=30.0.0.1,id=,type=8,code=0),reply=(src=173.0.2.2,dst=173.0.1.2,id=,type=0,code=0),zone=,mark=2 +]) +OVS_WAIT_UNTIL([ + total_icmp1_pkts=$(cat icmp.txt | grep ttl | wc -l) + test "${total_icmp1_pkts}" -gt "${total_icmp_pkts}" +]) + +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([NORTHD_TYPE]) + +as +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d +/connection dropped.*/d"]) +AT_CLEANUP +]) +