[RFC 1/2] net: net_up, net_down
Ramon Fried
rfried.dev at gmail.com
Wed May 5 04:02:21 CEST 2021
On Mon, May 3, 2021 at 2:55 PM Ian Ray <ian.ray at ge.com> wrote:
>
> Calls made to eth_halt() by network operations (ping, nfs, tftp, etc.)
> break netconsole if it is already running.
>
> * Maintain the network up / down state based on a reference count,
> using new APIs net_up() and net_down().
>
> Note that when network is brought up by netconsole client, then network
> will remain up until reboot (because there is no way to stop netconsole
> itself).
>
> * Remove net_loop_last_protocol, eth_is_on_demand_init(), and
> eth_set_last_protocol(). This functionality is replaced by
> net_up() / net_down().
>
> * Replace usage of eth_init(), eth_halt() with net_up() and
> net_down() in net.c, ping.c, tftp.c, and netconsole.c.
>
> Signed-off-by: Ian Ray <ian.ray at ge.com>
> ---
> drivers/net/netconsole.c | 26 +++--------------
> include/net.h | 23 ++-------------
> net/net.c | 75 +++++++++++++++++++++++++++++++++---------------
> net/ping.c | 1 -
> net/tftp.c | 4 ---
> net/wol.c | 1 -
> 6 files changed, 59 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index f1d0630..b6d2e22 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -27,11 +27,6 @@ static short nc_out_port; /* target output port */
> static short nc_in_port; /* source input port */
> static const char *output_packet; /* used by first send udp */
> static int output_packet_len;
> -/*
> - * Start with a default last protocol.
> - * We are only interested in NETCONS or not.
> - */
> -enum proto_t net_loop_last_protocol = BOOTP;
>
> static void nc_wait_arp_handler(uchar *pkt, unsigned dest,
> struct in_addr sip, unsigned src,
> @@ -177,7 +172,6 @@ static void nc_send_packet(const char *buf, int len)
> #else
> struct eth_device *eth;
> #endif
> - int inited = 0;
> uchar *pkt;
> uchar *ether;
> struct in_addr ip;
> @@ -200,29 +194,17 @@ static void nc_send_packet(const char *buf, int len)
> return;
> }
>
> - if (!eth_is_active(eth)) {
> - if (eth_is_on_demand_init()) {
> - if (eth_init() < 0)
> - return;
> - eth_set_last_protocol(NETCONS);
> - } else {
> - eth_init_state_only();
> - }
> -
> - inited = 1;
> + if (net_up(NETCONS) < 0) {
> + return;
> }
> +
> pkt = (uchar *)net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
> memcpy(pkt, buf, len);
> ether = nc_ether;
> ip = nc_ip;
> net_send_udp_packet(ether, ip, nc_out_port, nc_in_port, len);
>
> - if (inited) {
> - if (eth_is_on_demand_init())
> - eth_halt();
> - else
> - eth_halt_state_only();
> - }
> + net_down();
> }
>
> static int nc_stdio_start(struct stdio_dev *dev)
> diff --git a/include/net.h b/include/net.h
> index 1bf9867..cc6be60 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -599,6 +599,9 @@ int net_loop(enum proto_t);
> /* Load failed. Start again. */
> int net_start_again(void);
>
> +int net_up(enum proto_t protocol);
> +int net_down(void);
> +
> /* Get size of the ethernet header when we send */
> int net_eth_hdr_size(void);
>
> @@ -706,26 +709,6 @@ int nc_input_packet(uchar *pkt, struct in_addr src_ip, unsigned dest_port,
> unsigned src_port, unsigned len);
> #endif
>
> -static __always_inline int eth_is_on_demand_init(void)
> -{
> -#if defined(CONFIG_NETCONSOLE) && !defined(CONFIG_SPL_BUILD)
> - extern enum proto_t net_loop_last_protocol;
> -
> - return net_loop_last_protocol != NETCONS;
> -#else
> - return 1;
> -#endif
> -}
> -
> -static inline void eth_set_last_protocol(int protocol)
> -{
> -#if defined(CONFIG_NETCONSOLE) && !defined(CONFIG_SPL_BUILD)
> - extern enum proto_t net_loop_last_protocol;
> -
> - net_loop_last_protocol = protocol;
> -#endif
> -}
> -
> /*
> * Check if autoload is enabled. If so, use either NFS or TFTP to download
> * the boot file.
> diff --git a/net/net.c b/net/net.c
> index 28d9eeb..45d4f19 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -404,6 +404,51 @@ void net_init(void)
> * Main network processing loop.
> */
>
> +static int up_ref;
> +static bool up;
> +static bool keep_up;
> +
> +/// Bring net up/active if needed.
> +int net_up(enum proto_t protocol)
> +{
> + int ret;
> + if (!up) {
> + eth_halt();
> + eth_set_current();
> + ret = eth_init();
> + if (ret < 0) {
> + debug_cond(DEBUG_INT_STATE, "net_up failed\n");
> + eth_halt();
> + return ret;
> + }
> + up = true;
> + } else if (up_ref == 0) {
> + eth_init_state_only();
> + }
> + up_ref++;
> + if (protocol == NETCONS) {
> + keep_up = true;
> + }
> + return 0;
> +}
> +
> +/// Set net inactive if no clients remain.
> +int net_down(void)
> +{
> + if (up_ref > 0) {
> + up_ref--;
> + if (up_ref == 0) {
> + if (keep_up) {
> + eth_halt_state_only();
> + } else {
> + up = false;
> + eth_halt();
> + }
> + }
> + }
> + return 0;
> +}
> +
> int net_loop(enum proto_t protocol)
> {
> int ret = -EINVAL;
> @@ -420,16 +465,9 @@ int net_loop(enum proto_t protocol)
>
> bootstage_mark_name(BOOTSTAGE_ID_ETH_START, "eth_start");
> net_init();
> - if (eth_is_on_demand_init() || protocol != NETCONS) {
> - eth_halt();
> - eth_set_current();
> - ret = eth_init();
> - if (ret < 0) {
> - eth_halt();
> - return ret;
> - }
> - } else {
> - eth_init_state_only();
> + ret = net_up(protocol);
> + if (ret < 0) {
> + return ret;
> }
> restart:
> #ifdef CONFIG_USB_KEYBOARD
> @@ -448,7 +486,7 @@ restart:
> switch (net_check_prereq(protocol)) {
> case 1:
> /* network not configured */
> - eth_halt();
> + net_down();
> net_set_state(prev_net_state);
> return -ENODEV;
>
> @@ -589,9 +627,7 @@ restart:
> net_arp_wait_packet_ip.s_addr = 0;
>
> net_cleanup_loop();
> - eth_halt();
> - /* Invalidate the last protocol */
> - eth_set_last_protocol(BOOTP);
> + net_down();
>
> puts("\nAbort\n");
> /* include a debug print as well incase the debug
> @@ -647,12 +683,7 @@ restart:
> env_set_hex("filesize", net_boot_file_size);
> env_set_hex("fileaddr", image_load_addr);
> }
> - if (protocol != NETCONS)
> - eth_halt();
> - else
> - eth_halt_state_only();
> -
> - eth_set_last_protocol(protocol);
> + net_down();
>
> ret = net_boot_file_size;
> debug_cond(DEBUG_INT_STATE, "--- net_loop Success!\n");
> @@ -660,8 +691,7 @@ restart:
>
> case NETLOOP_FAIL:
> net_cleanup_loop();
> - /* Invalidate the last protocol */
> - eth_set_last_protocol(BOOTP);
> + net_down();
> debug_cond(DEBUG_INT_STATE, "--- net_loop Fail!\n");
> ret = -ENONET;
> goto done;
> @@ -719,7 +749,6 @@ int net_start_again(void)
> }
>
> if ((!retry_forever) && (net_try_count > retrycnt)) {
> - eth_halt();
> net_set_state(NETLOOP_FAIL);
> /*
> * We don't provide a way for the protocol to return an error,
> diff --git a/net/ping.c b/net/ping.c
> index 0e33660..2c92806 100644
> --- a/net/ping.c
> +++ b/net/ping.c
> @@ -64,7 +64,6 @@ static int ping_send(void)
>
> static void ping_timeout_handler(void)
> {
> - eth_halt();
> net_set_state(NETLOOP_FAIL); /* we did not get the reply */
> }
>
> diff --git a/net/tftp.c b/net/tftp.c
> index 84e970b..dcc387b 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -652,7 +652,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
> net_set_timeout_handler(timeout_ms, tftp_timeout_handler);
>
> if (store_block(tftp_cur_block - 1, pkt + 2, len)) {
> - eth_halt();
> net_set_state(NETLOOP_FAIL);
> break;
> }
> @@ -680,7 +679,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
> case TFTP_ERR_FILE_NOT_FOUND:
> case TFTP_ERR_ACCESS_DENIED:
> puts("Not retrying...\n");
> - eth_halt();
> net_set_state(NETLOOP_FAIL);
> break;
> case TFTP_ERR_UNDEFINED:
> @@ -829,7 +827,6 @@ void tftp_start(enum proto_t protocol)
> #endif
> {
> if (tftp_init_load_addr()) {
> - eth_halt();
> net_set_state(NETLOOP_FAIL);
> puts("\nTFTP error: ");
> puts("trying to overwrite reserved memory...\n");
> @@ -885,7 +882,6 @@ void tftp_start_server(void)
> tftp_filename[0] = 0;
>
> if (tftp_init_load_addr()) {
> - eth_halt();
> net_set_state(NETLOOP_FAIL);
> puts("\nTFTP error: trying to overwrite reserved memory...\n");
> return;
> diff --git a/net/wol.c b/net/wol.c
> index 0a62566..cd4e67e 100644
> --- a/net/wol.c
> +++ b/net/wol.c
> @@ -85,7 +85,6 @@ void wol_set_timeout(ulong timeout)
>
> static void wol_timeout_handler(void)
> {
> - eth_halt();
> net_set_state(NETLOOP_FAIL);
> }
>
> --
> 2.10.1
>
I like the idea, I think that network protocols should call net_down()
when they're done, however I'm not sure we need reference count.
net_down() can be a wrapper around eth_halt(), We can just check if
netconsole is running and call it eth_halt() only if its' not.
More information about the U-Boot
mailing list