[PATCH v8 3/3] net: bootp: add config option BOOTP_RANDOM_XID
The new config option BOOTP_RANDOM_XID will randomize the transaction ID
for each new BOOT/DHCPv4 exchange.
Signed-off-by: Sean Edmond
Reviewed-by: Tom Rini
---
(no changes since v5)
Changes in v5:
- fix depends for BOOTP_RANDOM_XID:
"depends on CMD_BOOTP && (LIB_RAND || LIB_HW_RAND)"
Changes in v3:
- Add depends for BOOTP_RANDOM_XID
cmd/Kconfig | 7 +++
net/bootp.c | 31 +--
2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 54c4ab8570c..82f6a089586 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1909,6 +1909,13 @@ config BOOTP_VCI_STRING
default "U-Boot.arm" if ARM
default "U-Boot"
+config BOOTP_RANDOM_XID
+ bool "Send random transaction ID to BOOTP/DHCP server"
+ depends on CMD_BOOTP && (LIB_RAND || LIB_HW_RAND)
+ help
+ Selecting this will allow for a random transaction ID to get
+ selected for each BOOTP/DHCPv4 exchange.
+
if CMD_DHCP6
config DHCP6_PXE_CLIENTARCH
diff --git a/net/bootp.c b/net/bootp.c
index c34ffe27854..285fcba7348 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -834,22 +834,25 @@ void bootp_request(void)
/* Only generate a new transaction ID for each new BOOTP request */
if (bootp_try == 1) {
- /*
-* Bootp ID is the lower 4 bytes of our ethernet address
-* plus the current time in ms.
-*/
- bootp_id = ((u32)net_ethaddr[2] << 24)
- | ((u32)net_ethaddr[3] << 16)
- | ((u32)net_ethaddr[4] << 8)
- | (u32)net_ethaddr[5];
- bootp_id += get_timer(0);
- bootp_id = htonl(bootp_id);
- bootp_add_id(bootp_id);
- net_copy_u32(>bp_id, _id);
- } else {
- net_copy_u32(>bp_id, _id);
+ if (IS_ENABLED(CONFIG_BOOTP_RANDOM_XID)) {
+ srand(get_ticks() + rand());
+ bootp_id = rand();
+ } else {
+ /*
+* Bootp ID is the lower 4 bytes of our ethernet
address
+* plus the current time in ms.
+*/
+ bootp_id = ((u32)net_ethaddr[2] << 24)
+ | ((u32)net_ethaddr[3] << 16)
+ | ((u32)net_ethaddr[4] << 8)
+ | (u32)net_ethaddr[5];
+ bootp_id += get_timer(0);
+ bootp_id = htonl(bootp_id);
+ }
}
+ bootp_add_id(bootp_id);
+ net_copy_u32(>bp_id, _id);
/*
* Calculate proper packet lengths taking into account the
* variable size of the options field
--
2.42.0
[PATCH v8 2/3] net: bootp: BOOTP/DHCPv4 retransmission improvements
This patch introduces 3 improvements to align with RFC 951:
- retransmission backoff interval maximum is configurable
- initial retranmission backoff interval is configurable
- transaction ID is kept the same for each BOOTP/DHCPv4 request
In applications where thousands of nodes are serviced by a single DHCP
server, maximizing the retransmission backoff interval at 2 seconds (the
current u-boot default) exerts high pressure on the DHCP server and
network layer.
RFC 951 “7.2. Client Retransmission Strategy” states that the
retransmission backoff interval should be limited to 60 seconds. This
patch allows the interval to be configurable using the environment
variable "bootpretransmitperiodmax"
The initial retranmission backoff period defaults to 250ms, which is
also too small for these scenarios with many clients. This patch makes
the initial retransmission interval to be configurable using the
environment variable "bootpretransmitperiodinit".
Also, on a retransmission it is not expected for the transaction ID to
change (only the 'secs' field should be updated). Let's save the
transaction ID and use the same transaction ID for each BOOTP/DHCPv4
exchange.
Signed-off-by: Sean Edmond
---
Changes in v8:
- remove unused variable "char *ep" from bootp_reset()
Changes in v7:
- Remove "depends on LIB_RAND || LIB_HW_RAND" from CMD_BOOTP
- Add CMD_BOOTP/CMD_DHCP/CMD_DHCP6 to LIB_RAND menu choice
- Add space after "config CMD_DHCP"
Changes in v6:
- CMD_BOOTP should "depends on LIB_RAND || LIB_HW_RAND"
Changes in v4:
- Add "select LIB_RAND" for "config CMD_BOOTP" (retransmission
improvements require rand())
Changes in v3:
- Set RETRANSMIT_PERIOD_MAX_MS=6
- Add randomization factor to retransmission timeout
Changes in v2:
- use env_get_ulong() to get environment variables
lib/Kconfig | 3 ++-
net/bootp.c | 68 -
2 files changed, 54 insertions(+), 17 deletions(-)
diff --git a/lib/Kconfig b/lib/Kconfig
index 189e6eb31aa..9b62b3bbea7 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -265,7 +265,8 @@ config REGEX
choice
prompt "Pseudo-random library support type"
depends on NET_RANDOM_ETHADDR || RANDOM_UUID || CMD_UUID || \
- RNG_SANDBOX || UT_LIB && AES || FAT_WRITE
+ RNG_SANDBOX || UT_LIB && AES || FAT_WRITE || CMD_BOOTP || \
+ CMD_DHCP || CMD_DHCP6
default LIB_RAND
help
Select the library to provide pseudo-random number generator
diff --git a/net/bootp.c b/net/bootp.c
index b9e3cccb4f9..c34ffe27854 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -42,6 +42,22 @@
*/
#define TIMEOUT_MS ((3 + (CONFIG_NET_RETRY_COUNT * 5)) * 1000)
+/*
+ * According to rfc951 : 7.2. Client Retransmission Strategy
+ * "After the 'average' backoff reaches about 60 seconds, it should be
+ * increased no further, but still randomized."
+ *
+ * U-Boot has saturated this backoff at 2 seconds for a long time.
+ * To modify, set the environment variable "bootpretransmitperiodmax"
+ */
+#define RETRANSMIT_PERIOD_MAX_MS 6
+
+/* Retransmission timeout for the initial packet (in milliseconds).
+ * This timeout will double on each retry. To modify, set the
+ * environment variable bootpretransmitperiodinit.
+ */
+#define RETRANSMIT_PERIOD_INIT_MS 250
+
#ifndef CFG_DHCP_MIN_EXT_LEN /* minimal length of extension list */
#define CFG_DHCP_MIN_EXT_LEN 64
#endif
@@ -53,6 +69,7 @@
u32bootp_ids[CFG_BOOTP_ID_CACHE_SIZE];
unsigned int bootp_num_ids;
intbootp_try;
+u32bootp_id;
ulong bootp_start;
ulong bootp_timeout;
char net_nis_domain[32] = {0,}; /* Our NIS domain */
@@ -60,6 +77,7 @@ char net_hostname[32] = {0,}; /* Our hostname */
char net_root_path[CONFIG_BOOTP_MAX_ROOT_PATH_LEN] = {0,}; /* Our bootpath */
static ulong time_taken_max;
+static u32 retransmit_period_max_ms;
#if defined(CONFIG_CMD_DHCP)
static dhcp_state_t dhcp_state = INIT;
@@ -396,6 +414,7 @@ static void bootp_handler(uchar *pkt, unsigned dest, struct
in_addr sip,
static void bootp_timeout_handler(void)
{
ulong time_taken = get_timer(bootp_start);
+ int rand_minus_plus_100;
if (time_taken >= time_taken_max) {
#ifdef CONFIG_BOOTP_MAY_FAIL
@@ -414,8 +433,17 @@ static void bootp_timeout_handler(void)
}
} else {
bootp_timeout *= 2;
- if (bootp_timeout > 2000)
- bootp_timeout = 2000;
+ if (bootp_timeout > retransmit_period_max_ms)
+ bootp_timeout = retransmit_period_max_ms;
+
+ /* Randomize by adding bootp_timeout*RAND, where RAND
+* is a randomization factor between -0.1..+0.1
+*/
+ s
[PATCH v8 1/3] net: Enhancements for dhcp option 209
- Enable option 209 by default
- Set pxelinux_configfile to NULL to avoid potential double free
- change hardcoded 209 to a define
Signed-off-by: Sean Edmond
---
(no changes since v7)
Changes in v7:
- Reword this commit
Changes in v6:
- Reword this commit
Changes in v4:
- rebase master and resolve conflicts
- default y for BOOTP_PXE_DHCP_OPTION (feedback from review)
- change commit description for this patch (this is now
an enhancement patch)
Changes in v3:
- add define for option 209 and rfc5071 reference
cmd/Kconfig | 1 +
cmd/pxe.c | 2 ++
net/bootp.c | 4 ++--
net/bootp.h | 2 ++
4 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index c182d73ddbd..54c4ab8570c 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1898,6 +1898,7 @@ config BOOTP_PXE_CLIENTARCH
config BOOTP_PXE_DHCP_OPTION
bool "Request & store 'pxe_configfile' from BOOTP/DHCP server"
+ default y
depends on BOOTP_PXE
config BOOTP_VCI_STRING
diff --git a/cmd/pxe.c b/cmd/pxe.c
index 21134eb7a30..9404f445187 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;
}
diff --git a/net/bootp.c b/net/bootp.c
index c15472f5d37..b9e3cccb4f9 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -603,7 +603,7 @@ static int dhcp_extended(u8 *e, int message_type, struct
in_addr server_ip,
*cnt += 1;
#endif
if (IS_ENABLED(CONFIG_BOOTP_PXE_DHCP_OPTION)) {
- *e++ = 209; /* PXELINUX Config File */
+ *e++ = DHCP_OPTION_PXE_CONFIG_FILE; /* PXELINUX Config File
*/
*cnt += 1;
}
/* no options, so back up to avoid sending an empty request list */
@@ -922,7 +922,7 @@ static void dhcp_process_options(uchar *popt, uchar *end)
net_boot_file_name[size] = 0;
}
break;
- case 209: /* PXELINUX Config File */
+ case DHCP_OPTION_PXE_CONFIG_FILE: /* PXELINUX Config File
*/
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.
diff --git a/net/bootp.h b/net/bootp.h
index 4e32b19d424..24b32c73f62 100644
--- a/net/bootp.h
+++ b/net/bootp.h
@@ -91,6 +91,8 @@ typedef enum { INIT,
#define DHCP_NAK 6
#define DHCP_RELEASE 7
+#define DHCP_OPTION_PXE_CONFIG_FILE209 /* "ConfigFile" option
according to rfc5071 */
+
/**/
#endif /* __BOOTP_H__ */
--
2.42.0
[PATCH v8 0/3] BOOTP/DHCPv4 enhancements
In our datacenter application, a single DHCP server is servicing 36000+ clients. Improvements are required to the DHCPv4 retransmission behavior to align with RFC and ensure less pressure is exerted on the server: - retransmission backoff interval maximum is configurable (environment variable bootpretransmitperiodmax) - initial retransmission backoff interval is configurable (environment variable bootpretransmitperiodinit) - transaction ID is kept the same for each BOOTP/DHCPv4 request (not recreated on each retry) For our application we'll use: - bootpretransmitperiodmax=16000 - bootpretransmitperiodinit=2000 A new configuration BOOTP_RANDOM_XID has been added to enable a randomized BOOTP/DHCPv4 transaction ID. Enhance DHCPv4 sending/parsing option 209 (PXE config file). A previous patch was accepted. A new patch fixes a possible double free() and addresses latest review comments. Changes in v8: - remove unused variable "char *ep" from bootp_reset() Changes in v7: - Remove "depends on LIB_RAND || LIB_HW_RAND" from CMD_BOOTP - Add CMD_BOOTP/CMD_DHCP/CMD_DHCP6 to LIB_RAND menu choice - Add space after "config CMD_DHCP" Changes in v6: - CMD_BOOTP should "depends on LIB_RAND || LIB_HW_RAND" Changes in v5: - add change log to individual patches - fix depends for BOOTP_RANDOM_XID: "depends on CMD_BOOTP && (LIB_RAND || LIB_HW_RAND)" Changes in v4: - rebase master and resolve conflicts - default y for BOOTP_PXE_DHCP_OPTION (feedback from review) - Add "select LIB_RAND" for "config CMD_BOOTP" (retransmission improvements require rand()) Changes in v3: - add define for option 209 and rfc5071 reference - Set RETRANSMIT_PERIOD_MAX_MS=6 - Add randomization factor to retransmission timeout - Add depends for BOOTP_RANDOM_XID Changes in v2: - use env_get_ulong() to get environment variables Sean Edmond (3): net: Enhancements for dhcp option 209 net: bootp: BOOTP/DHCPv4 retransmission improvements net: bootp: add config option BOOTP_RANDOM_XID cmd/Kconfig | 8 ++ cmd/pxe.c | 2 ++ lib/Kconfig | 3 ++- net/bootp.c | 73 - net/bootp.h | 2 ++ 5 files changed, 70 insertions(+), 18 deletions(-) -- 2.42.0
Re: [PATCH] Fix neighbor discovery ethernet address saving
Hi Vyachesla, Let's start by saying neighbour discovery protocol is definitely working properly. I'm only suggesting that it isn't properly caching the MAC address. During IPv6 TFTP I observe that neighbour discovery is performed before every packet sent from client(u-boot)->server. Here's a snapshot of what the packets from client(u-boot)<->server look like during IPv6 TFTP: -> neighbour solicitation <- neighbour advertisement -> read request <- Block 0 -> neighbour solicitation <- neighbour advertisement -> ACK block 0 <- Block 1 -> neighbour solicitation <- neighbour advertisement -> ACK block 1 - ... (continues for entire file transfer) The neighbour discovery on every TX TFTP packet isn't required and results in degraded performance. Note, when performing the test with IPv4, ARP is not performed before every TX packet. Here's a description of the code flow (including my proposal): - net.c defines "u8 net_server_ethaddr[6];" - tftp_send()-> net_send_udp_packet6(net_server_ethaddr, ...) - in net_send_udp_packet6(), "net_nd_packet_mac = ether;" (now, net_nd_packet_mac is pointing to net_server_ethaddr) - in ndisc_receive(), when NA is received the mac becomes available in neigh_eth_addr - My proposal is, "if pointer net_nd_packet_mac isn't NULL, copy this contents of neigh_eth_addr into net_nd_packet_mac" - For TFTP, my fix allows the server's MAC to get copied into net_server_ethaddr - on the next tftp_send(), in net_send_udp_packet6() neighbour discovery is skipped because "ether" isn't all zeros The memcpy isn't dangerous, because it's copying the discovered mac address into the already allocated net_server_ethaddr (and it's checking to make sure that net_nd_packet_mac isn't NULL before copying). Also, the current code serves no purpose. The current code is, "if net_nd_packet_mac is NULL, set it to stack variable neigh_eth_addr, then set net_nd_packet_mac to NULL when there is no ND pending (the mac address doesn't get saved in net_server_ethaddr). Sean On 2024-05-05 2:40 a.m., Vyacheslav V. Mitrofanov wrote: On Mon, 2024-04-29 at 11:51 -0700, seanedm...@linux.microsoft.com wrote: s...@yadro.com<mailto:s...@yadro.com> From: Sean Edmond When a successful neighbor advertisement is received, the ethernet address should be saved for later use to avoid having to redo the neighbor discovery process. For example, with TFTP the address should get saved into "net_server_ethaddr". This is being done correctly with ARP for IPv4, but not for neighbor discovery with IPv6. Signed-off-by: Sean Edmond --- net/ndisc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ndisc.c b/net/ndisc.c index d1cec0601c8..505515f2d95 100644 --- a/net/ndisc.c +++ b/net/ndisc.c @@ -461,8 +461,8 @@ int ndisc_receive(struct ethernet_hdr *et, struct ip6_hdr *ip6, int len) ndisc_extract_enetaddr(ndisc, neigh_eth_addr); /* save address for later use */ - if (!net_nd_packet_mac) - net_nd_packet_mac = neigh_eth_addr; + if (net_nd_packet_mac) + memcpy(net_nd_packet_mac, neigh_eth_addr, 6); /* modify header, and transmit it */ memcpy(((struct ethernet_hdr *)net_nd_tx_packet)->et_dest, -- 2.42.0 Hello, Sean. Thanks for your notice. I see that net_nd_packet_mac is just a uchar pointer without memory allocation. It is dangerous to do memcpy and not necessary. All works as it has to be.
[PATCH v6 3/3] net: bootp: add config option BOOTP_RANDOM_XID
The new config option BOOTP_RANDOM_XID will randomize the transaction ID
for each new BOOT/DHCPv4 exchange.
Signed-off-by: Sean Edmond
Reviewed-by: Tom Rini
---
(no changes since v5)
Changes in v5:
- fix depends for BOOTP_RANDOM_XID:
"depends on CMD_BOOTP && (LIB_RAND || LIB_HW_RAND)"
Changes in v3:
- Add depends for BOOTP_RANDOM_XID
cmd/Kconfig | 7 +++
net/bootp.c | 31 +--
2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 54c4ab8570c..82f6a089586 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1909,6 +1909,13 @@ config BOOTP_VCI_STRING
default "U-Boot.arm" if ARM
default "U-Boot"
+config BOOTP_RANDOM_XID
+ bool "Send random transaction ID to BOOTP/DHCP server"
+ depends on CMD_BOOTP && (LIB_RAND || LIB_HW_RAND)
+ help
+ Selecting this will allow for a random transaction ID to get
+ selected for each BOOTP/DHCPv4 exchange.
+
if CMD_DHCP6
config DHCP6_PXE_CLIENTARCH
diff --git a/net/bootp.c b/net/bootp.c
index 0109446dd6f..e336282cb8f 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -837,22 +837,25 @@ void bootp_request(void)
/* Only generate a new transaction ID for each new BOOTP request */
if (bootp_try == 1) {
- /*
-* Bootp ID is the lower 4 bytes of our ethernet address
-* plus the current time in ms.
-*/
- bootp_id = ((u32)net_ethaddr[2] << 24)
- | ((u32)net_ethaddr[3] << 16)
- | ((u32)net_ethaddr[4] << 8)
- | (u32)net_ethaddr[5];
- bootp_id += get_timer(0);
- bootp_id = htonl(bootp_id);
- bootp_add_id(bootp_id);
- net_copy_u32(>bp_id, _id);
- } else {
- net_copy_u32(>bp_id, _id);
+ if (IS_ENABLED(CONFIG_BOOTP_RANDOM_XID)) {
+ srand(get_ticks() + rand());
+ bootp_id = rand();
+ } else {
+ /*
+* Bootp ID is the lower 4 bytes of our ethernet
address
+* plus the current time in ms.
+*/
+ bootp_id = ((u32)net_ethaddr[2] << 24)
+ | ((u32)net_ethaddr[3] << 16)
+ | ((u32)net_ethaddr[4] << 8)
+ | (u32)net_ethaddr[5];
+ bootp_id += get_timer(0);
+ bootp_id = htonl(bootp_id);
+ }
}
+ bootp_add_id(bootp_id);
+ net_copy_u32(>bp_id, _id);
/*
* Calculate proper packet lengths taking into account the
* variable size of the options field
--
2.42.0
[PATCH v6 2/3] net: bootp: BOOTP/DHCPv4 retransmission improvements
This patch introduces 3 improvements to align with RFC 951:
- retransmission backoff interval maximum is configurable
- initial retranmission backoff interval is configurable
- transaction ID is kept the same for each BOOTP/DHCPv4 request
In applications where thousands of nodes are serviced by a single DHCP
server, maximizing the retransmission backoff interval at 2 seconds (the
current u-boot default) exerts high pressure on the DHCP server and
network layer.
RFC 951 “7.2. Client Retransmission Strategy” states that the
retransmission backoff interval should be limited to 60 seconds. This
patch allows the interval to be configurable using the environment
variable "bootpretransmitperiodmax"
The initial retranmission backoff period defaults to 250ms, which is
also too small for these scenarios with many clients. This patch makes
the initial retransmission interval to be configurable using the
environment variable "bootpretransmitperiodinit".
Also, on a retransmission it is not expected for the transaction ID to
change (only the 'secs' field should be updated). Let's save the
transaction ID and use the same transaction ID for each BOOTP/DHCPv4
exchange.
Signed-off-by: Sean Edmond
---
Changes in v7:
- Remove "depends on LIB_RAND || LIB_HW_RAND" from CMD_BOOTP
- Add CMD_BOOTP/CMD_DHCP/CMD_DHCP6 to LIB_RAND menu choice
- Add space after "config CMD_DHCP"
Changes in v6:
- CMD_BOOTP should "depends on LIB_RAND || LIB_HW_RAND"
Changes in v4:
- Add "select LIB_RAND" for "config CMD_BOOTP" (retransmission
improvements require rand())
Changes in v3:
- Set RETRANSMIT_PERIOD_MAX_MS=6
- Add randomization factor to retransmission timeout
Changes in v2:
- use env_get_ulong() to get environment variables
lib/Kconfig | 3 ++-
net/bootp.c | 71 +
2 files changed, 57 insertions(+), 17 deletions(-)
diff --git a/lib/Kconfig b/lib/Kconfig
index 189e6eb31aa..9b62b3bbea7 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -265,7 +265,8 @@ config REGEX
choice
prompt "Pseudo-random library support type"
depends on NET_RANDOM_ETHADDR || RANDOM_UUID || CMD_UUID || \
- RNG_SANDBOX || UT_LIB && AES || FAT_WRITE
+ RNG_SANDBOX || UT_LIB && AES || FAT_WRITE || CMD_BOOTP || \
+ CMD_DHCP || CMD_DHCP6
default LIB_RAND
help
Select the library to provide pseudo-random number generator
diff --git a/net/bootp.c b/net/bootp.c
index b9e3cccb4f9..0109446dd6f 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -42,6 +42,22 @@
*/
#define TIMEOUT_MS ((3 + (CONFIG_NET_RETRY_COUNT * 5)) * 1000)
+/*
+ * According to rfc951 : 7.2. Client Retransmission Strategy
+ * "After the 'average' backoff reaches about 60 seconds, it should be
+ * increased no further, but still randomized."
+ *
+ * U-Boot has saturated this backoff at 2 seconds for a long time.
+ * To modify, set the environment variable "bootpretransmitperiodmax"
+ */
+#define RETRANSMIT_PERIOD_MAX_MS 6
+
+/* Retransmission timeout for the initial packet (in milliseconds).
+ * This timeout will double on each retry. To modify, set the
+ * environment variable bootpretransmitperiodinit.
+ */
+#define RETRANSMIT_PERIOD_INIT_MS 250
+
#ifndef CFG_DHCP_MIN_EXT_LEN /* minimal length of extension list */
#define CFG_DHCP_MIN_EXT_LEN 64
#endif
@@ -53,6 +69,7 @@
u32bootp_ids[CFG_BOOTP_ID_CACHE_SIZE];
unsigned int bootp_num_ids;
intbootp_try;
+u32bootp_id;
ulong bootp_start;
ulong bootp_timeout;
char net_nis_domain[32] = {0,}; /* Our NIS domain */
@@ -60,6 +77,7 @@ char net_hostname[32] = {0,}; /* Our hostname */
char net_root_path[CONFIG_BOOTP_MAX_ROOT_PATH_LEN] = {0,}; /* Our bootpath */
static ulong time_taken_max;
+static u32 retransmit_period_max_ms;
#if defined(CONFIG_CMD_DHCP)
static dhcp_state_t dhcp_state = INIT;
@@ -396,6 +414,7 @@ static void bootp_handler(uchar *pkt, unsigned dest, struct
in_addr sip,
static void bootp_timeout_handler(void)
{
ulong time_taken = get_timer(bootp_start);
+ int rand_minus_plus_100;
if (time_taken >= time_taken_max) {
#ifdef CONFIG_BOOTP_MAY_FAIL
@@ -414,8 +433,17 @@ static void bootp_timeout_handler(void)
}
} else {
bootp_timeout *= 2;
- if (bootp_timeout > 2000)
- bootp_timeout = 2000;
+ if (bootp_timeout > retransmit_period_max_ms)
+ bootp_timeout = retransmit_period_max_ms;
+
+ /* Randomize by adding bootp_timeout*RAND, where RAND
+* is a randomization factor between -0.1..+0.1
+*/
+ srand(get_ticks() + rand());
+ rand_minus_plus_100 = ((rand() % 200) - 100);
+
[PATCH v6 1/3] net: Enhancements for dhcp option 209
- Enable option 209 by default
- Set pxelinux_configfile to NULL to avoid potential double free
- change hardcoded 209 to a define
Signed-off-by: Sean Edmond
---
Changes in v7:
- Reword this commit
Changes in v6:
- Reword this commit
Changes in v4:
- rebase master and resolve conflicts
- default y for BOOTP_PXE_DHCP_OPTION (feedback from review)
- change commit description for this patch (this is now
an enhancement patch)
Changes in v3:
- add define for option 209 and rfc5071 reference
cmd/Kconfig | 1 +
cmd/pxe.c | 2 ++
net/bootp.c | 4 ++--
net/bootp.h | 2 ++
4 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index c182d73ddbd..54c4ab8570c 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1898,6 +1898,7 @@ config BOOTP_PXE_CLIENTARCH
config BOOTP_PXE_DHCP_OPTION
bool "Request & store 'pxe_configfile' from BOOTP/DHCP server"
+ default y
depends on BOOTP_PXE
config BOOTP_VCI_STRING
diff --git a/cmd/pxe.c b/cmd/pxe.c
index 21134eb7a30..9404f445187 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;
}
diff --git a/net/bootp.c b/net/bootp.c
index c15472f5d37..b9e3cccb4f9 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -603,7 +603,7 @@ static int dhcp_extended(u8 *e, int message_type, struct
in_addr server_ip,
*cnt += 1;
#endif
if (IS_ENABLED(CONFIG_BOOTP_PXE_DHCP_OPTION)) {
- *e++ = 209; /* PXELINUX Config File */
+ *e++ = DHCP_OPTION_PXE_CONFIG_FILE; /* PXELINUX Config File
*/
*cnt += 1;
}
/* no options, so back up to avoid sending an empty request list */
@@ -922,7 +922,7 @@ static void dhcp_process_options(uchar *popt, uchar *end)
net_boot_file_name[size] = 0;
}
break;
- case 209: /* PXELINUX Config File */
+ case DHCP_OPTION_PXE_CONFIG_FILE: /* PXELINUX Config File
*/
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.
diff --git a/net/bootp.h b/net/bootp.h
index 4e32b19d424..24b32c73f62 100644
--- a/net/bootp.h
+++ b/net/bootp.h
@@ -91,6 +91,8 @@ typedef enum { INIT,
#define DHCP_NAK 6
#define DHCP_RELEASE 7
+#define DHCP_OPTION_PXE_CONFIG_FILE209 /* "ConfigFile" option
according to rfc5071 */
+
/**/
#endif /* __BOOTP_H__ */
--
2.42.0
[PATCH v6 0/3] BOOTP/DHCPv4 enhancements
In our datacenter application, a single DHCP server is servicing 36000+ clients. Improvements are required to the DHCPv4 retransmission behavior to align with RFC and ensure less pressure is exerted on the server: - retransmission backoff interval maximum is configurable (environment variable bootpretransmitperiodmax) - initial retransmission backoff interval is configurable (environment variable bootpretransmitperiodinit) - transaction ID is kept the same for each BOOTP/DHCPv4 request (not recreated on each retry) For our application we'll use: - bootpretransmitperiodmax=16000 - bootpretransmitperiodinit=2000 A new configuration BOOTP_RANDOM_XID has been added to enable a randomized BOOTP/DHCPv4 transaction ID. Enhance DHCPv4 sending/parsing option 209 (PXE config file). A previous patch was accepted. A new patch fixes a possible double free() and addresses latest review comments. Changes in v7: - Remove "depends on LIB_RAND || LIB_HW_RAND" from CMD_BOOTP - Add CMD_BOOTP/CMD_DHCP/CMD_DHCP6 to LIB_RAND menu choice - Add space after "config CMD_DHCP" Changes in v6: - CMD_BOOTP should "depends on LIB_RAND || LIB_HW_RAND" Changes in v5: - add change log to individual patches - fix depends for BOOTP_RANDOM_XID: "depends on CMD_BOOTP && (LIB_RAND || LIB_HW_RAND)" Changes in v4: - rebase master and resolve conflicts - default y for BOOTP_PXE_DHCP_OPTION (feedback from review) - Add "select LIB_RAND" for "config CMD_BOOTP" (retransmission improvements require rand()) Changes in v3: - add define for option 209 and rfc5071 reference - Set RETRANSMIT_PERIOD_MAX_MS=6 - Add randomization factor to retransmission timeout - Add depends for BOOTP_RANDOM_XID Changes in v2: - use env_get_ulong() to get environment variables Sean Edmond (3): net: Enhancements for dhcp option 209 net: bootp: BOOTP/DHCPv4 retransmission improvements net: bootp: add config option BOOTP_RANDOM_XID cmd/Kconfig | 8 ++ cmd/pxe.c | 2 ++ lib/Kconfig | 3 ++- net/bootp.c | 76 + net/bootp.h | 2 ++ 5 files changed, 73 insertions(+), 18 deletions(-) -- 2.42.0
[PATCH v6 5/5] fdt: Fix compile error for !OFNODE_MULTI_TREE
Required to fix the following compile error when building sandbox:
/tmp/cci9ibby.ltrans21.ltrans.o: In function `do_cedit_load':
:(.text+0x601d): undefined reference to `oftree_dispose'
Signed-off-by: Sean Edmond
Reviewed-by: Simon Glass
---
(no changes since v3)
Changes in v3:
- fix compile error for sandbox for !OFNODE_MULTI_TREE
drivers/core/ofnode.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 557a4a1b969..f4af184a476 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -135,12 +135,6 @@ int oftree_new(oftree *treep)
return 0;
}
-void oftree_dispose(oftree tree)
-{
- if (of_live_active())
- of_live_free(tree.np);
-}
-
void *ofnode_lookup_fdt(ofnode node)
{
if (gd->flags & GD_FLG_RELOC) {
@@ -243,6 +237,12 @@ int oftree_new(oftree *treep)
#endif /* OFNODE_MULTI_TREE */
+void oftree_dispose(oftree tree)
+{
+ if (of_live_active())
+ of_live_free(tree.np);
+}
+
int oftree_to_fdt(oftree tree, struct abuf *buf)
{
int ret;
--
2.42.0
[PATCH v6 4/5] dm: core: Modify default for OFNODE_MULTI_TREE
There is a preference to use the "ofnode" API for FDT fixups moving forward. The FDT fixup will usually be for the kernel FDT. To fixup the kernel FDT with the ofnode API, it's required to set the OFNODE_MULTI_TREE option. To ensure existing users of kaslr fdt fixup are not impacted, Let's modify the default value for OFNODE_MULTI_TREE to ensure it's always set. This will cause a 1007 byte increase in the code size. Signed-off-by: Sean Edmond Reviewed-by: Simon Glass --- (no changes since v5) Changes in v5: - Always enable OFNODE_MULTI_TREE Changes in v4: - Fix compile issue when CONFIG_OF_CONTROL not set Changes in v2: - Change default config for OFNODE_MULTI_TREE (y if !OF_LIVE) drivers/core/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig index 737d4590d5b..c01a8dc7e0a 100644 --- a/drivers/core/Kconfig +++ b/drivers/core/Kconfig @@ -423,7 +423,8 @@ config DM_DEV_READ_INLINE config OFNODE_MULTI_TREE bool "Allow the ofnode interface to access any tree" - default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE + depends on OF_CONTROL + default y help Normally U-Boot makes use of its control FDT, the one used to bind devices and provide options. In some cases, U-Boot must also process -- 2.42.0
[PATCH v6 0/5] Populate kaslr seed with RNG
This patch series creates a common API (fdt_fixup_kaslr_seed()) for populating the kaslr seed in the DTB. Existing users (kaslrseed, and ARMv8 sec firmware) have been updated to use this common API. New functionality has been introduced to populate the kaslr using the RNG. This can be enabled with CONFIG_RNG_TPM_SEED. Changes in v6: - root_ofnode_from_fdt()->ofnode_root_from_fdt() - Enclose fdt_rng_kaslr_seed() in "#if defined(CONFIG_KASLR_RNG_SEED)" Changes in v5: - include dm/ofnode-decl.h instead of dm/ofnode.h - cast buffer to (u8 *) in kaslrseed.c - Always enable OFNODE_MULTI_TREE Changes in v4: - Fix compile issue when CONFIG_OF_CONTROL not set Changes in v3: - Use event spy to do the FDT fixup - Populate with RNG device instead of TPM device (this is a more generic solution) - fix compile error for sandbox for !OFNODE_MULTI_TREE Changes in v2: - fdt_fixup_kaslr_seed() uses the ofnode API - Add root_ofnode_from_fdt() to get the root node from an FDT and perform error checking on the oftree - add comments to exported functions - Add error checking in image_setup_libfdt() for return from fdt_tpm_kaslr_seed() - uclass_get_device() -> uclass_first_device_err() - Change default config for OFNODE_MULTI_TREE (y if !OF_LIVE) Dhananjay Phadke (2): fdt: common API to populate kaslr seed fdt: kaslr seed from RNG device Sean Edmond (3): cmd: kaslrseed: Use common API to fixup FDT dm: core: Modify default for OFNODE_MULTI_TREE fdt: Fix compile error for !OFNODE_MULTI_TREE arch/arm/cpu/armv8/sec_firmware.c | 39 -- boot/fdt_support.c| 54 +++ cmd/kaslrseed.c | 20 configs/sandbox_defconfig | 2 +- drivers/core/Kconfig | 3 +- drivers/core/ofnode.c | 29 + include/dm/ofnode.h | 12 +++ include/fdt_support.h | 9 ++ lib/Kconfig | 7 9 files changed, 129 insertions(+), 46 deletions(-) -- 2.42.0
[PATCH v6 3/5] cmd: kaslrseed: Use common API to fixup FDT
Use the newly introduced common API fdt_fixup_kaslr_seed() in the
kaslrseed command.
Signed-off-by: Sean Edmond
Reviewed-by: Simon Glass
---
Changes in v6:
- root_ofnode_from_fdt()->ofnode_root_from_fdt()
- "goto CMD_RET_FAILURE" -> "return CMD_RET_FAILURE"
Changes in v5:
- cast buffer to (u8 *) in kaslrseed.c
cmd/kaslrseed.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/cmd/kaslrseed.c b/cmd/kaslrseed.c
index 9acb8e16386..9339d82649b 100644
--- a/cmd/kaslrseed.c
+++ b/cmd/kaslrseed.c
@@ -19,7 +19,7 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, int
argc, char *const
size_t n = 0x8;
struct udevice *dev;
u64 *buf;
- int nodeoffset;
+ ofnode root;
int ret = CMD_RET_SUCCESS;
if (uclass_get_device(UCLASS_RNG, 0, ) || !dev) {
@@ -45,21 +45,15 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag,
int argc, char *const
return CMD_RET_FAILURE;
}
- ret = fdt_check_header(working_fdt);
- if (ret < 0) {
- printf("fdt_chosen: %s\n", fdt_strerror(ret));
+ ret = ofnode_root_from_fdt(working_fdt, );
+ if (ret) {
+ printf("ERROR: Unable to get root ofnode\n");
return CMD_RET_FAILURE;
}
- nodeoffset = fdt_find_or_add_subnode(working_fdt, 0, "chosen");
- if (nodeoffset < 0) {
- printf("Reading chosen node failed\n");
- return CMD_RET_FAILURE;
- }
-
- ret = fdt_setprop(working_fdt, nodeoffset, "kaslr-seed", buf,
sizeof(buf));
- if (ret < 0) {
- printf("Unable to set kaslr-seed on chosen node: %s\n",
fdt_strerror(ret));
+ ret = fdt_fixup_kaslr_seed(root, (u8 *)buf, sizeof(buf));
+ if (ret) {
+ printf("ERROR: failed to add kaslr-seed to fdt\n");
return CMD_RET_FAILURE;
}
--
2.42.0
[PATCH v6 2/5] fdt: kaslr seed from RNG device
From: Dhananjay Phadke
Add support for KASLR seed from the RNG device. Invokes dm_rng_read()
API to read 8-bytes of random bytes. Performs the FDT fixup using event
spy. To enable use CONFIG_KASLR_RNG_SEED
Signed-off-by: Dhananjay Phadke
Signed-off-by: Drew Kluemke
Signed-off-by: Sean Edmond
Reviewed-by: Simon Glass
BAH
---
Changes in v6:
- Enclose fdt_rng_kaslr_seed() in "#if defined(CONFIG_KASLR_RNG_SEED)"
Changes in v3:
- Populate with RNG device instead of TPM device (this is a more generic
solution)
Changes in v2:
- Add error checking in image_setup_libfdt() for return from
fdt_tpm_kaslr_seed()
- uclass_get_device() -> uclass_first_device_err()
boot/fdt_support.c| 35 +++
configs/sandbox_defconfig | 2 +-
lib/Kconfig | 7 +++
3 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/boot/fdt_support.c b/boot/fdt_support.c
index 49d14a949be..b51154f83a5 100644
--- a/boot/fdt_support.c
+++ b/boot/fdt_support.c
@@ -12,7 +12,10 @@
#include
#include
#include
+#include
#include
+#include
+#include
#include
#include
#include
@@ -650,6 +653,38 @@ int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int
len)
return 0;
}
+#if defined(CONFIG_KASLR_RNG_SEED)
+int fdt_rng_kaslr_seed(void *ctx, struct event *event)
+{
+ u8 rand[8] = {0};
+ struct udevice *dev;
+ int ret;
+ oftree tree = event->data.ft_fixup.tree;
+ ofnode root_node = oftree_root(tree);
+
+ ret = uclass_first_device_err(UCLASS_RNG, );
+ if (ret) {
+ printf("ERROR: Failed to find RNG device\n");
+ return ret;
+ }
+
+ ret = dm_rng_read(dev, rand, sizeof(rand));
+ if (ret) {
+ printf("ERROR: RNG read failed, ret=%d\n", ret);
+ return ret;
+ }
+
+ ret = fdt_fixup_kaslr_seed(root_node, rand, sizeof(rand));
+ if (ret) {
+ printf("ERROR: failed to add kaslr-seed to fdt\n");
+ return ret;
+ }
+
+ return 0;
+}
+EVENT_SPY_FULL(EVT_FT_FIXUP, fdt_rng_kaslr_seed);
+#endif
+
int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index bc5bcb2a623..b71790e1532 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -348,4 +348,4 @@ CONFIG_EFI_SECURE_BOOT=y
CONFIG_TEST_FDTDEC=y
CONFIG_UNIT_TEST=y
CONFIG_UT_TIME=y
-CONFIG_UT_DM=y
+CONFIG_UT_DM=y
\ No newline at end of file
diff --git a/lib/Kconfig b/lib/Kconfig
index 19649517a39..4f5dfc00d6f 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -477,6 +477,13 @@ config VPL_TPM
for the low-level TPM interface, but only one TPM is supported at
a time by the TPM library.
+config KASLR_RNG_SEED
+ bool "Use RNG driver for KASLR random seed"
+ depends on DM_RNG
+ help
+ This enables support for using the RNG driver as entropy source for
+ KASLR seed populated in kernel's device tree.
+
endmenu
menu "Android Verified Boot"
--
2.42.0
[PATCH v6 1/5] fdt: common API to populate kaslr seed
From: Dhananjay Phadke
fdt_fixup_kaslr_seed() will update given ofnode with random seed value.
Source for random seed can be TPM or RNG driver in u-boot or sec
firmware (ARM).
Signed-off-by: Dhananjay Phadke
Signed-off-by: Sean Edmond
Signed-off-by: Sean Edmond
---
Changes in v6:
- root_ofnode_from_fdt()->ofnode_root_from_fdt()
Changes in v5:
- include dm/ofnode-decl.h instead of dm/ofnode.h
Changes in v3:
- Use event spy to do the FDT fixup
Changes in v2:
- fdt_fixup_kaslr_seed() uses the ofnode API
- Add root_ofnode_from_fdt() to get the root node from an FDT and
perform error checking on the oftree
- add comments to exported functions
arch/arm/cpu/armv8/sec_firmware.c | 39 +++
boot/fdt_support.c| 19 +++
drivers/core/ofnode.c | 17 ++
include/dm/ofnode.h | 12 ++
include/fdt_support.h | 9 +++
5 files changed, 71 insertions(+), 25 deletions(-)
diff --git a/arch/arm/cpu/armv8/sec_firmware.c
b/arch/arm/cpu/armv8/sec_firmware.c
index c0e8726346f..5f256939277 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -411,46 +411,35 @@ int sec_firmware_init(const void *sec_firmware_img,
/*
* fdt_fix_kaslr - Add kalsr-seed node in Device tree
* @fdt: Device tree
- * @eret: 0 in case of error, 1 for success
+ * @eret: 0 for success
*/
int fdt_fixup_kaslr(void *fdt)
{
- int nodeoffset;
- int err, ret = 0;
- u8 rand[8];
+ int ret = 0;
#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT)
+ u8 rand[8];
+ ofnode root;
+
/* Check if random seed generation is supported */
if (sec_firmware_support_hwrng() == false) {
printf("WARNING: SEC firmware not running, no kaslr-seed\n");
- return 0;
+ return -EOPNOTSUPP;
}
- err = sec_firmware_get_random(rand, 8);
- if (err < 0) {
+ ret = sec_firmware_get_random(rand, 8);
+ if (ret < 0) {
printf("WARNING: No random number to set kaslr-seed\n");
- return 0;
+ return ret;
}
- err = fdt_check_header(fdt);
- if (err < 0) {
- printf("fdt_chosen: %s\n", fdt_strerror(err));
- return 0;
+ ret = ofnode_root_from_fdt(fdt, );
+ if (ret < 0) {
+ printf("WARNING: Unable to get root ofnode\n");
+ return ret;
}
- /* find or create "/chosen" node. */
- nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
- if (nodeoffset < 0)
- return 0;
-
- err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", rand,
- sizeof(rand));
- if (err < 0) {
- printf("WARNING: can't set kaslr-seed %s.\n",
- fdt_strerror(err));
- return 0;
- }
- ret = 1;
+ ret = fdt_fixup_kaslr_seed(root, rand, sizeof(rand));
#endif
return ret;
diff --git a/boot/fdt_support.c b/boot/fdt_support.c
index b15d07765fe..49d14a949be 100644
--- a/boot/fdt_support.c
+++ b/boot/fdt_support.c
@@ -631,6 +631,25 @@ void fdt_fixup_ethernet(void *fdt)
}
}
+int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len)
+{
+ ofnode chosen;
+ int ret;
+
+ /* find or create "/chosen" node. */
+ ret = ofnode_add_subnode(node, "chosen", );
+ if (ret && ret != -EEXIST)
+ return -ENOENT;
+
+ ret = ofnode_write_prop(chosen, "kaslr-seed", seed, len, true);
+ if (ret) {
+ printf("WARNING: can't set kaslr-seed\n");
+ return ret;
+ }
+
+ return 0;
+}
+
int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index f72ea416cf1..557a4a1b969 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -966,6 +966,23 @@ ofnode oftree_path(oftree tree, const char *path)
}
}
+int ofnode_root_from_fdt(void *fdt, ofnode *root_node)
+{
+ oftree tree;
+ /* If OFNODE_MULTI_TREE is not set, and if fdt is not the control FDT,
+* oftree_from_fdt() will return NULL
+*/
+ tree = oftree_from_fdt(fdt);
+
+ if (!oftree_valid(tree)) {
+ printf("Cannot create oftree\n");
+ return -EINVAL;
+ }
+ *root_node = oftree_root(tree);
+
+ return 0;
+}
+
const void *ofnode_read_chosen_prop(const char *propname, int *sizep)
{
ofnode chosen_node;
diff --git a/include/dm/o
Re: [PATCH v5 2/5] fdt: kaslr seed from RNG device
Hi Ilias, Applogies, I'm fairly new to upstream contributions and I've
missed a few things on multi-version patch series such as this one.
Simon has elightened about patman though (I feel a bit sheepish that I
haven't been using this tool all along). Should I resubmit this as a new
series to ensure prior reviews and change logs are captured?
Sean
On 2023-11-17 11:33 p.m., Ilias Apalodimas wrote:
Hi Sean
There are a few reviewed-by tags missing since v3/v4. Any reason you
dropped those? The change log doesn't mention anything
Thanks
/Ilias
On Fri, 17 Nov 2023 at 04:02, wrote:
From: Dhananjay Phadke
Add support for KASLR seed from the RNG device. Invokes dm_rng_read()
API to read 8-bytes of random bytes. Performs the FDT fixup using event
spy. To enable use CONFIG_KASLR_RNG_SEED
Signed-off-by: Dhananjay Phadke
Signed-off-by: Drew Kluemke
Signed-off-by: Sean Edmond
---
boot/fdt_support.c | 36
lib/Kconfig| 7 +++
2 files changed, 43 insertions(+)
diff --git a/boot/fdt_support.c b/boot/fdt_support.c
index 49d14a949be..12defcf645c 100644
--- a/boot/fdt_support.c
+++ b/boot/fdt_support.c
@@ -12,7 +12,10 @@
#include
#include
#include
+#include
#include
+#include
+#include
#include
#include
#include
@@ -650,6 +653,39 @@ int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int
len)
return 0;
}
+int fdt_rng_kaslr_seed(void *ctx, struct event *event)
+{
+ u8 rand[8] = {0};
+ struct udevice *dev;
+ int ret;
+ oftree tree = event->data.ft_fixup.tree;
+ ofnode root_node = oftree_root(tree);
+
+ ret = uclass_first_device_err(UCLASS_RNG, );
+ if (ret) {
+ printf("ERROR: Failed to find RNG device\n");
+ return ret;
+ }
+
+ ret = dm_rng_read(dev, rand, sizeof(rand));
+ if (ret) {
+ printf("ERROR: RNG read failed, ret=%d\n", ret);
+ return ret;
+ }
+
+ ret = fdt_fixup_kaslr_seed(root_node, rand, sizeof(rand));
+ if (ret) {
+ printf("ERROR: failed to add kaslr-seed to fdt\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+#if defined(CONFIG_KASLR_RNG_SEED)
+EVENT_SPY(EVT_FT_FIXUP, fdt_rng_kaslr_seed);
+#endif
+
int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/lib/Kconfig b/lib/Kconfig
index 19649517a39..4f5dfc00d6f 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -477,6 +477,13 @@ config VPL_TPM
for the low-level TPM interface, but only one TPM is supported at
a time by the TPM library.
+config KASLR_RNG_SEED
+ bool "Use RNG driver for KASLR random seed"
+ depends on DM_RNG
+ help
+ This enables support for using the RNG driver as entropy source for
+ KASLR seed populated in kernel's device tree.
+
endmenu
menu "Android Verified Boot"
--
2.42.0
[PATCH v6 3/3] net: bootp: add config option BOOTP_RANDOM_XID
The new config option BOOTP_RANDOM_XID will randomize the transaction ID
for each new BOOT/DHCPv4 exchange.
Signed-off-by: Sean Edmond
Reviewed-by: Tom Rini
---
(no changes since v5)
Changes in v5:
- fix depends for BOOTP_RANDOM_XID:
"depends on CMD_BOOTP && (LIB_RAND || LIB_HW_RAND)"
Changes in v3:
- Add depends for BOOTP_RANDOM_XID
cmd/Kconfig | 7 +++
net/bootp.c | 31 +--
2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 6b7c5e6d521..8204f72f6ac 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1853,6 +1853,13 @@ config BOOTP_VCI_STRING
default "U-Boot.arm" if ARM
default "U-Boot"
+config BOOTP_RANDOM_XID
+ bool "Send random transaction ID to BOOTP/DHCP server"
+ depends on CMD_BOOTP && (LIB_RAND || LIB_HW_RAND)
+ help
+ Selecting this will allow for a random transaction ID to get
+ selected for each BOOTP/DHCPv4 exchange.
+
if CMD_DHCP6
config DHCP6_PXE_CLIENTARCH
diff --git a/net/bootp.c b/net/bootp.c
index 89425717328..49cd9064436 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -837,22 +837,25 @@ void bootp_request(void)
/* Only generate a new transaction ID for each new BOOTP request */
if (bootp_try == 1) {
- /*
-* Bootp ID is the lower 4 bytes of our ethernet address
-* plus the current time in ms.
-*/
- bootp_id = ((u32)net_ethaddr[2] << 24)
- | ((u32)net_ethaddr[3] << 16)
- | ((u32)net_ethaddr[4] << 8)
- | (u32)net_ethaddr[5];
- bootp_id += get_timer(0);
- bootp_id = htonl(bootp_id);
- bootp_add_id(bootp_id);
- net_copy_u32(>bp_id, _id);
- } else {
- net_copy_u32(>bp_id, _id);
+ if (IS_ENABLED(CONFIG_BOOTP_RANDOM_XID)) {
+ srand(get_ticks() + rand());
+ bootp_id = rand();
+ } else {
+ /*
+* Bootp ID is the lower 4 bytes of our ethernet
address
+* plus the current time in ms.
+*/
+ bootp_id = ((u32)net_ethaddr[2] << 24)
+ | ((u32)net_ethaddr[3] << 16)
+ | ((u32)net_ethaddr[4] << 8)
+ | (u32)net_ethaddr[5];
+ bootp_id += get_timer(0);
+ bootp_id = htonl(bootp_id);
+ }
}
+ bootp_add_id(bootp_id);
+ net_copy_u32(>bp_id, _id);
/*
* Calculate proper packet lengths taking into account the
* variable size of the options field
--
2.42.0
[PATCH v6 2/3] net: bootp: BOOTP/DHCPv4 retransmission improvements
This patch introduces 3 improvements to align with RFC 951:
- retransmission backoff interval maximum is configurable
- initial retranmission backoff interval is configurable
- transaction ID is kept the same for each BOOTP/DHCPv4 request
In applications where thousands of nodes are serviced by a single DHCP
server, maximizing the retransmission backoff interval at 2 seconds (the
current u-boot default) exerts high pressure on the DHCP server and
network layer.
RFC 951 “7.2. Client Retransmission Strategy” states that the
retransmission backoff interval should be limited to 60 seconds. This
patch allows the interval to be configurable using the environment
variable "bootpretransmitperiodmax"
The initial retranmission backoff period defaults to 250ms, which is
also too small for these scenarios with many clients. This patch makes
the initial retransmission interval to be configurable using the
environment variable "bootpretransmitperiodinit".
Also, on a retransmission it is not expected for the transaction ID to
change (only the 'secs' field should be updated). Let's save the
transaction ID and use the same transaction ID for each BOOTP/DHCPv4
exchange.
Signed-off-by: Sean Edmond
BAH
---
Changes in v6:
- CMD_BOOTP should "depends on LIB_RAND || LIB_HW_RAND"
Changes in v4:
- Add "select LIB_RAND" for "config CMD_BOOTP" (retransmission
improvements require rand())
Changes in v3:
- Set RETRANSMIT_PERIOD_MAX_MS=6
- Add randomization factor to retransmission timeout
Changes in v2:
- use env_get_ulong() to get environment variables
cmd/Kconfig | 2 +-
net/bootp.c | 71 +
2 files changed, 56 insertions(+), 17 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index ae846b204dc..6b7c5e6d521 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1716,6 +1716,7 @@ if CMD_NET
config CMD_BOOTP
bool "bootp"
+ depends on LIB_RAND || LIB_HW_RAND
default y
help
bootp - boot image via network using BOOTP/TFTP protocol
@@ -1725,7 +1726,6 @@ config CMD_DHCP
depends on CMD_BOOTP
help
Boot image via network using DHCP/TFTP protocol
-
config CMD_DHCP6
bool "dhcp6"
depends on IPV6
diff --git a/net/bootp.c b/net/bootp.c
index 24547cf7bfd..89425717328 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -42,6 +42,22 @@
*/
#define TIMEOUT_MS ((3 + (CONFIG_NET_RETRY_COUNT * 5)) * 1000)
+/*
+ * According to rfc951 : 7.2. Client Retransmission Strategy
+ * "After the 'average' backoff reaches about 60 seconds, it should be
+ * increased no further, but still randomized."
+ *
+ * U-Boot has saturated this backoff at 2 seconds for a long time.
+ * To modify, set the environment variable "bootpretransmitperiodmax"
+ */
+#define RETRANSMIT_PERIOD_MAX_MS 6
+
+/* Retransmission timeout for the initial packet (in milliseconds).
+ * This timeout will double on each retry. To modify, set the
+ * environment variable bootpretransmitperiodinit.
+ */
+#define RETRANSMIT_PERIOD_INIT_MS 250
+
#ifndef CFG_DHCP_MIN_EXT_LEN /* minimal length of extension list */
#define CFG_DHCP_MIN_EXT_LEN 64
#endif
@@ -53,6 +69,7 @@
u32bootp_ids[CFG_BOOTP_ID_CACHE_SIZE];
unsigned int bootp_num_ids;
intbootp_try;
+u32bootp_id;
ulong bootp_start;
ulong bootp_timeout;
char net_nis_domain[32] = {0,}; /* Our NIS domain */
@@ -60,6 +77,7 @@ char net_hostname[32] = {0,}; /* Our hostname */
char net_root_path[CONFIG_BOOTP_MAX_ROOT_PATH_LEN] = {0,}; /* Our bootpath */
static ulong time_taken_max;
+static u32 retransmit_period_max_ms;
#if defined(CONFIG_CMD_DHCP)
static dhcp_state_t dhcp_state = INIT;
@@ -396,6 +414,7 @@ static void bootp_handler(uchar *pkt, unsigned dest, struct
in_addr sip,
static void bootp_timeout_handler(void)
{
ulong time_taken = get_timer(bootp_start);
+ int rand_minus_plus_100;
if (time_taken >= time_taken_max) {
#ifdef CONFIG_BOOTP_MAY_FAIL
@@ -414,8 +433,17 @@ static void bootp_timeout_handler(void)
}
} else {
bootp_timeout *= 2;
- if (bootp_timeout > 2000)
- bootp_timeout = 2000;
+ if (bootp_timeout > retransmit_period_max_ms)
+ bootp_timeout = retransmit_period_max_ms;
+
+ /* Randomize by adding bootp_timeout*RAND, where RAND
+* is a randomization factor between -0.1..+0.1
+*/
+ srand(get_ticks() + rand());
+ rand_minus_plus_100 = ((rand() % 200) - 100);
+ bootp_timeout = bootp_timeout +
+ (((int)bootp_timeout * rand_minus_plus_100) /
1000);
+
net_set_timeout_handler(bootp_timeout, bootp_timeout_handler);
bo
[PATCH v6 1/3] net: Enhancements for dhcp option 209
- Enable option 209 by default
- Set pxelinux_configfile to NULL to avoid potential double free
- change hardcoced 209 to a define
Signed-off-by: Sean Edmond
---
Changes in v6:
- Reword this commit
Changes in v4:
- rebase master and resolve conflicts
- default y for BOOTP_PXE_DHCP_OPTION (feedback from review)
- change commit description for this patch (this is now
an enhancement patch)
Changes in v3:
- add define for option 209 and rfc5071 reference
cmd/Kconfig | 1 +
cmd/pxe.c | 2 ++
net/bootp.c | 4 ++--
net/bootp.h | 2 ++
4 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index df6d71c103f..ae846b204dc 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1842,6 +1842,7 @@ config BOOTP_PXE_CLIENTARCH
config BOOTP_PXE_DHCP_OPTION
bool "Request & store 'pxe_configfile' from BOOTP/DHCP server"
+ default y
depends on BOOTP_PXE
config BOOTP_VCI_STRING
diff --git a/cmd/pxe.c b/cmd/pxe.c
index 21134eb7a30..9404f445187 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;
}
diff --git a/net/bootp.c b/net/bootp.c
index 68002909634..24547cf7bfd 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -603,7 +603,7 @@ static int dhcp_extended(u8 *e, int message_type, struct
in_addr server_ip,
*cnt += 1;
#endif
if (IS_ENABLED(CONFIG_BOOTP_PXE_DHCP_OPTION)) {
- *e++ = 209; /* PXELINUX Config File */
+ *e++ = DHCP_OPTION_PXE_CONFIG_FILE; /* PXELINUX Config File
*/
*cnt += 1;
}
/* no options, so back up to avoid sending an empty request list */
@@ -914,7 +914,7 @@ static void dhcp_process_options(uchar *popt, uchar *end)
net_boot_file_name[size] = 0;
}
break;
- case 209: /* PXELINUX Config File */
+ case DHCP_OPTION_PXE_CONFIG_FILE: /* PXELINUX Config File
*/
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.
diff --git a/net/bootp.h b/net/bootp.h
index 4e32b19d424..24b32c73f62 100644
--- a/net/bootp.h
+++ b/net/bootp.h
@@ -91,6 +91,8 @@ typedef enum { INIT,
#define DHCP_NAK 6
#define DHCP_RELEASE 7
+#define DHCP_OPTION_PXE_CONFIG_FILE209 /* "ConfigFile" option
according to rfc5071 */
+
/**/
#endif /* __BOOTP_H__ */
--
2.42.0
[PATCH v6 0/3] BOOTP/DHCPv4 enhancements
In our datacenter application, a single DHCP server is servicing 36000+ clients. Improvements are required to the DHCPv4 retransmission behavior to align with RFC and ensure less pressure is exerted on the server: - retransmission backoff interval maximum is configurable (environment variable bootpretransmitperiodmax) - initial retransmission backoff interval is configurable (environment variable bootpretransmitperiodinit) - transaction ID is kept the same for each BOOTP/DHCPv4 request (not recreated on each retry) For our application we'll use: - bootpretransmitperiodmax=16000 - bootpretransmitperiodinit=2000 A new configuration BOOTP_RANDOM_XID has been added to enable a randomized BOOTP/DHCPv4 transaction ID. Enhance DHCPv4 sending/parsing option 209 (PXE config file). A previous patch was accepted. A new patch fixes a possible double free() and addresses latest review comments. Changes in v6: - CMD_BOOTP should "depends on LIB_RAND || LIB_HW_RAND" Changes in v5: - add change log to individual patches - fix depends for BOOTP_RANDOM_XID: "depends on CMD_BOOTP && (LIB_RAND || LIB_HW_RAND)" Changes in v4: - rebase master and resolve conflicts - default y for BOOTP_PXE_DHCP_OPTION (feedback from review) - Add "select LIB_RAND" for "config CMD_BOOTP" (retransmission improvements require rand()) Changes in v3: - add define for option 209 and rfc5071 reference - Set RETRANSMIT_PERIOD_MAX_MS=6 - Add randomization factor to retransmission timeout - Add depends for BOOTP_RANDOM_XID Changes in v2: - use env_get_ulong() to get environment variables Sean Edmond (3): net: Enhancements for dhcp option 209 net: bootp: BOOTP/DHCPv4 retransmission improvements net: bootp: add config option BOOTP_RANDOM_XID cmd/Kconfig | 10 ++- cmd/pxe.c | 2 ++ net/bootp.c | 76 + net/bootp.h | 2 ++ 4 files changed, 72 insertions(+), 18 deletions(-) -- 2.42.0
Re: [PATCH v2 1/3] net: Get pxe config file from dhcp option 209
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
Allow dhcp server pass pxe config file full path by using option 209
Signed-off-by: Sean Edmond
---
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(, 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(, 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
#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
Re: [PATCH v2 2/3] net: bootp: BOOTP/DHCPv4 retransmission improvements
On 2023-10-23 11:06 p.m., Heinrich Schuchardt wrote:
On 10/24/23 02:21, seanedm...@linux.microsoft.com wrote:
From: Sean Edmond
This patch introduces 3 improvements to align with RFC 951:
- retransmission backoff interval maximum is configurable
- initial retranmission backoff interval is configurable
- transaction ID is kept the same for each BOOTP/DHCPv4 request
In applications where thousands of nodes are serviced by a single DHCP
server, maximizing the retransmission backoff interval at 2 seconds (the
current u-boot default) exerts high pressure on the DHCP server and
network layer.
RFC 951 “7.2. Client Retransmission Strategy” states that the
retransmission backoff interval should maximize at 60 seconds. This
%s/maximize at/be limited to/
patch allows the interval to be configurable using the environment
variable "bootpretransmitperiodmax"
If there is an RFC defining the value, why do we need an environment
variable?
The initial retranmission backoff period defaults to 250ms, which is
also too small for these scenarios with many clients. This patch makes
the initial retransmission interval to be configurable using the
environment variable "bootpretransmitperiodinit".
Also, on a retransmission it is not expected for the transaction ID to
change (only the 'secs' field should be updated). Let's save the
transaction ID and use the same transaction ID for each BOOTP/DHCPv4
exchange.
Signed-off-by: Sean Edmond
---
net/bootp.c | 56 ++---
1 file changed, 40 insertions(+), 16 deletions(-)
diff --git a/net/bootp.c b/net/bootp.c
index 6800290963..bab17a9ceb 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -42,6 +42,17 @@
*/
#define TIMEOUT_MS ((3 + (CONFIG_NET_RETRY_COUNT * 5)) * 1000)
+/*
+ * According to rfc951 : 7.2. Client Retransmission Strategy
+ * "After the 'average' backoff reaches about 60 seconds, it should be
+ * increased no further, but still randomized."
+ *
+ * U-Boot has saturated this backoff at 2 seconds for a long time.
+ * To modify, set the environment variable "bootpretransmitperiodmax"
+ */
+#define RETRANSMIT_PERIOD_MAX_MS 2000
This does not match RFC 951. Please, why shouldn't we use the standard
value by default?
+#define RETRANSMIT_PERIOD_INIT_MS 250
This constant should be described too.
+
#ifndef CFG_DHCP_MIN_EXT_LEN /* minimal length of extension
list */
#define CFG_DHCP_MIN_EXT_LEN 64
#endif
@@ -53,6 +64,7 @@
u32 bootp_ids[CFG_BOOTP_ID_CACHE_SIZE];
unsigned int bootp_num_ids;
int bootp_try;
+u32 bootp_id;
ulong bootp_start;
ulong bootp_timeout;
char net_nis_domain[32] = {0,}; /* Our NIS domain */
@@ -60,6 +72,7 @@ char net_hostname[32] = {0,}; /* Our hostname */
char net_root_path[CONFIG_BOOTP_MAX_ROOT_PATH_LEN] = {0,}; /* Our
bootpath */
static ulong time_taken_max;
+static u32 retransmit_period_max_ms;
#if defined(CONFIG_CMD_DHCP)
static dhcp_state_t dhcp_state = INIT;
@@ -414,8 +427,8 @@ static void bootp_timeout_handler(void)
}
} else {
bootp_timeout *= 2;
- if (bootp_timeout > 2000)
- bootp_timeout = 2000;
+ if (bootp_timeout > retransmit_period_max_ms)
+ bootp_timeout = retransmit_period_max_ms;
RFC 951 requires that the backoff time is randomized.
I added randomization in v3. Note, in that update I haven't put any
guards around the use of rand()/srand() (as is the case in net_rand.h).
Perhaps it's safe to assume it will always be present for inclusion of net?
Best regards
Heinrich
net_set_timeout_handler(bootp_timeout, bootp_timeout_handler);
bootp_request();
}
@@ -711,10 +724,14 @@ static int bootp_extended(u8 *e)
void bootp_reset(void)
{
+ char *ep; /* Environment pointer */
+
bootp_num_ids = 0;
bootp_try = 0;
bootp_start = get_timer(0);
- bootp_timeout = 250;
+
+ bootp_timeout = env_get_ulong("bootpretransmitperiodinit", 10,
RETRANSMIT_PERIOD_INIT_MS);
+
}
void bootp_request(void)
@@ -726,7 +743,6 @@ void bootp_request(void)
#ifdef CONFIG_BOOTP_RANDOM_DELAY
ulong rand_ms;
#endif
- u32 bootp_id;
struct in_addr zero_ip;
struct in_addr bcast_ip;
char *ep; /* Environment pointer */
@@ -742,6 +758,9 @@ void bootp_request(void)
else
time_taken_max = TIMEOUT_MS;
+ retransmit_period_max_ms =
env_get_ulong("bootpretransmitperiodmax", 10,
+ RETRANSMIT_PERIOD_MAX_MS);
+
#ifdef CONFIG_BOOTP_RANDOM_DELAY /* Random BOOTP delay */
if (bootp_try == 0)
srand_mac();
@@ -801,18 +820,23 @@ void bootp_request(void)
extlen = bootp_extended((u8 *)bp->bp_vend);
#endif
- /*
- * Bootp ID is the lower 4 bytes of our ethernet address
- * plus the current time in ms.
- */
- bootp_i
Re: [PATCH v2 1/3] net: Get pxe config file from dhcp option 209
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
Allow dhcp server pass pxe config file full path by using option 209
Signed-off-by: Sean Edmond
---
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?
+
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(, 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(, 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
#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))
Re: [PATCH v2 2/3] net: bootp: BOOTP/DHCPv4 retransmission improvements
On 2023-10-23 11:06 p.m., Heinrich Schuchardt wrote:
On 10/24/23 02:21, seanedm...@linux.microsoft.com wrote:
From: Sean Edmond
This patch introduces 3 improvements to align with RFC 951:
- retransmission backoff interval maximum is configurable
- initial retranmission backoff interval is configurable
- transaction ID is kept the same for each BOOTP/DHCPv4 request
In applications where thousands of nodes are serviced by a single DHCP
server, maximizing the retransmission backoff interval at 2 seconds (the
current u-boot default) exerts high pressure on the DHCP server and
network layer.
RFC 951 “7.2. Client Retransmission Strategy” states that the
retransmission backoff interval should maximize at 60 seconds. This
%s/maximize at/be limited to/
patch allows the interval to be configurable using the environment
variable "bootpretransmitperiodmax"
If there is an RFC defining the value, why do we need an environment
variable?
The intention with this patch is to provide the option to satisfy RFC,
while giving the option to preserve the existing behavior. The
retransmission timing parameters in u-boot have been the same for the
last ~10 years, so I'm guessing some have gotten used to this behavior
(even though it's not correct).
The initial retranmission backoff period defaults to 250ms, which is
also too small for these scenarios with many clients. This patch makes
the initial retransmission interval to be configurable using the
environment variable "bootpretransmitperiodinit".
Also, on a retransmission it is not expected for the transaction ID to
change (only the 'secs' field should be updated). Let's save the
transaction ID and use the same transaction ID for each BOOTP/DHCPv4
exchange.
Signed-off-by: Sean Edmond
---
net/bootp.c | 56 ++---
1 file changed, 40 insertions(+), 16 deletions(-)
diff --git a/net/bootp.c b/net/bootp.c
index 6800290963..bab17a9ceb 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -42,6 +42,17 @@
*/
#define TIMEOUT_MS ((3 + (CONFIG_NET_RETRY_COUNT * 5)) * 1000)
+/*
+ * According to rfc951 : 7.2. Client Retransmission Strategy
+ * "After the 'average' backoff reaches about 60 seconds, it should be
+ * increased no further, but still randomized."
+ *
+ * U-Boot has saturated this backoff at 2 seconds for a long time.
+ * To modify, set the environment variable "bootpretransmitperiodmax"
+ */
+#define RETRANSMIT_PERIOD_MAX_MS 2000
This does not match RFC 951. Please, why shouldn't we use the standard
value by default?
You're right, better to use the standard value by default.
+#define RETRANSMIT_PERIOD_INIT_MS 250
This constant should be described too.
Will add a description.
+
#ifndef CFG_DHCP_MIN_EXT_LEN /* minimal length of extension
list */
#define CFG_DHCP_MIN_EXT_LEN 64
#endif
@@ -53,6 +64,7 @@
u32 bootp_ids[CFG_BOOTP_ID_CACHE_SIZE];
unsigned int bootp_num_ids;
int bootp_try;
+u32 bootp_id;
ulong bootp_start;
ulong bootp_timeout;
char net_nis_domain[32] = {0,}; /* Our NIS domain */
@@ -60,6 +72,7 @@ char net_hostname[32] = {0,}; /* Our hostname */
char net_root_path[CONFIG_BOOTP_MAX_ROOT_PATH_LEN] = {0,}; /* Our
bootpath */
static ulong time_taken_max;
+static u32 retransmit_period_max_ms;
#if defined(CONFIG_CMD_DHCP)
static dhcp_state_t dhcp_state = INIT;
@@ -414,8 +427,8 @@ static void bootp_timeout_handler(void)
}
} else {
bootp_timeout *= 2;
- if (bootp_timeout > 2000)
- bootp_timeout = 2000;
+ if (bootp_timeout > retransmit_period_max_ms)
+ bootp_timeout = retransmit_period_max_ms;
RFC 951 requires that the backoff time is randomized.
I'll can look into adding randomization to this as well.
Best regards
Heinrich
net_set_timeout_handler(bootp_timeout, bootp_timeout_handler);
bootp_request();
}
@@ -711,10 +724,14 @@ static int bootp_extended(u8 *e)
void bootp_reset(void)
{
+ char *ep; /* Environment pointer */
+
bootp_num_ids = 0;
bootp_try = 0;
bootp_start = get_timer(0);
- bootp_timeout = 250;
+
+ bootp_timeout = env_get_ulong("bootpretransmitperiodinit", 10,
RETRANSMIT_PERIOD_INIT_MS);
+
}
void bootp_request(void)
@@ -726,7 +743,6 @@ void bootp_request(void)
#ifdef CONFIG_BOOTP_RANDOM_DELAY
ulong rand_ms;
#endif
- u32 bootp_id;
struct in_addr zero_ip;
struct in_addr bcast_ip;
char *ep; /* Environment pointer */
@@ -742,6 +758,9 @@ void bootp_request(void)
else
time_taken_max = TIMEOUT_MS;
+ retransmit_period_max_ms =
env_get_ulong("bootpretransmitperiodmax", 10,
+ RETRANSMIT_PERIOD_MAX_MS);
+
#ifdef CONFIG_BOOTP_RANDOM_DELAY /* Random BOOTP delay */
if (bootp_try == 0)
srand_mac();
@@ -801,18 +820,23
Re: [PATCHv10 14/15] net/lwip: replace original net commands with lwip
On 2023-10-03 2:58 p.m., Peter Robinson wrote:
On Tue, Oct 3, 2023 at 6:58 PM Sean Edmond
wrote:
On 2023-09-26 2:41 a.m., Maxim Uvarov wrote:
Replace original commands: ping, tftp, dhcp and wget.
Signed-off-by: Maxim Uvarov
---
boot/bootmeth_efi.c | 18 +++---
boot/bootmeth_pxe.c | 21 ++-
cmd/net.c | 86 +
cmd/pxe.c | 19 +-
include/net.h | 8 +++--
include/net/ulwip.h | 64 +
6 files changed, 113 insertions(+), 103 deletions(-)
create mode 100644 include/net/ulwip.h
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index ae936c8daa..52399d627c 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -20,6 +20,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -319,9 +321,7 @@ static int distro_efi_try_bootflow_files(struct udevice
*dev,
static int distro_efi_read_bootflow_net(struct bootflow *bflow)
{
- char file_addr[17], fname[256];
- char *tftp_argv[] = {"tftp", file_addr, fname, NULL};
- struct cmd_tbl cmdtp = {}; /* dummy */
+ char fname[256];
const char *addr_str, *fdt_addr_str;
int ret, arch, size;
ulong addr, fdt_addr;
@@ -368,7 +368,6 @@ static int distro_efi_read_bootflow_net(struct bootflow
*bflow)
if (!fdt_addr_str)
return log_msg_ret("fdt", -EINVAL);
fdt_addr = hextoul(fdt_addr_str, NULL);
- sprintf(file_addr, "%lx", fdt_addr);
/* We only allow the first prefix with PXE */
ret = distro_efi_get_fdt_name(fname, sizeof(fname), 0);
@@ -379,7 +378,16 @@ static int distro_efi_read_bootflow_net(struct bootflow
*bflow)
if (!bflow->fdt_fname)
return log_msg_ret("fil", -ENOMEM);
- if (!do_tftpb(, 0, 3, tftp_argv)) {
+ ret = ulwip_init();
+ if (ret)
+ return log_msg_ret("ulwip_init", ret);
+
+ ret = ulwip_tftp(fdt_addr, fname);
+ if (ret)
+ return log_msg_ret("ulwip_tftp", ret);
+
+ ret = ulwip_loop();
+ if (!ret) {
bflow->fdt_size = env_get_hex("filesize", 0);
bflow->fdt_addr = fdt_addr;
} else {
diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c
index 8d489a11aa..fc6aabaa18 100644
--- a/boot/bootmeth_pxe.c
+++ b/boot/bootmeth_pxe.c
@@ -21,6 +21,8 @@
#include
#include
#include
+#include
+#include
#include
static int extlinux_pxe_getfile(struct pxe_context *ctx, const char
*file_path,
@@ -116,18 +118,21 @@ static int extlinux_pxe_read_file(struct udevice *dev,
struct bootflow *bflow,
const char *file_path, ulong addr,
ulong *sizep)
{
- char *tftp_argv[] = {"tftp", NULL, NULL, NULL};
- struct pxe_context *ctx = dev_get_priv(dev);
- char file_addr[17];
ulong size;
int ret;
- sprintf(file_addr, "%lx", addr);
- tftp_argv[1] = file_addr;
- tftp_argv[2] = (void *)file_path;
+ ret = ulwip_init();
+ if (ret)
+ return log_msg_ret("ulwip_init", ret);
+
+ ret = ulwip_tftp(addr, file_path);
+ if (ret)
+ return log_msg_ret("ulwip_tftp", ret);
+
+ ret = ulwip_loop();
+ if (ret)
+ return log_msg_ret("ulwip_loop", ret);
- if (do_tftpb(ctx->cmdtp, 0, 3, tftp_argv))
- return -ENOENT;
ret = pxe_get_file_size();
if (ret)
return log_msg_ret("tftp", ret);
diff --git a/cmd/net.c b/cmd/net.c
index d407d8320a..dc5a114309 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
static int netboot_common(enum proto_t, struct cmd_tbl *, int, char * const
[]);
@@ -40,19 +41,9 @@ U_BOOT_CMD(
#endif
#ifdef CONFIG_CMD_TFTPBOOT
-int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
-{
- int ret;
-
- bootstage_mark_name(BOOTSTAGE_KERNELREAD_START, "tftp_start");
- ret = netboot_common(TFTPGET, cmdtp, argc, argv);
- bootstage_mark_name(BOOTSTAGE_KERNELREAD_STOP, "tftp_done");
- return ret;
-}
-
#if IS_ENABLED(CONFIG_IPV6)
U_BOOT_CMD(
- tftpboot, 4, 1, do_tftpb,
+ tftpboot, 4, 1, do_lwip_tftp,
It looks like LWIP doesn't support TFTP with IPv6 addressing. Perhaps we need
to fall back onto the existing TFTP implementation until LWIP supports it?
Is it that LWIP upstream doesn't support IPv6 with TFTP or it just
hasn't been dealt with in this patch set? If the former might be
useful to reference details.
Apologies for the misleading comment, LWIP does support IPv6 addressing,
it just hasn't been dealt with in this patch.
Note, that currently, IPv6 TFTP is enabled using the "-ipv6" argument. The intention is that
netboot_common() sees the argument and sets the "use_ip6" variable. It looks like the new
implementation in do_lwip_tftp() doesn't re-use the argument parsing in netboot_common() and that it doesn't
handle the addition of the "-ipv6" flag.
Is there a reason why there's a need of an explicit argument for IPv6?
I would have thought if there was a local IP
Re: [PATCH 2/3] net: bootp: BOOTP/DHCPv4 retransmission improvements
On 2023-09-26 9:11 p.m., Lothar Waßmann wrote:
Hi,
On Mon, 25 Sep 2023 13:29:34 -0700 seanedm...@linux.microsoft.com wrote:
From: Sean Edmond
This patch introduces 3 improvements to align with RFC 951:
- retransmission backoff interval maximum is configurable
- initial retranmission backoff interval is configurable
- transaction ID is kept the same for each BOOTP/DHCPv4 request
In applications where thousands of nodes are serviced by a single DHCP
server, maximizing the retransmission backoff interval at 2 seconds (the
current u-boot default) exerts high pressure on the DHCP server and
network layer.
RFC 951 “7.2. Client Retransmission Strategy” states that the
retransmission backoff interval should maximize at 60 seconds. This
patch allows the interval to be configurable using the environment
variable "bootpretransmitperiodmax"
The initial retranmission backoff period defaults to 250ms, which is
also too small for these scenarios with many clients. This patch makes
the initial retransmission interval to be configurable using the
environment variable "bootpretransmitperiodinit".
Also, on a retransmission it is not expected for the transaction ID to
change (only the 'secs' field should be updated). Let's save the
transaction ID and use the same transaction ID for each BOOTP/DHCPv4
exchange.
Signed-off-by: Sean Edmond
---
net/bootp.c | 63 +++--
1 file changed, 47 insertions(+), 16 deletions(-)
diff --git a/net/bootp.c b/net/bootp.c
index 013d54c7ed..7248536cc4 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -42,6 +42,17 @@
*/
#define TIMEOUT_MS((3 + (CONFIG_NET_RETRY_COUNT * 5)) * 1000)
+/*
+ * According to rfc951 : 7.2. Client Retransmission Strategy
+ * "After the 'average' backoff reaches about 60 seconds, it should be
+ * increased no further, but still randomized."
+ *
+ * U-Boot has saturated this backoff at 2 seconds for a long time.
+ * To modify, set the environment variable "bootpretransmitperiodmax"
+ */
+#define RETRANSMIT_PERIOD_MAX_MS 2000
+#define RETRANSMIT_PERIOD_INIT_MS 250
+
#define PORT_BOOTPS 67 /* BOOTP server UDP port */
#define PORT_BOOTPC 68 /* BOOTP client UDP port */
@@ -56,6 +67,7 @@
u32 bootp_ids[CFG_BOOTP_ID_CACHE_SIZE];
unsigned int bootp_num_ids;
int bootp_try;
+u32bootp_id;
ulong bootp_start;
ulong bootp_timeout;
char net_nis_domain[32] = {0,}; /* Our NIS domain */
@@ -63,6 +75,7 @@ char net_hostname[32] = {0,}; /* Our hostname */
char net_root_path[CONFIG_BOOTP_MAX_ROOT_PATH_LEN] = {0,}; /* Our bootpath */
static ulong time_taken_max;
+static u32 retransmit_period_max_ms;
#if defined(CONFIG_CMD_DHCP)
static dhcp_state_t dhcp_state = INIT;
@@ -417,8 +430,8 @@ static void bootp_timeout_handler(void)
}
} else {
bootp_timeout *= 2;
- if (bootp_timeout > 2000)
- bootp_timeout = 2000;
+ if (bootp_timeout > retransmit_period_max_ms)
+ bootp_timeout = retransmit_period_max_ms;
net_set_timeout_handler(bootp_timeout, bootp_timeout_handler);
bootp_request();
}
@@ -714,10 +727,18 @@ static int bootp_extended(u8 *e)
void bootp_reset(void)
{
+ char *ep; /* Environment pointer */
+
bootp_num_ids = 0;
bootp_try = 0;
bootp_start = get_timer(0);
- bootp_timeout = 250;
+
+ ep = env_get("bootpretransmitperiodinit");
+ if (ep)
+ bootp_timeout = dectoul(ep, NULL);
+ else
+ bootp_timeout = RETRANSMIT_PERIOD_INIT_MS;
+
bootp_timeout = env_get_ulong("bootpretransmitperiodinit", 0,
RETRANSMIT_PERIOD_INIT_MS);
does the same...
Thanks for the suggestion, will fix in v2.
}
void bootp_request(void)
@@ -729,7 +750,6 @@ void bootp_request(void)
#ifdef CONFIG_BOOTP_RANDOM_DELAY
ulong rand_ms;
#endif
- u32 bootp_id;
struct in_addr zero_ip;
struct in_addr bcast_ip;
char *ep; /* Environment pointer */
@@ -745,6 +765,12 @@ void bootp_request(void)
else
time_taken_max = TIMEOUT_MS;
+ ep = env_get("bootpretransmitperiodmax");
+ if (ep)
+ retransmit_period_max_ms = dectoul(ep, NULL);
+ else
+ retransmit_period_max_ms = RETRANSMIT_PERIOD_MAX_MS;
+
see above
Lothar Waßmann
Re: [PATCH 0/3] BOOTP/DHCPv4 enhancements
On 2023-09-26 9:45 a.m., Peter Robinson wrote: Hi Sean, In our datacenter application, a single DHCP server is servicing 36000+ clients. Improvements are required to the DHCPv4 retransmission behavior to align with RFC and ensure less pressure is exerted on the server: - retransmission backoff interval maximum is configurable (environment variable bootpretransmitperiodmax) - initial retransmission backoff interval is configurable (environment variable bootpretransmitperiodinit) - transaction ID is kept the same for each BOOTP/DHCPv4 request (not recreated on each retry) Might be also worth looking at the series adding LWIP support [1] and see what impact that may have on this too. Peter [1]https://lists.denx.de/pipermail/u-boot/2023-September/531716.html I'm aware of the LWIP addition but haven't made any attempt to characterize the retransmission behavior. My understanding is that LWIP doesn't support DHCPv6 or TFTP with IPv6 yet. Our preference would be to stick with the u-boot networking stack for now until LWIP adds full support for IPv6. Given the importance of the retransmission behavior is for our use-case (36000+ clients pers server), we would have to qualify/validate LWIP before transitioning. For our application we'll use: - bootpretransmitperiodmax=16000 - bootpretransmitperiodinit=2000 A new configuration BOOTP_RANDOM_XID has been added to enable a randomized BOOTP/DHCPv4 transaction ID. Add functionality for DHCPv4 sending/parsing option 209 (PXE config file). Enabled with Kconfig BOOTP_PXE_DHCP_OPTION. Note, this patch was submitted previously but this latest version has been enhanced to avoid a possible double free(). Sean Edmond (3): net: Get pxe config file from dhcp option 209 net: bootp: BOOTP/DHCPv4 retransmission improvements net: bootp: add config option BOOTP_RANDOM_XID cmd/Kconfig | 11 +++ cmd/pxe.c | 10 +++ net/bootp.c | 85 +++-- 3 files changed, 91 insertions(+), 15 deletions(-) -- 2.40.0
Re: [PATCHv10 14/15] net/lwip: replace original net commands with lwip
On 2023-09-26 2:41 a.m., Maxim Uvarov wrote:
Replace original commands: ping, tftp, dhcp and wget.
Signed-off-by: Maxim Uvarov
---
boot/bootmeth_efi.c | 18 +++---
boot/bootmeth_pxe.c | 21 ++-
cmd/net.c | 86 +
cmd/pxe.c | 19 +-
include/net.h | 8 +++--
include/net/ulwip.h | 64 +
6 files changed, 113 insertions(+), 103 deletions(-)
create mode 100644 include/net/ulwip.h
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index ae936c8daa..52399d627c 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -20,6 +20,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -319,9 +321,7 @@ static int distro_efi_try_bootflow_files(struct udevice *dev,
static int distro_efi_read_bootflow_net(struct bootflow *bflow)
{
- char file_addr[17], fname[256];
- char *tftp_argv[] = {"tftp", file_addr, fname, NULL};
- struct cmd_tbl cmdtp = {}; /* dummy */
+ char fname[256];
const char *addr_str, *fdt_addr_str;
int ret, arch, size;
ulong addr, fdt_addr;
@@ -368,7 +368,6 @@ static int distro_efi_read_bootflow_net(struct bootflow
*bflow)
if (!fdt_addr_str)
return log_msg_ret("fdt", -EINVAL);
fdt_addr = hextoul(fdt_addr_str, NULL);
- sprintf(file_addr, "%lx", fdt_addr);
/* We only allow the first prefix with PXE */
ret = distro_efi_get_fdt_name(fname, sizeof(fname), 0);
@@ -379,7 +378,16 @@ static int distro_efi_read_bootflow_net(struct bootflow
*bflow)
if (!bflow->fdt_fname)
return log_msg_ret("fil", -ENOMEM);
- if (!do_tftpb(, 0, 3, tftp_argv)) {
+ ret = ulwip_init();
+ if (ret)
+ return log_msg_ret("ulwip_init", ret);
+
+ ret = ulwip_tftp(fdt_addr, fname);
+ if (ret)
+ return log_msg_ret("ulwip_tftp", ret);
+
+ ret = ulwip_loop();
+ if (!ret) {
bflow->fdt_size = env_get_hex("filesize", 0);
bflow->fdt_addr = fdt_addr;
} else {
diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c
index 8d489a11aa..fc6aabaa18 100644
--- a/boot/bootmeth_pxe.c
+++ b/boot/bootmeth_pxe.c
@@ -21,6 +21,8 @@
#include
#include
#include
+#include
+#include
#include
static int extlinux_pxe_getfile(struct pxe_context *ctx, const char *file_path,
@@ -116,18 +118,21 @@ static int extlinux_pxe_read_file(struct udevice *dev,
struct bootflow *bflow,
const char *file_path, ulong addr,
ulong *sizep)
{
- char *tftp_argv[] = {"tftp", NULL, NULL, NULL};
- struct pxe_context *ctx = dev_get_priv(dev);
- char file_addr[17];
ulong size;
int ret;
- sprintf(file_addr, "%lx", addr);
- tftp_argv[1] = file_addr;
- tftp_argv[2] = (void *)file_path;
+ ret = ulwip_init();
+ if (ret)
+ return log_msg_ret("ulwip_init", ret);
+
+ ret = ulwip_tftp(addr, file_path);
+ if (ret)
+ return log_msg_ret("ulwip_tftp", ret);
+
+ ret = ulwip_loop();
+ if (ret)
+ return log_msg_ret("ulwip_loop", ret);
- if (do_tftpb(ctx->cmdtp, 0, 3, tftp_argv))
- return -ENOENT;
ret = pxe_get_file_size();
if (ret)
return log_msg_ret("tftp", ret);
diff --git a/cmd/net.c b/cmd/net.c
index d407d8320a..dc5a114309 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
static int netboot_common(enum proto_t, struct cmd_tbl *, int, char * const []);
@@ -40,19 +41,9 @@ U_BOOT_CMD(
#endif
#ifdef CONFIG_CMD_TFTPBOOT
-int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
-{
- int ret;
-
- bootstage_mark_name(BOOTSTAGE_KERNELREAD_START, "tftp_start");
- ret = netboot_common(TFTPGET, cmdtp, argc, argv);
- bootstage_mark_name(BOOTSTAGE_KERNELREAD_STOP, "tftp_done");
- return ret;
-}
-
#if IS_ENABLED(CONFIG_IPV6)
U_BOOT_CMD(
- tftpboot, 4, 1, do_tftpb,
+ tftpboot, 4, 1, do_lwip_tftp,
It looks like LWIP doesn't support TFTP with IPv6 addressing. Perhaps
we need to fall back onto the existing TFTP implementation until LWIP
supports it?
Note, that currently, IPv6 TFTP is enabled using the "-ipv6" argument.
The intention is that netboot_common() sees the argument and sets the
"use_ip6" variable. It looks like the new implementation in
do_lwip_tftp() doesn't re-use the argument parsing in netboot_common()
and that it doesn't handle the addition of the "-ipv6" flag.
I support the addition of LWIP, but I'm concerned about how abrupt
changes like this one will be for existing users. The underlying stack
will change, with no easy way for the user to revert to
Re: [PATCH 2/5] drivers: security: Add TPM2 implementation of security devices
Hi Simon,
On 2023-08-17 6:41 a.m., Simon Glass wrote:
Hi Sean,
On Fri, 11 Aug 2023 at 18:28, wrote:
From: Stephen Carlson
This implementation of the security uclass driver allows existing TPM2
devices declared in the device tree to be referenced for storing the OS
anti-rollback counter, using the TPM2 non-volatile storage API.
Signed-off-by: Stephen Carlson
---
MAINTAINERS | 1 +
drivers/security/Makefile | 1 +
drivers/security/security-tpm.c | 173
include/tpm-v2.h| 1 +
4 files changed, 176 insertions(+)
create mode 100644 drivers/security/security-tpm.c
This is a bit wonky w.r.t driver model. The TPM itself should
implement this API, perhaps ina separate file shared with all v2 TPMs.
You should not be opening the device, etc. in here.
I hope that makes sense...you effectively need to turn the TPM into a
multi-function device within driver model. Of course TPMs are
multi-function devices anyway, but here you are making their functions
available more explicitly with a nicer API.
Then you can call the TPM API to do what you want, but at least you
know that the TPM has been probed before you start.
Regards,
Simon
Let's step back for a moment to understand our intention with this feature.
We want secure storage for the anti-rollback counter. The intention is
that this storage is provided by whatever "secure driver" (let's start
calling it the "rollback driver"). On our platform, we're using a
different "rollback driver" which accesses a non-TPM based secure
storage (which we can't upstream because it's proprietary). For the
purpose of making this feature publicly available, we've added the
TPM-backed "rollback driver" (this patch). I'll make this intention
more clear in the documentation, which should lead to less confusion?
At the end of the day, all we require is dm_security_arbvn_get() and
dm_security_arbvn_set(), to get/set from the secure storage (we'll
rename these to something less ugly, because yes arbvn is gross). We
don't want to lock this feature to the TPM, because it doesn't enable
us/others to choose a different secure storage mechanism.
W.r.t opening/initializing the TPM. Someone has to open it? In our
case, it's being opened earlier with our measured boot, so "-EBUSY" in
returned on tpm_open() (we return and everyone is happy). My
understanding is that this is the currently available way for multiple
drivers to access the TPM. Ilias has recommended we use
tpm_auto_start(), which is an enhancement I'm planning to make. This
should clean thing up? If this doesn't meet your expectations, can you
describe in more detail how we should turn the TPM into a
"multi-function device"?
diff --git a/MAINTAINERS b/MAINTAINERS
index 73b6943e03..257660a847 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1444,6 +1444,7 @@ S:Maintained
F: drivers/security/Kconfig
F: drivers/security/Makefile
F: drivers/security/sandbox_security.c
+F: drivers/security/security-tpm.c
F: drivers/security/security-uclass.c
SEMIHOSTING
diff --git a/drivers/security/Makefile b/drivers/security/Makefile
index ed10c3f234..e81966bb4a 100644
--- a/drivers/security/Makefile
+++ b/drivers/security/Makefile
@@ -4,3 +4,4 @@
obj-$(CONFIG_DM_SECURITY) += security-uclass.o
obj-$(CONFIG_SECURITY_SANDBOX) += sandbox_security.o
+obj-$(CONFIG_SECURITY_TPM) += security-tpm.o
diff --git a/drivers/security/security-tpm.c b/drivers/security/security-tpm.c
new file mode 100644
index 00..9070dd49ac
--- /dev/null
+++ b/drivers/security/security-tpm.c
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2021 Microsoft, Inc
+ * Written by Stephen Carlson
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+struct security_state {
+ u32 index_arbvn;
+ struct udevice *tpm_dev;
+};
+
+static int tpm_security_init(struct udevice *tpm_dev)
+{
+ int res;
+
+ /* Initialize TPM but allow reuse of existing session */
+ res = tpm_open(tpm_dev);
+ if (res == -EBUSY) {
+ log(UCLASS_SECURITY, LOGL_DEBUG,
+ "Existing TPM session found, reusing\n");
+ } else {
+ if (res) {
+ log(UCLASS_SECURITY, LOGL_ERR,
+ "TPM initialization failed (ret=%d)\n", res);
+ return res;
+ }
+
+ res = tpm2_startup(tpm_dev, TPM2_SU_CLEAR);
+ if (res) {
+ log(UCLASS_SECURITY, LOGL_ERR,
+ "TPM startup failed (ret=%d)\n", res);
+ return res;
+ }
+ }
+
+ return 0;
+}
+
+static int tpm_security_arbvn_get(struct udevice *dev, u64 *arbvn)
+{
+ struct security_state *priv = dev_get_priv(dev);
+ int ret;
+
+ if (!arbvn)
+ return -EINVAL;
+
+
Re: [PATCH 1/3] fdt: common API to populate kaslr seed
On 2023-08-15 10:46 a.m., Sean Edmond wrote: On 2023-08-15 7:44 a.m., Simon Glass wrote: Hi Sean, On Mon, 14 Aug 2023 at 13:12, Sean Edmond wrote: On 2023-08-12 6:09 a.m., Simon Glass wrote: Hi Sean, On Fri, 11 Aug 2023 at 11:14, Sean Edmond wrote: On 2023-08-09 6:49 p.m., Simon Glass wrote: Hi Sean, On Wed, 9 Aug 2023 at 16:35, Sean Edmond wrote: On 2023-08-08 7:03 p.m., Simon Glass wrote: Hi, On Fri, 4 Aug 2023 at 17:34, wrote: From: Dhananjay Phadke fdt_fixup_kaslr_seed() will update given FDT with random seed value. Source for random seed can be TPM or RNG driver in u-boot or sec firmware (ARM). Signed-off-by: Dhananjay Phadke --- arch/arm/cpu/armv8/sec_firmware.c | 32 +++ common/fdt_support.c | 31 ++ include/fdt_support.h | 3 +++ 3 files changed, 41 insertions(+), 25 deletions(-) We need to find a way to use the ofnode API here. diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c index c0e8726346..84ba49924e 100644 --- a/arch/arm/cpu/armv8/sec_firmware.c +++ b/arch/arm/cpu/armv8/sec_firmware.c @@ -411,46 +411,28 @@ int sec_firmware_init(const void *sec_firmware_img, /* * fdt_fix_kaslr - Add kalsr-seed node in Device tree * @fdt: Device tree - * @eret: 0 in case of error, 1 for success + * @eret: 0 for success */ int fdt_fixup_kaslr(void *fdt) You could pass an oftree to this function, e.g. obtained with: oftree_from_fdt(fdt) The common API I added is fdt_fixup_kaslr_seed(), which was added to "common/fdt_support.c". There are 3 callers: sec_firmware_init()->fdt_fixup_kaslr_seed() do_kaslr_seed()->fdt_fixup_kaslr_seed() image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed() I think the ask is to create a common API that uses the ofnode API. So, instead of fdt_fixup_kaslr_seed() I can create ofnode_fixup_kaslr_seed()? Where should it live? If you like you could add common/ofnode_support.c ? But it is OK to have it in the same file, I think. Are you also wanting the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as input too? So far as you can go, yes. Also you may want to pass an ofnode (the root node) so that things can deal with adding their stuff to any node. Regards, Simon I re-worked the API to use the ofnode API and tested it on our board. I was required to explicitly enable CONFIG_OFNODE_MULTI_TREE in order for it to work. I have concerns this will create a breaking change for users of the kaslr fdt touch up. In our case, if CONFIG_OFNODE_MULTI_TREE isn't set, the control FDT gets touched up, not the kernel FDT as required. Everything runs to completion, but "/proc/device-tree/chosen/kaslr-seed" isn't present after boot. Am I missing something? Perhaps there's a way to modify the default value for CONFIG_OFNODE_MULTI_TREE to ensure this works out-of-the-box? Yes, perhaps we should enable this when fixups are used? Is there a way to tell? I don't think there's a way to tell unfortunately. Fixups are always called if OF_LIBFDT is enabled, and if an FDT is found during bootm. I'm having trouble understanding the intention of the current default for OFNODE_MULTI_TREE: default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE Could we simplify to this? default y if !OF_LIVE I don't think it will build if inlining is used, but I can't remember... I wasn't able to break this by turning off DM_DEV_READ_INLINE, and enabling OFNODE_MULTI_TREE/SPL_DM_INLINE_OFNODE/TPL_DM_INLINE_OFNODE. Perhaps things have changes since this was created. The EVENT thing is because there is an of-fixup event, which was the original thing using ofnode fixups. I wonder what sort of size increment this will create with !OF_LIVE ? With default (OFNODE_MULTI_TREE is not set): textdatabssdechex filename 9380135536846752 1040133fdf05 u-boot/u-boot/u-boot With !OF_LIVE (OFNODE_MULTI_TREE is set with OFNODE_MULTI_TREE_MAX=4): seanedmond@ovlvm106:~/code/QEMU/qemu_uboot$ sizeu-boot/u-boot/u-boot textdatabssdechex filename 9390165536846752 1041136fe2f0 u-boot/u-boot/u-boot Sorry about the formatting... let's try that again. With default (OFNODE_MULTI_TREE is not set): text: 938013 data: 55368 bss:46752 dec: 1040133 hex: fdf05 With !OF_LIVE (OFNODE_MULTI_TREE is set with OFNODE_MULTI_TREE_MAX=4): text: 939016 data: 55368 bss: 46752 dec: 1041136 hex: fe2f0 Also, we should make it return an error when attempting to use a tree without that option enabled. I would expect oftree_ensure() to provide that? I'll add a check. OK thanks. Regards, Simon
Re: [PATCH 1/3] fdt: common API to populate kaslr seed
On 2023-08-15 7:44 a.m., Simon Glass wrote: Hi Sean, On Mon, 14 Aug 2023 at 13:12, Sean Edmond wrote: On 2023-08-12 6:09 a.m., Simon Glass wrote: Hi Sean, On Fri, 11 Aug 2023 at 11:14, Sean Edmond wrote: On 2023-08-09 6:49 p.m., Simon Glass wrote: Hi Sean, On Wed, 9 Aug 2023 at 16:35, Sean Edmond wrote: On 2023-08-08 7:03 p.m., Simon Glass wrote: Hi, On Fri, 4 Aug 2023 at 17:34, wrote: From: Dhananjay Phadke fdt_fixup_kaslr_seed() will update given FDT with random seed value. Source for random seed can be TPM or RNG driver in u-boot or sec firmware (ARM). Signed-off-by: Dhananjay Phadke --- arch/arm/cpu/armv8/sec_firmware.c | 32 +++ common/fdt_support.c | 31 ++ include/fdt_support.h | 3 +++ 3 files changed, 41 insertions(+), 25 deletions(-) We need to find a way to use the ofnode API here. diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c index c0e8726346..84ba49924e 100644 --- a/arch/arm/cpu/armv8/sec_firmware.c +++ b/arch/arm/cpu/armv8/sec_firmware.c @@ -411,46 +411,28 @@ int sec_firmware_init(const void *sec_firmware_img, /* * fdt_fix_kaslr - Add kalsr-seed node in Device tree * @fdt: Device tree - * @eret: 0 in case of error, 1 for success + * @eret: 0 for success */ int fdt_fixup_kaslr(void *fdt) You could pass an oftree to this function, e.g. obtained with: oftree_from_fdt(fdt) The common API I added is fdt_fixup_kaslr_seed(), which was added to "common/fdt_support.c". There are 3 callers: sec_firmware_init()->fdt_fixup_kaslr_seed() do_kaslr_seed()->fdt_fixup_kaslr_seed() image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed() I think the ask is to create a common API that uses the ofnode API. So, instead of fdt_fixup_kaslr_seed() I can create ofnode_fixup_kaslr_seed()? Where should it live? If you like you could add common/ofnode_support.c ? But it is OK to have it in the same file, I think. Are you also wanting the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as input too? So far as you can go, yes. Also you may want to pass an ofnode (the root node) so that things can deal with adding their stuff to any node. Regards, Simon I re-worked the API to use the ofnode API and tested it on our board. I was required to explicitly enable CONFIG_OFNODE_MULTI_TREE in order for it to work. I have concerns this will create a breaking change for users of the kaslr fdt touch up. In our case, if CONFIG_OFNODE_MULTI_TREE isn't set, the control FDT gets touched up, not the kernel FDT as required. Everything runs to completion, but "/proc/device-tree/chosen/kaslr-seed" isn't present after boot. Am I missing something? Perhaps there's a way to modify the default value for CONFIG_OFNODE_MULTI_TREE to ensure this works out-of-the-box? Yes, perhaps we should enable this when fixups are used? Is there a way to tell? I don't think there's a way to tell unfortunately. Fixups are always called if OF_LIBFDT is enabled, and if an FDT is found during bootm. I'm having trouble understanding the intention of the current default for OFNODE_MULTI_TREE: default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE Could we simplify to this? default y if !OF_LIVE I don't think it will build if inlining is used, but I can't remember... I wasn't able to break this by turning off DM_DEV_READ_INLINE, and enabling OFNODE_MULTI_TREE/SPL_DM_INLINE_OFNODE/TPL_DM_INLINE_OFNODE. Perhaps things have changes since this was created. The EVENT thing is because there is an of-fixup event, which was the original thing using ofnode fixups. I wonder what sort of size increment this will create with !OF_LIVE ? With default (OFNODE_MULTI_TREE is not set): textdatabssdechex filename 9380135536846752 1040133fdf05 u-boot/u-boot/u-boot With !OF_LIVE (OFNODE_MULTI_TREE is set with OFNODE_MULTI_TREE_MAX=4): seanedmond@ovlvm106:~/code/QEMU/qemu_uboot$ sizeu-boot/u-boot/u-boot textdatabssdechex filename 9390165536846752 1041136fe2f0 u-boot/u-boot/u-boot Also, we should make it return an error when attempting to use a tree without that option enabled. I would expect oftree_ensure() to provide that? I'll add a check. OK thanks. Regards, Simon
Re: [PATCH 2/5] drivers: security: Add TPM2 implementation of security devices
On 2023-08-14 1:39 a.m., Ilias Apalodimas wrote:
Hi Sean
On Sat, 12 Aug 2023 at 03:28, wrote:
From: Stephen Carlson
This implementation of the security uclass driver allows existing TPM2
devices declared in the device tree to be referenced for storing the OS
anti-rollback counter, using the TPM2 non-volatile storage API.
Signed-off-by: Stephen Carlson
---
MAINTAINERS | 1 +
drivers/security/Makefile | 1 +
drivers/security/security-tpm.c | 173
include/tpm-v2.h| 1 +
4 files changed, 176 insertions(+)
create mode 100644 drivers/security/security-tpm.c
diff --git a/MAINTAINERS b/MAINTAINERS
[...]
+
+struct security_state {
+ u32 index_arbvn;
+ struct udevice *tpm_dev;
+};
+
+static int tpm_security_init(struct udevice *tpm_dev)
+{
+ int res;
+
+ /* Initialize TPM but allow reuse of existing session */
+ res = tpm_open(tpm_dev);
+ if (res == -EBUSY) {
+ log(UCLASS_SECURITY, LOGL_DEBUG,
+ "Existing TPM session found, reusing\n");
+ } else {
+ if (res) {
+ log(UCLASS_SECURITY, LOGL_ERR,
+ "TPM initialization failed (ret=%d)\n", res);
+ return res;
+ }
+
+ res = tpm2_startup(tpm_dev, TPM2_SU_CLEAR);
+ if (res) {
+ log(UCLASS_SECURITY, LOGL_ERR,
+ "TPM startup failed (ret=%d)\n", res);
+ return res;
+ }
+ }
+
+ return 0;
+}
There's nothing security related in that wrapper. It looks like a
typical tpm startup sequence. Any reason you can't use
tpm_auto_start()?
Good suggestion, I'll make this change.
+
+static int tpm_security_arbvn_get(struct udevice *dev, u64 *arbvn)
+{
+ struct security_state *priv = dev_get_priv(dev);
+ int ret;
+
+ if (!arbvn)
+ return -EINVAL;
+
+ ret = tpm2_nv_read_value(priv->tpm_dev, priv->index_arbvn, arbvn,
+sizeof(u64));
+ if (ret == TPM2_RC_NV_UNINITIALIZED) {
+ /* Expected if no OS image has been loaded before */
+ log(UCLASS_SECURITY, LOGL_INFO,
+ "No previous OS image, defaulting ARBVN to 0\n");
+ *arbvn = 0ULL;
Why aren't we returning an error here? Looks like the code following
this is trying to reason with the validity of arbnv
On the first boot (before ARBVN has been set in NV memory), it's
expected that the NV index hasn't been initialized/written yet. In this
case, TPM2_RC_NV_UNINITIALIZED is expected. A value of 0 is returned to
ensure that the anti-rollback check always passes (which it should since
there's nothing to check on the first boot).
+ } else if (ret) {
+ log(UCLASS_SECURITY, LOGL_ERR,
+ "Unable to read ARBVN from TPM (ret=%d)\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int tpm_security_arbvn_set(struct udevice *dev, u64 arbvn)
+{
+ struct security_state *priv = dev_get_priv(dev);
+ struct udevice *tpm_dev = priv->tpm_dev;
+ u64 old_arbvn;
+ int ret;
+
+ ret = tpm_security_arbvn_get(dev, _arbvn);
+ if (ret)
+ return ret;
+
+ if (arbvn < old_arbvn)
+ return -EPERM;
+
What happens if they are equal ?
If they are equal, then we are booting the same OS that was previously
booted (we are not moving the OS version forward or back).
Note the actual "anti-rollback" check is in fit_image_verify_arbvn().
If it make things more clear, we could remove the value checks here
completely and just write the value.
+ if (arbvn > old_arbvn) {
You just check for this and exited
+ ret = tpm2_nv_write_value(tpm_dev, priv->index_arbvn, ,
+ sizeof(u64));
+ if (ret) {
+ log(UCLASS_SECURITY, LOGL_ERR,
+ "Unable to write ARBVN to TPM (ret=%d)\n", ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static const struct dm_security_ops tpm_security_ops = {
+ .arbvn_get = tpm_security_arbvn_get,
+ .arbvn_set = tpm_security_arbvn_set,
+};
+
+static int tpm_security_probe(struct udevice *dev)
+{
+ struct security_state *priv = dev_get_priv(dev);
+ struct udevice *tpm_dev = priv->tpm_dev;
+ u32 index = priv->index_arbvn;
+ int ret;
+
+ if (!tpm_dev) {
+ log(UCLASS_SECURITY, LOGL_ERR,
+ "TPM device not defined in DTS\n");
+ return -EINVAL;
+ }
+
+ ret = tpm_security_init(tpm_dev);
+ if (ret)
+ return ret;
+
+ ret = tpm2_nv_define_space(tpm_dev, index, sizeof(u64), TPMA_NV_PPREAD |
+
Re: [PATCH 1/3] fdt: common API to populate kaslr seed
On 2023-08-12 6:09 a.m., Simon Glass wrote: Hi Sean, On Fri, 11 Aug 2023 at 11:14, Sean Edmond wrote: On 2023-08-09 6:49 p.m., Simon Glass wrote: Hi Sean, On Wed, 9 Aug 2023 at 16:35, Sean Edmond wrote: On 2023-08-08 7:03 p.m., Simon Glass wrote: Hi, On Fri, 4 Aug 2023 at 17:34, wrote: From: Dhananjay Phadke fdt_fixup_kaslr_seed() will update given FDT with random seed value. Source for random seed can be TPM or RNG driver in u-boot or sec firmware (ARM). Signed-off-by: Dhananjay Phadke --- arch/arm/cpu/armv8/sec_firmware.c | 32 +++ common/fdt_support.c | 31 ++ include/fdt_support.h | 3 +++ 3 files changed, 41 insertions(+), 25 deletions(-) We need to find a way to use the ofnode API here. diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c index c0e8726346..84ba49924e 100644 --- a/arch/arm/cpu/armv8/sec_firmware.c +++ b/arch/arm/cpu/armv8/sec_firmware.c @@ -411,46 +411,28 @@ int sec_firmware_init(const void *sec_firmware_img, /* * fdt_fix_kaslr - Add kalsr-seed node in Device tree * @fdt: Device tree - * @eret: 0 in case of error, 1 for success + * @eret: 0 for success */ int fdt_fixup_kaslr(void *fdt) You could pass an oftree to this function, e.g. obtained with: oftree_from_fdt(fdt) The common API I added is fdt_fixup_kaslr_seed(), which was added to "common/fdt_support.c". There are 3 callers: sec_firmware_init()->fdt_fixup_kaslr_seed() do_kaslr_seed()->fdt_fixup_kaslr_seed() image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed() I think the ask is to create a common API that uses the ofnode API. So, instead of fdt_fixup_kaslr_seed() I can create ofnode_fixup_kaslr_seed()? Where should it live? If you like you could add common/ofnode_support.c ? But it is OK to have it in the same file, I think. Are you also wanting the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as input too? So far as you can go, yes. Also you may want to pass an ofnode (the root node) so that things can deal with adding their stuff to any node. Regards, Simon I re-worked the API to use the ofnode API and tested it on our board. I was required to explicitly enable CONFIG_OFNODE_MULTI_TREE in order for it to work. I have concerns this will create a breaking change for users of the kaslr fdt touch up. In our case, if CONFIG_OFNODE_MULTI_TREE isn't set, the control FDT gets touched up, not the kernel FDT as required. Everything runs to completion, but "/proc/device-tree/chosen/kaslr-seed" isn't present after boot. Am I missing something? Perhaps there's a way to modify the default value for CONFIG_OFNODE_MULTI_TREE to ensure this works out-of-the-box? Yes, perhaps we should enable this when fixups are used? Is there a way to tell? I don't think there's a way to tell unfortunately. Fixups are always called if OF_LIBFDT is enabled, and if an FDT is found during bootm. I'm having trouble understanding the intention of the current default for OFNODE_MULTI_TREE: default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE Could we simplify to this? default y if !OF_LIVE Also, we should make it return an error when attempting to use a tree without that option enabled. I would expect oftree_ensure() to provide that? I'll add a check. Regards, Simon
Re: [PATCH 1/3] fdt: common API to populate kaslr seed
On 2023-08-09 6:49 p.m., Simon Glass wrote: Hi Sean, On Wed, 9 Aug 2023 at 16:35, Sean Edmond wrote: On 2023-08-08 7:03 p.m., Simon Glass wrote: Hi, On Fri, 4 Aug 2023 at 17:34, wrote: From: Dhananjay Phadke fdt_fixup_kaslr_seed() will update given FDT with random seed value. Source for random seed can be TPM or RNG driver in u-boot or sec firmware (ARM). Signed-off-by: Dhananjay Phadke --- arch/arm/cpu/armv8/sec_firmware.c | 32 +++ common/fdt_support.c | 31 ++ include/fdt_support.h | 3 +++ 3 files changed, 41 insertions(+), 25 deletions(-) We need to find a way to use the ofnode API here. diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c index c0e8726346..84ba49924e 100644 --- a/arch/arm/cpu/armv8/sec_firmware.c +++ b/arch/arm/cpu/armv8/sec_firmware.c @@ -411,46 +411,28 @@ int sec_firmware_init(const void *sec_firmware_img, /* * fdt_fix_kaslr - Add kalsr-seed node in Device tree * @fdt: Device tree - * @eret: 0 in case of error, 1 for success + * @eret: 0 for success */ int fdt_fixup_kaslr(void *fdt) You could pass an oftree to this function, e.g. obtained with: oftree_from_fdt(fdt) The common API I added is fdt_fixup_kaslr_seed(), which was added to "common/fdt_support.c". There are 3 callers: sec_firmware_init()->fdt_fixup_kaslr_seed() do_kaslr_seed()->fdt_fixup_kaslr_seed() image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed() I think the ask is to create a common API that uses the ofnode API. So, instead of fdt_fixup_kaslr_seed() I can create ofnode_fixup_kaslr_seed()? Where should it live? If you like you could add common/ofnode_support.c ? But it is OK to have it in the same file, I think. Are you also wanting the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as input too? So far as you can go, yes. Also you may want to pass an ofnode (the root node) so that things can deal with adding their stuff to any node. Regards, Simon I re-worked the API to use the ofnode API and tested it on our board. I was required to explicitly enable CONFIG_OFNODE_MULTI_TREE in order for it to work. I have concerns this will create a breaking change for users of the kaslr fdt touch up. In our case, if CONFIG_OFNODE_MULTI_TREE isn't set, the control FDT gets touched up, not the kernel FDT as required. Everything runs to completion, but "/proc/device-tree/chosen/kaslr-seed" isn't present after boot. Am I missing something? Perhaps there's a way to modify the default value for CONFIG_OFNODE_MULTI_TREE to ensure this works out-of-the-box?
Re: [PATCH 1/3] fdt: common API to populate kaslr seed
On 2023-08-08 7:03 p.m., Simon Glass wrote:
Hi,
On Fri, 4 Aug 2023 at 17:34, wrote:
From: Dhananjay Phadke
fdt_fixup_kaslr_seed() will update given FDT with random seed value.
Source for random seed can be TPM or RNG driver in u-boot or sec
firmware (ARM).
Signed-off-by: Dhananjay Phadke
---
arch/arm/cpu/armv8/sec_firmware.c | 32 +++
common/fdt_support.c | 31 ++
include/fdt_support.h | 3 +++
3 files changed, 41 insertions(+), 25 deletions(-)
We need to find a way to use the ofnode API here.
diff --git a/arch/arm/cpu/armv8/sec_firmware.c
b/arch/arm/cpu/armv8/sec_firmware.c
index c0e8726346..84ba49924e 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -411,46 +411,28 @@ int sec_firmware_init(const void *sec_firmware_img,
/*
* fdt_fix_kaslr - Add kalsr-seed node in Device tree
* @fdt: Device tree
- * @eret: 0 in case of error, 1 for success
+ * @eret: 0 for success
*/
int fdt_fixup_kaslr(void *fdt)
You could pass an oftree to this function, e.g. obtained with:
oftree_from_fdt(fdt)
The common API I added is fdt_fixup_kaslr_seed(), which was added to
"common/fdt_support.c".
There are 3 callers:
sec_firmware_init()->fdt_fixup_kaslr_seed()
do_kaslr_seed()->fdt_fixup_kaslr_seed()
image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed()
I think the ask is to create a common API that uses the ofnode API. So,
instead of fdt_fixup_kaslr_seed() I can create
ofnode_fixup_kaslr_seed()? Where should it live? Are you also wanting
the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as
input too?
{
- int nodeoffset;
- int err, ret = 0;
- u8 rand[8];
+ int ret = 0;
#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT)
+ u8 rand[8];
+
/* Check if random seed generation is supported */
if (sec_firmware_support_hwrng() == false) {
printf("WARNING: SEC firmware not running, no kaslr-seed\n");
- return 0;
+ return -EOPNOTSUPP;
}
err = sec_firmware_get_random(rand, 8);
if (err < 0) {
printf("WARNING: No random number to set kaslr-seed\n");
- return 0;
- }
-
- err = fdt_check_header(fdt);
- if (err < 0) {
- printf("fdt_chosen: %s\n", fdt_strerror(err));
- return 0;
+ return ret;
}
- /* find or create "/chosen" node. */
- nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
- if (nodeoffset < 0)
- return 0;
-
- err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", rand,
- sizeof(rand));
- if (err < 0) {
- printf("WARNING: can't set kaslr-seed %s.\n",
- fdt_strerror(err));
- return 0;
- }
- ret = 1;
+ ret = fdt_fixup_kaslr_seed(fdt, rand, sizeof(rand));
#endif
return ret;
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 5e49078f8c..35d4f26dbd 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -631,6 +631,37 @@ void fdt_fixup_ethernet(void *fdt)
}
}
+/*
+ * fdt_fix_kaslr_seed - Add kalsr-seed node in Device tree
+ * @fdt: Device tree
+ * @eret: 0 for success
+ */
+int fdt_fixup_kaslr_seed(void *fdt, const u8 *seed, int len)
+{
+ int nodeoffset;
+ int err;
+
+ err = fdt_check_header(fdt);
+ if (err < 0) {
+ printf("fdt_chosen: %s\n", fdt_strerror(err));
+ return err;
+ }
+
+ /* find or create "/chosen" node. */
+ nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
+ if (nodeoffset < 0)
+ return -ENOENT;
+
+ err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", seed, len);
+ if (err < 0) {
+ printf("WARNING: can't set kaslr-seed %s.\n",
+ fdt_strerror(err));
+ return err;
+ }
+
+ return 0;
+}
+
int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 2cd8366898..d74ef4e0a7 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -121,6 +121,9 @@ static inline int fdt_fixup_memory_banks(void *blob, u64
start[], u64 size[],
#endif
void fdt_fixup_ethernet(void *fdt);
+
+int fdt_fixup_kaslr_seed(void *fdt, const u8 *seed, int len);
Please get in the habit of adding full comments to exported functions.
+
int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
const void *val, int len, int create);
void
Re: [PATCH v9 4/6] bootm: Support boot measurement
On 2023-03-08 1:25 p.m., Eddie James wrote:
Add a configuration option to measure the boot through the bootm
function. Add the measurement state to the booti and bootz paths
as well.
Signed-off-by: Eddie James
Reviewed-by: Simon Glass
---
Changes since v8:
- Added a configuration option to select to ignore any existing
event log. This would only be selected for systems that know
that U-Boot is the first stage bootloader. This is necessary
because the reserved memory region may persist through resets
and so U-Boot attempts to append to the previous boot's log.
Changes since v6:
- Added comment for bootm_measure
- Fixed line length in bootm_measure
boot/Kconfig| 32 +
boot/bootm.c| 74 +
cmd/booti.c | 1 +
cmd/bootm.c | 2 ++
cmd/bootz.c | 1 +
include/bootm.h | 11
include/image.h | 1 +
7 files changed, 122 insertions(+)
diff --git a/boot/Kconfig b/boot/Kconfig
index 5f491625c8..8119519c9f 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -629,6 +629,38 @@ config LEGACY_IMAGE_FORMAT
loaded. If a board needs the legacy image format support in this
case, enable it here.
+config MEASURED_BOOT
+ bool "Measure boot images and configuration to TPM and event log"
+ depends on HASH && TPM_V2
+ help
+ This option enables measurement of the boot process. Measurement
+ involves creating cryptographic hashes of the binary images that
+ are booting and storing them in the TPM. In addition, a log of
+ these hashes is stored in memory for the OS to verify the booted
+ images and configuration. Enable this if the OS has configured
+ some memory area for the event log and you intend to use some
+ attestation tools on your system.
+
+if MEASURED_BOOT
+ config MEASURE_DEVICETREE
+ bool "Measure the devicetree image"
+ default y if MEASURED_BOOT
+ help
+ On some platforms, the devicetree is not static as it may contain
+ random MAC addresses or other such data that changes each boot.
+ Therefore, it should not be measured into the TPM. In that case,
+ disable the measurement here.
+
+ config MEASURE_IGNORE_LOG
+ bool "Ignore the existing event log"
+ default n
+ help
+ On platforms that use an event log memory region that persists
+ through system resets and are the first stage bootloader, then
+ this option should be enabled to ignore any existing data in the
+ event log memory region.
+endif # MEASURED_BOOT
+
config SUPPORT_RAW_INITRD
bool "Enable raw initrd images"
help
diff --git a/boot/bootm.c b/boot/bootm.c
index 2eec60ec7b..2685bdbd74 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#if defined(CONFIG_CMD_USB)
#include
#endif
@@ -659,6 +660,75 @@ int bootm_process_cmdline_env(int flags)
return 0;
}
+int bootm_measure(struct bootm_headers *images)
+{
+ int ret = 0;
+
+ /* Skip measurement if EFI is going to do it */
+ if (images->os.os == IH_OS_EFI &&
+ IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL) &&
+ IS_ENABLED(CONFIG_BOOTM_EFI))
+ return ret;
+
it looks like your measured boot implementation is hardcoding the
following PCR indexes:
PCR #8 - kernel image measurement
PCR #9 - initrd measurement
PCR #0 - kernel DTB measurement
PCR #1 - bootargs measurement
I wasn't able to find any specificaton on this measured boot "profile".
Are you able to provide a reference?
We've implemented our own version of measured boot, which maps
measurements to different PCR indexes. In many cases, the data we're
measuring is also different.
To make this feature more usable by others it would be nice to see a
more generic interface that would allow the user to specify the PCR
indexes, and the data to hash into these indexes. This would allow
everyone to create their own custom measured boot "profile". This
request is probably beyond the scope of your current efforts, but I
except this implementation to evolve significantly if/when it's accepted.
+ if (IS_ENABLED(CONFIG_MEASURED_BOOT)) {
+ struct tcg2_event_log elog;
+ struct udevice *dev;
+ void *initrd_buf;
+ void *image_buf;
+ const char *s;
+ u32 rd_len;
+ bool ign;
+
+ elog.log_size = 0;
+ ign = IS_ENABLED(CONFIG_MEASURE_IGNORE_LOG);
+ ret = tcg2_measurement_init(, , ign);
+ if (ret)
+ return ret;
+
+ image_buf = map_sysmem(images->os.image_start,
+ images->os.image_len);
+ ret = tcg2_measure_data(dev, , 8, images->os.image_len,
+
Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot
On 2023-05-08 1:20 p.m., Tom Rini wrote:
Here's the latest defect report:
-- Forwarded message -
From:
Date: Mon, May 8, 2023, 2:29 PM
Subject: New Defects reported by Coverity Scan for Das U-Boot
To:
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot
found with Coverity Scan.
5 new defect(s) introduced to Das U-Boot found with Coverity Scan.
1 defect(s), reported by Coverity Scan earlier, were marked fixed in the
recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 5 of 5 defect(s)
** CID 453851: Memory - corruptions (OVERLAPPING_COPY)
/cmd/net.c: 279 in netboot_update_env()
*** CID 453851: Memory - corruptions (OVERLAPPING_COPY)
/cmd/net.c: 279 in netboot_update_env()
273
274 if (IS_ENABLED(CONFIG_IPV6)) {
275 if (!ip6_is_unspecified_addr(_ip6) ||
276 net_prefix_length != 0) {
277 sprintf(tmp, "%pI6c", _ip6);
278 if (net_prefix_length != 0)
CID 453851: Memory - corruptions (OVERLAPPING_COPY)
In the call to function "sprintf", the arguments "tmp" and "tmp"
Just submitted a patch to fix 453851.
may point to the same object.
279 sprintf(tmp, "%s/%d", tmp,
net_prefix_length);
280
281 env_set("ip6addr", tmp);
282 }
283
284 if (!ip6_is_unspecified_addr(_server_ip6)) {
** CID 450971: Insecure data handling (TAINTED_SCALAR)
/net/ndisc.c: 391 in process_ra()
*** CID 450971: Insecure data handling (TAINTED_SCALAR)
/net/ndisc.c: 391 in process_ra()
385 /* Ignore the packet if router lifetime is 0. */
386 if (!icmp->icmp6_rt_lifetime)
387 return -EOPNOTSUPP;
388
389 /* Processing the options */
390 option = msg->opt;
CID 450971: Insecure data handling (TAINTED_SCALAR)
Using tainted variable "remaining_option_len" as a loop boundary.
391 while (remaining_option_len > 0) {
392 /* The 2nd byte of the option is its length. */
393 option_len = option[1];
394 /* All included options should have a positive
length. */
395 if (option_len == 0)
396 return -EINVAL;
** CID 450969: Security best practices violations (DC.WEAK_CRYPTO)
/net/ndisc.c: 209 in ip6_send_rs()
*** CID 450969: Security best practices violations (DC.WEAK_CRYPTO)
/net/ndisc.c: 209 in ip6_send_rs()
203icmp_len, PROT_ICMPV6, pcsum);
204 msg->icmph.icmp6_cksum = csum;
205 pkt += icmp_len;
206
207 /* Wait up to 1 second if it is the first try to get the RA
*/
208 if (retry_count == 0)
CID 450969: Security best practices violations (DC.WEAK_CRYPTO)
"rand" should not be used for security-related applications,
because linear congruential algorithms are too easy to break.
209 udelay(((unsigned int)rand() % 100) *
MAX_SOLICITATION_DELAY);
210
211 /* send it! */
212 net_send_packet(net_tx_packet, (pkt - net_tx_packet));
213
214 retry_count++;
** CID 436282:(DC.WEAK_CRYPTO)
/net/dhcpv6.c: 621 in dhcp6_state_machine()
/net/dhcpv6.c: 627 in dhcp6_state_machine()
/net/dhcpv6.c: 628 in dhcp6_state_machine()
/net/dhcpv6.c: 662 in dhcp6_state_machine()
/net/dhcpv6.c: 613 in dhcp6_state_machine()
*** CID 436282:(DC.WEAK_CRYPTO)
/net/dhcpv6.c: 621 in dhcp6_state_machine()
615 /* handle state machine entry conditions */
616 if (sm_params.curr_state != sm_params.next_state) {
617 sm_params.retry_cnt = 0;
618
619 if (sm_params.next_state == DHCP6_SOLICIT) {
620 /* delay a random ammount (special for
SOLICIT) */
CID 436282:(DC.WEAK_CRYPTO)
"rand" should not be used for security-related applications,
because linear congruential algorithms are too easy to break.
621 udelay((rand() % SOL_MAX_DELAY_MS) * 1000);
622 /* init timestamp variables after SOLICIT
delay */
623 sm_params.dhcp6_start_ms = get_timer(0);
624 sm_params.dhcp6_retry_start_ms =
sm_params.dhcp6_start_ms;
625 sm_params.dhcp6_retry_ms =
sm_params.dhcp6_start_ms;
Re: [PATCH v2 1/3] net: dhcp6: Add DHCPv6 (DHCP for IPv6)
On 2023-04-25 12:03 p.m., Ramon Fried wrote:
On Fri, Apr 7, 2023 at 9:55 PM Simon Glass wrote:
Hi,
On Fri, 7 Apr 2023 at 18:56, wrote:
From: Sean Edmond
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
---
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
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 charnet_hostname[32]; /* Our hostname
*/
#ifdef CONFIG_NET
extern charnet_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 charnet_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 00..9204909c1f
--- /dev/null
+++ b/net/dhcpv6.c
@@ -0,0 +1,735 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) Microsoft Corporation
+ * Author: Sean Edmond
+ *
+ */
+
+/* Simple DHCP6 network layer implementation. */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "dhcpv6.h"
+#include
+#include
+#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 360
+#define REQ_TIMEOUT_MS 1000
+#define REQ_MAX_RT_MS 3
+#define REQ_MAX_RC 10
+#define MAX_WAIT_TIME_MS 6
+
+/* 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_b
Re: [PATCH v2 2/3] net: dhcp6: pxe: Add DHCP/PXE commands for IPv6
On 2023-04-07 11:55 a.m., Simon Glass wrote:
Hi Sean,
On Fri, 7 Apr 2023 at 18:56, wrote:
From: Sean Edmond
Adds commands to support DHCP and PXE with IPv6.
New configs added:
- CMD_DHCP6
- DHCP6_PXE_CLIENTARCH
- DHCP6_PXE_DHCP_OPTION
- DHCP6_ENTERPRISE_ID
New commands added (when IPv6 is enabled):
- dhcp6
- pxe get -ipv6
- pxe boot -ipv6
Signed-off-by: Sean Edmond
---
boot/bootmeth_distro.c | 2 +-
boot/bootmeth_pxe.c| 4 +-
boot/pxe_utils.c | 3 +-
cmd/Kconfig| 26 +
cmd/net.c | 23 +++
cmd/pxe.c | 86 +-
cmd/sysboot.c | 2 +-
include/pxe_utils.h| 10 -
8 files changed, 140 insertions(+), 16 deletions(-)
With nits below:
Reviewed-by: Simon Glass
[..]
+if CMD_DHCP6
+
+config DHCP6_PXE_CLIENTARCH
+ hex
+ default 0x16 if ARM64
+ default 0x15 if ARM
+ default 0xFF
Do we need a separate option or could we use BOOTP_PXE_CLIENTARCH ?
I created a new option because I wanted to change the default to 0xFF
("undefined" Processor Architecture Types according to
https://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xml).
I wanted to do this without changing BOOTP_PXE_CLIENTARCH , and
potentially disrupting exisiting DHCPv4 implementations.
+
+config DHCP6_PXE_DHCP_OPTION
+ bool "Request & store 'pxe_configfile' from DHCP6 server"
+
+config DHCP6_ENTERPRISE_ID
+ int "Enterprise ID to send in DHCPv6 Vendor Class Option"
+ default 0
+
+endif
+
config CMD_TFTPBOOT
bool "tftpboot"
default y
diff --git a/cmd/net.c b/cmd/net.c
index d5e20843dd..95529a9d12 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -111,6 +111,29 @@ U_BOOT_CMD(
);
#endif
+#if defined(CONFIG_CMD_DHCP6)
+static int do_dhcp6(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ int i;
+ int dhcp_argc;
+ char *dhcp_argv[] = {NULL, NULL, NULL, NULL};
+
+ /* Add -ipv6 flag for autoload */
+ for (i = 0; i < argc; i++)
+ dhcp_argv[i] = argv[i];
+ dhcp_argc = argc + 1;
+ dhcp_argv[dhcp_argc - 1] = USE_IP6_CMD_PARAM;
+
+ return netboot_common(DHCP6, cmdtp, dhcp_argc, dhcp_argv);
+}
+
+U_BOOT_CMD(dhcp6, 3, 1, do_dhcp6,
+ "boot image via network using DHCPv6/TFTP protocol. \n"
+ "Use IPv6 hostIPaddr framed with [] brackets",
+ "[loadAddress] [[hostIPaddr:]bootfilename]");
+#endif
+
#if defined(CONFIG_CMD_DHCP)
static int do_dhcp(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
diff --git a/cmd/pxe.c b/cmd/pxe.c
index db8e4697f2..71e6fd9633 100644
--- a/cmd/pxe.c
+++ b/cmd/pxe.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include "pxe_utils.h"
@@ -29,12 +30,20 @@ static int do_get_tftp(struct pxe_context *ctx, const char
*file_path,
{
char *tftp_argv[] = {"tftp", NULL, NULL, NULL};
int ret;
+ int num_args;
tftp_argv[1] = file_addr;
tftp_argv[2] = (void *)file_path;
+ if (ctx->use_ipv6) {
+ tftp_argv[3] = USE_IP6_CMD_PARAM;
+ num_args = 4;
+ } else {
+ num_args = 3;
+ }
- if (do_tftpb(ctx->cmdtp, 0, 3, tftp_argv))
+ if (do_tftpb(ctx->cmdtp, 0, num_args, tftp_argv))
return -ENOENT;
+
ret = pxe_get_file_size(sizep);
if (ret)
return log_msg_ret("tftp", ret);
@@ -43,6 +52,23 @@ static int do_get_tftp(struct pxe_context *ctx, const char
*file_path,
return 1;
}
+#if defined(CONFIG_DHCP6_PXE_DHCP_OPTION)
+/*
+ * Looks for a pxe file with specified config file name,
+ * which is received from DHCPv4 option 209 or
+ * DHCPv6 option 60.
+ *
+ * Returns 1 on success or < 0 on error.
+ */
+static inline int pxe_dhcp_option_path(struct pxe_context *ctx, unsigned long
pxefile_addr_r)
Please drop the inline as the compiler can handle that.
+{
+ int ret = get_pxe_file(ctx, pxelinux_configfile, pxefile_addr_r);
+
+ free(pxelinux_configfile);
+
+ return ret;
+}
+#endif
/*
* Looks for a pxe file with a name based on the pxeuuid environment variable.
*
@@ -105,15 +131,25 @@ static int pxe_ipaddr_paths(struct pxe_context *ctx,
unsigned long pxefile_addr_
return -ENOENT;
}
-int pxe_get(ulong pxefile_addr_r, char **bootdirp, ulong *sizep)
+int pxe_get(ulong pxefile_addr_r, char **bootdirp, ulong *sizep, bool use_ipv6)
{
struct cmd_tbl cmdtp[] = {};/* dummy */
struct pxe_context ctx;
int i;
if (pxe_setup_ctx(, cmdtp, do_get_tftp, NULL, false,
- env_get("bootfile")))
+ env_get("bootfile"), use_ipv6))
Re: [PATCH] net: ipv6: IPv6 environment variable cleanup
Sat, Feb 18, 2023 at 10:09:56PM +0200, Ramon Fried wrote:
> On Thu, Feb 16, 2023 at 6:39 AM wrote:
> >
> > From: Sean Edmond
> >
> > Fix "setenv gatewayip6".
> >
> > Synchronize IPv6 local variables with environment variables
> > in netboot_update_env()
> >
> > Signed-off-by: Sean Edmond
> > ---
> > cmd/net.c | 23 ++-
> > include/env_flags.h | 2 +-
> > 2 files changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/cmd/net.c b/cmd/net.c
> > index 88d53d14d5..0161c87529 100644
> > --- a/cmd/net.c
> > +++ b/cmd/net.c
> > @@ -208,7 +208,7 @@ U_BOOT_CMD(
> >
> > static void netboot_update_env(void)
> > {
> > - char tmp[22];
> > + char tmp[44];
> >
> > if (net_gateway.s_addr) {
> > ip_to_string(net_gateway, tmp);
> > @@ -269,6 +269,27 @@ static void netboot_update_env(void)
> > env_set("ntpserverip", tmp);
> > }
> > #endif
> > +
> > + if (IS_ENABLED(CONFIG_IPV6)) {
> > + if (!ip6_is_unspecified_addr(_ip6) ||
> > + net_prefix_length != 0) {
> > + sprintf(tmp, "%pI6c", _ip6);
> > + if (net_prefix_length != 0)
> > + sprintf(tmp, "%s/%d", tmp,
> > net_prefix_length);
> > +
> > + env_set("ip6addr", tmp);
> > + }
> > +
> > + if (!ip6_is_unspecified_addr(_server_ip6)) {
> > + sprintf(tmp, "%pI6c", _server_ip6);
> > + env_set("serverip6", tmp);
> > + }
> > +
> > + if (!ip6_is_unspecified_addr(_gateway6)) {
> > + sprintf(tmp, "%pI6c", _gateway6);
> > + env_set("gatewayip6", tmp);
> > + }
> > + }
> > }
> >
> > /**
> > diff --git a/include/env_flags.h b/include/env_flags.h
> > index 6bd574c2bd..7df40c59be 100644
> > --- a/include/env_flags.h
> > +++ b/include/env_flags.h
> > @@ -71,7 +71,7 @@ enum env_flags_varaccess {
> > #define NET6_FLAGS \
> > "ip6addr:s," \
> > "serverip6:s," \
> > - "gatewayip6:s"
> > + "gatewayip6:s,"
> Why is this needed ?
Without the comma, we see the following error when trying to set the gatewayip6
environment variable:
=> setenv gatewayip6 fe80::f0c7:7cff:fea9:c4ec
## Warning: Unknown environment variable access method 's'
## Error inserting "gatewayip6" variable, errno=3
> > #else
> > #define NET6_FLAGS
> > #endif
> > --
> > 2.39.0
> >
RE: [EXTERNAL] Re: [PATCH] net: ipv6: IPv6 environment variable cleanup
>-Original Message-
>From: Ramon Fried
>Sent: Saturday, February 18, 2023 12:10 PM
>To: seanedm...@linux.microsoft.com
>Cc: u-boot@lists.denx.de; joe.hershber...@ni.com; w...@denx.de; Sean
>Edmond
>Subject: [EXTERNAL] Re: [PATCH] net: ipv6: IPv6 environment variable cleanup
>
>On Thu, Feb 16, 2023 at 6:39 AM wrote:
>>
>> From: Sean Edmond
>>
>> Fix "setenv gatewayip6".
>>
>> Synchronize IPv6 local variables with environment variables in
>> netboot_update_env()
>>
>> Signed-off-by: Sean Edmond
>> ---
>> cmd/net.c | 23 ++-
>> include/env_flags.h | 2 +-
>> 2 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/cmd/net.c b/cmd/net.c
>> index 88d53d14d5..0161c87529 100644
>> --- a/cmd/net.c
>> +++ b/cmd/net.c
>> @@ -208,7 +208,7 @@ U_BOOT_CMD(
>>
>> static void netboot_update_env(void)
>> {
>> - char tmp[22];
>> + char tmp[44];
>>
>> if (net_gateway.s_addr) {
>> ip_to_string(net_gateway, tmp); @@ -269,6 +269,27 @@
>> static void netboot_update_env(void)
>> env_set("ntpserverip", tmp);
>> }
>> #endif
>> +
>> + if (IS_ENABLED(CONFIG_IPV6)) {
>> + if (!ip6_is_unspecified_addr(_ip6) ||
>> + net_prefix_length != 0) {
>> + sprintf(tmp, "%pI6c", _ip6);
>> + if (net_prefix_length != 0)
>> + sprintf(tmp, "%s/%d", tmp,
>> + net_prefix_length);
>> +
>> + env_set("ip6addr", tmp);
>> + }
>> +
>> + if (!ip6_is_unspecified_addr(_server_ip6)) {
>> + sprintf(tmp, "%pI6c", _server_ip6);
>> + env_set("serverip6", tmp);
>> + }
>> +
>> + if (!ip6_is_unspecified_addr(_gateway6)) {
>> + sprintf(tmp, "%pI6c", _gateway6);
>> + env_set("gatewayip6", tmp);
>> + }
>> + }
>> }
>>
>> /**
>> diff --git a/include/env_flags.h b/include/env_flags.h index
>> 6bd574c2bd..7df40c59be 100644
>> --- a/include/env_flags.h
>> +++ b/include/env_flags.h
>> @@ -71,7 +71,7 @@ enum env_flags_varaccess { #define NET6_FLAGS \
>> "ip6addr:s," \
>> "serverip6:s," \
>> - "gatewayip6:s"
>> + "gatewayip6:s,"
>Why is this needed ?
Without the comma, we see the following error when trying to set the gatewayip6
environment variable:
=> setenv gatewayip6 fe80::f0c7:7cff:fea9:c4ec
## Warning: Unknown environment variable access method 's'
## Error inserting "gatewayip6" variable, errno=3
>> #else
>> #define NET6_FLAGS
>> #endif
>> --
>> 2.39.0
>>
