| Message ID | 20231014204805.439009-18-seanga2@gmail.com |
|---|---|
| State | Accepted |
| Commit | acb96c170d7c9f9aed1755efe56387127a7dff0b |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | test: spl: Test some load methods | expand |
Hi Sean, On Sat, 14 Oct 2023 at 15:27, Sean Anderson <seanga2@gmail.com> wrote: > > On 10/14/23 17:22, Heinrich Schuchardt wrote: > > > > > > Am 14. Oktober 2023 22:47:53 MESZ schrieb Sean Anderson <seanga2@gmail.com>: > >> If we sent a DHCP packet and get a BOOTP response from the server, we > >> shouldn't try to send a DHCPREQUEST packet, since it won't be DHCPACKed. > >> Transition straight to BIND. This is only enabled for UNIT_TEST to avoid > >> bloat, since I suspect the number of BOOTP servers in the wild is > >> vanishingly small. > >> > >> Signed-off-by: Sean Anderson <seanga2@gmail.com> > >> Reviewed-by: Simon Glass <sjg@chromium.org> > >> --- > >> > >> (no changes since v1) > >> > >> net/bootp.c | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/net/bootp.c b/net/bootp.c > >> index 2053cce88c6..7b0f45e18a9 100644 > >> --- a/net/bootp.c > >> +++ b/net/bootp.c > >> @@ -1073,6 +1073,11 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip, > >> CONFIG_SYS_BOOTFILE_PREFIX, > >> strlen(CONFIG_SYS_BOOTFILE_PREFIX)) == 0) { > >> #endif /* CONFIG_SYS_BOOTFILE_PREFIX */ > >> + if (CONFIG_IS_ENABLED(UNIT_TEST) && > >> + dhcp_message_type((u8 *)bp->bp_vend) == -1) { > > > > As written before, please, do not add unit test specific code paths. > > While it is convenient for tests to implement a BOOTP server, there are > effectively no BOOTP servers in the wild. However, BOOTP is commonly enabled > in U-Boot. In an effort to avoid growing most U-Boots for testing purposes, > I enabled this path just for unit tests. That said, only 6 boards enable SPL_ETH, > so maybe it is not too bad. Yes, also if you are using networking in SPL, you can expect it to add quite a bit to code size. Regards, Simon
On Sun, Oct 15, 2023 at 08:39:50AM -0600, Simon Glass wrote: > Hi Sean, > > On Sat, 14 Oct 2023 at 15:27, Sean Anderson <seanga2@gmail.com> wrote: > > > > On 10/14/23 17:22, Heinrich Schuchardt wrote: > > > > > > > > > Am 14. Oktober 2023 22:47:53 MESZ schrieb Sean Anderson <seanga2@gmail.com>: > > >> If we sent a DHCP packet and get a BOOTP response from the server, we > > >> shouldn't try to send a DHCPREQUEST packet, since it won't be DHCPACKed. > > >> Transition straight to BIND. This is only enabled for UNIT_TEST to avoid > > >> bloat, since I suspect the number of BOOTP servers in the wild is > > >> vanishingly small. > > >> > > >> Signed-off-by: Sean Anderson <seanga2@gmail.com> > > >> Reviewed-by: Simon Glass <sjg@chromium.org> > > >> --- > > >> > > >> (no changes since v1) > > >> > > >> net/bootp.c | 6 ++++++ > > >> 1 file changed, 6 insertions(+) > > >> > > >> diff --git a/net/bootp.c b/net/bootp.c > > >> index 2053cce88c6..7b0f45e18a9 100644 > > >> --- a/net/bootp.c > > >> +++ b/net/bootp.c > > >> @@ -1073,6 +1073,11 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip, > > >> CONFIG_SYS_BOOTFILE_PREFIX, > > >> strlen(CONFIG_SYS_BOOTFILE_PREFIX)) == 0) { > > >> #endif /* CONFIG_SYS_BOOTFILE_PREFIX */ > > >> + if (CONFIG_IS_ENABLED(UNIT_TEST) && > > >> + dhcp_message_type((u8 *)bp->bp_vend) == -1) { > > > > > > As written before, please, do not add unit test specific code paths. > > > > While it is convenient for tests to implement a BOOTP server, there are > > effectively no BOOTP servers in the wild. However, BOOTP is commonly enabled > > in U-Boot. In an effort to avoid growing most U-Boots for testing purposes, > > I enabled this path just for unit tests. That said, only 6 boards enable SPL_ETH, > > so maybe it is not too bad. > > Yes, also if you are using networking in SPL, you can expect it to add > quite a bit to code size. And so long as this doesn't lead to overflowing them, OK. But size is not infinite there, am335x_evm and am43xx_hs_evm are both a bit constrained.
diff --git a/net/bootp.c b/net/bootp.c index 2053cce88c6..7b0f45e18a9 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -1073,6 +1073,11 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip, CONFIG_SYS_BOOTFILE_PREFIX, strlen(CONFIG_SYS_BOOTFILE_PREFIX)) == 0) { #endif /* CONFIG_SYS_BOOTFILE_PREFIX */ + if (CONFIG_IS_ENABLED(UNIT_TEST) && + dhcp_message_type((u8 *)bp->bp_vend) == -1) { + debug("got BOOTP response; transitioning to BOUND\n"); + goto dhcp_got_bootp; + } dhcp_packet_process_options(bp); if (CONFIG_IS_ENABLED(EFI_LOADER) && IS_ENABLED(CONFIG_NETDEVICES)) @@ -1099,6 +1104,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip, debug("DHCP State: REQUESTING\n"); if (dhcp_message_type((u8 *)bp->bp_vend) == DHCP_ACK) { +dhcp_got_bootp: dhcp_packet_process_options(bp); /* Store net params from reply */ store_net_params(bp);