mbox series

[v3,00/12] Introduce the lwIP network stack

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

Message

Jerome Forissier June 6, 2024, 1:35 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. (CC'd)
has 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

The first patch introduces a new Kconfig symbol: NET_LWIP, which selects
the lwIP implementation instead of the current one (NET). Contrary to the
approach chosen by Maxim in [1], NET_LWIP and NET cannot be enabled
simultaneously. The rationale is we want to start from a clean state and
not pull potentially duplicated functionality from both stacks. Note
however that a few files are still built in net/, they are the ones
related to ethernet device management and the ethernet bootflow.

The second patch introduces the Makefile to build lwIP when NET_LWIP is
enabled.

The subsequent patches implement various network-oriented commands and
features: dhcp, dns, ping, tftpboot, wget.

NET_LWIP is not enabled by default because it lacks functionality compared
to NET and many CI tests would fail to run or even build.

Some tests (dm dsa/eth) are disabled when NET_LWIP is selected because
they make strong assumptions on how the network stack is implemented and
how the packet flow occurs. For example, an ARP exchange is expected
when an ICMP packet goes out, but with lwIP no exchange will occur if the
host IP is already in the the ARP cache.

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_lwip_defconfig but with NET_LWIP and CMD_*_LWIP enabled.
Tests are added to test/py/tests/test_net.py for that configuration.

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>

Jerome Forissier (12):
  net: introduce alternative implementation as net-lwip/
  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-lwip: add wget command
  test: dm: dsa, eth: disable tests when CONFIG_NET_LWIP=y
  cmd: bdinfo: enable -e when CONFIG_CMD_NET_LWIP=y
  configs: add qemu_arm64_lwip_defconfig
  MAINTAINERS: net-lwip: add myself as a maintainer
  CI: add qemu_arm64_lwip to the test matrix

 .azure-pipelines.yml              |   7 +
 Kconfig                           |  40 ++++++
 MAINTAINERS                       |  11 ++
 Makefile                          |   3 +-
 boot/Kconfig                      |   5 +-
 cmd/Kconfig                       |  44 ++++++
 cmd/Makefile                      |   7 +
 cmd/bdinfo.c                      |   5 +-
 cmd/efidebug.c                    |   8 +-
 cmd/net-common.c                  | 115 +++++++++++++++
 cmd/net-lwip.c                    |  49 +++++++
 cmd/net.c                         | 115 ---------------
 common/Kconfig                    |   2 +-
 common/board_r.c                  |   4 +-
 common/spl/Kconfig                |   1 +
 configs/qemu_arm64_lwip_defconfig |   3 +
 drivers/dfu/Kconfig               |   2 +-
 drivers/fastboot/Kconfig          |   4 +-
 drivers/net/Kconfig               |   2 +-
 drivers/net/phy/Kconfig           |   2 +-
 drivers/usb/gadget/Kconfig        |   2 +-
 include/net-lwip.h                | 140 +++++++++++++++++++
 include/net.h                     |   2 +-
 lib/Makefile                      |   2 +
 lib/lwip/Makefile                 |  57 ++++++++
 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        | 197 ++++++++++++++++++++++++++
 net-lwip/Kconfig                  |  37 +++++
 net-lwip/Makefile                 |  18 +++
 net-lwip/dhcp.c                   | 114 +++++++++++++++
 net-lwip/dns.c                    | 107 ++++++++++++++
 net-lwip/eth_internal.h           |  35 +++++
 net-lwip/net-lwip.c               | 224 ++++++++++++++++++++++++++++++
 net-lwip/ping.c                   | 163 ++++++++++++++++++++++
 net-lwip/tftp.c                   | 209 ++++++++++++++++++++++++++++
 net-lwip/wget.c                   | 180 ++++++++++++++++++++++++
 net/Kconfig                       |  14 --
 test/dm/dsa.c                     |   2 +
 test/dm/eth.c                     |   4 +
 41 files changed, 1832 insertions(+), 147 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-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

Tom Rini June 6, 2024, 4:56 p.m. UTC | #1
On Thu, Jun 06, 2024 at 03:35:55PM +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. (CC'd)
> has 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

Alright, on Pi 3, I think we have more "output changed, test fails"
problems:
========================================== FAILURES ===========================================
_____________________________________ test_efi_fit_launch _____________________________________
test/py/tests/test_efi_fit.py:452: in test_efi_fit_launch
    launch_efi(False, False)
test/py/tests/test_efi_fit.py:394: in launch_efi
    net_set_up = net_dhcp()
test/py/tests/test_efi_fit.py:178: in net_dhcp
    assert 'DHCP client bound to address ' in output
E   AssertionError: assert 'DHCP client bound to address ' in 'Waiting for Ethernet connection... done.\r\neth0: smsc95xx_eth b8:27:eb:fc:64:a6 active'
------------------------------------ Captured stdout call -------------------------------------
U-Boot> usb start
U-Boot> U-Boot> setenv autoload no
U-Boot> U-Boot> dhcp
Waiting for Ethernet connection... done.
eth0: smsc95xx_eth b8:27:eb:fc:64:a6 active
U-Boot>
=================================== short test summary info ===================================

I'm going to skip that set of tests and see how far we get but I suspect
it'll keep failing on DHCP output being different.
Jerome Forissier June 7, 2024, 9:11 a.m. UTC | #2
On 6/6/24 18:56, Tom Rini wrote:
> On Thu, Jun 06, 2024 at 03:35:55PM +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. (CC'd)
>> has 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
> 
> Alright, on Pi 3, I think we have more "output changed, test fails"
> problems:
> ========================================== FAILURES ===========================================
> _____________________________________ test_efi_fit_launch _____________________________________
> test/py/tests/test_efi_fit.py:452: in test_efi_fit_launch
>     launch_efi(False, False)
> test/py/tests/test_efi_fit.py:394: in launch_efi
>     net_set_up = net_dhcp()
> test/py/tests/test_efi_fit.py:178: in net_dhcp
>     assert 'DHCP client bound to address ' in output
> E   AssertionError: assert 'DHCP client bound to address ' in 'Waiting for Ethernet connection... done.\r\neth0: smsc95xx_eth b8:27:eb:fc:64:a6 active'
> ------------------------------------ Captured stdout call -------------------------------------
> U-Boot> usb start
> U-Boot> U-Boot> setenv autoload no
> U-Boot> U-Boot> dhcp
> Waiting for Ethernet connection... done.
> eth0: smsc95xx_eth b8:27:eb:fc:64:a6 active
> U-Boot>
> =================================== short test summary info ===================================
> 
> I'm going to skip that set of tests and see how far we get but I suspect
> it'll keep failing on DHCP output being different.

As discussed offline with Ilias it might be a timeout issue, although 2000 ms
for DHCP_TIMEOUT_MS seems a large value to me, it might not be enough in your
environment. Can you please try to increase it? NET effectively has a timeout
of 28 seconds if we take the retries into account.

Thanks,
Tom Rini June 7, 2024, 1:52 p.m. UTC | #3
On Fri, Jun 07, 2024 at 11:11:49AM +0200, Jerome Forissier wrote:
> 
> 
> On 6/6/24 18:56, Tom Rini wrote:
> > On Thu, Jun 06, 2024 at 03:35:55PM +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. (CC'd)
> >> has 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
> > 
> > Alright, on Pi 3, I think we have more "output changed, test fails"
> > problems:
> > ========================================== FAILURES ===========================================
> > _____________________________________ test_efi_fit_launch _____________________________________
> > test/py/tests/test_efi_fit.py:452: in test_efi_fit_launch
> >     launch_efi(False, False)
> > test/py/tests/test_efi_fit.py:394: in launch_efi
> >     net_set_up = net_dhcp()
> > test/py/tests/test_efi_fit.py:178: in net_dhcp
> >     assert 'DHCP client bound to address ' in output
> > E   AssertionError: assert 'DHCP client bound to address ' in 'Waiting for Ethernet connection... done.\r\neth0: smsc95xx_eth b8:27:eb:fc:64:a6 active'
> > ------------------------------------ Captured stdout call -------------------------------------
> > U-Boot> usb start
> > U-Boot> U-Boot> setenv autoload no
> > U-Boot> U-Boot> dhcp
> > Waiting for Ethernet connection... done.
> > eth0: smsc95xx_eth b8:27:eb:fc:64:a6 active
> > U-Boot>
> > =================================== short test summary info ===================================
> > 
> > I'm going to skip that set of tests and see how far we get but I suspect
> > it'll keep failing on DHCP output being different.
> 
> As discussed offline with Ilias it might be a timeout issue, although 2000 ms
> for DHCP_TIMEOUT_MS seems a large value to me, it might not be enough in your
> environment. Can you please try to increase it? NET effectively has a timeout
> of 28 seconds if we take the retries into account.

I increased it to 20000 ms and no change.