[PATCHv7 04/15] net/lwip: implement dhcp cmd

[Simon Glass]

Hi Maxim,

On Tue, 22 Aug 2023 at 03:39, Maxim Uvarov <maxim.uvarov at linaro.org> wrote:
>
> U-Boot recently got support for an alternative network stack using LWIP.
> Replace dhcp command with the LWIP variant while keeping the output and
> error messages identical.
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov at linaro.org>
> ---
>  include/net/lwip.h             | 12 +++++++
>  net/lwip/Makefile              |  1 +
>  net/lwip/apps/dhcp/lwip-dhcp.c | 62 ++++++++++++++++++++++++++++++++++
>  3 files changed, 75 insertions(+)
>  create mode 100644 net/lwip/apps/dhcp/lwip-dhcp.c
>
> diff --git a/include/net/lwip.h b/include/net/lwip.h
> index cda896d062..240ebba354 100644
> --- a/include/net/lwip.h
> +++ b/include/net/lwip.h
> @@ -17,3 +17,15 @@ int do_lwip_dns(struct cmd_tbl *cmdtp, int flag, int argc,
>   *         Other value < 0, if error
>   */
>  int ulwip_dns(char *name, char *varname);
> +
> +/**
> + * ulwip_dhcp() -  create the DHCP request to obtain IP address.
> + *
> + * This function creates the DHCP request to obtain IP address. If DHCP server
> + * returns file name, this file will be downloaded with tftp.  After this
> + * function you need to invoke the polling loop to process network communication.
> + *
> + * Returns: 0 if success
> + *         Other value < 0, if error

So this is an errno value from errno.h ?

> +*/
> +int ulwip_dhcp(void);
> diff --git a/net/lwip/Makefile b/net/lwip/Makefile
> index 6d2c00605b..59323fb325 100644
> --- a/net/lwip/Makefile
> +++ b/net/lwip/Makefile
> @@ -65,4 +65,5 @@ obj-$(CONFIG_NET) += $(LWIPDIR)/netif/ethernet.o
>  obj-$(CONFIG_NET) += port/if.o
>  obj-$(CONFIG_NET) += port/sys-arch.o
>
> +obj-$(CONFIG_CMD_DHCP) += apps/dhcp/lwip-dhcp.o
>  obj-$(CONFIG_CMD_DNS) += apps/dns/lwip-dns.o
> diff --git a/net/lwip/apps/dhcp/lwip-dhcp.c b/net/lwip/apps/dhcp/lwip-dhcp.c
> new file mode 100644
> index 0000000000..cce1e367f9
> --- /dev/null
> +++ b/net/lwip/apps/dhcp/lwip-dhcp.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov at linaro.org>
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <console.h>
> +
> +#include <lwip/dhcp.h>
> +#include <lwip/prot/dhcp.h>
> +
> +#include <net/eth.h>
> +#include <net/ulwip.h>
> +
> +static struct dhcp dhcp;

Can we keep the state in the uclass?

> +
> +static int ulwip_dhcp_tmo(void)
> +{
> +       switch (dhcp.state) {
> +       case DHCP_STATE_BOUND:
> +               env_set("bootfile", dhcp.boot_file_name);
> +               env_set("ipaddr", ip4addr_ntoa(&dhcp.offered_ip_addr));
> +               env_set("netmask", ip4addr_ntoa(&dhcp.offered_sn_mask));
> +               env_set("serverip", ip4addr_ntoa(&dhcp.server_ip_addr));

Please check errors throughout your patches.

> +               printf("DHCP client bound to address %s\n", ip4addr_ntoa(&dhcp.offered_ip_addr));
> +               break;
> +       default:
> +               return -1;
> +       }
> +
> +       return 0;
> +}
> +
> +int ulwip_dhcp(void)
> +{
> +       int err;
> +       struct netif *netif;
> +       int eth_idx;
> +
> +       eth_idx = eth_get_dev_index();
> +       if (eth_idx < 0) {
> +               log_err("no eth idx\n");
> +               return -1;

That is -EPERM...please use a suitable error, perhaps -ENOENT?

> +       }
> +
> +       netif = netif_get_by_index(eth_idx + 1);
> +       if (!netif)
> +               return -1;
> +
> +       ulwip_set_tmo(ulwip_dhcp_tmo);
> +
> +       if (!netif_get_client_data(netif, LWIP_NETIF_CLIENT_DATA_INDEX_DHCP))
> +               dhcp_set_struct(netif, &dhcp);
> +
> +       err = dhcp_start(netif);
> +       if (err)
> +               log_err("dhcp_start error %d\n", err);

Ideally the caller would print the errors. We try to have commands do
that, since then it allows silent running for things which want it. It
also helps with code size.

> +
> +       return err;
> +}
> --
> 2.30.2
>

Regards,
Simon