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

Sean Anderson
2023-10-15T07:24:34Z

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.

--Sean