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

Tom Rini
2023-10-15T19:28:18Z

[-- Attachment #1: Type: text/plain, Size: 2205 bytes --]

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.

--
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]