[ovs-dev] [PATCH v5 1/2 ovn] Fix the data type for DHCP option tftp_server (66)

Numan Siddique numans at ovn.org
Wed Jun 17 09:09:48 UTC 2020


On Fri, Jun 12, 2020 at 2:14 AM Ankur Sharma <svc.mail.git at nutanix.com>
wrote:

> From: Dhathri Purohith <svc.eng.git-patch at nutanix.com>
>
> From: Dhathri Purohith <dhathri.purohith at nutanix.com>
>
> 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 at nutanix.com>
> Signed-off-by: Ankur Sharma <ankur.sharma at nutanix.com>
>
>
Thanks. I applied this patch to master.

Numan


> ---
>  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 c506151..f21be6d 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -1978,6 +1978,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.",
> @@ -2305,6 +2309,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 cc63d82..38555db 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 eb78f31..ef08414 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -11360,7 +11360,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 acf5648..54f5301 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -2790,12 +2790,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.
> @@ -2944,6 +2938,18 @@
>            </p>
>          </column>
>        </group>
> +
> +      <group title="Host_id DHCP Options">
> +        <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 c196dda..2ec729b 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 15204a8..208fb93 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -3219,6 +3219,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 8743e0e..d3b35ac 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1212,6 +1212,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.
> @@ -5546,6 +5552,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 a77d2f1..4391694 100644
> --- a/tests/test-ovn.c
> +++ b/tests/test-ovn.c
> @@ -174,7 +174,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");
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>


More information about the dev mailing list