Hi Maxim,
On Wed, 16 Aug 2023 at 03:09, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
> On Wed, 16 Aug 2023 at 14:42, Ilias Apalodimas <ilias.apalodimas@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@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@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
This functionality can be added. I do not see any restrictions implementing that.
Added I mean to U-Boot. lwip will definitely work with it.
There is no specific poll() for application. Application just do init() and request
the needed packets (port, type etc, like sockets) and then common lwip poll()
function is called.
To implement background we need to periodically call eth_rx() and ulwip_poll() to feed lwip
stack with rx packets when we are in the cmd shell. I see in some places we call schedule()
to reprogram wdt, if we call rx polling loop in the similar way the app will work in the background.
We might also want to support 2 applications at the same time. Like a telnet server
and ping. Multiply applications also work with lwip.
Even parallel download can save some time:
wget -B http//url2/dtb
(-B = background)
But I'm thinking if we can implement background in a more generic way in U-Boot cmd. Things like
mount UBI fs, or download from SPI flash is slow. Or probably we can also do some loading while
verifying the zImage checksum.
BR,
Maxim.