[ovs-dev] [PATCH ovn v3] northd: Add support for DHCP Option 12 (Hostname)

Vladislav Odintsov odivlad at gmail.com
Tue Jun 22 13:03:48 UTC 2021


Thanks Numan for adding ddlog support here and additional tests as well.
Okay, I’ll try to do it by myself next time, if any.

Regards,
Vladislav Odintsov

> On 22 Jun 2021, at 13:47, Numan Siddique <numans at ovn.org> wrote:
> 
> On Tue, Jun 15, 2021 at 11:34 AM Vladislav Odintsov <odivlad at gmail.com <mailto:odivlad at gmail.com>> wrote:
>> 
>> DHCP option Hostname (opcode 12) can be configured in
>> Logical_Switch_Port as well as in DHCP_Options table's
>> options:hostname column. If both methods of configuration were
>> used, Logical_Switch_Port's setting takes precedence.
>> 
>> Signed-off-by: Vladislav Odintsov <odivlad at gmail.com <mailto:odivlad at gmail.com>>
> 
> Thanks. I applied to the main branch with the below changes which adds
> ddlog support
> and adds a test case in northd to make sure both the version of
> ovn-northd generates the same logical flows.
> 
> I'd appreciate it if you could try out the ddlog implementation for
> your future patches modifying ovn-northd.
> You can always ask questions if you're finding it hard to add the
> ddlog code to the ML.
> 
> ***************
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index 3afa80a3b5..cd32df0dbf 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -1007,6 +1007,13 @@ sb::Out_DHCP_Options (
>     .__type = "ipv4"
> ).
> 
> +sb::Out_DHCP_Options (
> +    ._uuid  = 128'hae312373_2261_41b5_a2c4_186f426dd929,
> +    .name   = "hostname",
> +    .code   = 12,
> +    .__type = "str"
> +).
> +
> sb::Out_DHCP_Options (
>     ._uuid  = 128'hae35e575_226a_4ab5_a1c4_166f426dd999,
>     .name   = "domain_name",
> @@ -3429,7 +3436,8 @@ Flow(.logical_datapath = sp.sw._uuid,
> function build_dhcpv4_action(
>     lsp_json_key: string,
>     dhcpv4_options: Intern<nb::DHCP_Options>,
> -    offer_ip: in_addr) : Option<(string, string, string)> =
> +    offer_ip: in_addr,
> +    lsp_options: Map<string,string>) : Option<(string, string, string)> =
> {
>     match (ip_parse_masked(dhcpv4_options.cidr)) {
>         Left{err} -> {
> @@ -3454,6 +3462,11 @@ function build_dhcpv4_action(
>                         options_map.remove("server_mac");
>                         options_map.insert("netmask", "${mask}");
> 
> +                        match (lsp_options.get("hostname")) {
> +                            None -> (),
> +                            Some{port_hostname} ->
> options_map.insert("hostname", "${port_hostname}")
> +                        };
> +
>                         /* We're not using SMAP_FOR_EACH because we
> want a consistent order of the
>                          * options on different architectures (big or
> little endian, SSE4.2) */
>                         var options = vec_empty();
> @@ -3616,7 +3629,7 @@ for (lsp in &SwitchPort
>                 for (dhcpv4_options in &nb::DHCP_Options(._uuid =
> dhcpv4_options_uuid)) {
>                     for (SwitchPortIPv4Address(.port =
> &SwitchPort{.lsp = &nb::Logical_Switch_Port{._uuid = lsp.lsp._uuid}},
> .ea = ea, .addr = addr)) {
>                         Some{(var options_action, var
> response_action, var ipv4_addr_match)} =
> -                            build_dhcpv4_action(json_key,
> dhcpv4_options, addr.addr) in
> +                            build_dhcpv4_action(json_key,
> dhcpv4_options, addr.addr, lsp.lsp.options) in
>                         {
>                             var __match =
>                                 pfx ++ "eth.src == ${ea} && "
> diff --git a/tests/ovn-northd.at <http://ovn-northd.at/> b/tests/ovn-northd.at <http://ovn-northd.at/>
> index d81975cb18..0fd84c770a 100644
> --- a/tests/ovn-northd.at <http://ovn-northd.at/>
> +++ b/tests/ovn-northd.at <http://ovn-northd.at/>
> @@ -3737,3 +3737,53 @@ AT_CHECK([ovn-trace --minimal 'inport ==
> "sw1-port1" && eth.src == 50:54:00:00:0
> 
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn -- DHCP options])
> +AT_KEYWORDS([dnat])
> +ovn_start
> +
> +check ovn-nbctl ls-add sw0
> +check ovn-nbctl lsp-add sw0 sw0-port1
> +check ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.2"
> +check ovn-nbctl lsp-add sw0 sw0-port2
> +check ovn-nbctl lsp-set-addresses sw0-port2 "50:54:00:00:00:02 10.0.0.3"
> +
> +ovn-nbctl dhcp-options-create 10.0.0.0/24
> +
> +CIDR_UUID=$(ovn-nbctl --bare --columns=_uuid find dhcp_options
> cidr="10.0.0.0/24")
> +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
> hostname="\"foo\""
> +ovn-nbctl --wait=sb lsp-set-dhcpv4-options sw0-port1 $CIDR_UUID
> +
> +ovn-sbctl dump-flows sw0 > sw0flows
> +AT_CAPTURE_FILE([sw0flows])
> +
> +AT_CHECK([grep -w "ls_in_dhcp_options" sw0flows | sort], [0], [dnl
> +  table=17(ls_in_dhcp_options ), priority=0    , match=(1), action=(next;)
> +  table=17(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=17(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;)
> +])
> +
> +check ovn-nbctl --wait=sb lsp-set-options sw0-port1 hostname="\"port1\""
> +ovn-sbctl dump-flows sw0 > sw0flows
> +AT_CAPTURE_FILE([sw0flows])
> +
> +AT_CHECK([grep -w "ls_in_dhcp_options" sw0flows | sort], [0], [dnl
> +  table=17(ls_in_dhcp_options ), priority=0    , match=(1), action=(next;)
> +  table=17(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=17(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;)
> +])
> +
> +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
> +check ovn-nbctl --wait=sb lsp-set-options sw0-port1 hostname="\"bar\""
> +ovn-sbctl dump-flows sw0 > sw0flows
> +AT_CAPTURE_FILE([sw0flows])
> +
> +AT_CHECK([grep -w "ls_in_dhcp_options" sw0flows | sort], [0], [dnl
> +  table=17(ls_in_dhcp_options ), priority=0    , match=(1), action=(next;)
> +  table=17(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=17(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;)
> +])
> +
> +AT_CLEANUP
> +])
> ***********
> 
> Thanks
> Numan
> 
>> ---
>> v2 -> v3
>> * Fixed style warning.
>> ---
>> v1 -> v2
>> * Added support for configuring hostname dhcp 12 option in DHCP_Options.
>> * Updated ovn-nb manpage.
>> ---
>> lib/ovn-l7.h        |  1 +
>> northd/ovn-northd.c |  9 +++++++++
>> ovn-nb.xml          | 23 +++++++++++++++++++++++
>> tests/ovn.at        |  3 +++
>> tests/test-ovn.c    |  1 +
>> 5 files changed, 37 insertions(+)
>> 
>> diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
>> index 5e33d619c..9a33f5cda 100644
>> --- a/lib/ovn-l7.h
>> +++ b/lib/ovn-l7.h
>> @@ -81,6 +81,7 @@ struct gen_opts_map {
>> #define DHCP_OPT_DNS_SERVER  DHCP_OPTION("dns_server", 6, "ipv4")
>> #define DHCP_OPT_LOG_SERVER  DHCP_OPTION("log_server", 7, "ipv4")
>> #define DHCP_OPT_LPR_SERVER  DHCP_OPTION("lpr_server", 9, "ipv4")
>> +#define DHCP_OPT_HOSTNAME    DHCP_OPTION("hostname", 12, "str")
>> #define DHCP_OPT_DOMAIN_NAME DHCP_OPTION("domain_name", 15, "str")
>> #define DHCP_OPT_SWAP_SERVER DHCP_OPTION("swap_server", 16, "ipv4")
>> 
>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>> index c39d451ec..45486daac 100644
>> --- a/northd/ovn-northd.c
>> +++ b/northd/ovn-northd.c
>> @@ -4574,6 +4574,14 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip,
>>                   REGBIT_DHCP_OPTS_RESULT" = put_dhcp_opts(offerip = "
>>                   IP_FMT", ", IP_ARGS(offer_ip));
>> 
>> +    /* Try to get hostname DHCP option from ovn_port as it can be passed there
>> +     * instead of DHCP_Options set. Logical_Switch_Port options:hostname takes
>> +     precedence over DHCP_Options options:hostname. */
>> +    const char *hostname = smap_get(&op->nbsp->options, "hostname");
>> +    if (hostname) {
>> +        smap_replace(&dhcpv4_options, "hostname", hostname);
>> +    }
>> +
>>     /* We're not using SMAP_FOR_EACH because we want a consistent order of the
>>      * options on different architectures (big or little endian, SSE4.2) */
>>     const struct smap_node **sorted_opts = smap_sort(&dhcpv4_options);
>> @@ -13561,6 +13569,7 @@ static struct gen_opts_map supported_dhcp_opts[] = {
>>     DHCP_OPT_BOOTFILE,
>>     DHCP_OPT_PATH_PREFIX,
>>     DHCP_OPT_TFTP_SERVER_ADDRESS,
>> +    DHCP_OPT_HOSTNAME,
>>     DHCP_OPT_DOMAIN_NAME,
>>     DHCP_OPT_ARP_CACHE_TIMEOUT,
>>     DHCP_OPT_TCP_KEEPALIVE_INTERVAL,
>> diff --git a/ovn-nb.xml b/ovn-nb.xml
>> index cf1e3aac4..10cb5c8d4 100644
>> --- a/ovn-nb.xml
>> +++ b/ovn-nb.xml
>> @@ -929,6 +929,15 @@
>>           If set, indicates the maximum burst size for data sent from this
>>           interface, in bits.
>>         </column>
>> +
>> +        <column name="options" key="hostname">
>> +          <p>
>> +            If set, indicates the DHCPv4 option "Hostname" (option code 12)
>> +            associated for this Logical Switch Port. If DHCPv4 is enabled for
>> +            this Logical Switch Port, hostname dhcp option will be included in
>> +            DHCP reply.
>> +          </p>
>> +        </column>
>>       </group>
>> 
>>       <group title="Virtual port Options">
>> @@ -3297,6 +3306,20 @@
>>           </p>
>>         </column>
>> 
>> +        <column name="options" key="hostname">
>> +          <p>
>> +            The DHCPv4 option code for this option is 12.
>> +            If set, indicates the DHCPv4 option "Hostname".
>> +            Alternatively, this option can be configured in
>> +            <ref column="options:hostname" table="Logical_Switch_Port"
>> +            db="OVN_NB"/> column in table <ref table="Logical_Switch_Port"/>.
>> +            If Hostname option value is set in both conflicting
>> +            <ref table="Logical_Switch_Port"/> and
>> +            <ref table="DHCP_Options"/> tables,
>> +            <ref table="Logical_Switch_Port"/> takes precedence.
>> +          </p>
>> +        </column>
>> +
>>         <column name="options" key="domain_name">
>>           <p>
>>             The DHCPv4 option code for this option is 15. This option
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index 71d2bab4d..757e15972 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -1345,6 +1345,9 @@ reg2[5] = put_dhcp_opts(offerip=10.0.0.4,router=10.0.0.1,netmask=255.255.254.0,m
>> reg2[5] = put_dhcp_opts(offerip=10.0.0.4,router=10.0.0.1,netmask=255.255.254.0,mtu=1400,domain_name="ovn.org",wpad="https://example.org",bootfile_name="https://127.0.0.1/boot.ipxe",path_prefix="/tftpboot",domain_search_list="ovn.org,abc.ovn.org,def.ovn.org,ovn.test,def.ovn.test,test.org,abc.com");
>>     formats as reg2[5] = put_dhcp_opts(offerip = 10.0.0.4, router = 10.0.0.1, netmask = 255.255.254.0, mtu = 1400, domain_name = "ovn.org", wpad = "https://example.org", bootfile_name = "https://127.0.0.1/boot.ipxe", path_prefix = "/tftpboot", domain_search_list = "ovn.org,abc.ovn.org,def.ovn.org,ovn.test,def.ovn.test,test.org,abc.com");
>>     encodes as controller(userdata=00.00.00.02.00.00.00.00.00.01.de.10.00.00.00.25.0a.00.00.04.43.1b.68.74.74.70.73.3a.2f.2f.31.32.37.2e.30.2e.30.2e.31.2f.62.6f.6f.74.2e.69.70.78.65.03.04.0a.00.00.01.01.04.ff.ff.fe.00.1a.02.05.78.0f.07.6f.76.6e.2e.6f.72.67.fc.13.68.74.74.70.73.3a.2f.2f.65.78.61.6d.70.6c.65.2e.6f.72.67.d2.09.2f.74.66.74.70.62.6f.6f.74.77.35.03.6f.76.6e.03.6f.72.67.00.03.61.62.63.c0.00.03.64.65.66.c0.00.03.6f.76.6e.04.74.65.73.74.00.03.64.65.66.c0.15.04.74.65.73.74.c0.04.03.61.62.63.03.63.6f.6d.00,pause)
>> +reg2[5] = put_dhcp_opts(offerip=10.0.0.4,router=10.0.0.1,netmask=255.255.254.0,mtu=1400,domain_name="ovn.org",hostname="ip-10-0-0-4");
>> +    formats as reg2[5] = put_dhcp_opts(offerip = 10.0.0.4, router = 10.0.0.1, netmask = 255.255.254.0, mtu = 1400, domain_name = "ovn.org", hostname = "ip-10-0-0-4");
>> +    encodes as controller(userdata=00.00.00.02.00.00.00.00.00.01.de.10.00.00.00.25.0a.00.00.04.03.04.0a.00.00.01.01.04.ff.ff.fe.00.1a.02.05.78.0f.07.6f.76.6e.2e.6f.72.67.0c.0b.69.70.2d.31.30.2d.30.2d.30.2d.34,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 98cc2c503..a4701b4cb 100644
>> --- a/tests/test-ovn.c
>> +++ b/tests/test-ovn.c
>> @@ -168,6 +168,7 @@ create_gen_opts(struct hmap *dhcp_opts, struct hmap *dhcpv6_opts,
>>     dhcp_opt_add(dhcp_opts, "dns_server", 6, "ipv4");
>>     dhcp_opt_add(dhcp_opts, "log_server", 7, "ipv4");
>>     dhcp_opt_add(dhcp_opts, "lpr_server",  9, "ipv4");
>> +    dhcp_opt_add(dhcp_opts, "hostname", 12, "str");
>>     dhcp_opt_add(dhcp_opts, "domain_name", 15, "str");
>>     dhcp_opt_add(dhcp_opts, "swap_server", 16, "ipv4");
>>     dhcp_opt_add(dhcp_opts, "policy_filter", 21, "ipv4");
>> --
>> 2.30.0
>> 
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org <mailto:dev at openvswitch.org>
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
>> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org <mailto:dev at openvswitch.org>
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>


More information about the dev mailing list