mbox series

[v5,00/20] Introduce the lwIP network stack

Message ID cover.1721910373.git.jerome.forissier@linaro.org
Headers show
Series Introduce the lwIP network stack | expand

Message

Jerome Forissier July 25, 2024, 12:57 p.m. UTC
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

Notes:

1. A number of features are currently incompatible with NET_LWIP: SANDBOX,
DFU_TFTP, FASTBOOT, SPL_NET. All make assumptions on how the network
stack is implemented and/or pull sybols that are not trivially exported
from lwIP. Some interface rework may be needed.

2. Due to the above and in order to provide some level of testing, a new QEMU
configuration is introduced (qemu_arm64_lwip_defconfig) which is the same
as qemu_arm64_defconfig but with NET_LWIP and CMD_*_LWIP enabled.
Tests are added to test/py/tests/test_net.py for that configuration.

3. The default QEMU networking doesn't work with NET_LWIP. wget
pauses and resets the connection. Wireshark shows [TCP Window Full] and
[TCP ZeroWindow]. Wen using an emulated e1000 however all is fine
(that is "-nic tap,model=e1000" on the QEMU command line, with a bridge
configured on the host).

Changes in v5:

- Rebased on next
- Refactor Kconfig options to avoid duplicates
- Library functions use a more consistent naming (dhcp_loop(),
ping_loop() etc.) and take a struct udevice * parameter (Heinrich S.)
- Do not use net_process_receive_packet() for input anymore. Instead of
calling eth_rx() which would invoke net_process_receive_packet(), we
call a new net_lwip_rx(udev) function which invokes the device recv()
and pushes the packets up the lwIP stack. Thus everything is tied to
a udevice now. (Heinrich S.)
- Add "configs: replace '# CONFIG_NET is not set' with CONFIG_NO_NET=y"
(Tom R.)
- tftp: unify display with legacy command: add throughput, 65 hashes per
line, one every 10 blocks received (Tom R.)
- Moved net-lwip/* to net/lwip/* (Simon G.)
- Rename static function low_level_output() to linkoutput() since it is
the name used in the lwIP netif struct.
- Fixed off-by-one in parse_url() which could cause wget to fail when
passed a URL with a host name (as opposed to a URL with an IP address).
- Improved TFTP performance by adding support for the blksize option
(patches "lwip: tftp: add support of blksize option to client" and
"net-lwip: add TFTP_BLOCKSIZE) (Tom R.)
- Add an optional port number to the tftp command for easier testing
(syntax: tftp [[ip:[port:]]filename])
- wget: display an "unsupported URI" error if uri is not http://
(Jon H.)
- Adjusted the lwIP TCP options in lib/lwip/u-boot/lwipopts.h for
better performance, in particular TCP_WND.
- Add "net: fec_mxc_init(): do not ignore return status of fec_open()"
- Set the proper environment variables when DHCP succeeds (ipaddr%d
etc.) and read the proper ones for the given device in new_netif(),
allowing correct behavior when several adapters are available (tested
on i.MX8M Plus).
- Fix an alignment issue with outgoing packets (see the linkoutput()
function). With that the i.MX8M Plus ENET1 interface works properly.
(reported by Tim H.).
- Add review tags

Changes in v4:

- Fixed the DHCP algorithm which was missing a sys_timeout() call in
the "fine timer" callback. This should close the issue that Tom R.
reported with his Raspberry Pi 3 (it does fix it on mine).
- The DHCP exchange timeout is increased from 2 to 10 seconds
- The DHCP exchange can be interrupted with Ctrl-C.
- "net: introduce alternative implementation as net-lwip/": rework
dependencies. A few symbols have 'depends on !NET_LWIP' and in addition
'NET_LWIP depends on !SANDBOX'. Sandbox, DSA and fastboot are
unsupported, because they are deeply welded to the current stack.
- All network commands (dns, ping, tftp and wget):
  * Get rid of global variables (Ilias A.)
  * Use printf() rather than log_info()
- "net-lwip: add ping command": use packet count instead of
timeout, fix code style (Ilias A.)
- Add "net: split cmd/net.c into cmd/net.c and cmd/net-common.c"
extracted from the wget patch (Ilias A.).
- Add "net: split include/net.h into net{,-common,-legacy,-lwip}.h"
(Ilias A.)
- Add "flash: prefix error codes with FL_" which is required to
avoid name clashes when splitting net.h
- Reworked the initialization of the lwIP stack. One and only
one network interface (struct netif) is added for the duration
of the command that uses that interface. That's commit "net-lwip:
add DHCP support and dhcp commmand".
- Drop "test: dm: dsa, eth: disable tests when CONFIG_NET_LWIP=y",
not needed now that NET_LWIP depend on !SANDBOX.
- qemu_arm64_lwip_defconfig now enables CMD_DNS and CMD_WGET (so
that all the supported network commands are available).

Changes in v3:

- Make NET_LWIP a Kconfig choice in patch "net: introduce alternative
implementation as net-lwip/" (Tom R.)
- Drop the patch introducing lwIP as a Git subtree and document the git
command in the cover letter instead (Tom R.)
- "net-lwip: add TFTP support and tftpboot command": use the same
"Bytes transferred =" message as in the legacy implementation (Tom R.,
Maxim U.)
- Drop "test/py: net: add _lwip variants of dhcp, ping and tftpboot
tests" which is not needed anymore.
- Add missing kfree() calls in cmd/net-common.c and fix the parsing of
decimal address in net-lwip/wget.c (patch "net-lwip: add wget command")
(Maxim U.)
- "net-lwip: add ping command": drop the ICMP payload (Ilias A.). Set
the sequence number to zero when entering ping_loop().

Changes in v2:

** Address comments from Ilias A.

- "net-lwip: add wget command"
Implement the wget_with_dns() function to do most of the wget work and
call it from do_wget(). This allows to simplify patch "net-lwip: add
support for EFI_HTTP_BOOT".

- "net-lwip: import net command from cmd/net.c"
Move a few functions from cmd/net.c to a new file cmd/net-common.c
rather than duplicating then in cmd/net-lwip.c.

- "net-lwip: add support for EFI_HTTP_BOOT"
Since wget_with_dns() is now implemented in "net-lwip: add wget command",
just enable the wget command when the lwIP stack is enabled and
EFI_HTTP_BOOT is requested.

** Address comments from Tom R.

- "net-lwip: add DHCP support and dhcp commmand",
  "net-lwip: add TFTP support and tftpboot command",
  "net-lwip: add ping command",
  "net-lwip: add dns command",
  "net-lwip: add wget command"
Do not introduce new CMD_XXX_LWIP symbols and use existing CMD_XXX
instead.

- "configs: add qemu_arm64_lwip_defconfig"
Use #include <configs/qemu_arm64_defconfig>.

- "net-lwip: import lwIP library under lib/lwip"
Patch removed and replaced by the introduction of a Git subtree:
"Squashed 'lib/lwip/lwip/' content from commit 0a0452b2c3".

Note that I have not yet addressed your comments on "test: dm: dsa,
eth: disable tests when CONFIG_NET_LWIP=y"). I need some more time
for that and I think running CI on this v2 will help better understand
what is needed for v3.

** Miscellaneous improvements

- "net: introduce alternative implementation as net-lwip/":

* Make DFU_OVER_TFTP not DFU_TFTP incompatible with NET_LWIP. It seems
quite natural to supplement "depends on NET" with "&& !NET_LWIP".
* Make PROT_*_LWIP not visible by removing the Kconfig prompt.

[1] https://lore.kernel.org/all/20231127125726.3735-1-maxim.uvarov@linaro.org/
[2] https://www.nongnu.org/lwip/
[3] https://en.wikipedia.org/wiki/LwIP

CC: Javier Tia <javier.tia@linaro.org>
CC: Raymond Mao <raymond.mao@linaro.org>

Jerome Forissier (19):
  flash: prefix error codes with FL_
  net: introduce alternative implementation as net-lwip/
  configs: replace '# CONFIG_NET is not set' with CONFIG_NO_NET=y
  net: fec_mxc_init(): do not ignore return status of fec_open()
  net: split include/net.h into net{,-common,-legacy,-lwip}.h
  net: eth-uclass: add function eth_start_udev()
  net-lwip: build lwIP
  net-lwip: add DHCP support and dhcp commmand
  net-lwip: add TFTP support and tftpboot command
  net-lwip: add ping command
  net-lwip: add dns command
  net: split cmd/net.c into cmd/net.c and cmd/net-common.c
  net-lwip: add wget command
  cmd: bdinfo: enable -e when CONFIG_CMD_NET_LWIP=y
  configs: add qemu_arm64_lwip_defconfig
  lwip: tftp: add support of blksize option to client
  net-lwip: add TFTP_BLOCKSIZE
  CI: add qemu_arm64_lwip to the test matrix
  MAINTAINERS: net-lwip: add myself as a maintainer

Jonathan Humphreys (1):
  net-lwip: lwIP wget supports user defined port in the uri, so allow
    it.

 .azure-pipelines.yml                          |   7 +
 Kconfig                                       |  26 +
 MAINTAINERS                                   |  11 +
 Makefile                                      |   4 +-
 board/cobra5272/flash.c                       |  26 +-
 board/freescale/m5253demo/flash.c             |   6 +-
 boot/Kconfig                                  |   3 +-
 cmd/Kconfig                                   |  84 +-
 cmd/Makefile                                  |   9 +-
 cmd/bdinfo.c                                  |   5 +-
 cmd/elf.c                                     |   2 +-
 cmd/net-common.c                              | 109 ++
 cmd/net-lwip.c                                |  45 +
 cmd/net.c                                     | 115 ---
 common/Kconfig                                |   2 +-
 common/board_r.c                              |   4 +-
 common/flash.c                                |  44 +-
 common/spl/Kconfig                            |   1 +
 common/usb_kbd.c                              |   2 +-
 configs/LicheePi_Zero_defconfig               |   2 +-
 configs/M5249EVB_defconfig                    |   2 +-
 configs/am335x_pdu001_defconfig               |   2 +-
 configs/am62ax_evm_r5_defconfig               |   2 +-
 configs/am62px_evm_r5_defconfig               |   2 +-
 configs/am62x_beagleplay_r5_defconfig         |   2 +-
 configs/amcore_defconfig                      |   2 +-
 configs/anbernic-rgxx3-rk3566_defconfig       |   2 +-
 configs/ap143_defconfig                       |   2 +-
 configs/ap152_defconfig                       |   2 +-
 configs/apple_m1_defconfig                    |   2 +-
 configs/astro_mcf5373l_defconfig              |   2 +-
 configs/at91sam9rlek_dataflash_defconfig      |   2 +-
 configs/at91sam9rlek_mmc_defconfig            |   2 +-
 configs/at91sam9rlek_nandflash_defconfig      |   2 +-
 configs/bcm7260_defconfig                     |   2 +-
 configs/bcm7445_defconfig                     |   2 +-
 configs/bcm968380gerg_ram_defconfig           |   2 +-
 configs/bcmns_defconfig                       |   2 +-
 configs/chromebook_samus_tpl_defconfig        |   2 +-
 configs/cortina_presidio-asic-base_defconfig  |   2 +-
 configs/cortina_presidio-asic-pnand_defconfig |   2 +-
 configs/durian_defconfig                      |   2 +-
 configs/e850-96_defconfig                     |   2 +-
 configs/ea-lpc3250devkitv2_defconfig          |   2 +-
 configs/efi-x86_app32_defconfig               |   2 +-
 configs/efi-x86_app64_defconfig               |   2 +-
 configs/emsdp_defconfig                       |   2 +-
 configs/evb-px5_defconfig                     |   2 +-
 configs/generic-rk3568_defconfig              |   2 +-
 configs/generic-rk3588_defconfig              |   2 +-
 configs/hc2910_2aghd05_defconfig              |   2 +-
 configs/igep00x0_defconfig                    |   2 +-
 configs/imx6q_bosch_acc_defconfig             |   2 +-
 configs/imx6ulz_smm_m2_defconfig              |   2 +-
 configs/iot_devkit_defconfig                  |   2 +-
 configs/legoev3_defconfig                     |   2 +-
 configs/mk808_defconfig                       |   2 +-
 configs/mx23evk_defconfig                     |   2 +-
 configs/mx28evk_defconfig                     |   2 +-
 configs/mx6memcal_defconfig                   |   2 +-
 configs/mx6ulz_14x14_evk_defconfig            |   2 +-
 configs/mx7ulp_com_defconfig                  |   2 +-
 configs/mx7ulp_evk_defconfig                  |   2 +-
 configs/mx7ulp_evk_plugin_defconfig           |   2 +-
 configs/netgear_cg3100d_ram_defconfig         |   2 +-
 configs/nsim_700_defconfig                    |   2 +-
 configs/nsim_700be_defconfig                  |   2 +-
 configs/nsim_hs38be_defconfig                 |   2 +-
 configs/openpiton_riscv64_defconfig           |   2 +-
 configs/openpiton_riscv64_spl_defconfig       |   2 +-
 configs/origen_defconfig                      |   2 +-
 configs/pe2201_defconfig                      |   2 +-
 configs/pinecube_defconfig                    |   2 +-
 configs/pm9261_defconfig                      |   2 +-
 configs/qemu_arm64_lwip_defconfig             |   5 +
 configs/s5p4418_nanopi2_defconfig             |   2 +-
 configs/s5p_goni_defconfig                    |   2 +-
 configs/s5pc210_universal_defconfig           |   2 +-
 configs/sama5d27_giantboard_defconfig         |   2 +-
 configs/sama5d29_curiosity_mmc1_defconfig     |   2 +-
 configs/sama5d29_curiosity_mmc_defconfig      |   2 +-
 .../sama5d29_curiosity_qspiflash_defconfig    |   2 +-
 configs/sama7g54_curiosity_mmc_defconfig      |   2 +-
 .../sama7g54_curiosity_nandflash_defconfig    |   2 +-
 .../sama7g54_curiosity_qspiflash_defconfig    |   2 +-
 configs/sipeed_maix_bitm_defconfig            |   2 +-
 configs/sipeed_maix_smode_defconfig           |   2 +-
 configs/stemmy_defconfig                      |   2 +-
 configs/stm32f429-discovery_defconfig         |   2 +-
 configs/stm32f429-evaluation_defconfig        |   2 +-
 configs/stm32f469-discovery_defconfig         |   2 +-
 configs/stm32h743-disco_defconfig             |   2 +-
 configs/stm32h743-eval_defconfig              |   2 +-
 configs/stm32h750-art-pi_defconfig            |   2 +-
 configs/stm32mp25_defconfig                   |   2 +-
 configs/stmark2_defconfig                     |   2 +-
 configs/th1520_lpi4a_defconfig                |   2 +-
 configs/thunderx_88xx_defconfig               |   2 +-
 configs/tools-only_defconfig                  |   2 +-
 configs/topic_miami_defconfig                 |   2 +-
 configs/topic_miamilite_defconfig             |   2 +-
 configs/topic_miamiplus_defconfig             |   2 +-
 configs/total_compute_defconfig               |   2 +-
 configs/trats2_defconfig                      |   2 +-
 configs/trats_defconfig                       |   2 +-
 configs/xenguest_arm64_defconfig              |   2 +-
 configs/xenguest_arm64_virtio_defconfig       |   2 +-
 configs/xilinx_versal_mini_defconfig          |   2 +-
 configs/xilinx_versal_mini_emmc0_defconfig    |   2 +-
 configs/xilinx_versal_mini_emmc1_defconfig    |   2 +-
 configs/xilinx_versal_mini_ospi_defconfig     |   2 +-
 configs/xilinx_versal_mini_qspi_defconfig     |   2 +-
 configs/xilinx_versal_net_mini_defconfig      |   2 +-
 configs/xilinx_versal_net_mini_emmc_defconfig |   2 +-
 configs/xilinx_versal_net_mini_ospi_defconfig |   2 +-
 configs/xilinx_versal_net_mini_qspi_defconfig |   2 +-
 configs/xilinx_zynqmp_mini_defconfig          |   2 +-
 configs/xilinx_zynqmp_mini_emmc0_defconfig    |   2 +-
 configs/xilinx_zynqmp_mini_emmc1_defconfig    |   2 +-
 configs/xilinx_zynqmp_mini_nand_defconfig     |   2 +-
 .../xilinx_zynqmp_mini_nand_single_defconfig  |   2 +-
 configs/xilinx_zynqmp_mini_qspi_defconfig     |   2 +-
 configs/zynq_cse_nand_defconfig               |   2 +-
 configs/zynq_cse_nor_defconfig                |   2 +-
 configs/zynq_cse_qspi_defconfig               |   2 +-
 drivers/dfu/Kconfig                           |   1 +
 drivers/fastboot/Kconfig                      |   1 +
 drivers/mtd/cfi_flash.c                       |  36 +-
 drivers/net/Kconfig                           |   3 +-
 drivers/net/fec_mxc.c                         |   3 +-
 drivers/net/phy/Kconfig                       |   2 +-
 drivers/usb/gadget/Kconfig                    |   2 +-
 include/flash.h                               |  20 +-
 include/net-common.h                          | 413 ++++++++
 include/net-legacy.h                          | 635 ++++++++++++
 include/net-lwip.h                            |  37 +
 include/net.h                                 | 944 +-----------------
 lib/Makefile                                  |   2 +
 lib/lwip/Makefile                             |  55 +
 lib/lwip/lwip/src/apps/tftp/tftp.c            |  94 +-
 .../lwip/src/include/lwip/apps/tftp_client.h  |   1 +
 lib/lwip/u-boot/arch/cc.h                     |  43 +
 lib/lwip/u-boot/arch/sys_arch.h               |   0
 lib/lwip/u-boot/limits.h                      |   0
 lib/lwip/u-boot/lwipopts.h                    | 157 +++
 net/Kconfig                                   |  49 +-
 net/Makefile                                  |  19 +-
 net/eth-uclass.c                              |  38 +-
 net/lwip/Kconfig                              |  34 +
 net/lwip/Makefile                             |   8 +
 net/lwip/dhcp.c                               | 136 +++
 net/lwip/dns.c                                | 127 +++
 net/lwip/eth_internal.h                       |  35 +
 net/lwip/net-lwip.c                           | 292 ++++++
 net/lwip/ping.c                               | 177 ++++
 net/lwip/tftp.c                               | 276 +++++
 net/lwip/wget.c                               | 272 +++++
 157 files changed, 3311 insertions(+), 1321 deletions(-)
 create mode 100644 cmd/net-common.c
 create mode 100644 cmd/net-lwip.c
 create mode 100644 configs/qemu_arm64_lwip_defconfig
 create mode 100644 include/net-common.h
 create mode 100644 include/net-legacy.h
 create mode 100644 include/net-lwip.h
 create mode 100644 lib/lwip/Makefile
 create mode 100644 lib/lwip/u-boot/arch/cc.h
 create mode 100644 lib/lwip/u-boot/arch/sys_arch.h
 create mode 100644 lib/lwip/u-boot/limits.h
 create mode 100644 lib/lwip/u-boot/lwipopts.h
 create mode 100644 net/lwip/Kconfig
 create mode 100644 net/lwip/Makefile
 create mode 100644 net/lwip/dhcp.c
 create mode 100644 net/lwip/dns.c
 create mode 100644 net/lwip/eth_internal.h
 create mode 100644 net/lwip/net-lwip.c
 create mode 100644 net/lwip/ping.c
 create mode 100644 net/lwip/tftp.c
 create mode 100644 net/lwip/wget.c

Comments

Fabio Estevam July 25, 2024, 1:03 p.m. UTC | #1
Hi Jerome,

On Thu, Jul 25, 2024 at 9:58 AM Jerome Forissier
<jerome.forissier@linaro.org> wrote:

> - Add "net: fec_mxc_init(): do not ignore return status of fec_open()"
> - Set the proper environment variables when DHCP succeeds (ipaddr%d
> etc.) and read the proper ones for the given device in new_netif(),
> allowing correct behavior when several adapters are available (tested
> on i.MX8M Plus).
> - Fix an alignment issue with outgoing packets (see the linkoutput()
> function). With that the i.MX8M Plus ENET1 interface works properly.
> (reported by Tim H.).

Adding Tim.

Thanks for fixing the i.MX8MP problems.

Cheers
Tom Rini July 25, 2024, 5:22 p.m. UTC | #2
On Thu, Jul 25, 2024 at 02:57:21PM +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

This is better than v4, and on the hardware platforms I could build and
boot on (which was most of mine except the am62x_beagleplay), the tests
ran and completed, including the tftp+boot a Linux kernel.

The bad news is CI blows up, a lot:
https://source.denx.de/u-boot/u-boot/-/pipelines/21764
And:
https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/9014/logs/106
which is another Kconfig dependency problem. I don't _think_ I
introduced that, but since this wasn't against top of tree, I had to
apply the cmd/Kconfig patch manually.

I have my world build running still and may have more comments based on
that.
Tom Rini July 25, 2024, 10:34 p.m. UTC | #3
On Thu, Jul 25, 2024 at 11:22:20AM -0600, Tom Rini wrote:
> On Thu, Jul 25, 2024 at 02:57:21PM +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
> 
> This is better than v4, and on the hardware platforms I could build and
> boot on (which was most of mine except the am62x_beagleplay), the tests
> ran and completed, including the tftp+boot a Linux kernel.
> 
> The bad news is CI blows up, a lot:
> https://source.denx.de/u-boot/u-boot/-/pipelines/21764
> And:
> https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/9014/logs/106
> which is another Kconfig dependency problem. I don't _think_ I
> introduced that, but since this wasn't against top of tree, I had to
> apply the cmd/Kconfig patch manually.
> 
> I have my world build running still and may have more comments based on
> that.

First, with NET_LWIP being default rather than NET, there's a lot of
other Kconfig dependency issues. Unfortunately I don't see an easy tool
for making sure this is all clean aside from a shell loop like:
for C in `(cd configs;ls)`;do make -s $C;done

Once those are fixed, this is feeling pretty OK I think. I assume PXE
support is high on the follow-up TODO list? That said, after taking
tiger-rk3588 as an example platform and hacking out PXE related stuff and
turning on lwIP:
   aarch64: (for 1/1 boards) all +10144.0 bss -4040.0 data -64.0 rodata -100.0 text +14348.0
            tiger-rk3588   : all +10144 bss -4040 data -64 rodata -100 text +14348
               u-boot: add: 161/-115, grow: 8/-6 bytes: 24552/-14382 (10170)
                 function                                   old     new   delta
                 dhcp_recv                                    -    1296   +1296
                 tftp_recv                                    -    1152   +1152
                 ip4_input                                    -     596    +596
                 new_netif                                    -     556    +556
                 etharp_query                                 -     552    +552
                 udp_input                                    -     540    +540
                 etharp_input                                 -     532    +532
                 distro_efi_read_bootflow_net                 -     516    +516
                 dhcp_loop                                    -     512    +512
                 ip4addr_aton                                 -     492    +492
                 static.dhcp_select                           -     484    +484
                 ip4_output_if_src                            -     436    +436
                 udp_sendto_if_src                            -     408    +408
                 static.dhcp_reboot                           -     400    +400
                 etharp_output                                -     400    +400
                 dhcp_create_msg                              -     384    +384
                 etharp_find_entry                            -     376    +376
                 dhcp_bind                                    -     364    +364
                 send_request                                 -     352    +352
                 dhcp_discover                                -     352    +352
                 raw_sendto_if_src                            -     328    +328
                 pbuf_copy_partial_pbuf                       -     328    +328
                 netif_add                                    -     324    +324
                 do_tftpb                                   544     868    +324
                 pbuf_alloc                                   -     308    +308
                 dhcp_release_and_stop                        -     308    +308
                 net_lwip_rx                                  -     292    +292
                 raw_input                                    -     288    +288
                 udp_bind                                     -     284    +284
                 ethernet_input                               -     284    +284
                 etharp_raw                                   -     284    +284
                 static.dhcp_handle_ack                       -     272    +272
                 ping_send                                    -     248    +248
                 ip4addr_ntoa_r                               -     220    +220
                 inet_chksum_pseudo                           -     216    +216
                 ping_recv                                    -     212    +212
                 etharp_output_to_arp_index                   -     208    +208
                 netif_set_addr                               -     204    +204
                 do_ping                                    124     328    +204
                 dhcp_start                                   -     204    +204
                 send_data                                    -     192    +192
                 dhcp_fine_tmr                                -     192    +192
                 pbuf_copy_partial                            -     184    +184
                 tftp_close                                   -     176    +176
                 script_read_bootflow                       420     596    +176
                 linkoutput                                   -     176    +176
                 ethernet_output                              -     176    +176
                 tftp_write                                   -     172    +172
                 pbuf_memcmp                                  -     168    +168
                 static.send_error                            -     164    +164
                 netif_remove                                 -     164    +164
                 raw_sendto                                   -     160    +160
                 arp_table                                    -     160    +160
                 tftp_init_common                             -     156    +156
                 pbuf_realloc                                 -     152    +152
                 dhcp_inc_pcb_refcount                        -     152    +152
                 tftp_tmr                                     -     148    +148
                 pbuf_free                                    -     148    +148
                 udp_sendto                                   -     144    +144
                 udp_connect                                  -     144    +144
                 sys_timeout_abs                              -     140    +140
                 ip4_route                                    -     136    +136
                 static.resend_data                           -     132    +132
                 pbuf_memfind                                 -     128    +128
                 pbuf_add_header_impl                         -     128    +128
                 boot_file_name                               -     128    +128
                 static.netif_do_set_ipaddr                   -     124    +124
                 lwip_standard_chksum                         -     124    +124
                 sys_check_timeouts                           -     120    +120
                 tftp_error                                   -     116    +116
                 etharp_free_entry                            -     116    +116
                 tftp_get                                     -     112    +112
                 dhcp_run                                     -     112    +112
                 static.send_ack                              -     108    +108
                 eth_start_udev                               -     104    +104
                 close_handle                                 -      96     +96
                 sys_untimeout                                -      92     +92
                 ip4_addr_isbroadcast_u32                     -      92     +92
                 init_packet                                  -      92     +92
                 etharp_cleanup_netif                         -      92     +92
                 raw_new                                      -      88     +88
                 pbuf_remove_header                           -      88     +88
                 net_lwip_get_netif                           -      88     +88
                 dhcp_option_trailer                          -      88     +88
                 udp_sendto_if                                -      84     +84
                 pbuf_alloc_reference                         -      84     +84
                 udp_remove                                   -      80     +80
                 ip4_input_accept                             -      80     +80
                 netif_set_link_up                            -      76     +76
                 udp_netif_ip_addr_changed                    -      72     +72
                 raw_remove                                   -      72     +72
                 raw_netif_ip_addr_changed                    -      72     +72
                 dhcp_option_long                             -      68     +68
                 dhcp_dec_pcb_refcount                        -      68     +68
                 udp_new                                      -      64     +64
                 sys_timeout                                  -      60     +60
                 pbuf_try_get_at                              -      60     +60
                 pbuf_clone                                   -      60     +60
                 dhcp_network_changed_link_up                 -      60     +60
                 tftp_cleanup                                 -      56     +56
                 pbuf_cat                                     -      56     +56
                 netif_get_by_index                           -      56     +56
                 _u_boot_list_2_cmd_2_dhcp                    -      56     +56
                 tftp_state                                   4      56     +52
                 pbuf_skip_const                              -      48     +48
                 net_lwip_if_init                             -      48     +48
                 dhcp_supplied_address                        -      48     +48
                 netif_issue_reports                          -      44     +44
                 tftp_context                                 -      40     +40
                 ip_data                                      -      40     +40
                 etharp_request                               -      40     +40
                 raw_bind                                     -      36     +36
                 net_lwip_remove_netif                        -      36     +36
                 memp_malloc                                  -      36     +36
                 ip4_output_if                                -      36     +36
                 dhcp_option_short                            -      36     +36
                 call_lwip_dhcp_fine_tmr                      -      36     +36
                 netif_set_up                                 -      32     +32
                 netif_set_down                               -      32     +32
                 inet_chksum                                  -      32     +32
                 dhcp_set_state                               -      32     +32
                 dhcp_rx_options_val                          -      32     +32
                 pbuf_get_at                                  -      28     +28
                 distro_efi_read_bootflow                    60      88     +28
                 distro_efi_boot                            360     388     +28
                 tftp_client_set_blksize                      -      24     +24
                 sys_now                                      -      24     +24
                 pbuf_copy                                    -      24     +24
                 pbuf_chain                                   -      24     +24
                 memp_free                                    -      24     +24
                 eth_bootdev_hunt                            44      68     +24
                 do_dhcp                                      -      24     +24
                 pbuf_ref                                     -      20     +20
                 netif_input                                  -      20     +20
                 do_net_stats                               328     348     +20
                 static.str                                   -      16     +16
                 ip4addr_ntoa                                 -      16     +16
                 udp_recv                                     -      12     +12
                 tftp_init_client                             -      12     +12
                 netif_set_default                            -      12     +12
                 udp_pcbs                                     -       8      +8
                 tftp_read                                    -       8      +8
                 tftp_open                                    -       8      +8
                 raw_recv                                     -       8      +8
                 raw_pcbs                                     -       8      +8
                 pbuf_add_header                              -       8      +8
                 next_timeout                                 -       8      +8
                 netif_null_output_ip4                        -       8      +8
                 netif_list                                   -       8      +8
                 netif_default                                -       8      +8
                 net_lwip_new_netif_noip                      -       8      +8
                 net_lwip_new_netif                           -       8      +8
                 lwip_htons                                   -       8      +8
                 lwip_htonl                                   -       8      +8
                 dhcp_rx_options_given                        -       8      +8
                 dhcp_pcb                                     -       8      +8
                 udp_new_ip_type                              -       4      +4
                 static.xid                                   -       4      +4
                 mem_trim                                     -       4      +4
                 mem_malloc                                   -       4      +4
                 mem_free                                     -       4      +4
                 ip_chksum_pseudo                             -       4      +4
                 current_timeout_due_time                     -       4      +4
                 udp_port                                     -       2      +2
                 ip_id                                        -       2      +2
                 static.called                                -       1      +1
                 netif_num                                    -       1      +1
                 etharp_cached_entry                          -       1      +1
                 dhcp_pcb_refcount                            -       1      +1
                 net_boot_file_name_explicit                  1       -      -1
                 tftp_windowsize                              2       -      -2
                 tftp_window_size_option                      2       -      -2
                 tftp_next_ack                                2       -      -2
                 tftp_last_nack                               2       -      -2
                 tftp_block_size_option                       2       -      -2
                 tftp_block_size                              2       -      -2
                 ping_seq_number                              2       -      -2
                 net_our_vlan                                 2       -      -2
                 net_native_vlan                              2       -      -2
                 env_flags_vartype_rep                        7       5      -2
                 timeout_count_max                            4       -      -4
                 timeout_count                                4       -      -4
                 tftp_timeout_count_max                       4       -      -4
                 tftp_remote_port                             4       -      -4
                 tftp_remote_ip                               4       -      -4
                 tftp_our_port                                4       -      -4
                 static.first_call                            4       -      -4
                 saved_tftp_block_size_option                 4       -      -4
                 net_try_count                                4       -      -4
                 net_state                                    4       -      -4
                 net_server_ip                                4       -      -4
                 net_rx_packet_len                            4       -      -4
                 net_restarted                                4       -      -4
                 net_ping_ip                                  4       -      -4
                 net_netmask                                  4       -      -4
                 net_ip_id                                    4       -      -4
                 net_ip                                       4       -      -4
                 net_gateway                                  4       -      -4
                 net_dns_server                               4       -      -4
                 net_dev_exists                               4       -      -4
                 net_boot_file_size                           4       -      -4
                 net_boot_file_expected_size_in_blocks        4       -      -4
                 net_arp_wait_reply_ip                        4       -      -4
                 net_arp_wait_packet_ip                       4       -      -4
                 dummy_handler                                4       -      -4
                 arp_wait_tx_packet_size                      4       -      -4
                 arp_wait_try                                 4       -      -4
                 net_server_ethaddr                           6       -      -6
                 net_ethaddr                                  6       -      -6
                 udp_packet_handler                           8       -      -8
                 timeout_ms                                   8       -      -8
                 time_handler                                 8       -      -8
                 time_delta                                   8       -      -8
                 tftp_prev_block                              8       -      -8
                 tftp_load_size                               8       -      -8
                 tftp_load_addr                               8       -      -8
                 tftp_cur_block                               8       -      -8
                 tftp_block_wrap_offset                       8       -      -8
                 tftp_block_wrap                              8       -      -8
                 net_tx_packet                                8       -      -8
                 net_rx_packet                                8       -      -8
                 arp_wait_timer_start                         8       -      -8
                 arp_wait_packet_ethaddr                      8       -      -8
                 arp_tx_packet                                8       -      -8
                 arp_packet_handler                           8       -      -8
                 net_get_arp_handler                         12       -     -12
                 default_filename                            13       -     -13
                 time_start                                  16       -     -16
                 start_again_timeout_handler                 16       -     -16
                 _u_boot_list_2_env_clbk_2_vlan              16       -     -16
                 _u_boot_list_2_env_clbk_2_serverip          16       -     -16
                 _u_boot_list_2_env_clbk_2_nvlan             16       -     -16
                 _u_boot_list_2_env_clbk_2_netmask           16       -     -16
                 _u_boot_list_2_env_clbk_2_ipaddr            16       -     -16
                 _u_boot_list_2_env_clbk_2_gatewayip         16       -     -16
                 arp_is_waiting                              20       -     -20
                 net_set_udp_handler                         24       -     -24
                 ip_checksum_ok                              28       -     -28
                 ping_timeout_handler                        32       -     -32
                 net_clear_handlers                          36       -     -36
                 ip_to_string                                36       -     -36
                 is_serverip_in_cmd                          40       -     -40
                 net_send_udp_packet                         44       -     -44
                 net_get_async_tx_pkt_buf                    44       -     -44
                 eth_halt_state_only                         44       -     -44
                 on_vlan                                     48       -     -48
                 on_serverip                                 48       -     -48
                 on_nvlan                                    48       -     -48
                 on_netmask                                  48       -     -48
                 on_ipaddr                                   48       -     -48
                 on_gatewayip                                48       -     -48
                 net_eth_hdr_size                            56       -     -56
                 arp_init                                    56       -     -56
                 net_init_loop                               60       -     -60
                 eth_init                                   300     232     -68
                 env_flags_validate                         644     576     -68
                 copy_filename                               76       -     -76
                 net_init                                   112      32     -80
                 string_to_vlan                              84       -     -84
                 net_set_timeout_handler                     84       -     -84
                 compute_ip_checksum                         92       -     -92
                 tftp_init_load_addr                        100       -    -100
                 skip_num                                   104       -    -104
                 tftp_filename                              128       -    -128
                 arp_request                                128       -    -128
                 tftp_timeout_handler                       132       -    -132
                 lmb_get_free_size                          136       -    -136
                 net_set_udp_header                         144       -    -144
                 arp_timeout_check                          148       -    -148
                 net_update_ether                           152       -    -152
                 eth_validate_ethaddr_str                   152       -    -152
                 string_to_ip                               156       -    -156
                 net_parse_bootfile                         160       -    -160
                 net_set_ether                              180       -    -180
                 net_set_ip_header                          184       -    -184
                 arp_raw_request                            228       -    -228
                 ping_start                                 276       -    -276
                 net_send_ip_packet                         292       -    -292
                 net_check_prereq                           308       -    -308
                 ping_receive                               332       -    -332
                 net_start_again                            336       -    -336
                 net_loop                                   544       -    -544
                 arp_receive                                560       -    -560
                 tftp_send                                  584       -    -584
                 net_process_received_packet                764       4    -760
                 tftp_start                                 908       -    -908
                 net_boot_file_name                        1024       -   -1024
                 tftp_handler                              1308       -   -1308
                 net_pkt_buf                               7744    6208   -1536
                 arp_tx_packet_buf                         1600       -   -1600

Although I'm not 100% sure that config is functionally equivalent, so
perhaps it would be helpful if you could take a board or two and
reconfigure them with the legacy stack, but equivalent functionality to
with lwIP, for comparison sake? Thanks!
Jerome Forissier July 30, 2024, 9:48 a.m. UTC | #4
On 7/25/24 19:22, Tom Rini wrote:
> On Thu, Jul 25, 2024 at 02:57:21PM +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
> 
> This is better than v4, and on the hardware platforms I could build and
> boot on (which was most of mine except the am62x_beagleplay), the tests
> ran and completed, including the tftp+boot a Linux kernel.
> 
> The bad news is CI blows up, a lot:
> https://source.denx.de/u-boot/u-boot/-/pipelines/21764

They're all failing with: undefined reference to `tftp_start'
The reason is I inadvertently removed the 'default y' on CMD_TFTPBOOT :/
Fixed in v6.

> And:
> https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/9014/logs/106
> which is another Kconfig dependency problem. I don't _think_ I
> introduced that, but since this wasn't against top of tree, I had to
> apply the cmd/Kconfig patch manually.

What's the build command? I can see:

/bin/bash --noprofile --norc /Users/runner/work/_temp/9710a6d2-6670-4f76-81bc-990cc45b1898.sh

...but that's not super helpful ;)

> 
> I have my world build running still and may have more comments based on
> that.


Thanks,
Tom Rini July 30, 2024, 2:02 p.m. UTC | #5
On Tue, Jul 30, 2024 at 11:48:06AM +0200, Jerome Forissier wrote:
> 
> 
> On 7/25/24 19:22, Tom Rini wrote:
> > On Thu, Jul 25, 2024 at 02:57:21PM +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
> > 
> > This is better than v4, and on the hardware platforms I could build and
> > boot on (which was most of mine except the am62x_beagleplay), the tests
> > ran and completed, including the tftp+boot a Linux kernel.
> > 
> > The bad news is CI blows up, a lot:
> > https://source.denx.de/u-boot/u-boot/-/pipelines/21764
> 
> They're all failing with: undefined reference to `tftp_start'
> The reason is I inadvertently removed the 'default y' on CMD_TFTPBOOT :/
> Fixed in v6.
> 
> > And:
> > https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/9014/logs/106
> > which is another Kconfig dependency problem. I don't _think_ I
> > introduced that, but since this wasn't against top of tree, I had to
> > apply the cmd/Kconfig patch manually.
> 
> What's the build command? I can see:
> 
> /bin/bash --noprofile --norc /Users/runner/work/_temp/9710a6d2-6670-4f76-81bc-990cc45b1898.sh
> 
> ...but that's not super helpful ;)

make tools-only_defconfig or so, sorry.
Jerome Forissier July 30, 2024, 2:23 p.m. UTC | #6
On 7/30/24 16:02, Tom Rini wrote:
> On Tue, Jul 30, 2024 at 11:48:06AM +0200, Jerome Forissier wrote:
>>
>>
>> On 7/25/24 19:22, Tom Rini wrote:
>>> On Thu, Jul 25, 2024 at 02:57:21PM +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
>>>
>>> This is better than v4, and on the hardware platforms I could build and
>>> boot on (which was most of mine except the am62x_beagleplay), the tests
>>> ran and completed, including the tftp+boot a Linux kernel.
>>>
>>> The bad news is CI blows up, a lot:
>>> https://source.denx.de/u-boot/u-boot/-/pipelines/21764
>>
>> They're all failing with: undefined reference to `tftp_start'
>> The reason is I inadvertently removed the 'default y' on CMD_TFTPBOOT :/
>> Fixed in v6.
>>
>>> And:
>>> https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/9014/logs/106
>>> which is another Kconfig dependency problem. I don't _think_ I
>>> introduced that, but since this wasn't against top of tree, I had to
>>> apply the cmd/Kconfig patch manually.
>>
>> What's the build command? I can see:
>>
>> /bin/bash --noprofile --norc /Users/runner/work/_temp/9710a6d2-6670-4f76-81bc-990cc45b1898.sh
>>
>> ...but that's not super helpful ;)
> 
> make tools-only_defconfig or so, sorry.

Thanks. With this config on my build machine, v5 fails with
'CONFIG_SYS_RX_BUFFER undeclared here'. It is an issue that you mentioned
(and which I fixed for v6) but it is not the same error reported in the CI.
Anyways with the SYS_RX_BUFFER issue fixed the build is now successful:

$ make tools-only_defconfig && make -s -j$(nproc) && echo OK
#
# configuration written to .config
#
OK

Thanks,
Jerome Forissier Aug. 1, 2024, 2:40 p.m. UTC | #7
On 7/26/24 00:34, Tom Rini wrote:
> On Thu, Jul 25, 2024 at 11:22:20AM -0600, Tom Rini wrote:
>> On Thu, Jul 25, 2024 at 02:57:21PM +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
>>
>> This is better than v4, and on the hardware platforms I could build and
>> boot on (which was most of mine except the am62x_beagleplay), the tests
>> ran and completed, including the tftp+boot a Linux kernel.
>>
>> The bad news is CI blows up, a lot:
>> https://source.denx.de/u-boot/u-boot/-/pipelines/21764
>> And:
>> https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/9014/logs/106
>> which is another Kconfig dependency problem. I don't _think_ I
>> introduced that, but since this wasn't against top of tree, I had to
>> apply the cmd/Kconfig patch manually.
>>
>> I have my world build running still and may have more comments based on
>> that.
> 
> First, with NET_LWIP being default rather than NET, there's a lot of
> other Kconfig dependency issues. Unfortunately I don't see an easy tool
> for making sure this is all clean aside from a shell loop like:
> for C in `(cd configs;ls)`;do make -s $C;done

I have run this loop successfully with the upcoming v6 version. Some
configs do print some warnings but there is no error.


> Once those are fixed, this is feeling pretty OK I think. I assume PXE
> support is high on the follow-up TODO list?

Certainly, although I'm not sure I'll be able to spend time on it in the
very near future.

> That said, after taking
> tiger-rk3588 as an example platform and hacking out PXE related stuff and
> turning on lwIP:
>    aarch64: (for 1/1 boards) all +10144.0 bss -4040.0 data -64.0 rodata -100.0 text +14348.0
>             tiger-rk3588   : all +10144 bss -4040 data -64 rodata -100 text +14348
>                u-boot: add: 161/-115, grow: 8/-6 bytes: 24552/-14382 (10170)
[snip]
 
> Although I'm not 100% sure that config is functionally equivalent, so
> perhaps it would be helpful if you could take a board or two and
> reconfigure them with the legacy stack, but equivalent functionality to
> with lwIP, for comparison sake? Thanks!

I tried two boards and compared NET (u-boot.net) agains NET_LWIP
(u-boot). I will give more details on how to remove PXE from the NET
build and select/unselect the proper Kconfig symbols to obtain equivalent
functionality in the cover letter for v6. Note that dhcp, ping, dns, tftp
and wget are enabled in both builds. Here are the results.

- For imx8mp_evk_defconfig:

$ ~/work/linux/scripts/bloat-o-meter u-boot.net u-boot | sed -n '1p;$p'
add/remove: 228/162 grow/shrink: 49/4 up/down: 51217/-29078 (22139)
Total: Before=651990, After=674129, chg +3.40%

- For rpi_3_32b_defconfig:

$ ~/work/linux/scripts/bloat-o-meter u-boot.net u-boot | sed -n '1p;$p'
add/remove: 256/92 grow/shrink: 5/8 up/down: 50934/-16780 (34154)
Total: Before=418877, After=453031, chg +8.15%

I will post v6 soon.

Thanks,
Tom Rini Aug. 1, 2024, 2:43 p.m. UTC | #8
On Thu, Aug 01, 2024 at 04:40:03PM +0200, Jerome Forissier wrote:
> 
> 
> On 7/26/24 00:34, Tom Rini wrote:
> > On Thu, Jul 25, 2024 at 11:22:20AM -0600, Tom Rini wrote:
> >> On Thu, Jul 25, 2024 at 02:57:21PM +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
> >>
> >> This is better than v4, and on the hardware platforms I could build and
> >> boot on (which was most of mine except the am62x_beagleplay), the tests
> >> ran and completed, including the tftp+boot a Linux kernel.
> >>
> >> The bad news is CI blows up, a lot:
> >> https://source.denx.de/u-boot/u-boot/-/pipelines/21764
> >> And:
> >> https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/9014/logs/106
> >> which is another Kconfig dependency problem. I don't _think_ I
> >> introduced that, but since this wasn't against top of tree, I had to
> >> apply the cmd/Kconfig patch manually.
> >>
> >> I have my world build running still and may have more comments based on
> >> that.
> > 
> > First, with NET_LWIP being default rather than NET, there's a lot of
> > other Kconfig dependency issues. Unfortunately I don't see an easy tool
> > for making sure this is all clean aside from a shell loop like:
> > for C in `(cd configs;ls)`;do make -s $C;done
> 
> I have run this loop successfully with the upcoming v6 version. Some
> configs do print some warnings but there is no error.

Those warnings are the problem, I wasn't clear, sorry. If I could make
it error, I would since then CI would catch it.
Jerome Forissier Aug. 1, 2024, 3:15 p.m. UTC | #9
On 8/1/24 16:43, Tom Rini wrote:
> On Thu, Aug 01, 2024 at 04:40:03PM +0200, Jerome Forissier wrote:
>>
>>
>> On 7/26/24 00:34, Tom Rini wrote:
>>> On Thu, Jul 25, 2024 at 11:22:20AM -0600, Tom Rini wrote:
>>>> On Thu, Jul 25, 2024 at 02:57:21PM +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
>>>>
>>>> This is better than v4, and on the hardware platforms I could build and
>>>> boot on (which was most of mine except the am62x_beagleplay), the tests
>>>> ran and completed, including the tftp+boot a Linux kernel.
>>>>
>>>> The bad news is CI blows up, a lot:
>>>> https://source.denx.de/u-boot/u-boot/-/pipelines/21764
>>>> And:
>>>> https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/9014/logs/106
>>>> which is another Kconfig dependency problem. I don't _think_ I
>>>> introduced that, but since this wasn't against top of tree, I had to
>>>> apply the cmd/Kconfig patch manually.
>>>>
>>>> I have my world build running still and may have more comments based on
>>>> that.
>>>
>>> First, with NET_LWIP being default rather than NET, there's a lot of
>>> other Kconfig dependency issues. Unfortunately I don't see an easy tool
>>> for making sure this is all clean aside from a shell loop like:
>>> for C in `(cd configs;ls)`;do make -s $C;done
>>
>> I have run this loop successfully with the upcoming v6 version. Some
>> configs do print some warnings but there is no error.
> 
> Those warnings are the problem, I wasn't clear, sorry. If I could make
> it error, I would since then CI would catch it.

v6 won't add any warning besides the ones in the new
qemu_arm64_lwip_defconfig file that you said were OK:

$ git co origin/next
$ for C in `(cd configs;ls)`;do echo $C...; make -s $C || break;done 2>&1 | tee configs_next.log
$ git checkout to-upstream/v6-wip
$ for C in `(cd configs;ls)`;do echo $C...; make -s $C || break;done 2>&1 | tee configs.log
$ diff -u configs_next.log configs.log
--- configs_next.log    2024-08-01 17:10:50.903533652 +0200
+++ configs.log 2024-08-01 17:03:55.497799493 +0200
@@ -2016,6 +2016,11 @@
 q8_a33_tablet_800x480_defconfig...
 qcom_defconfig...
 qemu_arm64_defconfig...
+qemu_arm64_lwip_defconfig...
+generated_defconfig:71:warning: override: reassigning to symbol ARM
+generated_defconfig:71:warning: override: ARM changes choice state
+generated_defconfig:72:warning: override: reassigning to symbol ARCH_QEMU
+generated_defconfig:72:warning: override: ARCH_QEMU changes choice state
 qemu_arm_defconfig...
 qemu-ppce500_defconfig...
 qemu-riscv32_defconfig...

Thanks,
Tom Rini Aug. 1, 2024, 3:21 p.m. UTC | #10
On Thu, Aug 01, 2024 at 05:15:34PM +0200, Jerome Forissier wrote:
> 
> 
> On 8/1/24 16:43, Tom Rini wrote:
> > On Thu, Aug 01, 2024 at 04:40:03PM +0200, Jerome Forissier wrote:
> >>
> >>
> >> On 7/26/24 00:34, Tom Rini wrote:
> >>> On Thu, Jul 25, 2024 at 11:22:20AM -0600, Tom Rini wrote:
> >>>> On Thu, Jul 25, 2024 at 02:57:21PM +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
> >>>>
> >>>> This is better than v4, and on the hardware platforms I could build and
> >>>> boot on (which was most of mine except the am62x_beagleplay), the tests
> >>>> ran and completed, including the tftp+boot a Linux kernel.
> >>>>
> >>>> The bad news is CI blows up, a lot:
> >>>> https://source.denx.de/u-boot/u-boot/-/pipelines/21764
> >>>> And:
> >>>> https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/9014/logs/106
> >>>> which is another Kconfig dependency problem. I don't _think_ I
> >>>> introduced that, but since this wasn't against top of tree, I had to
> >>>> apply the cmd/Kconfig patch manually.
> >>>>
> >>>> I have my world build running still and may have more comments based on
> >>>> that.
> >>>
> >>> First, with NET_LWIP being default rather than NET, there's a lot of
> >>> other Kconfig dependency issues. Unfortunately I don't see an easy tool
> >>> for making sure this is all clean aside from a shell loop like:
> >>> for C in `(cd configs;ls)`;do make -s $C;done
> >>
> >> I have run this loop successfully with the upcoming v6 version. Some
> >> configs do print some warnings but there is no error.
> > 
> > Those warnings are the problem, I wasn't clear, sorry. If I could make
> > it error, I would since then CI would catch it.
> 
> v6 won't add any warning besides the ones in the new
> qemu_arm64_lwip_defconfig file that you said were OK:
> 
> $ git co origin/next
> $ for C in `(cd configs;ls)`;do echo $C...; make -s $C || break;done 2>&1 | tee configs_next.log
> $ git checkout to-upstream/v6-wip
> $ for C in `(cd configs;ls)`;do echo $C...; make -s $C || break;done 2>&1 | tee configs.log
> $ diff -u configs_next.log configs.log
> --- configs_next.log    2024-08-01 17:10:50.903533652 +0200
> +++ configs.log 2024-08-01 17:03:55.497799493 +0200
> @@ -2016,6 +2016,11 @@
>  q8_a33_tablet_800x480_defconfig...
>  qcom_defconfig...
>  qemu_arm64_defconfig...
> +qemu_arm64_lwip_defconfig...
> +generated_defconfig:71:warning: override: reassigning to symbol ARM
> +generated_defconfig:71:warning: override: ARM changes choice state
> +generated_defconfig:72:warning: override: reassigning to symbol ARCH_QEMU
> +generated_defconfig:72:warning: override: ARCH_QEMU changes choice state
>  qemu_arm_defconfig...
>  qemu-ppce500_defconfig...
>  qemu-riscv32_defconfig...

Ah, ok, thanks.
Tom Rini Aug. 2, 2024, 8:16 p.m. UTC | #11
On Thu, Aug 01, 2024 at 04:40:03PM +0200, Jerome Forissier wrote:
> 
> 
> On 7/26/24 00:34, Tom Rini wrote:
> > On Thu, Jul 25, 2024 at 11:22:20AM -0600, Tom Rini wrote:
> >> On Thu, Jul 25, 2024 at 02:57:21PM +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
> >>
> >> This is better than v4, and on the hardware platforms I could build and
> >> boot on (which was most of mine except the am62x_beagleplay), the tests
> >> ran and completed, including the tftp+boot a Linux kernel.
> >>
> >> The bad news is CI blows up, a lot:
> >> https://source.denx.de/u-boot/u-boot/-/pipelines/21764
> >> And:
> >> https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/9014/logs/106
> >> which is another Kconfig dependency problem. I don't _think_ I
> >> introduced that, but since this wasn't against top of tree, I had to
> >> apply the cmd/Kconfig patch manually.
> >>
> >> I have my world build running still and may have more comments based on
> >> that.
> > 
> > First, with NET_LWIP being default rather than NET, there's a lot of
> > other Kconfig dependency issues. Unfortunately I don't see an easy tool
> > for making sure this is all clean aside from a shell loop like:
> > for C in `(cd configs;ls)`;do make -s $C;done
> 
> I have run this loop successfully with the upcoming v6 version. Some
> configs do print some warnings but there is no error.

Oh. Right. So, to make the rest of the problems show up, switch the
default stack to NET_LWIP and then we see other issues.
Jerome Forissier Aug. 7, 2024, 3:06 p.m. UTC | #12
On 8/2/24 22:16, Tom Rini wrote:
> On Thu, Aug 01, 2024 at 04:40:03PM +0200, Jerome Forissier wrote:
>>
>>
>> On 7/26/24 00:34, Tom Rini wrote:
>>> On Thu, Jul 25, 2024 at 11:22:20AM -0600, Tom Rini wrote:
>>>> On Thu, Jul 25, 2024 at 02:57:21PM +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
>>>>
>>>> This is better than v4, and on the hardware platforms I could build and
>>>> boot on (which was most of mine except the am62x_beagleplay), the tests
>>>> ran and completed, including the tftp+boot a Linux kernel.
>>>>
>>>> The bad news is CI blows up, a lot:
>>>> https://source.denx.de/u-boot/u-boot/-/pipelines/21764
>>>> And:
>>>> https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/9014/logs/106
>>>> which is another Kconfig dependency problem. I don't _think_ I
>>>> introduced that, but since this wasn't against top of tree, I had to
>>>> apply the cmd/Kconfig patch manually.
>>>>
>>>> I have my world build running still and may have more comments based on
>>>> that.
>>>
>>> First, with NET_LWIP being default rather than NET, there's a lot of
>>> other Kconfig dependency issues. Unfortunately I don't see an easy tool
>>> for making sure this is all clean aside from a shell loop like:
>>> for C in `(cd configs;ls)`;do make -s $C;done
>>
>> I have run this loop successfully with the upcoming v6 version. Some
>> configs do print some warnings but there is no error.
> 
> Oh. Right. So, to make the rest of the problems show up, switch the
> default stack to NET_LWIP and then we see other issues.

Indeed. I fixed a couple "unmet direct dependencies detected" warnings in
v8.

Thanks,
Michael Nazzareno Trimarchi Aug. 7, 2024, 6:54 p.m. UTC | #13
Hi Jerome

On Thu, Jul 25, 2024 at 2:58 PM Jerome Forissier
<jerome.forissier@linaro.org> 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
>
> Notes:
>
> 1. A number of features are currently incompatible with NET_LWIP: SANDBOX,
> DFU_TFTP, FASTBOOT, SPL_NET. All make assumptions on how the network
> stack is implemented and/or pull sybols that are not trivially exported
> from lwIP. Some interface rework may be needed.
>
> 2. Due to the above and in order to provide some level of testing, a new QEMU
> configuration is introduced (qemu_arm64_lwip_defconfig) which is the same
> as qemu_arm64_defconfig but with NET_LWIP and CMD_*_LWIP enabled.
> Tests are added to test/py/tests/test_net.py for that configuration.
>
> 3. The default QEMU networking doesn't work with NET_LWIP. wget
> pauses and resets the connection. Wireshark shows [TCP Window Full] and
> [TCP ZeroWindow]. Wen using an emulated e1000 however all is fine
> (that is "-nic tap,model=e1000" on the QEMU command line, with a bridge
> configured on the host).
>
> Changes in v5:
>
> - Rebased on next
> - Refactor Kconfig options to avoid duplicates
> - Library functions use a more consistent naming (dhcp_loop(),
> ping_loop() etc.) and take a struct udevice * parameter (Heinrich S.)
> - Do not use net_process_receive_packet() for input anymore. Instead of
> calling eth_rx() which would invoke net_process_receive_packet(), we
> call a new net_lwip_rx(udev) function which invokes the device recv()
> and pushes the packets up the lwIP stack. Thus everything is tied to
> a udevice now. (Heinrich S.)
> - Add "configs: replace '# CONFIG_NET is not set' with CONFIG_NO_NET=y"
> (Tom R.)

Here I have some questions. You can have CONFIG_NET with two alternatives and
the alternatives are exclusive. Why do we need a CONFIG_NO_NET definition?

Michael

> - tftp: unify display with legacy command: add throughput, 65 hashes per
> line, one every 10 blocks received (Tom R.)
> - Moved net-lwip/* to net/lwip/* (Simon G.)
> - Rename static function low_level_output() to linkoutput() since it is
> the name used in the lwIP netif struct.
> - Fixed off-by-one in parse_url() which could cause wget to fail when
> passed a URL with a host name (as opposed to a URL with an IP address).
> - Improved TFTP performance by adding support for the blksize option
> (patches "lwip: tftp: add support of blksize option to client" and
> "net-lwip: add TFTP_BLOCKSIZE) (Tom R.)
> - Add an optional port number to the tftp command for easier testing
> (syntax: tftp [[ip:[port:]]filename])
> - wget: display an "unsupported URI" error if uri is not http://
> (Jon H.)
> - Adjusted the lwIP TCP options in lib/lwip/u-boot/lwipopts.h for
> better performance, in particular TCP_WND.
> - Add "net: fec_mxc_init(): do not ignore return status of fec_open()"
> - Set the proper environment variables when DHCP succeeds (ipaddr%d
> etc.) and read the proper ones for the given device in new_netif(),
> allowing correct behavior when several adapters are available (tested
> on i.MX8M Plus).
> - Fix an alignment issue with outgoing packets (see the linkoutput()
> function). With that the i.MX8M Plus ENET1 interface works properly.
> (reported by Tim H.).
> - Add review tags
>
> Changes in v4:
>
> - Fixed the DHCP algorithm which was missing a sys_timeout() call in
> the "fine timer" callback. This should close the issue that Tom R.
> reported with his Raspberry Pi 3 (it does fix it on mine).
> - The DHCP exchange timeout is increased from 2 to 10 seconds
> - The DHCP exchange can be interrupted with Ctrl-C.
> - "net: introduce alternative implementation as net-lwip/": rework
> dependencies. A few symbols have 'depends on !NET_LWIP' and in addition
> 'NET_LWIP depends on !SANDBOX'. Sandbox, DSA and fastboot are
> unsupported, because they are deeply welded to the current stack.
> - All network commands (dns, ping, tftp and wget):
>   * Get rid of global variables (Ilias A.)
>   * Use printf() rather than log_info()
> - "net-lwip: add ping command": use packet count instead of
> timeout, fix code style (Ilias A.)
> - Add "net: split cmd/net.c into cmd/net.c and cmd/net-common.c"
> extracted from the wget patch (Ilias A.).
> - Add "net: split include/net.h into net{,-common,-legacy,-lwip}.h"
> (Ilias A.)
> - Add "flash: prefix error codes with FL_" which is required to
> avoid name clashes when splitting net.h
> - Reworked the initialization of the lwIP stack. One and only
> one network interface (struct netif) is added for the duration
> of the command that uses that interface. That's commit "net-lwip:
> add DHCP support and dhcp commmand".
> - Drop "test: dm: dsa, eth: disable tests when CONFIG_NET_LWIP=y",
> not needed now that NET_LWIP depend on !SANDBOX.
> - qemu_arm64_lwip_defconfig now enables CMD_DNS and CMD_WGET (so
> that all the supported network commands are available).
>
> Changes in v3:
>
> - Make NET_LWIP a Kconfig choice in patch "net: introduce alternative
> implementation as net-lwip/" (Tom R.)
> - Drop the patch introducing lwIP as a Git subtree and document the git
> command in the cover letter instead (Tom R.)
> - "net-lwip: add TFTP support and tftpboot command": use the same
> "Bytes transferred =" message as in the legacy implementation (Tom R.,
> Maxim U.)
> - Drop "test/py: net: add _lwip variants of dhcp, ping and tftpboot
> tests" which is not needed anymore.
> - Add missing kfree() calls in cmd/net-common.c and fix the parsing of
> decimal address in net-lwip/wget.c (patch "net-lwip: add wget command")
> (Maxim U.)
> - "net-lwip: add ping command": drop the ICMP payload (Ilias A.). Set
> the sequence number to zero when entering ping_loop().
>
> Changes in v2:
>
> ** Address comments from Ilias A.
>
> - "net-lwip: add wget command"
> Implement the wget_with_dns() function to do most of the wget work and
> call it from do_wget(). This allows to simplify patch "net-lwip: add
> support for EFI_HTTP_BOOT".
>
> - "net-lwip: import net command from cmd/net.c"
> Move a few functions from cmd/net.c to a new file cmd/net-common.c
> rather than duplicating then in cmd/net-lwip.c.
>
> - "net-lwip: add support for EFI_HTTP_BOOT"
> Since wget_with_dns() is now implemented in "net-lwip: add wget command",
> just enable the wget command when the lwIP stack is enabled and
> EFI_HTTP_BOOT is requested.
>
> ** Address comments from Tom R.
>
> - "net-lwip: add DHCP support and dhcp commmand",
>   "net-lwip: add TFTP support and tftpboot command",
>   "net-lwip: add ping command",
>   "net-lwip: add dns command",
>   "net-lwip: add wget command"
> Do not introduce new CMD_XXX_LWIP symbols and use existing CMD_XXX
> instead.
>
> - "configs: add qemu_arm64_lwip_defconfig"
> Use #include <configs/qemu_arm64_defconfig>.
>
> - "net-lwip: import lwIP library under lib/lwip"
> Patch removed and replaced by the introduction of a Git subtree:
> "Squashed 'lib/lwip/lwip/' content from commit 0a0452b2c3".
>
> Note that I have not yet addressed your comments on "test: dm: dsa,
> eth: disable tests when CONFIG_NET_LWIP=y"). I need some more time
> for that and I think running CI on this v2 will help better understand
> what is needed for v3.
>
> ** Miscellaneous improvements
>
> - "net: introduce alternative implementation as net-lwip/":
>
> * Make DFU_OVER_TFTP not DFU_TFTP incompatible with NET_LWIP. It seems
> quite natural to supplement "depends on NET" with "&& !NET_LWIP".
> * Make PROT_*_LWIP not visible by removing the Kconfig prompt.
>
> [1] https://lore.kernel.org/all/20231127125726.3735-1-maxim.uvarov@linaro.org/
> [2] https://www.nongnu.org/lwip/
> [3] https://en.wikipedia.org/wiki/LwIP
>
> CC: Javier Tia <javier.tia@linaro.org>
> CC: Raymond Mao <raymond.mao@linaro.org>
>
> Jerome Forissier (19):
>   flash: prefix error codes with FL_
>   net: introduce alternative implementation as net-lwip/
>   configs: replace '# CONFIG_NET is not set' with CONFIG_NO_NET=y
>   net: fec_mxc_init(): do not ignore return status of fec_open()
>   net: split include/net.h into net{,-common,-legacy,-lwip}.h
>   net: eth-uclass: add function eth_start_udev()
>   net-lwip: build lwIP
>   net-lwip: add DHCP support and dhcp commmand
>   net-lwip: add TFTP support and tftpboot command
>   net-lwip: add ping command
>   net-lwip: add dns command
>   net: split cmd/net.c into cmd/net.c and cmd/net-common.c
>   net-lwip: add wget command
>   cmd: bdinfo: enable -e when CONFIG_CMD_NET_LWIP=y
>   configs: add qemu_arm64_lwip_defconfig
>   lwip: tftp: add support of blksize option to client
>   net-lwip: add TFTP_BLOCKSIZE
>   CI: add qemu_arm64_lwip to the test matrix
>   MAINTAINERS: net-lwip: add myself as a maintainer
>
> Jonathan Humphreys (1):
>   net-lwip: lwIP wget supports user defined port in the uri, so allow
>     it.
>
>  .azure-pipelines.yml                          |   7 +
>  Kconfig                                       |  26 +
>  MAINTAINERS                                   |  11 +
>  Makefile                                      |   4 +-
>  board/cobra5272/flash.c                       |  26 +-
>  board/freescale/m5253demo/flash.c             |   6 +-
>  boot/Kconfig                                  |   3 +-
>  cmd/Kconfig                                   |  84 +-
>  cmd/Makefile                                  |   9 +-
>  cmd/bdinfo.c                                  |   5 +-
>  cmd/elf.c                                     |   2 +-
>  cmd/net-common.c                              | 109 ++
>  cmd/net-lwip.c                                |  45 +
>  cmd/net.c                                     | 115 ---
>  common/Kconfig                                |   2 +-
>  common/board_r.c                              |   4 +-
>  common/flash.c                                |  44 +-
>  common/spl/Kconfig                            |   1 +
>  common/usb_kbd.c                              |   2 +-
>  configs/LicheePi_Zero_defconfig               |   2 +-
>  configs/M5249EVB_defconfig                    |   2 +-
>  configs/am335x_pdu001_defconfig               |   2 +-
>  configs/am62ax_evm_r5_defconfig               |   2 +-
>  configs/am62px_evm_r5_defconfig               |   2 +-
>  configs/am62x_beagleplay_r5_defconfig         |   2 +-
>  configs/amcore_defconfig                      |   2 +-
>  configs/anbernic-rgxx3-rk3566_defconfig       |   2 +-
>  configs/ap143_defconfig                       |   2 +-
>  configs/ap152_defconfig                       |   2 +-
>  configs/apple_m1_defconfig                    |   2 +-
>  configs/astro_mcf5373l_defconfig              |   2 +-
>  configs/at91sam9rlek_dataflash_defconfig      |   2 +-
>  configs/at91sam9rlek_mmc_defconfig            |   2 +-
>  configs/at91sam9rlek_nandflash_defconfig      |   2 +-
>  configs/bcm7260_defconfig                     |   2 +-
>  configs/bcm7445_defconfig                     |   2 +-
>  configs/bcm968380gerg_ram_defconfig           |   2 +-
>  configs/bcmns_defconfig                       |   2 +-
>  configs/chromebook_samus_tpl_defconfig        |   2 +-
>  configs/cortina_presidio-asic-base_defconfig  |   2 +-
>  configs/cortina_presidio-asic-pnand_defconfig |   2 +-
>  configs/durian_defconfig                      |   2 +-
>  configs/e850-96_defconfig                     |   2 +-
>  configs/ea-lpc3250devkitv2_defconfig          |   2 +-
>  configs/efi-x86_app32_defconfig               |   2 +-
>  configs/efi-x86_app64_defconfig               |   2 +-
>  configs/emsdp_defconfig                       |   2 +-
>  configs/evb-px5_defconfig                     |   2 +-
>  configs/generic-rk3568_defconfig              |   2 +-
>  configs/generic-rk3588_defconfig              |   2 +-
>  configs/hc2910_2aghd05_defconfig              |   2 +-
>  configs/igep00x0_defconfig                    |   2 +-
>  configs/imx6q_bosch_acc_defconfig             |   2 +-
>  configs/imx6ulz_smm_m2_defconfig              |   2 +-
>  configs/iot_devkit_defconfig                  |   2 +-
>  configs/legoev3_defconfig                     |   2 +-
>  configs/mk808_defconfig                       |   2 +-
>  configs/mx23evk_defconfig                     |   2 +-
>  configs/mx28evk_defconfig                     |   2 +-
>  configs/mx6memcal_defconfig                   |   2 +-
>  configs/mx6ulz_14x14_evk_defconfig            |   2 +-
>  configs/mx7ulp_com_defconfig                  |   2 +-
>  configs/mx7ulp_evk_defconfig                  |   2 +-
>  configs/mx7ulp_evk_plugin_defconfig           |   2 +-
>  configs/netgear_cg3100d_ram_defconfig         |   2 +-
>  configs/nsim_700_defconfig                    |   2 +-
>  configs/nsim_700be_defconfig                  |   2 +-
>  configs/nsim_hs38be_defconfig                 |   2 +-
>  configs/openpiton_riscv64_defconfig           |   2 +-
>  configs/openpiton_riscv64_spl_defconfig       |   2 +-
>  configs/origen_defconfig                      |   2 +-
>  configs/pe2201_defconfig                      |   2 +-
>  configs/pinecube_defconfig                    |   2 +-
>  configs/pm9261_defconfig                      |   2 +-
>  configs/qemu_arm64_lwip_defconfig             |   5 +
>  configs/s5p4418_nanopi2_defconfig             |   2 +-
>  configs/s5p_goni_defconfig                    |   2 +-
>  configs/s5pc210_universal_defconfig           |   2 +-
>  configs/sama5d27_giantboard_defconfig         |   2 +-
>  configs/sama5d29_curiosity_mmc1_defconfig     |   2 +-
>  configs/sama5d29_curiosity_mmc_defconfig      |   2 +-
>  .../sama5d29_curiosity_qspiflash_defconfig    |   2 +-
>  configs/sama7g54_curiosity_mmc_defconfig      |   2 +-
>  .../sama7g54_curiosity_nandflash_defconfig    |   2 +-
>  .../sama7g54_curiosity_qspiflash_defconfig    |   2 +-
>  configs/sipeed_maix_bitm_defconfig            |   2 +-
>  configs/sipeed_maix_smode_defconfig           |   2 +-
>  configs/stemmy_defconfig                      |   2 +-
>  configs/stm32f429-discovery_defconfig         |   2 +-
>  configs/stm32f429-evaluation_defconfig        |   2 +-
>  configs/stm32f469-discovery_defconfig         |   2 +-
>  configs/stm32h743-disco_defconfig             |   2 +-
>  configs/stm32h743-eval_defconfig              |   2 +-
>  configs/stm32h750-art-pi_defconfig            |   2 +-
>  configs/stm32mp25_defconfig                   |   2 +-
>  configs/stmark2_defconfig                     |   2 +-
>  configs/th1520_lpi4a_defconfig                |   2 +-
>  configs/thunderx_88xx_defconfig               |   2 +-
>  configs/tools-only_defconfig                  |   2 +-
>  configs/topic_miami_defconfig                 |   2 +-
>  configs/topic_miamilite_defconfig             |   2 +-
>  configs/topic_miamiplus_defconfig             |   2 +-
>  configs/total_compute_defconfig               |   2 +-
>  configs/trats2_defconfig                      |   2 +-
>  configs/trats_defconfig                       |   2 +-
>  configs/xenguest_arm64_defconfig              |   2 +-
>  configs/xenguest_arm64_virtio_defconfig       |   2 +-
>  configs/xilinx_versal_mini_defconfig          |   2 +-
>  configs/xilinx_versal_mini_emmc0_defconfig    |   2 +-
>  configs/xilinx_versal_mini_emmc1_defconfig    |   2 +-
>  configs/xilinx_versal_mini_ospi_defconfig     |   2 +-
>  configs/xilinx_versal_mini_qspi_defconfig     |   2 +-
>  configs/xilinx_versal_net_mini_defconfig      |   2 +-
>  configs/xilinx_versal_net_mini_emmc_defconfig |   2 +-
>  configs/xilinx_versal_net_mini_ospi_defconfig |   2 +-
>  configs/xilinx_versal_net_mini_qspi_defconfig |   2 +-
>  configs/xilinx_zynqmp_mini_defconfig          |   2 +-
>  configs/xilinx_zynqmp_mini_emmc0_defconfig    |   2 +-
>  configs/xilinx_zynqmp_mini_emmc1_defconfig    |   2 +-
>  configs/xilinx_zynqmp_mini_nand_defconfig     |   2 +-
>  .../xilinx_zynqmp_mini_nand_single_defconfig  |   2 +-
>  configs/xilinx_zynqmp_mini_qspi_defconfig     |   2 +-
>  configs/zynq_cse_nand_defconfig               |   2 +-
>  configs/zynq_cse_nor_defconfig                |   2 +-
>  configs/zynq_cse_qspi_defconfig               |   2 +-
>  drivers/dfu/Kconfig                           |   1 +
>  drivers/fastboot/Kconfig                      |   1 +
>  drivers/mtd/cfi_flash.c                       |  36 +-
>  drivers/net/Kconfig                           |   3 +-
>  drivers/net/fec_mxc.c                         |   3 +-
>  drivers/net/phy/Kconfig                       |   2 +-
>  drivers/usb/gadget/Kconfig                    |   2 +-
>  include/flash.h                               |  20 +-
>  include/net-common.h                          | 413 ++++++++
>  include/net-legacy.h                          | 635 ++++++++++++
>  include/net-lwip.h                            |  37 +
>  include/net.h                                 | 944 +-----------------
>  lib/Makefile                                  |   2 +
>  lib/lwip/Makefile                             |  55 +
>  lib/lwip/lwip/src/apps/tftp/tftp.c            |  94 +-
>  .../lwip/src/include/lwip/apps/tftp_client.h  |   1 +
>  lib/lwip/u-boot/arch/cc.h                     |  43 +
>  lib/lwip/u-boot/arch/sys_arch.h               |   0
>  lib/lwip/u-boot/limits.h                      |   0
>  lib/lwip/u-boot/lwipopts.h                    | 157 +++
>  net/Kconfig                                   |  49 +-
>  net/Makefile                                  |  19 +-
>  net/eth-uclass.c                              |  38 +-
>  net/lwip/Kconfig                              |  34 +
>  net/lwip/Makefile                             |   8 +
>  net/lwip/dhcp.c                               | 136 +++
>  net/lwip/dns.c                                | 127 +++
>  net/lwip/eth_internal.h                       |  35 +
>  net/lwip/net-lwip.c                           | 292 ++++++
>  net/lwip/ping.c                               | 177 ++++
>  net/lwip/tftp.c                               | 276 +++++
>  net/lwip/wget.c                               | 272 +++++
>  157 files changed, 3311 insertions(+), 1321 deletions(-)
>  create mode 100644 cmd/net-common.c
>  create mode 100644 cmd/net-lwip.c
>  create mode 100644 configs/qemu_arm64_lwip_defconfig
>  create mode 100644 include/net-common.h
>  create mode 100644 include/net-legacy.h
>  create mode 100644 include/net-lwip.h
>  create mode 100644 lib/lwip/Makefile
>  create mode 100644 lib/lwip/u-boot/arch/cc.h
>  create mode 100644 lib/lwip/u-boot/arch/sys_arch.h
>  create mode 100644 lib/lwip/u-boot/limits.h
>  create mode 100644 lib/lwip/u-boot/lwipopts.h
>  create mode 100644 net/lwip/Kconfig
>  create mode 100644 net/lwip/Makefile
>  create mode 100644 net/lwip/dhcp.c
>  create mode 100644 net/lwip/dns.c
>  create mode 100644 net/lwip/eth_internal.h
>  create mode 100644 net/lwip/net-lwip.c
>  create mode 100644 net/lwip/ping.c
>  create mode 100644 net/lwip/tftp.c
>  create mode 100644 net/lwip/wget.c
>
> --
> 2.40.1
>
Tom Rini Aug. 7, 2024, 7:08 p.m. UTC | #14
On Wed, Aug 07, 2024 at 08:54:08PM +0200, Michael Nazzareno Trimarchi wrote:
> Hi Jerome
> 
> On Thu, Jul 25, 2024 at 2:58 PM Jerome Forissier
> <jerome.forissier@linaro.org> 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
> >
> > Notes:
> >
> > 1. A number of features are currently incompatible with NET_LWIP: SANDBOX,
> > DFU_TFTP, FASTBOOT, SPL_NET. All make assumptions on how the network
> > stack is implemented and/or pull sybols that are not trivially exported
> > from lwIP. Some interface rework may be needed.
> >
> > 2. Due to the above and in order to provide some level of testing, a new QEMU
> > configuration is introduced (qemu_arm64_lwip_defconfig) which is the same
> > as qemu_arm64_defconfig but with NET_LWIP and CMD_*_LWIP enabled.
> > Tests are added to test/py/tests/test_net.py for that configuration.
> >
> > 3. The default QEMU networking doesn't work with NET_LWIP. wget
> > pauses and resets the connection. Wireshark shows [TCP Window Full] and
> > [TCP ZeroWindow]. Wen using an emulated e1000 however all is fine
> > (that is "-nic tap,model=e1000" on the QEMU command line, with a bridge
> > configured on the host).
> >
> > Changes in v5:
> >
> > - Rebased on next
> > - Refactor Kconfig options to avoid duplicates
> > - Library functions use a more consistent naming (dhcp_loop(),
> > ping_loop() etc.) and take a struct udevice * parameter (Heinrich S.)
> > - Do not use net_process_receive_packet() for input anymore. Instead of
> > calling eth_rx() which would invoke net_process_receive_packet(), we
> > call a new net_lwip_rx(udev) function which invokes the device recv()
> > and pushes the packets up the lwIP stack. Thus everything is tied to
> > a udevice now. (Heinrich S.)
> > - Add "configs: replace '# CONFIG_NET is not set' with CONFIG_NO_NET=y"
> > (Tom R.)
> 
> Here I have some questions. You can have CONFIG_NET with two alternatives and
> the alternatives are exclusive. Why do we need a CONFIG_NO_NET definition?

For the platforms that today don't enable networking at all (of which
there are a small but reasonable number). Since it's a "choice"
statement we need a "none of the above" option.