[PATCHv6 07/14] net/lwip: implement ping cmd
Simon Glass
sjg at google.com
Wed Aug 16 16:39:25 CEST 2023
Hi Maxim,
On Wed, 16 Aug 2023 at 03:09, Maxim Uvarov <maxim.uvarov at linaro.org> wrote:
>
> On Wed, 16 Aug 2023 at 14:42, Ilias Apalodimas <ilias.apalodimas at linaro.org>
> wrote:
>
> > On Mon, Aug 14, 2023 at 07:32:46PM +0600, Maxim Uvarov wrote:
> > > * can return immediately if previous request was cached or it might
> > require
> > > @@ -38,3 +39,28 @@ int ulwip_dhcp(void);
> > > * !0 if error
> > > */
> > > int ulwip_wget(ulong addr, char *url);
> > > +
> > > +/**
> > > + * ulwip_tftp() - load file with tftp
> > > + *
> > > + * Load file with tftp to specific address
> > > + *
> > > + * @param addr - address to store downloaded file
> > > + * @param filename - file name on remote tftp server to download
> >
> > Please fix function comments properly
> >
> > > + *
> > > + *
> > > + * @return 0 if success, !0 if error
> > > + */
> > > +int ulwip_tftp(ulong addr, const char *filename);
> > > +
> > > +/*
> > > +* This function creates the ping for address provided in parameters.
> > > +* After this function you need to invoke the polling
> > > +* loop to process network communication.
> > > +*
> > > +*
> > > +* @ping_addr start address to download result
> > > +* Return: 0 for success
> > > +* !0 if error
> > > +*/
> > > +int ulwip_ping(char *ping_addr);
> > > diff --git a/net/lwip/Makefile b/net/lwip/Makefile
> > > index 4c6df94807..8b3e843426 100644
> > > --- a/net/lwip/Makefile
> > > +++ b/net/lwip/Makefile
> > > @@ -67,5 +67,6 @@ obj-$(CONFIG_NET) += port/sys-arch.o
> > >
> > > obj-$(CONFIG_CMD_DHCP) += apps/dhcp/lwip-dhcp.o
> > > obj-$(CONFIG_CMD_DNS) += apps/dns/lwip-dns.o
> > > +obj-$(CONFIG_CMD_PING) += apps/ping/
> > > obj-$(CONFIG_CMD_TFTPBOOT) += apps/tftp/
> > > obj-$(CONFIG_CMD_WGET) += apps/http/
> > > diff --git a/net/lwip/apps/ping/Makefile b/net/lwip/apps/ping/Makefile
> > > new file mode 100644
> > > index 0000000000..dc63feb7b5
> > > --- /dev/null
> > > +++ b/net/lwip/apps/ping/Makefile
> > > @@ -0,0 +1,11 @@
> > > +ccflags-y += -I$(srctree)/net/lwip/port/include
> > > +ccflags-y += -I$(srctree)/net/lwip/lwip-external/src/include
> > -I$(srctree)/net/lwip
> > > +ccflags-y += -I$(obj)
> > > +
> > > +.PHONY: $(obj)/ping.c
> > > +$(obj)/ping.o: $(obj)/ping.c
> > > +$(obj)/ping.c:
> > > + cp $(srctree)/net/lwip/lwip-external/contrib/apps/ping/ping.c
> > $(obj)/ping.c
> > > +
> > > +obj-$(CONFIG_CMD_PING) += ping.o
> > > +obj-$(CONFIG_CMD_PING) += lwip_ping.o
> > > diff --git a/net/lwip/apps/ping/lwip_ping.c
> > b/net/lwip/apps/ping/lwip_ping.c
> > > new file mode 100644
> > > index 0000000000..611fcaf591
> > > --- /dev/null
> > > +++ b/net/lwip/apps/ping/lwip_ping.c
> > > @@ -0,0 +1,37 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +/*
> > > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov at linaro.org>
> > > + */
> > > +
> > > +#include "lwip/opt.h"
> > > +#include "lwip/ip_addr.h"
> > > +#include "ping.h"
> > > +#include "lwip_ping.h"
> > > +
> > > +static ip_addr_t ip_target;
> > > +
> > > +static int ulwip_ping_tmo(void)
> > > +{
> > > +
> > > + log_err("ping failed; host %s is not alive\n",
> > ipaddr_ntoa(&ip_target));
> > > + return 1;
> > > +}
> > > +
> > > +int ulwip_ping(char *ping_addr)
> > > +{
> > > + int err;
> > > +
> > > + err = ipaddr_aton(ping_addr, &ip_target);
> > > + if (!err) {
> > > + log_err("wrong ping addr string \"%s\" \n", ping_addr);
> >
> > Invalid ip address is enough
> >
> > > + return -1;
> > > + }
> > > +
> > > + ulwip_set_tmo(ulwip_ping_tmo);
> > > +
> > > + ping_init(&ip_target);
> > > + ping_send_now();
> > > +
> > > + return 0;
> > > +}
> > > diff --git a/net/lwip/apps/ping/lwip_ping.h
> > b/net/lwip/apps/ping/lwip_ping.h
> > > new file mode 100644
> > > index 0000000000..0374f07d9e
> > > --- /dev/null
> > > +++ b/net/lwip/apps/ping/lwip_ping.h
> > > @@ -0,0 +1,15 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +
> > > +/*
> > > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov at linaro.org>
> > > + */
> > > +
> > > +#ifndef LWIP_PING_H
> > > +#define LWIP_PING_H
> > > +
> > > +#include <lwip/ip_addr.h>
> > > +
> > > +void ping_raw_init(void);
> > > +void ping_send_now(void);
> > > +
> > > +#endif /* LWIP_PING_H */
> > > diff --git a/net/lwip/apps/ping/ping.h b/net/lwip/apps/ping/ping.h
> > > new file mode 100644
> > > index 0000000000..006a18c658
> > > --- /dev/null
> > > +++ b/net/lwip/apps/ping/ping.h
> > > @@ -0,0 +1,19 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +
> > > +#include <net/ulwip.h>
> > > +#include "lwip/ip_addr.h"
> > > +
> > > +#define LWIP_DEBUG 1 /* ping_time is under ifdef*/
> > > +#define PING_RESULT(cond) { \
> > > + if (cond == 1) { \
> > > + printf("host %s a alive\n", ipaddr_ntoa(addr)); \
> > > + printf(" %"U32_F" ms\n", (sys_now() - ping_time)); \
> > > + ulwip_exit(0); \
> > > + } else { \
> > > + printf("ping failed; host %s in not alive\n",\
> > > + ipaddr_ntoa(addr)); \
> > > + ulwip_exit(-1); \
> > > + } \
> > > + } while (0);
> >
> > On the previous patch you are defining a function to do something similar
> > (httpc_result()). We need to be consistent on this. Can we define a
> > common function for all failures? Certianly don't define a macro here and
> > a function elsewhere
> >
>
> Ilias, there I reuse lwip example ping.c. It has PING_RESULT which can be
> redefined by the application.
> This is a way to not modify this original ping.c example code.
> The common part of this function and httpc_result() is ulwip_exit(err) with
> some additional print (print is differ).
I'll reply on this patch, although it is a general comment.
I would like to see if we can use the cyclic feature to allow network
operations to happen in the background. This would involve splitting
each operation into:
- setup start and kick off, e.g. ping_sttart()
- checking what needs doing, e.g. ping_poll()
- finish and clean-up, e.g. ping_finish()
So something like:
ping -B 1.2.3.4
(-B is background)
That would fire off the packet but immediately return with a prompt.
When the pings are received they would show on the console, with a new
prompt displayed. The same for tftp -B...it would start the transfer
but allow other commands to be sent while it is in progress.
Part of the reason for this is that when booting there are a lot of
operations which take a long time and nothing happens until everything
is done. For example, with standard boot, mmc devices can be shown
right away, but USB and network ones take a lot longer, so having OS's
on those devices appear later on the slower devices is desirable for
the user.
Is this something that lwip can support? It is one reason why I have
advocating putting the state in a struct instead of having a lot of
separate vars.
Regards,
Simon
More information about the U-Boot
mailing list