diff mbox series

[PATCHv7,03/15] net/lwip: implement dns cmd

Message ID 20230822093614.4717-4-maxim.uvarov@linaro.org
State New
Headers show
Series net/lwip: add lwip library for the network stack | expand

Commit Message

Maxim Uvarov Aug. 22, 2023, 9:36 a.m. UTC
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.uvarov@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

Comments

Simon Glass Aug. 22, 2023, 6:56 p.m. UTC | #1
Hi Maxim,

On Tue, 22 Aug 2023 at 03:39, Maxim Uvarov <maxim.uvarov@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.uvarov@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.uvarov@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
Maxim Uvarov Sept. 1, 2023, 2:49 p.m. UTC | #2
On Wed, 23 Aug 2023 at 00:58, Simon Glass <sjg@google.com> wrote:

> Hi Maxim,
>
> On Tue, 22 Aug 2023 at 03:39, Maxim Uvarov <maxim.uvarov@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.uvarov@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
> > + */
>

here.


> > +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.uvarov@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?
>
>
all of these functions will not crash, they have null check. You can set
dnsip or dnsip2 or both. If both are not set then dns cmd will not look up
anything.
We can exit earlier here, but that is a common case and nothing bad if we
make code simpler and exit a little bit later.



> > +
> > +       dns_init();
> > +       dns_setserver(0, &dns1);
> > +       dns_setserver(1, &dns2);
>
> Can either of these fail?
>
> Please be very attentive to error-checking.
>

All  above functions are void. But they have LWIP_ASSERT() for sanity
checks in some places.


>
> > +
> > +       err = dns_gethostbyname(name, &ipaddr, dns_found_cb, varname);
> > +       if (err == ERR_OK)
>
> Is that 0? If so, then if (err) is better
>
>
dns_gethostbyname() returns ERR_OK which is defined
net/lwip/lwip-external/src/include/lwip/err.h.
Yes it's defined to 0 and maybe always will be defined to 0. But  that may
change. And I would keep
the check against the same return macro that the function does.



> > +               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.
>
> In the header file of this function is an explanation. It's above in this
patch:
+ * 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
+ */


> > +}
> > --
> > 2.30.2
> >
>
> Regards,
> Simon
>
Simon Glass Sept. 2, 2023, 12:09 a.m. UTC | #3
Hi Maxim,

On Fri, 1 Sept 2023 at 08:49, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
>
>
> On Wed, 23 Aug 2023 at 00:58, Simon Glass <sjg@google.com> wrote:
>>
>> Hi Maxim,
>>
>> On Tue, 22 Aug 2023 at 03:39, Maxim Uvarov <maxim.uvarov@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.uvarov@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
>> > + */
>
>
> here.

That seems to be a different function?

>
>>
>> > +int ulwip_dns(char *name, char *varname);

This one has no comment?

>> > 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.uvarov@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));

That can fail

>> > +
>> > +       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?
>>
>
> all of these functions will not crash, they have null check. You can set dnsip or dnsip2 or both. If both are not set then dns cmd will not look up anything.
> We can exit earlier here, but that is a common case and nothing bad if we make code simpler and exit a little bit later.

OK but I cannot see where the error is returned?

>
>
>>
>> > +
>> > +       dns_init();
>> > +       dns_setserver(0, &dns1);
>> > +       dns_setserver(1, &dns2);
>>
>> Can either of these fail?
>>
>> Please be very attentive to error-checking.
>
>
> All  above functions are void. But they have LWIP_ASSERT() for sanity checks in some places.
>
>>
>>
>> > +
>> > +       err = dns_gethostbyname(name, &ipaddr, dns_found_cb, varname);
>> > +       if (err == ERR_OK)
>>
>> Is that 0? If so, then if (err) is better
>>
>
> dns_gethostbyname() returns ERR_OK which is defined net/lwip/lwip-external/src/include/lwip/err.h.
> Yes it's defined to 0 and maybe always will be defined to 0. But  that may change. And I would keep
> the check against the same return macro that the function does.

I cannot imagine it changing.

>
>
>>
>> > +               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.
>>
> In the header file of this function is an explanation. It's above in this patch:
> + * 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
> + */

OK, so what I am getting at is trying to understand the error
conversion. It seems that lwip uses different numbering from
U-Boot/Linux, so we need to do a conversion somewhere?

Regards,
Simon
Maxim Uvarov Sept. 4, 2023, 6:58 a.m. UTC | #4
On Sat, 2 Sept 2023 at 06:09, Simon Glass <sjg@google.com> wrote:

> Hi Maxim,
>
> On Fri, 1 Sept 2023 at 08:49, Maxim Uvarov <maxim.uvarov@linaro.org>
> wrote:
> >
> >
> >
> > On Wed, 23 Aug 2023 at 00:58, Simon Glass <sjg@google.com> wrote:
> >>
> >> Hi Maxim,
> >>
> >> On Tue, 22 Aug 2023 at 03:39, Maxim Uvarov <maxim.uvarov@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.uvarov@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
> >> > + */
> >
> >
> > here.
>
> That seems to be a different function?
>
> >
> >>
> >> > +int ulwip_dns(char *name, char *varname);
>
> This one has no comment?
>
> >> > 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.uvarov@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));
>
> That can fail
>
>
yea, will add a check.


> >> > +
> >> > +       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?
> >>
> >
> > all of these functions will not crash, they have null check. You can set
> dnsip or dnsip2 or both. If both are not set then dns cmd will not look up
> anything.
> > We can exit earlier here, but that is a common case and nothing bad if
> we make code simpler and exit a little bit later.
>
> OK but I cannot see where the error is returned?
>
>
I will add more checks.  dns_gethostbyname() will return an error if dns
were set wrongly. But I will also add more checks here to be clear for
other people also.


> >
> >
> >>
> >> > +
> >> > +       dns_init();
> >> > +       dns_setserver(0, &dns1);
> >> > +       dns_setserver(1, &dns2);
> >>
> >> Can either of these fail?
> >>
> >> Please be very attentive to error-checking.
> >
> >
> > All  above functions are void. But they have LWIP_ASSERT() for sanity
> checks in some places.
> >
> >>
> >>
> >> > +
> >> > +       err = dns_gethostbyname(name, &ipaddr, dns_found_cb, varname);
> >> > +       if (err == ERR_OK)
> >>
> >> Is that 0? If so, then if (err) is better
> >>
> >
> > dns_gethostbyname() returns ERR_OK which is defined
> net/lwip/lwip-external/src/include/lwip/err.h.
> > Yes it's defined to 0 and maybe always will be defined to 0. But  that
> may change. And I would keep
> > the check against the same return macro that the function does.
>
> I cannot imagine it changing.
>
> >
> >
> >>
> >> > +               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.
> >>
> > In the header file of this function is an explanation. It's above in
> this patch:
> > + * 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
> > + */
>
> OK, so what I am getting at is trying to understand the error
> conversion. It seems that lwip uses different numbering from
> U-Boot/Linux, so we need to do a conversion somewhere?
>
> Regards,
> Simon
>

Yes, that is exactly what happens here. Lwip  ERR_INPROGRESS(-5) might be
U-boot:
#define EINPROGRESS     115     /* Operation now in progress */

I can do a conversion here with some comments to make it more clear.
diff mbox series

Patch

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[]);
+
+/**
+ * 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.uvarov@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);
+
+	dns_init();
+	dns_setserver(0, &dns1);
+	dns_setserver(1, &dns2);
+
+	err = dns_gethostbyname(name, &ipaddr, dns_found_cb, varname);
+	if (err == ERR_OK)
+		dns_found_cb(name, &ipaddr, varname);
+
+	return err;
+}