| Message ID | 20230802140658.10319-5-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | net/lwip: add lwip library for the network stack | expand |
Hi Maxim, The split looks a lot easier to review! On Wed, Aug 02, 2023 at 08:06:49PM +0600, Maxim Uvarov wrote: > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > lib/lwip/Makefile | 1 + > lib/lwip/apps/dhcp/lwip-dhcp.c | 53 ++++++++++++++++++++++++++++++++++ > 2 files changed, 54 insertions(+) > create mode 100644 lib/lwip/apps/dhcp/lwip-dhcp.c > > diff --git a/lib/lwip/Makefile b/lib/lwip/Makefile > index 2815662093..a3521a9d18 100644 > --- a/lib/lwip/Makefile > +++ b/lib/lwip/Makefile > @@ -65,4 +65,5 @@ obj-$(CONFIG_NET) += $(LWIPDIR)/netif/ethernet.o > obj-$(CONFIG_NET) += port/if.o > obj-$(CONFIG_NET) += port/sys-arch.o > > +obj-$(CONFIG_CMD_DHCP) += apps/dhcp/lwip-dhcp.o > obj-$(CONFIG_CMD_DNS) += apps/dns/lwip-dns.o > diff --git a/lib/lwip/apps/dhcp/lwip-dhcp.c b/lib/lwip/apps/dhcp/lwip-dhcp.c > new file mode 100644 > index 0000000000..9fda43db9f > --- /dev/null > +++ b/lib/lwip/apps/dhcp/lwip-dhcp.c > @@ -0,0 +1,53 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> > + */ > + > +#include <common.h> > +#include <command.h> > +#include <console.h> > + > +#include <lwip/dhcp.h> > +#include <lwip/prot/dhcp.h> > + > +#include "../../../lwip/ulwip.h" I think we had similar comments on the previous version, you need to get rid of this '../../../' > + > +static struct dhcp dhcp; > +static bool dhcp_is_set; > + > +static int ulwip_dhcp_tmo(void) > +{ > + switch (dhcp.state) { > + case DHCP_STATE_BOUND: > + env_set("bootfile", dhcp.boot_file_name); > + env_set("ipaddr", ip4addr_ntoa(&dhcp.offered_ip_addr)); > + env_set("netmask", ip4addr_ntoa(&dhcp.offered_sn_mask)); > + env_set("serverip", ip4addr_ntoa(&dhcp.server_ip_addr)); > + printf("DHCP client bound to address %s\n", ip4addr_ntoa(&dhcp.offered_ip_addr)); > + break; > + default: > + return -1; > + } > + > + return 0; > +} > + > +int ulwip_dhcp(void) > +{ > + int err; > + struct netif *netif; > + > + ulwip_set_tmo(ulwip_dhcp_tmo); > + netif = netif_get_by_index(1); > + > + if (!dhcp_is_set) { Why do we need dhcp_is_set? Can't we check using netif_get_client_data()? > + dhcp_set_struct(netif, &dhcp); > + dhcp_is_set = true; > + } > + err = dhcp_start(netif); > + if (err) > + printf("dhcp_start error %d\n", err); > + > + return err; > +} > -- > 2.30.2 >
On Thu, 3 Aug 2023 at 12:26, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > Hi Maxim, > > The split looks a lot easier to review! > > On Wed, Aug 02, 2023 at 08:06:49PM +0600, Maxim Uvarov wrote: > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > > --- > > lib/lwip/Makefile | 1 + > > lib/lwip/apps/dhcp/lwip-dhcp.c | 53 ++++++++++++++++++++++++++++++++++ > > 2 files changed, 54 insertions(+) > > create mode 100644 lib/lwip/apps/dhcp/lwip-dhcp.c > > > > diff --git a/lib/lwip/Makefile b/lib/lwip/Makefile > > index 2815662093..a3521a9d18 100644 > > --- a/lib/lwip/Makefile > > +++ b/lib/lwip/Makefile > > @@ -65,4 +65,5 @@ obj-$(CONFIG_NET) += $(LWIPDIR)/netif/ethernet.o > > obj-$(CONFIG_NET) += port/if.o > > obj-$(CONFIG_NET) += port/sys-arch.o > > > > +obj-$(CONFIG_CMD_DHCP) += apps/dhcp/lwip-dhcp.o > > obj-$(CONFIG_CMD_DNS) += apps/dns/lwip-dns.o > > diff --git a/lib/lwip/apps/dhcp/lwip-dhcp.c > b/lib/lwip/apps/dhcp/lwip-dhcp.c > > new file mode 100644 > > index 0000000000..9fda43db9f > > --- /dev/null > > +++ b/lib/lwip/apps/dhcp/lwip-dhcp.c > > @@ -0,0 +1,53 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +/* > > + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> > > + */ > > + > > +#include <common.h> > > +#include <command.h> > > +#include <console.h> > > + > > +#include <lwip/dhcp.h> > > +#include <lwip/prot/dhcp.h> > > + > > +#include "../../../lwip/ulwip.h" > > I think we had similar comments on the previous version, you need to get > rid of this '../../../' > > yes, sorry. Replaced everywhere to <ulwip.h>. Header is already in the path. > > + > > +static struct dhcp dhcp; > > +static bool dhcp_is_set; > > + > > +static int ulwip_dhcp_tmo(void) > > +{ > > + switch (dhcp.state) { > > + case DHCP_STATE_BOUND: > > + env_set("bootfile", dhcp.boot_file_name); > > + env_set("ipaddr", ip4addr_ntoa(&dhcp.offered_ip_addr)); > > + env_set("netmask", ip4addr_ntoa(&dhcp.offered_sn_mask)); > > + env_set("serverip", ip4addr_ntoa(&dhcp.server_ip_addr)); > > + printf("DHCP client bound to address %s\n", > ip4addr_ntoa(&dhcp.offered_ip_addr)); > > + break; > > + default: > > + return -1; > > + } > > + > > + return 0; > > +} > > + > > +int ulwip_dhcp(void) > > +{ > > + int err; > > + struct netif *netif; > > + > > + ulwip_set_tmo(ulwip_dhcp_tmo); > > + netif = netif_get_by_index(1); > > + > > + if (!dhcp_is_set) { > > Why do we need dhcp_is_set? Can't we check using netif_get_client_data()? > > yes, that works also. changed. > > + dhcp_set_struct(netif, &dhcp); > > + dhcp_is_set = true; > > + } > > + err = dhcp_start(netif); > > + if (err) > > + printf("dhcp_start error %d\n", err); > > + > > + return err; > > +} > > -- > > 2.30.2 > > >
diff --git a/lib/lwip/Makefile b/lib/lwip/Makefile index 2815662093..a3521a9d18 100644 --- a/lib/lwip/Makefile +++ b/lib/lwip/Makefile @@ -65,4 +65,5 @@ obj-$(CONFIG_NET) += $(LWIPDIR)/netif/ethernet.o obj-$(CONFIG_NET) += port/if.o obj-$(CONFIG_NET) += port/sys-arch.o +obj-$(CONFIG_CMD_DHCP) += apps/dhcp/lwip-dhcp.o obj-$(CONFIG_CMD_DNS) += apps/dns/lwip-dns.o diff --git a/lib/lwip/apps/dhcp/lwip-dhcp.c b/lib/lwip/apps/dhcp/lwip-dhcp.c new file mode 100644 index 0000000000..9fda43db9f --- /dev/null +++ b/lib/lwip/apps/dhcp/lwip-dhcp.c @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov@linaro.org> + */ + +#include <common.h> +#include <command.h> +#include <console.h> + +#include <lwip/dhcp.h> +#include <lwip/prot/dhcp.h> + +#include "../../../lwip/ulwip.h" + +static struct dhcp dhcp; +static bool dhcp_is_set; + +static int ulwip_dhcp_tmo(void) +{ + switch (dhcp.state) { + case DHCP_STATE_BOUND: + env_set("bootfile", dhcp.boot_file_name); + env_set("ipaddr", ip4addr_ntoa(&dhcp.offered_ip_addr)); + env_set("netmask", ip4addr_ntoa(&dhcp.offered_sn_mask)); + env_set("serverip", ip4addr_ntoa(&dhcp.server_ip_addr)); + printf("DHCP client bound to address %s\n", ip4addr_ntoa(&dhcp.offered_ip_addr)); + break; + default: + return -1; + } + + return 0; +} + +int ulwip_dhcp(void) +{ + int err; + struct netif *netif; + + ulwip_set_tmo(ulwip_dhcp_tmo); + netif = netif_get_by_index(1); + + if (!dhcp_is_set) { + dhcp_set_struct(netif, &dhcp); + dhcp_is_set = true; + } + err = dhcp_start(netif); + if (err) + printf("dhcp_start error %d\n", err); + + return err; +}
Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- lib/lwip/Makefile | 1 + lib/lwip/apps/dhcp/lwip-dhcp.c | 53 ++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 lib/lwip/apps/dhcp/lwip-dhcp.c