From patchwork Fri Nov 24 15:56:17 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1868217 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4ScKNb0CYCz1ySj for ; Sat, 25 Nov 2023 02:56:46 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id EE02041A66; Fri, 24 Nov 2023 15:56:44 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org EE02041A66 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 9RwuicphB47m; Fri, 24 Nov 2023 15:56:43 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id 67520404FF; Fri, 24 Nov 2023 15:56:42 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 67520404FF Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3B603C0072; Fri, 24 Nov 2023 15:56:42 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 7AE01C0039 for ; Fri, 24 Nov 2023 15:56:41 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 51191404FF for ; Fri, 24 Nov 2023 15:56:41 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 51191404FF X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id FOFAjr3xB96a for ; Fri, 24 Nov 2023 15:56:39 +0000 (UTC) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::221]) by smtp2.osuosl.org (Postfix) with ESMTPS id 38BA04032C for ; Fri, 24 Nov 2023 15:56:38 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 38BA04032C Received: by mail.gandi.net (Postfix) with ESMTPSA id 32506240003; Fri, 24 Nov 2023 15:56:34 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Fri, 24 Nov 2023 10:56:17 -0500 Message-ID: <20231124155617.3615988-1-numans@ovn.org> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 X-GND-Sasl: numans@ovn.org Cc: Dumitru Ceara Subject: [ovs-dev] [PATCH ovn] Reduce number of DHCP responder flows for LSPs 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" From: Jacob Tanenbaum Currently for every LSP two DHCP responder flows are created. These two flows are almost exactly the same differing only in the match. ex. Unicast flow (for RENEW/REBIND): "match":"inport == \"0e4b7821-b5ae-4bff-9424-d7245c679150\" && eth.src == fa:16:3e:68:5e:c1 && ip4.src == 1.65.5.169 && ip4.dst == {1.65.5.1, 255.255.255.255} && udp.src == 68 && udp.dst == 67" Broadcast flow (for DISCOVER): "match":"inport == \"0e4b7821-b5ae-4bff-9424-d7245c679150\" && eth.src == fa:16:3e:68:5e:c1 && ip4.src == 0.0.0.0 && ip4.dst == 255.255.255.255 && udp.src == 68 && udp.dst == 67" I consolidated the flows to use the conjunctive match (ip4.src == {1.65.5.169, 0.0.0.0} && ip4.dst == {1.65.5.1, 255.255.255.255}) there is potential for a faulty DHCP discovery and DHCP Request packet getting through - added a check in pinctrl.c to check for illegal dhcp discovery packet sending to host Submitted-at: https://github.com/ovn-org/ovn/pull/224 Signed-off-by: Jacob Tanenbaum Acked-by: Dumitru Ceara --- controller/pinctrl.c | 5 +++++ northd/northd.c | 23 ++--------------------- tests/ovn-northd.at | 9 +++------ tests/ovn.at | 4 ++-- 4 files changed, 12 insertions(+), 29 deletions(-) diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 62cf4da324..2f2f945b71 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -2039,6 +2039,11 @@ pinctrl_handle_put_dhcp_opts( switch (*in_dhcp_msg_type) { case DHCP_MSG_DISCOVER: msg_type = DHCP_MSG_OFFER; + if (in_flow->nw_dst != htonl(INADDR_BROADCAST)) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); + VLOG_WARN_RL(&rl, "DHCP DISCOVER must be Broadcast"); + goto exit; + } break; case DHCP_MSG_REQUEST: { msg_type = DHCP_MSG_ACK; diff --git a/northd/northd.c b/northd/northd.c index e93d0c8f82..d0fe1929fc 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -6805,7 +6805,8 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip, server_mac, server_ip); ds_put_format(ipv4_addr_match, - "ip4.src == "IP_FMT" && ip4.dst == {%s, 255.255.255.255}", + "(ip4.src == {"IP_FMT", 0.0.0.0} " + "&& ip4.dst == {%s, 255.255.255.255})", IP_ARGS(offer_ip), server_ip); smap_destroy(&dhcpv4_options); return true; @@ -9438,27 +9439,7 @@ build_dhcpv4_options_flows(struct ovn_port *op, op, lsp_addrs->ipv4_addrs[j].addr, &options_action, &response_action, &ipv4_addr_match)) { ds_clear(&match); - ds_put_format( - &match, "inport == %s && eth.src == %s && " - "ip4.src == 0.0.0.0 && ip4.dst == 255.255.255.255 && " - "udp.src == 68 && udp.dst == 67", - inport->json_key, lsp_addrs->ea_s); - if (is_external) { - ds_put_format(&match, " && is_chassis_resident(%s)", - op->json_key); - } - - ovn_lflow_add_with_hint__(lflows, op->od, - S_SWITCH_IN_DHCP_OPTIONS, 100, - ds_cstr(&match), - ds_cstr(&options_action), - inport->key, - copp_meter_get(COPP_DHCPV4_OPTS, - op->od->nbs->copp, - meter_groups), - &op->nbsp->dhcpv4_options->header_); - ds_clear(&match); /* Allow ip4.src = OFFER_IP and * ip4.dst = {SERVER_IP, 255.255.255.255} for the below * cases diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 66ea495e5d..b0e464e77c 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -4768,8 +4768,7 @@ AT_CAPTURE_FILE([sw0flows]) AT_CHECK([grep -w "ls_in_dhcp_options" sw0flows | sort | sed 's/table=../table=??/'], [0], [dnl table=??(ls_in_dhcp_options ), priority=0 , match=(1), action=(next;) - table=??(ls_in_dhcp_options ), priority=100 , match=(inport == "sw0-port1" && eth.src == 50:54:00:00:00:01 && ip4.src == 0.0.0.0 && ip4.dst == 255.255.255.255 && udp.src == 68 && udp.dst == 67), action=(reg0[[3]] = put_dhcp_opts(offerip = 10.0.0.2, hostname = "foo", lease_time = 3600, netmask = 255.255.255.0, router = 10.0.0.1, server_id = 10.0.0.1); next;) - table=??(ls_in_dhcp_options ), priority=100 , match=(inport == "sw0-port1" && eth.src == 50:54:00:00:00:01 && ip4.src == 10.0.0.2 && ip4.dst == {10.0.0.1, 255.255.255.255} && udp.src == 68 && udp.dst == 67), action=(reg0[[3]] = put_dhcp_opts(offerip = 10.0.0.2, hostname = "foo", lease_time = 3600, netmask = 255.255.255.0, router = 10.0.0.1, server_id = 10.0.0.1); next;) + table=??(ls_in_dhcp_options ), priority=100 , match=(inport == "sw0-port1" && eth.src == 50:54:00:00:00:01 && (ip4.src == {10.0.0.2, 0.0.0.0} && ip4.dst == {10.0.0.1, 255.255.255.255}) && udp.src == 68 && udp.dst == 67), action=(reg0[[3]] = put_dhcp_opts(offerip = 10.0.0.2, hostname = "foo", lease_time = 3600, netmask = 255.255.255.0, router = 10.0.0.1, server_id = 10.0.0.1); next;) ]) check ovn-nbctl --wait=sb lsp-set-options sw0-port1 hostname="\"port1\"" @@ -4778,8 +4777,7 @@ AT_CAPTURE_FILE([sw0flows]) AT_CHECK([grep -w "ls_in_dhcp_options" sw0flows | sort | sed 's/table=../table=??/'], [0], [dnl table=??(ls_in_dhcp_options ), priority=0 , match=(1), action=(next;) - table=??(ls_in_dhcp_options ), priority=100 , match=(inport == "sw0-port1" && eth.src == 50:54:00:00:00:01 && ip4.src == 0.0.0.0 && ip4.dst == 255.255.255.255 && udp.src == 68 && udp.dst == 67), action=(reg0[[3]] = put_dhcp_opts(offerip = 10.0.0.2, hostname = "port1", lease_time = 3600, netmask = 255.255.255.0, router = 10.0.0.1, server_id = 10.0.0.1); next;) - table=??(ls_in_dhcp_options ), priority=100 , match=(inport == "sw0-port1" && eth.src == 50:54:00:00:00:01 && ip4.src == 10.0.0.2 && ip4.dst == {10.0.0.1, 255.255.255.255} && udp.src == 68 && udp.dst == 67), action=(reg0[[3]] = put_dhcp_opts(offerip = 10.0.0.2, hostname = "port1", lease_time = 3600, netmask = 255.255.255.0, router = 10.0.0.1, server_id = 10.0.0.1); next;) + table=??(ls_in_dhcp_options ), priority=100 , match=(inport == "sw0-port1" && eth.src == 50:54:00:00:00:01 && (ip4.src == {10.0.0.2, 0.0.0.0} && ip4.dst == {10.0.0.1, 255.255.255.255}) && udp.src == 68 && udp.dst == 67), action=(reg0[[3]] = put_dhcp_opts(offerip = 10.0.0.2, hostname = "port1", lease_time = 3600, netmask = 255.255.255.0, router = 10.0.0.1, server_id = 10.0.0.1); next;) ]) ovn-nbctl dhcp-options-set-options $CIDR_UUID lease_time=3600 router=10.0.0.1 server_id=10.0.0.1 server_mac=c0:ff:ee:00:00:01 @@ -4789,8 +4787,7 @@ AT_CAPTURE_FILE([sw0flows]) AT_CHECK([grep -w "ls_in_dhcp_options" sw0flows | sort | sed 's/table=../table=??/'], [0], [dnl table=??(ls_in_dhcp_options ), priority=0 , match=(1), action=(next;) - table=??(ls_in_dhcp_options ), priority=100 , match=(inport == "sw0-port1" && eth.src == 50:54:00:00:00:01 && ip4.src == 0.0.0.0 && ip4.dst == 255.255.255.255 && udp.src == 68 && udp.dst == 67), action=(reg0[[3]] = put_dhcp_opts(offerip = 10.0.0.2, hostname = "bar", lease_time = 3600, netmask = 255.255.255.0, router = 10.0.0.1, server_id = 10.0.0.1); next;) - table=??(ls_in_dhcp_options ), priority=100 , match=(inport == "sw0-port1" && eth.src == 50:54:00:00:00:01 && ip4.src == 10.0.0.2 && ip4.dst == {10.0.0.1, 255.255.255.255} && udp.src == 68 && udp.dst == 67), action=(reg0[[3]] = put_dhcp_opts(offerip = 10.0.0.2, hostname = "bar", lease_time = 3600, netmask = 255.255.255.0, router = 10.0.0.1, server_id = 10.0.0.1); next;) + table=??(ls_in_dhcp_options ), priority=100 , match=(inport == "sw0-port1" && eth.src == 50:54:00:00:00:01 && (ip4.src == {10.0.0.2, 0.0.0.0} && ip4.dst == {10.0.0.1, 255.255.255.255}) && udp.src == 68 && udp.dst == 67), action=(reg0[[3]] = put_dhcp_opts(offerip = 10.0.0.2, hostname = "bar", lease_time = 3600, netmask = 255.255.255.0, router = 10.0.0.1, server_id = 10.0.0.1); next;) ]) AT_CLEANUP diff --git a/tests/ovn.at b/tests/ovn.at index 17ad1fb243..b2d69ecccf 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -19639,7 +19639,7 @@ wait_for_ports_up ls1-lp_ext1 as hv1 ovs-ofctl dump-flows br-int > brintflows AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | \ grep controller | grep "0a.00.00.06" | grep reg14=0x$ln_public_key | \ -wc -l], [0], [3 +wc -l], [0], [1 ]) AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | \ grep controller | grep tp_src=546 | grep \ @@ -19891,7 +19891,7 @@ wait_for_ports_up ls1-lp_ext1 # There should be OF flows for DHCP4/v6 for the ls1-lp_ext1 port in hv2 AT_CHECK([as hv2 ovs-ofctl dump-flows br-int | \ grep controller | grep "0a.00.00.06" | grep reg14=0x$ln_public_key | \ -wc -l], [0], [3 +wc -l], [0], [1 ]) AT_CHECK([as hv2 ovs-ofctl dump-flows br-int | \ grep controller | grep tp_src=546 | grep \