| Message ID | 20230802140658.10319-10-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | net/lwip: add lwip library for the network stack | expand |
Hi Maxim, On Wed, 2 Aug 2023 at 08:11, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > subject: U-Boot commit message please (throughout series) > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > lib/lwip/port/if.c | 256 ++++++++++++++++++++++++++ > lib/lwip/port/include/arch/cc.h | 46 +++++ > lib/lwip/port/include/arch/sys_arch.h | 59 ++++++ > lib/lwip/port/include/limits.h | 0 > lib/lwip/port/sys-arch.c | 20 ++ > 5 files changed, 381 insertions(+) > create mode 100644 lib/lwip/port/if.c > create mode 100644 lib/lwip/port/include/arch/cc.h > create mode 100644 lib/lwip/port/include/arch/sys_arch.h > create mode 100644 lib/lwip/port/include/limits.h > create mode 100644 lib/lwip/port/sys-arch.c I wonder if this port/ directory should go away and this code should be in net/ ? It feels a bit odd, as lib/ code suggests it is for libraries, not the integration with them. > > diff --git a/lib/lwip/port/if.c b/lib/lwip/port/if.c > new file mode 100644 > index 0000000000..2ed59a0c4e > --- /dev/null > +++ b/lib/lwip/port/if.c > @@ -0,0 +1,256 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> > + */ > + > +#include <common.h> > +#include <command.h> > +extern int eth_init(void); /* net.h */ > +extern void string_to_enetaddr(const char *addr, uint8_t *enetaddr); /* net.h */ > +extern struct in_addr net_ip; > +extern u8 net_ethaddr[6]; Can that go in a header file? > + > +#include "lwip/debug.h" > +#include "lwip/arch.h" > +#include "netif/etharp.h" > +#include "lwip/stats.h" > +#include "lwip/def.h" > +#include "lwip/mem.h" > +#include "lwip/pbuf.h" > +#include "lwip/sys.h" > +#include "lwip/netif.h" > + > +#include "lwip/ip.h" > + > +#define IFNAME0 'e' > +#define IFNAME1 '0' > + > +static struct pbuf *low_level_input(struct netif *netif); > +static int uboot_net_use_lwip; Can we put this stuff in the UCLASS_ETH private data instead of using statics? They require BSS which is typically not available in SPL builds. > + > +int ulwip_enabled(void) > +{ > + return uboot_net_use_lwip; > +} > + > +/* 1 - in loop > + * 0 - no loop > + */ > +static int loop_lwip; > + > +/* ret 1 - in loop > + * 0 - no loop ?? This all needs some more detail in the comments > + */ > +int ulwip_in_loop(void) > +{ > + return loop_lwip; > +} > + > +void ulwip_loop_set(int loop) > +{ > + loop_lwip = loop; > +} > + > +static int ulwip_app_err; > + > +void ulwip_exit(int err) > +{ > + ulwip_app_err = err; > + ulwip_loop_set(0); > +} > + > +int ulwip_app_get_err(void) > +{ > + return ulwip_app_err; > +} > + > +struct uboot_lwip_if { > +}; > + > +static struct netif uboot_netif; > + > +#define LWIP_PORT_INIT_NETMASK(addr) IP4_ADDR((addr), 255, 255, 255, 0) > + > +extern uchar *net_rx_packet; > +extern int net_rx_packet_len; header file please > + > +int uboot_lwip_poll(void) > +{ > + struct pbuf *p; > + int err; > + > + p = low_level_input(&uboot_netif); > + if (!p) { > + printf("error p = low_level_input = NULL\n"); > + return 0; > + } > + > + err = ethernet_input(p, &uboot_netif); > + if (err) > + printf("ip4_input err %d\n", err); log_err() ? But what is going on here? Just return the error code, rather than suppressing it, then the caller can handle it. > + > + return 0; > +} > + > +static struct pbuf *low_level_input(struct netif *netif) > +{ > + struct pbuf *p, *q; > + u16_t len = net_rx_packet_len; > + uchar *data = net_rx_packet; > + > +#if ETH_PAD_SIZE > + len += ETH_PAD_SIZE; /* allow room for Ethernet padding */ Please find a way to drop any use of #if This can be defined to 0 if not needed, for example. Or you could have a static inline eth_pad_size() in the header file (where #if is permitted) > +#endif > + > + /* We allocate a pbuf chain of pbufs from the pool. */ > + p = pbuf_alloc(PBUF_RAW, len, PBUF_POOL); > + if (p) { > +#if ETH_PAD_SIZE > + pbuf_remove_header(p, ETH_PAD_SIZE); /* drop the padding word */ > +#endif > + /* We iterate over the pbuf chain until we have read the entire > + * packet into the pbuf. > + */ > + for (q = p; q != NULL; q = q->next) { > + /* Read enough bytes to fill this pbuf in the chain. The Comment style /* * Read > + * available data in the pbuf is given by the q->len > + * variable. > + * This does not necessarily have to be a memcpy, you can also preallocate > + * pbufs for a DMA-enabled MAC and after receiving truncate it to the > + * actually received size. In this case, ensure the tot_len member of the > + * pbuf is the sum of the chained pbuf len members. > + */ > + MEMCPY(q->payload, data, q->len); > + data += q->len; > + } > + //acknowledge that packet has been read(); Space after // (please fix throughout) > + > +#if ETH_PAD_SIZE > + pbuf_add_header(p, ETH_PAD_SIZE); /* reclaim the padding word */ > +#endif > + LINK_STATS_INC(link.recv); > + } else { > + //drop packet(); > + LINK_STATS_INC(link.memerr); > + LINK_STATS_INC(link.drop); > + } > + > + return p; > +} > + > +static int ethernetif_input(struct pbuf *p, struct netif *netif) > +{ > + struct ethernetif *ethernetif; > + > + ethernetif = netif->state; > + > + /* move received packet into a new pbuf */ > + p = low_level_input(netif); > + > + /* if no packet could be read, silently ignore this */ > + if (p) { > + /* pass all packets to ethernet_input, which decides what packets it supports */ > + if (netif->input(p, netif) != ERR_OK) { > + LWIP_DEBUGF(NETIF_DEBUG, ("%s: IP input error\n", __func__)); > + pbuf_free(p); > + p = NULL; > + } > + } blank line before final return > + return 0; > +} > + > +static err_t low_level_output(struct netif *netif, struct pbuf *p) > +{ > + int err; > + > + err = eth_send(p->payload, p->len); > + if (err != 0) { if (err) > + printf("eth_send error %d\n", err); > + return ERR_ABRT; > + } > + return ERR_OK; > +} > + > +err_t uboot_lwip_if_init(struct netif *netif) > +{ > + struct uboot_lwip_if *uif = (struct uboot_lwip_if *)malloc(sizeof(struct uboot_lwip_if)); > + > + if (!uif) { > + printf("uboot_lwip_if: out of memory\n"); > + return ERR_MEM; > + } > + netif->state = uif; > + > + netif->name[0] = IFNAME0; > + netif->name[1] = IFNAME1; > + > + netif->hwaddr_len = ETHARP_HWADDR_LEN; > + string_to_enetaddr(env_get("ethaddr"), netif->hwaddr); > +#if defined(CONFIG_LWIP_LIB_DEBUG) > + printf(" MAC: %02X:%02X:%02X:%02X:%02X:%02X\n", > + netif->hwaddr[0], netif->hwaddr[1], netif->hwaddr[2], > + netif->hwaddr[3], netif->hwaddr[4], netif->hwaddr[5]); > +#endif > + > +#if LWIP_IPV4 > + netif->output = etharp_output; > +#endif /* LWIP_IPV4 */ > +#if LWIP_IPV6 > + netif->output_ip6 = ethip6_output; > +#endif /* LWIP_IPV6 */ You may need to add accessors in the header file (as global_data.h) so you don't need the #if > + netif->linkoutput = low_level_output; > + netif->mtu = 1500; > + netif->flags = NETIF_FLAG_BROADCAST | NETIF_FLAG_ETHARP | NETIF_FLAG_LINK_UP; > + > + eth_init(); /* activate u-boot eth dev */ > + > + if (IS_ENABLED(CONFIG_LWIP_LIB_DEBUG)) { > + printf("Initialized LWIP stack\n"); > + } > + > + return ERR_OK; > +} > + > +int uboot_lwip_init(void) > +{ > + ip4_addr_t ipaddr, netmask, gw; > + > + if (uboot_net_use_lwip) > + return CMD_RET_SUCCESS; > + > + ip4_addr_set_zero(&gw); > + ip4_addr_set_zero(&ipaddr); > + ip4_addr_set_zero(&netmask); > + > + ipaddr_aton(env_get("ipaddr"), &ipaddr); > + ipaddr_aton(env_get("ipaddr"), &netmask); > + > + LWIP_PORT_INIT_NETMASK(&netmask); > + if (IS_ENABLED(CONFIG_LWIP_LIB_DEBUG)) { > + printf("Starting lwIP, IP: %s\n", ip4addr_ntoa(&ipaddr)); > + printf(" GW: %s\n", ip4addr_ntoa(&gw)); > + printf(" mask: %s\n", ip4addr_ntoa(&netmask)); > + } > + > + if (!netif_add(&uboot_netif, &ipaddr, &netmask, &gw, > + &uboot_netif, uboot_lwip_if_init, ethernetif_input)) > + printf("err: netif_add failed!\n"); > + > + netif_set_up(&uboot_netif); > + netif_set_link_up(&uboot_netif); > +#if LWIP_IPV6 if () > + netif_create_ip6_linklocal_address(&uboot_netif, 1); > + printf(" IPv6: %s\n", ip6addr_ntoa(netif_ip6_addr(uboot_netif, 0))); > +#endif /* LWIP_IPV6 */ > + > + uboot_net_use_lwip = 1; > + > + return CMD_RET_SUCCESS; > +} > + > +/* placeholder, not used now */ > +void uboot_lwip_destroy(void) > +{ > + uboot_net_use_lwip = 0; > +} > diff --git a/lib/lwip/port/include/arch/cc.h b/lib/lwip/port/include/arch/cc.h > new file mode 100644 > index 0000000000..db30d7614e > --- /dev/null > +++ b/lib/lwip/port/include/arch/cc.h > @@ -0,0 +1,46 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +/* It would help to have a little one-line comment as to what these files are for. > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> > + */ > + > +#ifndef LWIP_ARCH_CC_H > +#define LWIP_ARCH_CC_H > + > +#include <linux/types.h> > +#include <linux/kernel.h> > + > +#define LWIP_ERRNO_INCLUDE <errno.h> > + > +#define LWIP_ERRNO_STDINCLUDE 1 > +#define LWIP_NO_UNISTD_H 1 > +#define LWIP_TIMEVAL_PRIVATE 1 > + > +extern unsigned int lwip_port_rand(void); > +#define LWIP_RAND() (lwip_port_rand()) > + > +/* different handling for unit test, normally not needed */ > +#ifdef LWIP_NOASSERT_ON_ERROR > +#define LWIP_ERROR(message, expression, handler) do { if (!(expression)) { \ > + handler; }} while (0) > +#endif > + > +#define LWIP_DONT_PROVIDE_BYTEORDER_FUNCTIONS > + > +#define LWIP_PLATFORM_ASSERT(x) do {printf("Assertion \"%s\" failed at line %d in %s\n", \ > + x, __LINE__, __FILE__); } while (0) > + > +static inline int atoi(const char *str) Can we use U-Boot's version? > +{ > + int r = 0; > + int i; > + > + for (i = 0; str[i] != '\0'; ++i) > + r = r * 10 + str[i] - '0'; > + > + return r; > +} > + > +#define LWIP_ERR_T int > + > +#endif /* LWIP_ARCH_CC_H */ > diff --git a/lib/lwip/port/include/arch/sys_arch.h b/lib/lwip/port/include/arch/sys_arch.h > new file mode 100644 > index 0000000000..8d95146275 > --- /dev/null > +++ b/lib/lwip/port/include/arch/sys_arch.h > @@ -0,0 +1,59 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +/* > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> > + */ > + > +#ifndef LWIP_ARCH_SYS_ARCH_H > +#define LWIP_ARCH_SYS_ARCH_H > + > +#include "lwip/opt.h" > +#include "lwip/arch.h" > +#include "lwip/err.h" > + > +#define ERR_NEED_SCHED 123 > + > +void sys_arch_msleep(u32_t delay_ms); > +#define sys_msleep(ms) sys_arch_msleep(ms) > + > +#if SYS_LIGHTWEIGHT_PROT > +typedef u32_t sys_prot_t; > +#endif /* SYS_LIGHTWEIGHT_PROT */ > + > +#include <errno.h> > + > +#define SYS_MBOX_NULL NULL > +#define SYS_SEM_NULL NULL > + > +typedef u32_t sys_prot_t; > + > +struct sys_sem; > +typedef struct sys_sem *sys_sem_t; > +#define sys_sem_valid(sem) (((sem) != NULL) && (*(sem) != NULL)) > +#define sys_sem_set_invalid(sem) do { if ((sem) != NULL) { *(sem) = NULL; }} while (0) > + > +/* let sys.h use binary semaphores for mutexes */ > +#define LWIP_COMPAT_MUTEX 1 > +#define LWIP_COMPAT_MUTEX_ALLOWED 1 > + > +struct sys_mbox; > +typedef struct sys_mbox *sys_mbox_t; > +#define sys_mbox_valid(mbox) (((mbox) != NULL) && (*(mbox) != NULL)) > +#define sys_mbox_set_invalid(mbox) do { if ((mbox) != NULL) { *(mbox) = NULL; }} while (0) > + > +struct sys_thread; > +typedef struct sys_thread *sys_thread_t; > + > +static inline u32_t sys_arch_sem_wait(sys_sem_t *sem, u32_t timeout) > +{ > + return 0; > +}; > + > +static inline err_t sys_mbox_trypost(sys_mbox_t *mbox, void *msg) > +{ > + return 0; > +}; > + > +#define sys_sem_signal(s) > + > +#endif /* LWIP_ARCH_SYS_ARCH_H */ > diff --git a/lib/lwip/port/include/limits.h b/lib/lwip/port/include/limits.h > new file mode 100644 > index 0000000000..e69de29bb2 > diff --git a/lib/lwip/port/sys-arch.c b/lib/lwip/port/sys-arch.c > new file mode 100644 > index 0000000000..609eeccf8c > --- /dev/null > +++ b/lib/lwip/port/sys-arch.c > @@ -0,0 +1,20 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> > + */ > + > +#include <common.h> > +#include <rand.h> > +#include "lwip/opt.h" > + > +u32_t sys_now(void) > +{ > + return get_timer(0); > +} > + > +u32_t lwip_port_rand(void) > +{ > + return (u32_t)rand(); > +} > + > -- > 2.30.2 > Regards, Simon
On Thu, 3 Aug 2023 at 03:32, Simon Glass <sjg@google.com> wrote: > Hi Maxim, > > On Wed, 2 Aug 2023 at 08:11, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > > > subject: U-Boot > > commit message please (throughout series) > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > > --- > > lib/lwip/port/if.c | 256 ++++++++++++++++++++++++++ > > lib/lwip/port/include/arch/cc.h | 46 +++++ > > lib/lwip/port/include/arch/sys_arch.h | 59 ++++++ > > lib/lwip/port/include/limits.h | 0 > > lib/lwip/port/sys-arch.c | 20 ++ > > 5 files changed, 381 insertions(+) > > create mode 100644 lib/lwip/port/if.c > > create mode 100644 lib/lwip/port/include/arch/cc.h > > create mode 100644 lib/lwip/port/include/arch/sys_arch.h > > create mode 100644 lib/lwip/port/include/limits.h > > create mode 100644 lib/lwip/port/sys-arch.c > > I wonder if this port/ directory should go away and this code should > be in net/ ? It feels a bit odd, as lib/ code suggests it is for > libraries, not the integration with them. > > > > > diff --git a/lib/lwip/port/if.c b/lib/lwip/port/if.c > > new file mode 100644 > > index 0000000000..2ed59a0c4e > > --- /dev/null > > +++ b/lib/lwip/port/if.c > > @@ -0,0 +1,256 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +/* > > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> > > + */ > > + > > +#include <common.h> > > +#include <command.h> > > +extern int eth_init(void); /* net.h */ > > +extern void string_to_enetaddr(const char *addr, uint8_t *enetaddr); /* > net.h */ > > +extern struct in_addr net_ip; > > +extern u8 net_ethaddr[6]; > > Can that go in a header file? > I tried to do as minimal changes as I could. In general these are definitions from include/net.h. And the problem that net.h has not only ethernet things, but also protocol defines which overlaps with lwip protocol defines and data structures. More clean fix will be to clean up net.h and move ip to net/ip.h udp to net/udp.h and so on. So here we can include <net.h> to get eth_init() and friends accessed. > > > + > > +#include "lwip/debug.h" > > +#include "lwip/arch.h" > > +#include "netif/etharp.h" > > +#include "lwip/stats.h" > > +#include "lwip/def.h" > > +#include "lwip/mem.h" > > +#include "lwip/pbuf.h" > > +#include "lwip/sys.h" > > +#include "lwip/netif.h" > > + > > +#include "lwip/ip.h" > > + > > +#define IFNAME0 'e' > > +#define IFNAME1 '0' > > + > > +static struct pbuf *low_level_input(struct netif *netif); > > +static int uboot_net_use_lwip; > > Can we put this stuff in the UCLASS_ETH private data instead of using > statics? They require BSS which is typically not available in SPL > builds. > > yes, that will work. I expect this flag to be used for compatibility mode. I.e. if it's set before cmd then lwip controls net_loop(), if not set then an original code controls net_loop(). I can even add an argument to eth_init(). But because there are too many eth_init() calls I think I will add an entry to uclass. > > + > > +int ulwip_enabled(void) > > +{ > > + return uboot_net_use_lwip; > > +} > > + > > +/* 1 - in loop > > + * 0 - no loop > > + */ > > +static int loop_lwip; > > + > > +/* ret 1 - in loop > > + * 0 - no loop > > ?? > > This all needs some more detail in the comments > Yes, maybe with UCLASS_ETH some of that will go away. > > > + */ > > +int ulwip_in_loop(void) > > +{ > > + return loop_lwip; > > +} > > + > > +void ulwip_loop_set(int loop) > > +{ > > + loop_lwip = loop; > > +} > > + > > +static int ulwip_app_err; > > + > > +void ulwip_exit(int err) > > +{ > > + ulwip_app_err = err; > > + ulwip_loop_set(0); > > +} > > + > > +int ulwip_app_get_err(void) > > +{ > > + return ulwip_app_err; > > +} > > + > > +struct uboot_lwip_if { > > +}; > > + > > +static struct netif uboot_netif; > > + > > +#define LWIP_PORT_INIT_NETMASK(addr) IP4_ADDR((addr), 255, 255, 255, 0) > > + > > +extern uchar *net_rx_packet; > > +extern int net_rx_packet_len; > > header file please > > ok, the same thing here, it's from net.h > > + > > +int uboot_lwip_poll(void) > > +{ > > + struct pbuf *p; > > + int err; > > + > > + p = low_level_input(&uboot_netif); > > + if (!p) { > > + printf("error p = low_level_input = NULL\n"); > > + return 0; > > + } > > + > > + err = ethernet_input(p, &uboot_netif); > > + if (err) > > + printf("ip4_input err %d\n", err); > > log_err() ? > > But what is going on here? Just return the error code, rather than > suppressing it, then the caller can handle it. > > Looked more detail - current version ethernet_input() (lwip master) always returns ERR_OK (0). When I wrote I added a return code check for non function. So I expect that it can be considered as a bug if some time later we receive something non 0. But in general the poll() function has to be void. I will correct it. > + > > + return 0; > > +} > > + > > +static struct pbuf *low_level_input(struct netif *netif) > > +{ > > + struct pbuf *p, *q; > > + u16_t len = net_rx_packet_len; > > + uchar *data = net_rx_packet; > > + > > +#if ETH_PAD_SIZE > > + len += ETH_PAD_SIZE; /* allow room for Ethernet padding */ > > Please find a way to drop any use of #if > > This can be defined to 0 if not needed, for example. Or you could have > a static inline eth_pad_size() in the header file (where #if is > permitted) > > > +#endif > > + > > + /* We allocate a pbuf chain of pbufs from the pool. */ > > + p = pbuf_alloc(PBUF_RAW, len, PBUF_POOL); > > + if (p) { > > +#if ETH_PAD_SIZE > > + pbuf_remove_header(p, ETH_PAD_SIZE); /* drop the padding > word */ > > +#endif > > + /* We iterate over the pbuf chain until we have read the > entire > > + * packet into the pbuf. > > + */ > > + for (q = p; q != NULL; q = q->next) { > > + /* Read enough bytes to fill this pbuf in the > chain. The > > Comment style > > /* > * Read > > > + * available data in the pbuf is given by the > q->len > > + * variable. > > + * This does not necessarily have to be a > memcpy, you can also preallocate > > + * pbufs for a DMA-enabled MAC and after > receiving truncate it to the > > + * actually received size. In this case, ensure > the tot_len member of the > > + * pbuf is the sum of the chained pbuf len > members. > > + */ > > + MEMCPY(q->payload, data, q->len); > > + data += q->len; > > + } > > + //acknowledge that packet has been read(); > > Space after // (please fix throughout) > > > + > > +#if ETH_PAD_SIZE > > + pbuf_add_header(p, ETH_PAD_SIZE); /* reclaim the padding > word */ > > +#endif > > + LINK_STATS_INC(link.recv); > > + } else { > > + //drop packet(); > > + LINK_STATS_INC(link.memerr); > > + LINK_STATS_INC(link.drop); > > + } > > + > > + return p; > > +} > > + > > +static int ethernetif_input(struct pbuf *p, struct netif *netif) > > +{ > > + struct ethernetif *ethernetif; > > + > > + ethernetif = netif->state; > > + > > + /* move received packet into a new pbuf */ > > + p = low_level_input(netif); > > + > > + /* if no packet could be read, silently ignore this */ > > + if (p) { > > + /* pass all packets to ethernet_input, which decides > what packets it supports */ > > + if (netif->input(p, netif) != ERR_OK) { > > + LWIP_DEBUGF(NETIF_DEBUG, ("%s: IP input > error\n", __func__)); > > + pbuf_free(p); > > + p = NULL; > > + } > > + } > > blank line before final return > > > + return 0; > > +} > > + > > +static err_t low_level_output(struct netif *netif, struct pbuf *p) > > +{ > > + int err; > > + > > + err = eth_send(p->payload, p->len); > > + if (err != 0) { > > if (err) > > > + printf("eth_send error %d\n", err); > > + return ERR_ABRT; > > + } > > + return ERR_OK; > > +} > > + > > +err_t uboot_lwip_if_init(struct netif *netif) > > +{ > > + struct uboot_lwip_if *uif = (struct uboot_lwip_if > *)malloc(sizeof(struct uboot_lwip_if)); > > + > > + if (!uif) { > > + printf("uboot_lwip_if: out of memory\n"); > > + return ERR_MEM; > > + } > > + netif->state = uif; > > + > > + netif->name[0] = IFNAME0; > > + netif->name[1] = IFNAME1; > > + > > + netif->hwaddr_len = ETHARP_HWADDR_LEN; > > + string_to_enetaddr(env_get("ethaddr"), netif->hwaddr); > > +#if defined(CONFIG_LWIP_LIB_DEBUG) > > + printf(" MAC: %02X:%02X:%02X:%02X:%02X:%02X\n", > > + netif->hwaddr[0], netif->hwaddr[1], netif->hwaddr[2], > > + netif->hwaddr[3], netif->hwaddr[4], netif->hwaddr[5]); > > +#endif > > + > > +#if LWIP_IPV4 > > + netif->output = etharp_output; > > +#endif /* LWIP_IPV4 */ > > +#if LWIP_IPV6 > > + netif->output_ip6 = ethip6_output; > > +#endif /* LWIP_IPV6 */ > > You may need to add accessors in the header file (as global_data.h) so > you don't need the #if > > > + netif->linkoutput = low_level_output; > > + netif->mtu = 1500; > > + netif->flags = NETIF_FLAG_BROADCAST | NETIF_FLAG_ETHARP | > NETIF_FLAG_LINK_UP; > > + > > + eth_init(); /* activate u-boot eth dev */ > > + > > + if (IS_ENABLED(CONFIG_LWIP_LIB_DEBUG)) { > > + printf("Initialized LWIP stack\n"); > > + } > > + > > + return ERR_OK; > > +} > > + > > +int uboot_lwip_init(void) > > +{ > > + ip4_addr_t ipaddr, netmask, gw; > > + > > + if (uboot_net_use_lwip) > > + return CMD_RET_SUCCESS; > > + > > + ip4_addr_set_zero(&gw); > > + ip4_addr_set_zero(&ipaddr); > > + ip4_addr_set_zero(&netmask); > > + > > + ipaddr_aton(env_get("ipaddr"), &ipaddr); > > + ipaddr_aton(env_get("ipaddr"), &netmask); > > + > > + LWIP_PORT_INIT_NETMASK(&netmask); > > + if (IS_ENABLED(CONFIG_LWIP_LIB_DEBUG)) { > > + printf("Starting lwIP, IP: %s\n", ip4addr_ntoa(&ipaddr)); > > + printf(" GW: %s\n", ip4addr_ntoa(&gw)); > > + printf(" mask: %s\n", > ip4addr_ntoa(&netmask)); > > + } > > + > > + if (!netif_add(&uboot_netif, &ipaddr, &netmask, &gw, > > + &uboot_netif, uboot_lwip_if_init, > ethernetif_input)) > > + printf("err: netif_add failed!\n"); > > + > > + netif_set_up(&uboot_netif); > > + netif_set_link_up(&uboot_netif); > > +#if LWIP_IPV6 > > if () > > > + netif_create_ip6_linklocal_address(&uboot_netif, 1); > > + printf(" IPv6: %s\n", > ip6addr_ntoa(netif_ip6_addr(uboot_netif, 0))); > > +#endif /* LWIP_IPV6 */ > > + > > + uboot_net_use_lwip = 1; > > + > > + return CMD_RET_SUCCESS; > > +} > > + > > +/* placeholder, not used now */ > > +void uboot_lwip_destroy(void) > > +{ > > + uboot_net_use_lwip = 0; > > +} > > diff --git a/lib/lwip/port/include/arch/cc.h > b/lib/lwip/port/include/arch/cc.h > > new file mode 100644 > > index 0000000000..db30d7614e > > --- /dev/null > > +++ b/lib/lwip/port/include/arch/cc.h > > @@ -0,0 +1,46 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > + > > +/* > > It would help to have a little one-line comment as to what these files are > for. > > > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> > > + */ > > + > > +#ifndef LWIP_ARCH_CC_H > > +#define LWIP_ARCH_CC_H > > + > > +#include <linux/types.h> > > +#include <linux/kernel.h> > > + > > +#define LWIP_ERRNO_INCLUDE <errno.h> > > + > > +#define LWIP_ERRNO_STDINCLUDE 1 > > +#define LWIP_NO_UNISTD_H 1 > > +#define LWIP_TIMEVAL_PRIVATE 1 > > + > > +extern unsigned int lwip_port_rand(void); > > +#define LWIP_RAND() (lwip_port_rand()) > > + > > +/* different handling for unit test, normally not needed */ > > +#ifdef LWIP_NOASSERT_ON_ERROR > > +#define LWIP_ERROR(message, expression, handler) do { if > (!(expression)) { \ > > + handler; }} while (0) > > +#endif > > + > > +#define LWIP_DONT_PROVIDE_BYTEORDER_FUNCTIONS > > + > > +#define LWIP_PLATFORM_ASSERT(x) do {printf("Assertion \"%s\" failed at > line %d in %s\n", \ > > + x, __LINE__, __FILE__); } while (0) > > + > > +static inline int atoi(const char *str) > > Can we use U-Boot's version? > > > +{ > > + int r = 0; > > + int i; > > + > > + for (i = 0; str[i] != '\0'; ++i) > > + r = r * 10 + str[i] - '0'; > > + > > + return r; > > +} > > + > > +#define LWIP_ERR_T int > > + > > +#endif /* LWIP_ARCH_CC_H */ > > diff --git a/lib/lwip/port/include/arch/sys_arch.h > b/lib/lwip/port/include/arch/sys_arch.h > > new file mode 100644 > > index 0000000000..8d95146275 > > --- /dev/null > > +++ b/lib/lwip/port/include/arch/sys_arch.h > > @@ -0,0 +1,59 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > + > > +/* > > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> > > + */ > > + > > +#ifndef LWIP_ARCH_SYS_ARCH_H > > +#define LWIP_ARCH_SYS_ARCH_H > > + > > +#include "lwip/opt.h" > > +#include "lwip/arch.h" > > +#include "lwip/err.h" > > + > > +#define ERR_NEED_SCHED 123 > > + > > +void sys_arch_msleep(u32_t delay_ms); > > +#define sys_msleep(ms) sys_arch_msleep(ms) > > + > > +#if SYS_LIGHTWEIGHT_PROT > > +typedef u32_t sys_prot_t; > > +#endif /* SYS_LIGHTWEIGHT_PROT */ > > + > > +#include <errno.h> > > + > > +#define SYS_MBOX_NULL NULL > > +#define SYS_SEM_NULL NULL > > + > > +typedef u32_t sys_prot_t; > > + > > +struct sys_sem; > > +typedef struct sys_sem *sys_sem_t; > > +#define sys_sem_valid(sem) (((sem) != NULL) && (*(sem) != NULL)) > > +#define sys_sem_set_invalid(sem) do { if ((sem) != NULL) { *(sem) = > NULL; }} while (0) > > + > > +/* let sys.h use binary semaphores for mutexes */ > > +#define LWIP_COMPAT_MUTEX 1 > > +#define LWIP_COMPAT_MUTEX_ALLOWED 1 > > + > > +struct sys_mbox; > > +typedef struct sys_mbox *sys_mbox_t; > > +#define sys_mbox_valid(mbox) (((mbox) != NULL) && (*(mbox) != NULL)) > > +#define sys_mbox_set_invalid(mbox) do { if ((mbox) != NULL) { *(mbox) = > NULL; }} while (0) > > + > > +struct sys_thread; > > +typedef struct sys_thread *sys_thread_t; > > + > > +static inline u32_t sys_arch_sem_wait(sys_sem_t *sem, u32_t timeout) > > +{ > > + return 0; > > +}; > > + > > +static inline err_t sys_mbox_trypost(sys_mbox_t *mbox, void *msg) > > +{ > > + return 0; > > +}; > > + > > +#define sys_sem_signal(s) > > + > > +#endif /* LWIP_ARCH_SYS_ARCH_H */ > > diff --git a/lib/lwip/port/include/limits.h > b/lib/lwip/port/include/limits.h > > new file mode 100644 > > index 0000000000..e69de29bb2 > > diff --git a/lib/lwip/port/sys-arch.c b/lib/lwip/port/sys-arch.c > > new file mode 100644 > > index 0000000000..609eeccf8c > > --- /dev/null > > +++ b/lib/lwip/port/sys-arch.c > > @@ -0,0 +1,20 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +/* > > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> > > + */ > > + > > +#include <common.h> > > +#include <rand.h> > > +#include "lwip/opt.h" > > + > > +u32_t sys_now(void) > > +{ > > + return get_timer(0); > > +} > > + > > +u32_t lwip_port_rand(void) > > +{ > > + return (u32_t)rand(); > > +} > > + > > -- > > 2.30.2 > > > > > Regards, > Simon >
On Thu, 3 Aug 2023 at 22:21, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > > On Thu, 3 Aug 2023 at 03:32, Simon Glass <sjg@google.com> wrote: > >> Hi Maxim, >> >> On Wed, 2 Aug 2023 at 08:11, Maxim Uvarov <maxim.uvarov@linaro.org> >> wrote: >> > >> >> subject: U-Boot >> >> commit message please (throughout series) >> >> > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> > --- >> > lib/lwip/port/if.c | 256 ++++++++++++++++++++++++++ >> > lib/lwip/port/include/arch/cc.h | 46 +++++ >> > lib/lwip/port/include/arch/sys_arch.h | 59 ++++++ >> > lib/lwip/port/include/limits.h | 0 >> > lib/lwip/port/sys-arch.c | 20 ++ >> > 5 files changed, 381 insertions(+) >> > create mode 100644 lib/lwip/port/if.c >> > create mode 100644 lib/lwip/port/include/arch/cc.h >> > create mode 100644 lib/lwip/port/include/arch/sys_arch.h >> > create mode 100644 lib/lwip/port/include/limits.h >> > create mode 100644 lib/lwip/port/sys-arch.c >> >> I wonder if this port/ directory should go away and this code should >> be in net/ ? It feels a bit odd, as lib/ code suggests it is for >> libraries, not the integration with them. >> >> In all lwIP examples I was port/ directory with specific files for the platform. And here I also planned to follow the same rule. And yes, I think it's better to move all this code to net/ and cmd/. > > >> > diff --git a/lib/lwip/port/if.c b/lib/lwip/port/if.c >> > new file mode 100644 >> > index 0000000000..2ed59a0c4e >> > --- /dev/null >> > +++ b/lib/lwip/port/if.c >> > @@ -0,0 +1,256 @@ >> > +// SPDX-License-Identifier: GPL-2.0 >> > + >> > +/* >> > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> >> > + */ >> > + >> > +#include <common.h> >> > +#include <command.h> >> > +extern int eth_init(void); /* net.h */ >> > +extern void string_to_enetaddr(const char *addr, uint8_t *enetaddr); >> /* net.h */ >> > +extern struct in_addr net_ip; >> > +extern u8 net_ethaddr[6]; >> >> Can that go in a header file? >> > > I tried to do as minimal changes as I could. In general these are > definitions from include/net.h. > And the problem that net.h has not only ethernet things, but also protocol > defines which overlaps > with lwip protocol defines and data structures. More clean fix will be to > clean up net.h and move > ip to net/ip.h udp to net/udp.h and so on. So here we can include <net.h> > to get eth_init() and > friends accessed. > > >> >> > + >> > +#include "lwip/debug.h" >> > +#include "lwip/arch.h" >> > +#include "netif/etharp.h" >> > +#include "lwip/stats.h" >> > +#include "lwip/def.h" >> > +#include "lwip/mem.h" >> > +#include "lwip/pbuf.h" >> > +#include "lwip/sys.h" >> > +#include "lwip/netif.h" >> > + >> > +#include "lwip/ip.h" >> > + >> > +#define IFNAME0 'e' >> > +#define IFNAME1 '0' >> > + >> > +static struct pbuf *low_level_input(struct netif *netif); >> > +static int uboot_net_use_lwip; >> >> Can we put this stuff in the UCLASS_ETH private data instead of using >> statics? They require BSS which is typically not available in SPL >> builds. >> >> > yes, that will work. I expect this flag to be used for compatibility mode. > I.e. if it's set before cmd then lwip controls net_loop(), if not set then > an original code controls net_loop(). I can even add an argument to > eth_init(). But because there are too many eth_init() calls I think I will > add an entry to uclass. > moved to UCLASS_ETH with split net.h on net.h, eth.h and arp.h. And I like how it looks now, thanks! > > >> > + >> > +int ulwip_enabled(void) >> > +{ >> > + return uboot_net_use_lwip; >> > +} >> > + >> > +/* 1 - in loop >> > + * 0 - no loop >> > + */ >> > +static int loop_lwip; >> > + >> > +/* ret 1 - in loop >> > + * 0 - no loop >> >> ?? >> >> This all needs some more detail in the comments >> > > Yes, maybe with UCLASS_ETH some of that will go away. > > Yes, that goes away after moving. > >> > + */ >> > +int ulwip_in_loop(void) >> > +{ >> > + return loop_lwip; >> > +} >> > + >> > +void ulwip_loop_set(int loop) >> > +{ >> > + loop_lwip = loop; >> > +} >> > + >> > +static int ulwip_app_err; >> > + >> > +void ulwip_exit(int err) >> > +{ >> > + ulwip_app_err = err; >> > + ulwip_loop_set(0); >> > +} >> > + >> > +int ulwip_app_get_err(void) >> > +{ >> > + return ulwip_app_err; >> > +} >> > + >> > +struct uboot_lwip_if { >> > +}; >> > + >> > +static struct netif uboot_netif; >> > + >> > +#define LWIP_PORT_INIT_NETMASK(addr) IP4_ADDR((addr), 255, 255, 255, >> 0) >> > + >> > +extern uchar *net_rx_packet; >> > +extern int net_rx_packet_len; >> >> header file please >> >> > ok, the same thing here, it's from net.h > > >> > + >> > +int uboot_lwip_poll(void) >> > +{ >> > + struct pbuf *p; >> > + int err; >> > + >> > + p = low_level_input(&uboot_netif); >> > + if (!p) { >> > + printf("error p = low_level_input = NULL\n"); >> > + return 0; >> > + } >> > + >> > + err = ethernet_input(p, &uboot_netif); >> > + if (err) >> > + printf("ip4_input err %d\n", err); >> >> log_err() ? >> >> But what is going on here? Just return the error code, rather than >> suppressing it, then the caller can handle it. >> >> > Looked more detail - current version ethernet_input() (lwip master) always > returns ERR_OK (0). When I wrote I added a return code check for non > function. > So I expect that it can be considered as a bug if some time later we > receive something non 0. > > But in general the poll() function has to be void. I will correct it. > > > + >> > + return 0; >> > +} >> > + >> > +static struct pbuf *low_level_input(struct netif *netif) >> > +{ >> > + struct pbuf *p, *q; >> > + u16_t len = net_rx_packet_len; >> > + uchar *data = net_rx_packet; >> > + >> > +#if ETH_PAD_SIZE >> > + len += ETH_PAD_SIZE; /* allow room for Ethernet padding */ >> >> Please find a way to drop any use of #if >> >> This can be defined to 0 if not needed, for example. Or you could have >> a static inline eth_pad_size() in the header file (where #if is >> permitted) >> >> let's drop for n > > +#endif >> > + >> > + /* We allocate a pbuf chain of pbufs from the pool. */ >> > + p = pbuf_alloc(PBUF_RAW, len, PBUF_POOL); >> > + if (p) { >> > +#if ETH_PAD_SIZE >> > + pbuf_remove_header(p, ETH_PAD_SIZE); /* drop the >> padding word */ >> > +#endif >> > + /* We iterate over the pbuf chain until we have read >> the entire >> > + * packet into the pbuf. >> > + */ >> > + for (q = p; q != NULL; q = q->next) { >> > + /* Read enough bytes to fill this pbuf in the >> chain. The >> >> Comment style >> >> /* >> * Read >> >> > + * available data in the pbuf is given by the >> q->len >> > + * variable. >> > + * This does not necessarily have to be a >> memcpy, you can also preallocate >> > + * pbufs for a DMA-enabled MAC and after >> receiving truncate it to the >> > + * actually received size. In this case, ensure >> the tot_len member of the >> > + * pbuf is the sum of the chained pbuf len >> members. >> > + */ >> > + MEMCPY(q->payload, data, q->len); >> > + data += q->len; >> > + } >> > + //acknowledge that packet has been read(); >> >> Space after // (please fix throughout) >> >> > + >> > +#if ETH_PAD_SIZE >> > + pbuf_add_header(p, ETH_PAD_SIZE); /* reclaim the >> padding word */ >> > +#endif >> > + LINK_STATS_INC(link.recv); >> > + } else { >> > + //drop packet(); >> > + LINK_STATS_INC(link.memerr); >> > + LINK_STATS_INC(link.drop); >> > + } >> > + >> > + return p; >> > +} >> > + >> > +static int ethernetif_input(struct pbuf *p, struct netif *netif) >> > +{ >> > + struct ethernetif *ethernetif; >> > + >> > + ethernetif = netif->state; >> > + >> > + /* move received packet into a new pbuf */ >> > + p = low_level_input(netif); >> > + >> > + /* if no packet could be read, silently ignore this */ >> > + if (p) { >> > + /* pass all packets to ethernet_input, which decides >> what packets it supports */ >> > + if (netif->input(p, netif) != ERR_OK) { >> > + LWIP_DEBUGF(NETIF_DEBUG, ("%s: IP input >> error\n", __func__)); >> > + pbuf_free(p); >> > + p = NULL; >> > + } >> > + } >> >> blank line before final return >> >> > + return 0; >> > +} >> > + >> > +static err_t low_level_output(struct netif *netif, struct pbuf *p) >> > +{ >> > + int err; >> > + >> > + err = eth_send(p->payload, p->len); >> > + if (err != 0) { >> >> if (err) >> >> > + printf("eth_send error %d\n", err); >> > + return ERR_ABRT; >> > + } >> > + return ERR_OK; >> > +} >> > + >> > +err_t uboot_lwip_if_init(struct netif *netif) >> > +{ >> > + struct uboot_lwip_if *uif = (struct uboot_lwip_if >> *)malloc(sizeof(struct uboot_lwip_if)); >> > + >> > + if (!uif) { >> > + printf("uboot_lwip_if: out of memory\n"); >> > + return ERR_MEM; >> > + } >> > + netif->state = uif; >> > + >> > + netif->name[0] = IFNAME0; >> > + netif->name[1] = IFNAME1; >> > + >> > + netif->hwaddr_len = ETHARP_HWADDR_LEN; >> > + string_to_enetaddr(env_get("ethaddr"), netif->hwaddr); >> > +#if defined(CONFIG_LWIP_LIB_DEBUG) >> > + printf(" MAC: %02X:%02X:%02X:%02X:%02X:%02X\n", >> > + netif->hwaddr[0], netif->hwaddr[1], netif->hwaddr[2], >> > + netif->hwaddr[3], netif->hwaddr[4], netif->hwaddr[5]); >> > +#endif >> > + >> > +#if LWIP_IPV4 >> > + netif->output = etharp_output; >> > +#endif /* LWIP_IPV4 */ >> > +#if LWIP_IPV6 >> > + netif->output_ip6 = ethip6_output; >> > +#endif /* LWIP_IPV6 */ >> >> You may need to add accessors in the header file (as global_data.h) so >> you don't need the #if >> >> I did not understand the comment about global_data.h. lwiIP as I understand can work in 3 modes 1. ipv4 2.ipv6 3. ipv4+ipv6. If we want optimization for size I think we need to pass this to Kconfig because lwip has some structs under ifdefs. > > + netif->linkoutput = low_level_output; >> > + netif->mtu = 1500; >> > + netif->flags = NETIF_FLAG_BROADCAST | NETIF_FLAG_ETHARP | >> NETIF_FLAG_LINK_UP; >> > + >> > + eth_init(); /* activate u-boot eth dev */ >> > + >> > + if (IS_ENABLED(CONFIG_LWIP_LIB_DEBUG)) { >> > + printf("Initialized LWIP stack\n"); >> > + } >> > + >> > + return ERR_OK; >> > +} >> > + >> > +int uboot_lwip_init(void) >> > +{ >> > + ip4_addr_t ipaddr, netmask, gw; >> > + >> > + if (uboot_net_use_lwip) >> > + return CMD_RET_SUCCESS; >> > + >> > + ip4_addr_set_zero(&gw); >> > + ip4_addr_set_zero(&ipaddr); >> > + ip4_addr_set_zero(&netmask); >> > + >> > + ipaddr_aton(env_get("ipaddr"), &ipaddr); >> > + ipaddr_aton(env_get("ipaddr"), &netmask); >> > + >> > + LWIP_PORT_INIT_NETMASK(&netmask); >> > + if (IS_ENABLED(CONFIG_LWIP_LIB_DEBUG)) { >> > + printf("Starting lwIP, IP: %s\n", >> ip4addr_ntoa(&ipaddr)); >> > + printf(" GW: %s\n", ip4addr_ntoa(&gw)); >> > + printf(" mask: %s\n", >> ip4addr_ntoa(&netmask)); >> > + } >> > + >> > + if (!netif_add(&uboot_netif, &ipaddr, &netmask, &gw, >> > + &uboot_netif, uboot_lwip_if_init, >> ethernetif_input)) >> > + printf("err: netif_add failed!\n"); >> > + >> > + netif_set_up(&uboot_netif); >> > + netif_set_link_up(&uboot_netif); >> > +#if LWIP_IPV6 >> >> if () >> >> No, we are using lwip header files. it will not work if LWIP_IPV6 set to 0. > > + netif_create_ip6_linklocal_address(&uboot_netif, 1); >> > + printf(" IPv6: %s\n", >> ip6addr_ntoa(netif_ip6_addr(uboot_netif, 0))); >> > +#endif /* LWIP_IPV6 */ >> > + >> > + uboot_net_use_lwip = 1; >> > + >> > + return CMD_RET_SUCCESS; >> > +} >> > + >> > +/* placeholder, not used now */ >> > +void uboot_lwip_destroy(void) >> > +{ >> > + uboot_net_use_lwip = 0; >> > +} >> > diff --git a/lib/lwip/port/include/arch/cc.h >> b/lib/lwip/port/include/arch/cc.h >> > new file mode 100644 >> > index 0000000000..db30d7614e >> > --- /dev/null >> > +++ b/lib/lwip/port/include/arch/cc.h >> > @@ -0,0 +1,46 @@ >> > +/* SPDX-License-Identifier: GPL-2.0 */ >> > + >> > +/* >> >> It would help to have a little one-line comment as to what these files >> are for. >> >> > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> >> > + */ >> > + >> > +#ifndef LWIP_ARCH_CC_H >> > +#define LWIP_ARCH_CC_H >> > + >> > +#include <linux/types.h> >> > +#include <linux/kernel.h> >> > + >> > +#define LWIP_ERRNO_INCLUDE <errno.h> >> > + >> > +#define LWIP_ERRNO_STDINCLUDE 1 >> > +#define LWIP_NO_UNISTD_H 1 >> > +#define LWIP_TIMEVAL_PRIVATE 1 >> > + >> > +extern unsigned int lwip_port_rand(void); >> > +#define LWIP_RAND() (lwip_port_rand()) >> > + >> > +/* different handling for unit test, normally not needed */ >> > +#ifdef LWIP_NOASSERT_ON_ERROR >> > +#define LWIP_ERROR(message, expression, handler) do { if >> (!(expression)) { \ >> > + handler; }} while >> (0) >> > +#endif >> > + >> > +#define LWIP_DONT_PROVIDE_BYTEORDER_FUNCTIONS >> > + >> > +#define LWIP_PLATFORM_ASSERT(x) do {printf("Assertion \"%s\" failed at >> line %d in %s\n", \ >> > + x, __LINE__, __FILE__); } while (0) >> > + >> > +static inline int atoi(const char *str) >> >> Can we use U-Boot's version? >> >> #include <stdlib.h> /* getenv, atoi */ compiles but generates error on linking. I guess there is no atoi in U-Boot. > > +{ >> > + int r = 0; >> > + int i; >> > + >> > + for (i = 0; str[i] != '\0'; ++i) >> > + r = r * 10 + str[i] - '0'; >> > + >> > + return r; >> > +} >> > + >> > +#define LWIP_ERR_T int >> > + >> > +#endif /* LWIP_ARCH_CC_H */ >> > diff --git a/lib/lwip/port/include/arch/sys_arch.h >> b/lib/lwip/port/include/arch/sys_arch.h >> > new file mode 100644 >> > index 0000000000..8d95146275 >> > --- /dev/null >> > +++ b/lib/lwip/port/include/arch/sys_arch.h >> > @@ -0,0 +1,59 @@ >> > +/* SPDX-License-Identifier: GPL-2.0 */ >> > + >> > +/* >> > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> >> > + */ >> > + >> > +#ifndef LWIP_ARCH_SYS_ARCH_H >> > +#define LWIP_ARCH_SYS_ARCH_H >> > + >> > +#include "lwip/opt.h" >> > +#include "lwip/arch.h" >> > +#include "lwip/err.h" >> > + >> > +#define ERR_NEED_SCHED 123 >> > + >> > +void sys_arch_msleep(u32_t delay_ms); >> > +#define sys_msleep(ms) sys_arch_msleep(ms) >> > + >> > +#if SYS_LIGHTWEIGHT_PROT >> > +typedef u32_t sys_prot_t; >> > +#endif /* SYS_LIGHTWEIGHT_PROT */ >> > + >> > +#include <errno.h> >> > + >> > +#define SYS_MBOX_NULL NULL >> > +#define SYS_SEM_NULL NULL >> > + >> > +typedef u32_t sys_prot_t; >> > + >> > +struct sys_sem; >> > +typedef struct sys_sem *sys_sem_t; >> > +#define sys_sem_valid(sem) (((sem) != NULL) && (*(sem) != NULL)) >> > +#define sys_sem_set_invalid(sem) do { if ((sem) != NULL) { *(sem) = >> NULL; }} while (0) >> > + >> > +/* let sys.h use binary semaphores for mutexes */ >> > +#define LWIP_COMPAT_MUTEX 1 >> > +#define LWIP_COMPAT_MUTEX_ALLOWED 1 >> > + >> > +struct sys_mbox; >> > +typedef struct sys_mbox *sys_mbox_t; >> > +#define sys_mbox_valid(mbox) (((mbox) != NULL) && (*(mbox) != NULL)) >> > +#define sys_mbox_set_invalid(mbox) do { if ((mbox) != NULL) { *(mbox) >> = NULL; }} while (0) >> > + >> > +struct sys_thread; >> > +typedef struct sys_thread *sys_thread_t; >> > + >> > +static inline u32_t sys_arch_sem_wait(sys_sem_t *sem, u32_t timeout) >> > +{ >> > + return 0; >> > +}; >> > + >> > +static inline err_t sys_mbox_trypost(sys_mbox_t *mbox, void *msg) >> > +{ >> > + return 0; >> > +}; >> > + >> > +#define sys_sem_signal(s) >> > + >> > +#endif /* LWIP_ARCH_SYS_ARCH_H */ >> > diff --git a/lib/lwip/port/include/limits.h >> b/lib/lwip/port/include/limits.h >> > new file mode 100644 >> > index 0000000000..e69de29bb2 >> > diff --git a/lib/lwip/port/sys-arch.c b/lib/lwip/port/sys-arch.c >> > new file mode 100644 >> > index 0000000000..609eeccf8c >> > --- /dev/null >> > +++ b/lib/lwip/port/sys-arch.c >> > @@ -0,0 +1,20 @@ >> > +// SPDX-License-Identifier: GPL-2.0 >> > + >> > +/* >> > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> >> > + */ >> > + >> > +#include <common.h> >> > +#include <rand.h> >> > +#include "lwip/opt.h" >> > + >> > +u32_t sys_now(void) >> > +{ >> > + return get_timer(0); >> > +} >> > + >> > +u32_t lwip_port_rand(void) >> > +{ >> > + return (u32_t)rand(); >> > +} >> > + >> > -- >> > 2.30.2 >> > >> >> >> Regards, >> Simon >> >
Hi Maxim, On Tue, 8 Aug 2023 at 04:07, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > > > > On Thu, 3 Aug 2023 at 22:21, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >> >> >> >> On Thu, 3 Aug 2023 at 03:32, Simon Glass <sjg@google.com> wrote: >>> >>> Hi Maxim, >>> >>> On Wed, 2 Aug 2023 at 08:11, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >>> > >>> >>> subject: U-Boot >>> >>> commit message please (throughout series) >>> >>> > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >>> > --- >>> > lib/lwip/port/if.c | 256 ++++++++++++++++++++++++++ >>> > lib/lwip/port/include/arch/cc.h | 46 +++++ >>> > lib/lwip/port/include/arch/sys_arch.h | 59 ++++++ >>> > lib/lwip/port/include/limits.h | 0 >>> > lib/lwip/port/sys-arch.c | 20 ++ >>> > 5 files changed, 381 insertions(+) >>> > create mode 100644 lib/lwip/port/if.c >>> > create mode 100644 lib/lwip/port/include/arch/cc.h >>> > create mode 100644 lib/lwip/port/include/arch/sys_arch.h >>> > create mode 100644 lib/lwip/port/include/limits.h >>> > create mode 100644 lib/lwip/port/sys-arch.c >>> [..] >>> > + >>> > +#if LWIP_IPV4 >>> > + netif->output = etharp_output; >>> > +#endif /* LWIP_IPV4 */ >>> > +#if LWIP_IPV6 >>> > + netif->output_ip6 = ethip6_output; >>> > +#endif /* LWIP_IPV6 */ >>> >>> You may need to add accessors in the header file (as global_data.h) so >>> you don't need the #if >>> > > I did not understand the comment about global_data.h. lwiIP as I understand can work > in 3 modes 1. ipv4 2.ipv6 3. ipv4+ipv6. If we want optimization for size I think we need to pass > this to Kconfig because lwip has some structs under ifdefs. I mean that you can follow what global_data.h does. gd_malloc_start() is an example of using a header-file construct to avoid #ifdef in the source. [..] >>> > +#if LWIP_IPV6 >>> >>> if () >>> > > No, we are using lwip header files. it will not work if LWIP_IPV6 set to 0. In general we should always include all header files needed for the source, and this should be harmless. We don't normally #ifdef header files. > >>> >>> > + netif_create_ip6_linklocal_address(&uboot_netif, 1); >>> > + printf(" IPv6: %s\n", ip6addr_ntoa(netif_ip6_addr(uboot_netif, 0))); >>> > +#endif /* LWIP_IPV6 */ >>> > + >>> > + uboot_net_use_lwip = 1; >>> > + >>> > + return CMD_RET_SUCCESS; >>> > +} >>> > + >>> > +/* placeholder, not used now */ >>> > +void uboot_lwip_destroy(void) >>> > +{ >>> > + uboot_net_use_lwip = 0; >>> > +} >>> > diff --git a/lib/lwip/port/include/arch/cc.h b/lib/lwip/port/include/arch/cc.h >>> > new file mode 100644 >>> > index 0000000000..db30d7614e >>> > --- /dev/null >>> > +++ b/lib/lwip/port/include/arch/cc.h >>> > @@ -0,0 +1,46 @@ >>> > +/* SPDX-License-Identifier: GPL-2.0 */ >>> > + >>> > +/* >>> >>> It would help to have a little one-line comment as to what these files are for. >>> >>> > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> >>> > + */ >>> > + >>> > +#ifndef LWIP_ARCH_CC_H >>> > +#define LWIP_ARCH_CC_H >>> > + >>> > +#include <linux/types.h> >>> > +#include <linux/kernel.h> >>> > + >>> > +#define LWIP_ERRNO_INCLUDE <errno.h> >>> > + >>> > +#define LWIP_ERRNO_STDINCLUDE 1 >>> > +#define LWIP_NO_UNISTD_H 1 >>> > +#define LWIP_TIMEVAL_PRIVATE 1 >>> > + >>> > +extern unsigned int lwip_port_rand(void); >>> > +#define LWIP_RAND() (lwip_port_rand()) >>> > + >>> > +/* different handling for unit test, normally not needed */ >>> > +#ifdef LWIP_NOASSERT_ON_ERROR >>> > +#define LWIP_ERROR(message, expression, handler) do { if (!(expression)) { \ >>> > + handler; }} while (0) >>> > +#endif >>> > + >>> > +#define LWIP_DONT_PROVIDE_BYTEORDER_FUNCTIONS >>> > + >>> > +#define LWIP_PLATFORM_ASSERT(x) do {printf("Assertion \"%s\" failed at line %d in %s\n", \ >>> > + x, __LINE__, __FILE__); } while (0) >>> > + >>> > +static inline int atoi(const char *str) >>> >>> Can we use U-Boot's version? >>> > > #include <stdlib.h> /* getenv, atoi */ > compiles but generates error on linking. I guess there is no atoi in U-Boot. No...simple_strtoul() is similar so you could call that from your atoi(). Regards, Simon
diff --git a/lib/lwip/port/if.c b/lib/lwip/port/if.c new file mode 100644 index 0000000000..2ed59a0c4e --- /dev/null +++ b/lib/lwip/port/if.c @@ -0,0 +1,256 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> + */ + +#include <common.h> +#include <command.h> +extern int eth_init(void); /* net.h */ +extern void string_to_enetaddr(const char *addr, uint8_t *enetaddr); /* net.h */ +extern struct in_addr net_ip; +extern u8 net_ethaddr[6]; + +#include "lwip/debug.h" +#include "lwip/arch.h" +#include "netif/etharp.h" +#include "lwip/stats.h" +#include "lwip/def.h" +#include "lwip/mem.h" +#include "lwip/pbuf.h" +#include "lwip/sys.h" +#include "lwip/netif.h" + +#include "lwip/ip.h" + +#define IFNAME0 'e' +#define IFNAME1 '0' + +static struct pbuf *low_level_input(struct netif *netif); +static int uboot_net_use_lwip; + +int ulwip_enabled(void) +{ + return uboot_net_use_lwip; +} + +/* 1 - in loop + * 0 - no loop + */ +static int loop_lwip; + +/* ret 1 - in loop + * 0 - no loop + */ +int ulwip_in_loop(void) +{ + return loop_lwip; +} + +void ulwip_loop_set(int loop) +{ + loop_lwip = loop; +} + +static int ulwip_app_err; + +void ulwip_exit(int err) +{ + ulwip_app_err = err; + ulwip_loop_set(0); +} + +int ulwip_app_get_err(void) +{ + return ulwip_app_err; +} + +struct uboot_lwip_if { +}; + +static struct netif uboot_netif; + +#define LWIP_PORT_INIT_NETMASK(addr) IP4_ADDR((addr), 255, 255, 255, 0) + +extern uchar *net_rx_packet; +extern int net_rx_packet_len; + +int uboot_lwip_poll(void) +{ + struct pbuf *p; + int err; + + p = low_level_input(&uboot_netif); + if (!p) { + printf("error p = low_level_input = NULL\n"); + return 0; + } + + err = ethernet_input(p, &uboot_netif); + if (err) + printf("ip4_input err %d\n", err); + + return 0; +} + +static struct pbuf *low_level_input(struct netif *netif) +{ + struct pbuf *p, *q; + u16_t len = net_rx_packet_len; + uchar *data = net_rx_packet; + +#if ETH_PAD_SIZE + len += ETH_PAD_SIZE; /* allow room for Ethernet padding */ +#endif + + /* We allocate a pbuf chain of pbufs from the pool. */ + p = pbuf_alloc(PBUF_RAW, len, PBUF_POOL); + if (p) { +#if ETH_PAD_SIZE + pbuf_remove_header(p, ETH_PAD_SIZE); /* drop the padding word */ +#endif + /* We iterate over the pbuf chain until we have read the entire + * packet into the pbuf. + */ + for (q = p; q != NULL; q = q->next) { + /* Read enough bytes to fill this pbuf in the chain. The + * available data in the pbuf is given by the q->len + * variable. + * This does not necessarily have to be a memcpy, you can also preallocate + * pbufs for a DMA-enabled MAC and after receiving truncate it to the + * actually received size. In this case, ensure the tot_len member of the + * pbuf is the sum of the chained pbuf len members. + */ + MEMCPY(q->payload, data, q->len); + data += q->len; + } + //acknowledge that packet has been read(); + +#if ETH_PAD_SIZE + pbuf_add_header(p, ETH_PAD_SIZE); /* reclaim the padding word */ +#endif + LINK_STATS_INC(link.recv); + } else { + //drop packet(); + LINK_STATS_INC(link.memerr); + LINK_STATS_INC(link.drop); + } + + return p; +} + +static int ethernetif_input(struct pbuf *p, struct netif *netif) +{ + struct ethernetif *ethernetif; + + ethernetif = netif->state; + + /* move received packet into a new pbuf */ + p = low_level_input(netif); + + /* if no packet could be read, silently ignore this */ + if (p) { + /* pass all packets to ethernet_input, which decides what packets it supports */ + if (netif->input(p, netif) != ERR_OK) { + LWIP_DEBUGF(NETIF_DEBUG, ("%s: IP input error\n", __func__)); + pbuf_free(p); + p = NULL; + } + } + return 0; +} + +static err_t low_level_output(struct netif *netif, struct pbuf *p) +{ + int err; + + err = eth_send(p->payload, p->len); + if (err != 0) { + printf("eth_send error %d\n", err); + return ERR_ABRT; + } + return ERR_OK; +} + +err_t uboot_lwip_if_init(struct netif *netif) +{ + struct uboot_lwip_if *uif = (struct uboot_lwip_if *)malloc(sizeof(struct uboot_lwip_if)); + + if (!uif) { + printf("uboot_lwip_if: out of memory\n"); + return ERR_MEM; + } + netif->state = uif; + + netif->name[0] = IFNAME0; + netif->name[1] = IFNAME1; + + netif->hwaddr_len = ETHARP_HWADDR_LEN; + string_to_enetaddr(env_get("ethaddr"), netif->hwaddr); +#if defined(CONFIG_LWIP_LIB_DEBUG) + printf(" MAC: %02X:%02X:%02X:%02X:%02X:%02X\n", + netif->hwaddr[0], netif->hwaddr[1], netif->hwaddr[2], + netif->hwaddr[3], netif->hwaddr[4], netif->hwaddr[5]); +#endif + +#if LWIP_IPV4 + netif->output = etharp_output; +#endif /* LWIP_IPV4 */ +#if LWIP_IPV6 + netif->output_ip6 = ethip6_output; +#endif /* LWIP_IPV6 */ + netif->linkoutput = low_level_output; + netif->mtu = 1500; + netif->flags = NETIF_FLAG_BROADCAST | NETIF_FLAG_ETHARP | NETIF_FLAG_LINK_UP; + + eth_init(); /* activate u-boot eth dev */ + + if (IS_ENABLED(CONFIG_LWIP_LIB_DEBUG)) { + printf("Initialized LWIP stack\n"); + } + + return ERR_OK; +} + +int uboot_lwip_init(void) +{ + ip4_addr_t ipaddr, netmask, gw; + + if (uboot_net_use_lwip) + return CMD_RET_SUCCESS; + + ip4_addr_set_zero(&gw); + ip4_addr_set_zero(&ipaddr); + ip4_addr_set_zero(&netmask); + + ipaddr_aton(env_get("ipaddr"), &ipaddr); + ipaddr_aton(env_get("ipaddr"), &netmask); + + LWIP_PORT_INIT_NETMASK(&netmask); + if (IS_ENABLED(CONFIG_LWIP_LIB_DEBUG)) { + printf("Starting lwIP, IP: %s\n", ip4addr_ntoa(&ipaddr)); + printf(" GW: %s\n", ip4addr_ntoa(&gw)); + printf(" mask: %s\n", ip4addr_ntoa(&netmask)); + } + + if (!netif_add(&uboot_netif, &ipaddr, &netmask, &gw, + &uboot_netif, uboot_lwip_if_init, ethernetif_input)) + printf("err: netif_add failed!\n"); + + netif_set_up(&uboot_netif); + netif_set_link_up(&uboot_netif); +#if LWIP_IPV6 + netif_create_ip6_linklocal_address(&uboot_netif, 1); + printf(" IPv6: %s\n", ip6addr_ntoa(netif_ip6_addr(uboot_netif, 0))); +#endif /* LWIP_IPV6 */ + + uboot_net_use_lwip = 1; + + return CMD_RET_SUCCESS; +} + +/* placeholder, not used now */ +void uboot_lwip_destroy(void) +{ + uboot_net_use_lwip = 0; +} diff --git a/lib/lwip/port/include/arch/cc.h b/lib/lwip/port/include/arch/cc.h new file mode 100644 index 0000000000..db30d7614e --- /dev/null +++ b/lib/lwip/port/include/arch/cc.h @@ -0,0 +1,46 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> + */ + +#ifndef LWIP_ARCH_CC_H +#define LWIP_ARCH_CC_H + +#include <linux/types.h> +#include <linux/kernel.h> + +#define LWIP_ERRNO_INCLUDE <errno.h> + +#define LWIP_ERRNO_STDINCLUDE 1 +#define LWIP_NO_UNISTD_H 1 +#define LWIP_TIMEVAL_PRIVATE 1 + +extern unsigned int lwip_port_rand(void); +#define LWIP_RAND() (lwip_port_rand()) + +/* different handling for unit test, normally not needed */ +#ifdef LWIP_NOASSERT_ON_ERROR +#define LWIP_ERROR(message, expression, handler) do { if (!(expression)) { \ + handler; }} while (0) +#endif + +#define LWIP_DONT_PROVIDE_BYTEORDER_FUNCTIONS + +#define LWIP_PLATFORM_ASSERT(x) do {printf("Assertion \"%s\" failed at line %d in %s\n", \ + x, __LINE__, __FILE__); } while (0) + +static inline int atoi(const char *str) +{ + int r = 0; + int i; + + for (i = 0; str[i] != '\0'; ++i) + r = r * 10 + str[i] - '0'; + + return r; +} + +#define LWIP_ERR_T int + +#endif /* LWIP_ARCH_CC_H */ diff --git a/lib/lwip/port/include/arch/sys_arch.h b/lib/lwip/port/include/arch/sys_arch.h new file mode 100644 index 0000000000..8d95146275 --- /dev/null +++ b/lib/lwip/port/include/arch/sys_arch.h @@ -0,0 +1,59 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> + */ + +#ifndef LWIP_ARCH_SYS_ARCH_H +#define LWIP_ARCH_SYS_ARCH_H + +#include "lwip/opt.h" +#include "lwip/arch.h" +#include "lwip/err.h" + +#define ERR_NEED_SCHED 123 + +void sys_arch_msleep(u32_t delay_ms); +#define sys_msleep(ms) sys_arch_msleep(ms) + +#if SYS_LIGHTWEIGHT_PROT +typedef u32_t sys_prot_t; +#endif /* SYS_LIGHTWEIGHT_PROT */ + +#include <errno.h> + +#define SYS_MBOX_NULL NULL +#define SYS_SEM_NULL NULL + +typedef u32_t sys_prot_t; + +struct sys_sem; +typedef struct sys_sem *sys_sem_t; +#define sys_sem_valid(sem) (((sem) != NULL) && (*(sem) != NULL)) +#define sys_sem_set_invalid(sem) do { if ((sem) != NULL) { *(sem) = NULL; }} while (0) + +/* let sys.h use binary semaphores for mutexes */ +#define LWIP_COMPAT_MUTEX 1 +#define LWIP_COMPAT_MUTEX_ALLOWED 1 + +struct sys_mbox; +typedef struct sys_mbox *sys_mbox_t; +#define sys_mbox_valid(mbox) (((mbox) != NULL) && (*(mbox) != NULL)) +#define sys_mbox_set_invalid(mbox) do { if ((mbox) != NULL) { *(mbox) = NULL; }} while (0) + +struct sys_thread; +typedef struct sys_thread *sys_thread_t; + +static inline u32_t sys_arch_sem_wait(sys_sem_t *sem, u32_t timeout) +{ + return 0; +}; + +static inline err_t sys_mbox_trypost(sys_mbox_t *mbox, void *msg) +{ + return 0; +}; + +#define sys_sem_signal(s) + +#endif /* LWIP_ARCH_SYS_ARCH_H */ diff --git a/lib/lwip/port/include/limits.h b/lib/lwip/port/include/limits.h new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lib/lwip/port/sys-arch.c b/lib/lwip/port/sys-arch.c new file mode 100644 index 0000000000..609eeccf8c --- /dev/null +++ b/lib/lwip/port/sys-arch.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> + */ + +#include <common.h> +#include <rand.h> +#include "lwip/opt.h" + +u32_t sys_now(void) +{ + return get_timer(0); +} + +u32_t lwip_port_rand(void) +{ + return (u32_t)rand(); +} +
Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- lib/lwip/port/if.c | 256 ++++++++++++++++++++++++++ lib/lwip/port/include/arch/cc.h | 46 +++++ lib/lwip/port/include/arch/sys_arch.h | 59 ++++++ lib/lwip/port/include/limits.h | 0 lib/lwip/port/sys-arch.c | 20 ++ 5 files changed, 381 insertions(+) create mode 100644 lib/lwip/port/if.c create mode 100644 lib/lwip/port/include/arch/cc.h create mode 100644 lib/lwip/port/include/arch/sys_arch.h create mode 100644 lib/lwip/port/include/limits.h create mode 100644 lib/lwip/port/sys-arch.c