| Message ID | 20220329205136.32435-1-vincent.stehle@laposte.net |
|---|---|
| State | Accepted |
| Headers | show |
| Series | configs/qemu_xtensa_lx60_nommu: use busybox minimal config | expand |
On 29/03/2022 22:51, Vincent Stehlé via buildroot wrote: > Update the qemu_xtensa_lx60_nommu_defconfig to use the > busybox-minimal.config, to make it more consistent with the other no-MMU > defconfigs. That's not a valid reason IMHO. We use the minimal busybox for boards that are extremely tight on memory - which is often the case for noMMU boards. But if we can spare the size, full busybox is a lot more useable. I've marked this patch as Rejected in patchwork, but if there's a good reason to do this, we can always recover it. Regards, Arnout > > After commit 3de486f8b052 ("package/busybox: fix udhcpc options in minimal > config"), this has the benefit of fixing the following network > initialization failure: > > udhcpc: invalid option -- b > > Signed-off-by: Vincent Stehlé <vincent.stehle@laposte.net> > Cc: Romain Naour <romain.naour@gmail.com> > Cc: Gerome Burlats <gerome.burlats@smile.fr> > --- > configs/qemu_xtensa_lx60_nommu_defconfig | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/configs/qemu_xtensa_lx60_nommu_defconfig b/configs/qemu_xtensa_lx60_nommu_defconfig > index c4473fb32a..44fb81bd74 100644 > --- a/configs/qemu_xtensa_lx60_nommu_defconfig > +++ b/configs/qemu_xtensa_lx60_nommu_defconfig > @@ -7,6 +7,9 @@ BR2_XTENSA_OVERLAY_FILE="https://github.com/jcmvbkbc/xtensa-toolchain-build/raw/ > BR2_PACKAGE_HOST_ELF2FLT=y > # BR2_USE_MMU is not set > > +# Use minimal busybox with hush and networking tools > +BR2_PACKAGE_BUSYBOX_CONFIG="package/busybox/busybox-minimal.config" > + > # System > BR2_SYSTEM_DHCP="eth0" > BR2_TARGET_GENERIC_GETTY_PORT="ttyS0"
On Sun, Apr 03, 2022 at 05:07:48PM +0200, Arnout Vandecappelle wrote: > > > On 29/03/2022 22:51, Vincent Stehlé via buildroot wrote: > > Update the qemu_xtensa_lx60_nommu_defconfig to use the > > busybox-minimal.config, to make it more consistent with the other no-MMU > > defconfigs. > > That's not a valid reason IMHO. We use the minimal busybox for boards that > are extremely tight on memory - which is often the case for noMMU boards. > But if we can spare the size, full busybox is a lot more useable. Hi Arnout, Thanks for reviewing this patch. > I've marked this patch as Rejected in patchwork, but if there's a good > reason to do this, we can always recover it. There is another incentive to using the minimal config on no-MMU platforms: it fixes the udhcpc error and repairs the network initialization. Just let me know if you want me to reword the commit message to insist more on this aspect and submit a v2. Or we can abandon it for good if you prefer. Best regards, Vincent. > > Regards, > Arnout > > > > > After commit 3de486f8b052 ("package/busybox: fix udhcpc options in minimal > > config"), this has the benefit of fixing the following network > > initialization failure: > > > > udhcpc: invalid option -- b > > > > Signed-off-by: Vincent Stehlé <vincent.stehle@laposte.net> > > Cc: Romain Naour <romain.naour@gmail.com> > > Cc: Gerome Burlats <gerome.burlats@smile.fr> > > --- > > configs/qemu_xtensa_lx60_nommu_defconfig | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/configs/qemu_xtensa_lx60_nommu_defconfig b/configs/qemu_xtensa_lx60_nommu_defconfig > > index c4473fb32a..44fb81bd74 100644 > > --- a/configs/qemu_xtensa_lx60_nommu_defconfig > > +++ b/configs/qemu_xtensa_lx60_nommu_defconfig > > @@ -7,6 +7,9 @@ BR2_XTENSA_OVERLAY_FILE="https://github.com/jcmvbkbc/xtensa-toolchain-build/raw/ > > BR2_PACKAGE_HOST_ELF2FLT=y > > # BR2_USE_MMU is not set > > +# Use minimal busybox with hush and networking tools > > +BR2_PACKAGE_BUSYBOX_CONFIG="package/busybox/busybox-minimal.config" > > + > > # System > > BR2_SYSTEM_DHCP="eth0" > > BR2_TARGET_GENERIC_GETTY_PORT="ttyS0"
On 04/04/2022 21:38, Vincent Stehlé wrote: > On Sun, Apr 03, 2022 at 05:07:48PM +0200, Arnout Vandecappelle wrote: >> >> >> On 29/03/2022 22:51, Vincent Stehlé via buildroot wrote: >>> Update the qemu_xtensa_lx60_nommu_defconfig to use the >>> busybox-minimal.config, to make it more consistent with the other no-MMU >>> defconfigs. >> >> That's not a valid reason IMHO. We use the minimal busybox for boards that >> are extremely tight on memory - which is often the case for noMMU boards. >> But if we can spare the size, full busybox is a lot more useable. > > Hi Arnout, > > Thanks for reviewing this patch. > >> I've marked this patch as Rejected in patchwork, but if there's a good >> reason to do this, we can always recover it. > > There is another incentive to using the minimal config on no-MMU platforms: it > fixes the udhcpc error and repairs the network initialization. Ooh, now I understand what that commit does... But that actually causes a problem: if udhcpc doesn't keep running the background any more, it's not going to refresh its lease, but will keep on using its IP address indefinitely. Although this seems to work at first hand, it's going to cause some problem somewhere down the line... So we should find a different solution, and we should probably use that in the MMU case as well (just to keep things consistent. Regards, Arnout > > Just let me know if you want me to reword the commit message to insist more on > this aspect and submit a v2. Or we can abandon it for good if you prefer. > > Best regards, > Vincent. > >> >> Regards, >> Arnout >> >>> >>> After commit 3de486f8b052 ("package/busybox: fix udhcpc options in minimal >>> config"), this has the benefit of fixing the following network >>> initialization failure: >>> >>> udhcpc: invalid option -- b >>> >>> Signed-off-by: Vincent Stehlé <vincent.stehle@laposte.net> >>> Cc: Romain Naour <romain.naour@gmail.com> >>> Cc: Gerome Burlats <gerome.burlats@smile.fr> >>> --- >>> configs/qemu_xtensa_lx60_nommu_defconfig | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/configs/qemu_xtensa_lx60_nommu_defconfig b/configs/qemu_xtensa_lx60_nommu_defconfig >>> index c4473fb32a..44fb81bd74 100644 >>> --- a/configs/qemu_xtensa_lx60_nommu_defconfig >>> +++ b/configs/qemu_xtensa_lx60_nommu_defconfig >>> @@ -7,6 +7,9 @@ BR2_XTENSA_OVERLAY_FILE="https://github.com/jcmvbkbc/xtensa-toolchain-build/raw/ >>> BR2_PACKAGE_HOST_ELF2FLT=y >>> # BR2_USE_MMU is not set >>> +# Use minimal busybox with hush and networking tools >>> +BR2_PACKAGE_BUSYBOX_CONFIG="package/busybox/busybox-minimal.config" >>> + >>> # System >>> BR2_SYSTEM_DHCP="eth0" >>> BR2_TARGET_GENERIC_GETTY_PORT="ttyS0"
On Mon, Apr 04, 2022 at 10:28:12PM +0200, Arnout Vandecappelle wrote: .. > But that actually causes a problem: if udhcpc doesn't keep running the > background any more, it's not going to refresh its lease, but will keep on > using its IP address indefinitely. Although this seems to work at first > hand, it's going to cause some problem somewhere down the line... Hi Arnout, After your comment at first I thought all the no-MMU buildroot defconfigs were subtly broken :-S I did some investigations and now my understanding is that udhcpc does indeed daemonize by default in the no-MMU case. See [1]: #if !BB_MMU /* on NOMMU reexec (i.e., background) early */ if (!(opt & OPT_f)) { bb_daemonize_or_rexec(0 /* flags */, argv); logmode = LOGMODE_NONE; } #endif That behaviour was added in busybox udhcpc while removing the -b option (in commit [2]). Also, this is confirmed on qemu where we can see the udhcpc process running in the background: ~ # ps PID USER VSZ STAT COMMAND 1 root 616 S init ... 62 root 612 S udhcpc -R -p /var/run/udhcpc.eth0.pid -i eth0 -x hos > So we should find a different solution, and we should probably use that in > the MMU case as well (just to keep things consistent. Now I think that we are safe. What is your opinion on this, please? Best regards, Vincent. [1]: https://git.busybox.net/busybox/tree/networking/udhcp/dhcpc.c#n1352 [2]: https://git.busybox.net/busybox/commit/?id=21765fa063830923d13426ec6989c16da9210e49
On 06/04/2022 22:00, Vincent Stehlé wrote: > On Mon, Apr 04, 2022 at 10:28:12PM +0200, Arnout Vandecappelle wrote: > .. >> But that actually causes a problem: if udhcpc doesn't keep running the >> background any more, it's not going to refresh its lease, but will keep on >> using its IP address indefinitely. Although this seems to work at first >> hand, it's going to cause some problem somewhere down the line... > > Hi Arnout, > > After your comment at first I thought all the no-MMU buildroot defconfigs were > subtly broken :-S > > I did some investigations and now my understanding is that udhcpc does indeed > daemonize by default in the no-MMU case. See [1]: > > #if !BB_MMU > /* on NOMMU reexec (i.e., background) early */ > if (!(opt & OPT_f)) { > bb_daemonize_or_rexec(0 /* flags */, argv); > logmode = LOGMODE_NONE; > } > #endif > > That behaviour was added in busybox udhcpc while removing the -b option (in > commit [2]). > > Also, this is confirmed on qemu where we can see the udhcpc process running in > the background: > > ~ # ps > PID USER VSZ STAT COMMAND > 1 root 616 S init > ... > 62 root 612 S udhcpc -R -p /var/run/udhcpc.eth0.pid -i eth0 -x hos > >> So we should find a different solution, and we should probably use that in >> the MMU case as well (just to keep things consistent. > > Now I think that we are safe. What is your opinion on this, please? I'll add a summary of this entire explanation and commit it. Regards, Arnout > > Best regards, > Vincent. > > [1]: https://git.busybox.net/busybox/tree/networking/udhcp/dhcpc.c#n1352 > [2]: https://git.busybox.net/busybox/commit/?id=21765fa063830923d13426ec6989c16da9210e49
On 29/03/2022 22:51, Vincent Stehlé via buildroot wrote: > Update the qemu_xtensa_lx60_nommu_defconfig to use the > busybox-minimal.config, to make it more consistent with the other no-MMU > defconfigs. > > After commit 3de486f8b052 ("package/busybox: fix udhcpc options in minimal > config"), this has the benefit of fixing the following network > initialization failure: > > udhcpc: invalid option -- b > > Signed-off-by: Vincent Stehlé <vincent.stehle@laposte.net> > Cc: Romain Naour <romain.naour@gmail.com> > Cc: Gerome Burlats <gerome.burlats@smile.fr> As promised: applied to master with an extended commit message. However, the proper approach would be to revert both this and 3de486f8b052, and instead add a config fixup to busybox.mk that removes the -b option from CONFIG_IFUPDOWN_UDHCPC_CMD_OPTIONS on NOMMU. The regex to match it is going to be tricky to write though... (/CONFIG_IFUPDOWN_UDHCPC_CMD_OPTIONS="-b\b/ works for our config, but we also want to remove it from a user-supplied config where -b may not be the first option). So in order to get this fixed expediently, I applied this patch. Regards, Arnout > --- > configs/qemu_xtensa_lx60_nommu_defconfig | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/configs/qemu_xtensa_lx60_nommu_defconfig b/configs/qemu_xtensa_lx60_nommu_defconfig > index c4473fb32a..44fb81bd74 100644 > --- a/configs/qemu_xtensa_lx60_nommu_defconfig > +++ b/configs/qemu_xtensa_lx60_nommu_defconfig > @@ -7,6 +7,9 @@ BR2_XTENSA_OVERLAY_FILE="https://github.com/jcmvbkbc/xtensa-toolchain-build/raw/ > BR2_PACKAGE_HOST_ELF2FLT=y > # BR2_USE_MMU is not set > > +# Use minimal busybox with hush and networking tools > +BR2_PACKAGE_BUSYBOX_CONFIG="package/busybox/busybox-minimal.config" > + > # System > BR2_SYSTEM_DHCP="eth0" > BR2_TARGET_GENERIC_GETTY_PORT="ttyS0"
>>>>> "Vincent" == Vincent Stehlé via buildroot <buildroot@buildroot.org> writes: > Update the qemu_xtensa_lx60_nommu_defconfig to use the > busybox-minimal.config, to make it more consistent with the other no-MMU > defconfigs. > After commit 3de486f8b052 ("package/busybox: fix udhcpc options in minimal > config"), this has the benefit of fixing the following network > initialization failure: > udhcpc: invalid option -- b > Signed-off-by: Vincent Stehlé <vincent.stehle@laposte.net> > Cc: Romain Naour <romain.naour@gmail.com> > Cc: Gerome Burlats <gerome.burlats@smile.fr> Committed to 2022.02.x, thanks.
diff --git a/configs/qemu_xtensa_lx60_nommu_defconfig b/configs/qemu_xtensa_lx60_nommu_defconfig index c4473fb32a..44fb81bd74 100644 --- a/configs/qemu_xtensa_lx60_nommu_defconfig +++ b/configs/qemu_xtensa_lx60_nommu_defconfig @@ -7,6 +7,9 @@ BR2_XTENSA_OVERLAY_FILE="https://github.com/jcmvbkbc/xtensa-toolchain-build/raw/ BR2_PACKAGE_HOST_ELF2FLT=y # BR2_USE_MMU is not set +# Use minimal busybox with hush and networking tools +BR2_PACKAGE_BUSYBOX_CONFIG="package/busybox/busybox-minimal.config" + # System BR2_SYSTEM_DHCP="eth0" BR2_TARGET_GENERIC_GETTY_PORT="ttyS0"
Update the qemu_xtensa_lx60_nommu_defconfig to use the busybox-minimal.config, to make it more consistent with the other no-MMU defconfigs. After commit 3de486f8b052 ("package/busybox: fix udhcpc options in minimal config"), this has the benefit of fixing the following network initialization failure: udhcpc: invalid option -- b Signed-off-by: Vincent Stehlé <vincent.stehle@laposte.net> Cc: Romain Naour <romain.naour@gmail.com> Cc: Gerome Burlats <gerome.burlats@smile.fr> --- configs/qemu_xtensa_lx60_nommu_defconfig | 3 +++ 1 file changed, 3 insertions(+)