Hi Maxim, On Tue, 26 Sept 2023 at 03:44, Maxim Uvarov <maxim.uva...@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.uva...@linaro.org> > --- > include/net/lwip.h | 12 +++++ > net/lwip/Makefile | 1 + > net/lwip/apps/dhcp/lwip-dhcp.c | 85 ++++++++++++++++++++++++++++++++++ > 3 files changed, 98 insertions(+) > create mode 100644 net/lwip/apps/dhcp/lwip-dhcp.c
Reviewed-by: Simon Glass <s...@chromium.org> with nits > > diff --git a/include/net/lwip.h b/include/net/lwip.h > index ab3db1a214..6a8fcef392 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 > +*/ > +int ulwip_dhcp(void); > diff --git a/net/lwip/Makefile b/net/lwip/Makefile > index 5d8d5527c6..a3a33b7f71 100644 > --- a/net/lwip/Makefile > +++ b/net/lwip/Makefile > @@ -63,4 +63,5 @@ obj-$(CONFIG_NET) += lwip-external/src/netif/ethernet.o > obj-$(CONFIG_NET) += port/if.o > obj-$(CONFIG_NET) += port/sys-arch.o > > +obj-y += apps/dhcp/lwip-dhcp.o > obj-y += 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..f0b0e26f6e > --- /dev/null > +++ b/net/lwip/apps/dhcp/lwip-dhcp.c > @@ -0,0 +1,85 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* > + * (C) Copyright 2023 Linaro Ltd. <maxim.uva...@linaro.org> > + */ > + > +#include <common.h> > +#include <command.h> > +#include <console.h> > + > +#include <lwip/dhcp.h> > +#include <lwip/prot/dhcp.h> > +#include "lwip/timeouts.h" > + > +#include <net/eth.h> > +#include <net/ulwip.h> > + > +#define DHCP_TMO_TIME 500 /* poll for DHCP state change */ > +#define DHCP_TMO_NUM 10 /* number of tries */ > + > +typedef struct dhcp_priv { can you drop typedef? > + int num_tries; > + struct netif *netif; > +} dhcp_priv; > + > +static void dhcp_tmo(void *arg) > +{ > + struct dhcp_priv *dpriv = (struct dhcp_priv *)arg; > + struct netif *netif = dpriv->netif; > + struct dhcp *dhcp; > + > + dhcp = netif_get_client_data(netif, > LWIP_NETIF_CLIENT_DATA_INDEX_DHCP); > + if (!dhcp) > + return; > + > + if (dhcp->state == DHCP_STATE_BOUND) { > + int err = 0; > + > + err -= env_set("bootfile", dhcp->boot_file_name); > + err -= env_set("ipaddr", > ip4addr_ntoa(&dhcp->offered_ip_addr)); > + err -= env_set("netmask", > ip4addr_ntoa(&dhcp->offered_sn_mask)); > + err -= env_set("serverip", > ip4addr_ntoa(&dhcp->server_ip_addr)); > + if (err) > + log_err("error update envs\n"); but don't you need to return the effort? How does a script know it failed? I think this function need an error return...if that is not possible then you need to stash the error somehow so you can report it when needed. > + log_info("DHCP client bound to address %s\n", > ip4addr_ntoa(&dhcp->offered_ip_addr)); > + free(dpriv); > + ulwip_exit(err); > + return; > + } > + > + dpriv->num_tries--; > + if (dpriv->num_tries < 0) { > + log_err("DHCP client timeout\n"); > + free(dpriv); > + ulwip_exit(-1); > + return; > + } > + > + sys_timeout(DHCP_TMO_TIME, dhcp_tmo, dpriv); > +} > + > +int ulwip_dhcp(void) > +{ > + struct netif *netif; > + int eth_idx; > + struct dhcp_priv *dpriv; > + > + dpriv = malloc(sizeof(struct dhcp_priv)); > + if (!dpriv) > + return -EPERM; -ENOMEM > + > + eth_idx = eth_get_dev_index(); > + if (eth_idx < 0) > + return -EPERM; > + > + netif = netif_get_by_index(eth_idx + 1); > + if (!netif) > + return -ENOENT; > + > + dpriv->num_tries = DHCP_TMO_NUM; > + dpriv->netif = netif; > + sys_timeout(DHCP_TMO_TIME, dhcp_tmo, dpriv); > + > + return dhcp_start(netif) ? 0 : -EPERM; Why -EPERM? Is it a permission error? Can we not get a real error number? > +} > -- > 2.30.2 > Regards, SImon