Hi Maxim, On Tue, 22 Aug 2023 at 03:39, Maxim Uvarov <maxim.uva...@linaro.org> wrote: > > U-Boot recently got support for an alternative network stack using LWIP. > Replace dns 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 | 19 +++++++++++++++ > net/lwip/Makefile | 2 ++ > net/lwip/apps/dns/lwip-dns.c | 46 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 67 insertions(+) > create mode 100644 include/net/lwip.h > create mode 100644 net/lwip/apps/dns/lwip-dns.c > > diff --git a/include/net/lwip.h b/include/net/lwip.h > new file mode 100644 > index 0000000000..cda896d062 > --- /dev/null > +++ b/include/net/lwip.h > @@ -0,0 +1,19 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +int do_lwip_dns(struct cmd_tbl *cmdtp, int flag, int argc, > + char *const argv[]);
Please make sure you comment all exported functions, including the return value. > + > +/** > + * ulwip_dns() - creates the DNS request to resolve a domain host name > + * > + * This function creates the DNS request to resolve a domain host name. > Function > + * can return immediately if previous request was cached or it might require > + * entering the polling loop for a request to a remote server. > + * > + * @name: dns name to resolve > + * @varname: (optional) U-Boot variable name to store the result > + * Returns: ERR_OK(0) for fetching entry from the cache > + * ERR_INPROGRESS(-5) success, can go to the polling loop > + * Other value < 0, if error > + */ > +int ulwip_dns(char *name, char *varname); > diff --git a/net/lwip/Makefile b/net/lwip/Makefile > index d1161bea78..6d2c00605b 100644 > --- a/net/lwip/Makefile > +++ b/net/lwip/Makefile > @@ -64,3 +64,5 @@ obj-$(CONFIG_NET) += $(LWIPDIR)/netif/ethernet.o > > obj-$(CONFIG_NET) += port/if.o > obj-$(CONFIG_NET) += port/sys-arch.o > + > +obj-$(CONFIG_CMD_DNS) += apps/dns/lwip-dns.o > diff --git a/net/lwip/apps/dns/lwip-dns.c b/net/lwip/apps/dns/lwip-dns.c > new file mode 100644 > index 0000000000..6e205c1153 > --- /dev/null > +++ b/net/lwip/apps/dns/lwip-dns.c > @@ -0,0 +1,46 @@ > +// 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/dns.h> > +#include <lwip/ip_addr.h> > + > +#include <net/ulwip.h> > + > +static void dns_found_cb(const char *name, const ip_addr_t *ipaddr, void > *callback_arg) > +{ > + char *varname = (char *)callback_arg; > + > + if (varname) > + env_set(varname, ip4addr_ntoa(ipaddr)); > + > + log_info("resolved %s to %s\n", name, ip4addr_ntoa(ipaddr)); > + ulwip_exit(0); > +} > + > +int ulwip_dns(char *name, char *varname) > +{ > + int err; > + ip_addr_t ipaddr; /* not used */ > + ip_addr_t dns1; > + ip_addr_t dns2; > + > + ipaddr_aton(env_get("dnsip"), &dns1); > + ipaddr_aton(env_get("dnsip2"), &dns2); What if the env_get() fails? Won't that return NULL? > + > + dns_init(); > + dns_setserver(0, &dns1); > + dns_setserver(1, &dns2); Can either of these fail? Please be very attentive to error-checking. > + > + err = dns_gethostbyname(name, &ipaddr, dns_found_cb, varname); > + if (err == ERR_OK) Is that 0? If so, then if (err) is better > + dns_found_cb(name, &ipaddr, varname); > + > + return err; I am not sure what that is, but will review it when you add the header comments. > +} > -- > 2.30.2 > Regards, Simon