diff mbox series

[v2,07/14] net-lwip: add dns command

Message ID b77a239e1fb9a81744d1da9a000979aac298fd29.1716566960.git.jerome.forissier@linaro.org
State Superseded
Headers show
Series Introduce the lwIP network stack | expand

Commit Message

Jerome Forissier May 24, 2024, 4:20 p.m. UTC 
Add CONFIG_CMD_DNS_LWIP depending on CONFIG_NET_LWIP to provide the
dns command using lwIP.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 cmd/Kconfig        |   6 +++
 cmd/net-lwip.c     |   8 ++++
 include/net-lwip.h |   1 +
 net-lwip/Makefile  |   1 +
 net-lwip/dns.c     | 107 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 123 insertions(+)
 create mode 100644 net-lwip/dns.c

Comments

Ilias Apalodimas June 6, 2024, 6:29 a.m. UTC | #1 
Hi Jerome,

[...]

> +
> +static ulong start;
> +static ip_addr_t host_ipaddr;
> +static bool done;
> +
> +static void do_dns_tmr(void *arg)
> +{
> +     dns_tmr();
> +}
> +
> +static void dns_cb(const char *name, const ip_addr_t *ipaddr, void *arg)
> +{
> +     char *var = (char *)arg;

const char *


> +     char *ipstr = ip4addr_ntoa(ipaddr);
> +
> +     done = true;
> +
> +     if (!ipaddr) {
> +             printf("DNS: host not found\n");
> +             host_ipaddr.addr = 0;
> +             return;
> +     }
> +
> +     if (var)
> +             env_set(var, ipstr);
> +
> +     printf("%s\n", ipstr);
> +}
> +
> +int do_dns(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> +{
> +     bool has_server = false;
> +     ip_addr_t ipaddr;
> +     ip_addr_t ns;
> +     char *nsenv;
> +     char *name;
> +     char *var;
> +     int ret;
> +
> +     if (argc == 1 || argc > 3)
> +             return CMD_RET_USAGE;
> +
> +     if (argc >= 2)
> +             name = argv[1];
> +
> +     if (argc == 3)
> +             var = argv[2];
> +
> +     dns_init();
> +
> +     nsenv = env_get("dnsip");
> +     if (nsenv && ipaddr_aton(nsenv, &ns)) {
> +             dns_setserver(0, &ns);
> +             has_server = true;
> +     }
> +
> +     nsenv = env_get("dnsip2");
> +     if (nsenv && ipaddr_aton(nsenv, &ns)) {
> +             dns_setserver(1, &ns);
> +             has_server = true;
> +     }
> +
> +     if (!has_server) {
> +             log_err("No valid name server (dnsip/dnsip2)\n");
> +             return CMD_RET_FAILURE;
> +     }
> +
> +     done = false;
> +
> +     ret = dns_gethostbyname(name, &ipaddr, dns_cb, var);
> +
> +     if (ret == ERR_OK) {
> +             dns_cb(name, &ipaddr, var);
> +     } else if (ret == ERR_INPROGRESS) {
> +             start = get_timer(0);
> +             sys_timeout(DNS_RESEND_MS, do_dns_tmr, NULL);
> +             do {
> +                     eth_rx();
> +                     if (done)
> +                             break;
> +                     sys_check_timeouts();
> +                     if (ctrlc()) {
> +                             printf("\nAbort\n");
> +                             break;
> +                     }
> +             } while (get_timer(start) < DNS_TIMEOUT_MS);
> +             sys_untimeout(do_dns_tmr, NULL);
> +     }

Looking at the lwip code there are other ret values than just ERR_OK,
ERR_INPROGRESS. Should we have an 'else' handling the rest?

> +
> +     if (done && host_ipaddr.addr != 0)
> +             return CMD_RET_SUCCESS;
> +
> +     return CMD_RET_FAILURE;
> +}
> +
> --
> 2.40.1
>

Thanks
/Ilias
Maxim Uvarov June 6, 2024, 8:51 a.m. UTC | #2 
чт, 6 июн. 2024 г. в 09:29, Ilias Apalodimas <ilias.apalodimas@linaro.org>:
>
> Hi Jerome,
>
> [...]
>
> > +
> > +static ulong start;
> > +static ip_addr_t host_ipaddr;
> > +static bool done;
> > +
> > +static void do_dns_tmr(void *arg)
> > +{
> > +     dns_tmr();
> > +}
> > +
> > +static void dns_cb(const char *name, const ip_addr_t *ipaddr, void *arg)
> > +{
> > +     char *var = (char *)arg;
>
> const char *
>
>
> > +     char *ipstr = ip4addr_ntoa(ipaddr);
> > +
> > +     done = true;
> > +
> > +     if (!ipaddr) {
> > +             printf("DNS: host not found\n");
> > +             host_ipaddr.addr = 0;
> > +             return;
> > +     }
> > +
> > +     if (var)
> > +             env_set(var, ipstr);
> > +
> > +     printf("%s\n", ipstr);
> > +}
> > +
> > +int do_dns(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> > +{
> > +     bool has_server = false;
> > +     ip_addr_t ipaddr;
> > +     ip_addr_t ns;
> > +     char *nsenv;
> > +     char *name;
> > +     char *var;
> > +     int ret;
> > +
> > +     if (argc == 1 || argc > 3)
> > +             return CMD_RET_USAGE;
> > +
> > +     if (argc >= 2)
> > +             name = argv[1];
> > +
> > +     if (argc == 3)
> > +             var = argv[2];
> > +
> > +     dns_init();
> > +
> > +     nsenv = env_get("dnsip");
> > +     if (nsenv && ipaddr_aton(nsenv, &ns)) {
> > +             dns_setserver(0, &ns);
> > +             has_server = true;
> > +     }
> > +
> > +     nsenv = env_get("dnsip2");
> > +     if (nsenv && ipaddr_aton(nsenv, &ns)) {
> > +             dns_setserver(1, &ns);
> > +             has_server = true;
> > +     }
> > +
> > +     if (!has_server) {
> > +             log_err("No valid name server (dnsip/dnsip2)\n");
> > +             return CMD_RET_FAILURE;
> > +     }
> > +
> > +     done = false;
> > +
> > +     ret = dns_gethostbyname(name, &ipaddr, dns_cb, var);
> > +
> > +     if (ret == ERR_OK) {
> > +             dns_cb(name, &ipaddr, var);
> > +     } else if (ret == ERR_INPROGRESS) {
> > +             start = get_timer(0);
> > +             sys_timeout(DNS_RESEND_MS, do_dns_tmr, NULL);
> > +             do {
> > +                     eth_rx();
> > +                     if (done)
> > +                             break;
> > +                     sys_check_timeouts();
> > +                     if (ctrlc()) {
> > +                             printf("\nAbort\n");
> > +                             break;
> > +                     }
> > +             } while (get_timer(start) < DNS_TIMEOUT_MS);
> > +             sys_untimeout(do_dns_tmr, NULL);
> > +     }
>
> Looking at the lwip code there are other ret values than just ERR_OK,
> ERR_INPROGRESS. Should we have an 'else' handling the rest?
>
+1
And check the return code of eth_rx(), that is int function
and current code for some reason skips this check.

BR,
Maxim.

> > +
> > +     if (done && host_ipaddr.addr != 0)
> > +             return CMD_RET_SUCCESS;
> > +
> > +     return CMD_RET_FAILURE;
> > +}
> > +
> > --
> > 2.40.1
> >
>
> Thanks
> /Ilias
Jerome Forissier June 6, 2024, 12:19 p.m. UTC | #3 
On 6/6/24 08:29, Ilias Apalodimas wrote:
> Hi Jerome,
> 
> [...]
> 
>> +
>> +static ulong start;
>> +static ip_addr_t host_ipaddr;
>> +static bool done;
>> +
>> +static void do_dns_tmr(void *arg)
>> +{
>> +     dns_tmr();
>> +}
>> +
>> +static void dns_cb(const char *name, const ip_addr_t *ipaddr, void *arg)
>> +{
>> +     char *var = (char *)arg;
> 
> const char *

Fixed in v3.

> 
>> +     char *ipstr = ip4addr_ntoa(ipaddr);
>> +
>> +     done = true;
>> +
>> +     if (!ipaddr) {
>> +             printf("DNS: host not found\n");
>> +             host_ipaddr.addr = 0;
>> +             return;
>> +     }
>> +
>> +     if (var)
>> +             env_set(var, ipstr);
>> +
>> +     printf("%s\n", ipstr);
>> +}
>> +
>> +int do_dns(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>> +{
>> +     bool has_server = false;
>> +     ip_addr_t ipaddr;
>> +     ip_addr_t ns;
>> +     char *nsenv;
>> +     char *name;
>> +     char *var;
>> +     int ret;
>> +
>> +     if (argc == 1 || argc > 3)
>> +             return CMD_RET_USAGE;
>> +
>> +     if (argc >= 2)
>> +             name = argv[1];
>> +
>> +     if (argc == 3)
>> +             var = argv[2];
>> +
>> +     dns_init();
>> +
>> +     nsenv = env_get("dnsip");
>> +     if (nsenv && ipaddr_aton(nsenv, &ns)) {
>> +             dns_setserver(0, &ns);
>> +             has_server = true;
>> +     }
>> +
>> +     nsenv = env_get("dnsip2");
>> +     if (nsenv && ipaddr_aton(nsenv, &ns)) {
>> +             dns_setserver(1, &ns);
>> +             has_server = true;
>> +     }
>> +
>> +     if (!has_server) {
>> +             log_err("No valid name server (dnsip/dnsip2)\n");
>> +             return CMD_RET_FAILURE;
>> +     }
>> +
>> +     done = false;
>> +
>> +     ret = dns_gethostbyname(name, &ipaddr, dns_cb, var);
>> +
>> +     if (ret == ERR_OK) {
>> +             dns_cb(name, &ipaddr, var);
>> +     } else if (ret == ERR_INPROGRESS) {
>> +             start = get_timer(0);
>> +             sys_timeout(DNS_RESEND_MS, do_dns_tmr, NULL);
>> +             do {
>> +                     eth_rx();
>> +                     if (done)
>> +                             break;
>> +                     sys_check_timeouts();
>> +                     if (ctrlc()) {
>> +                             printf("\nAbort\n");
>> +                             break;
>> +                     }
>> +             } while (get_timer(start) < DNS_TIMEOUT_MS);
>> +             sys_untimeout(do_dns_tmr, NULL);
>> +     }
> 
> Looking at the lwip code there are other ret values than just ERR_OK,
> ERR_INPROGRESS. Should we have an 'else' handling the rest?

Not needed, since in this case we will reach the next line
'if (done && host_ipaddr.addr != 0)' which is the true success condition.

Thanks,
diff mbox series

Patch

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 07cfe824e3f..6ef0b52cd34 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2098,6 +2098,12 @@  config CMD_DHCP
 	help
 	  Boot image via network using DHCP/TFTP protocol
 
+config CMD_DNS
+	bool "dns"
+	select PROT_DNS_LWIP
+	help
+	  Lookup the IP of a hostname
+
 config CMD_PING
 	bool "ping"
 	select PROT_RAW_LWIP
diff --git a/cmd/net-lwip.c b/cmd/net-lwip.c
index 13856703fcf..3abafdf7969 100644
--- a/cmd/net-lwip.c
+++ b/cmd/net-lwip.c
@@ -27,3 +27,11 @@  U_BOOT_CMD(
 	"[loadAddress] [[hostIPaddr:]bootfilename]"
 );
 #endif
+
+#if defined(CONFIG_CMD_DNS)
+U_BOOT_CMD(
+	dns,	3,	1,	do_dns,
+	"lookup the IP of a hostname",
+	"hostname [envvar]"
+);
+#endif
diff --git a/include/net-lwip.h b/include/net-lwip.h
index 2abaaa3b4e3..0019d1524e5 100644
--- a/include/net-lwip.h
+++ b/include/net-lwip.h
@@ -82,6 +82,7 @@  int net_lwip_init(void);
 struct netif *net_lwip_get_netif(void);
 
 int do_dhcp(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
+int do_dns(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_ping(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 
diff --git a/net-lwip/Makefile b/net-lwip/Makefile
index e68d4e24197..aa247859483 100644
--- a/net-lwip/Makefile
+++ b/net-lwip/Makefile
@@ -7,6 +7,7 @@  obj-$(CONFIG_$(SPL_)DM_ETH) += ../net/eth_common.o
 obj-$(CONFIG_$(SPL_)DM_ETH) += ../net/eth-uclass.o
 obj-$(CONFIG_$(SPL_)DM_ETH) += net-lwip.o
 obj-$(CONFIG_CMD_DHCP) += dhcp.o
+obj-$(CONFIG_CMD_DNS) += dns.o
 obj-$(CONFIG_CMD_PING) += ping.o
 obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
 
diff --git a/net-lwip/dns.c b/net-lwip/dns.c
new file mode 100644
index 00000000000..6ef9d0b510b
--- /dev/null
+++ b/net-lwip/dns.c
@@ -0,0 +1,107 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2024 Linaro Ltd. */
+
+#include <command.h>
+#include <console.h>
+#include <lwip/dns.h>
+#include <lwip/timeouts.h>
+#include <net-lwip.h>
+#include <time.h>
+
+#define DNS_RESEND_MS 1000
+#define DNS_TIMEOUT_MS 10000
+
+static ulong start;
+static ip_addr_t host_ipaddr;
+static bool done;
+
+static void do_dns_tmr(void *arg)
+{
+	dns_tmr();
+}
+
+static void dns_cb(const char *name, const ip_addr_t *ipaddr, void *arg)
+{
+	char *var = (char *)arg;
+	char *ipstr = ip4addr_ntoa(ipaddr);
+
+	done = true;
+
+	if (!ipaddr) {
+		printf("DNS: host not found\n");
+		host_ipaddr.addr = 0;
+		return;
+	}
+
+	if (var)
+		env_set(var, ipstr);
+
+	printf("%s\n", ipstr);
+}
+
+int do_dns(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
+{
+	bool has_server = false;
+	ip_addr_t ipaddr;
+	ip_addr_t ns;
+	char *nsenv;
+	char *name;
+	char *var;
+	int ret;
+
+	if (argc == 1 || argc > 3)
+		return CMD_RET_USAGE;
+
+	if (argc >= 2)
+		name = argv[1];
+
+	if (argc == 3)
+		var = argv[2];
+
+	dns_init();
+
+	nsenv = env_get("dnsip");
+	if (nsenv && ipaddr_aton(nsenv, &ns)) {
+		dns_setserver(0, &ns);
+		has_server = true;
+	}
+
+	nsenv = env_get("dnsip2");
+	if (nsenv && ipaddr_aton(nsenv, &ns)) {
+		dns_setserver(1, &ns);
+		has_server = true;
+	}
+
+	if (!has_server) {
+		log_err("No valid name server (dnsip/dnsip2)\n");
+		return CMD_RET_FAILURE;
+	}
+
+	done = false;
+
+	ret = dns_gethostbyname(name, &ipaddr, dns_cb, var);
+
+	if (ret == ERR_OK) {
+		dns_cb(name, &ipaddr, var);
+	} else if (ret == ERR_INPROGRESS) {
+		start = get_timer(0);
+		sys_timeout(DNS_RESEND_MS, do_dns_tmr, NULL);
+		do {
+			eth_rx();
+			if (done)
+				break;
+			sys_check_timeouts();
+			if (ctrlc()) {
+				printf("\nAbort\n");
+				break;
+			}
+		} while (get_timer(start) < DNS_TIMEOUT_MS);
+		sys_untimeout(do_dns_tmr, NULL);
+	}
+
+	if (done && host_ipaddr.addr != 0)
+		return CMD_RET_SUCCESS;
+
+	return CMD_RET_FAILURE;
+}
+