All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ramon Fried <rfried.dev@gmail.com>
To: u-boot@lists.denx.de
Subject: [RFC 1/2] net: net_up, net_down
Date: Wed, 5 May 2021 05:02:21 +0300	[thread overview]
Message-ID: <CAGi-RUJDLOyx63ftc+KE03B1HsVNGPDr-TitnO3ruoj80QcKZw@mail.gmail.com> (raw)
In-Reply-To: <48c36f0a30192f3fa39dcecbec45dbf7c734db80.1620039770.git.ian.ray@ge.com>

On Mon, May 3, 2021 at 2:55 PM Ian Ray <ian.ray@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@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.

  reply	other threads:[~2021-05-05  2:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-03 11:55 [RFC 0/2] netconsole and networking co-existence Ian Ray
2021-05-03 11:55 ` [RFC 1/2] net: net_up, net_down Ian Ray
2021-05-05  2:02   ` Ramon Fried [this message]
2021-05-05  7:04     ` EXT: " Ian Ray
2021-05-03 11:55 ` [RFC 2/2] netconsole and networking co-existence Ian Ray

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAGi-RUJDLOyx63ftc+KE03B1HsVNGPDr-TitnO3ruoj80QcKZw@mail.gmail.com \
    --to=rfried.dev@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.