[PATCH v8 00/23] Introduce the lwIP network stack

[Jerome Forissier]

On 8/16/24 20:40, Tom Rini wrote:
> On Fri, Aug 16, 2024 at 06:21:24PM +0200, Jerome Forissier wrote:
>>
>>
>> On 8/7/24 22:44, Tom Rini wrote:
>>> On Wed, Aug 07, 2024 at 07:11:44PM +0200, Jerome Forissier wrote:
>>>
>>>> This is a rework of a patch series by Maxim Uvarov: "net/lwip: add lwip
>>>> library for the network stack" [1]. The goal is to introduce the lwIP TCP/IP
>>>> stack [2] [3] as an alternative to the current implementation in net/,
>>>> selectable with Kconfig, and ultimately keep only lwIP if possible. Some
>>>> reasons for doing so are:
>>>> - Make the support of HTTPS in the wget command easier. Javier T. and
>>>> Raymond M. (CC'd) have some additional lwIP and Mbed TLS patches to do
>>>> so. With that it becomes possible to fetch and launch a distro installer
>>>> such as Debian etc. using a secure, authenticated connection directly
>>>> from the U-Boot shell. Several use cases:
>>>>   * Authentication: prevent MITM attack (third party replacing the
>>>> binary with a different one)
>>>>   * Confidentiality: prevent third parties from grabbing a copy of the
>>>> image as it is being downloaded
>>>>   * Allow connection to servers that do not support plain HTTP anymore
>>>> (this is becoming more and more common on the Internet these days)
>>>> - Possibly benefit from additional features implemented in lwIP
>>>> - Less code to maintain in U-Boot
>>>>
>>>> Prior to applying this series, the lwIP stack needs to be added as a
>>>> Git subtree with the following command:
>>>>
>>>>  $  git subtree add --squash --prefix lib/lwip/lwip https://git.savannah.gnu.org/git/lwip.git STABLE-2_2_0_RELEASE
>>>
>>> For v9, I think it would be good to do a CI run with NET_LWIP default
>>> and seeing what fails from that too. There's a few problems still
>>> leading to a lot of failures, in that case. Thanks.
>>>
>>
>> See here: https://github.com/u-boot/u-boot/pull/635
>>
>> I fixed a number of issues, see the commit descriptions. As for the
>> remaining ones:
>> - There is no http server in the CI so the wget test I added fails
> 
> Ah yes. So the test needs some enable-me type flag, like other tests
> that require external configuration.

Can you please suggest how to do that?
 
>> - tftp is super slow in QEMU (~145 KiB/s) which causes timeouts. This is
>> for two reasons: (1) the tftp windowsize option is not supported in lwIP
>> (while the legacy NET does support it) so the sender waits for an ACK
>> before sending a new packet; and (2) the latency is very high due to
>> memcpy() being incredibly slow in QEMU (I am mainly referring to the
>> memcpy() call in tftp_write() in net/lwip/tftp.c). I measured ~20-60 ms
>> to copy a few hundred bytes (!) and if CONFIG_USE_ARCH_MEMCPY is enabled
>> it is slightly better but not much (~15 ms). Also it seems the QEMU
>> networking emulation is fragmenting the UDP packets because with
>> CONFIG_TFTP_BLOCKSIZE=1468 the tftp_write() function never receives
>> 1468 bytes but only a few hundreds at a time (max 544 bytes).
> 
> I thought you fixed that? Or was that only for on real hardware?

It works OK on real hardware...

> But also, some of those numbers sound unusually terrible to me. We can
> get some bad luck with the free instances, but, still.  Are you sure
> there's nothing else going on?

...and yes that's absolutely terrible. In fact I found something curious.
With the following patch applied to provide a memcpy() speed test:

diff --git a/cmd/test.c b/cmd/test.c
index b4c3eabf9f6..35b8d62af61 100644
--- a/cmd/test.c
+++ b/cmd/test.c
@@ -7,6 +7,8 @@
 #include <command.h>
 #include <fs.h>
 #include <log.h>
+#include <stdlib.h>
+#include <time.h>
 #include <vsprintf.h>
 
 #define OP_INVALID	0
@@ -215,3 +217,49 @@ U_BOOT_CMD(
 	"do nothing, successfully",
 	NULL
 );
+
+static int do_mtest(struct cmd_tbl *cmdtp, int flag, int argc,
+		    char *const argv[])
+{
+	ulong t0, delta;
+	void *src, *dst;
+	long sz;
+
+	if (argc < 2) {
+		printf("Usage: %s <size_in_bytes> [dst [src]]\n", argv[0]);
+		return 1;
+	}
+	sz = simple_strtol(argv[1], NULL, 10);
+	if (sz < 0) {
+		printf("%s: %s: invalid argument\n", argv[0], argv[1]);
+		return 1;
+	}
+	if (argc > 2) {
+		dst = (void *)hextoul(argv[2], NULL);
+		if (argc > 3) {
+			src = (void *)hextoul(argv[3], NULL);
+		} else {
+			src = malloc(sz);
+		}
+	} else {
+		dst = malloc(sz);
+		src = malloc(sz);
+	}
+	if (!src || !dst) {
+		printf("%s: out of memory or NULL address\n", argv[0]);
+		return 1;
+	}
+	printf("%ld bytes from 0x%p to 0x%p: ", sz, src, dst);
+	t0 = get_timer(0);
+	memcpy(dst, src, sz);
+	delta = get_timer(t0);
+	printf("%ld ms\n", delta);
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	mtest,	CONFIG_SYS_MAXARGS,	0,	do_mtest,
+	"memcpy() speed test",
+	NULL
+);

I can see that there are ranges of memory that are very slow to *write*
to (and the loadaddr used by the tftp test happens to fall in that
range):

$ make qemu_arm64_defconfig
$ make -j$(nproc) CROSS_COMPILE="ccache aarch64-linux-gnu-"
$ qemu-system-aarch64 -M virt -nographic -cpu cortex-a57 -bios u-boot.bin

U-Boot 2024.10-rc1-00195-g35ce7016c9cb-dirty (Aug 19 2024 - 16:42:15 +0200)
[...]
=> mtest
Usage: mtest <size_in_bytes> [dst [src]]
=> mtest 1000
1000 bytes from 0x00000000466baed0 to 0x00000000466baae0: 0 ms
=> mtest 1000 0x2000000 0x00000000466baed0
1000 bytes from 0x00000000466baed0 to 0x0000000002000000: 22 ms
=> mtest 1000 0x00000000466baed0 0x2000000
1000 bytes from 0x0000000002000000 to 0x00000000466baed0: 0 ms
=> 


>> - The r2dplus_* tests fail for reasons I don't understand.
> 
> Consistently, too?

Yes.


Thanks,
-- 
Jerome