Re: [PATCH v11 00/29] Introduce the lwIP network stack
On 10/4/24 20:30, Tom Rini wrote: > On Thu, Oct 03, 2024 at 05:22:46PM +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://github.com/lwip-tcpip/lwip.git STABLE-2_2_0_RELEASE > > So, assuming the outstanding questions raised on v11 are easy enough to > resolve, I think v12 should be posted with being merged in mind (and so > no TESTING patches, etc, since I'll "b4 shazam -SM" it. Thanks for > working on this so hard! Sure. Thanks for reviewing and testing! -- Jerome
Re: [PATCH v11 00/29] Introduce the lwIP network stack
On Thu, Oct 03, 2024 at 05:22:46PM +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://github.com/lwip-tcpip/lwip.git STABLE-2_2_0_RELEASE Tested-by: Tom Rini # Raspberry Pi 3 (32b, 64b, arm64), Raspberry Pi 4 (32b, 64b, arm64), am64x_evm, am62x_evm, am62x_beagleplay. And the Pi targets were built with gcc-13 and llvm-17. -- Tom signature.asc Description: PGP signature
Re: [PATCH v11 00/29] Introduce the lwIP network stack
Tom On Fri, 4 Oct 2024 at 21:30, Tom Rini wrote: > > On Thu, Oct 03, 2024 at 05:22:46PM +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://github.com/lwip-tcpip/lwip.git STABLE-2_2_0_RELEASE > > So, assuming the outstanding questions raised on v11 are easy enough to > resolve, I think v12 should be posted with being merged in mind (and so > no TESTING patches, etc, since I'll "b4 shazam -SM" it. Thanks for > working on this so hard! FWIW I am generally happy with the whole series. Unfortunately I dont have time to do a full detailed review, but we've fixed enough to be confident it's 'ok' to pull. I think we'll improve it faster over time once people start using it Thanks /Ilias > > -- > Tom
Re: [PATCH v11 00/29] Introduce the lwIP network stack
On Thu, Oct 03, 2024 at 05:22:46PM +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://github.com/lwip-tcpip/lwip.git STABLE-2_2_0_RELEASE So, assuming the outstanding questions raised on v11 are easy enough to resolve, I think v12 should be posted with being merged in mind (and so no TESTING patches, etc, since I'll "b4 shazam -SM" it. Thanks for working on this so hard! -- Tom signature.asc Description: PGP signature
Re: [PATCH v11 00/29] Introduce the lwIP network stack
On Thu, 3 Oct 2024 at 19:10, Jerome Forissier wrote: > > > > On 10/3/24 18:00, Ilias Apalodimas wrote: > > Hi Jerome, > > > > I'll have a look, but there seems to be reviewed-by and acked-by tags > > missing? > > Any chance you lost those during rebasing ? > > I only dropped your R-b from the MAINTAINERS patch because I changed it > slightly. > Now I see you Acked it so that's good :) Ah thanks, yea I saw that was missing and assumed more would be dropped. Ack is more appropriate in this case, so let's preserve that Thanks /Ilias > > -- > Jerome > > > > > Thanks > > /Ilias > > On Thu, 3 Oct 2024 at 18:23, 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://github.com/lwip-tcpip/lwip.git STABLE-2_2_0_RELEASE > >> > >> Notes > >> > >> 1. A number of features are currently incompatible with NET_LWIP: > >> DFU_TFTP, FASTBOOT, SPL_NET, ETH_SANDBOX, ETH_SANDBOX_RAW, DM_ETH. They > >> 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 of the > >> lwIP code in CI even when the legacy NET is the default, a new QEMU > >> configuration is introduced (qemu_arm64_lwip_defconfig) which is > >> based on qemu_arm64_defconfig with NET_LWIP and CMD_*_LWIP enabled. > >> In addition to that, this series has some [TESTING] patches > >> which make NET_LWIP the default. > >> > >> [1] > >> https://lore.kernel.org/all/20231127125726.3735-1-maxim.uva...@linaro.org/ > >> [2] https://www.nongnu.org/lwip/ > >> [3] https://en.wikipedia.org/wiki/LwIP > >> > >> Changes in v11: > >> > >> - Rebased onto next > >> - "Miscellaneous fixes" removed (patches were merged in next). The > >> series still begins with small fixes posted separately [1] [2] [3]. > >> - Add (some) support for CMD_PXE and therefore drop patch "[TESTING] > >> configs: set CONFIG_NET=y when PXE is enabled". This is build-tested > >> only and it is very likely that some work is needed to make it useful. > >> For example, adding the code for DHCP option 209 to lwIP so that > >> BOOTP_PXE_DHCP_OPTION can be supported. > >> - SANDBOX is now supported, but with the dm eth and wget tests > >> disabled. > >> - Move eth_set_enable_bootdevs() declaration to net-common.h > >> Fixes warning with snow_defconfig: > >> test/test-main.c:310:17: warning: implicit declaration of function > >> ‘eth_set_enable_bootdevs’ [] > >> - Do eth_init() and eth_init_rings() only once, and do not forget > >> lwip_init()! Fixes TFTP stalls on TI K3 (reported by Tom R. and > >> tested by Ilias A.) > >> - Set MEM_ALIGNMENT to 8 in lib/lwip/u-boot/lwipopts.h. Fixes TFTP > >> random crashes on TI K3 (Ilias A.) > >> - net_lwip_rx(): call free_pkt() even when recv() has returned 0, > >> as required by the driver model documentation (and imx8mp_evk). Fixes > >> a regression introduced in v9. Goes together with patch [2]. > >> - Add "lwip: tftp: bind to TFTP port only when in server mode" to > >> fix an issue with interrupted tftp commands (the tftp command hangs > >> if it is interrupted with Ctrl-C and started again). > >> - TFTP: fix uninitialized "ret" variable in do_tftpb(); print "Abort" > >> on Ctrl-C. > >> - MAINTAINERS: remove README and add sandbox ethernet driver to the > >> list of maintained files. > >> - AFAICT, CI should be all good except qemu_xtensa_dc233c which is > >> broken when NET_LWIP=y (QEMU just hangs with no output). I could > >> not find or build a suitable GDB binary to debug that. > >> > >> [1] > >> https://patchwork.ozlabs.org/project/uboot/
Re: [PATCH v11 00/29] Introduce the lwIP network stack
On 10/3/24 18:00, Ilias Apalodimas wrote: > Hi Jerome, > > I'll have a look, but there seems to be reviewed-by and acked-by tags missing? > Any chance you lost those during rebasing ? I only dropped your R-b from the MAINTAINERS patch because I changed it slightly. Now I see you Acked it so that's good :) -- Jerome > > Thanks > /Ilias > On Thu, 3 Oct 2024 at 18:23, 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://github.com/lwip-tcpip/lwip.git STABLE-2_2_0_RELEASE >> >> Notes >> >> 1. A number of features are currently incompatible with NET_LWIP: >> DFU_TFTP, FASTBOOT, SPL_NET, ETH_SANDBOX, ETH_SANDBOX_RAW, DM_ETH. They >> 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 of the >> lwIP code in CI even when the legacy NET is the default, a new QEMU >> configuration is introduced (qemu_arm64_lwip_defconfig) which is >> based on qemu_arm64_defconfig with NET_LWIP and CMD_*_LWIP enabled. >> In addition to that, this series has some [TESTING] patches >> which make NET_LWIP the default. >> >> [1] >> https://lore.kernel.org/all/20231127125726.3735-1-maxim.uva...@linaro.org/ >> [2] https://www.nongnu.org/lwip/ >> [3] https://en.wikipedia.org/wiki/LwIP >> >> Changes in v11: >> >> - Rebased onto next >> - "Miscellaneous fixes" removed (patches were merged in next). The >> series still begins with small fixes posted separately [1] [2] [3]. >> - Add (some) support for CMD_PXE and therefore drop patch "[TESTING] >> configs: set CONFIG_NET=y when PXE is enabled". This is build-tested >> only and it is very likely that some work is needed to make it useful. >> For example, adding the code for DHCP option 209 to lwIP so that >> BOOTP_PXE_DHCP_OPTION can be supported. >> - SANDBOX is now supported, but with the dm eth and wget tests >> disabled. >> - Move eth_set_enable_bootdevs() declaration to net-common.h >> Fixes warning with snow_defconfig: >> test/test-main.c:310:17: warning: implicit declaration of function >> ‘eth_set_enable_bootdevs’ [] >> - Do eth_init() and eth_init_rings() only once, and do not forget >> lwip_init()! Fixes TFTP stalls on TI K3 (reported by Tom R. and >> tested by Ilias A.) >> - Set MEM_ALIGNMENT to 8 in lib/lwip/u-boot/lwipopts.h. Fixes TFTP >> random crashes on TI K3 (Ilias A.) >> - net_lwip_rx(): call free_pkt() even when recv() has returned 0, >> as required by the driver model documentation (and imx8mp_evk). Fixes >> a regression introduced in v9. Goes together with patch [2]. >> - Add "lwip: tftp: bind to TFTP port only when in server mode" to >> fix an issue with interrupted tftp commands (the tftp command hangs >> if it is interrupted with Ctrl-C and started again). >> - TFTP: fix uninitialized "ret" variable in do_tftpb(); print "Abort" >> on Ctrl-C. >> - MAINTAINERS: remove README and add sandbox ethernet driver to the >> list of maintained files. >> - AFAICT, CI should be all good except qemu_xtensa_dc233c which is >> broken when NET_LWIP=y (QEMU just hangs with no output). I could >> not find or build a suitable GDB binary to debug that. >> >> [1] >> https://patchwork.ozlabs.org/project/uboot/patch/20241002144845.1439316-1-jerome.foriss...@linaro.org/ >> [2] >> https://patchwork.ozlabs.org/project/uboot/patch/20240927142038.879037-1-jerome.foriss...@linaro.org/ >> [3] >> https://patchwork.ozlabs.org/project/uboot/patch/20241003142030.1610222-1-jerome.foriss...@linaro.org/ >> >> Changes in v10: >> >> - Rebase onto next >> - URL for lwIP changed in cover letter: using GitHub now since all >> tags h
Re: [PATCH v11 00/29] Introduce the lwIP network stack
Hi Jerome,
I'll have a look, but there seems to be reviewed-by and acked-by tags missing?
Any chance you lost those during rebasing ?
Thanks
/Ilias
On Thu, 3 Oct 2024 at 18:23, 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://github.com/lwip-tcpip/lwip.git STABLE-2_2_0_RELEASE
>
> Notes
>
> 1. A number of features are currently incompatible with NET_LWIP:
> DFU_TFTP, FASTBOOT, SPL_NET, ETH_SANDBOX, ETH_SANDBOX_RAW, DM_ETH. They
> 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 of the
> lwIP code in CI even when the legacy NET is the default, a new QEMU
> configuration is introduced (qemu_arm64_lwip_defconfig) which is
> based on qemu_arm64_defconfig with NET_LWIP and CMD_*_LWIP enabled.
> In addition to that, this series has some [TESTING] patches
> which make NET_LWIP the default.
>
> [1] https://lore.kernel.org/all/20231127125726.3735-1-maxim.uva...@linaro.org/
> [2] https://www.nongnu.org/lwip/
> [3] https://en.wikipedia.org/wiki/LwIP
>
> Changes in v11:
>
> - Rebased onto next
> - "Miscellaneous fixes" removed (patches were merged in next). The
> series still begins with small fixes posted separately [1] [2] [3].
> - Add (some) support for CMD_PXE and therefore drop patch "[TESTING]
> configs: set CONFIG_NET=y when PXE is enabled". This is build-tested
> only and it is very likely that some work is needed to make it useful.
> For example, adding the code for DHCP option 209 to lwIP so that
> BOOTP_PXE_DHCP_OPTION can be supported.
> - SANDBOX is now supported, but with the dm eth and wget tests
> disabled.
> - Move eth_set_enable_bootdevs() declaration to net-common.h
> Fixes warning with snow_defconfig:
> test/test-main.c:310:17: warning: implicit declaration of function
> ‘eth_set_enable_bootdevs’ []
> - Do eth_init() and eth_init_rings() only once, and do not forget
> lwip_init()! Fixes TFTP stalls on TI K3 (reported by Tom R. and
> tested by Ilias A.)
> - Set MEM_ALIGNMENT to 8 in lib/lwip/u-boot/lwipopts.h. Fixes TFTP
> random crashes on TI K3 (Ilias A.)
> - net_lwip_rx(): call free_pkt() even when recv() has returned 0,
> as required by the driver model documentation (and imx8mp_evk). Fixes
> a regression introduced in v9. Goes together with patch [2].
> - Add "lwip: tftp: bind to TFTP port only when in server mode" to
> fix an issue with interrupted tftp commands (the tftp command hangs
> if it is interrupted with Ctrl-C and started again).
> - TFTP: fix uninitialized "ret" variable in do_tftpb(); print "Abort"
> on Ctrl-C.
> - MAINTAINERS: remove README and add sandbox ethernet driver to the
> list of maintained files.
> - AFAICT, CI should be all good except qemu_xtensa_dc233c which is
> broken when NET_LWIP=y (QEMU just hangs with no output). I could
> not find or build a suitable GDB binary to debug that.
>
> [1]
> https://patchwork.ozlabs.org/project/uboot/patch/20241002144845.1439316-1-jerome.foriss...@linaro.org/
> [2]
> https://patchwork.ozlabs.org/project/uboot/patch/20240927142038.879037-1-jerome.foriss...@linaro.org/
> [3]
> https://patchwork.ozlabs.org/project/uboot/patch/20241003142030.1610222-1-jerome.foriss...@linaro.org/
>
> Changes in v10:
>
> - Rebase onto next
> - URL for lwIP changed in cover letter: using GitHub now since all
> tags have suddenly disappeared from the repository on gnu.org.
> - Post fixes as a separate series [1] or individual patches [2] [3]
> - Add "if NET_LWIP" to net/lwip/Kconfig to fix a kconfig warning
> when doing the full branch build with buildman ("[NEW]")
> - net/ is added to libs-y only when NET o
