* [PATCH v2 1/3] net: ifup: have ifup -a poll for link up in parallel
2023-01-30 7:20 [PATCH v2 0/3] net: ifup: greatly reduce ifup -a time for multiple network interface Ahmad Fatoum
@ 2023-01-30 7:20 ` Ahmad Fatoum
2023-01-30 7:20 ` [PATCH v2 2/3] net: ifup: have ifup -a1 stop at first DHCP-set global.net.server Ahmad Fatoum
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2023-01-30 7:20 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
DHCP is usually fairly quick, but link up check timeout is 10 seconds,
which adds up, especially on systems with bigger DSA switches.
The workaround is to set ethX.mode=disabled for other ports, but let's
improve the default a bit and have barebox poll link ups in parallel, so
instead of (number_of_ports_wihout_link * 10s), we just wait 10s at
most.
For setups where this is a problem, users may revert to ifup in sequence
by doing ifup -a -s.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
- new patch
---
include/net.h | 3 ++
net/eth.c | 28 ++++++++++----
net/ifup.c | 103 ++++++++++++++++++++++++++++++++++++++++----------
3 files changed, 105 insertions(+), 29 deletions(-)
diff --git a/include/net.h b/include/net.h
index 0555b0bd6bed..a7da282412fa 100644
--- a/include/net.h
+++ b/include/net.h
@@ -109,6 +109,7 @@ static inline int eth_send_raw(struct eth_device *edev, void *packet,
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_carrier_poll_once(struct eth_device *edev);
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 */
@@ -511,6 +512,8 @@ int net_icmp_send(struct net_connection *con, int len);
void led_trigger_network(enum led_trigger trigger);
#define IFUP_FLAG_FORCE (1 << 0)
+#define IFUP_FLAG_PARALLEL (1 << 1)
+#define IFUP_FLAG_SKIP_CONF (1 << 2)
int ifup_edev(struct eth_device *edev, unsigned flags);
int ifup(const char *name, unsigned flags);
diff --git a/net/eth.c b/net/eth.c
index 6fb64afea024..ccac5e2a6488 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -176,13 +176,29 @@ int eth_complete(struct string_list *sl, char *instr)
}
#endif
+int eth_carrier_poll_once(struct eth_device *edev)
+{
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_PHYLIB))
+ return 0;
+
+ if (!edev->phydev)
+ return 0;
+
+ ret = phy_update_status(edev->phydev);
+ if (ret)
+ return ret;
+
+ edev->last_link_check = get_time_ns();
+ return edev->phydev->link ? 0 : -ENETDOWN;
+}
+
/*
* Check for link if we haven't done so for longer.
*/
static int eth_carrier_check(struct eth_device *edev, bool may_wait)
{
- int ret;
-
if (!IS_ENABLED(CONFIG_PHYLIB))
return 0;
@@ -190,12 +206,8 @@ static int eth_carrier_check(struct eth_device *edev, bool may_wait)
return 0;
if (!edev->last_link_check ||
- is_timeout(edev->last_link_check, 5 * SECOND)) {
- ret = phy_update_status(edev->phydev);
- if (ret)
- return ret;
- edev->last_link_check = get_time_ns();
- }
+ is_timeout(edev->last_link_check, 5 * SECOND))
+ eth_carrier_poll_once(edev);
if (may_wait && !edev->phydev->link) {
phy_wait_aneg_done(edev->phydev);
diff --git a/net/ifup.c b/net/ifup.c
index e4d5374db3ae..c491ea03c1c8 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -179,6 +179,28 @@ static void set_linux_bootarg(struct eth_device *edev)
}
}
+static int ifup_edev_conf(struct eth_device *edev, unsigned flags)
+{
+ int ret;
+
+ if (edev->global_mode == ETH_MODE_DHCP) {
+ if (IS_ENABLED(CONFIG_NET_DHCP)) {
+ ret = dhcp(edev, NULL);
+ } else {
+ dev_err(&edev->dev, "DHCP support not available\n");
+ ret = -ENOSYS;
+ }
+ if (ret)
+ return ret;
+ }
+
+ set_linux_bootarg(edev);
+
+ edev->ifup = true;
+
+ return 0;
+}
+
int ifup_edev(struct eth_device *edev, unsigned flags)
{
int ret;
@@ -205,22 +227,10 @@ int ifup_edev(struct eth_device *edev, unsigned flags)
if (ret)
return ret;
- if (edev->global_mode == ETH_MODE_DHCP) {
- if (IS_ENABLED(CONFIG_NET_DHCP)) {
- ret = dhcp(edev, NULL);
- } else {
- dev_err(&edev->dev, "DHCP support not available\n");
- ret = -ENOSYS;
- }
- if (ret)
- return ret;
- }
-
- set_linux_bootarg(edev);
-
- edev->ifup = true;
+ if (flags & IFUP_FLAG_SKIP_CONF)
+ return 1;
- return 0;
+ return ifup_edev_conf(edev, flags);
}
void ifdown_edev(struct eth_device *edev)
@@ -260,9 +270,54 @@ int ifdown(const char *ethname)
static int net_ifup_force_detect;
-int ifup_all(unsigned flags)
+static bool ifup_edev_need_conf(struct eth_device *edev)
+{
+ return edev->active && !edev->ifup &&
+ edev->global_mode != ETH_MODE_DISABLED;
+}
+
+static void __ifup_all_parallel(unsigned flags)
{
struct eth_device *edev;
+ unsigned netdev_count = 0;
+ u64 start;
+ int ret;
+
+ for_each_netdev(edev) {
+ ret = ifup_edev(edev, flags | IFUP_FLAG_SKIP_CONF);
+ if (ret == 1)
+ netdev_count++;
+ }
+
+ start = get_time_ns();
+ while (netdev_count && !is_timeout(start, PHY_AN_TIMEOUT * SECOND)) {
+ for_each_netdev(edev) {
+ if (!ifup_edev_need_conf(edev))
+ continue;
+
+ ret = eth_carrier_poll_once(edev);
+ if (ret)
+ continue;
+
+ ifup_edev_conf(edev, flags);
+ if (!edev->ifup)
+ continue;
+
+ netdev_count--;
+ }
+ }
+}
+
+static void __ifup_all_sequence(unsigned flags)
+{
+ struct eth_device *edev;
+
+ for_each_netdev(edev)
+ ifup_edev(edev, flags);
+}
+
+int ifup_all(unsigned flags)
+{
DIR *dir;
struct dirent *d;
@@ -285,8 +340,10 @@ int ifup_all(unsigned flags)
list_empty(&netdev_list))
device_detect_all();
- for_each_netdev(edev)
- ifup_edev(edev, flags);
+ if (flags & IFUP_FLAG_PARALLEL)
+ __ifup_all_parallel(flags);
+ else
+ __ifup_all_sequence(flags);
return 0;
}
@@ -315,14 +372,17 @@ BAREBOX_MAGICVAR(global.net.ifup_force_detect,
static int do_ifup(int argc, char *argv[])
{
int opt;
- unsigned flags = 0;
+ unsigned flags = IFUP_FLAG_PARALLEL;
int all = 0;
- while ((opt = getopt(argc, argv, "af")) > 0) {
+ while ((opt = getopt(argc, argv, "asf")) > 0) {
switch (opt) {
case 'f':
flags |= IFUP_FLAG_FORCE;
break;
+ case 's':
+ flags &= ~IFUP_FLAG_PARALLEL;
+ break;
case 'a':
all = 1;
break;
@@ -346,13 +406,14 @@ BAREBOX_CMD_HELP_TEXT("/env/network/<intf> file. See Documentation/user/networki
BAREBOX_CMD_HELP_TEXT("")
BAREBOX_CMD_HELP_TEXT("Options:")
BAREBOX_CMD_HELP_OPT ("-a", "bring up all interfaces")
+BAREBOX_CMD_HELP_OPT ("-s", "bring up interfaces in sequence, not in parallel")
BAREBOX_CMD_HELP_OPT ("-f", "Force. Configure even if ip already set")
BAREBOX_CMD_HELP_END
BAREBOX_CMD_START(ifup)
.cmd = do_ifup,
BAREBOX_CMD_DESC("bring a network interface up")
- BAREBOX_CMD_OPTS("[-af] [INTF]")
+ BAREBOX_CMD_OPTS("[-asf] [INTF]")
BAREBOX_CMD_GROUP(CMD_GRP_NET)
BAREBOX_CMD_COMPLETE(eth_complete)
BAREBOX_CMD_HELP(cmd_ifup_help)
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 2/3] net: ifup: have ifup -a1 stop at first DHCP-set global.net.server
2023-01-30 7:20 [PATCH v2 0/3] net: ifup: greatly reduce ifup -a time for multiple network interface Ahmad Fatoum
2023-01-30 7:20 ` [PATCH v2 1/3] net: ifup: have ifup -a poll for link up in parallel Ahmad Fatoum
@ 2023-01-30 7:20 ` Ahmad Fatoum
2023-01-30 7:20 ` [PATCH v2 3/3] defaultenv-2: automount: use ifup -a1 for NFS/TFTP automounts Ahmad Fatoum
2023-02-03 8:05 ` [PATCH v2 0/3] net: ifup: greatly reduce ifup -a time for multiple network interface Sascha Hauer
3 siblings, 0 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2023-01-30 7:20 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
The normal use case for ifup -a is to get *some* interface working and
not really wait for all interfaces to come up and then timeout waiting
for those without link up to never get a DHCP lease.
When using automounts with a previously empty $global.net.server, we
know this to be the case, because the first DHCP interface will set the
variable and all remaining ones won't affect this. Therefore, let's add
a ifup -a1 option, which would stop after $global.net.server was set via
DHCP. This is a special case of a possible future ifup -ar, which would
check after each ifup if $global.net.server was either newly set or
became resolvable, but that would be a bigger change, so we skip that
for now.
Times after eth_open_all has brought up CPU Ethernet and 4 DSA ports:
barebox$ time ifup -a
time: 10002ms
barebox$ time ifup -a1
time: 1072ms
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
- v1 -> v2:
- add new option for this behavior instead of just -a
- stop after global.net.server is set instead of just
stopping after first upped interface
---
include/net.h | 2 ++
net/ifup.c | 27 ++++++++++++++++++++++++---
net/net.c | 7 ++++++-
3 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/include/net.h b/include/net.h
index a7da282412fa..b7cbab7d720a 100644
--- a/include/net.h
+++ b/include/net.h
@@ -257,6 +257,7 @@ extern unsigned char *NetRxPackets[PKTBUFSRX];/* Receive packets */
void net_set_ip(struct eth_device *edev, IPaddr_t ip);
void net_set_serverip(IPaddr_t ip);
+const char *net_get_server(void);
void net_set_serverip_empty(IPaddr_t ip);
void net_set_netmask(struct eth_device *edev, IPaddr_t ip);
void net_set_gateway(IPaddr_t ip);
@@ -514,6 +515,7 @@ void led_trigger_network(enum led_trigger trigger);
#define IFUP_FLAG_FORCE (1 << 0)
#define IFUP_FLAG_PARALLEL (1 << 1)
#define IFUP_FLAG_SKIP_CONF (1 << 2)
+#define IFUP_FLAG_UNTIL_NET_SERVER (1 << 3)
int ifup_edev(struct eth_device *edev, unsigned flags);
int ifup(const char *name, unsigned flags);
diff --git a/net/ifup.c b/net/ifup.c
index c491ea03c1c8..64298b44ebf3 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -304,6 +304,9 @@ static void __ifup_all_parallel(unsigned flags)
continue;
netdev_count--;
+
+ if ((flags & IFUP_FLAG_UNTIL_NET_SERVER) && net_get_server())
+ return;
}
}
}
@@ -312,8 +315,12 @@ static void __ifup_all_sequence(unsigned flags)
{
struct eth_device *edev;
- for_each_netdev(edev)
+ for_each_netdev(edev) {
ifup_edev(edev, flags);
+
+ if ((flags & IFUP_FLAG_UNTIL_NET_SERVER) && net_get_server())
+ return;
+ }
}
int ifup_all(unsigned flags)
@@ -340,6 +347,16 @@ int ifup_all(unsigned flags)
list_empty(&netdev_list))
device_detect_all();
+ /*
+ * In the future, we may add an iproute -r option that tries to
+ * resolve $global.net.server using each interface. For now,
+ * we only support the special case of $global.net.server being
+ * empty, i.e. the first DHCP lease setting $global.net.server
+ * will be what we're going with.
+ */
+ if (net_get_server())
+ flags &= ~IFUP_FLAG_UNTIL_NET_SERVER;
+
if (flags & IFUP_FLAG_PARALLEL)
__ifup_all_parallel(flags);
else
@@ -375,11 +392,14 @@ static int do_ifup(int argc, char *argv[])
unsigned flags = IFUP_FLAG_PARALLEL;
int all = 0;
- while ((opt = getopt(argc, argv, "asf")) > 0) {
+ while ((opt = getopt(argc, argv, "asf1")) > 0) {
switch (opt) {
case 'f':
flags |= IFUP_FLAG_FORCE;
break;
+ case '1':
+ flags |= IFUP_FLAG_UNTIL_NET_SERVER;
+ break;
case 's':
flags &= ~IFUP_FLAG_PARALLEL;
break;
@@ -408,12 +428,13 @@ BAREBOX_CMD_HELP_TEXT("Options:")
BAREBOX_CMD_HELP_OPT ("-a", "bring up all interfaces")
BAREBOX_CMD_HELP_OPT ("-s", "bring up interfaces in sequence, not in parallel")
BAREBOX_CMD_HELP_OPT ("-f", "Force. Configure even if ip already set")
+BAREBOX_CMD_HELP_OPT ("-1", "Early exit if DHCP sets $global.net.server")
BAREBOX_CMD_HELP_END
BAREBOX_CMD_START(ifup)
.cmd = do_ifup,
BAREBOX_CMD_DESC("bring a network interface up")
- BAREBOX_CMD_OPTS("[-asf] [INTF]")
+ BAREBOX_CMD_OPTS("[-asf1] [INTF]")
BAREBOX_CMD_GROUP(CMD_GRP_NET)
BAREBOX_CMD_COMPLETE(eth_complete)
BAREBOX_CMD_HELP(cmd_ifup_help)
diff --git a/net/net.c b/net/net.c
index 22931509d0d9..19161d2e828e 100644
--- a/net/net.c
+++ b/net/net.c
@@ -325,9 +325,14 @@ void net_set_serverip(IPaddr_t ip)
net_server = xasprintf("%pI4", &ip);
}
+const char *net_get_server(void)
+{
+ return net_server && *net_server ? net_server : NULL;
+}
+
void net_set_serverip_empty(IPaddr_t ip)
{
- if (net_server && *net_server)
+ if (net_get_server())
return;
net_set_serverip(ip);
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 3/3] defaultenv-2: automount: use ifup -a1 for NFS/TFTP automounts
2023-01-30 7:20 [PATCH v2 0/3] net: ifup: greatly reduce ifup -a time for multiple network interface Ahmad Fatoum
2023-01-30 7:20 ` [PATCH v2 1/3] net: ifup: have ifup -a poll for link up in parallel Ahmad Fatoum
2023-01-30 7:20 ` [PATCH v2 2/3] net: ifup: have ifup -a1 stop at first DHCP-set global.net.server Ahmad Fatoum
@ 2023-01-30 7:20 ` Ahmad Fatoum
2023-02-03 8:05 ` [PATCH v2 0/3] net: ifup: greatly reduce ifup -a time for multiple network interface Sascha Hauer
3 siblings, 0 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2023-01-30 7:20 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
The ifup -a in the automounts has too big a scope: The remote server is
specified at mount time, so we could stop bringing up interfaces once
$global.net.server becomes resolvable. For the common case, where
$global.net.server is initially empty and then set via DHCP, ifup
provides the new -1 option to early-exit once $global.net.server was set.
For setups where one, but not all, Ethernet interfaces have a cable
attached, this normally reduces ifup -a time from 10s to 1s.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
- new patch
---
Documentation/user/automount.rst | 2 +-
defaultenv/defaultenv-2-base/boot/net | 2 +-
defaultenv/defaultenv-2-base/init/automount | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/user/automount.rst b/Documentation/user/automount.rst
index 1fdeffc66336..b41b41ef021d 100644
--- a/Documentation/user/automount.rst
+++ b/Documentation/user/automount.rst
@@ -15,7 +15,7 @@ TFTP server, the following is required:
.. code-block:: sh
mkdir -p /mnt/tftp
- automount /mnt/tftp 'ifup -a && mount -t tftp $global.net.server /mnt/tftp'
+ automount /mnt/tftp 'ifup -a1 && mount -t tftp $global.net.server /mnt/tftp'
This creates an automountpoint on ``/mnt/tftp``. Whenever this directory is accessed,
the command ``ifup eth0 && mount -t tftp $eth0.serverip /mnt/tftp`` is executed.
diff --git a/defaultenv/defaultenv-2-base/boot/net b/defaultenv/defaultenv-2-base/boot/net
index e79432eb277c..98286ff5e050 100644
--- a/defaultenv/defaultenv-2-base/boot/net
+++ b/defaultenv/defaultenv-2-base/boot/net
@@ -3,7 +3,7 @@
path="/mnt/tftp"
# global.net.server and global.hostname may be set by DHCP, so trigger it first
-ifup -a
+ifup -a1
global.bootm.image="${path}/${global.user}-linux-${global.hostname}"
diff --git a/defaultenv/defaultenv-2-base/init/automount b/defaultenv/defaultenv-2-base/init/automount
index 0996cea04df7..39e823489793 100644
--- a/defaultenv/defaultenv-2-base/init/automount
+++ b/defaultenv/defaultenv-2-base/init/automount
@@ -3,12 +3,12 @@
# automount tftp server
mkdir -p /mnt/tftp
-automount /mnt/tftp 'ifup -a && mount -t tftp $global.net.server /mnt/tftp'
+automount /mnt/tftp 'ifup -a1 && mount -t tftp $global.net.server /mnt/tftp'
# automount nfs server's nfsroot
mkdir -p /mnt/nfs
-automount /mnt/nfs 'ifup -a && mount -t nfs ${global.net.server}:/home/${global.user}/nfsroot/${global.hostname} /mnt/nfs'
+automount /mnt/nfs 'ifup -a1 && mount -t nfs ${global.net.server}:/home/${global.user}/nfsroot/${global.hostname} /mnt/nfs'
# FAT on usb disk example
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2 0/3] net: ifup: greatly reduce ifup -a time for multiple network interface
2023-01-30 7:20 [PATCH v2 0/3] net: ifup: greatly reduce ifup -a time for multiple network interface Ahmad Fatoum
` (2 preceding siblings ...)
2023-01-30 7:20 ` [PATCH v2 3/3] defaultenv-2: automount: use ifup -a1 for NFS/TFTP automounts Ahmad Fatoum
@ 2023-02-03 8:05 ` Sascha Hauer
3 siblings, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2023-02-03 8:05 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox
On Mon, Jan 30, 2023 at 08:20:54AM +0100, Ahmad Fatoum wrote:
> So far, ifup -a tries to up all non-disabled interfaces in sequence.
> This can take a quite a while, because interfaces with the link down,
> will be polled for 10s, before giving up on doing DHCP on them.
>
> This series accelerates this in the common case by doing link up
> check in parallel, getting down from (number_of_ports_wihout_link * 10s)
> to 10s and then adds an optimization for the common case of automounts
> with $global.net.server not set.
>
> Times after eth_open_all has brought up CPU Ethernet and 4 DSA ports:
>
> barebox$ time ifup -a -s # old behavior
> time: 31081ms
>
> barebox$ time ifup -a
> time: 10002ms
>
> barebox$ time ifup -a1
> time: 1072ms
>
> Ahmad Fatoum (3):
> net: ifup: have ifup -a poll for link up in parallel
> net: ifup: have ifup -a1 stop at first DHCP-set global.net.server
> defaultenv-2: automount: use ifup -a1 for NFS/TFTP automounts
Applied, thanks
Sascha
>
> Documentation/user/automount.rst | 2 +-
> defaultenv/defaultenv-2-base/boot/net | 2 +-
> defaultenv/defaultenv-2-base/init/automount | 4 +-
> include/net.h | 5 +
> net/eth.c | 28 +++--
> net/ifup.c | 124 ++++++++++++++++----
> net/net.c | 7 +-
> 7 files changed, 138 insertions(+), 34 deletions(-)
>
> --
> 2.30.2
>
>
>
--
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] 5+ messages in thread