Blob Blame History Raw
From fe0be84c42c1b304bfbe49e59aa11eea100e16b1 Mon Sep 17 00:00:00 2001
From: Dhathri Purohith <dhathri.purohith@nutanix.com>
Date: Thu, 11 Jun 2020 13:44:40 -0700
Subject: [PATCH 01/22] Fix the data type for DHCP option tftp_server (66)

Currently, DHCP option is of type ipv4. But, according to RFC 2132,
option 66 can be a hostname i.e, we should also accept string type.
In order to be backward compatible, a new type called "host_id" is
introduced, which accepts both ipv4 address and string. Type for DHCP
option 66 is changed to "host_id" instead of ipv4.
OVN northd code that updates the OVN southbound database is enhanced to
consider the change in the type and code for DHCP option, so that the
change in datatype is reflected.

Signed-off-by: Dhathri Purohith <dhathri.purohith@nutanix.com>
Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com>
Signed-off-by: Numan Siddique <numans@ovn.org>

(cherry-picked from upstream master commit b06319993debbeb6d116901afa511d627543c10d)

Change-Id: I45b4d783bfc8849a69d7c8c8584429c2740e668c
---
 lib/actions.c       | 12 ++++++++
 lib/ovn-l7.h        |  2 +-
 northd/ovn-northd.c |  7 ++++-
 ovn-nb.xml          | 18 ++++++++----
 ovn-sb.ovsschema    |  7 +++--
 ovn-sb.xml          | 13 +++++++++
 tests/ovn.at        | 68 +++++++++++++++++++++++++++++++++++++++++++++
 tests/test-ovn.c    |  2 +-
 8 files changed, 117 insertions(+), 12 deletions(-)

diff --git a/lib/actions.c b/lib/actions.c
index 6d0d687b3..616c93e8a 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -2083,6 +2083,10 @@ parse_gen_opt(struct action_context *ctx, struct ovnact_gen_option *o,
         return;
     }
 
+    if (!strcmp(o->option->type, "host_id")) {
+        return;
+    }
+
     if (!strcmp(o->option->type, "str")) {
         if (o->value.type != EXPR_C_STRING) {
             lexer_error(ctx->lexer, "%s option %s requires string value.",
@@ -2410,6 +2414,14 @@ encode_put_dhcpv4_option(const struct ovnact_gen_option *o,
     } else if (!strcmp(o->option->type, "str")) {
         opt_header[1] = strlen(c->string);
         ofpbuf_put(ofpacts, c->string, opt_header[1]);
+    } else if (!strcmp(o->option->type, "host_id")) {
+        if (o->value.type == EXPR_C_STRING) {
+            opt_header[1] = strlen(c->string);
+            ofpbuf_put(ofpacts, c->string, opt_header[1]);
+        } else {
+           opt_header[1] = sizeof(ovs_be32);
+           ofpbuf_put(ofpacts, &c->value.ipv4, sizeof(ovs_be32));
+        }
     }
 }
 
diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
index d5c6feaeb..22a2153de 100644
--- a/lib/ovn-l7.h
+++ b/lib/ovn-l7.h
@@ -57,7 +57,7 @@ struct gen_opts_map {
 #define DHCP_OPT_NIS_SERVER  DHCP_OPTION("nis_server", 41, "ipv4")
 #define DHCP_OPT_NTP_SERVER  DHCP_OPTION("ntp_server", 42, "ipv4")
 #define DHCP_OPT_SERVER_ID   DHCP_OPTION("server_id", 54, "ipv4")
-#define DHCP_OPT_TFTP_SERVER DHCP_OPTION("tftp_server", 66, "ipv4")
+#define DHCP_OPT_TFTP_SERVER DHCP_OPTION("tftp_server", 66, "host_id")
 
 #define DHCP_OPT_CLASSLESS_STATIC_ROUTE \
     DHCP_OPTION("classless_static_route", 121, "static_routes")
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 6858bf8fd..14be87435 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -11785,7 +11785,12 @@ check_and_add_supported_dhcp_opts_to_sb_db(struct northd_context *ctx)
         struct gen_opts_map *dhcp_opt =
             dhcp_opts_find(&dhcp_opts_to_add, opt_row->name);
         if (dhcp_opt) {
-            hmap_remove(&dhcp_opts_to_add, &dhcp_opt->hmap_node);
+            if (!strcmp(dhcp_opt->type, opt_row->type) &&
+                 dhcp_opt->code == opt_row->code) {
+                hmap_remove(&dhcp_opts_to_add, &dhcp_opt->hmap_node);
+            } else {
+                sbrec_dhcp_options_delete(opt_row);
+            }
         } else {
             sbrec_dhcp_options_delete(opt_row);
         }
diff --git a/ovn-nb.xml b/ovn-nb.xml
index e947c440d..8d04d3d3b 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -2830,12 +2830,6 @@
           </p>
         </column>
 
-        <column name="options" key="tftp_server">
-          <p>
-            The DHCPv4 option code for this option is 66.
-          </p>
-        </column>
-
         <column name="options" key="classless_static_route">
           <p>
             The DHCPv4 option code for this option is 121.
@@ -2984,6 +2978,18 @@
           </p>
         </column>
       </group>
+
+      <group title="DHCP Options of type host_id">
+        <p>
+          These options accept either an IPv4 address or a string value.
+        </p>
+
+        <column name="options" key="tftp_server">
+          <p>
+            The DHCPv4 option code for this option is 66.
+          </p>
+        </column>
+      </group>
     </group>
 
     <group title="DHCPv6 options">
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index c196ddaf3..2ec729b77 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
 {
     "name": "OVN_Southbound",
-    "version": "2.8.0",
-    "cksum": "1643994484 21853",
+    "version": "2.8.1",
+    "cksum": "236203406 21905",
     "tables": {
         "SB_Global": {
             "columns": {
@@ -217,7 +217,8 @@
                     "type": {"key": {
                         "type": "string",
                         "enum": ["set", ["bool", "uint8", "uint16", "uint32",
-                                         "ipv4", "static_routes", "str"]]}}}},
+                                         "ipv4", "static_routes", "str",
+                                         "host_id"]]}}}},
             "isRoot": true},
         "DHCPv6_Options": {
             "columns": {
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 0641e4942..2edafd48f 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -3274,6 +3274,19 @@ tcp.flags = RST;
             Example. "name=host_name", "code=12", "type=str".
           </p>
         </dd>
+
+        <dt><code>value: host_id</code></dt>
+        <dd>
+          <p>
+            This indicates that the value of the DHCP option is a host_id.
+            It can either be a host_name or an IP address.
+          </p>
+
+          <p>
+            Example. "name=tftp_server", "code=66", "type=host_id".
+          </p>
+        </dd>
+
       </dl>
     </column>
   </table>
diff --git a/tests/ovn.at b/tests/ovn.at
index f6adbb7a3..4e98790af 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1249,6 +1249,12 @@ reg2[5] = put_dhcp_opts(offerip=10.0.0.4,router=10.0.0.1,netmask=255.255.254.0,m
 reg0[15] = put_dhcp_opts(offerip=10.0.0.4,router=10.0.0.1,netmask=255.255.255.0,mtu=1400,ip_forward_enable=1,default_ttl=121,dns_server={8.8.8.8,7.7.7.7},classless_static_route={30.0.0.0/24,10.0.0.4,40.0.0.0/16,10.0.0.6,0.0.0.0/0,10.0.0.1},ethernet_encap=1,router_discovery=0,tftp_server_address={10.0.0.4,10.0.0.5},arp_cache_timeout=10,tcp_keepalive_interval=10);
     formats as reg0[15] = put_dhcp_opts(offerip = 10.0.0.4, router = 10.0.0.1, netmask = 255.255.255.0, mtu = 1400, ip_forward_enable = 1, default_ttl = 121, dns_server = {8.8.8.8, 7.7.7.7}, classless_static_route = {30.0.0.0/24, 10.0.0.4, 40.0.0.0/16, 10.0.0.6, 0.0.0.0/0, 10.0.0.1}, ethernet_encap = 1, router_discovery = 0, tftp_server_address = {10.0.0.4, 10.0.0.5}, arp_cache_timeout = 10, tcp_keepalive_interval = 10);
     encodes as controller(userdata=00.00.00.02.00.00.00.00.00.01.de.10.00.00.00.6f.0a.00.00.04.03.04.0a.00.00.01.01.04.ff.ff.ff.00.1a.02.05.78.13.01.01.17.01.79.06.08.08.08.08.08.07.07.07.07.79.14.18.1e.00.00.0a.00.00.04.10.28.00.0a.00.00.06.00.0a.00.00.01.24.01.01.1f.01.00.96.08.0a.00.00.04.0a.00.00.05.23.04.00.00.00.0a.26.04.00.00.00.0a,pause)
+reg0[15] = put_dhcp_opts(offerip=10.0.0.4,router=10.0.0.1,netmask=255.255.255.0,mtu=1400,ip_forward_enable=1,default_ttl=121,dns_server={8.8.8.8,7.7.7.7},classless_static_route={30.0.0.0/24,10.0.0.4,40.0.0.0/16,10.0.0.6,0.0.0.0/0,10.0.0.1},ethernet_encap=1,router_discovery=0,tftp_server=10.0.0.10);
+    formats as reg0[15] = put_dhcp_opts(offerip = 10.0.0.4, router = 10.0.0.1, netmask = 255.255.255.0, mtu = 1400, ip_forward_enable = 1, default_ttl = 121, dns_server = {8.8.8.8, 7.7.7.7}, classless_static_route = {30.0.0.0/24, 10.0.0.4, 40.0.0.0/16, 10.0.0.6, 0.0.0.0/0, 10.0.0.1}, ethernet_encap = 1, router_discovery = 0, tftp_server = 10.0.0.10);
+    encodes as controller(userdata=00.00.00.02.00.00.00.00.00.01.de.10.00.00.00.6f.0a.00.00.04.03.04.0a.00.00.01.01.04.ff.ff.ff.00.1a.02.05.78.13.01.01.17.01.79.06.08.08.08.08.08.07.07.07.07.79.14.18.1e.00.00.0a.00.00.04.10.28.00.0a.00.00.06.00.0a.00.00.01.24.01.01.1f.01.00.42.04.0a.00.00.0a,pause)
+reg0[15] = put_dhcp_opts(offerip=10.0.0.4,router=10.0.0.1,netmask=255.255.255.0,mtu=1400,ip_forward_enable=1,default_ttl=121,dns_server={8.8.8.8,7.7.7.7},classless_static_route={30.0.0.0/24,10.0.0.4,40.0.0.0/16,10.0.0.6,0.0.0.0/0,10.0.0.1},ethernet_encap=1,router_discovery=0,tftp_server="tftp_server_test");
+    formats as reg0[15] = put_dhcp_opts(offerip = 10.0.0.4, router = 10.0.0.1, netmask = 255.255.255.0, mtu = 1400, ip_forward_enable = 1, default_ttl = 121, dns_server = {8.8.8.8, 7.7.7.7}, classless_static_route = {30.0.0.0/24, 10.0.0.4, 40.0.0.0/16, 10.0.0.6, 0.0.0.0/0, 10.0.0.1}, ethernet_encap = 1, router_discovery = 0, tftp_server = "tftp_server_test");
+    encodes as controller(userdata=00.00.00.02.00.00.00.00.00.01.de.10.00.00.00.6f.0a.00.00.04.03.04.0a.00.00.01.01.04.ff.ff.ff.00.1a.02.05.78.13.01.01.17.01.79.06.08.08.08.08.08.07.07.07.07.79.14.18.1e.00.00.0a.00.00.04.10.28.00.0a.00.00.06.00.0a.00.00.01.24.01.01.1f.01.00.42.10.74.66.74.70.5f.73.65.72.76.65.72.5f.74.65.73.74,pause)
 
 reg1[0..1] = put_dhcp_opts(offerip = 1.2.3.4, router = 10.0.0.1);
     Cannot use 2-bit field reg1[0..1] where 1-bit field is required.
@@ -5624,6 +5630,68 @@ AT_CHECK([cat 1.packets | cut -c -48], [0], [expout])
 cat 1.expected | cut -c 53- > expout
 AT_CHECK([cat 1.packets | cut -c 53-], [0], [expout])
 
+reset_pcap_file hv1-vif1 hv1/vif1
+reset_pcap_file hv1-vif2 hv1/vif2
+rm -f 1.expected
+rm -f 2.expected
+
+# Set tftp server option (IPv4 address) for ls1
+echo "------ Set tftp server (IPv4 address) --------"
+ovn-nbctl dhcp-options-set-options $d1 server_id=10.0.0.1 \
+server_mac=ff:10:00:00:00:01 lease_time=3600 router=10.0.0.1 \
+tftp_server=10.10.10.10
+echo "----------------------------------------------"
+
+# Send DHCPREQUEST in the SELECTING/INIT-REBOOT state with the offered IP
+# address in the Requested IP Address option.
+offer_ip=`ip_to_hex 10 0 0 6`
+server_ip=`ip_to_hex 10 0 0 1`
+ciaddr=`ip_to_hex 0 0 0 0`
+request_ip=$offer_ip
+expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a00000142040a0a0a0a
+test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 05 $expected_dhcp_opts
+
+# NXT_RESUMEs should be 10.
+OVS_WAIT_UNTIL([test 10 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
+
+$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
+cat 2.expected | cut -c -48 > expout
+AT_CHECK([cat 2.packets | cut -c -48], [0], [expout])
+# Skipping the IPv4 checksum.
+cat 2.expected | cut -c 53- > expout
+AT_CHECK([cat 2.packets | cut -c 53-], [0], [expout])
+
+reset_pcap_file hv1-vif1 hv1/vif1
+reset_pcap_file hv1-vif2 hv1/vif2
+rm -f 1.expected
+rm -f 2.expected
+
+# Set tftp server option (Hostname) for ls1
+echo "------ Set tftp server (hostname) --------"
+ovn-nbctl dhcp-options-set-options $d1 server_id=10.0.0.1 \
+server_mac=ff:10:00:00:00:01 lease_time=3600 router=10.0.0.1 \
+tftp_server=\"test_tftp_server\"
+echo "------------------------------------------"
+
+# Send DHCPREQUEST in the SELECTING/INIT-REBOOT state with the offered IP
+# address in the Requested IP Address option.
+offer_ip=`ip_to_hex 10 0 0 6`
+server_ip=`ip_to_hex 10 0 0 1`
+ciaddr=`ip_to_hex 0 0 0 0`
+request_ip=$offer_ip
+expected_dhcp_opts=330400000e100104ffffff0003040a00000136040a0000014210746573745f746674705f736572766572
+test_dhcp 2 f00000000002 03 0 $ciaddr $offer_ip $request_ip 0 ff1000000001 $server_ip 05 $expected_dhcp_opts
+
+# NXT_RESUMEs should be 11.
+OVS_WAIT_UNTIL([test 11 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
+
+$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
+cat 2.expected | cut -c -48 > expout
+AT_CHECK([cat 2.packets | cut -c -48], [0], [expout])
+# Skipping the IPv4 checksum.
+cat 2.expected | cut -c 53- > expout
+AT_CHECK([cat 2.packets | cut -c 53-], [0], [expout])
+
 OVN_CLEANUP([hv1])
 
 AT_CLEANUP
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index 8f1bb7e01..29d343b60 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -175,7 +175,7 @@ create_gen_opts(struct hmap *dhcp_opts, struct hmap *dhcpv6_opts,
     dhcp_opt_add(dhcp_opts, "nis_server", 41, "ipv4");
     dhcp_opt_add(dhcp_opts, "ntp_server", 42, "ipv4");
     dhcp_opt_add(dhcp_opts, "server_id",  54, "ipv4");
-    dhcp_opt_add(dhcp_opts, "tftp_server", 66, "ipv4");
+    dhcp_opt_add(dhcp_opts, "tftp_server", 66, "host_id");
     dhcp_opt_add(dhcp_opts, "classless_static_route", 121, "static_routes");
     dhcp_opt_add(dhcp_opts, "ip_forward_enable",  19, "bool");
     dhcp_opt_add(dhcp_opts, "router_discovery", 31, "bool");
-- 
2.26.2