
On Thu, 3 Aug 2023 at 03:31, Simon Glass sjg@google.com wrote:
Hi Maxim,
On Wed, 2 Aug 2023 at 08:09, Maxim Uvarov maxim.uvarov@linaro.org wrote:
Please can you add a commit message in each case?
lib/Kconfig | 2 ++ lib/Makefile | 3 +++ lib/lwip/Kconfig | 67 +++++++++++++++++++++++++++++++++++++++++++++++ lib/lwip/Makefile | 66 ++++++++++++++++++++++++++++++++++++++++++++++ net/Kconfig | 1 + net/net.c | 24 +++++++++++++++++ 6 files changed, 163 insertions(+) create mode 100644 lib/lwip/Kconfig create mode 100644 lib/lwip/Makefile
diff --git a/lib/Kconfig b/lib/Kconfig index 3926652db6..79f7d5bc5d 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -1112,3 +1112,5 @@ menu "FWU Multi Bank Updates" source lib/fwu_updates/Kconfig
endmenu
+source lib/lwip/Kconfig diff --git a/lib/Makefile b/lib/Makefile index 8d8ccc8bbc..598b5755dd 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -11,6 +11,7 @@ obj-$(CONFIG_EFI_LOADER) += efi_loader/ obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += efi_selftest/ obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_updates/ obj-$(CONFIG_LZMA) += lzma/ +obj-$(CONFIG_LWIP_LIB) += lwip/
Do we need the _LIB ?
Yea, _LIB can be removed.
obj-$(CONFIG_BZIP2) += bzip2/ obj-$(CONFIG_FIT) += libfdt/ obj-$(CONFIG_OF_LIVE) += of_live.o @@ -92,6 +93,8 @@ obj-$(CONFIG_LIBAVB) += libavb/ obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += libfdt/ obj-$(CONFIG_$(SPL_TPL_)OF_REAL) += fdtdec_common.o fdtdec.o
+obj-y += lwip/
ifdef CONFIG_SPL_BUILD obj-$(CONFIG_SPL_YMODEM_SUPPORT) += crc16-ccitt.o obj-$(CONFIG_$(SPL_TPL_)HASH) += crc16-ccitt.o diff --git a/lib/lwip/Kconfig b/lib/lwip/Kconfig new file mode 100644 index 0000000000..5d2603701c --- /dev/null +++ b/lib/lwip/Kconfig @@ -0,0 +1,67 @@ +menu "LWIP" +config LWIP_LIB
bool "Support LWIP library"
help
This option will enable the lwIP library code with
s/will enable/enables/
or even
s/This option will enable/Enable /
all dependencies (cmd commands implemented with lwIP
library. This option is automatically enabled if CONFIG_NET=y.
lwIP library (https://git.savannah.nongnu.org/git/lwip.git)
provides
network stack and application code for U-Boot cmd commands.
Please see doc/... for more information
+menu "LWIP options"
+config LWIP_LIB_DEBUG
bool "enable debug"
default n
+config LWIP_LIB_NOASSERT
bool "disable asserts"
default y
help
Disabling asserts reduces binary size on 16k.
by 16K ?
+config LWIP_LIB_TCP
bool "tcp"
default y
+config LWIP_LIB_UDP
bool "udp"
default y
need help
+config LWIP_LIB_DNS
bool "dns"
default y
+config LWIP_LIB_DHCP
bool "dhcp"
default y
+config LWIP_LIB_LOOPBACK
bool "loopback"
help
Increases size on 1k.
by 1K
+config LWIP_LIB_SOCKET
bool "socket API"
+config LWIP_LIB_NETCONN
bool "netconn API"
+config LWIP_LIB_MEM_SIZE
int "mem size"
default 1600
range 1 4096
help
MEM_SIZE: the size of the heap memory. If the application
will send
a lot of data that needs to be copied, this should be set
high.
Can you add more detail? What does it mean to copy data??
Actually currently this option is not used. There are 2 ways to define mem pool memory: 1. MEM_USE_POOLS (place on the stack). 2. use malloc (or use lwip variant of malloc with alignments.). Now I use malloc to simplify current integration. But I expect then later we will want to optimize for speed and need some dma friendly memory and will use MEM_USE_POOLS. To simplify settings I think it's reasonable to drop it for now from Kconfig.
+config LWIP_LIB_PBUF_LINK_HLEN
int "pbuf link hlen"
default 14
range 4 1024
help
PBUF_LINK_HLEN: the number of bytes that should be allocated
for a
link level header. The default is 14, the standard value for
Ethernet.
Why would you change it? Please add a little more detail
I think I will remove it from here. I tried to add more options to Kconfig to customize lwip and that was really helpful to measure size. This exact option can be 14 or 16 or 18, according to other code and examples. I think we don't need to change this value in our config. And then less configuration numbers we will have, then it's better for us.
/**
* PBUF_LINK_HLEN: the number of bytes that should be allocated for a
* link level header. The default is 14, the standard value for
* Ethernet.
*/
#if !defined PBUF_LINK_HLEN || defined __DOXYGEN__
#if (defined LWIP_HOOK_VLAN_SET || LWIP_VLAN_PCP) && !defined __DOXYGEN__
#define PBUF_LINK_HLEN (18 + ETH_PAD_SIZE)
#else /* LWIP_HOOK_VLAN_SET || LWIP_VLAN_PCP */
#define PBUF_LINK_HLEN (14 + ETH_PAD_SIZE)
#endif /* LWIP_HOOK_VLAN_SET || LWIP_VLAN_PCP */
#endif
+endmenu
+endmenu diff --git a/lib/lwip/Makefile b/lib/lwip/Makefile new file mode 100644 index 0000000000..35f34d7afa --- /dev/null +++ b/lib/lwip/Makefile @@ -0,0 +1,66 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# (C) Copyright 2023 Linaro Ltd. maxim.uvarov@linaro.org
+LWIPDIR=lwip-external/src
+ccflags-y += -I$(srctree)/lib/lwip/port/include +ccflags-y += -I$(srctree)/lib/lwip/lwip-external/src/include
-I$(srctree)/lib/lwip
+obj-$(CONFIG_NET) += $(LWIPDIR)/core/init.o \
$(LWIPDIR)/core/def.o \
$(LWIPDIR)/core/dns.o \
$(LWIPDIR)/core/inet_chksum.o \
$(LWIPDIR)/core/ip.o \
$(LWIPDIR)/core/mem.o \
$(LWIPDIR)/core/memp.o \
$(LWIPDIR)/core/netif.o \
$(LWIPDIR)/core/pbuf.o \
$(LWIPDIR)/core/raw.o \
$(LWIPDIR)/core/stats.o \
$(LWIPDIR)/core/sys.o \
$(LWIPDIR)/core/altcp.o \
$(LWIPDIR)/core/altcp_alloc.o \
$(LWIPDIR)/core/altcp_tcp.o \
$(LWIPDIR)/core/tcp.o \
$(LWIPDIR)/core/tcp_in.o \
$(LWIPDIR)/core/tcp_out.o \
$(LWIPDIR)/core/timeouts.o \
$(LWIPDIR)/core/udp.o
+# IPv4 +obj-$(CONFIG_NET) += $(LWIPDIR)/core/ipv4/acd.o \
$(LWIPDIR)/core/ipv4/autoip.o \
$(LWIPDIR)/core/ipv4/dhcp.o \
$(LWIPDIR)/core/ipv4/etharp.o \
$(LWIPDIR)/core/ipv4/icmp.o \
$(LWIPDIR)/core/ipv4/igmp.o \
$(LWIPDIR)/core/ipv4/ip4_frag.o \
$(LWIPDIR)/core/ipv4/ip4.o \
$(LWIPDIR)/core/ipv4/ip4_addr.o
+# IPv6 +obj-$(CONFIG_NET) += $(LWIPDIR)/core/ipv6/dhcp6.o \
$(LWIPDIR)/core/ipv6/ethip6.o \
$(LWIPDIR)/core/ipv6/icmp6.o \
$(LWIPDIR)/core/ipv6/inet6.o \
$(LWIPDIR)/core/ipv6/ip6.o \
$(LWIPDIR)/core/ipv6/ip6_addr.o \
$(LWIPDIR)/core/ipv6/ip6_frag.o \
$(LWIPDIR)/core/ipv6/mld6.o \
$(LWIPDIR)/core/ipv6/nd6.o
+# API +obj-$(CONFIG_NET) += $(LWIPDIR)/api/api_lib.o \
$(LWIPDIR)/api/api_msg.o \
$(LWIPDIR)/api/err.o \
$(LWIPDIR)/api/if_api.o \
$(LWIPDIR)/api/netbuf.o \
$(LWIPDIR)/api/netdb.o \
$(LWIPDIR)/api/netifapi.o \
$(LWIPDIR)/api/sockets.o \
$(LWIPDIR)/api/tcpip.o
+# Netdevs +obj-$(CONFIG_NET) += $(LWIPDIR)/netif/ethernet.o
+obj-$(CONFIG_NET) += port/if.o +obj-$(CONFIG_NET) += port/sys-arch.o diff --git a/net/Kconfig b/net/Kconfig index 4215889127..c3f4a7cae7 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -5,6 +5,7 @@ menuconfig NET bool "Networking support" default y
select LWIP_LIB
I agree this is best in the long term, but 'imply' might be safer until we are happy to delete the old code.
I agree.
if NET
diff --git a/net/net.c b/net/net.c index 43abbac7c3..d98e51cb80 100644 --- a/net/net.c +++ b/net/net.c @@ -125,6 +125,7 @@ #endif #include "dhcpv6.h" #include "net_rand.h" +#include "../lib/lwip/ulwip.h"
/** BOOTP EXTENTIONS **/
@@ -452,7 +453,11 @@ int net_loop(enum proto_t protocol) #endif
bootstage_mark_name(BOOTSTAGE_ID_ETH_START, "eth_start");
+#if defined(CONFIG_LWIP_LIB)
if (!ulwip_enabled() || !ulwip_in_loop())
+#endif net_init();
if (eth_is_on_demand_init()) { eth_halt(); eth_set_current();
@@ -649,6 +654,18 @@ restart: */ eth_rx();
+#if defined(CONFIG_LWIP_LIB)
if ()
if (ulwip_enabled()) {
net_set_state(NETLOOP_CONTINUE);
if (!ulwip_in_loop()) {
if (ulwip_app_get_err())
net_set_state(NETLOOP_FAIL);
else
net_set_state(NETLOOP_SUCCESS);
goto done;
}
}
+#endif /* * Abort if ctrl-c was pressed. */ @@ -1213,6 +1230,13 @@ void net_process_received_packet(uchar
*in_packet, int len)
if (len < ETHER_HDR_SIZE) return;
+#if defined(CONFIG_LWIP_LIB)
same
if (ulwip_enabled()) {
uboot_lwip_poll();
do we need the uboot_ prefix?
lwip has lwip_ prefix. U-Boot doesn't have a prefix. I named the connecting code between U-Boot and lwip as ulwip_ for several functions. Here it's also has to be ulwip_
return;
}
+#endif
#if defined(CONFIG_API) || defined(CONFIG_EFI_LOADER) if (push_packet) { (*push_packet)(in_packet, len); -- 2.30.2
Regards, Simon
Other comments are valuable, thanks, fixed.