On Mon, Aug 22, 2022 at 3:46 PM Mark Michelson <[email protected]> wrote:
>
> From: Lucas Alvares Gomes <[email protected]>
>
> In order to PXE boot a baremetal server using the OVN DHCP server we
> need to allow users to set the "next-server" (siaddr) [0] field in the
> DHCP header.
>
> While investigating this issue by comparing the DHCPOFFER and DHCPACK
> packets sent my dnsmasq and OVN we saw that the "next-server" field
> was the problem for OVN, without it PXE booting was timing out while
> fetching the iPXE image from the TFTP server (see the bugzilla ticket
> below for reference).
>
> To confirm this problem we created a bogus patch hardcoding the TFTP
> address in the siaddr of the DHCP header (see the discussion in the
> maillist below) and with this in place we were able to deploy a
> baremetal node using the OVN DHCP end-to-end.
>
> This patch is a proper implementation that creates a new DHCP
> configuration option called "next_server" to allow users to set this
> field dynamically. This patch uses the DHCP code 253 which is a unsed
> code for DHCP specification as this is not a normal DHCP option but a
> special use case in OVN.
>
> [0]
> https://github.com/openvswitch/ovs/blob/9dd3031d2e0e9597449e95428320ccaaff7d8b3d/lib/dhcp.h#L42
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2083629
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2022-May/051821.html
> Signed-off-by: Lucas Alvares Gomes <[email protected]>
> Signed-off-by: Numan Siddique <[email protected]>
> (cherry picked from commit 0057cde2a64749bd2dbbaff525f7a1edccbd9c8a)
> Signed-off-by: Mark Michelson <[email protected]>
Is this patch still not merged to OVN 22.03 ? Since it already has
all the Acks and merged in the main branch. I think you can go ahead
and apply it.
+1 from me.
Thanks
Numan
> ---
> NEWS | 2 ++
> controller/pinctrl.c | 69 ++++++++++++++++++++++++++++++--------------
> lib/actions.c | 13 ++++++++-
> lib/ovn-l7.h | 8 +++++
> northd/ovn-northd.c | 1 +
> ovn-nb.xml | 7 +++++
> tests/ovn.at | 6 ++--
> tests/test-ovn.c | 1 +
> 8 files changed, 82 insertions(+), 25 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 73f92b447..6bd9f054d 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -2,6 +2,8 @@ OVN v22.03.2 - xx xxx xxxx
> --------------------------
> - Support (LSP.options:qos_min_rate) to guarantee minimal bandwidth
> available
> for a logical port.
> + - Added support for setting the Next server IP in the DHCP header
> + using the private DHCP option - 253 in native OVN DHCPv4 responder.
>
> OVN v22.03.1 - 03 Jun 2022
> --------------------------
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 2c1dd8501..fd4d9a875 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -2206,30 +2206,56 @@ pinctrl_handle_put_dhcp_opts(
> *| 4 Bytes padding | 1 Byte (option end 0xFF ) | 4 Bytes padding|
> * --------------------------------------------------------------
> */
> - struct dhcp_opt_header *in_dhcp_opt =
> - (struct dhcp_opt_header *)reply_dhcp_opts_ptr->data;
> - if (in_dhcp_opt->code == DHCP_OPT_BOOTFILE_CODE) {
> - unsigned char *ptr = (unsigned char *)in_dhcp_opt;
> - int len = sizeof *in_dhcp_opt + in_dhcp_opt->len;
> - struct dhcp_opt_header *next_dhcp_opt =
> - (struct dhcp_opt_header *)(ptr + len);
> -
> - if (next_dhcp_opt->code == DHCP_OPT_BOOTFILE_ALT_CODE) {
> - if (!ipxe_req) {
> - ofpbuf_pull(reply_dhcp_opts_ptr, len);
> - next_dhcp_opt->code = DHCP_OPT_BOOTFILE_CODE;
> - } else {
> - char *buf = xmalloc(len);
> + ovs_be32 next_server = in_dhcp_data->siaddr;
> + bool bootfile_name_set = false;
> + in_dhcp_ptr = reply_dhcp_opts_ptr->data;
> + end = (const char *)reply_dhcp_opts_ptr->data +
> reply_dhcp_opts_ptr->size;
> +
> + while (in_dhcp_ptr < end) {
> + struct dhcp_opt_header *in_dhcp_opt =
> + (struct dhcp_opt_header *)in_dhcp_ptr;
> +
> + switch (in_dhcp_opt->code) {
> + case DHCP_OPT_NEXT_SERVER_CODE:
> + next_server = get_unaligned_be32(DHCP_OPT_PAYLOAD(in_dhcp_opt));
> + break;
> + case DHCP_OPT_BOOTFILE_CODE: ;
> + unsigned char *ptr = (unsigned char *)in_dhcp_opt;
> + int len = sizeof *in_dhcp_opt + in_dhcp_opt->len;
> + struct dhcp_opt_header *next_dhcp_opt =
> + (struct dhcp_opt_header *)(ptr + len);
> +
> + if (next_dhcp_opt->code == DHCP_OPT_BOOTFILE_ALT_CODE) {
> + if (!ipxe_req) {
> + ofpbuf_pull(reply_dhcp_opts_ptr, len);
> + next_dhcp_opt->code = DHCP_OPT_BOOTFILE_CODE;
> + } else {
> + char *buf = xmalloc(len);
>
> - memcpy(buf, in_dhcp_opt, len);
> - ofpbuf_pull(reply_dhcp_opts_ptr,
> - sizeof *in_dhcp_opt + next_dhcp_opt->len);
> - memcpy(reply_dhcp_opts_ptr->data, buf, len);
> - free(buf);
> + memcpy(buf, in_dhcp_opt, len);
> + ofpbuf_pull(reply_dhcp_opts_ptr,
> + sizeof *in_dhcp_opt + next_dhcp_opt->len);
> + memcpy(reply_dhcp_opts_ptr->data, buf, len);
> + free(buf);
> + }
> + }
> + bootfile_name_set = true;
> + break;
> + case DHCP_OPT_BOOTFILE_ALT_CODE:
> + if (!bootfile_name_set) {
> + in_dhcp_opt->code = DHCP_OPT_BOOTFILE_CODE;
> }
> + break;
> + }
> +
> + in_dhcp_ptr += sizeof *in_dhcp_opt;
> + if (in_dhcp_ptr > end) {
> + break;
> + }
> + in_dhcp_ptr += in_dhcp_opt->len;
> + if (in_dhcp_ptr > end) {
> + break;
> }
> - } else if (in_dhcp_opt->code == DHCP_OPT_BOOTFILE_ALT_CODE) {
> - in_dhcp_opt->code = DHCP_OPT_BOOTFILE_CODE;
> }
>
> uint16_t new_l4_size = UDP_HEADER_LEN + DHCP_HEADER_LEN + 16;
> @@ -2262,6 +2288,7 @@ pinctrl_handle_put_dhcp_opts(
>
> if (*in_dhcp_msg_type != OVN_DHCP_MSG_INFORM) {
> dhcp_data->yiaddr = (msg_type == DHCP_MSG_NAK) ? 0 : *offer_ip;
> + dhcp_data->siaddr = (msg_type == DHCP_MSG_NAK) ? 0 : next_server;
> } else {
> dhcp_data->yiaddr = 0;
> }
> diff --git a/lib/actions.c b/lib/actions.c
> index a9c27600c..24d6818c2 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -2862,10 +2862,21 @@ encode_PUT_DHCPV4_OPTS(const struct ovnact_put_opts
> *pdo,
> opt_header[1] = strlen(c->string);
> ofpbuf_put(ofpacts, c->string, opt_header[1]);
> }
> + /* Encode next_server opt (253) */
> + const struct ovnact_gen_option *next_server_opt = find_opt(
> + pdo->options, pdo->n_options, DHCP_OPT_NEXT_SERVER_CODE);
> + if (next_server_opt) {
> + uint8_t *opt_header = ofpbuf_put_zeros(ofpacts, 2);
> + const union expr_constant *c = next_server_opt->value.values;
> + opt_header[0] = next_server_opt->option->code;
> + opt_header[1] = sizeof(ovs_be32);
> + ofpbuf_put(ofpacts, &c->value.ipv4, sizeof(ovs_be32));
> + }
>
> for (size_t i = 0; i < pdo->n_options; i++) {
> const struct ovnact_gen_option *o = &pdo->options[i];
> - if (o != offerip_opt && o != boot_opt && o != boot_alt_opt) {
> + if (o != offerip_opt && o != boot_opt && o != boot_alt_opt && \
> + o != next_server_opt) {
> encode_put_dhcpv4_option(o, ofpacts);
> }
> }
> diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
> index 49ecea81f..0b2da9f7b 100644
> --- a/lib/ovn-l7.h
> +++ b/lib/ovn-l7.h
> @@ -145,6 +145,14 @@ struct gen_opts_map {
> #define DHCP_OPT_TCP_KEEPALIVE_INTERVAL \
> DHCP_OPTION("tcp_keepalive_interval", 38, "uint32")
>
> +/* Use unused 253 option for DHCP next-server DHCP option.
> + * This option is used for setting the "Next server IP address"
> + * field in the DHCP header.
> + */
> +#define DHCP_OPT_NEXT_SERVER_CODE 253
> +#define DHCP_OPT_NEXT_SERVER \
> + DHCP_OPTION("next_server", DHCP_OPT_NEXT_SERVER_CODE, "ipv4")
> +
> static inline uint32_t
> gen_opt_hash(char *opt_name)
> {
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 9d191ce90..4d2baefe4 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -246,6 +246,7 @@ static struct gen_opts_map supported_dhcp_opts[] = {
> DHCP_OPT_BROADCAST_ADDRESS,
> DHCP_OPT_NETBIOS_NAME_SERVER,
> DHCP_OPT_NETBIOS_NODE_TYPE,
> + DHCP_OPT_NEXT_SERVER,
> };
>
> static struct gen_opts_map supported_dhcpv6_opts[] = {
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index db13e0ee5..765bab241 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -3549,6 +3549,13 @@
> </p>
> </column>
>
> + <column name="options" key="next_server">
> + <p>
> + The DHCPv4 option code for setting the "Next server IP
> + address" field in the DHCP header.
> + </p>
> + </column>
> +
> </group>
>
> <group title="Boolean DHCP Options">
> diff --git a/tests/ovn.at b/tests/ovn.at
> index adb64d775..71bffb8a0 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1404,9 +1404,9 @@ reg0[0] = lookup_arp_ip(inport, eth.dst);
> # put_dhcp_opts
> reg1[0] = put_dhcp_opts(offerip = 1.2.3.4, router = 10.0.0.1);
> encodes as
> controller(userdata=00.00.00.02.00.00.00.00.00.01.de.10.00.00.00.40.01.02.03.04.03.04.0a.00.00.01,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",wpad="https://example.org",bootfile_name="https://127.0.0.1/boot.ipxe",path_prefix="/tftpboot";);
> - 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");
> - 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,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",wpad="https://example.org",bootfile_name="https://127.0.0.1/boot.ipxe",path_prefix="/tftpboot";,
> next_server=10.0.0.9);
> + 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", next_server = 10.0.0.9);
> + 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.fd.04.0a.00.00.09.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,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_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)
> diff --git a/tests/test-ovn.c b/tests/test-ovn.c
> index 6704f612b..ea121efa7 100644
> --- a/tests/test-ovn.c
> +++ b/tests/test-ovn.c
> @@ -196,6 +196,7 @@ create_gen_opts(struct hmap *dhcp_opts, struct hmap
> *dhcpv6_opts,
> dhcp_opt_add(dhcp_opts, "broadcast_address", 28, "ipv4");
> dhcp_opt_add(dhcp_opts, "netbios_name_server", 44, "ipv4");
> dhcp_opt_add(dhcp_opts, "netbios_node_type", 46, "uint8");
> + dhcp_opt_add(dhcp_opts, "next_server", 253, "ipv4");
>
> /* DHCPv6 options. */
> hmap_init(dhcpv6_opts);
> --
> 2.37.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
