From patchwork Mon Mar 6 11:07:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xavier Simonart X-Patchwork-Id: 1752367 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=140.211.166.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=A+k2c8lA; dkim-atps=neutral Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.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 4PVbPt5PvJz1yWw for ; Mon, 6 Mar 2023 22:07:14 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id D80ED405AD; Mon, 6 Mar 2023 11:07:10 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org D80ED405AD 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=A+k2c8lA 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 M4ozS08X8-q4; Mon, 6 Mar 2023 11:07:09 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id 382EB400C4; Mon, 6 Mar 2023 11:07:08 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 382EB400C4 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 05D05C0071; Mon, 6 Mar 2023 11:07:08 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 6D054C0032 for ; Mon, 6 Mar 2023 11:07:06 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 3495240925 for ; Mon, 6 Mar 2023 11:07:06 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 3495240925 Authentication-Results: smtp4.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=A+k2c8lA X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id vTOQWCk765Bi for ; Mon, 6 Mar 2023 11:07:04 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org DB75840919 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp4.osuosl.org (Postfix) with ESMTPS id DB75840919 for ; Mon, 6 Mar 2023 11:07:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1678100822; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=LGDXn21PG3TZjoFGvvDDeiu68YN92D6bWGdEcKG/HJY=; b=A+k2c8lAc+4b0gQR94QkMZtPbT/VDP/oq0SGopXxCtyOAg3G8/aGtP97ue+hV60otKWcCW wWfMKco2ulqATs5o24mgmh1KMXyRxZIN4hBjy63vsTtWf/DWRd06JTlG49f+sbBCnm4JJt 3OMAqs/8HfPWhJRhySQV2yjHR9prgZ0= 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-534-96hl7DkfNXO4QjzNFuf7Fw-1; Mon, 06 Mar 2023 06:07:01 -0500 X-MC-Unique: 96hl7DkfNXO4QjzNFuf7Fw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5B0E53C0DDA1 for ; Mon, 6 Mar 2023 11:07:01 +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 0252840C83B6; Mon, 6 Mar 2023 11:07:00 +0000 (UTC) From: Xavier Simonart To: xsimonar@redhat.com, dev@openvswitch.org Date: Mon, 6 Mar 2023 12:07:00 +0100 Message-Id: <20230306110700.2107451-1-xsimonar@redhat.com> In-Reply-To: <20230209101908.1020980-1-xsimonar@redhat.com> References: <20230209101908.1020980-1-xsimonar@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: dceara@redhat.com Subject: [ovs-dev] [PATCH ovn v2] 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. The patch does this by issuing ct_clear in ls_out_pre_lb stage so that hints are not set in ls_out_acl_hint and ls_out_acl stages. Note that ct_clear is not added for ingress for router ports as already done for patch ports (no change by this patch on this aspect). Also, 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 Acked-by: Ales Musil --- v2: - handled Dumitru's comments - handled Ales' comments - added change to xml documentation - do not do ct_clear for ingress as already done --- northd/northd.c | 24 +++--- northd/ovn-northd.8.xml | 11 +++ tests/ovn-northd.at | 11 +-- tests/system-ovn.at | 166 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 197 insertions(+), 15 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 7ad4cdfad..b356faf64 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -5779,20 +5779,24 @@ 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); + const char *ingress_action = "next;"; + const char *egress_action = od->has_stateful_acl + ? "next;" + : "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_match); + free(egress_match); } static void diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 2eab2c4ae..efadfe808 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -2056,6 +2056,17 @@ output; db="OVN_Northbound"/> table.

+

+ This table also has a priority-110 flow with the match + outport == I for all logical switch + datapaths to move traffic to the next table, and, if there are no + stateful_acl, clear the ct_state. Where I + is the peer of a logical router port. This flow is added to + skip the connection tracking of packets which will be entering + logical router datapath from logical switch datapath for routing. +

+ +

Egress Table 2: Pre-stateful

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 3fa02d2b3..d0f6893e9 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -4111,6 +4111,7 @@ 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]) @@ -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 84a459d6a..73636feac 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -10706,3 +10706,169 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d /connection dropped.*/d"]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ACL and committing 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 +]) +