[PATCH v10 00/25] Introduce the lwIP network stack

[Jerome Forissier]

On 9/13/24 15:59, Tom Rini wrote:
> On Fri, Sep 13, 2024 at 11:33:31AM +0200, Jerome Forissier wrote:
>>
>>
>> On 9/9/24 18:19, Tom Rini wrote:
>>> On Mon, Sep 09, 2024 at 04:11:37PM +0200, Jerome Forissier wrote:
>>>>
>>>>
>>>> On 9/6/24 19:54, Tom Rini wrote:
>>>>> On Fri, Sep 06, 2024 at 02:33:16PM +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
>>>>>
>>>>> On am64x-sk (am64x_evm_a53_defconfig) I'm seeing:
>>>>> => tftpboot 80200000 EFI/arm64/grubaa64.efi
>>>>> Using ethernet at 8000000port@1 device
>>>>> TFTP from server 192.168.116.10; our IP address is 192.168.116.23
>>>>> Filename 'EFI/arm64/grubaa64.efi'.
>>>>> Load address: 0x80200000
>>>>> Loading:
>>>>> ... silent hang ...
>>>>>
>>>>> Which I didn't see with v9. I can test other TI K3 platforms if it would
>>>>> help.
>>>>
>>>> Weird. I compared v9 and v10 (rebased onto the same commit as v9) but
>>>> I saw nothing obvious. Would you mind running the test again with these
>>>> traces added?
>>>>
>>>> diff --git a/net/lwip/net-lwip.c b/net/lwip/net-lwip.c
>>>> index 1948fc1c309..9bbfd8ee5a7 100644
>>>> --- a/net/lwip/net-lwip.c
>>>> +++ b/net/lwip/net-lwip.c
>>>> @@ -35,6 +35,7 @@ static err_t linkoutput(struct netif *netif, struct pbuf *p)
>>>>  	void *pp = NULL;
>>>>  	int err;
>>>>  
>>>> +	printf("[OUT|%d]", p->len);
>>>>  	if ((unsigned long)p->payload % PKTALIGN) {
>>>>  		/*
>>>>  		 * Some net drivers have strict alignment requirements and may
>>>> @@ -252,12 +253,16 @@ int net_lwip_rx(struct udevice *udev, struct netif *netif)
>>>>  	int len;
>>>>  	int i;
>>>>  
>>>> -	if (!eth_is_active(udev))
>>>> +	printf("[IN]");
>>>> +	if (!eth_is_active(udev)) {
>>>> +		printf("ERR: !eth_is_active()\n");
>>>>  		return -EINVAL;
>>>> +	}
>>>>  
>>>>  	flags = ETH_RECV_CHECK_DEVICE;
>>>>  	for (i = 0; i < ETH_PACKETS_BATCH_RECV; i++) {
>>>>  		len = eth_get_ops(udev)->recv(udev, flags, &packet);
>>>> +		printf("[IN|%d]", len);
>>>>  		flags = 0;
>>>>  
>>>>  		if (len > 0) {
>>>
>>> I have the log, if it helps. However, with debug prints added, now it
>>> completes.
>>
>> Duh! :-/
>>
>>> And I can see (as part of trimming down my test setup) that
>>> without the prints, some tests are OK. The much smaller "helloworld.efi"
>>> file and test is fine.
>>
>> I suspect we might be hitting the test framework timeout, because TFTP
>> with lwIP is slower than with the legacy stack as mentioned in [1] (lack
>> of window size support). Although it seems weird that you don't see the
>> hash signs at all, as if no data were received?
> 
> Yeah, it's acting like no data was received or otherwise just locked up.

OK

> If you have a beagleplay it should also show the problem.

Unfortunately no, I don't have the hardware.

> I can try and
> find some time to check it out, outside the test framework, but also the> Pi platforms pass this test fine (same binary, same physical network).


That would be great because I don't have any better suggestion...


In the meantime I do have v11 ready, with one minor fix, patches removed
(posted separately) and more importantly a change to enable CMD_PXE. From
the upcoming changelog:

===
- Add (some) support for CMD_PXE and therefore drop patch "[TESTING]
configs: set CONFIG_NET=y when PXE is enabled". This is build-tested
only and it is very likely that some work is needed to make it useful.
For example, adding the code for DHCP option 209 to lwIP so that
BOOTP_PXE_DHCP_OPTION can be supported.
===

Would you like me to post this v11 today, or do you prefer we sort out
the TFTP issue with TI K3 first?

Thanks again for you support on this series!

Regards,
-- 
Jerome