Re: [PATCH v2 17/29] net: bootp: Fall back to BOOTP from DHCP when unit testing

Simon Glass
2023-10-15T14:47:00Z

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