mbox series

[v2,00/14] Introduce the lwIP network stack

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

Message

Jerome Forissier May 24, 2024, 4:19 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. directly from the U-Boot shell.
- Possibly benefit from additional features implemented in lwIP
- Less code to maintain in U-Boot

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 imports the lwIP code as a Git subtree under
lib/lwip/lwip. Some glue code is added under lib/lwip/u-boot.

The third patch introduces the Makefile to build lwIP when
CONFIG_NET_LWIP=y.

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 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 (14):
  net: introduce alternative implementation as net-lwip/
  Squashed 'lib/lwip/lwip/' content from commit 0a0452b2c39
  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
  test/py: net: add _lwip variants of dhcp, ping and tftpboot tests
  MAINTAINERS: net-lwip: add myself as a maintainer
  CI: add qemu_arm64_lwip to the test matrix

Comments

Francesco Dolcini May 27, 2024, 9:23 a.m. UTC | #1
Hello Jerome,

On Fri, May 24, 2024 at 06:19:54PM +0200, Jerome Forissier wrote:
> - 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. directly from the U-Boot shell.

Why this is enabling this use case? Or it is just that currently,
without TLS, is not supposed to be something you should do?
I am a little bit confused reading this sentence, since to me this is
already possible using tftp.

Francesco
Jerome Forissier May 27, 2024, 9:36 a.m. UTC | #2
Hi Francesco,


On 5/27/24 11:23, Francesco Dolcini wrote:
> Hello Jerome,
> 
> On Fri, May 24, 2024 at 06:19:54PM +0200, Jerome Forissier wrote:
>> - 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. directly from the U-Boot shell.
> 
> Why this is enabling this use case? Or it is just that currently,
> without TLS, is not supposed to be something you should do?
> I am a little bit confused reading this sentence, since to me this is
> already possible using tftp.

You're correct. The point I am making is about using a secure
(authenticated) connection, and I should have clarified that. While using
HTTPS might not be critical on a local network, things are different when
downloading from the internet (think man-in-the-middle attacks). Also,
many web sites are switching off HTTP in favor of HTTPS so it could be
that your favorite distro becomes unavailable without it.

Thanks,
Martin Husemann May 27, 2024, 9:45 a.m. UTC | #3
On Mon, May 27, 2024 at 11:36:26AM +0200, Jerome Forissier wrote:
> You're correct. The point I am making is about using a secure
> (authenticated) connection, and I should have clarified that. While using
> HTTPS might not be critical on a local network, things are different when
> downloading from the internet (think man-in-the-middle attacks).

(Sorry if this sounds like nitpkicking, but I am genuinely curious)
How is it supposed to work?

You need not only https but also verify the presented certificate chain,
and for that you need up-to-date root certificates (e.g. the bundle
available from mozilla).

This sounds a bit outside the scope of u-boot to me (or you should 
avoid the man-in-the-middle argument, which leaves the still valid
"sites stop offering plain http" argument).

If you really worry about man-in-the-middle you need to download via
https in an environment that does certificate validation, and then
even better verify the hash of the downloaded image. After that you
can offer the image locally - via http, https or tftp - for installations.

Martin
Jerome Forissier May 27, 2024, 12:45 p.m. UTC | #4
On 5/27/24 11:45, Martin Husemann wrote:
> On Mon, May 27, 2024 at 11:36:26AM +0200, Jerome Forissier wrote:
>> You're correct. The point I am making is about using a secure
>> (authenticated) connection, and I should have clarified that. While using
>> HTTPS might not be critical on a local network, things are different when
>> downloading from the internet (think man-in-the-middle attacks).
> 
> (Sorry if this sounds like nitpkicking, but I am genuinely curious)

No worries. Well-defined use cases are certainly essential to
proper implementations.

> How is it supposed to work?
> 
> You need not only https but also verify the presented certificate chain,
> and for that you need up-to-date root certificates (e.g. the bundle
> available from mozilla).

Yes. I will let Javier comment further since he's more directly
concerned with that part.

> This sounds a bit outside the scope of u-boot to me

Maybe, maybe not. I would argue it is a matter of deployment policy.
For example assume the device is an IP camera or some other IoT thing
that comes with the bare minimum to boot and fetch its OS on first
power up. No guarantee about what is on the local network -- just
plain internet access. https would be quite helpful in such a case.

> (or you should 
> avoid the man-in-the-middle argument, which leaves the still valid
> "sites stop offering plain http" argument).

As long as there is at least one valid argument then I'm fine :)

> If you really worry about man-in-the-middle you need to download via
> https in an environment that does certificate validation, and then
> even better verify the hash of the downloaded image. After that you
> can offer the image locally - via http, https or tftp - for installations.

That certainly would work but again, isn't it a decision to be made
by the device manufacturer or more generally the one who builds u-boot
for the device?
Martin Husemann May 27, 2024, 12:47 p.m. UTC | #5
On Mon, May 27, 2024 at 02:45:01PM +0200, Jerome Forissier wrote:
> That certainly would work but again, isn't it a decision to be made
> by the device manufacturer or more generally the one who builds u-boot
> for the device?

Very true, and if you get the verification part working, your full solution
is better!

Martin
Tom Rini May 27, 2024, 3:34 p.m. UTC | #6
On Fri, May 24, 2024 at 06:19:54PM +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. directly from the U-Boot shell.
> - Possibly benefit from additional features implemented in lwIP
> - Less code to maintain in U-Boot
> 
> 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 imports the lwIP code as a Git subtree under
> lib/lwip/lwip. Some glue code is added under lib/lwip/u-boot.

For next time, please just make it a pre-req to run the git subtree
command (and note it in the cover letter).

In more specific feedback, I tried this on a Pi 3, and:
U-Boot> tftpboot 200000 EFI/arm64/helloworld.efi
TFTP from server 192.168.1.10; our IP address is 192.168.1.100
Filename 'EFI/arm64/helloworld.efi'.
Load address: 0x200000
Loading:
....
FAILED test/py/tests/test_efi_loader.py::test_efi_helloworld_net - u_boot_spawn.Timeout

So some amount of networking is working (that's a reasonable dhcp
response it got), but tftp'ing a file fails.
Ilias Apalodimas May 28, 2024, 11:50 a.m. UTC | #7
Hi all

On Mon, 27 May 2024 at 15:47, Martin Husemann <martin@netbsd.org> wrote:
>
> On Mon, May 27, 2024 at 02:45:01PM +0200, Jerome Forissier wrote:
> > That certainly would work but again, isn't it a decision to be made
> > by the device manufacturer or more generally the one who builds u-boot
> > for the device?
>
> Very true, and if you get the verification part working, your full solution
> is better!

Just to chime in a bit on the why. HTTPs is one of the options we can
easily plug in with LWIP & mbedTLS. But apart from that, getting a
richer TCP/IP stack that has been used for years in RTOSes is a win on
its own.

We can now easily add multiple net-based applications without having
to implement the underlying TCP/IP stack.  LWIP is a very slight
increase in size (~1%) and the advantages are well worth it.

Cheers
/Ilias
>
> Martin
Peter Robinson June 4, 2024, 11:13 p.m. UTC | #8
> On Fri, May 24, 2024 at 06:19:54PM +0200, Jerome Forissier wrote:
> > - 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. directly from the U-Boot shell.
>
> Why this is enabling this use case? Or it is just that currently,
> without TLS, is not supposed to be something you should do?
> I am a little bit confused reading this sentence, since to me this is
> already possible using tftp.

A lot of companies don't want to run tftp any more when http(s) is
more straight forward, especially when used outside of the data
centre, it can be cached/proxied etc and has a bunch of other benefits
before you even get to things like security and auth. Also a lot of FW
vendors are starting to drop PXE/TFTP support in standards due to
various things like standards requirements from governments.

Peter
Jerome Forissier June 6, 2024, 9:15 a.m. UTC | #9
On 5/27/24 17:34, Tom Rini wrote:
> On Fri, May 24, 2024 at 06:19:54PM +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. directly from the U-Boot shell.
>> - Possibly benefit from additional features implemented in lwIP
>> - Less code to maintain in U-Boot
>>
>> 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 imports the lwIP code as a Git subtree under
>> lib/lwip/lwip. Some glue code is added under lib/lwip/u-boot.
> 
> For next time, please just make it a pre-req to run the git subtree
> command (and note it in the cover letter).
> 
> In more specific feedback, I tried this on a Pi 3, and:
> U-Boot> tftpboot 200000 EFI/arm64/helloworld.efi
> TFTP from server 192.168.1.10; our IP address is 192.168.1.100
> Filename 'EFI/arm64/helloworld.efi'.
> Load address: 0x200000
> Loading:
> ....
> FAILED test/py/tests/test_efi_loader.py::test_efi_helloworld_net - u_boot_spawn.Timeout
> 
> So some amount of networking is working (that's a reasonable dhcp
> response it got), but tftp'ing a file fails.

Thanks for testing! This fails because I changed the command output. The
legacy command prints "Bytes transferred =" while the newer one prints
"%lu bytes transferred" plus throughput information. I will use the older
message in v3 because several tests as well as documentation would need
updating. We'll do that later if desired.

Thanks,
Tom Rini June 6, 2024, 2:25 p.m. UTC | #10
On Thu, Jun 06, 2024 at 11:15:54AM +0200, Jerome Forissier wrote:
> 
> 
> On 5/27/24 17:34, Tom Rini wrote:
> > On Fri, May 24, 2024 at 06:19:54PM +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. directly from the U-Boot shell.
> >> - Possibly benefit from additional features implemented in lwIP
> >> - Less code to maintain in U-Boot
> >>
> >> 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 imports the lwIP code as a Git subtree under
> >> lib/lwip/lwip. Some glue code is added under lib/lwip/u-boot.
> > 
> > For next time, please just make it a pre-req to run the git subtree
> > command (and note it in the cover letter).
> > 
> > In more specific feedback, I tried this on a Pi 3, and:
> > U-Boot> tftpboot 200000 EFI/arm64/helloworld.efi
> > TFTP from server 192.168.1.10; our IP address is 192.168.1.100
> > Filename 'EFI/arm64/helloworld.efi'.
> > Load address: 0x200000
> > Loading:
> > ....
> > FAILED test/py/tests/test_efi_loader.py::test_efi_helloworld_net - u_boot_spawn.Timeout
> > 
> > So some amount of networking is working (that's a reasonable dhcp
> > response it got), but tftp'ing a file fails.
> 
> Thanks for testing! This fails because I changed the command output. The
> legacy command prints "Bytes transferred =" while the newer one prints
> "%lu bytes transferred" plus throughput information. I will use the older
> message in v3 because several tests as well as documentation would need
> updating. We'll do that later if desired.

Thanks. And yeah, I would like to keep the output unchanged as much as
is feasible, it's in many ways a form of ABI.