From patchwork Fri Jul 3 18:33:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1322596 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.137; helo=fraxinus.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=EagiTVBz; dkim-atps=neutral Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49z3Wr0fPTz9sRk for ; Sat, 4 Jul 2020 04:34:00 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 8E44D87E75; Fri, 3 Jul 2020 18:33:58 +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 5_Q9KXK5wrF1; Fri, 3 Jul 2020 18:33:56 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 7E8A387E38; Fri, 3 Jul 2020 18:33:56 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 61820C08A0; Fri, 3 Jul 2020 18:33:56 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5CB9FC08A0 for ; Fri, 3 Jul 2020 18:33:55 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 4CCAE88D6A for ; Fri, 3 Jul 2020 18:33:55 +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 nSwn+WSNXkOf for ; Fri, 3 Jul 2020 18:33:53 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) by whitealder.osuosl.org (Postfix) with ESMTPS id A9FD888D9F for ; Fri, 3 Jul 2020 18:33:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593801232; 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=/WCjTqzZhaWRxZx6/rDFJkO/9aTO0BXvRfkg79M3yfs=; b=EagiTVBzfHSsgB0oWlFfsd8xxAUGWk15xqkgU0Yw/QxjXlVuy5i3z7IW1y4OOPCASi/I8p 8SoNFQofHX3PZl6Lv9Y40RWKtTOZrDOTeJyCyRHVNiWtc6VoZ2grjYLyhTBgney3iQ4etL 5Euifc8T13Ouik/80Dkq5nNE6+JBOFk= 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-62-NhQ13Fr5PA-ClUgQX2I4NQ-1; Fri, 03 Jul 2020 14:33:48 -0400 X-MC-Unique: NhQ13Fr5PA-ClUgQX2I4NQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7F549107ACCA; Fri, 3 Jul 2020 18:33:47 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-61.ams2.redhat.com [10.36.114.61]) by smtp.corp.redhat.com (Postfix) with ESMTP id CD98D196B8; Fri, 3 Jul 2020 18:33:45 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Fri, 3 Jul 2020 20:33:43 +0200 Message-Id: <20200703183337.22921.268.stgit@dceara.remote.csb> In-Reply-To: <20200703183301.22921.95116.stgit@dceara.remote.csb> References: <20200703183301.22921.95116.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.12 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 v4 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. Acked-by: Han Zhou Signed-off-by: Dumitru Ceara --- northd/ovn-northd.c | 161 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 113 insertions(+), 48 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 417dbb6..605ae2a 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -227,8 +227,58 @@ 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 - R9 | UNUSED | + * +---------+-------------------------------------+ + * + * Logical Router pipeline: + * +-----+--------------------------+---+---------------+ + * | R0 | REGBIT_ND_RA_OPTS_RESULT | | | + * | | (= IN_ND_RA_OPTIONS) | | | + * | | NEXT_HOP_IPV4 | X | | + * | | (>= IP_INPUT) | X | | + * +-----+--------------------------+ R | | + * | R1 | SRC_IPV4 for ARP-REQ | E | NEXT_HOP_IPV6 | + * | | (>= IP_INPUT) | G | (>= IP_INPUT) | + * +-----+--------------------------+ 0 | | + * | R2 | UNUSED | | | + * +-----+--------------------------+ | | + * | R3 | UNUSED | | | + * +-----+--------------------------+---+---------------+ + * | R4 | UNUSED | | | + * +-----+--------------------------+ X | | + * | R5 | UNUSED | X |SRC_IPV6 for NS| + * +-----+--------------------------+ R | (>= IP_INPUT) | + * | R6 | UNUSED | E | | + * +-----+--------------------------+ G | | + * | R7 | UNUSED | 1 | | + * +-----+--------------------------+---+---------------+ + * | R8 | ECMP_GROUP_ID | + * | | ECMP_MEMBER_ID | + * +-----+--------------------------+ + * | | REGBIT_{ | + * | | EGRESS_LOOPBACK/ | + * | R9 | PKT_LARGER/ | + * | | LOOKUP_NEIGHBOR_RESULT/| + * | | SKIP_LOOKUP_NEIGHBOR} | + * +-----+--------------------------+ + * + */ + /* 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, @@ -7139,15 +7189,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); @@ -7537,14 +7587,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); @@ -7579,8 +7629,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); @@ -7588,12 +7638,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); @@ -9064,7 +9114,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;", @@ -9226,9 +9277,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); @@ -9238,8 +9290,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), @@ -9561,14 +9614,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; @@ -9705,8 +9759,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) { @@ -9743,9 +9797,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)) { @@ -9754,17 +9810,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); @@ -9779,7 +9836,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); @@ -9850,7 +9908,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); @@ -9886,7 +9945,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); @@ -9937,8 +9997,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, @@ -9983,8 +10044,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); @@ -10038,7 +10100,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); @@ -10053,7 +10116,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); @@ -10075,10 +10139,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. @@ -10224,7 +10288,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); @@ -10252,15 +10317,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 Fri Jul 3 18:34:11 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1322598 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=ZrVO2r/9; 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 49z3Xt5b78z9sDX for ; Sat, 4 Jul 2020 04:34:54 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id ED1D189919; Fri, 3 Jul 2020 18:34:52 +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 C-AGufOPzEUs; Fri, 3 Jul 2020 18:34:49 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 5DBF2898ED; Fri, 3 Jul 2020 18:34:49 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 38CE7C08A8; Fri, 3 Jul 2020 18:34:49 +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 6E8C7C08A0 for ; Fri, 3 Jul 2020 18:34:48 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 3AB1A24F0B for ; Fri, 3 Jul 2020 18:34:48 +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 IQUaG8s70svV for ; Fri, 3 Jul 2020 18:34:44 +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 silver.osuosl.org (Postfix) with ESMTPS id A29AA23B31 for ; Fri, 3 Jul 2020 18:34:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593801283; 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=QSLWChLSiovMN5AzCSSHpAcHnnFYyswlDD0MsWrwWwo=; b=ZrVO2r/9xteAPUMBHQSeHqxx/CwD8w3e1a5sf9FD5ctO8INRV3avdG/S0NkXyY5qsET9xv j2FVEL6oFNMtiFlfI3902B/QK9r7AbeZ2/IgZdiPZhfxaq6+pAk95lNlbs4KV0VlA1tT97 cU7KVxtdQQ1+ujtUKJ3IiYdqo+O6geY= 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-205-HuuTmXqKOl-l9BEXaPEhrQ-1; Fri, 03 Jul 2020 14:34:16 -0400 X-MC-Unique: HuuTmXqKOl-l9BEXaPEhrQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A0CE01005510; Fri, 3 Jul 2020 18:34:15 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-61.ams2.redhat.com [10.36.114.61]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8A2B81E2; Fri, 3 Jul 2020 18:34:14 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Fri, 3 Jul 2020 20:34:11 +0200 Message-Id: <20200703183352.22921.93459.stgit@dceara.remote.csb> In-Reply-To: <20200703183301.22921.95116.stgit@dceara.remote.csb> References: <20200703183301.22921.95116.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.14 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 v4 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 Acked-by: Han Zhou Signed-off-by: Dumitru Ceara --- northd/ovn-northd.8.xml | 22 ++++--- northd/ovn-northd.c | 153 +++++++++++++++++++++++++---------------------- tests/ovn-northd.at | 140 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 235 insertions(+), 80 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 605ae2a..4b4d059 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,36 +251,42 @@ enum ovn_stage {
  * +---------+-------------------------------------+
  *
  * Logical Router pipeline:
- * +-----+--------------------------+---+---------------+
- * | R0  | REGBIT_ND_RA_OPTS_RESULT |   |               |
- * |     |   (= IN_ND_RA_OPTIONS)   |   |               |
- * |     |      NEXT_HOP_IPV4       | X |               |
- * |     |      (>= IP_INPUT)       | X |               |
- * +-----+--------------------------+ R |               |
- * | R1  |   SRC_IPV4 for ARP-REQ   | E | NEXT_HOP_IPV6 |
- * |     |      (>= IP_INPUT)       | G | (>= IP_INPUT) |
- * +-----+--------------------------+ 0 |               |
- * | R2  |        UNUSED            |   |               |
- * +-----+--------------------------+   |               |
- * | R3  |        UNUSED            |   |               |
- * +-----+--------------------------+---+---------------+
- * | R4  |        UNUSED            |   |               |
- * +-----+--------------------------+ X |               |
- * | R5  |        UNUSED            | X |SRC_IPV6 for NS|
- * +-----+--------------------------+ R | (>= IP_INPUT) |
- * | R6  |        UNUSED            | E |               |
- * +-----+--------------------------+ G |               |
- * | R7  |        UNUSED            | 1 |               |
- * +-----+--------------------------+---+---------------+
- * | R8  |     ECMP_GROUP_ID        |
- * |     |     ECMP_MEMBER_ID       |
- * +-----+--------------------------+
- * |     | REGBIT_{                 |
- * |     |   EGRESS_LOOPBACK/       |
- * | R9  |   PKT_LARGER/            |
- * |     |   LOOKUP_NEIGHBOR_RESULT/|
- * |     |   SKIP_LOOKUP_NEIGHBOR}  |
- * +-----+--------------------------+
+ * +-----+--------------------------+---+-----------------+---+---------------+
+ * | R0  | REGBIT_ND_RA_OPTS_RESULT |   |                 |   |               |
+ * |     |   (= IN_ND_RA_OPTIONS)   | X |                 |   |               |
+ * |     |      NEXT_HOP_IPV4       | R |                 |   |               |
+ * |     |      (>= IP_INPUT)       | E | INPORT_ETH_ADDR | X |               |
+ * +-----+--------------------------+ G |   (< IP_INPUT)  | X |               |
+ * | R1  |   SRC_IPV4 for ARP-REQ   | 0 |                 | R |               |
+ * |     |      (>= IP_INPUT)       |   |                 | E | NEXT_HOP_IPV6 |
+ * +-----+--------------------------+---+-----------------+ G | (>= IP_INPUT) |
+ * | R2  |        UNUSED            | X |                 | 0 |               |
+ * |     |                          | R |                 |   |               |
+ * +-----+--------------------------+ E |     UNUSED      |   |               |
+ * | R3  |        UNUSED            | G |                 |   |               |
+ * |     |                          | 1 |                 |   |               |
+ * +-----+--------------------------+---+-----------------+---+---------------+
+ * | R4  |        UNUSED            | X |                 |   |               |
+ * |     |                          | R |                 |   |               |
+ * +-----+--------------------------+ E |     UNUSED      | X |               |
+ * | R5  |        UNUSED            | G |                 | X |               |
+ * |     |                          | 2 |                 | R |SRC_IPV6 for NS|
+ * +-----+--------------------------+---+-----------------+ E | (>= IP_INPUT) |
+ * | R6  |        UNUSED            | X |                 | G |               |
+ * |     |                          | R |                 | 1 |               |
+ * +-----+--------------------------+ E |     UNUSED      |   |               |
+ * | R7  |        UNUSED            | G |                 |   |               |
+ * |     |                          | 3 |                 |   |               |
+ * +-----+--------------------------+---+-----------------+---+---------------+
+ * | R8  |     ECMP_GROUP_ID        |   |                 |
+ * |     |     ECMP_MEMBER_ID       | X |                 |
+ * +-----+--------------------------+ R |                 |
+ * |     | REGBIT_{                 | E |                 |
+ * |     |   EGRESS_LOOPBACK/       | G |     UNUSED      |
+ * | R9  |   PKT_LARGER/            | 4 |                 |
+ * |     |   LOOKUP_NEIGHBOR_RESULT/|   |                 |
+ * |     |   SKIP_LOOKUP_NEIGHBOR}  |   |                 |
+ * +-----+--------------------------+---+-----------------+
  *
  */
 
@@ -8010,10 +8021,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",
@@ -8026,7 +8046,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
@@ -8293,17 +8314,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,
@@ -8330,17 +8349,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);
 
@@ -8361,18 +8378,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));
@@ -8495,18 +8510,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
@@ -8520,17 +8531,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) {
@@ -8748,18 +8755,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_);
@@ -9261,6 +9266,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"
@@ -9269,7 +9282,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 Fri Jul  3 18:34:24 2020
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Dumitru Ceara 
X-Patchwork-Id: 1322599
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=Z32oJsFT;
	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 49z3Y70qq4z9sDX
	for ; Sat,  4 Jul 2020 04:35:07 +1000 (AEST)
Received: from localhost (localhost [127.0.0.1])
	by hemlock.osuosl.org (Postfix) with ESMTP id 724AD898E8;
	Fri,  3 Jul 2020 18:35:05 +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 C7uw6vwY-HHB; Fri,  3 Jul 2020 18:35:02 +0000 (UTC)
Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56])
	by hemlock.osuosl.org (Postfix) with ESMTP id 6522989883;
	Fri,  3 Jul 2020 18:35:02 +0000 (UTC)
Received: from lf-lists.osuosl.org (localhost [127.0.0.1])
	by lists.linuxfoundation.org (Postfix) with ESMTP id 4C527C0888;
	Fri,  3 Jul 2020 18:35:02 +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 0438AC0733
 for ; Fri,  3 Jul 2020 18:35:00 +0000 (UTC)
Received: from localhost (localhost [127.0.0.1])
 by fraxinus.osuosl.org (Postfix) with ESMTP id E56FE87E79
 for ; Fri,  3 Jul 2020 18:34:59 +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 bDml9V2dgnp9 for ;
 Fri,  3 Jul 2020 18:34:58 +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 BD60387E83
 for ; Fri,  3 Jul 2020 18:34:57 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1593801296;
 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=dHX461zrr1gXT9xabRXbvLK/REnVYpJkHFV90GTt7gc=;
 b=Z32oJsFTUaRdZw17gklCmh6li/lQZoPZaGi6AU3GDZ/tbusHw96atVmsMAnHImYs8VRmc6
 DKz2Qm7rnk1DsH/fa9d53KiD0mu/sVNgI0ypJF0vHJ0qhxz1FVAjvCHx0ryKFfS22eWAO0
 A6++1weL2IZYO/js+TIL/VuARILkveI=
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-215-MlcLOoE-NIyJ0uj8A1lnxg-1; Fri, 03 Jul 2020 14:34:30 -0400
X-MC-Unique: MlcLOoE-NIyJ0uj8A1lnxg-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 1540B1009615;
 Fri,  3 Jul 2020 18:34:29 +0000 (UTC)
Received: from dceara.remote.csb (ovpn-114-61.ams2.redhat.com [10.36.114.61])
 by smtp.corp.redhat.com (Postfix) with ESMTP id 343C610027AC;
 Fri,  3 Jul 2020 18:34:26 +0000 (UTC)
From: Dumitru Ceara 
To: dev@openvswitch.org
Date: Fri,  3 Jul 2020 20:34:24 +0200
Message-Id: <20200703183420.22921.44802.stgit@dceara.remote.csb>
In-Reply-To: <20200703183301.22921.95116.stgit@dceara.remote.csb>
References: <20200703183301.22921.95116.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 v4 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.

Acked-by: Han Zhou 
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 4b4d059..14e1215 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -7978,6 +7978,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,
@@ -8273,13 +8372,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) {
@@ -8311,23 +8406,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. */
@@ -8338,59 +8420,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);
@@ -8446,123 +8495,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")
@@ -8734,13 +8720,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
@@ -8748,26 +8727,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 Fri Jul  3 18:34:37 2020
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Dumitru Ceara 
X-Patchwork-Id: 1322597
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=cCUTdcGC;
	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 49z3Xq2f74z9sRN
	for ; Sat,  4 Jul 2020 04:34:51 +1000 (AEST)
Received: from localhost (localhost [127.0.0.1])
	by whitealder.osuosl.org (Postfix) with ESMTP id 9E86D8889B;
	Fri,  3 Jul 2020 18:34:49 +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 aVrdGmnJwgh6; Fri,  3 Jul 2020 18:34:48 +0000 (UTC)
Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56])
	by whitealder.osuosl.org (Postfix) with ESMTP id 896B887598;
	Fri,  3 Jul 2020 18:34:48 +0000 (UTC)
Received: from lf-lists.osuosl.org (localhost [127.0.0.1])
	by lists.linuxfoundation.org (Postfix) with ESMTP id 64D91C0888;
	Fri,  3 Jul 2020 18:34:48 +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 3B602C0733
 for ; Fri,  3 Jul 2020 18:34:47 +0000 (UTC)
Received: from localhost (localhost [127.0.0.1])
 by hemlock.osuosl.org (Postfix) with ESMTP id 2A449898DC
 for ; Fri,  3 Jul 2020 18:34:47 +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 F3IbiDVWxXIR for ;
 Fri,  3 Jul 2020 18:34:46 +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 5BC0889883
 for ; Fri,  3 Jul 2020 18:34:46 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1593801285;
 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=XBgnyaOehqMDbnNnsqvI1HF8rWz+k0jfKNjPbmgFejE=;
 b=cCUTdcGCDWh0zkafP0o2cynOpJft/BPXd6/XXuXSLGN2tw5+3VR32ptkCSfEx8klglbUNO
 EUUsL7jWcfV9ZqBpF42EPGDkB1ipmefuqkFW6BCPQoewoirJxyc4r062spK1QYyigW1Ysf
 0Xs8OuruR21qFiodrSNT1SCGcfe2QPs=
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-260-mlAnJKo4MtO20SxaaTWMvw-1; Fri, 03 Jul 2020 14:34:43 -0400
X-MC-Unique: mlAnJKo4MtO20SxaaTWMvw-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 38F89107ACCA;
 Fri,  3 Jul 2020 18:34:42 +0000 (UTC)
Received: from dceara.remote.csb (ovpn-114-61.ams2.redhat.com [10.36.114.61])
 by smtp.corp.redhat.com (Postfix) with ESMTP id C448810027AC;
 Fri,  3 Jul 2020 18:34:40 +0000 (UTC)
From: Dumitru Ceara 
To: dev@openvswitch.org
Date: Fri,  3 Jul 2020 20:34:37 +0200
Message-Id: <20200703183434.22921.95770.stgit@dceara.remote.csb>
In-Reply-To: <20200703183301.22921.95116.stgit@dceara.remote.csb>
References: <20200703183301.22921.95116.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 v4 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.

Acked-by: Han Zhou 
Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.c |  119 ++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 89 insertions(+), 30 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 14e1215..2dc4e20 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -607,6 +607,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;
 
@@ -619,6 +622,69 @@ 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. */
@@ -676,6 +742,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);
@@ -1104,6 +1172,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);
     }
 }
@@ -8465,30 +8534,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;
                 }
@@ -8531,22 +8594,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 Fri Jul  3 18:34:51 2020
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Dumitru Ceara 
X-Patchwork-Id: 1322601
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.136; helo=silver.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=B5gCK/9n;
	dkim-atps=neutral
Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by ozlabs.org (Postfix) with ESMTPS id 49z3Zf2bnHz9sDX
	for ; Sat,  4 Jul 2020 04:36:26 +1000 (AEST)
Received: from localhost (localhost [127.0.0.1])
	by silver.osuosl.org (Postfix) with ESMTP id 82821266B3;
	Fri,  3 Jul 2020 18:36:24 +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 f-BTnjisa3Ev; Fri,  3 Jul 2020 18:35:58 +0000 (UTC)
Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56])
	by silver.osuosl.org (Postfix) with ESMTP id B56DA24F0B;
	Fri,  3 Jul 2020 18:35:29 +0000 (UTC)
Received: from lf-lists.osuosl.org (localhost [127.0.0.1])
	by lists.linuxfoundation.org (Postfix) with ESMTP id A9630C0888;
	Fri,  3 Jul 2020 18:35:29 +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 DA836C0733
 for ; Fri,  3 Jul 2020 18:35:28 +0000 (UTC)
Received: from localhost (localhost [127.0.0.1])
 by silver.osuosl.org (Postfix) with ESMTP id AC59524F0B
 for ; Fri,  3 Jul 2020 18:35:28 +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 kwHeOzUrs0XS for ;
 Fri,  3 Jul 2020 18:35:06 +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
 [207.211.31.120])
 by silver.osuosl.org (Postfix) with ESMTPS id 27D2B268AF
 for ; Fri,  3 Jul 2020 18:35:01 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1593801299;
 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=OvTYjcjdakoMT5fzYcPiQeRKG6pja2RrkSbBObZy7BA=;
 b=B5gCK/9nBweWV3IORpWqBafjTA8tw7oYKcomMdnOuTUFnHUrCATnQAXO8j0svT3ChcYDOH
 OAay3uqXgzts0c1E+NOcoiD8OSVR3BMTTF9XOPkiakOqnjTYEEwpg3KF6NysvgblSmvZOz
 waqH5AbSoXJcU2S67sea/AgR93P3S1s=
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-57-sOunX6k1N86FMsD1vvMTWQ-1; Fri, 03 Jul 2020 14:34:56 -0400
X-MC-Unique: sOunX6k1N86FMsD1vvMTWQ-1
Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com
 [10.5.11.12])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DABCF1902EA7;
 Fri,  3 Jul 2020 18:34:54 +0000 (UTC)
Received: from dceara.remote.csb (ovpn-114-61.ams2.redhat.com [10.36.114.61])
 by smtp.corp.redhat.com (Postfix) with ESMTP id BF98B6FEF4;
 Fri,  3 Jul 2020 18:34:53 +0000 (UTC)
From: Dumitru Ceara 
To: dev@openvswitch.org
Date: Fri,  3 Jul 2020 20:34:51 +0200
Message-Id: <20200703183447.22921.75705.stgit@dceara.remote.csb>
In-Reply-To: <20200703183301.22921.95116.stgit@dceara.remote.csb>
References: <20200703183301.22921.95116.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.12
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 v4 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 
Acked-by: Numan Siddique 
---
 northd/ovn-northd.8.xml |   16 ++
 northd/ovn-northd.c     |  342 +++++++++++++++++++++++++++++------------------
 tests/ovn-northd.at     |   65 +++++----
 tests/ovn.at            |    8 +
 4 files changed, 260 insertions(+), 171 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 2dc4e20..5643317 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -8054,7 +8054,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;
@@ -8069,20 +8069,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);
@@ -8101,7 +8105,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)
 {
@@ -8123,21 +8127,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);
@@ -8327,7 +8335,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.
@@ -8477,8 +8519,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. */
@@ -8496,7 +8538,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) {
@@ -8508,108 +8550,12 @@ 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);
         sset_destroy(&all_ips_v6);
 
-        /* A gateway router can have 2 SNAT IP addresses to force DNATed and
-         * LBed traffic respectively to be SNATed.  In addition, there can be
-         * a number of SNAT rules in the NAT table. */
-        struct v46_ip *snat_ips = xmalloc(sizeof *snat_ips
-                                          * (op->od->nbr->n_nat + 2));
-        size_t n_snat_ips = 0;
-
-        struct v46_ip snat_ip;
-        const char *dnat_force_snat_ip = get_force_snat_ip(op->od, "dnat",
-                                                           &snat_ip);
-        if (dnat_force_snat_ip) {
-            snat_ips[n_snat_ips++] = snat_ip;
-        }
-
-        const char *lb_force_snat_ip = get_force_snat_ip(op->od, "lb",
-                                                         &snat_ip);
-        if (lb_force_snat_ip) {
-            snat_ips[n_snat_ips++] = snat_ip;
-        }
-
-        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;
-
-            /* Skip entries we failed to parse. */
-            if (!nat_entry_is_valid(nat_entry)) {
-                continue;
-            }
-
-            if (!strcmp(nat->type, "snat")) {
-                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;
-                } 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;
-                }
-                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. */
-            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 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,
-                                      lflows, &nat->header_);
-            } else {
-                build_lrouter_arp_flow(op->od, op,
-                                       ext_addrs->ipv4_addrs[0].addr_s,
-                                       mac_s, &match, 90,
-                                       lflows, &nat->header_);
-            }
-        }
-
         if (!smap_get(&op->od->nbr->options, "chassis")
             && !op->od->l3dgw_port) {
             /* UDP/TCP port unreachable. */
@@ -8658,6 +8604,50 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             }
         }
 
+        /* A gateway router can have 2 SNAT IP addresses to force DNATed and
+         * LBed traffic respectively to be SNATed.  In addition, there can be
+         * a number of SNAT rules in the NAT table. */
+        struct v46_ip *snat_ips = xmalloc(sizeof *snat_ips
+                                          * (op->od->nbr->n_nat + 2));
+        size_t n_snat_ips = 0;
+
+        struct v46_ip snat_ip;
+        const char *dnat_force_snat_ip = get_force_snat_ip(op->od, "dnat",
+                                                           &snat_ip);
+        if (dnat_force_snat_ip) {
+            snat_ips[n_snat_ips++] = snat_ip;
+        }
+
+        const char *lb_force_snat_ip = get_force_snat_ip(op->od, "lb",
+                                                         &snat_ip);
+        if (lb_force_snat_ip) {
+            snat_ips[n_snat_ips++] = snat_ip;
+        }
+
+        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;
+
+            /* Skip entries we failed to parse. */
+            if (!nat_entry_is_valid(nat_entry)) {
+                continue;
+            }
+
+            if (!strcmp(nat->type, "snat")) {
+                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;
+                } 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;
+                }
+            }
+        }
+
         ds_clear(&match);
         ds_put_cstr(&match, "ip4.dst == {");
         bool has_drop_ips = false;
@@ -8719,6 +8709,90 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
         }
 
         free(snat_ips);
+
+        /* 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;
+        }
+
+        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;
+
+            /* Skip entries we failed to parse. */
+            if (!nat_entry_is_valid(nat_entry)) {
+                continue;
+            }
+
+            if (!strcmp(nat->type, "snat")) {
+                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. */
+
+            struct eth_addr mac;
+
+            ds_clear(&match);
+            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);
+                }
+            }
+
+            /* 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, 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, NULL, true, 91,
+                                      lflows, &nat->header_);
+            } else {
+                build_lrouter_arp_flow(op->od, op,
+                                       ext_addrs->ipv4_addrs[0].addr_s,
+                                       mac_s, &match, false, 92,
+                                       lflows, &nat->header_);
+                build_lrouter_arp_flow(op->od, op,
+                                       ext_addrs->ipv4_addrs[0].addr_s,
+                                       mac_s, NULL, true, 91,
+                                       lflows, &nat->header_);
+            }
+        }
     }
 
     /* DHCPv6 reply handling */
@@ -8793,8 +8867,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..5693f84 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), 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), 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), 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 1a1cfbf..9522b55 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -19336,7 +19336,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([
@@ -19356,7 +19356,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([
@@ -19400,7 +19400,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([
@@ -19422,7 +19422,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([