Re: [RFC PATCH 1/2] net: ifup: prefer interfaces where the link is already up

[Sascha Hauer <sha@pengutronix.de>]

On Wed, Jan 25, 2023 at 10:52:35AM +0100, Ahmad Fatoum wrote:
> ifup -a on a platform with a DSA switch can take quite a while, because
> barebox will attempt sending a DHCP DISCOVER on every port in sequence
> and waiting until timeout. This could use some refactoring to make it
> possible to make dhcp() non-blocking, but let's do an easier workaround
> for that: Let's prefer ports that already have a link-up, which is the
> case if the user has been sitting idly on the shell for a few seconds,
> because we enable all interfaces that weren't explicitly disabled then.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  include/net.h | 1 +
>  net/eth.c     | 2 +-
>  net/ifup.c    | 8 ++++++++
>  3 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net.h b/include/net.h
> index 0555b0bd6bed..fdd21481dbf5 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -110,6 +110,7 @@ int eth_register(struct eth_device* dev);    /* Register network device		*/
>  void eth_unregister(struct eth_device* dev); /* Unregister network device	*/
>  int eth_set_ethaddr(struct eth_device *edev, const char *ethaddr);
>  int eth_open(struct eth_device *edev);
> +int eth_carrier_check(struct eth_device *edev, bool may_wait);
>  void eth_close(struct eth_device *edev);
>  int eth_send(struct eth_device *edev, void *packet, int length);	   /* Send a packet		*/
>  int eth_rx(void);			/* Check for received packets	*/
> diff --git a/net/eth.c b/net/eth.c
> index 6fb64afea024..6c201d7dfc30 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -179,7 +179,7 @@ int eth_complete(struct string_list *sl, char *instr)
>  /*
>   * Check for link if we haven't done so for longer.
>   */
> -static int eth_carrier_check(struct eth_device *edev, bool may_wait)
> +int eth_carrier_check(struct eth_device *edev, bool may_wait)
>  {
>  	int ret;
>  
> diff --git a/net/ifup.c b/net/ifup.c
> index e4d5374db3ae..a0167eeb8a82 100644
> --- a/net/ifup.c
> +++ b/net/ifup.c
> @@ -285,6 +285,14 @@ int ifup_all(unsigned flags)
>  	    list_empty(&netdev_list))
>  		device_detect_all();
>  
> +	/* Prefer interfaces where the link is already up */
> +	for_each_netdev(edev) {
> +		if (eth_carrier_check(edev, false))
> +			continue;
> +
> +		ifup_edev(edev, flags);
> +	}
> +

This patch confused my while reading it. It doesn't "prefer" interfaces,
instead it only changes the order they are brought up. This change
doesn't make sense by itself, only reading patch 2/2 makes your intention
clear. I think you should merge both patches together.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |