| Message ID | 20230814133253.4150-4-maxim.uvarov@linaro.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Ramon Fried |
| Headers | show |
| Series | net/lwip: add lwip library for the network stack | expand |
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
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 >
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? [...]
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?
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
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 >
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
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 --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; +}
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