diff mbox series

[PATCHv6,03/14] net/lwip: implement dns cmd

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

Commit Message

Maxim Uvarov Aug. 14, 2023, 1:32 p.m. UTC
Implement function for dns command with lwIP variant. Usage and output is
the same as the original command. This code called by compatibility code
between U-Boot and lwIP.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 include/net/lwip.h           | 17 +++++++++++++
 net/lwip/Makefile            |  2 ++
 net/lwip/apps/dns/lwip-dns.c | 46 ++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+)
 create mode 100644 include/net/lwip.h
 create mode 100644 net/lwip/apps/dns/lwip-dns.c

Comments

Ilias Apalodimas Aug. 14, 2023, 2:19 p.m. UTC | #1
On Mon, Aug 14, 2023 at 07:32:42PM +0600, Maxim Uvarov wrote:
> Implement function for dns command with lwIP variant. Usage and output is
> the same as the original command. This code called by compatibility code
> between U-Boot and lwIP.

What's compatibility code?
I think something along the lines of 

'U-Boot recently got support for an alternative network stack using LWIP.  
Replace XXXX command with the LWIP variant while keeping the output and
error messages identical"

> +
> +int do_lwip_dns(struct cmd_tbl *cmdtp, int flag, int argc,
> +		char *const argv[]);
> +
> +/*
> +* This function creates the DNS request to resolve a domain host name. Function

You need the name of the function as well.  Please have a look at how the
rest of the code is documented. 

> +* 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

@name: etc

> +* @varname (optional) U-Boot variable name to store the result
> +* Return: 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
> + */
> +
> +#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));

Why do we have to set the varname to the resolved address?  Is this
something the dns command already does?

> +
> +	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;
> +}
> -- 
> 2.30.2
> 

Thanks
/Ilias
Maxim Uvarov Aug. 14, 2023, 3:15 p.m. UTC | #2
On Mon, 14 Aug 2023 at 20:19, Ilias Apalodimas <ilias.apalodimas@linaro.org>
wrote:

> On Mon, Aug 14, 2023 at 07:32:42PM +0600, Maxim Uvarov wrote:
> > Implement function for dns command with lwIP variant. Usage and output is
> > the same as the original command. This code called by compatibility code
> > between U-Boot and lwIP.
>
> What's compatibility code?
> I think something along the lines of
>
> 'U-Boot recently got support for an alternative network stack using LWIP.
> Replace XXXX command with the LWIP variant while keeping the output and
> error messages identical"
>

ok, that message is a good one. By 'compatibility code' I meant code to
merge lwip applications and other U-Boot code.

So in general all net/lwip/apps/{dns,ping,wget} and etc. These applications
do not have U-Boot headers and can be compiled with
any other lwIP port. I.e. for example, you can take the linux userland port
with a tap device and directly compile these apps, run and debug.
This might be named lwIP apps code.

And code which does lwIP init, calls application initialization, then goes
to net polling loop - I named here 'a compatibility code'. Might be not the
base naming, but that was an idea.



>
> > +
> > +int do_lwip_dns(struct cmd_tbl *cmdtp, int flag, int argc,
> > +             char *const argv[]);
> > +
> > +/*
> > +* This function creates the DNS request to resolve a domain host name.
> Function
>
> You need the name of the function as well.  Please have a look at how the
> rest of the code is documented.
>
> > +* 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
>
> @name: etc
>
> > +* @varname (optional) U-Boot variable name to store the result
> > +* Return: 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
> > + */
> > +
> > +#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));
>
> Why do we have to set the varname to the resolved address?  Is this
> something the dns command already does?
>
> Exactly, original  command is:

U_BOOT_CMD(
dns, 3, 1, do_dns,
"lookup the IP of a hostname",
"hostname [envvar]"
);



> > +
> > +     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;
> > +}
> > --
> > 2.30.2
> >
>
> Thanks
> /Ilias
>
Maxim Uvarov Aug. 15, 2023, 12:42 p.m. UTC | #3
On Mon, 14 Aug 2023 at 20:19, Ilias Apalodimas <ilias.apalodimas@linaro.org>
wrote:

[...]


> > +/*
> > +* This function creates the DNS request to resolve a domain host name.
> Function
>
> You need the name of the function as well.  Please have a look at how the
> rest of the code is documented.
>
> > +* 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
>
> @name: etc
>
> > +* @varname (optional) U-Boot variable name to store the result
> > +* Return: ERR_OK(0) for fetching entry from the cache
> > +*         ERR_INPROGRESS(-5) success, can go to the polling loop
> > +*         Other value < 0, if error
> > +*/
> >
>

Is there any command to check all the files for the comments style?

[...]
Tom Rini Aug. 15, 2023, 2:42 p.m. UTC | #4
On Tue, Aug 15, 2023 at 06:42:14PM +0600, Maxim Uvarov wrote:
> On Mon, 14 Aug 2023 at 20:19, Ilias Apalodimas <ilias.apalodimas@linaro.org>
> wrote:
> 
> [...]
> 
> 
> > > +/*
> > > +* This function creates the DNS request to resolve a domain host name.
> > Function
> >
> > You need the name of the function as well.  Please have a look at how the
> > rest of the code is documented.
> >
> > > +* 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
> >
> > @name: etc
> >
> > > +* @varname (optional) U-Boot variable name to store the result
> > > +* Return: ERR_OK(0) for fetching entry from the cache
> > > +*         ERR_INPROGRESS(-5) success, can go to the polling loop
> > > +*         Other value < 0, if error
> > > +*/
> > >
> >
> 
> Is there any command to check all the files for the comments style?

I suspect if you included them in docs, htmldocs/etc would note
problems, Heinrich?
Ilias Apalodimas Aug. 16, 2023, 10:26 a.m. UTC | #5
On Tue, 15 Aug 2023 at 17:42, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Aug 15, 2023 at 06:42:14PM +0600, Maxim Uvarov wrote:
> > On Mon, 14 Aug 2023 at 20:19, Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > wrote:
> >
> > [...]
> >
> >
> > > > +/*
> > > > +* This function creates the DNS request to resolve a domain host name.
> > > Function
> > >
> > > You need the name of the function as well.  Please have a look at how the
> > > rest of the code is documented.
> > >
> > > > +* 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
> > >
> > > @name: etc
> > >
> > > > +* @varname (optional) U-Boot variable name to store the result
> > > > +* Return: ERR_OK(0) for fetching entry from the cache
> > > > +*         ERR_INPROGRESS(-5) success, can go to the polling loop
> > > > +*         Other value < 0, if error
> > > > +*/
> > > >
> > >
> >
> > Is there any command to check all the files for the comments style?
>
> I suspect if you included them in docs, htmldocs/etc would note
> problems, Heinrich?
>

Yes 'make htmldocs' blows up on errors, at least for the EFI subsystem

Cheers
/Ilias
> --
> Tom
Maxim Uvarov Aug. 16, 2023, 11:01 a.m. UTC | #6
On Wed, 16 Aug 2023 at 16:26, Ilias Apalodimas <ilias.apalodimas@linaro.org>
wrote:

> On Tue, 15 Aug 2023 at 17:42, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Aug 15, 2023 at 06:42:14PM +0600, Maxim Uvarov wrote:
> > > On Mon, 14 Aug 2023 at 20:19, Ilias Apalodimas <
> ilias.apalodimas@linaro.org>
> > > wrote:
> > >
> > > [...]
> > >
> > >
> > > > > +/*
> > > > > +* This function creates the DNS request to resolve a domain host
> name.
> > > > Function
> > > >
> > > > You need the name of the function as well.  Please have a look at
> how the
> > > > rest of the code is documented.
> > > >
> > > > > +* 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
> > > >
> > > > @name: etc
> > > >
> > > > > +* @varname (optional) U-Boot variable name to store the result
> > > > > +* Return: ERR_OK(0) for fetching entry from the cache
> > > > > +*         ERR_INPROGRESS(-5) success, can go to the polling loop
> > > > > +*         Other value < 0, if error
> > > > > +*/
> > > > >
> > > >
> > >
> > > Is there any command to check all the files for the comments style?
> >
> > I suspect if you included them in docs, htmldocs/etc would note
> > problems, Heinrich?
> >
>
> Yes 'make htmldocs' blows up on errors, at least for the EFI subsystem
>
> Cheers
> /Ilias
>

O! Ilias thanks for the reference. First patch for doc/develop/net_lwip.rst
needed direct references:

.. kernel-doc:: include/net/lwip.h
   :internal:

to force Sphinx  to generate docs for these files. (as I understand for .c
files it does checks and for .h files it generates docs).

Without specification syntax is not validated. It will be good to do
something to check all U-Boot .h files for the common syntax later.

BR,
Maxim.




> > --
> > Tom
>
Ilias Apalodimas Aug. 16, 2023, 11:54 a.m. UTC | #7
On Wed, 16 Aug 2023 at 14:01, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
>
>
> On Wed, 16 Aug 2023 at 16:26, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
>>
>> On Tue, 15 Aug 2023 at 17:42, Tom Rini <trini@konsulko.com> wrote:
>> >
>> > On Tue, Aug 15, 2023 at 06:42:14PM +0600, Maxim Uvarov wrote:
>> > > On Mon, 14 Aug 2023 at 20:19, Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> > > wrote:
>> > >
>> > > [...]
>> > >
>> > >
>> > > > > +/*
>> > > > > +* This function creates the DNS request to resolve a domain host name.
>> > > > Function
>> > > >
>> > > > You need the name of the function as well.  Please have a look at how the
>> > > > rest of the code is documented.
>> > > >
>> > > > > +* 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
>> > > >
>> > > > @name: etc
>> > > >
>> > > > > +* @varname (optional) U-Boot variable name to store the result
>> > > > > +* Return: ERR_OK(0) for fetching entry from the cache
>> > > > > +*         ERR_INPROGRESS(-5) success, can go to the polling loop
>> > > > > +*         Other value < 0, if error
>> > > > > +*/
>> > > > >
>> > > >
>> > >
>> > > Is there any command to check all the files for the comments style?
>> >
>> > I suspect if you included them in docs, htmldocs/etc would note
>> > problems, Heinrich?
>> >
>>
>> Yes 'make htmldocs' blows up on errors, at least for the EFI subsystem
>>
>> Cheers
>> /Ilias
>
>
> O! Ilias thanks for the reference. First patch for doc/develop/net_lwip.rst needed direct references:
>
> .. kernel-doc:: include/net/lwip.h
>    :internal:
>
> to force Sphinx  to generate docs for these files. (as I understand for .c files it does checks and for .h files it generates docs).
>
> Without specification syntax is not validated. It will be good to do something to check all U-Boot .h files for the common syntax later.

You need comments that look like this
  /**
   * parse_event_log_header() -  Parse and verify the event log header
fields
   *
   * @buffer:                     Pointer to the start of the eventlog
   * @size:                       Size of the eventlog
   * @pos:                        Return offset of the next event in
buffer right
   *                              after the event header i.e specID
   *
   * Return:      status code
   */

Cheers
/Ilias
>
> BR,
> Maxim.
>
>
>
>>
>> > --
>> > Tom
Maxim Uvarov Aug. 16, 2023, 12:24 p.m. UTC | #8
On Wed, 16 Aug 2023 at 17:54, Ilias Apalodimas <ilias.apalodimas@linaro.org>
wrote:

> On Wed, 16 Aug 2023 at 14:01, Maxim Uvarov <maxim.uvarov@linaro.org>
> wrote:
> >
> >
> >
> > On Wed, 16 Aug 2023 at 16:26, Ilias Apalodimas <
> ilias.apalodimas@linaro.org> wrote:
> >>
> >> On Tue, 15 Aug 2023 at 17:42, Tom Rini <trini@konsulko.com> wrote:
> >> >
> >> > On Tue, Aug 15, 2023 at 06:42:14PM +0600, Maxim Uvarov wrote:
> >> > > On Mon, 14 Aug 2023 at 20:19, Ilias Apalodimas <
> ilias.apalodimas@linaro.org>
> >> > > wrote:
> >> > >
> >> > > [...]
> >> > >
> >> > >
> >> > > > > +/*
> >> > > > > +* This function creates the DNS request to resolve a domain
> host name.
> >> > > > Function
> >> > > >
> >> > > > You need the name of the function as well.  Please have a look at
> how the
> >> > > > rest of the code is documented.
> >> > > >
> >> > > > > +* 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
> >> > > >
> >> > > > @name: etc
> >> > > >
> >> > > > > +* @varname (optional) U-Boot variable name to store the result
> >> > > > > +* Return: ERR_OK(0) for fetching entry from the cache
> >> > > > > +*         ERR_INPROGRESS(-5) success, can go to the polling
> loop
> >> > > > > +*         Other value < 0, if error
> >> > > > > +*/
> >> > > > >
> >> > > >
> >> > >
> >> > > Is there any command to check all the files for the comments style?
> >> >
> >> > I suspect if you included them in docs, htmldocs/etc would note
> >> > problems, Heinrich?
> >> >
> >>
> >> Yes 'make htmldocs' blows up on errors, at least for the EFI subsystem
> >>
> >> Cheers
> >> /Ilias
> >
> >
> > O! Ilias thanks for the reference. First patch for
> doc/develop/net_lwip.rst needed direct references:
> >
> > .. kernel-doc:: include/net/lwip.h
> >    :internal:
> >
> > to force Sphinx  to generate docs for these files. (as I understand for
> .c files it does checks and for .h files it generates docs).
> >
> > Without specification syntax is not validated. It will be good to do
> something to check all U-Boot .h files for the common syntax later.
>
> You need comments that look like this
>   /**
>    * parse_event_log_header() -  Parse and verify the event log header
> fields
>    *
>    * @buffer:                     Pointer to the start of the eventlog
>    * @size:                       Size of the eventlog
>    * @pos:                        Return offset of the next event in
> buffer right
>    *                              after the event header i.e specID
>    *
>    * Return:      status code
>    */
>
> Cheers
> /Ilias
>


Yes, I added html generation for these files and now I can fix style.


> >
> > BR,
> > Maxim.
> >
> >
> >
> >>
> >> > --
> >> > Tom
>
diff mbox series

Patch

diff --git a/include/net/lwip.h b/include/net/lwip.h
new file mode 100644
index 0000000000..c83b5c8231
--- /dev/null
+++ b/include/net/lwip.h
@@ -0,0 +1,17 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+int do_lwip_dns(struct cmd_tbl *cmdtp, int flag, int argc,
+		char *const argv[]);
+
+/*
+* 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
+* Return: 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;
+}