mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] net: Bring up all interfaces before going interactive
@ 2022-09-16 12:49 Sascha Hauer
  2022-09-16 13:16 ` Sascha Hauer
  2022-09-20  8:22 ` Marco Felsch
  0 siblings, 2 replies; 7+ messages in thread
From: Sascha Hauer @ 2022-09-16 12:49 UTC (permalink / raw)
  To: Barebox List

So far we only bring up network interfaces when we actually need them.
This means we could be idling in the shell for long and once the user
decides to do networking he has to wait for the link to be established.
We can do better: Before going interactive bring up all known network
interfaces which makes the links established when the user needs them.

To implement this we have to rework carrier checking a bit, because
otherwise barebox would wait for the links to be established before
dropping to the shell.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/startup.c |  3 +++
 include/net.h    |  3 +++
 net/eth.c        | 34 +++++++++++++++++++++-------------
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/common/startup.c b/common/startup.c
index f53b73f81a..f63a0ae7d5 100644
--- a/common/startup.c
+++ b/common/startup.c
@@ -37,6 +37,7 @@
 #include <linux/ctype.h>
 #include <watchdog.h>
 #include <glob.h>
+#include <net.h>
 #include <bselftest.h>
 
 extern initcall_t __barebox_initcalls_start[], __barebox_early_initcalls_end[],
@@ -208,6 +209,8 @@ enum autoboot_state do_autoboot_countdown(void)
 				&outkey);
 	command_slice_acquire();
 
+	eth_open_all();
+
 	if (ret == 0)
 		autoboot_state = AUTOBOOT_BOOT;
 	else if (menu_exists && outkey == 'm')
diff --git a/include/net.h b/include/net.h
index 310bf5169d..338033d698 100644
--- a/include/net.h
+++ b/include/net.h
@@ -79,6 +79,8 @@ struct eth_device {
 #define ETH_MODE_STATIC 1
 #define ETH_MODE_DISABLED 2
 	unsigned int global_mode;
+
+	uint64_t last_link_check;
 };
 
 #define dev_to_edev(d) container_of(d, struct eth_device, dev)
@@ -109,6 +111,7 @@ int eth_open(struct eth_device *edev);
 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	*/
+void eth_open_all(void);
 struct eth_device *of_find_eth_device_by_node(struct device_node *np);
 
 /* associate a MAC address to a ethernet device. Should be called by
diff --git a/net/eth.c b/net/eth.c
index bc641dc8e4..8f6ff7db3a 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -20,8 +20,6 @@
 #include <linux/ctype.h>
 #include <linux/stat.h>
 
-static uint64_t last_link_check;
-
 LIST_HEAD(netdev_list);
 
 struct eth_ethaddr {
@@ -173,7 +171,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, int force)
+static int eth_carrier_check(struct eth_device *edev, bool may_wait)
 {
 	int ret;
 
@@ -183,15 +181,17 @@ static int eth_carrier_check(struct eth_device *edev, int force)
 	if (!edev->phydev)
 		return 0;
 
-	if (force)
-		phy_wait_aneg_done(edev->phydev);
-
-	if (force || is_timeout(last_link_check, 5 * SECOND) ||
-			!edev->phydev->link) {
+	if (!edev->last_link_check ||
+	    is_timeout(edev->last_link_check, 5 * SECOND)) {
 		ret = phy_update_status(edev->phydev);
 		if (ret)
 			return ret;
-		last_link_check = get_time_ns();
+		edev->last_link_check = get_time_ns();
+	}
+
+	if (may_wait && !edev->phydev->link) {
+		phy_wait_aneg_done(edev->phydev);
+		edev->last_link_check = get_time_ns();
 	}
 
 	return edev->phydev->link ? 0 : -ENETDOWN;
@@ -237,7 +237,7 @@ int eth_send(struct eth_device *edev, void *packet, int length)
 	if (slice_acquired(eth_device_slice(edev)))
 		return eth_queue(edev, packet, length);
 
-	ret = eth_carrier_check(edev, 0);
+	ret = eth_carrier_check(edev, true);
 	if (ret)
 		return ret;
 
@@ -258,7 +258,7 @@ static void eth_do_work(struct eth_device *edev)
 	int ret;
 
 	if (!phy_acquired(edev->phydev)) {
-		ret = eth_carrier_check(edev, 0);
+		ret = eth_carrier_check(edev, false);
 		if (ret)
 			return;
 	}
@@ -455,12 +455,12 @@ int eth_open(struct eth_device *edev)
 	if (edev->active)
 		return 0;
 
+	edev->last_link_check = 0;
+
 	ret = edev->open(edev);
 	if (!ret)
 		edev->active = 1;
 
-	eth_carrier_check(edev, 1);
-
 	return ret;
 }
 
@@ -522,6 +522,14 @@ struct eth_device *of_find_eth_device_by_node(struct device_node *np)
 }
 EXPORT_SYMBOL(of_find_eth_device_by_node);
 
+void eth_open_all(void)
+{
+	struct eth_device *edev;
+
+	list_for_each_entry(edev, &netdev_list, list)
+		eth_open(edev);
+}
+
 static int of_populate_ethaddr(void)
 {
 	char str[sizeof("xx:xx:xx:xx:xx:xx")];
-- 
2.30.2




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: Bring up all interfaces before going interactive
  2022-09-16 12:49 [PATCH] net: Bring up all interfaces before going interactive Sascha Hauer
@ 2022-09-16 13:16 ` Sascha Hauer
  2022-09-19 17:20   ` Trent Piepho
  2022-09-20  8:22 ` Marco Felsch
  1 sibling, 1 reply; 7+ messages in thread
From: Sascha Hauer @ 2022-09-16 13:16 UTC (permalink / raw)
  To: Barebox List

On Fri, Sep 16, 2022 at 02:49:42PM +0200, Sascha Hauer wrote:
> So far we only bring up network interfaces when we actually need them.
> This means we could be idling in the shell for long and once the user
> decides to do networking he has to wait for the link to be established.
> We can do better: Before going interactive bring up all known network
> interfaces which makes the links established when the user needs them.
> 
> To implement this we have to rework carrier checking a bit, because
> otherwise barebox would wait for the links to be established before
> dropping to the shell.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  common/startup.c |  3 +++
>  include/net.h    |  3 +++
>  net/eth.c        | 34 +++++++++++++++++++++-------------
>  3 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/common/startup.c b/common/startup.c
> index f53b73f81a..f63a0ae7d5 100644
> --- a/common/startup.c
> +++ b/common/startup.c
> @@ -37,6 +37,7 @@
>  #include <linux/ctype.h>
>  #include <watchdog.h>
>  #include <glob.h>
> +#include <net.h>
>  #include <bselftest.h>
>  
>  extern initcall_t __barebox_initcalls_start[], __barebox_early_initcalls_end[],
> @@ -208,6 +209,8 @@ enum autoboot_state do_autoboot_countdown(void)
>  				&outkey);
>  	command_slice_acquire();
>  
> +	eth_open_all();
> +

We could make the user experience even a bit better by doing this call
before waiting for the autoboot timeout which then speeds up netboot a
bit.

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 |



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: Bring up all interfaces before going interactive
  2022-09-16 13:16 ` Sascha Hauer
@ 2022-09-19 17:20   ` Trent Piepho
  2022-09-20  7:41     ` Sascha Hauer
  0 siblings, 1 reply; 7+ messages in thread
From: Trent Piepho @ 2022-09-19 17:20 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On Fri, Sep 16, 2022 at 6:16 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Fri, Sep 16, 2022 at 02:49:42PM +0200, Sascha Hauer wrote:
> > So far we only bring up network interfaces when we actually need them.
> > This means we could be idling in the shell for long and once the user
> > decides to do networking he has to wait for the link to be established.
>
> We could make the user experience even a bit better by doing this call
> before waiting for the autoboot timeout which then speeds up netboot a
> bit.

The Linux phy driver almost invariably does not bother to avoid
resetting the network phy when not necessary.  So if network
auto-negotiation is started in barebox, it will be reset and then
restarted shortly after in Linux.  The link auto-negotiation impacts
the LAN's spanning tree if that is in use.

So there is a drawback, besides the small added boot time to
initialize the network hardware, to adding an unneeded network restart
in Barebox in the case where interactive mode is not entered and
network boot is not used.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: Bring up all interfaces before going interactive
  2022-09-19 17:20   ` Trent Piepho
@ 2022-09-20  7:41     ` Sascha Hauer
  2022-09-20  9:47       ` Trent Piepho
  0 siblings, 1 reply; 7+ messages in thread
From: Sascha Hauer @ 2022-09-20  7:41 UTC (permalink / raw)
  To: Trent Piepho; +Cc: Barebox List

On Mon, Sep 19, 2022 at 10:20:53AM -0700, Trent Piepho wrote:
> On Fri, Sep 16, 2022 at 6:16 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > On Fri, Sep 16, 2022 at 02:49:42PM +0200, Sascha Hauer wrote:
> > > So far we only bring up network interfaces when we actually need them.
> > > This means we could be idling in the shell for long and once the user
> > > decides to do networking he has to wait for the link to be established.
> >
> > We could make the user experience even a bit better by doing this call
> > before waiting for the autoboot timeout which then speeds up netboot a
> > bit.
> 
> The Linux phy driver almost invariably does not bother to avoid
> resetting the network phy when not necessary.  So if network
> auto-negotiation is started in barebox, it will be reset and then
> restarted shortly after in Linux.  The link auto-negotiation impacts
> the LAN's spanning tree if that is in use.
> 
> So there is a drawback, besides the small added boot time to
> initialize the network hardware, to adding an unneeded network restart
> in Barebox in the case where interactive mode is not entered and
> network boot is not used.

So is this a vote to only do the autonegotiaton when going interactive
in barebox, or a vote to do autonegotiation only when the network is
being used in barebox?

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 |



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: Bring up all interfaces before going interactive
  2022-09-16 12:49 [PATCH] net: Bring up all interfaces before going interactive Sascha Hauer
  2022-09-16 13:16 ` Sascha Hauer
@ 2022-09-20  8:22 ` Marco Felsch
  1 sibling, 0 replies; 7+ messages in thread
From: Marco Felsch @ 2022-09-20  8:22 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

Hi Sascha,

On 22-09-16, Sascha Hauer wrote:
> So far we only bring up network interfaces when we actually need them.
> This means we could be idling in the shell for long and once the user
> decides to do networking he has to wait for the link to be established.
> We can do better: Before going interactive bring up all known network
> interfaces which makes the links established when the user needs them.

can we do that conditional e.g. by using a globalvar? Since this can
cause regression e.g. if your use-case is to boot as fast as possible.

Regards,
  Marco



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: Bring up all interfaces before going interactive
  2022-09-20  7:41     ` Sascha Hauer
@ 2022-09-20  9:47       ` Trent Piepho
  2022-09-22  9:41         ` Sascha Hauer
  0 siblings, 1 reply; 7+ messages in thread
From: Trent Piepho @ 2022-09-20  9:47 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On Tue, Sep 20, 2022 at 12:41 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Mon, Sep 19, 2022 at 10:20:53AM -0700, Trent Piepho wrote:
> > On Fri, Sep 16, 2022 at 6:16 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > >
> > > We could make the user experience even a bit better by doing this call
> > > before waiting for the autoboot timeout which then speeds up netboot a
> > > bit.
> >
> > The Linux phy driver almost invariably does not bother to avoid
> > resetting the network phy when not necessary.  So if network
> > auto-negotiation is started in barebox, it will be reset and then
> > restarted shortly after in Linux.  The link auto-negotiation impacts
> > the LAN's spanning tree if that is in use.
> >
> > So there is a drawback, besides the small added boot time to
> > initialize the network hardware, to adding an unneeded network restart
> > in Barebox in the case where interactive mode is not entered and
> > network boot is not used.
>
> So is this a vote to only do the autonegotiaton when going interactive
> in barebox, or a vote to do autonegotiation only when the network is
> being used in barebox?

I would vote to only do it when going interactive.

It also seems reasonable to bring the network up quickly when network
booting, but I think determining if the system will network boot,
before actually trying to boot, is probably unreasonably difficult to
do.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: Bring up all interfaces before going interactive
  2022-09-20  9:47       ` Trent Piepho
@ 2022-09-22  9:41         ` Sascha Hauer
  0 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2022-09-22  9:41 UTC (permalink / raw)
  To: Trent Piepho; +Cc: Barebox List

On Tue, Sep 20, 2022 at 02:47:07AM -0700, Trent Piepho wrote:
> On Tue, Sep 20, 2022 at 12:41 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Mon, Sep 19, 2022 at 10:20:53AM -0700, Trent Piepho wrote:
> > > On Fri, Sep 16, 2022 at 6:16 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > >
> > > > We could make the user experience even a bit better by doing this call
> > > > before waiting for the autoboot timeout which then speeds up netboot a
> > > > bit.
> > >
> > > The Linux phy driver almost invariably does not bother to avoid
> > > resetting the network phy when not necessary.  So if network
> > > auto-negotiation is started in barebox, it will be reset and then
> > > restarted shortly after in Linux.  The link auto-negotiation impacts
> > > the LAN's spanning tree if that is in use.
> > >
> > > So there is a drawback, besides the small added boot time to
> > > initialize the network hardware, to adding an unneeded network restart
> > > in Barebox in the case where interactive mode is not entered and
> > > network boot is not used.
> >
> > So is this a vote to only do the autonegotiaton when going interactive
> > in barebox, or a vote to do autonegotiation only when the network is
> > being used in barebox?
> 
> I would vote to only do it when going interactive.

I changed the behaviour accordingly.

> 
> It also seems reasonable to bring the network up quickly when network
> booting, but I think determining if the system will network boot,
> before actually trying to boot, is probably unreasonably difficult to
> do.

I don't know how we could determine if we are going to do a network
boot.
We could do the network initialisation earlier based on some globalvar
like Marco suggested, but currently I do not have enough motivation to
implement it.

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 |



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-09-22 10:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-16 12:49 [PATCH] net: Bring up all interfaces before going interactive Sascha Hauer
2022-09-16 13:16 ` Sascha Hauer
2022-09-19 17:20   ` Trent Piepho
2022-09-20  7:41     ` Sascha Hauer
2022-09-20  9:47       ` Trent Piepho
2022-09-22  9:41         ` Sascha Hauer
2022-09-20  8:22 ` Marco Felsch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox