Hi Heinrich,
On 6/2/25 23:23, Heinrich Schuchardt wrote:
> Am 21. Mai 2025 17:14:42 MESZ schrieb Jerome Forissier
> <jerome.foriss...@linaro.org>:
>> Implement the sntp command when NET_LWIP=y.
>>
>> Signed-off-by: Jerome Forissier <jerome.foriss...@linaro.org>
>> ---
>>
>> cmd/Kconfig | 13 ++--
>> cmd/net-lwip.c | 5 ++
>> include/net-common.h | 11 ++++
>> lib/lwip/Makefile | 1 +
>> lib/lwip/u-boot/arch/cc.h | 4 ++
>> lib/lwip/u-boot/lwipopts.h | 4 ++
>> net/lwip/Makefile | 1 +
>> net/lwip/sntp.c | 128 +++++++++++++++++++++++++++++++++++++
>> 8 files changed, 161 insertions(+), 6 deletions(-)
>> create mode 100644 net/lwip/sntp.c
>>
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index f21d27cb27f..58f629e1c34 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -2061,12 +2061,6 @@ config CMD_CDP
>> and to retrieve configuration data including the VLAN id using the
>> proprietary Cisco Discovery Protocol (CDP).
>>
>> -config CMD_SNTP
>> - bool "sntp"
>> - select PROT_UDP
>> - help
>> - Synchronize RTC via network
>> -
>> config CMD_LINK_LOCAL
>> bool "linklocal"
>> depends on (LIB_RAND || LIB_HW_RAND)
>> @@ -2144,6 +2138,13 @@ config CMD_PING
>> help
>> Send ICMP ECHO_REQUEST to network host
>>
>> +config CMD_SNTP
>
> The command should depend on RTC support.
>
>> + bool "sntp"
>> + select PROT_UDP if NET
>> + select PROT_UDP_LWIP if NET_LWIP
>> + help
>> + Synchronize RTC via network
>> +
>> config CMD_TFTPBOOT
>> bool "tftp"
>> select PROT_UDP_LWIP if NET_LWIP
>> diff --git a/cmd/net-lwip.c b/cmd/net-lwip.c
>> index cecf8d02555..2ffee64f97e 100644
>> --- a/cmd/net-lwip.c
>> +++ b/cmd/net-lwip.c
>
> Please, put every command into a separate file instead of duplicating the
> #ifdef chaos of the old network stack.
While I agree that cmd/net.c is painful re. #ifdef's, I believe
cmd/net-lwip.c is not bad at all. In fact I took care of having only the
command wrappers in there (U_BOOT_CMD(...)), so it's just 55 lines at
the moment. The implementations are in
net/lwip/{dhcp,dns,ping,sntp,tftp,wget}.c. So I don't think there is
a need to change that. Or I could move the U_BOOT_CMD(...) into
net/lwip/*.c as well, leaving only the non-net commands under cmd/ (apart
from the legacy NET stuff).
Any opinions on that?
>
>> @@ -21,6 +21,11 @@ U_BOOT_CMD(tftpboot, 3, 0, do_tftpb,
>> "[loadAddress] [[hostIPaddr:]bootfilename]");
>> #endif
>>
>> +#if defined(CONFIG_CMD_SNTP)
>> +U_BOOT_CMD(sntp, 2, 1, do_sntp, "synchronize RTC via network",
>> + "[NTP server IP]");
>> +#endif
>
> Why wouldn't we support server names ( e.g. 0.de.pool.ntp.org) here if we
> have DNS enabled?
Good idea. I will add support for server names to v2.
>
> Please, add the missing documentation change for the commands.
Sure.
> Best regards
>
> Heinrich
>
>
>> +
>> #if defined(CONFIG_CMD_DNS)
>> U_BOOT_CMD(dns, 3, 1, do_dns, "lookup the IP of a hostname",
>> "hostname [envvar]");
>> diff --git a/include/net-common.h b/include/net-common.h
>> index a021bf503ff..b06cafb497e 100644
>> --- a/include/net-common.h
>> +++ b/include/net-common.h
>> @@ -505,6 +505,17 @@ int dhcp_run(ulong addr, const char *fname, bool
>> autoload);
>> */
>> int do_ping(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
>>
>> +/**
>> + * do_sntp - Run the sntp command
>> + *
>> + * @cmdtp: Unused
>> + * @flag: Command flags (CMD_FLAG_...)
>> + * @argc: Number of arguments
>> + * @argv: List of arguments
>> + * Return: result (see enum command_ret_t)
>> + */
>> +int do_sntp(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
>> +
>> /**
>> * do_tftpb - Run the tftpboot command
>> *
>> diff --git a/lib/lwip/Makefile b/lib/lwip/Makefile
>> index e9e8caee93a..c3f0e916f66 100644
>> --- a/lib/lwip/Makefile
>> +++ b/lib/lwip/Makefile
>> @@ -13,6 +13,7 @@ obj-y += \
>> lwip/src/api/sockets.o \
>> lwip/src/api/tcpip.o \
>> lwip/src/apps/http/http_client.o \
>> + lwip/src/apps/sntp/sntp.o \
>> lwip/src/apps/tftp/tftp.o \
>> lwip/src/core/altcp_alloc.o \
>> lwip/src/core/altcp.o \
>> diff --git a/lib/lwip/u-boot/arch/cc.h b/lib/lwip/u-boot/arch/cc.h
>> index 6104c296f6f..f91127ac565 100644
>> --- a/lib/lwip/u-boot/arch/cc.h
>> +++ b/lib/lwip/u-boot/arch/cc.h
>> @@ -43,4 +43,8 @@
>> #define BYTE_ORDER BIG_ENDIAN
>> #endif
>>
>> +#define SNTP_STARTUP_DELAY 0
>> +void sntp_set_system_time(uint32_t sec);
>> +#define SNTP_SET_SYSTEM_TIME(sec) sntp_set_system_time(sec)
>> +
>> #endif /* LWIP_ARCH_CC_H */
>> diff --git a/lib/lwip/u-boot/lwipopts.h b/lib/lwip/u-boot/lwipopts.h
>> index edac74ff7a2..14ba7bd7ae2 100644
>> --- a/lib/lwip/u-boot/lwipopts.h
>> +++ b/lib/lwip/u-boot/lwipopts.h
>> @@ -162,4 +162,8 @@
>> #define LWIP_ALTCP_TLS_MBEDTLS 1
>> #endif
>>
>> +#if defined(CONFIG_CMD_SNTP)
>> +#define LWIP_DHCP_GET_NTP_SRV 1
>> +#endif
>> +
>> #endif /* LWIP_UBOOT_LWIPOPTS_H */
>> diff --git a/net/lwip/Makefile b/net/lwip/Makefile
>> index 5df222589b8..5bb98dc4d98 100644
>> --- a/net/lwip/Makefile
>> +++ b/net/lwip/Makefile
>> @@ -4,6 +4,7 @@ obj-$(CONFIG_$(PHASE_)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_SNTP) += sntp.o
>> obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
>> obj-$(CONFIG_WGET) += wget.o
>>
>> diff --git a/net/lwip/sntp.c b/net/lwip/sntp.c
>> new file mode 100644
>> index 00000000000..3b2bc3c679d
>> --- /dev/null
>> +++ b/net/lwip/sntp.c
>> @@ -0,0 +1,128 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/* Copyright (C) 2025 Linaro Ltd. */
>> +
>> +#include <command.h>
>> +#include <console.h>
>> +#include <dm/device.h>
>> +#include <lwip/apps/sntp.h>
>> +#include <lwip/timeouts.h>
>> +#include <net.h>
>> +
>> +#define SNTP_TIMEOUT 10000
>> +
>> +static enum done_state {
>> + NOT_DONE = 0,
>> + SUCCESS,
>> + ABORTED,
>> + TIMED_OUT
>> +} sntp_state;
>> +
>> +static void no_response(void *arg)
>> +{
>> + sntp_state = TIMED_OUT;
>> +}
>> +
>> +/* Called by lwIP via the SNTP_SET_SYSTEM_TIME() macro */
>> +void sntp_set_system_time(uint32_t seconds)
>> +{
>> + char *toff;
>> + int net_ntp_time_offset = 0;
>> +
>> + toff = env_get("timeoffset");
>> + if (toff)
>> + net_ntp_time_offset = simple_strtol(toff, NULL, 10);
>> +
>> + net_sntp_set_rtc(seconds + net_ntp_time_offset);
>> + sntp_state = SUCCESS;
>> +}
>> +
>> +static bool ntp_server_known(void)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < SNTP_MAX_SERVERS; i++) {
>> + const ip_addr_t *ip = sntp_getserver(i);
>> +
>> + if (ip && ip->addr)
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static int sntp_loop(struct udevice *udev, ip_addr_t *srvip)
>> +{
>> + struct netif *netif;
>> +
>> + netif = net_lwip_new_netif(udev);
>> + if (!netif)
>> + return -1;
>> +
>> + sntp_state = NOT_DONE;
>> +
>> + sntp_setoperatingmode(SNTP_OPMODE_POLL);
>> + sntp_servermode_dhcp(CONFIG_IS_ENABLED(CMD_DHCP));
>> + if (srvip) {
>> + sntp_setserver(0, srvip);
>> + } else {
>> + if (!ntp_server_known()) {
>> + log_err("error: ntpserverip not set\n");
>> + return -1;
>> + }
>> + }
>> + sntp_init();
>> +
>> + sys_timeout(SNTP_TIMEOUT, no_response, NULL);
>> + while (sntp_state == NOT_DONE) {
>> + net_lwip_rx(udev, netif);
>> + sys_check_timeouts();
>> + if (ctrlc()) {
>> + printf("\nAbort\n");
>> + sntp_state = ABORTED;
>> + break;
>> + }
>> + }
>> + sys_untimeout(no_response, NULL);
>> +
>> + sntp_stop();
>> + net_lwip_remove_netif(netif);
>> +
>> + if (sntp_state == SUCCESS)
>> + return 0;
>> +
>> + return -1;
>> +}
>> +
>> +int do_sntp(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>> +{
>> + ip_addr_t *srvip = NULL;
>> + char *server_ip = NULL;
>> + ip_addr_t ipaddr;
>> +
>> + switch (argc) {
>> + case 1:
>> + server_ip = env_get("ntpserverip");
>> + break;
>> + case 2:
>> + server_ip = argv[1];
>> + break;
>> + default:
>> + return CMD_RET_USAGE;
>> + }
>> +
>> + if (server_ip) {
>> + if (!ipaddr_aton(server_ip, &ipaddr)) {
>> + log_err("error: ipaddr_aton\n");
>> + return CMD_RET_FAILURE;
>> + }
>> + srvip = &ipaddr;
>> + }
>> +
>> + if (net_lwip_eth_start() < 0)
>> + return CMD_RET_FAILURE;
>> +
>> + if (sntp_loop(eth_get_dev(), srvip) < 0)
>> + return CMD_RET_FAILURE;
>> +
>> + return CMD_RET_SUCCESS;
>> +}
>
