On 2023-11-04 12:53 a.m., Heinrich Schuchardt wrote:
On 11/4/23 03:03, Sean Edmond wrote:
On 2023-10-23 10:54 p.m., Heinrich Schuchardt wrote:
On 10/24/23 02:21, seanedm...@linux.microsoft.com wrote:
From: Sean Edmond <seanedm...@microsoft.com>
Allow dhcp server pass pxe config file full path by using option 209
Signed-off-by: Sean Edmond <seanedm...@microsoft.com>
---
cmd/Kconfig | 4 ++++
cmd/pxe.c | 10 ++++++++++
net/bootp.c | 21 +++++++++++++++++++++
3 files changed, 35 insertions(+)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 5bc0a92d57..adbb1a6187 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1826,6 +1826,10 @@ config BOOTP_PXE_CLIENTARCH
default 0x15 if ARM
default 0x0 if X86
+config BOOTP_PXE_DHCP_OPTION
+ bool "Request & store 'pxe_configfile' from BOOTP/DHCP server"
+ depends on BOOTP_PXE
Why should this be disabled by default?
Do we really want a separate config variable?
I expect most won't use this option to get the file path (they'll use
the default paths as per the PXE specification). It makes more sense
for me to keep it optional, like many of the other options?
RFC 5701 seems to require this option. Hence we should make it default
yes. Boards that have a build size issue can opt out.
The PXELINUX specification
(https://wiki.syslinux.org/wiki/index.php?title=PXELINUX) doesn't state
that option 209 is required. In the abense of option 209, PXELINUX will
try the following default configuration files (this example is provided
on the wiki link):
/mybootdir/pxelinux.cfg/b8945908-d6a6-41a9-611d-74a6ab80b83d
/mybootdir/pxelinux.cfg/01-88-99-aa-bb-cc-dd
/mybootdir/pxelinux.cfg/C0A8025B
/mybootdir/pxelinux.cfg/C0A8025
/mybootdir/pxelinux.cfg/C0A802
/mybootdir/pxelinux.cfg/C0A80
/mybootdir/pxelinux.cfg/C0A8
/mybootdir/pxelinux.cfg/C0A
/mybootdir/pxelinux.cfg/C0
/mybootdir/pxelinux.cfg/C
/mybootdir/pxelinux.cfg/default
If 209 is requested/provided it tries the provided "config file" before
the default paths. RFC 5071 should be seen as an extension for PXELINUX
(not a requirement). RFC 5071 only states "The Config File Option MUST
be supplied by the DHCP server if it appears on the Parameter Request List".
Best regards
Heinrich
+
config BOOTP_VCI_STRING
string
depends on CMD_BOOTP
diff --git a/cmd/pxe.c b/cmd/pxe.c
index 704589702f..9404f44518 100644
--- a/cmd/pxe.c
+++ b/cmd/pxe.c
@@ -65,6 +65,8 @@ static int pxe_dhcp_option_path(struct pxe_context
*ctx, unsigned long pxefile_a
int ret = get_pxe_file(ctx, pxelinux_configfile,
pxefile_addr_r);
free(pxelinux_configfile);
+ /* set to NULL to avoid double-free if DHCP is tried again */
+ pxelinux_configfile = NULL;
return ret;
}
@@ -141,6 +143,14 @@ int pxe_get(ulong pxefile_addr_r, char
**bootdirp, ulong *sizep, bool use_ipv6)
env_get("bootfile"), use_ipv6))
return -ENOMEM;
+ if (IS_ENABLED(CONFIG_BOOTP_PXE_DHCP_OPTION) &&
+ pxelinux_configfile && !use_ipv6) {
+ if (pxe_dhcp_option_path(&ctx, pxefile_addr_r) > 0)
+ goto done;
+
+ goto error_exit;
+ }
+
if (IS_ENABLED(CONFIG_DHCP6_PXE_DHCP_OPTION) &&
pxelinux_configfile && use_ipv6) {
if (pxe_dhcp_option_path(&ctx, pxefile_addr_r) > 0)
diff --git a/net/bootp.c b/net/bootp.c
index 7b0f45e18a..6800290963 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -26,6 +26,7 @@
#ifdef CONFIG_BOOTP_RANDOM_DELAY
#include "net_rand.h"
#endif
+#include <malloc.h>
#define BOOTP_VENDOR_MAGIC 0x63825363 /* RFC1048 Magic
Cookie */
@@ -601,6 +602,10 @@ static int dhcp_extended(u8 *e, int
message_type, struct in_addr server_ip,
*e++ = 42;
*cnt += 1;
#endif
+ if (IS_ENABLED(CONFIG_BOOTP_PXE_DHCP_OPTION)) {
+ *e++ = 209; /* PXELINUX Config File */
+ *cnt += 1;
+ }
/* no options, so back up to avoid sending an empty request
list */
if (*cnt == 0)
e -= 2;
@@ -909,6 +914,22 @@ static void dhcp_process_options(uchar *popt,
uchar *end)
net_boot_file_name[size] = 0;
}
break;
+ case 209: /* PXELINUX Config File */
This 209 appears in multiple places. Please, define a constant, e.g.
DHCP_OPTION_CONFIG_FILE, in bootp.h and use the symbolic name. Please,
document the constant referring to RFC 5071.
fixed in v3.
+ if (IS_ENABLED(CONFIG_BOOTP_PXE_DHCP_OPTION)) {
+ /* In case it has already been allocated when get
DHCP Offer packet,
+ * free first to avoid memory leak.
+ */
+ if (pxelinux_configfile)
+ free(pxelinux_configfile);
+
+ pxelinux_configfile = (char *)malloc((oplen + 1) *
sizeof(char));
+
+ if (pxelinux_configfile)
+ strlcpy(pxelinux_configfile, popt + 2, oplen +
1);
+ else
+ printf("Error: Failed to allocate
pxelinux_configfile\n");
We do the same in dhcp6_parse_options(). Please, factor out a common
function to avoid code duplication.
I would prefer to keep these seperate for several reasons:
- our DHCP server team has asked for the DHCP6 "pxe config file" parsing
to change. I attempted to add to upstream here:
https://lore.kernel.org/u-boot/20230725231329.5653-1-seanedm...@linux.microsoft.com/.
I will attempt to submitted that patch again
- PXE config file isn't standardized yet for DHCPv6 (the current
implementation was a proposal from our DHCP server team)
- LWIP will eventually superceed bootp.c in u-boot, but we'll probably
be keeping dhcpv6.c for a bit (keeping the duplicate code will make the
migration easier)
Best regards
Heinrich
+ }
+ break;
default:
#if defined(CONFIG_BOOTP_VENDOREX)
if (dhcp_vendorex_proc(popt))
