On Mon, 31 Aug 2020 at 11:27, Philippe Reynes <philippe.rey...@softathome.com> wrote: > > This commit adds a generic udp protocol framework in the > network loop. So protocol based on udp may be implemented > without modifying the network loop (for example custom > wait magic packet). > > Signed-off-by: Philippe Reynes <philippe.rey...@softathome.com> > --- > > Changelog: > v2: > - no change > > include/net.h | 2 +- > include/net/udp.h | 31 +++++++++++++++++++++++++++++++ > net/Kconfig | 6 ++++++ > net/Makefile | 1 + > net/net.c | 15 ++++++++++++++- > net/udp.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 101 insertions(+), 2 deletions(-) > create mode 100644 include/net/udp.h > create mode 100644 net/udp.c >
Reviewed-by: Simon Glass <s...@chromium.org> Needs a mention in docs somewhere. nits below > diff --git a/include/net.h b/include/net.h > index 1bf9867..2191071 100644 > --- a/include/net.h > +++ b/include/net.h > @@ -551,7 +551,7 @@ extern int net_restart_wrap; /* Tried all > network devices */ > > enum proto_t { > BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP, > - TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL > + TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP > }; > > extern char net_boot_file_name[1024];/* Boot File name */ > diff --git a/include/net/udp.h b/include/net/udp.h > new file mode 100644 > index 0000000..f97df8d > --- /dev/null > +++ b/include/net/udp.h > @@ -0,0 +1,31 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright (C) 2020 Philippe Reynes <philippe.rey...@softathome.com> > + */ > + > +#ifndef __UDP > +#define __UDP > + > +/** > + * struct udp_ops - function to handle udp packet > + * > + * This structure provides the function to handle udp packet in > + * the network loop. > + * > + * @prereq: callback called to check the requirement > + * @start: callback called to start the protocol/feature > + * @data: pointer to store private data (used by prereq and start) > + */ > +struct udp_ops { > + int (*prereq)(void *data); > + int (*start)(void *data); > + void *data; > +}; > + > +int udp_prereq(void); > + > +int udp_start(void); > + > +int udp_loop(struct udp_ops *h); Need function comments for these. Also how about 'ops' instead of 'h'? > + > +#endif > diff --git a/net/Kconfig b/net/Kconfig > index 6874b55..db8d796 100644 > --- a/net/Kconfig > +++ b/net/Kconfig > @@ -8,6 +8,12 @@ menuconfig NET > > if NET > > +config PROT_UDP > + bool "Enable generic udp framework" > + help > + Enable a generic udp framework that allow to define custom that allows defining a custom > + handler for udp protocol. > + > config BOOTP_SEND_HOSTNAME > bool "Send hostname to DNS server" > help > diff --git a/net/Makefile b/net/Makefile > index fef71b9..76527f7 100644 > --- a/net/Makefile > +++ b/net/Makefile > @@ -27,6 +27,7 @@ obj-$(CONFIG_CMD_SNTP) += sntp.o > obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o > obj-$(CONFIG_UDP_FUNCTION_FASTBOOT) += fastboot.o > obj-$(CONFIG_CMD_WOL) += wol.o > +obj-$(CONFIG_PROT_UDP) += udp.o > > # Disable this warning as it is triggered by: > # sprintf(buf, index ? "foo%d" : "foo", index) > diff --git a/net/net.c b/net/net.c > index 28d9eeb..f6ae814 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -102,6 +102,7 @@ > #if defined(CONFIG_CMD_PCAP) > #include <net/pcap.h> > #endif > +#include <net/udp.h> > #if defined(CONFIG_LED_STATUS) > #include <miiphy.h> > #include <status_led.h> > @@ -540,6 +541,11 @@ restart: > wol_start(); > break; > #endif > +#if defined(CONFIG_PROT_UDP) I think you can get the same effect by putting if() after the case > + case UDP: > + udp_start(); > + break; > +#endif > default: > break; > } > @@ -1364,6 +1370,13 @@ static int net_check_prereq(enum proto_t protocol) > } > goto common; > #endif > +#if defined(CONFIG_PROT_UDP) Same here > + case UDP: > + if (udp_prereq()) > + return 1; > + goto common; > +#endif > + > #if defined(CONFIG_CMD_NFS) > case NFS: > #endif > @@ -1375,7 +1388,7 @@ static int net_check_prereq(enum proto_t protocol) > return 1; > } > #if defined(CONFIG_CMD_PING) || defined(CONFIG_CMD_SNTP) || \ > - defined(CONFIG_CMD_DNS) > + defined(CONFIG_CMD_DNS) || defined(CONFIG_PROT_UDP) > common: > #endif > /* Fall through */ > diff --git a/net/udp.c b/net/udp.c > new file mode 100644 > index 0000000..2409ce4 > --- /dev/null > +++ b/net/udp.c > @@ -0,0 +1,48 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2020 Philippe Reynes <philippe.rey...@softathome.com> > + */ > + > +#include <common.h> > +#include <net.h> > +#include <net/udp.h> > + > +static struct udp_ops *udp_ops; > + > +int udp_prereq(void) > +{ > + int ret = 0; > + > + if (udp_ops && udp_ops->prereq) You already check udp_ops is non-NULL below. > + ret = udp_ops->prereq(udp_ops->data); > + > + return ret; > +} > + > +int udp_start(void) > +{ > + int ret = -1; > + > + if (udp_ops && udp_ops->start) > + ret = udp_ops->start(udp_ops->data); > + else > + printf("%s: no start function defined\n", __func__); Should return this error so the caller can print it. > + > + return ret; > +} > + > +int udp_loop(struct udp_ops *ops) > +{ > + int ret = -1; > + > + if (!ops) { > + printf("%s: ops should not be null\n", __func__); > + goto out; > + } > + > + udp_ops = ops; > + ret = net_loop(UDP); > + > + out: > + return ret; > +} > -- > 2.7.4 > Regards, Simon