Hi Numan, Thanks for review.
I’ve been thinking about such approach to set hostnames via DHCP_Options. In my opinion it can be an overkill to have a special DHCP_Options record per each logical switch port in case where we want to return hostnames in DHCP Reply. I understand hostname as a special per-LSP specific DHCP option (maybe even mostly unique for project/tenant). So in such behaviour where hostname is in DHCP_Options instead of LSP, if one wants to return hostnames in DHCP, he/she would always have many DHCP_Options records with same content which differs only in one: hostname. For CMS to manage reduced count of DHCP_Options (comparing to per-LSP DHCP_Options records) looks better too. I don’t have statistics about common DHCP usage pattern in OVN, but in our deployment we have one DHCP_Options record for each L3 subnet. Updating DHCP settings for this subnet is very comfortable just modifying options in one associated record. And if we want just to add hostnames to this scheme, right away we have to maintain separate DHCP_Options record for each LSP and each time we want to update, for instance domain_name, we have to rewrite tens of DHCP_Options records, which differ only in hostnames. Let me know your thoughts on this, please. Regards, Vladislav Odintsov > On 14 Jun 2021, at 21:29, Numan Siddique <num...@ovn.org> wrote: > > On Fri, May 28, 2021 at 7:32 AM Vladislav Odintsov <odiv...@gmail.com > <mailto:odiv...@gmail.com>> wrote: >> >> DHCP Option Hostname is a per-Logical_Switch_Port property, configured in >> Logical_Switch_Port's options:hostname field. It is used if DHCPv4 is >> enabled for this LSP. >> >> Signed-off-by: Vladislav Odintsov <odiv...@gmail.com >> <mailto:odiv...@gmail.com>> > > Thanks for the patch. Each Logical switch port has a column - dhcp_options. > > Openstack neutron when using the native OVN DHCP feature creates a > dhcp_options row > in the DHCP_option table and associates this with the logical ports. > In case if there are > neutron port specific dhcp options which need to be overridden, > neutron creates an additional row in DHCP_Options table and > associates that row to the specific logical port. I think the same > can be done here instead of setting the hostname in the options > of logical switch port. > > Can you also please update the documentation in ovn-nb.xml about the > new supported option in the DHCP_Options table section. > > Thanks > Numan > > > > > >> --- >> The implementation for ovn-northd-ddlog is absent, it needs help from >> somebody, who's familiar with ddlog. >> >> lib/ovn-l7.h | 1 + >> northd/ovn-northd.c | 9 +++++++++ >> ovn-nb.xml | 9 +++++++++ >> tests/ovn.at | 3 +++ >> tests/test-ovn.c | 1 + >> 5 files changed, 23 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..e43c6d91a 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)); >> >> + /* hostname is a per Logical_Switch_Port dhcp option, >> + * so try to get it from ovn_port and put it to options_action >> + * if it exists. */ >> + const char *hostname = smap_get(&op->nbsp->options, "hostname"); >> + if (hostname) { >> + ds_put_format(options_action, "%s = %s, ", "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..404b15608 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"> >> 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 >> d...@openvswitch.org <mailto:d...@openvswitch.org> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> >> > _______________________________________________ > dev mailing list > d...@openvswitch.org <mailto:d...@openvswitch.org> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
