[PATCH v11 13/29] net-lwip: add DHCP support and dhcp commmand

[Jerome Forissier]

On 10/4/24 08:50, Ilias Apalodimas wrote:
> Hi Jerome,
> 
> [...]
> 
> 
>> +#define DHCP_TIMEOUT_MS 10000
>> +
>> +#ifdef CONFIG_CMD_TFTPBOOT
>> +/* Boot file obtained from DHCP (if present) */
>> +static char boot_file_name[DHCP_BOOT_FILE_LEN];
>> +#endif
>> +
>> +static void call_lwip_dhcp_fine_tmr(void *ctx)
>> +{
>> +       dhcp_fine_tmr();
>> +       sys_timeout(10, call_lwip_dhcp_fine_tmr, NULL);
>> +}
>> +
>> +static int dhcp_loop(struct udevice *udev)
>> +{
>> +       char *ipstr = "ipaddr\0\0";
>> +       char *maskstr = "netmask\0\0";
>> +       char *gwstr = "gatewayip\0\0";
>> +       unsigned long start;
>> +       struct netif *netif;
>> +       struct dhcp *dhcp;
>> +       bool bound;
>> +       int idx;
>> +
>> +       idx = dev_seq(udev);
>> +       if (idx < 0 || idx > 99) {
>> +               log_err("unexpected idx %d\n", idx);
>> +               return CMD_RET_FAILURE;
>> +       }
>> +
>> +       netif = net_lwip_new_netif_noip(udev);
>> +       if (!netif)
>> +               return CMD_RET_FAILURE;
>> +
>> +       start = get_timer(0);
>> +       dhcp_start(netif);
> 
> This can fail. Don't we have to check that and exit?

Yes, will fix.

>> +       call_lwip_dhcp_fine_tmr(NULL);
>> +
>> +       /* Wait for DHCP to complete */
>> +       do {
>> +               net_lwip_rx(udev, netif);
>> +               sys_check_timeouts();
>> +               bound = dhcp_supplied_address(netif);
>> +               if (bound)
>> +                       break;
>> +               if (ctrlc()) {
>> +                       printf("Abort\n");
>> +                       break;
>> +               }
>> +               mdelay(1);
>> +       } while (get_timer(start) < DHCP_TIMEOUT_MS);
>> +
>> +       sys_untimeout(call_lwip_dhcp_fine_tmr, NULL);
>> +
> 
> [...]
> 
>> +       free(pp);
>> +       if (err) {
>> +               log_err("send error %d\n", err);
>> +               return ERR_ABRT;
>> +       }
>> +
>> +       return ERR_OK;
>> +}
>> +
>> +static err_t net_lwip_if_init(struct netif *netif)
>> +{
>> +#if LWIP_IPV4
> 
> Do we ever not set this?

No. Removed in v12.

 
>> +       netif->output = etharp_output;
>> +#endif
>> +       netif->linkoutput = linkoutput;
>> +       netif->mtu = 1500;
>> +       netif->flags = NETIF_FLAG_BROADCAST | NETIF_FLAG_ETHARP | NETIF_FLAG_LINK_UP;
>> +
>> +       return ERR_OK;
>> +}
>> +
>> +static void eth_init_rings(void)
>> +{
>> +       int i;
>> +
>> +       for (i = 0; i < PKTBUFSRX; i++)
>> +               net_rx_packets[i] = net_pkt_buf + i  * PKTSIZE_ALIGN;
>> +}
>> +
>> +struct netif *net_lwip_get_netif(void)
>> +{
>> +       struct netif *netif, *found = NULL;
>> +
>> +       NETIF_FOREACH(netif) {
>> +               if (!found)
>> +                       found = netif;
>> +               else
>> +                       printf("Error: more than one netif in lwIP\n");
>> +       }
>> +       return found;
>> +}
>> +
>> +static int get_udev_ipv4_info(struct udevice *dev, ip4_addr_t *ip,
>> +                             ip4_addr_t *mask, ip4_addr_t *gw)
>> +{
>> +       char *ipstr = "ipaddr\0\0";
>> +       char *maskstr = "netmask\0\0";
>> +       char *gwstr = "gatewayip\0\0";
>> +       int idx = dev_seq(dev);
>> +       char *env;
>> +
>> +       if (idx < 0 || idx > 99) {
>> +               log_err("unexpected idx %d\n", idx);
>> +               return -1;
>> +       }
>> +
> 
> [...]
> 
> FWIW dhcp seems to work fine
> 
> Tested-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> 
> Thanks
> /Ilias

Thanks,
-- 
Jerome