From patchwork Tue Jun 30 21:41:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1320133 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: 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=Q4gYyimQ; dkim-atps=neutral Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49xHqt0n7tz9sPF for ; Wed, 1 Jul 2020 07:41:46 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 95130884E6; Tue, 30 Jun 2020 21:41:44 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id oyq56jDxV1i4; Tue, 30 Jun 2020 21:41:43 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 35D07884CA; Tue, 30 Jun 2020 21:41:43 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1C88FC016E; Tue, 30 Jun 2020 21:41:43 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 65EB3C016E for ; Tue, 30 Jun 2020 21:41:41 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 49CE6230E6 for ; Tue, 30 Jun 2020 21:41:41 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id I6NjiX9cCdZX for ; Tue, 30 Jun 2020 21:41:38 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) by silver.osuosl.org (Postfix) with ESMTPS id 99C992052B for ; Tue, 30 Jun 2020 21:41:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593553297; 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=OynS/lruaaKeATcOD1c/v6QDsQREKi9LtIu8+hVMDho=; b=Q4gYyimQoKAOKSvBa3EB63A2DTxbKFPdlW+jX8sk/ts4ONJ66Onqv/s5sZwTxUw21BHKzj fRrCUEurdbJMQEHQ+YQiYsa2v3fGNVkhyJ4e+yVW/rIaawBIRpbIFpFPSUCZbQwt1yTNvd U+zyyuvNpP5tEJiyevtZ71y/RtpmJ1s= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-360-aM9A1o9gP4G94OzgHKoNbg-1; Tue, 30 Jun 2020 17:41:33 -0400 X-MC-Unique: aM9A1o9gP4G94OzgHKoNbg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4EA7A7BAF; Tue, 30 Jun 2020 21:41:32 +0000 (UTC) Received: from dceara.remote.csb (ovpn-112-145.ams2.redhat.com [10.36.112.145]) by smtp.corp.redhat.com (Postfix) with ESMTP id 593209CFF1; Tue, 30 Jun 2020 21:41:31 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Tue, 30 Jun 2020 23:41:27 +0200 Message-Id: <20200630214124.26800.43533.stgit@dceara.remote.csb> In-Reply-To: <20200630214107.26800.21305.stgit@dceara.remote.csb> References: <20200630214107.26800.21305.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: hzhou@ovn.org Subject: [ovs-dev] [PATCH v2 ovn 1/5] ovn-northd: Document OVS register usage in logical flows. 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" Also, use macros instead of bare references to register names. Signed-off-by: Dumitru Ceara Acked-by: Han Zhou --- northd/ovn-northd.c | 166 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 118 insertions(+), 48 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index ae1b85a..1a47f5f 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -227,8 +227,63 @@ enum ovn_stage { #define REG_ECMP_GROUP_ID "reg8[0..15]" #define REG_ECMP_MEMBER_ID "reg8[16..31]" +/* Registers used for routing. */ +#define REG_NEXT_HOP_IPV4 "reg0" +#define REG_NEXT_HOP_IPV6 "xxreg0" +#define REG_SRC_IPV4 "reg1" +#define REG_SRC_IPV6 "xxreg1" + #define FLAGBIT_NOT_VXLAN "flags[1] == 0" +/* + * OVS register usage: + * + * Logical Switch pipeline: + * +----------+-------------------------------------+ + * | R0 | REGBIT_{CONNTRACK/DHCP/DNS/HAIRPIN} | + * +----------+-------------------------------------+ + * | R1 - R15 | UNUSED | + * +----------+-------------------------------------+ + * + * Logical Router pipeline: + * +-----+--------------------------+---+-------------+ + * | R0 | REGBIT_ND_RA_OPTS_RESULT | | | + * | | IPv4-NEXT-HOP | | | + * +-----+--------------------------+ | | + * | R1 | IPv4-SRC-IP for ARP-REQ | | | + * +-----+--------------------------+ | | + * | R2 | UNUSED | X | | + * +-----+--------------------------+ X | | + * | R3 | UNUSED | R | IPv6 | + * +-----+--------------------------+ E | NEXT-HOP | + * | R4 | UNUSED | G | | + * +-----+--------------------------+ 0 | | + * | R5 | UNUSED | | | + * +-----+--------------------------+ | | + * | R6 | UNUSED | | | + * +-----+--------------------------+ | | + * | R7 | UNUSED | | | + * +-----+--------------------------+---+-------------+ + * | R8 | ECMP_GROUP_ID | | | + * | | ECMP_MEMBER_ID | | | + * +-----+--------------------------+ | | + * | R9 | UNUSED | | | + * +-----+--------------------------+ | | + * | R10 | UNUSED | X | | + * +-----+--------------------------+ X | | + * | R11 | UNUSED | R | IPv6-SRC-IP | + * +-----+--------------------------+ E | for NS | + * | R12 | UNUSED | G | | + * +-----+--------------------------+ 1 | | + * | R13 | UNUSED | | | + * +-----+--------------------------+ | | + * | R14 | UNUSED | | | + * +-----+--------------------------+ | | + * | R15 | UNUSED | | | + * +-----+--------------------------+---+-------------+ + * + */ + /* Returns an "enum ovn_stage" built from the arguments. */ static enum ovn_stage ovn_stage_build(enum ovn_datapath_type dp_type, enum ovn_pipeline pipeline, @@ -7127,15 +7182,15 @@ build_routing_policy_flow(struct hmap *lflows, struct ovn_datapath *od, ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark); } bool is_ipv4 = strchr(rule->nexthop, '.') ? true : false; - ds_put_format(&actions, "%sreg0 = %s; " - "%sreg1 = %s; " + ds_put_format(&actions, "%s = %s; " + "%s = %s; " "eth.src = %s; " "outport = %s; " "flags.loopback = 1; " "next;", - is_ipv4 ? "" : "xx", + is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6, rule->nexthop, - is_ipv4 ? "" : "xx", + is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, lrp_addr_s, out_port->lrp_networks.ea_s, out_port->json_key); @@ -7525,14 +7580,14 @@ build_ecmp_route_flow(struct hmap *lflows, struct ovn_datapath *od, REG_ECMP_MEMBER_ID" == %"PRIu16, eg->id, er->id); ds_clear(&actions); - ds_put_format(&actions, "%sreg0 = %s; " - "%sreg1 = %s; " + ds_put_format(&actions, "%s = %s; " + "%s = %s; " "eth.src = %s; " "outport = %s; " "next;", - is_ipv4 ? "" : "xx", + is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6, route->nexthop, - is_ipv4 ? "" : "xx", + is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, lrp_addr_s, out_port->lrp_networks.ea_s, out_port->json_key); @@ -7567,8 +7622,8 @@ add_route(struct hmap *lflows, const struct ovn_port *op, &match, &priority); struct ds actions = DS_EMPTY_INITIALIZER; - ds_put_format(&actions, "ip.ttl--; "REG_ECMP_GROUP_ID" = 0; %sreg0 = ", - is_ipv4 ? "" : "xx"); + ds_put_format(&actions, "ip.ttl--; "REG_ECMP_GROUP_ID" = 0; %s = ", + is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6); if (gateway) { ds_put_cstr(&actions, gateway); @@ -7576,12 +7631,12 @@ add_route(struct hmap *lflows, const struct ovn_port *op, ds_put_format(&actions, "ip%s.dst", is_ipv4 ? "4" : "6"); } ds_put_format(&actions, "; " - "%sreg1 = %s; " + "%s = %s; " "eth.src = %s; " "outport = %s; " "flags.loopback = 1; " "next;", - is_ipv4 ? "" : "xx", + is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, lrp_addr_s, op->lrp_networks.ea_s, op->json_key); @@ -9052,7 +9107,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, ds_put_format( &match, "outport == %s && %s == %s", od->l3dgw_port->json_key, - is_v6 ? "xxreg0" : "reg0", nat->external_ip); + is_v6 ? REG_NEXT_HOP_IPV6 : REG_NEXT_HOP_IPV4, + nat->external_ip); ds_clear(&actions); ds_put_format( &actions, "eth.dst = %s; next;", @@ -9214,9 +9270,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, * router, add flows that are specific to a NAT rule. These * flows indicate the presence of an applicable NAT rule that * can be applied in a distributed manner. - * In particulr reg1 and eth.src are set to NAT external IP and - * NAT external mac so the ARP request generated in the following - * stage is sent out with proper IP/MAC src addresses + * In particulr REG_SRC_IPV4/REG_SRC_IPV6 and eth.src are set to + * NAT external IP and NAT external mac so the ARP request + * generated in the following stage is sent out with proper IP/MAC + * src addresses. */ if (distributed) { ds_clear(&match); @@ -9226,8 +9283,9 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, "is_chassis_resident(\"%s\")", is_v6 ? "6" : "4", nat->logical_ip, od->l3dgw_port->json_key, nat->logical_port); - ds_put_format(&actions, "eth.src = %s; %sreg1 = %s; next;", - nat->external_mac, is_v6 ? "xx" : "", + ds_put_format(&actions, "eth.src = %s; %s = %s; next;", + nat->external_mac, + is_v6 ? REG_SRC_IPV6 : REG_SRC_IPV4, nat->external_ip); ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_GW_REDIRECT, 100, ds_cstr(&match), @@ -9549,14 +9607,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, * * For regular routes without ECMP, table IP_ROUTING sets outport to the * correct output port, eth.src to the output port's MAC address, and - * '[xx]reg0' to the next-hop IP address (leaving 'ip[46].dst', the - * packet’s final destination, unchanged), and advances to the next table. + * REG_NEXT_HOP_IPV4/REG_NEXT_HOP_IPV6 to the next-hop IP address + * (leaving 'ip[46].dst', the packet’s final destination, unchanged), and + * advances to the next table. * * For ECMP routes, i.e. multiple routes with same policy and prefix, table * IP_ROUTING remembers ECMP group id and selects a member id, and advances - * to table IP_ROUTING_ECMP, which sets outport, eth.src and '[xx]reg0' for - * the selected ECMP member. - * */ + * to table IP_ROUTING_ECMP, which sets outport, eth.src and + * REG_NEXT_HOP_IPV4/REG_NEXT_HOP_IPV6 for the selected ECMP member. + */ HMAP_FOR_EACH (op, key_node, ports) { if (!op->nbrp) { continue; @@ -9693,8 +9752,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, * permitted/denied/rerouted to the address in the rule's nexthop. * This table sets outport to the correct out_port, * eth.src to the output port's MAC address, - * and '[xx]reg0' to the next-hop IP address (leaving - * 'ip[46].dst', the packet’s final destination, unchanged), and + * and REG_NEXT_HOP_IPV4/REG_NEXT_HOP_IPV6 to the next-hop IP address + * (leaving 'ip[46].dst', the packet’s final destination, unchanged), and * advances to the next table for ARP/ND resolution. */ HMAP_FOR_EACH (od, key_node, datapaths) { if (!od->nbr) { @@ -9731,9 +9790,11 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, /* Local router ingress table ARP_RESOLVE: ARP Resolution. * * Any unicast packet that reaches this table is an IP packet whose - * next-hop IP address is in reg0. (ip4.dst is the final destination.) - * This table resolves the IP address in reg0 into an output port in - * outport and an Ethernet address in eth.dst. + * next-hop IP address is in REG_NEXT_HOP_IPV4/REG_NEXT_HOP_IPV6 + * (ip4.dst/ipv6.dst is the final destination). + * This table resolves the IP address in + * REG_NEXT_HOP_IPV4/REG_NEXT_HOP_IPV6 into an output port in outport and + * an Ethernet address in eth.dst. */ HMAP_FOR_EACH (op, key_node, ports) { if (op->nbsp && !lsp_is_enabled(op->nbsp)) { @@ -9742,17 +9803,18 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (op->nbrp) { /* This is a logical router port. If next-hop IP address in - * '[xx]reg0' matches IP address of this router port, then - * the packet is intended to eventually be sent to this - * logical port. Set the destination mac address using this - * port's mac address. + * REG_NEXT_HOP_IPV4/REG_NEXT_HOP_IPV6 matches IP address of this + * router port, then the packet is intended to eventually be sent + * to this logical port. Set the destination mac address using + * this port's mac address. * * The packet is still in peer's logical pipeline. So the match * should be on peer's outport. */ if (op->peer && op->nbrp->peer) { if (op->lrp_networks.n_ipv4_addrs) { ds_clear(&match); - ds_put_format(&match, "outport == %s && reg0 == ", + ds_put_format(&match, "outport == %s && " + REG_NEXT_HOP_IPV4 "== ", op->peer->json_key); op_put_v4_networks(&match, op, false); @@ -9767,7 +9829,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (op->lrp_networks.n_ipv6_addrs) { ds_clear(&match); - ds_put_format(&match, "outport == %s && xxreg0 == ", + ds_put_format(&match, "outport == %s && " + REG_NEXT_HOP_IPV6 " == ", op->peer->json_key); op_put_v6_networks(&match, op); @@ -9838,7 +9901,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, } ds_clear(&match); - ds_put_format(&match, "outport == %s && reg0 == %s", + ds_put_format(&match, "outport == %s && " + REG_NEXT_HOP_IPV4 " == %s", peer->json_key, ip_s); ds_clear(&actions); @@ -9874,7 +9938,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, } ds_clear(&match); - ds_put_format(&match, "outport == %s && xxreg0 == %s", + ds_put_format(&match, "outport == %s && " + REG_NEXT_HOP_IPV6 " == %s", peer->json_key, ip_s); ds_clear(&actions); @@ -9925,8 +9990,9 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (find_lrp_member_ip(peer, vip)) { ds_clear(&match); - ds_put_format(&match, "outport == %s && reg0 == %s", - peer->json_key, vip); + ds_put_format(&match, "outport == %s && " + REG_NEXT_HOP_IPV4 " == %s", + peer->json_key, vip); ds_clear(&actions); ds_put_format(&actions, @@ -9971,8 +10037,9 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, } ds_clear(&match); - ds_put_format(&match, "outport == %s && reg0 == %s", - peer->json_key, vip); + ds_put_format(&match, "outport == %s && " + REG_NEXT_HOP_IPV4 " == %s", + peer->json_key, vip); ds_clear(&actions); ds_put_format(&actions, "eth.dst = %s; next;", ea_s); @@ -10026,7 +10093,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (router_port->lrp_networks.n_ipv4_addrs) { ds_clear(&match); - ds_put_format(&match, "outport == %s && reg0 == ", + ds_put_format(&match, "outport == %s && " + REG_NEXT_HOP_IPV4 " == ", peer->json_key); op_put_v4_networks(&match, router_port, false); @@ -10041,7 +10109,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (router_port->lrp_networks.n_ipv6_addrs) { ds_clear(&match); - ds_put_format(&match, "outport == %s && xxreg0 == ", + ds_put_format(&match, "outport == %s && " + REG_NEXT_HOP_IPV6 " == ", peer->json_key); op_put_v6_networks(&match, router_port); @@ -10063,10 +10132,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, } ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_RESOLVE, 0, "ip4", - "get_arp(outport, reg0); next;"); + "get_arp(outport, " REG_NEXT_HOP_IPV4 "); next;"); ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_RESOLVE, 0, "ip6", - "get_nd(outport, xxreg0); next;"); + "get_nd(outport, " REG_NEXT_HOP_IPV6 "); next;"); } /* Local router ingress table CHK_PKT_LEN: Check packet length. @@ -10212,7 +10281,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, ds_clear(&match); ds_put_format(&match, "eth.dst == 00:00:00:00:00:00 && " - "ip6 && xxreg0 == %s", route->nexthop); + "ip6 && " REG_NEXT_HOP_IPV6 " == %s", + route->nexthop); struct in6_addr sn_addr; struct eth_addr eth_dst; in6_addr_solicited_node(&sn_addr, &gw_ip6); @@ -10240,15 +10310,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, "eth.dst == 00:00:00:00:00:00 && ip4", "arp { " "eth.dst = ff:ff:ff:ff:ff:ff; " - "arp.spa = reg1; " - "arp.tpa = reg0; " + "arp.spa = " REG_SRC_IPV4 "; " + "arp.tpa = " REG_NEXT_HOP_IPV4 "; " "arp.op = 1; " /* ARP request */ "output; " "};"); ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 100, "eth.dst == 00:00:00:00:00:00 && ip6", "nd_ns { " - "nd.target = xxreg0; " + "nd.target = " REG_NEXT_HOP_IPV6 "; " "output; " "};"); ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 0, "1", "output;"); From patchwork Tue Jun 30 21:41:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1320135 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: 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=cnVIb26t; dkim-atps=neutral Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49xHrg6vJnz9sPF for ; Wed, 1 Jul 2020 07:42:27 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 75142885D5; Tue, 30 Jun 2020 21:42:26 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6Lb7nuvoFdEy; Tue, 30 Jun 2020 21:42:24 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 810908852F; Tue, 30 Jun 2020 21:42:24 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 638FCC088F; Tue, 30 Jun 2020 21:42:24 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0503FC016E for ; Tue, 30 Jun 2020 21:42:23 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id E6CC7884DC for ; Tue, 30 Jun 2020 21:42:22 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Zo+DekQPVdtl for ; Tue, 30 Jun 2020 21:42:21 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by hemlock.osuosl.org (Postfix) with ESMTPS id 2A326885D5 for ; Tue, 30 Jun 2020 21:42:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593553340; 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=pcOshKq5ZWrK8BSmn9BNVZ/fmUqW8CU5UCpEwCjNq50=; b=cnVIb26tc97JiulDk32rl7DgY5nR06uJZofyyXUDCnxaCMDSexSTjCCpoxqVyF7G5A8quU /E5d2F0pdonEQZYvmDKuF/jK6G74M3KvZs73rRQtGPJwK20fj9mUksh05HpJA6OLvYxMb6 ZmQkAkQeIznlFRolCh9sliXOTbdPo+E= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-232-X_oj-wHjOD2hDK3m6UkiXw-1; Tue, 30 Jun 2020 17:41:46 -0400 X-MC-Unique: X_oj-wHjOD2hDK3m6UkiXw-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 99DDD800C64; Tue, 30 Jun 2020 21:41:45 +0000 (UTC) Received: from dceara.remote.csb (ovpn-112-145.ams2.redhat.com [10.36.112.145]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8441D1042A79; Tue, 30 Jun 2020 21:41:44 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Tue, 30 Jun 2020 23:41:42 +0200 Message-Id: <20200630214137.26800.27606.stgit@dceara.remote.csb> In-Reply-To: <20200630214107.26800.21305.stgit@dceara.remote.csb> References: <20200630214107.26800.21305.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: hzhou@ovn.org Subject: [ovs-dev] [PATCH v2 ovn 2/5] ovn-northd: Store ETH address of router inport in xreg0. 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" This helps simplifying logical flows that need to use the port's configured ETH address: - ARP responders for owned IPs - NS responders for owned IPs Signed-off-by: Dumitru Ceara --- northd/ovn-northd.8.xml | 22 ++++--- northd/ovn-northd.c | 159 +++++++++++++++++++++++++---------------------- tests/ovn-northd.at | 140 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 236 insertions(+), 85 deletions(-) diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index a7639f3..78e2a71 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -1487,7 +1487,9 @@ output; For each enabled router port P with Ethernet address E, a priority-50 flow that matches inport == P && (eth.mcast || eth.dst == - E), with action next;. + E), stores the router port ethernet address + and advances to next table, with action + xreg0[0..47]=E; next;.

@@ -1507,7 +1509,7 @@ output; a priority-50 flow that matches inport == GW && eth.dst == E, where GW is the logical router gateway port, with action - next;. + xreg0[0..47]=E; next;.

@@ -1770,10 +1772,10 @@ next; 

 eth.dst = eth.src;
-eth.src = E;
+eth.src = xreg0[0..47];
 arp.op = 2; /* ARP reply. */
 arp.tha = arp.sha;
-arp.sha = E;
+arp.sha = xreg0[0..47];
 arp.tpa = arp.spa;
 arp.spa = A;
 outport = P;
@@ -1822,10 +1824,10 @@ output;
 
         
 nd_na_router {
-    eth.src = E;
+    eth.src = xreg0[0..47];
     ip6.src = A;
     nd.target = A;
-    nd.tll = E;
+    nd.tll = xreg0[0..47];
     outport = inport;
     flags.loopback = 1;
     output;
@@ -1862,10 +1864,10 @@ nd_na_router {
 
         
 eth.dst = eth.src;
-eth.src = E;
+eth.src = xreg0[0..47];
 arp.op = 2; /* ARP reply. */
 arp.tha = arp.sha;
-arp.sha = E;
+arp.sha = xreg0[0..47];
 arp.tpa = arp.spa;
 arp.spa = A;
 outport = P;
@@ -1894,8 +1896,8 @@ output;
         
 eth.dst = eth.src;
 nd_na {
-    eth.src = E;
-    nd.tll = E;
+    eth.src = xreg0[0..47];
+    nd.tll = xreg0[0..47];
     ip6.src = A;
     nd.target = A;
     outport = P;
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 1a47f5f..9277040 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -223,6 +223,11 @@ enum ovn_stage {
 #define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[2]"
 #define REGBIT_SKIP_LOOKUP_NEIGHBOR "reg9[3]"
 
+/* Register to store the eth address associated to a router port for packets
+ * received in S_ROUTER_IN_ADMISSION.
+ */
+#define REG_INPORT_ETH_ADDR "xreg0[0..47]"
+
 /* Register for ECMP bucket selection. */
 #define REG_ECMP_GROUP_ID       "reg8[0..15]"
 #define REG_ECMP_MEMBER_ID      "reg8[16..31]"
@@ -246,41 +251,43 @@ enum ovn_stage {
  * +----------+-------------------------------------+
  *
  * Logical Router pipeline:
- * +-----+--------------------------+---+-------------+
- * | R0  | REGBIT_ND_RA_OPTS_RESULT |   |             |
- * |     |    IPv4-NEXT-HOP         |   |             |
- * +-----+--------------------------+   |             |
- * | R1  | IPv4-SRC-IP for ARP-REQ  |   |             |
- * +-----+--------------------------+   |             |
- * | R2  |        UNUSED            | X |             |
- * +-----+--------------------------+ X |             |
- * | R3  |        UNUSED            | R |    IPv6     |
- * +-----+--------------------------+ E |  NEXT-HOP   |
- * | R4  |        UNUSED            | G |             |
- * +-----+--------------------------+ 0 |             |
- * | R5  |        UNUSED            |   |             |
- * +-----+--------------------------+   |             |
- * | R6  |        UNUSED            |   |             |
- * +-----+--------------------------+   |             |
- * | R7  |        UNUSED            |   |             |
- * +-----+--------------------------+---+-------------+
- * | R8  |     ECMP_GROUP_ID        |   |             |
- * |     |     ECMP_MEMBER_ID       |   |             |
- * +-----+--------------------------+   |             |
- * | R9  |        UNUSED            |   |             |
- * +-----+--------------------------+   |             |
- * | R10 |        UNUSED            | X |             |
- * +-----+--------------------------+ X |             |
- * | R11 |        UNUSED            | R | IPv6-SRC-IP |
- * +-----+--------------------------+ E |   for NS    |
- * | R12 |        UNUSED            | G |             |
- * +-----+--------------------------+ 1 |             |
- * | R13 |        UNUSED            |   |             |
- * +-----+--------------------------+   |             |
- * | R14 |        UNUSED            |   |             |
- * +-----+--------------------------+   |             |
- * | R15 |        UNUSED            |   |             |
- * +-----+--------------------------+---+-------------+
+ * +-----+--------------------------+---+-----------------+---+-------------+
+ * | R0  | REGBIT_ND_RA_OPTS_RESULT |   |                 |   |             |
+ * |     |    IPv4-NEXT-HOP         |   |                 |   |             |
+ * |     |    (>= IP_INPUT)         |   |                 |   |             |
+ * +-----+--------------------------+ X | INPORT_ETH_ADDR |   |             |
+ * | R1  | IPv4-SRC-IP for ARP-REQ  | R |   (< IP_INPUT)  |   |             |
+ * |     |    (>= IP_INPUT)         | E |                 |   |             |
+ * +-----+--------------------------+ G |                 | X |             |
+ * | R2  |        UNUSED            | 0 |                 | X |             |
+ * +-----+--------------------------+   |                 | R |             |
+ * | R3  |        UNUSED            |   |                 | E |    IPv6     |
+ * +-----+--------------------------+---+-----------------+ G |  NEXT-HOP   |
+ * | R4  |        UNUSED            |   |                 | 0 |(>= IP_INPUT)|
+ * +-----+--------------------------+ X |                 |   |             |
+ * | R5  |        UNUSED            | R |                 |   |             |
+ * +-----+--------------------------+ E |     UNUSED      |   |             |
+ * | R6  |        UNUSED            | G |                 |   |             |
+ * +-----+--------------------------+ 1 |                 |   |             |
+ * | R7  |        UNUSED            |   |                 |   |             |
+ * +-----+--------------------------+---+-----------------+---+-------------+
+ * | R8  |     ECMP_GROUP_ID        |   |                 |   |             |
+ * |     |     ECMP_MEMBER_ID       |   |                 |   |             |
+ * +-----+--------------------------+ X |                 |   |             |
+ * | R9  |        UNUSED            | R |                 |   |             |
+ * +-----+--------------------------+ E |     UNUSED      |   |             |
+ * | R10 |        UNUSED            | G |                 | X |             |
+ * +-----+--------------------------+ 2 |                 | X |             |
+ * | R11 |        UNUSED            |   |                 | R | IPv6-SRC-IP |
+ * +-----+--------------------------+---+-----------------+ E |   for NS    |
+ * | R12 |        UNUSED            |   |                 | G |(>= IP_INPUT)|
+ * +-----+--------------------------+ X |                 | 1 |             |
+ * | R13 |        UNUSED            | R |                 |   |             |
+ * +-----+--------------------------+ E |     UNUSED      |   |             |
+ * | R14 |        UNUSED            | G |                 |   |             |
+ * +-----+--------------------------+ 3 |                 |   |             |
+ * | R15 |        UNUSED            |   |                 |   |             |
+ * +-----+--------------------------+---+-----------------+---+-------------+
  *
  */
 
@@ -8003,10 +8010,19 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             continue;
         }
 
+        /* Store the ethernet address of the port receiving the packet.
+         * This will save us from having to match on inport further down in
+         * the pipeline.
+         */
+        ds_clear(&actions);
+        ds_put_format(&actions, REG_INPORT_ETH_ADDR " = %s; next;",
+                      op->lrp_networks.ea_s);
+
         ds_clear(&match);
         ds_put_format(&match, "eth.mcast && inport == %s", op->json_key);
         ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_ADMISSION, 50,
-                                ds_cstr(&match), "next;", &op->nbrp->header_);
+                                ds_cstr(&match), ds_cstr(&actions),
+                                &op->nbrp->header_);
 
         ds_clear(&match);
         ds_put_format(&match, "eth.dst == %s && inport == %s",
@@ -8019,7 +8035,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                           op->od->l3redirect_port->json_key);
         }
         ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_ADMISSION, 50,
-                                ds_cstr(&match), "next;", &op->nbrp->header_);
+                                ds_cstr(&match),  ds_cstr(&actions),
+                                &op->nbrp->header_);
     }
 
     /* Logical router ingress table 1: LOOKUP_NEIGHBOR and
@@ -8286,17 +8303,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             ds_clear(&actions);
             ds_put_format(&actions,
                 "eth.dst = eth.src; "
-                "eth.src = %s; "
+                "eth.src = " REG_INPORT_ETH_ADDR "; "
                 "arp.op = 2; /* ARP reply */ "
                 "arp.tha = arp.sha; "
-                "arp.sha = %s; "
+                "arp.sha = " REG_INPORT_ETH_ADDR "; "
                 "arp.tpa = arp.spa; "
                 "arp.spa = %s; "
                 "outport = %s; "
                 "flags.loopback = 1; "
                 "output;",
-                op->lrp_networks.ea_s,
-                op->lrp_networks.ea_s,
                 op->lrp_networks.ipv4_addrs[i].addr_s,
                 op->json_key);
             ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
@@ -8323,17 +8338,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             ds_clear(&actions);
             ds_put_format(&actions,
                           "eth.dst = eth.src; "
-                          "eth.src = %s; "
+                          "eth.src = " REG_INPORT_ETH_ADDR "; "
                           "arp.op = 2; /* ARP reply */ "
                           "arp.tha = arp.sha; "
-                          "arp.sha = %s; "
+                          "arp.sha = " REG_INPORT_ETH_ADDR "; "
                           "arp.tpa = arp.spa; "
                           "arp.spa = %s; "
                           "outport = %s; "
                           "flags.loopback = 1; "
                           "output;",
-                          op->lrp_networks.ea_s,
-                          op->lrp_networks.ea_s,
                           ip_address,
                           op->json_key);
 
@@ -8354,18 +8367,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             ds_clear(&actions);
             ds_put_format(&actions,
                           "nd_na { "
-                          "eth.src = %s; "
+                          "eth.src = " REG_INPORT_ETH_ADDR "; "
                           "ip6.src = %s; "
                           "nd.target = %s; "
-                          "nd.tll = %s; "
+                          "nd.tll = " REG_INPORT_ETH_ADDR "; "
                           "outport = inport; "
                           "flags.loopback = 1; "
                           "output; "
                           "};",
-                          op->lrp_networks.ea_s,
                           ip_address,
-                          ip_address,
-                          op->lrp_networks.ea_s);
+                          ip_address);
 
             ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
                           ds_cstr(&match), ds_cstr(&actions));
@@ -8488,18 +8499,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                                   nat->logical_port);
                 } else {
                     if (is_v6) {
-                        ds_put_format(&actions,
-                            "eth.src = %s; "
-                            "nd.tll = %s; ",
-                            op->lrp_networks.ea_s,
-                            op->lrp_networks.ea_s);
+                        ds_put_cstr(&actions,
+                                    "eth.src = " REG_INPORT_ETH_ADDR "; "
+                                    "nd.tll = " REG_INPORT_ETH_ADDR "; ");
 
                     } else {
-                        ds_put_format(&actions,
-                            "eth.src = %s; "
-                            "arp.sha = %s; ",
-                            op->lrp_networks.ea_s,
-                            op->lrp_networks.ea_s);
+                        ds_put_cstr(&actions,
+                                    "eth.src = "REG_INPORT_ETH_ADDR "; "
+                                    "arp.sha = " REG_INPORT_ETH_ADDR "; ");
                     }
                     /* Traffic with eth.src = l3dgw_port->lrp_networks.ea_s
                      * should only be sent from the "redirect-chassis", so that
@@ -8513,17 +8520,13 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                 }
             } else {
                 if (is_v6) {
-                    ds_put_format(&actions,
-                        "eth.src = %s; "
-                        "nd.tll = %s; ",
-                        op->lrp_networks.ea_s,
-                        op->lrp_networks.ea_s);
+                    ds_put_cstr(&actions,
+                                "eth.src = " REG_INPORT_ETH_ADDR "; "
+                                "nd.tll = " REG_INPORT_ETH_ADDR "; ");
                 } else {
                     ds_put_format(&actions,
-                        "eth.src = %s; "
-                        "arp.sha = %s; ",
-                        op->lrp_networks.ea_s,
-                        op->lrp_networks.ea_s);
+                                  "eth.src = " REG_INPORT_ETH_ADDR "; "
+                                  "arp.sha = " REG_INPORT_ETH_ADDR "; ");
                 }
             }
             if (is_v6) {
@@ -8741,18 +8744,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             ds_clear(&actions);
             ds_put_format(&actions,
                           "nd_na_router { "
-                          "eth.src = %s; "
+                          "eth.src = " REG_INPORT_ETH_ADDR "; "
                           "ip6.src = %s; "
                           "nd.target = %s; "
-                          "nd.tll = %s; "
+                          "nd.tll = " REG_INPORT_ETH_ADDR "; "
                           "outport = inport; "
                           "flags.loopback = 1; "
                           "output; "
                           "};",
-                          op->lrp_networks.ea_s,
-                          op->lrp_networks.ipv6_addrs[i].addr_s,
                           op->lrp_networks.ipv6_addrs[i].addr_s,
-                          op->lrp_networks.ea_s);
+                          op->lrp_networks.ipv6_addrs[i].addr_s);
             ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
                                     ds_cstr(&match), ds_cstr(&actions),
                                     &op->nbrp->header_);
@@ -9254,6 +9255,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
              * on the l3dgw_port instance where nat->logical_port is
              * resident. */
             if (distributed) {
+                /* Store the ethernet address of the port receiving the packet.
+                 * This will save us from having to match on inport further
+                 * down in the pipeline.
+                 */
+                ds_clear(&actions);
+                ds_put_format(&actions, REG_INPORT_ETH_ADDR " = %s; next;",
+                              od->l3dgw_port->lrp_networks.ea_s);
+
                 ds_clear(&match);
                 ds_put_format(&match,
                               "eth.dst == "ETH_ADDR_FMT" && inport == %s"
@@ -9262,7 +9271,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                               od->l3dgw_port->json_key,
                               nat->logical_port);
                 ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_ADMISSION, 50,
-                                        ds_cstr(&match), "next;",
+                                        ds_cstr(&match), ds_cstr(&actions),
                                         &nat->header_);
             }
 
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index d7a940f..ef1ac04 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1551,3 +1551,143 @@ lsp2
 ])
 
 AT_CLEANUP
+
+AT_SETUP([ovn -- check router ARP/NS responder])
+ovn_start
+
+ovn-sbctl chassis-add ch geneve 127.0.0.1
+
+ovn-nbctl lr-add lr
+ovn-nbctl lrp-add lr lrp-public 00:00:00:00:01:00 43.43.43.1/24
+ovn-nbctl lrp-add lr lrp 00:00:00:00:00:01 42.42.42.1/24
+
+ovn-nbctl ls-add ls
+ovn-nbctl lsp-add ls ls-rp
+ovn-nbctl lsp-set-type ls-rp router
+ovn-nbctl lsp-set-addresses ls-rp router
+ovn-nbctl lsp-set-options ls-rp router-port=lrp
+ovn-nbctl lsp-add ls ls-vm
+
+ovn-nbctl set logical_router lr options:chassis=ch
+ovn-nbctl lr-nat-add lr dnat_and_snat 43.43.43.2 42.42.42.2
+ovn-nbctl lr-nat-add lr dnat 43.43.43.3 42.42.42.3
+ovn-nbctl lr-nat-add lr dnat_and_snat 43.43.43.4 42.42.42.4 ls-vm 00:00:00:00:00:02
+
+ovn-nbctl --wait=sb sync
+
+# Ingress router port ETH address is stored in lr_in_admission.
+AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_admission.*xreg0\[[0..47\]]" | sort], [0], [dnl
+  table=0 (lr_in_admission    ), priority=50   , dnl
+match=(eth.dst == 00:00:00:00:00:01 && inport == "lrp"), dnl
+action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;)
+  table=0 (lr_in_admission    ), priority=50   , dnl
+match=(eth.dst == 00:00:00:00:01:00 && inport == "lrp-public"), dnl
+action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
+  table=0 (lr_in_admission    ), priority=50   , dnl
+match=(eth.mcast && inport == "lrp"), dnl
+action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;)
+  table=0 (lr_in_admission    ), priority=50   , dnl
+match=(eth.mcast && inport == "lrp-public"), dnl
+action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
+])
+
+# Ingress router port ETH address is used for ARP reply/NA in lr_in_ip_input.
+AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp" && arp.spa == 42.42.42.0/24 && arp.tpa == 42.42.42.1 && arp.op == 1), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 42.42.42.1; outport = "lrp"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp" && arp.tpa == 43.43.43.2 && arp.op == 1), dnl
+action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = "lrp"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp" && arp.tpa == 43.43.43.3 && arp.op == 1), dnl
+action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = "lrp"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp" && arp.tpa == 43.43.43.4 && arp.op == 1), dnl
+action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = "lrp"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp" && nd_ns && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd.target == fe80::200:ff:fe00:1), dnl
+action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:1; nd.target = fe80::200:ff:fe00:1; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp-public" && arp.spa == 43.43.43.0/24 && arp.tpa == 43.43.43.1 && arp.op == 1), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.1; outport = "lrp-public"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp-public" && arp.tpa == 43.43.43.2 && arp.op == 1), dnl
+action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = "lrp-public"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp-public" && arp.tpa == 43.43.43.3 && arp.op == 1), dnl
+action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = "lrp-public"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp-public" && arp.tpa == 43.43.43.4 && arp.op == 1), dnl
+action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = "lrp-public"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp-public" && nd_ns && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd.target == fe80::200:ff:fe00:100), dnl
+action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:100; nd.target = fe80::200:ff:fe00:100; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
+])
+
+# xreg0[0..47] isn't used anywhere else.
+AT_CHECK([ovn-sbctl lflow-list | grep "xreg0\[[0..47\]]" | grep -vE 'lr_in_admission|lr_in_ip_input'], [1], [])
+
+# Test chassis redirect port.
+ovn-nbctl remove logical_router lr options chassis
+ovn-nbctl lrp-set-gateway-chassis lrp-public ch
+ovn-nbctl --wait=sb sync
+
+# Ingress router port ETH address is stored in lr_in_admission.
+AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_admission.*xreg0\[[0..47\]]" | sort], [0], [dnl
+  table=0 (lr_in_admission    ), priority=50   , dnl
+match=(eth.dst == 00:00:00:00:00:01 && inport == "lrp"), dnl
+action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;)
+  table=0 (lr_in_admission    ), priority=50   , dnl
+match=(eth.dst == 00:00:00:00:00:02 && inport == "lrp-public" && is_chassis_resident("ls-vm")), dnl
+action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
+  table=0 (lr_in_admission    ), priority=50   , dnl
+match=(eth.dst == 00:00:00:00:01:00 && inport == "lrp-public" && is_chassis_resident("cr-lrp-public")), dnl
+action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
+  table=0 (lr_in_admission    ), priority=50   , dnl
+match=(eth.mcast && inport == "lrp"), dnl
+action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;)
+  table=0 (lr_in_admission    ), priority=50   , dnl
+match=(eth.mcast && inport == "lrp-public"), dnl
+action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
+])
+
+# Ingress router port is used for ARP reply/NA in lr_in_ip_input.
+# xxreg0[0..47] is used unless external_mac is set.
+AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp" && arp.spa == 42.42.42.0/24 && arp.tpa == 42.42.42.1 && arp.op == 1), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 42.42.42.1; outport = "lrp"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp" && arp.tpa == 43.43.43.2 && arp.op == 1), dnl
+action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = "lrp"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp" && arp.tpa == 43.43.43.3 && arp.op == 1), dnl
+action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = "lrp"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp" && arp.tpa == 43.43.43.4 && arp.op == 1), dnl
+action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = "lrp"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp" && nd_ns && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd.target == fe80::200:ff:fe00:1), dnl
+action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:1; nd.target = fe80::200:ff:fe00:1; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp-public" && arp.spa == 43.43.43.0/24 && arp.tpa == 43.43.43.1 && arp.op == 1), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.1; outport = "lrp-public"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp-public" && arp.tpa == 43.43.43.2 && arp.op == 1 && is_chassis_resident("cr-lrp-public")), dnl
+action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = "lrp-public"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp-public" && arp.tpa == 43.43.43.3 && arp.op == 1 && is_chassis_resident("cr-lrp-public")), dnl
+action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = "lrp-public"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp-public" && arp.tpa == 43.43.43.4 && arp.op == 1 && is_chassis_resident("ls-vm")), dnl
+action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = 00:00:00:00:00:02; arp.sha = 00:00:00:00:00:02; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = "lrp-public"; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp-public" && nd_ns && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd.target == fe80::200:ff:fe00:100 && is_chassis_resident("cr-lrp-public")), dnl
+action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:100; nd.target = fe80::200:ff:fe00:100; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
+])
+
+# xreg0[0..47] isn't used anywhere else.
+AT_CHECK([ovn-sbctl lflow-list | grep "xreg0\[[0..47\]]" | grep -vE 'lr_in_admission|lr_in_ip_input'], [1], [])
+
+AT_CLEANUP

From patchwork Tue Jun 30 21:41:53 2020
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Dumitru Ceara 
X-Patchwork-Id: 1320137
Return-Path: 
X-Original-To: incoming@patchwork.ozlabs.org
Delivered-To: patchwork-incoming@bilbo.ozlabs.org
Authentication-Results: ozlabs.org;
 spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org
 (client-ip=140.211.166.138; helo=whitealder.osuosl.org;
 envelope-from=ovs-dev-bounces@openvswitch.org; receiver=)
Authentication-Results: ozlabs.org;
 dmarc=fail (p=none dis=none) header.from=redhat.com
Authentication-Results: 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=iOoKSyOb;
	dkim-atps=neutral
Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by ozlabs.org (Postfix) with ESMTPS id 49xHsf1WdVz9sPF
	for ; Wed,  1 Jul 2020 07:43:18 +1000 (AEST)
Received: from localhost (localhost [127.0.0.1])
	by whitealder.osuosl.org (Postfix) with ESMTP id 985A98829F;
	Tue, 30 Jun 2020 21:43:16 +0000 (UTC)
X-Virus-Scanned: amavisd-new at osuosl.org
Received: from whitealder.osuosl.org ([127.0.0.1])
	by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id buuNpdCslcUZ; Tue, 30 Jun 2020 21:43:03 +0000 (UTC)
Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56])
	by whitealder.osuosl.org (Postfix) with ESMTP id 70DCA88257;
	Tue, 30 Jun 2020 21:43:00 +0000 (UTC)
Received: from lf-lists.osuosl.org (localhost [127.0.0.1])
	by lists.linuxfoundation.org (Postfix) with ESMTP id 52B18C088F;
	Tue, 30 Jun 2020 21:43:00 +0000 (UTC)
X-Original-To: dev@openvswitch.org
Delivered-To: ovs-dev@lists.linuxfoundation.org
Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136])
 by lists.linuxfoundation.org (Postfix) with ESMTP id DB27DC0865
 for ; Tue, 30 Jun 2020 21:42:58 +0000 (UTC)
Received: from localhost (localhost [127.0.0.1])
 by silver.osuosl.org (Postfix) with ESMTP id B3B20231B1
 for ; Tue, 30 Jun 2020 21:42:58 +0000 (UTC)
X-Virus-Scanned: amavisd-new at osuosl.org
Received: from silver.osuosl.org ([127.0.0.1])
 by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)
 with ESMTP id 4xALxUygmaoz for ;
 Tue, 30 Jun 2020 21:42:29 +0000 (UTC)
X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6
Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com
 [205.139.110.120])
 by silver.osuosl.org (Postfix) with ESMTPS id 7EC8E2324E
 for ; Tue, 30 Jun 2020 21:42:25 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1593553344;
 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=qFIdaw3v9CqMhz1j2DZf49O+0PkiFDlQp7AtR5KCHyg=;
 b=iOoKSyOb7C39Ux3gUm1LcDsTnNjEy5URfDMYL8eH74mxv2HdqrubXjmbx7kAzNJsd2Wrbh
 nrqp1Pz3aw9Eru7RDpWZxEmqpbtjTIuMrf5QR5YZ3pCPSiLSePWDlw7FOko/tC7Ip2QeH7
 OrSrlQjiUavOTe8gXaof8zDoIOoXHk0=
Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com
 [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id
 us-mta-204-1Qa1fqEfMxWj1jsoo46BGA-1; Tue, 30 Jun 2020 17:41:57 -0400
X-MC-Unique: 1Qa1fqEfMxWj1jsoo46BGA-1
Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com
 [10.5.11.16])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6BD4C800138;
 Tue, 30 Jun 2020 21:41:56 +0000 (UTC)
Received: from dceara.remote.csb (ovpn-112-145.ams2.redhat.com
 [10.36.112.145])
 by smtp.corp.redhat.com (Postfix) with ESMTP id 509535C1D3;
 Tue, 30 Jun 2020 21:41:55 +0000 (UTC)
From: Dumitru Ceara 
To: dev@openvswitch.org
Date: Tue, 30 Jun 2020 23:41:53 +0200
Message-Id: <20200630214150.26800.98213.stgit@dceara.remote.csb>
In-Reply-To: <20200630214107.26800.21305.stgit@dceara.remote.csb>
References: <20200630214107.26800.21305.stgit@dceara.remote.csb>
User-Agent: StGit/0.17.1-dirty
MIME-Version: 1.0
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16
Authentication-Results: relay.mimecast.com;
 auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Cc: hzhou@ovn.org
Subject: [ovs-dev] [PATCH v2 ovn 3/5] ovn-northd: Refactor ARP/NS responder
	in router pipeline.
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" 

Add functions to build the ARP/NS responder flows for table
S_ROUTER_IN_IP_INPUT and use them in all places where responder
flows are created.

Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.8.xml |    8 +
 northd/ovn-northd.c     |  314 +++++++++++++++++++++--------------------------
 tests/ovn-northd.at     |   72 +++++------
 3 files changed, 181 insertions(+), 213 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 78e2a71..84224ff 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1778,7 +1778,7 @@ arp.tha = arp.sha;
 arp.sha = xreg0[0..47];
 arp.tpa = arp.spa;
 arp.spa = A;
-outport = P;
+outport = inport;
 flags.loopback = 1;
 output;
         

@@ -1870,7 +1870,7 @@ arp.tha = arp.sha;
 arp.sha = xreg0[0..47];
 arp.tpa = arp.spa;
 arp.spa = A;
-outport = P;
+outport = inport;
 flags.loopback = 1;
 output;
         

@@ -1900,7 +1900,7 @@ nd_na {
     nd.tll = xreg0[0..47];
     ip6.src = A;
     nd.target = A;
-    outport = P;
+    outport = inport;
     flags.loopback = 1;
     output;
 }
@@ -2570,7 +2570,7 @@ reg8[0..15] = 0;
 xxreg0 = G;
 xxreg1 = A;
 eth.src = E;
-outport = P;
+outport = inport;
 flags.loopback = 1;
 next;
         

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 9277040..fde9198 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -7967,6 +7967,105 @@ lrouter_nat_is_stateless(const struct nbrec_nat *nat)
     return false;
 }
 
+/* Builds the logical flow that replies to ARP requests for an 'ip_address'
+ * owned by the router. The flow is inserted in table S_ROUTER_IN_IP_INPUT
+ * with the given priority.
+ */
+static void
+build_lrouter_arp_flow(struct ovn_datapath *od, struct ovn_port *op,
+                       const char *ip_address, const char *eth_addr,
+                       struct ds *extra_match, uint16_t priority,
+                       struct hmap *lflows, const struct ovsdb_idl_row *hint)
+{
+    struct ds match = DS_EMPTY_INITIALIZER;
+    struct ds actions = DS_EMPTY_INITIALIZER;
+
+    if (op) {
+        ds_put_format(&match, "inport == %s && ", op->json_key);
+    }
+
+    ds_put_format(&match, "arp.op == 1 && arp.tpa == %s", ip_address);
+
+    if (extra_match && ds_last(extra_match) != EOF) {
+        ds_put_format(&match, " && %s", ds_cstr(extra_match));
+    }
+    ds_put_format(&actions,
+                  "eth.dst = eth.src; "
+                  "eth.src = %s; "
+                  "arp.op = 2; /* ARP reply */ "
+                  "arp.tha = arp.sha; "
+                  "arp.sha = %s; "
+                  "arp.tpa = arp.spa; "
+                  "arp.spa = %s; "
+                  "outport = inport; "
+                  "flags.loopback = 1; "
+                  "output;",
+                  eth_addr,
+                  eth_addr,
+                  ip_address);
+
+    ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
+                            ds_cstr(&match), ds_cstr(&actions), hint);
+
+    ds_destroy(&match);
+    ds_destroy(&actions);
+}
+
+/* Builds the logical flow that replies to NS requests for an 'ip_address'
+ * owned by the router. The flow is inserted in table S_ROUTER_IN_IP_INPUT
+ * with the given priority. If 'sn_ip_address' is non-NULL, requests are
+ * restricted only to packets with IP destination 'ip_address' or
+ * 'sn_ip_address'.
+ */
+static void
+build_lrouter_nd_flow(struct ovn_datapath *od, struct ovn_port *op,
+                      const char *action, const char *ip_address,
+                      const char *sn_ip_address, const char *eth_addr,
+                      struct ds *extra_match, uint16_t priority,
+                      struct hmap *lflows,
+                      const struct ovsdb_idl_row *hint)
+{
+    struct ds match = DS_EMPTY_INITIALIZER;
+    struct ds actions = DS_EMPTY_INITIALIZER;
+
+    if (op) {
+        ds_put_format(&match, "inport == %s && ", op->json_key);
+    }
+
+    if (sn_ip_address) {
+        ds_put_format(&match, "ip6.dst == {%s, %s} && ",
+                      ip_address, sn_ip_address);
+    }
+
+    ds_put_format(&match, "nd_ns && nd.target == %s", ip_address);
+
+    if (extra_match && ds_last(extra_match) != EOF) {
+        ds_put_format(&match, " && %s", ds_cstr(extra_match));
+    }
+
+    ds_put_format(&actions,
+                  "%s { "
+                    "eth.src = %s; "
+                    "ip6.src = %s; "
+                    "nd.target = %s; "
+                    "nd.tll = %s; "
+                    "outport = inport; "
+                    "flags.loopback = 1; "
+                    "output; "
+                  "};",
+                  action,
+                  eth_addr,
+                  ip_address,
+                  ip_address,
+                  eth_addr);
+
+    ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
+                            ds_cstr(&match), ds_cstr(&actions), hint);
+
+    ds_destroy(&match);
+    ds_destroy(&actions);
+}
+
 static void
 build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                     struct hmap *lflows, struct shash *meter_groups,
@@ -8262,13 +8361,9 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
          * IP address. */
         for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
             ds_clear(&match);
-            ds_put_format(&match,
-                          "inport == %s && arp.spa == %s/%u && arp.tpa == %s"
-                          " && arp.op == 1",
-                          op->json_key,
+            ds_put_format(&match, "arp.spa == %s/%u",
                           op->lrp_networks.ipv4_addrs[i].network_s,
-                          op->lrp_networks.ipv4_addrs[i].plen,
-                          op->lrp_networks.ipv4_addrs[i].addr_s);
+                          op->lrp_networks.ipv4_addrs[i].plen);
 
             if (op->od->l3dgw_port && op->od->l3redirect_port && op->peer
                 && op->peer->od->n_localnet_ports) {
@@ -8300,23 +8395,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                 }
             }
 
-            ds_clear(&actions);
-            ds_put_format(&actions,
-                "eth.dst = eth.src; "
-                "eth.src = " REG_INPORT_ETH_ADDR "; "
-                "arp.op = 2; /* ARP reply */ "
-                "arp.tha = arp.sha; "
-                "arp.sha = " REG_INPORT_ETH_ADDR "; "
-                "arp.tpa = arp.spa; "
-                "arp.spa = %s; "
-                "outport = %s; "
-                "flags.loopback = 1; "
-                "output;",
-                op->lrp_networks.ipv4_addrs[i].addr_s,
-                op->json_key);
-            ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
-                                    ds_cstr(&match), ds_cstr(&actions),
-                                    &op->nbrp->header_);
+            build_lrouter_arp_flow(op->od, op,
+                                   op->lrp_networks.ipv4_addrs[i].addr_s,
+                                   REG_INPORT_ETH_ADDR, &match, 90, lflows,
+                                   &op->nbrp->header_);
         }
 
         /* A set to hold all load-balancer vips that need ARP responses. */
@@ -8327,59 +8409,26 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
         const char *ip_address;
         SSET_FOR_EACH (ip_address, &all_ips_v4) {
             ds_clear(&match);
-            ds_put_format(&match,
-                          "inport == %s && arp.tpa == %s && arp.op == 1",
-                          op->json_key, ip_address);
-
             if (op == op->od->l3dgw_port) {
-                ds_put_format(&match, " && is_chassis_resident(%s)",
+                ds_put_format(&match, "is_chassis_resident(%s)",
                               op->od->l3redirect_port->json_key);
             }
-            ds_clear(&actions);
-            ds_put_format(&actions,
-                          "eth.dst = eth.src; "
-                          "eth.src = " REG_INPORT_ETH_ADDR "; "
-                          "arp.op = 2; /* ARP reply */ "
-                          "arp.tha = arp.sha; "
-                          "arp.sha = " REG_INPORT_ETH_ADDR "; "
-                          "arp.tpa = arp.spa; "
-                          "arp.spa = %s; "
-                          "outport = %s; "
-                          "flags.loopback = 1; "
-                          "output;",
-                          ip_address,
-                          op->json_key);
 
-            ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
-                          ds_cstr(&match), ds_cstr(&actions));
+            build_lrouter_arp_flow(op->od, op,
+                                   ip_address, REG_INPORT_ETH_ADDR,
+                                   &match, 90, lflows, NULL);
         }
 
         SSET_FOR_EACH (ip_address, &all_ips_v6) {
             ds_clear(&match);
-            ds_put_format(&match,
-                          "inport == %s && nd_ns && nd.target == %s",
-                          op->json_key, ip_address);
-
             if (op == op->od->l3dgw_port) {
-                ds_put_format(&match, " && is_chassis_resident(%s)",
+                ds_put_format(&match, "is_chassis_resident(%s)",
                               op->od->l3redirect_port->json_key);
             }
-            ds_clear(&actions);
-            ds_put_format(&actions,
-                          "nd_na { "
-                          "eth.src = " REG_INPORT_ETH_ADDR "; "
-                          "ip6.src = %s; "
-                          "nd.target = %s; "
-                          "nd.tll = " REG_INPORT_ETH_ADDR "; "
-                          "outport = inport; "
-                          "flags.loopback = 1; "
-                          "output; "
-                          "};",
-                          ip_address,
-                          ip_address);
 
-            ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
-                          ds_cstr(&match), ds_cstr(&actions));
+            build_lrouter_nd_flow(op->od, op, "nd_na",
+                                  ip_address, NULL, REG_INPORT_ETH_ADDR,
+                                  &match, 90, lflows, NULL);
         }
 
         sset_destroy(&all_ips_v4);
@@ -8435,123 +8484,60 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                 continue;
             }
 
+            /* Mac address to use when replying to ARP/NS. */
+            const char *mac_s = REG_INPORT_ETH_ADDR;
+
             /* ARP / ND handling for external IP addresses.
              *
              * DNAT IP addresses are external IP addresses that need ARP
              * handling. */
-            char addr_s[INET6_ADDRSTRLEN + 1];
             ds_clear(&match);
-            ds_clear(&actions);
-            if (is_v6) {
-                /* For ND solicitations, we need to listen for both the
-                 * unicast IPv6 address and its all-nodes multicast address,
-                 * but always respond with the unicast IPv6 address. */
-                char sn_addr_s[INET6_ADDRSTRLEN + 1];
-                struct in6_addr sn_addr;
-                in6_addr_solicited_node(&sn_addr, &ipv6);
-                ipv6_string_mapped(sn_addr_s, &sn_addr);
-                ipv6_string_mapped(addr_s, &ipv6);
-
-                ds_put_format(&match, "inport == %s && "
-                        "nd_ns && ip6.dst == {%s, %s} && nd.target == %s",
-                        op->json_key, addr_s, sn_addr_s, addr_s);
 
-                ds_put_format(&actions,
-                    "eth.dst = eth.src; "
-                    "nd_na { ");
-            } else {
-                ds_put_format(&match,
-                              "inport == %s "
-                              "&& arp.tpa == "IP_FMT" && arp.op == 1",
-                              op->json_key, IP_ARGS(ip));
-
-                ds_put_format(&actions,
-                    "eth.dst = eth.src; "
-                    "arp.op = 2; /* ARP reply */ "
-                    "arp.tha = arp.sha; ");
-            }
             if (op->od->l3dgw_port && op == op->od->l3dgw_port) {
                 struct eth_addr mac;
                 if (nat->external_mac &&
                     eth_addr_from_string(nat->external_mac, &mac)
                     && nat->logical_port) {
                     /* distributed NAT case, use nat->external_mac */
-                    if (is_v6) {
-                        ds_put_format(&actions,
-                            "eth.src = "ETH_ADDR_FMT"; "
-                            "nd.tll = "ETH_ADDR_FMT"; ",
-                            ETH_ADDR_ARGS(mac),
-                            ETH_ADDR_ARGS(mac));
-
-                    } else {
-                        ds_put_format(&actions,
-                            "eth.src = "ETH_ADDR_FMT"; "
-                            "arp.sha = "ETH_ADDR_FMT"; ",
-                            ETH_ADDR_ARGS(mac),
-                            ETH_ADDR_ARGS(mac));
-                    }
+                    mac_s = nat->external_mac;
                     /* Traffic with eth.src = nat->external_mac should only be
                      * sent from the chassis where nat->logical_port is
                      * resident, so that upstream MAC learning points to the
                      * correct chassis.  Also need to avoid generation of
                      * multiple ARP responses from different chassis. */
-                    ds_put_format(&match, " && is_chassis_resident(\"%s\")",
+                    ds_put_format(&match, "is_chassis_resident(\"%s\")",
                                   nat->logical_port);
                 } else {
-                    if (is_v6) {
-                        ds_put_cstr(&actions,
-                                    "eth.src = " REG_INPORT_ETH_ADDR "; "
-                                    "nd.tll = " REG_INPORT_ETH_ADDR "; ");
-
-                    } else {
-                        ds_put_cstr(&actions,
-                                    "eth.src = "REG_INPORT_ETH_ADDR "; "
-                                    "arp.sha = " REG_INPORT_ETH_ADDR "; ");
-                    }
+                    mac_s = REG_INPORT_ETH_ADDR;
                     /* Traffic with eth.src = l3dgw_port->lrp_networks.ea_s
                      * should only be sent from the "redirect-chassis", so that
                      * upstream MAC learning points to the "redirect-chassis".
                      * Also need to avoid generation of multiple ARP responses
                      * from different chassis. */
                     if (op->od->l3redirect_port) {
-                        ds_put_format(&match, " && is_chassis_resident(%s)",
+                        ds_put_format(&match, "is_chassis_resident(%s)",
                                       op->od->l3redirect_port->json_key);
                     }
                 }
-            } else {
-                if (is_v6) {
-                    ds_put_cstr(&actions,
-                                "eth.src = " REG_INPORT_ETH_ADDR "; "
-                                "nd.tll = " REG_INPORT_ETH_ADDR "; ");
-                } else {
-                    ds_put_format(&actions,
-                                  "eth.src = " REG_INPORT_ETH_ADDR "; "
-                                  "arp.sha = " REG_INPORT_ETH_ADDR "; ");
-                }
             }
             if (is_v6) {
-                ds_put_format(&actions,
-                    "ip6.src = %s; "
-                    "nd.target = %s; "
-                    "outport = %s; "
-                    "flags.loopback = 1; "
-                    "output; "
-                    "};",
-                    addr_s, addr_s, op->json_key);
+                /* For ND solicitations, we need to listen for both the
+                 * unicast IPv6 address and its all-nodes multicast address,
+                 * but always respond with the unicast IPv6 address. */
+                char sn_addr_s[INET6_ADDRSTRLEN + 1];
+                struct in6_addr sn_addr;
+                in6_addr_solicited_node(&sn_addr, &ipv6);
+                ipv6_string_mapped(sn_addr_s, &sn_addr);
+
+                build_lrouter_nd_flow(op->od, op, "nd_na",
+                                       nat->external_ip, sn_addr_s,
+                                       mac_s, &match, 90,
+                                       lflows, &nat->header_);
             } else {
-                ds_put_format(&actions,
-                    "arp.tpa = arp.spa; "
-                    "arp.spa = "IP_FMT"; "
-                    "outport = %s; "
-                    "flags.loopback = 1; "
-                    "output;",
-                    IP_ARGS(ip),
-                    op->json_key);
+                build_lrouter_arp_flow(op->od, op,
+                                       nat->external_ip, mac_s, &match, 90,
+                                       lflows, &nat->header_);
             }
-
-            ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
-                                    ds_cstr(&match), ds_cstr(&actions),
-                                    &nat->header_);
         }
 
         if (!smap_get(&op->od->nbr->options, "chassis")
@@ -8723,13 +8709,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
          * router's own IP address. */
         for (int i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
             ds_clear(&match);
-            ds_put_format(&match,
-                    "inport == %s && nd_ns && ip6.dst == {%s, %s} "
-                    "&& nd.target == %s",
-                    op->json_key,
-                    op->lrp_networks.ipv6_addrs[i].addr_s,
-                    op->lrp_networks.ipv6_addrs[i].sn_addr_s,
-                    op->lrp_networks.ipv6_addrs[i].addr_s);
             if (op->od->l3dgw_port && op == op->od->l3dgw_port
                 && op->od->l3redirect_port) {
                 /* Traffic with eth.src = l3dgw_port->lrp_networks.ea_s
@@ -8737,26 +8716,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                  * upstream MAC learning points to the "redirect-chassis".
                  * Also need to avoid generation of multiple ND replies
                  * from different chassis. */
-                ds_put_format(&match, " && is_chassis_resident(%s)",
+                ds_put_format(&match, "is_chassis_resident(%s)",
                               op->od->l3redirect_port->json_key);
             }
 
-            ds_clear(&actions);
-            ds_put_format(&actions,
-                          "nd_na_router { "
-                          "eth.src = " REG_INPORT_ETH_ADDR "; "
-                          "ip6.src = %s; "
-                          "nd.target = %s; "
-                          "nd.tll = " REG_INPORT_ETH_ADDR "; "
-                          "outport = inport; "
-                          "flags.loopback = 1; "
-                          "output; "
-                          "};",
-                          op->lrp_networks.ipv6_addrs[i].addr_s,
-                          op->lrp_networks.ipv6_addrs[i].addr_s);
-            ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
-                                    ds_cstr(&match), ds_cstr(&actions),
-                                    &op->nbrp->header_);
+            build_lrouter_nd_flow(op->od, op, "nd_na_router",
+                                  op->lrp_networks.ipv6_addrs[i].addr_s,
+                                  op->lrp_networks.ipv6_addrs[i].sn_addr_s,
+                                  REG_INPORT_ETH_ADDR, &match, 90, lflows,
+                                  &op->nbrp->header_);
         }
 
         /* UDP/TCP port unreachable */
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index ef1ac04..a3fb7ef 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1594,34 +1594,34 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
 # Ingress router port ETH address is used for ARP reply/NA in lr_in_ip_input.
 AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.spa == 42.42.42.0/24 && arp.tpa == 42.42.42.1 && arp.op == 1), dnl
-action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 42.42.42.1; outport = "lrp"; flags.loopback = 1; output;)
+match=(inport == "lrp" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.0/24), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 42.42.42.1; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.tpa == 43.43.43.2 && arp.op == 1), dnl
-action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = "lrp"; flags.loopback = 1; output;)
+match=(inport == "lrp" && arp.op == 1 && arp.tpa == 43.43.43.2), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.tpa == 43.43.43.3 && arp.op == 1), dnl
-action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = "lrp"; flags.loopback = 1; output;)
+match=(inport == "lrp" && arp.op == 1 && arp.tpa == 43.43.43.3), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.tpa == 43.43.43.4 && arp.op == 1), dnl
-action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = "lrp"; flags.loopback = 1; output;)
+match=(inport == "lrp" && arp.op == 1 && arp.tpa == 43.43.43.4), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && nd_ns && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd.target == fe80::200:ff:fe00:1), dnl
+match=(inport == "lrp" && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd_ns && nd.target == fe80::200:ff:fe00:1), dnl
 action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:1; nd.target = fe80::200:ff:fe00:1; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.spa == 43.43.43.0/24 && arp.tpa == 43.43.43.1 && arp.op == 1), dnl
-action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.1; outport = "lrp-public"; flags.loopback = 1; output;)
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.1 && arp.spa == 43.43.43.0/24), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.1; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.tpa == 43.43.43.2 && arp.op == 1), dnl
-action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = "lrp-public"; flags.loopback = 1; output;)
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.tpa == 43.43.43.3 && arp.op == 1), dnl
-action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = "lrp-public"; flags.loopback = 1; output;)
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.tpa == 43.43.43.4 && arp.op == 1), dnl
-action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = "lrp-public"; flags.loopback = 1; output;)
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.4), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && nd_ns && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd.target == fe80::200:ff:fe00:100), dnl
+match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd_ns && nd.target == fe80::200:ff:fe00:100), dnl
 action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:100; nd.target = fe80::200:ff:fe00:100; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
 ])
 
@@ -1656,34 +1656,34 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
 # xxreg0[0..47] is used unless external_mac is set.
 AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.spa == 42.42.42.0/24 && arp.tpa == 42.42.42.1 && arp.op == 1), dnl
-action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 42.42.42.1; outport = "lrp"; flags.loopback = 1; output;)
+match=(inport == "lrp" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.0/24), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 42.42.42.1; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.tpa == 43.43.43.2 && arp.op == 1), dnl
-action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = "lrp"; flags.loopback = 1; output;)
+match=(inport == "lrp" && arp.op == 1 && arp.tpa == 43.43.43.2), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.tpa == 43.43.43.3 && arp.op == 1), dnl
-action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = "lrp"; flags.loopback = 1; output;)
+match=(inport == "lrp" && arp.op == 1 && arp.tpa == 43.43.43.3), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.tpa == 43.43.43.4 && arp.op == 1), dnl
-action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = "lrp"; flags.loopback = 1; output;)
+match=(inport == "lrp" && arp.op == 1 && arp.tpa == 43.43.43.4), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && nd_ns && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd.target == fe80::200:ff:fe00:1), dnl
+match=(inport == "lrp" && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd_ns && nd.target == fe80::200:ff:fe00:1), dnl
 action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:1; nd.target = fe80::200:ff:fe00:1; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.spa == 43.43.43.0/24 && arp.tpa == 43.43.43.1 && arp.op == 1), dnl
-action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.1; outport = "lrp-public"; flags.loopback = 1; output;)
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.1 && arp.spa == 43.43.43.0/24), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.1; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.tpa == 43.43.43.2 && arp.op == 1 && is_chassis_resident("cr-lrp-public")), dnl
-action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = "lrp-public"; flags.loopback = 1; output;)
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2 && is_chassis_resident("cr-lrp-public")), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.tpa == 43.43.43.3 && arp.op == 1 && is_chassis_resident("cr-lrp-public")), dnl
-action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = xreg0[[0..47]]; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = "lrp-public"; flags.loopback = 1; output;)
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3 && is_chassis_resident("cr-lrp-public")), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.tpa == 43.43.43.4 && arp.op == 1 && is_chassis_resident("ls-vm")), dnl
-action=(eth.dst = eth.src; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; eth.src = 00:00:00:00:00:02; arp.sha = 00:00:00:00:00:02; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = "lrp-public"; flags.loopback = 1; output;)
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.4 && is_chassis_resident("ls-vm")), dnl
+action=(eth.dst = eth.src; eth.src = 00:00:00:00:00:02; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 00:00:00:00:00:02; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && nd_ns && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd.target == fe80::200:ff:fe00:100 && is_chassis_resident("cr-lrp-public")), dnl
+match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd_ns && nd.target == fe80::200:ff:fe00:100 && is_chassis_resident("cr-lrp-public")), dnl
 action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:100; nd.target = fe80::200:ff:fe00:100; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
 ])
 

From patchwork Tue Jun 30 21:42:03 2020
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Dumitru Ceara 
X-Patchwork-Id: 1320134
Return-Path: 
X-Original-To: incoming@patchwork.ozlabs.org
Delivered-To: patchwork-incoming@bilbo.ozlabs.org
Authentication-Results: ozlabs.org;
 spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org
 (client-ip=140.211.166.138; helo=whitealder.osuosl.org;
 envelope-from=ovs-dev-bounces@openvswitch.org; receiver=)
Authentication-Results: ozlabs.org;
 dmarc=fail (p=none dis=none) header.from=redhat.com
Authentication-Results: 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=ZiGJHrla;
	dkim-atps=neutral
Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by ozlabs.org (Postfix) with ESMTPS id 49xHrT0q2zz9sPF
	for ; Wed,  1 Jul 2020 07:42:17 +1000 (AEST)
Received: from localhost (localhost [127.0.0.1])
	by whitealder.osuosl.org (Postfix) with ESMTP id 9268188211;
	Tue, 30 Jun 2020 21:42:15 +0000 (UTC)
X-Virus-Scanned: amavisd-new at osuosl.org
Received: from whitealder.osuosl.org ([127.0.0.1])
	by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id TL0A49AudlgA; Tue, 30 Jun 2020 21:42:14 +0000 (UTC)
Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56])
	by whitealder.osuosl.org (Postfix) with ESMTP id 5961787742;
	Tue, 30 Jun 2020 21:42:14 +0000 (UTC)
Received: from lf-lists.osuosl.org (localhost [127.0.0.1])
	by lists.linuxfoundation.org (Postfix) with ESMTP id 4D0CAC0865;
	Tue, 30 Jun 2020 21:42:14 +0000 (UTC)
X-Original-To: dev@openvswitch.org
Delivered-To: ovs-dev@lists.linuxfoundation.org
Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137])
 by lists.linuxfoundation.org (Postfix) with ESMTP id 9E25BC016E
 for ; Tue, 30 Jun 2020 21:42:12 +0000 (UTC)
Received: from localhost (localhost [127.0.0.1])
 by fraxinus.osuosl.org (Postfix) with ESMTP id 8B97E864EA
 for ; Tue, 30 Jun 2020 21:42:12 +0000 (UTC)
X-Virus-Scanned: amavisd-new at osuosl.org
Received: from fraxinus.osuosl.org ([127.0.0.1])
 by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)
 with ESMTP id b1vM7cY-ae7Z for ;
 Tue, 30 Jun 2020 21:42:11 +0000 (UTC)
X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6
Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com
 [207.211.31.81])
 by fraxinus.osuosl.org (Postfix) with ESMTPS id BAA2986216
 for ; Tue, 30 Jun 2020 21:42:11 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1593553330;
 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=ndEO67jowHGTAHAMQPXn0uOV8kYXkiJxX2fh3PdFqGM=;
 b=ZiGJHrlaY51dUuNSJVXdM5c90bsF0XybvkS9gVXALU5H+47vWkOLNqCl90jC4usX7iMrgf
 uJJ2lXDLM1sD2hcp4gEovBEM7Rv5EJsnM0/V2+OTzTzwkt4XFsQ+fva68+gzBnkDKh7IJf
 1nUZjxlUcyFN1JaR+R8aQ9wtL4iwA60=
Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com
 [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id
 us-mta-465-q8d_gZe-MpGrDB7C6S5Xeg-1; Tue, 30 Jun 2020 17:42:07 -0400
X-MC-Unique: q8d_gZe-MpGrDB7C6S5Xeg-1
Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com
 [10.5.11.15])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A0B16107ACCD;
 Tue, 30 Jun 2020 21:42:06 +0000 (UTC)
Received: from dceara.remote.csb (ovpn-112-145.ams2.redhat.com
 [10.36.112.145])
 by smtp.corp.redhat.com (Postfix) with ESMTP id B408D9CFF1;
 Tue, 30 Jun 2020 21:42:05 +0000 (UTC)
From: Dumitru Ceara 
To: dev@openvswitch.org
Date: Tue, 30 Jun 2020 23:42:03 +0200
Message-Id: <20200630214201.26800.53920.stgit@dceara.remote.csb>
In-Reply-To: <20200630214107.26800.21305.stgit@dceara.remote.csb>
References: <20200630214107.26800.21305.stgit@dceara.remote.csb>
User-Agent: StGit/0.17.1-dirty
MIME-Version: 1.0
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15
Authentication-Results: relay.mimecast.com;
 auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Cc: hzhou@ovn.org
Subject: [ovs-dev] [PATCH v2 ovn 4/5] ovn-northd: Refactor NAT address
	parsing.
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" 

Store NAT entries pointers in ovn_datapath and pre-parse the external IP
addresses. This simplifies the code and makes it easier to reuse the parsed
external IP and solicited-node address without reparsing.

Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.c |  115 ++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 85 insertions(+), 30 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index fde9198..51ef532 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -608,6 +608,9 @@ struct ovn_datapath {
      * the "redirect-chassis". */
     struct ovn_port *l3redirect_port;
 
+    /* NAT entries configured on the router. */
+    struct ovn_nat *nat_entries;
+
     struct ovn_port **localnet_ports;
     size_t n_localnet_ports;
 
@@ -620,6 +623,65 @@ struct ovn_datapath {
     struct hmap nb_pgs;
 };
 
+/* Contains a NAT entry with the external addresses pre-parsed. */
+struct ovn_nat {
+    const struct nbrec_nat *nb;
+    struct lport_addresses ext_addrs;
+};
+
+/* Returns true if a 'nat_entry' is valid, i.e.:
+ * - parsing was successful.
+ * - the string yielded exactly one IPv4 address or exactly one IPv6 address.
+ */
+static bool nat_entry_is_valid(const struct ovn_nat *nat_entry)
+{
+    const struct lport_addresses *ext_addrs = &nat_entry->ext_addrs;
+
+    return (ext_addrs->n_ipv4_addrs == 1 && ext_addrs->n_ipv6_addrs == 0) ||
+        (ext_addrs->n_ipv4_addrs == 0 && ext_addrs->n_ipv6_addrs == 1);
+}
+
+static bool nat_entry_is_v6(const struct ovn_nat *nat_entry)
+{
+    return nat_entry->ext_addrs.n_ipv6_addrs > 0;
+}
+
+static void init_nat_entries(struct ovn_datapath *od)
+{
+    if (!od->nbr || od->nbr->n_nat == 0) {
+        return;
+    }
+
+    od->nat_entries = xmalloc(od->nbr->n_nat * sizeof *od->nat_entries);
+
+    for (size_t i = 0; i < od->nbr->n_nat; i++) {
+        const struct nbrec_nat *nat = od->nbr->nat[i];
+        struct ovn_nat *nat_entry = &od->nat_entries[i];
+
+        nat_entry->nb = nat;
+        if (!extract_ip_addresses(nat->external_ip,
+                                  &nat_entry->ext_addrs) ||
+                !nat_entry_is_valid(nat_entry)) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+
+            VLOG_WARN_RL(&rl,
+                         "Bad ip address %s in nat configuration "
+                         "for router %s", nat->external_ip, od->nbr->name);
+        }
+    }
+}
+
+static void destroy_nat_entries(struct ovn_datapath *od)
+{
+    if (!od->nbr) {
+        return;
+    }
+
+    for (size_t i = 0; i < od->nbr->n_nat; i++) {
+        destroy_lport_addresses(&od->nat_entries[i].ext_addrs);
+    }
+}
+
 /* A group of logical router datapaths which are connected - either
  * directly or indirectly.
  * Each logical router can belong to only one group. */
@@ -677,6 +739,8 @@ ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od)
         ovn_destroy_tnlids(&od->port_tnlids);
         bitmap_free(od->ipam_info.allocated_ipv4s);
         free(od->router_ports);
+        destroy_nat_entries(od);
+        free(od->nat_entries);
         free(od->localnet_ports);
         ovn_ls_port_group_destroy(&od->nb_pgs);
         destroy_mcast_info_for_datapath(od);
@@ -1105,6 +1169,7 @@ join_datapaths(struct northd_context *ctx, struct hmap *datapaths,
             ovs_list_push_back(nb_only, &od->list);
         }
         init_mcast_info_for_datapath(od);
+        init_nat_entries(od);
         ovs_list_push_back(lr_list, &od->lr_list);
     }
 }
@@ -8454,30 +8519,24 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             snat_ips[n_snat_ips++] = snat_ip;
         }
 
-        for (int i = 0; i < op->od->nbr->n_nat; i++) {
-            const struct nbrec_nat *nat;
-
-            nat = op->od->nbr->nat[i];
+        for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
+            struct ovn_nat *nat_entry = &op->od->nat_entries[i];
+            const struct nbrec_nat *nat = nat_entry->nb;
 
-            ovs_be32 ip;
-            struct in6_addr ipv6;
-            bool is_v6 = false;
-            if (!ip_parse(nat->external_ip, &ip) || !ip) {
-                if (!ipv6_parse(nat->external_ip, &ipv6)) {
-                    static struct vlog_rate_limit rl =
-                        VLOG_RATE_LIMIT_INIT(5, 1);
-                    VLOG_WARN_RL(&rl, "bad ip address %s in nat configuration "
-                                 "for router %s", nat->external_ip, op->key);
-                    continue;
-                }
-                is_v6 = true;
+            /* Skip entries we failed to parse. */
+            if (!nat_entry_is_valid(nat_entry)) {
+                continue;
             }
 
             if (!strcmp(nat->type, "snat")) {
-                if (is_v6) {
+                if (nat_entry_is_v6(nat_entry)) {
+                    struct in6_addr *ipv6 =
+                        &nat_entry->ext_addrs.ipv6_addrs[0].addr;
+
                     snat_ips[n_snat_ips].family = AF_INET6;
-                    snat_ips[n_snat_ips++].ipv6 = ipv6;
+                    snat_ips[n_snat_ips++].ipv6 = *ipv6;
                 } else {
+                    ovs_be32 ip = nat_entry->ext_addrs.ipv4_addrs[0].addr;
                     snat_ips[n_snat_ips].family = AF_INET;
                     snat_ips[n_snat_ips++].ipv4 = ip;
                 }
@@ -8520,22 +8579,18 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                     }
                 }
             }
-            if (is_v6) {
-                /* For ND solicitations, we need to listen for both the
-                 * unicast IPv6 address and its all-nodes multicast address,
-                 * but always respond with the unicast IPv6 address. */
-                char sn_addr_s[INET6_ADDRSTRLEN + 1];
-                struct in6_addr sn_addr;
-                in6_addr_solicited_node(&sn_addr, &ipv6);
-                ipv6_string_mapped(sn_addr_s, &sn_addr);
 
+            struct lport_addresses *ext_addrs = &nat_entry->ext_addrs;
+            if (nat_entry_is_v6(nat_entry)) {
                 build_lrouter_nd_flow(op->od, op, "nd_na",
-                                       nat->external_ip, sn_addr_s,
-                                       mac_s, &match, 90,
-                                       lflows, &nat->header_);
+                                      ext_addrs->ipv6_addrs[0].addr_s,
+                                      ext_addrs->ipv6_addrs[0].sn_addr_s,
+                                      mac_s, &match, 90,
+                                      lflows, &nat->header_);
             } else {
                 build_lrouter_arp_flow(op->od, op,
-                                       nat->external_ip, mac_s, &match, 90,
+                                       ext_addrs->ipv4_addrs[0].addr_s,
+                                       mac_s, &match, 90,
                                        lflows, &nat->header_);
             }
         }

From patchwork Tue Jun 30 21:42:16 2020
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Dumitru Ceara 
X-Patchwork-Id: 1320136
Return-Path: 
X-Original-To: incoming@patchwork.ozlabs.org
Delivered-To: patchwork-incoming@bilbo.ozlabs.org
Authentication-Results: ozlabs.org;
 spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org
 (client-ip=140.211.166.133; helo=hemlock.osuosl.org;
 envelope-from=ovs-dev-bounces@openvswitch.org; receiver=)
Authentication-Results: ozlabs.org;
 dmarc=fail (p=none dis=none) header.from=redhat.com
Authentication-Results: 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=AxngptkI;
	dkim-atps=neutral
Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by ozlabs.org (Postfix) with ESMTPS id 49xHrs4xLgz9sTV
	for ; Wed,  1 Jul 2020 07:42:37 +1000 (AEST)
Received: from localhost (localhost [127.0.0.1])
	by hemlock.osuosl.org (Postfix) with ESMTP id 0094E88602;
	Tue, 30 Jun 2020 21:42:36 +0000 (UTC)
X-Virus-Scanned: amavisd-new at osuosl.org
Received: from hemlock.osuosl.org ([127.0.0.1])
	by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id pcKQ+qJ4nDVh; Tue, 30 Jun 2020 21:42:32 +0000 (UTC)
Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56])
	by hemlock.osuosl.org (Postfix) with ESMTP id AA2338851D;
	Tue, 30 Jun 2020 21:42:32 +0000 (UTC)
Received: from lf-lists.osuosl.org (localhost [127.0.0.1])
	by lists.linuxfoundation.org (Postfix) with ESMTP id 7216AC088F;
	Tue, 30 Jun 2020 21:42:32 +0000 (UTC)
X-Original-To: dev@openvswitch.org
Delivered-To: ovs-dev@lists.linuxfoundation.org
Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137])
 by lists.linuxfoundation.org (Postfix) with ESMTP id 6CF80C016E
 for ; Tue, 30 Jun 2020 21:42:31 +0000 (UTC)
Received: from localhost (localhost [127.0.0.1])
 by fraxinus.osuosl.org (Postfix) with ESMTP id 61BC986268
 for ; Tue, 30 Jun 2020 21:42:31 +0000 (UTC)
X-Virus-Scanned: amavisd-new at osuosl.org
Received: from fraxinus.osuosl.org ([127.0.0.1])
 by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)
 with ESMTP id PfVljvQJQC55 for ;
 Tue, 30 Jun 2020 21:42:27 +0000 (UTC)
X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6
Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com
 [205.139.110.61])
 by fraxinus.osuosl.org (Postfix) with ESMTPS id E7EDE86287
 for ; Tue, 30 Jun 2020 21:42:24 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1593553344;
 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=o8QJQVhd9had89Xmm+8iivJW7LvuaqiW6uCKDs74S8k=;
 b=AxngptkI7gxnCmitSIXpvzlzGwCz0UaH/1ent1bmoT2vUx7pZmaIyPhDQwA5FdHgJzg1Le
 YIoZqCZnb1ujf46sO/TUeaG51CgKZnP3CmZgUpDLGhcj95Y6enNDxESWqaAA0A++1hff/1
 pQrEd52ESJKPV3BRfVwtn/gYFXAv6Dw=
Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com
 [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id
 us-mta-300-5Dhb2G63OWSzdmMZtcBh_Q-1; Tue, 30 Jun 2020 17:42:20 -0400
X-MC-Unique: 5Dhb2G63OWSzdmMZtcBh_Q-1
Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com
 [10.5.11.16])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5E8391932481;
 Tue, 30 Jun 2020 21:42:19 +0000 (UTC)
Received: from dceara.remote.csb (ovpn-112-145.ams2.redhat.com
 [10.36.112.145])
 by smtp.corp.redhat.com (Postfix) with ESMTP id 45DB85C1D3;
 Tue, 30 Jun 2020 21:42:18 +0000 (UTC)
From: Dumitru Ceara 
To: dev@openvswitch.org
Date: Tue, 30 Jun 2020 23:42:16 +0200
Message-Id: <20200630214211.26800.39678.stgit@dceara.remote.csb>
In-Reply-To: <20200630214107.26800.21305.stgit@dceara.remote.csb>
References: <20200630214107.26800.21305.stgit@dceara.remote.csb>
User-Agent: StGit/0.17.1-dirty
MIME-Version: 1.0
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16
Authentication-Results: relay.mimecast.com;
 auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Cc: hzhou@ovn.org
Subject: [ovs-dev] [PATCH v2 ovn 5/5] ovn-northd: Minimize number of ARP/NS
	responder flows for DNAT.
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" 

Most ARP/NS responder flows can be configured per datapath instead of per
router port.

The only exception is with distributed gateway router ports which need
special treatment. This patch changes the ARP/NS responder behavior and adds:
- Priority 92 flows to reply to ARP requests on distributed gateway router
  ports, on the chassis where the DNAT entry is bound.
- Priority 91 flows to drop ARP requests on distributed gateway router ports,
  on chassis where the DNAT entry is not bound.
- Priority 90 flows to reply to ARP requests on all other router ports. This
  last type of flows is programmed exactly once per logical router limiting
  the total number of required logical flows.

Suggested-by: Han Zhou 
Reported-by: Girish Moodalbail 
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2020-June/050186.html
Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.8.xml |   16 +++-
 northd/ovn-northd.c     |  203 ++++++++++++++++++++++++++++++++---------------
 tests/ovn-northd.at     |   65 +++++++++------
 tests/ovn.at            |    8 +-
 4 files changed, 190 insertions(+), 102 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 84224ff..11607c0 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1857,9 +1857,8 @@ nd_na_router {
           IPv4: For a configured DNAT IP address or a load balancer
           IPv4 VIP A, for each router port P with
           Ethernet address E, a priority-90 flow matches
-          inport == P && arp.op == 1 &&
-          arp.tpa == A (ARP request)
-          with the following actions:
+          arp.op == 1 && arp.tpa == A
+          (ARP request) with the following actions:
         

 
         
@@ -1876,6 +1875,11 @@ output;
         

 
         

+          IPv4: For a configured load balancer IPv4 VIP, a similar flow is
+          added with the additional match inport == P.
+        

+
+        

           If the router port P is a distributed gateway router
           port, then the is_chassis_resident(P) is
           also added in the match condition for the load balancer IPv4
@@ -1922,9 +1926,11 @@ nd_na {
         
    
           
  • 
             If the corresponding NAT rule cannot be handled in a
-            distributed manner, then this flow is only programmed on
+            distributed manner, then a priority-92 flow is programmed on
             the gateway port instance on the
-            redirect-chassis.  This behavior avoids
+            redirect-chassis.  A priority-91 drop flow is
+            programmed on the other chassis when ARP requests/NS packets
+            are received on the gateway port. This behavior avoids
             generation of multiple ARP responses from different chassis,
             and allows upstream MAC learning to point to the
             redirect-chassis.
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 51ef532..25fb90f 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -8039,7 +8039,7 @@ lrouter_nat_is_stateless(const struct nbrec_nat *nat)
 static void
 build_lrouter_arp_flow(struct ovn_datapath *od, struct ovn_port *op,
                        const char *ip_address, const char *eth_addr,
-                       struct ds *extra_match, uint16_t priority,
+                       struct ds *extra_match, bool drop, uint16_t priority,
                        struct hmap *lflows, const struct ovsdb_idl_row *hint)
 {
     struct ds match = DS_EMPTY_INITIALIZER;
@@ -8054,20 +8054,24 @@ build_lrouter_arp_flow(struct ovn_datapath *od, struct ovn_port *op,
     if (extra_match && ds_last(extra_match) != EOF) {
         ds_put_format(&match, " && %s", ds_cstr(extra_match));
     }
-    ds_put_format(&actions,
-                  "eth.dst = eth.src; "
-                  "eth.src = %s; "
-                  "arp.op = 2; /* ARP reply */ "
-                  "arp.tha = arp.sha; "
-                  "arp.sha = %s; "
-                  "arp.tpa = arp.spa; "
-                  "arp.spa = %s; "
-                  "outport = inport; "
-                  "flags.loopback = 1; "
-                  "output;",
-                  eth_addr,
-                  eth_addr,
-                  ip_address);
+    if (drop) {
+        ds_put_format(&actions, "drop;");
+    } else {
+        ds_put_format(&actions,
+                      "eth.dst = eth.src; "
+                      "eth.src = %s; "
+                      "arp.op = 2; /* ARP reply */ "
+                      "arp.tha = arp.sha; "
+                      "arp.sha = %s; "
+                      "arp.tpa = arp.spa; "
+                      "arp.spa = %s; "
+                      "outport = inport; "
+                      "flags.loopback = 1; "
+                      "output;",
+                      eth_addr,
+                      eth_addr,
+                      ip_address);
+    }
 
     ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
                             ds_cstr(&match), ds_cstr(&actions), hint);
@@ -8086,7 +8090,7 @@ static void
 build_lrouter_nd_flow(struct ovn_datapath *od, struct ovn_port *op,
                       const char *action, const char *ip_address,
                       const char *sn_ip_address, const char *eth_addr,
-                      struct ds *extra_match, uint16_t priority,
+                      struct ds *extra_match, bool drop, uint16_t priority,
                       struct hmap *lflows,
                       const struct ovsdb_idl_row *hint)
 {
@@ -8108,21 +8112,25 @@ build_lrouter_nd_flow(struct ovn_datapath *od, struct ovn_port *op,
         ds_put_format(&match, " && %s", ds_cstr(extra_match));
     }
 
-    ds_put_format(&actions,
-                  "%s { "
-                    "eth.src = %s; "
-                    "ip6.src = %s; "
-                    "nd.target = %s; "
-                    "nd.tll = %s; "
-                    "outport = inport; "
-                    "flags.loopback = 1; "
-                    "output; "
-                  "};",
-                  action,
-                  eth_addr,
-                  ip_address,
-                  ip_address,
-                  eth_addr);
+    if (drop) {
+        ds_put_format(&actions, "drop;");
+    } else {
+        ds_put_format(&actions,
+                      "%s { "
+                        "eth.src = %s; "
+                        "ip6.src = %s; "
+                        "nd.target = %s; "
+                        "nd.tll = %s; "
+                        "outport = inport; "
+                        "flags.loopback = 1; "
+                        "output; "
+                      "};",
+                      action,
+                      eth_addr,
+                      ip_address,
+                      ip_address,
+                      eth_addr);
+    }
 
     ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
                             ds_cstr(&match), ds_cstr(&actions), hint);
@@ -8312,7 +8320,41 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                       "ip4.dst == 0.0.0.0/8",
                       "drop;");
 
-        /* Priority-90 flows reply to ARP requests and ND packets. */
+        /* Priority-90-92 flows handle ARP requests and ND packets. Most are
+         * per logical port but DNAT addresses can be handled per datapath
+         * for non gateway router ports.
+         */
+        for (int i = 0; i < od->nbr->n_nat; i++) {
+            struct ovn_nat *nat_entry = &od->nat_entries[i];
+            const struct nbrec_nat *nat = nat_entry->nb;
+
+            /* Skip entries we failed to parse. */
+            if (!nat_entry_is_valid(nat_entry)) {
+                continue;
+            }
+
+            if (!strcmp(nat->type, "snat")) {
+                continue;
+            }
+
+            /* Priority 91 and 92 flows are added for each gateway router
+             * port to handle the special cases. In case we get the packet
+             * on a regular port, just reply with the port's ETH address.
+             */
+            struct lport_addresses *ext_addrs = &nat_entry->ext_addrs;
+            if (nat_entry_is_v6(nat_entry)) {
+                build_lrouter_nd_flow(od, NULL, "nd_na",
+                                      ext_addrs->ipv6_addrs[0].addr_s,
+                                      ext_addrs->ipv6_addrs[0].sn_addr_s,
+                                      REG_INPORT_ETH_ADDR, NULL, false, 90,
+                                      lflows, &nat->header_);
+            } else {
+                build_lrouter_arp_flow(od, NULL,
+                                       ext_addrs->ipv4_addrs[0].addr_s,
+                                       REG_INPORT_ETH_ADDR, NULL, false, 90,
+                                       lflows, &nat->header_);
+            }
+        }
 
         /* Drop ARP packets (priority 85). ARP request packets for router's own
          * IPs are handled with priority-90 flows.
@@ -8462,8 +8504,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
 
             build_lrouter_arp_flow(op->od, op,
                                    op->lrp_networks.ipv4_addrs[i].addr_s,
-                                   REG_INPORT_ETH_ADDR, &match, 90, lflows,
-                                   &op->nbrp->header_);
+                                   REG_INPORT_ETH_ADDR, &match, false, 90,
+                                   lflows, &op->nbrp->header_);
         }
 
         /* A set to hold all load-balancer vips that need ARP responses. */
@@ -8481,7 +8523,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
 
             build_lrouter_arp_flow(op->od, op,
                                    ip_address, REG_INPORT_ETH_ADDR,
-                                   &match, 90, lflows, NULL);
+                                   &match, false, 90, lflows, NULL);
         }
 
         SSET_FOR_EACH (ip_address, &all_ips_v6) {
@@ -8493,7 +8535,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
 
             build_lrouter_nd_flow(op->od, op, "nd_na",
                                   ip_address, NULL, REG_INPORT_ETH_ADDR,
-                                  &match, 90, lflows, NULL);
+                                  &match, false, 90, lflows, NULL);
         }
 
         sset_destroy(&all_ips_v4);
@@ -8546,53 +8588,84 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             /* Mac address to use when replying to ARP/NS. */
             const char *mac_s = REG_INPORT_ETH_ADDR;
 
+            /* ARP/NS packets are taken care of per router. The only exception
+             * is on the l3dgw_port where we might need to use a different
+             * ETH address.
+             */
+            if (op != op->od->l3dgw_port) {
+                continue;
+            }
+
             /* ARP / ND handling for external IP addresses.
              *
              * DNAT IP addresses are external IP addresses that need ARP
              * handling. */
             ds_clear(&match);
 
-            if (op->od->l3dgw_port && op == op->od->l3dgw_port) {
-                struct eth_addr mac;
-                if (nat->external_mac &&
-                    eth_addr_from_string(nat->external_mac, &mac)
-                    && nat->logical_port) {
-                    /* distributed NAT case, use nat->external_mac */
-                    mac_s = nat->external_mac;
-                    /* Traffic with eth.src = nat->external_mac should only be
-                     * sent from the chassis where nat->logical_port is
-                     * resident, so that upstream MAC learning points to the
-                     * correct chassis.  Also need to avoid generation of
-                     * multiple ARP responses from different chassis. */
-                    ds_put_format(&match, "is_chassis_resident(\"%s\")",
-                                  nat->logical_port);
-                } else {
-                    mac_s = REG_INPORT_ETH_ADDR;
-                    /* Traffic with eth.src = l3dgw_port->lrp_networks.ea_s
-                     * should only be sent from the "redirect-chassis", so that
-                     * upstream MAC learning points to the "redirect-chassis".
-                     * Also need to avoid generation of multiple ARP responses
-                     * from different chassis. */
-                    if (op->od->l3redirect_port) {
-                        ds_put_format(&match, "is_chassis_resident(%s)",
-                                      op->od->l3redirect_port->json_key);
-                    }
+            struct ds match_cr_port = DS_EMPTY_INITIALIZER;
+            struct ds match_non_cr_port = DS_EMPTY_INITIALIZER;
+
+            struct eth_addr mac;
+            if (nat->external_mac &&
+                eth_addr_from_string(nat->external_mac, &mac)
+                && nat->logical_port) {
+                /* distributed NAT case, use nat->external_mac */
+                mac_s = nat->external_mac;
+                /* Traffic with eth.src = nat->external_mac should only be
+                 * sent from the chassis where nat->logical_port is
+                 * resident, so that upstream MAC learning points to the
+                 * correct chassis.  Also need to avoid generation of
+                 * multiple ARP responses from different chassis. */
+                ds_put_format(&match_cr_port,
+                              "is_chassis_resident(\"%s\")",
+                              nat->logical_port);
+                ds_put_format(&match_non_cr_port,
+                              "!is_chassis_resident(\"%s\")",
+                              nat->logical_port);
+            } else {
+                mac_s = REG_INPORT_ETH_ADDR;
+                /* Traffic with eth.src = l3dgw_port->lrp_networks.ea_s
+                 * should only be sent from the "redirect-chassis", so that
+                 * upstream MAC learning points to the "redirect-chassis".
+                 * Also need to avoid generation of multiple ARP responses
+                 * from different chassis. */
+                if (op->od->l3redirect_port) {
+                    ds_put_format(&match_cr_port,
+                                  "is_chassis_resident(\"%s\")",
+                                  op->od->l3redirect_port->json_key);
+                    ds_put_format(&match_non_cr_port,
+                                  "!is_chassis_resident(\"%s\")",
+                                  op->od->l3redirect_port->json_key);
                 }
             }
 
+            /* Respond to ARP/NS requests on the chassis that binds the gw
+             * port. Drop the ARP/NS requests on other chassis.
+             */
             struct lport_addresses *ext_addrs = &nat_entry->ext_addrs;
             if (nat_entry_is_v6(nat_entry)) {
                 build_lrouter_nd_flow(op->od, op, "nd_na",
                                       ext_addrs->ipv6_addrs[0].addr_s,
                                       ext_addrs->ipv6_addrs[0].sn_addr_s,
-                                      mac_s, &match, 90,
+                                      mac_s, &match_cr_port, false, 92,
+                                      lflows, &nat->header_);
+                build_lrouter_nd_flow(op->od, op, "nd_na",
+                                      ext_addrs->ipv6_addrs[0].addr_s,
+                                      ext_addrs->ipv6_addrs[0].sn_addr_s,
+                                      mac_s, &match_non_cr_port, true, 91,
                                       lflows, &nat->header_);
             } else {
                 build_lrouter_arp_flow(op->od, op,
                                        ext_addrs->ipv4_addrs[0].addr_s,
-                                       mac_s, &match, 90,
+                                       mac_s, &match_cr_port, false, 92,
+                                       lflows, &nat->header_);
+                build_lrouter_arp_flow(op->od, op,
+                                       ext_addrs->ipv4_addrs[0].addr_s,
+                                       mac_s, &match_non_cr_port, true, 91,
                                        lflows, &nat->header_);
             }
+            ds_destroy(&match_cr_port);
+            ds_destroy(&match_non_cr_port);
         }
 
         if (!smap_get(&op->od->nbr->options, "chassis")
@@ -8778,8 +8851,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             build_lrouter_nd_flow(op->od, op, "nd_na_router",
                                   op->lrp_networks.ipv6_addrs[i].addr_s,
                                   op->lrp_networks.ipv6_addrs[i].sn_addr_s,
-                                  REG_INPORT_ETH_ADDR, &match, 90, lflows,
-                                  &op->nbrp->header_);
+                                  REG_INPORT_ETH_ADDR, &match, false, 90,
+                                  lflows, &op->nbrp->header_);
         }
 
         /* UDP/TCP port unreachable */
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index a3fb7ef..4a13456 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1594,33 +1594,24 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
 # Ingress router port ETH address is used for ARP reply/NA in lr_in_ip_input.
 AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.0/24), dnl
-action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 42.42.42.1; outport = inport; flags.loopback = 1; output;)
-  table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.op == 1 && arp.tpa == 43.43.43.2), dnl
+match=(arp.op == 1 && arp.tpa == 43.43.43.2), dnl
 action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.op == 1 && arp.tpa == 43.43.43.3), dnl
+match=(arp.op == 1 && arp.tpa == 43.43.43.3), dnl
 action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.op == 1 && arp.tpa == 43.43.43.4), dnl
+match=(arp.op == 1 && arp.tpa == 43.43.43.4), dnl
 action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.0/24), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 42.42.42.1; outport = inport; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
 match=(inport == "lrp" && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd_ns && nd.target == fe80::200:ff:fe00:1), dnl
 action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:1; nd.target = fe80::200:ff:fe00:1; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
 match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.1 && arp.spa == 43.43.43.0/24), dnl
 action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.1; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2), dnl
-action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
-  table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3), dnl
-action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
-  table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.4), dnl
-action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
-  table=3 (lr_in_ip_input     ), priority=90   , dnl
 match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd_ns && nd.target == fe80::200:ff:fe00:100), dnl
 action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:100; nd.target = fe80::200:ff:fe00:100; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
 ])
@@ -1654,37 +1645,55 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
 
 # Ingress router port is used for ARP reply/NA in lr_in_ip_input.
 # xxreg0[0..47] is used unless external_mac is set.
+# Priority 90 flows (per router).
 AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.0/24), dnl
-action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 42.42.42.1; outport = inport; flags.loopback = 1; output;)
-  table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.op == 1 && arp.tpa == 43.43.43.2), dnl
+match=(arp.op == 1 && arp.tpa == 43.43.43.2), dnl
 action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.op == 1 && arp.tpa == 43.43.43.3), dnl
+match=(arp.op == 1 && arp.tpa == 43.43.43.3), dnl
 action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && arp.op == 1 && arp.tpa == 43.43.43.4), dnl
+match=(arp.op == 1 && arp.tpa == 43.43.43.4), dnl
 action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
+match=(inport == "lrp" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.0/24), dnl
+action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 42.42.42.1; outport = inport; flags.loopback = 1; output;)
+  table=3 (lr_in_ip_input     ), priority=90   , dnl
 match=(inport == "lrp" && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd_ns && nd.target == fe80::200:ff:fe00:1), dnl
 action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:1; nd.target = fe80::200:ff:fe00:1; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
 match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.1 && arp.spa == 43.43.43.0/24), dnl
 action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.1; outport = inport; flags.loopback = 1; output;)
   table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2 && is_chassis_resident("cr-lrp-public")), dnl
+match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd_ns && nd.target == fe80::200:ff:fe00:100 && is_chassis_resident("cr-lrp-public")), dnl
+action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:100; nd.target = fe80::200:ff:fe00:100; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
+])
+
+# Priority 91 drop flows (per distributed gw port), if port is not resident.
+AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=91" | grep "arp\|nd" | sort], [0], [dnl
+  table=3 (lr_in_ip_input     ), priority=91   , dnl
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2 && !is_chassis_resident(""cr-lrp-public"")), dnl
+action=(drop;)
+  table=3 (lr_in_ip_input     ), priority=91   , dnl
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3 && !is_chassis_resident(""cr-lrp-public"")), dnl
+action=(drop;)
+  table=3 (lr_in_ip_input     ), priority=91   , dnl
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.4 && !is_chassis_resident("ls-vm")), dnl
+action=(drop;)
+])
+
+# Priority 92 ARP/NS responders (per distributed gw port), if port is resident.
+AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=92" | grep "arp\|nd" | sort], [0], [dnl
+  table=3 (lr_in_ip_input     ), priority=92   , dnl
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2 && is_chassis_resident(""cr-lrp-public"")), dnl
 action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
-  table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3 && is_chassis_resident("cr-lrp-public")), dnl
+  table=3 (lr_in_ip_input     ), priority=92   , dnl
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3 && is_chassis_resident(""cr-lrp-public"")), dnl
 action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa = arp.spa; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
-  table=3 (lr_in_ip_input     ), priority=90   , dnl
+  table=3 (lr_in_ip_input     ), priority=92   , dnl
 match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.4 && is_chassis_resident("ls-vm")), dnl
 action=(eth.dst = eth.src; eth.src = 00:00:00:00:00:02; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 00:00:00:00:00:02; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
-  table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd_ns && nd.target == fe80::200:ff:fe00:100 && is_chassis_resident("cr-lrp-public")), dnl
-action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = fe80::200:ff:fe00:100; nd.target = fe80::200:ff:fe00:100; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
 ])
 
 # xreg0[0..47] isn't used anywhere else.
diff --git a/tests/ovn.at b/tests/ovn.at
index 6a9bdf1..8e11d2d 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -19324,7 +19324,7 @@ OVS_WAIT_UNTIL([
 send_arp_request 1 0 ${src_mac} $(ip_to_hex 10 0 0 254) $(ip_to_hex 10 0 0 121)
 
 # Verify that the ARP request is replied to from hv1 and not hv2.
-match_arp_req="priority=90.*${match_r1_metadata}.*arp_tpa=10.0.0.121,arp_op=1"
+match_arp_req="priority=92.*${match_r1_metadata}.*arp_tpa=10.0.0.121,arp_op=1"
 
 as hv1
 OVS_WAIT_UNTIL([
@@ -19344,7 +19344,7 @@ OVS_WAIT_UNTIL([
 send_arp_request 1 0 ${src_mac} $(ip_to_hex 10 0 0 254) $(ip_to_hex 10 0 0 122)
 
 # Verify that the ARP request is replied to from hv2 and not hv1.
-match_arp_req="priority=90.*${match_r1_metadata}.*arp_tpa=10.0.0.122,arp_op=1"
+match_arp_req="priority=92.*${match_r1_metadata}.*arp_tpa=10.0.0.122,arp_op=1"
 
 as hv2
 OVS_WAIT_UNTIL([
@@ -19388,7 +19388,7 @@ dst_ipv6=00100000000000000000000000000121
 send_nd_ns 1 0 ${src_mac} ${src_ipv6} ${dst_ipv6} 72dd
 
 # Verify that the ND_NS is replied to from hv1 and not hv2.
-match_nd_ns="priority=90.*${match_r1_metadata}.*icmp_type=135.*nd_target=10::121"
+match_nd_ns="priority=92.*${match_r1_metadata}.*icmp_type=135.*nd_target=10::121"
 
 as hv1
 OVS_WAIT_UNTIL([
@@ -19410,7 +19410,7 @@ dst_ipv6=00100000000000000000000000000122
 send_nd_ns 1 0 ${src_mac} ${src_ipv6} ${dst_ipv6} 72db
 
 # Verify that the ND_NS is replied to from hv2 and not hv1.
-match_nd_ns="priority=90.*${match_r1_metadata}.*icmp_type=135.*nd_target=10::122"
+match_nd_ns="priority=92.*${match_r1_metadata}.*icmp_type=135.*nd_target=10::122"
 
 as hv2
 OVS_WAIT_UNTIL([