Hi,
On Fri, 7 Apr 2023 at 18:56, seanedmond@linux.microsoft.com wrote:
From: Sean Edmond seanedmond@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 seanedmond@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 sjg@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 seanedmond@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-fil...
+#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 seanedmond@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