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

Dhathri Purohith dhathri.purohith at nutanix.com
Thu Jun 4 23:04:45 UTC 2020


Hi Numan,

We have fixed the schema checksum error and split the patch into two.
Please take a look and let us know if you have any further comments.

Thanks,
Dhathri

On 6/1/20, 11:14 AM, "svc.mail.git" <svc.mail.git at nutanix.com> wrote:

    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>

    ---
     lib/actions.c       | 12 ++++++++++++
     lib/ovn-l7.h        |  2 +-
     northd/ovn-northd.c |  7 ++++++-
     ovn-sb.ovsschema    |  7 ++++---
     ovn-sb.xml          | 13 +++++++++++++
     tests/ovn.at        |  6 ++++++
     tests/test-ovn.c    |  2 +-
     7 files changed, 43 insertions(+), 6 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-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 15b40ca..fa2592c 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.
    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




More information about the dev mailing list