Hi,
On Fri, 7 Apr 2023 at 18:56, <seanedm...@linux.microsoft.com> wrote:
From: Sean Edmond <seanedm...@microsoft.com>
Adds DHCPv6 protocol to u-boot.
Allows for address assignement with DHCPv6 4-message exchange
(SOLICIT->ADVERTISE->REQUEST->REPLY). Includes DHCPv6 options
required by RFC 8415. Also adds DHCPv6 options required
for PXE boot.
Possible enhancements:
- Duplicate address detection on DHCPv6 assigned address
- IPv6 address assignement through SLAAC
- Sending/parsing other DHCPv6 options (NTP, DNS, etc...)
Signed-off-by: Sean Edmond <seanedm...@microsoft.com>
---
include/net.h | 8 +-
net/Makefile | 1 +
net/dhcpv6.c | 735 ++++++++++++++++++++++++++++++++++++++++++++++++++
net/dhcpv6.h | 212 +++++++++++++++
net/net.c | 12 +
5 files changed, 966 insertions(+), 2 deletions(-)
create mode 100644 net/dhcpv6.c
create mode 100644 net/dhcpv6.h
This looks good to me. I just have a few nits below. With those fixed:
Reviewed-by: Simon Glass <s...@chromium.org>
diff --git a/include/net.h b/include/net.h
index 399af5e064..cac818e292 100644
--- a/include/net.h
+++ b/include/net.h
@@ -484,6 +484,10 @@ extern char net_hostname[32]; /* Our hostname
*/
#ifdef CONFIG_NET
extern char net_root_path[CONFIG_BOOTP_MAX_ROOT_PATH_LEN]; /* Our root
path */
#endif
+#if defined(CONFIG_DHCP6_PXE_DHCP_OPTION)
You can drop this #ifdef as any reference to a non-existent var will
give a build error.
+/* Indicates whether the pxe path prefix / config file was specified in dhcp
option */
+extern char *pxelinux_configfile;
+#endif
/** END OF BOOTP EXTENTIONS **/
extern u8 net_ethaddr[ARP_HLEN]; /* Our ethernet
address */
extern u8 net_server_ethaddr[ARP_HLEN]; /* Boot server enet
address */
@@ -504,8 +508,8 @@ extern ushort net_native_vlan; /* Our
Native VLAN */
extern int net_restart_wrap; /* Tried all network devices */
enum proto_t {
- BOOTP, RARP, ARP, TFTPGET, DHCP, PING, PING6, DNS, NFS, CDP, NETCONS,
- SNTP, TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, NCSI, WGET
+ BOOTP, RARP, ARP, TFTPGET, DHCP, DHCP6, PING, PING6, DNS, NFS, CDP,
+ NETCONS, SNTP, TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, NCSI,
WGET
};
extern char net_boot_file_name[1024];/* Boot File name */
diff --git a/net/Makefile b/net/Makefile
index bea000b206..5968110170 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_IPV6) += net6.o
obj-$(CONFIG_CMD_NFS) += nfs.o
obj-$(CONFIG_CMD_PING) += ping.o
obj-$(CONFIG_CMD_PING6) += ping6.o
+obj-$(CONFIG_CMD_DHCP6) += dhcpv6.o
obj-$(CONFIG_CMD_PCAP) += pcap.o
obj-$(CONFIG_CMD_RARP) += rarp.o
obj-$(CONFIG_CMD_SNTP) += sntp.o
diff --git a/net/dhcpv6.c b/net/dhcpv6.c
new file mode 100644
index 0000000000..9204909c1f
--- /dev/null
+++ b/net/dhcpv6.c
@@ -0,0 +1,735 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) Microsoft Corporation
+ * Author: Sean Edmond <seanedm...@microsoft.com>
+ *
+ */
+
+/* Simple DHCP6 network layer implementation. */
+
+#include <common.h>
+#include <bootstage.h>
+#include <command.h>
+#include <env.h>
+#include <efi_loader.h>
+#include <log.h>
+#include <net.h>
+#include <rand.h>
+#include <uuid.h>
+#include <linux/delay.h>
+#include <net/tftp.h>
+#include "dhcpv6.h"
+#include <net6.h>
+#include <malloc.h>
+#include "net_rand.h"
Please fix header order:
https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files
+
+#define PORT_DHCP6_S 547 /* DHCP6 server UDP port */
+#define PORT_DHCP6_C 546 /* DHCP6 client UDP port */
+
+/* default timeout parameters (in ms) */
+#define SOL_MAX_DELAY_MS 1000
+#define SOL_TIMEOUT_MS 1000
+#define SOL_MAX_RT_MS 3600000
+#define REQ_TIMEOUT_MS 1000
+#define REQ_MAX_RT_MS 30000
+#define REQ_MAX_RC 10
+#define MAX_WAIT_TIME_MS 60000
+
+/* global variable to track any updates from DHCP6 server */
+int updated_sol_max_rt_ms = SOL_MAX_RT_MS;
+
+static void dhcp6_timeout_handler(void);
+static void dhcp6_state_machine(bool timeout, uchar *rx_pkt, unsigned int len);
+static void dhcp6_parse_options(uchar *rx_pkt, unsigned int len);
Rather than forward decls can you reorder the functions?
+
+struct dhcp6_sm_params sm_params;
+
+/*
+ * Handle DHCP received packets (set as UDP handler)
+ */
Please check single-line comment style
+static void dhcp6_handler(uchar *pkt, unsigned int dest, struct in_addr sip,
+ unsigned int src, unsigned int len)
+{
+ /* return if ports don't match DHCPv6 ports */
+ if (dest != PORT_DHCP6_C || src != PORT_DHCP6_S)
+ return;
+
+ dhcp6_state_machine(false, pkt, len);
+}
+
[..]
+ case DHCP6_OPTION_OPT_BOOTFILE_URL:
+ debug("DHCP6_OPTION_OPT_BOOTFILE_URL FOUND\n");
+ copy_filename(net_boot_file_name, option_ptr,
option_len + 1);
+ debug("net_boot_file_name: %s\n", net_boot_file_name);
+
+ /* copy server_ip6 (required for PXE) */
+ s = strchr(net_boot_file_name, '[');
+ e = strchr(net_boot_file_name, ']');
+ if (s && e && e > s)
+ string_to_ip6(s + 1, e - s - 1,
&net_server_ip6);
+ break;
+#if IS_ENABLED(CONFIG_DHCP6_PXE_DHCP_OPTION)
+ case DHCP6_OPTION_OPT_BOOTFILE_PARAM:
Can you do something like this to avoid the #ifdef ?
case DHCP6_OPTION_OPT_BOOTFILE_PARAM:
if (IS_ENABLED(CONFIG_DHCP6_PXE_DHCP_OPTION) {
...
}
You could even add a 'fallthough' if you move it to last position in
the switch().
+ debug("DHCP6_OPTION_OPT_BOOTFILE_PARAM FOUND\n");
+
+ if (pxelinux_configfile)
+ free(pxelinux_configfile);
+
+ pxelinux_configfile = (char *)malloc((option_len + 1) *
sizeof(char));
+ if (pxelinux_configfile) {
+ memcpy(pxelinux_configfile, option_ptr,
option_len);
+ pxelinux_configfile[option_len] = '\0';
Does strlcpy() with option_len + 1 work here?
+ }
If malloc() fails it needs to be reported.
+
+ debug("PXE CONFIG FILE %s\n", pxelinux_configfile);
+ break;
+#endif
+ case DHCP6_OPTION_PREFERENCE:
+ debug("DHCP6_OPTION_PREFERENCE FOUND\n");
+ sm_params.rx_status.preference = *option_ptr;
+ break;
+ default:
+ debug("Unknown Option ID: %d, skipping parsing\n",
+ ntohs(option_hdr->option_id));
+ break;
+ }
+ /* Increment to next option header */
+ option_hdr = (struct dhcp6_option_hdr *)(((uchar *)option_hdr) +
+ sizeof(struct dhcp6_option_hdr) + option_len);
+ }
+}
+
[..]
+/*
+ * Timeout for DHCP6 SOLICIT/REQUEST.
Fix single-line comment format (globally)
+ */
+static void dhcp6_timeout_handler(void)
+{
+ /* call state machine with the timeout flag */
+ dhcp6_state_machine(true, NULL, 0);
+}
+
+/*
+ * Start or restart DHCP6
+ */
+void dhcp6_start(void)
+{
+ memset(&sm_params, 0, sizeof(struct dhcp6_sm_params));
+
+ /* seed the RNG with MAC address */
+ srand_mac();
+
+ sm_params.curr_state = DHCP6_INIT;
+ dhcp6_state_machine(false, NULL, 0);
+}
diff --git a/net/dhcpv6.h b/net/dhcpv6.h
new file mode 100644
index 0000000000..6a0158127a
--- /dev/null
+++ b/net/dhcpv6.h
@@ -0,0 +1,212 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) Microsoft Corporation
+ * Author: Sean Edmond <seanedm...@microsoft.com>
+ *
+ */
+
+#ifndef __DHCP6_H__
+#define __DHCP6_H__
+
+#include <net6.h>
Is this needed? If so it might be better to split out the bits you
need into a net6_defs.h or something like that.
+#include <net.h>
Please don't include net.h as it brings in heaps of stuff.
+
+/* Message types */
+#define DHCP6_MSG_SOLICIT 1
+#define DHCP6_MSG_ADVERTISE 2
+#define DHCP6_MSG_REQUEST 3
+#define DHCP6_MSG_REPLY 7
[..]
+
+/* Send a DHCPv6 request */
Can you add a bit more detail here, e.g. mention the state machine and
what happens next?
+void dhcp6_start(void);
+
+#endif /* __DHCP6_H__ */
diff --git a/net/net.c b/net/net.c
index c9a749f6cc..73d5b2bc80 100644
--- a/net/net.c
+++ b/net/net.c
@@ -122,6 +122,9 @@
#endif
#include <net/tcp.h>
#include <net/wget.h>
+#if defined(CONFIG_CMD_DHCP6)
+#include "dhcpv6.h"
No need to exclude the header so you can drop the #ifdef
+#endif
/** BOOTP EXTENTIONS **/
@@ -135,6 +138,10 @@ struct in_addr net_dns_server;
/* Our 2nd DNS IP address */
struct in_addr net_dns_server2;
#endif
+#if defined(CONFIG_DHCP6_PXE_DHCP_OPTION)
+/* Indicates whether the pxe path prefix / config file was specified in dhcp
option */
+char *pxelinux_configfile;
+#endif
/** END OF BOOTP EXTENTIONS **/
@@ -510,6 +517,11 @@ restart:
dhcp_request(); /* Basically same as BOOTP */
break;
#endif
+#if defined(CONFIG_CMD_DHCP6)
+ case DHCP6:
Can we use the if (IS_ENABLED()) thing again here to drop this #ifdef?
+ dhcp6_start();
+ break;
+#endif
#if defined(CONFIG_CMD_BOOTP)
case BOOTP:
bootp_reset();
--
2.40.0
Regards,
Simon
