[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#688218: unblock: busybox/1:1.20.0-7



Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Please unblock package busybox.

There aren't many changes, and they aren't large either, but fixes
a few problematic cases, and neither change has great breakage
potential.  Below each change is described in detail.

 * set CONFIG_FEATURE_COPYBUF_KB from 4 to 64 for all flavours.  This
   increases speed of various applets *dramatically*.  For example, wget
   applet downlad time for certain file decreased from 3.8sec to 0.6sec.

This is a rather trivial change which makes certain applets in busybox,
especially networking (wget, ftpget), actually useful.  On my 3.2GHz
AthlonII machine busybox wget, when copying file from a webserver
running locally, can achieve about 300Kb/sec speed, at 100% CPU usage.
This is because 4Kb-sized buffers as used by default are, well, very
small, and causes lots of extra/unnecessary syscalls.  Increase of runtime
memory is minimal for a modern system, but the speedup is really huge.
Without this change, wget in particular is barely useful on a local
network.

This change affects all variants, including d-i (udeb).

 * modprobe-read-modules-builtin.patch: stop modprobe from complaining
   when asked to load a "module" which is built-in.  Thank you
   Ben Hutchings for the patch. (Closes: #652672)

This is an important bugfix.  The problem is that modprobe complains
on every boot about being unable to load built-in modules requested
to be loaded during initramfs run on every Debian system, and this
message annoys and scare users and gives bad impression about Debian
as whole - initramfs is the first thing users see when booting their
systems.

 * udeb: enable ping6 applet and FEATURE_FANCY_PING - more featureful
   ping(6) variant with statistics et al.  This makes ping6 available in
   d-i (initial network debugging), and makes ping there not only more
   useful but also using a more tested code, as used in other variants.
   4Kb file size increase on i386.

This change only affects udeb (d-i), and is only about auxilary minimal
networking debugging tool, ping.  After this change, it is possible to
use full-featured ping in d-i, and to use ping6 as well - which is very
usefult to diagnose network-related problems, *especially* on IPv6 since
it is a new (for d-i) environment and some stuff may not work right.
Note that even other busybox applets - wget to name one - are having
one or other problems with IPv6 (wget does not fallback to IPv4 if
IPv6 address for a host does not respond, there's a bug about that).
In d-i, with its minimal amount of tools available, network debugging
is important to have.

This change not only enables ping6, but also switches to common, more
featured ping implementation as used in other variants.  There's no
reason to save 4Kb of code in todays, even embedded, systems for
having barely useful tools - "non-fancy" ping was about 600 bytes
but it only printed "host is alive" or "timeout", nothing more.

 * stop-checking-ancient-kernel-version.patch: a rather trivial patch
   to disable compatibility code for kernels <2.2.18 in nfs mount.
   This was the only place in debian busybox which used
   get_linux_version_code() function which was buggy on kernels with
   less than 3 components version number.  So instead of fixing that
   function, we got rid of the last user of it.  (Closes: #684611)

This one is rather fun.  Before this change, busybox's mount were
segfaulting with 2-component kernel version string, like will be
used past wheezy.  Mount applet was the only place where common
function, get_linux_version_code(), was called (other place is
modutils, to decide whenever use pre-2.4 or post-2.4 kernel module
interface, but pre-2.4 code is disabled in debian package for a
long time).  In mount, that was in order to check for different
interface for NFS mount, which changed in 2.2.18 kernel.

Initially I added a patch to teach get_linux_version_code() about
less-than-3-component version strings, but later Philipp Kern asked
_why_ it checks version, and I decided to just remove the whole
thing and always assume our kernel is > 2.2.18.

This change is not useful for wheezy itself since its official
kernel has 3-component version string (3.2.0), but people will
definitely try more recent kernels on wheezy, and will bug us
about busybox segfaulting.

The change is a band-aid really, it just #define's some info
which were determined at runtime to a fixed value, so lots
of dead code remains after that, proper upstream cleanup is
necessary which I didn't want to do this late in the release
process.

I verified that the resulting code works correctly, including
kernels with 2-component versions :)

Complete debdiff is below.

Thank you for your time!

/mjt

unblock busybox/1:1.20.0-7


diff -Nru busybox-1.20.0/debian/changelog busybox-1.20.0/debian/changelog
--- busybox-1.20.0/debian/changelog	2012-07-22 12:30:25.000000000 +0400
+++ busybox-1.20.0/debian/changelog	2012-09-20 10:32:55.000000000 +0400
@@ -1,3 +1,25 @@
+busybox (1:1.20.0-7) unstable; urgency=low
+
+  * set CONFIG_FEATURE_COPYBUF_KB from 4 to 64 for all flavours.  This
+    increases speed of various applets *dramatically*.  For example, wget
+    applet downlad time for certain file decreased from 3.8sec to 0.6sec.
+  * modprobe-read-modules-builtin.patch: stop modprobe from complaining
+    when asked to load a "module" which is built-in.  Thank you
+    Ben Hutchings for the patch. (Closes: #652672)
+  * udeb: enable ping6 applet and FEATURE_FANCY_PING - more featureful
+    ping(6) variant with statistics et al.  This makes ping6 available in
+    d-i (initial network debugging), and makes ping there not only more
+    useful but also using a more tested code, as used in other variants.
+    4Kb file size increase on i386.
+  * stop-checking-ancient-kernel-version.patch: a rather trivial patch
+    to disable compatibility code for kernels <2.2.18 in nfs mount.
+    This was the only place in debian busybox which used
+    get_linux_version_code() function which was buggy on kernels with
+    less than 3 components version number.  So instead of fixing that
+    function, we got rid of the last user of it.  (Closes: #684611)
+
+ -- Michael Tokarev <mjt@tls.msk.ru>  Thu, 20 Sep 2012 10:32:55 +0400
+
 busybox (1:1.20.0-6) unstable; urgency=low
 
   * reorder patches in debian/patches/series: all upstream first,
diff -Nru busybox-1.20.0/debian/config/pkg/deb busybox-1.20.0/debian/config/pkg/deb
--- busybox-1.20.0/debian/config/pkg/deb	2012-07-06 11:54:06.000000000 +0400
+++ busybox-1.20.0/debian/config/pkg/deb	2012-09-17 19:36:37.000000000 +0400
@@ -112,7 +112,7 @@
 CONFIG_FEATURE_EDITING_ASK_TERMINAL=y
 # CONFIG_FEATURE_NON_POSIX_CP is not set
 CONFIG_FEATURE_VERBOSE_CP_MESSAGE=y
-CONFIG_FEATURE_COPYBUF_KB=4
+CONFIG_FEATURE_COPYBUF_KB=64
 CONFIG_FEATURE_SKIP_ROOTFS=y
 CONFIG_MONOTONIC_SYSCALL=y
 CONFIG_IOCTL_HEX2STR_ERROR=y
diff -Nru busybox-1.20.0/debian/config/pkg/static busybox-1.20.0/debian/config/pkg/static
--- busybox-1.20.0/debian/config/pkg/static	2012-07-06 11:54:06.000000000 +0400
+++ busybox-1.20.0/debian/config/pkg/static	2012-09-17 19:42:22.000000000 +0400
@@ -112,7 +112,7 @@
 CONFIG_FEATURE_EDITING_ASK_TERMINAL=y
 # CONFIG_FEATURE_NON_POSIX_CP is not set
 CONFIG_FEATURE_VERBOSE_CP_MESSAGE=y
-CONFIG_FEATURE_COPYBUF_KB=4
+CONFIG_FEATURE_COPYBUF_KB=64
 CONFIG_FEATURE_SKIP_ROOTFS=y
 CONFIG_MONOTONIC_SYSCALL=y
 CONFIG_IOCTL_HEX2STR_ERROR=y
diff -Nru busybox-1.20.0/debian/config/pkg/udeb busybox-1.20.0/debian/config/pkg/udeb
--- busybox-1.20.0/debian/config/pkg/udeb	2012-07-06 19:21:26.000000000 +0400
+++ busybox-1.20.0/debian/config/pkg/udeb	2012-09-20 01:22:14.000000000 +0400
@@ -112,7 +112,7 @@
 CONFIG_FEATURE_EDITING_ASK_TERMINAL=y
 # CONFIG_FEATURE_NON_POSIX_CP is not set
 CONFIG_FEATURE_VERBOSE_CP_MESSAGE=y
-CONFIG_FEATURE_COPYBUF_KB=4
+CONFIG_FEATURE_COPYBUF_KB=64
 CONFIG_FEATURE_SKIP_ROOTFS=y
 CONFIG_MONOTONIC_SYSCALL=y
 CONFIG_IOCTL_HEX2STR_ERROR=y
@@ -726,8 +726,8 @@
 CONFIG_NC_EXTRA=y
 # CONFIG_NC_110_COMPAT is not set
 CONFIG_PING=y
-# CONFIG_PING6 is not set
-# CONFIG_FEATURE_FANCY_PING is not set
+CONFIG_PING6=y
+CONFIG_FEATURE_FANCY_PING=y
 # CONFIG_WHOIS is not set
 CONFIG_FEATURE_IPV6=y
 # CONFIG_FEATURE_UNIX_LOCAL is not set
diff -Nru busybox-1.20.0/debian/patches/modprobe-read-modules-builtin.patch busybox-1.20.0/debian/patches/modprobe-read-modules-builtin.patch
--- busybox-1.20.0/debian/patches/modprobe-read-modules-builtin.patch	1970-01-01 03:00:00.000000000 +0300
+++ busybox-1.20.0/debian/patches/modprobe-read-modules-builtin.patch	2012-08-12 22:31:13.000000000 +0400
@@ -0,0 +1,52 @@
+From: Ben Hutchings <ben@decadent.org.uk>
+Subject: modprobe: Read modules.builtin
+Bug-Debian: http://bugs.debian.org/652672
+
+This allows explicit probing to succeed when the requested module
+is actually built-in, and corrects the error message for removal.
+
+Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
+--- a/modutils/modprobe.c
++++ b/modutils/modprobe.c
+@@ -146,6 +146,7 @@ static const char modprobe_longopts[] AL
+ /* "was seen in modules.dep": */
+ #define MODULE_FLAG_FOUND_IN_MODDEP     0x0004
+ #define MODULE_FLAG_BLACKLISTED         0x0008
++#define MODULE_FLAG_BUILTIN             0x0010
+ 
+ struct module_entry { /* I'll call it ME. */
+ 	unsigned flags;
+@@ -240,7 +241,7 @@ static void add_probe(const char *name)
+ 
+ 	m = get_or_add_modentry(name);
+ 	if (!(option_mask32 & (OPT_REMOVE | OPT_SHOW_DEPS))
+-	 && (m->flags & MODULE_FLAG_LOADED)
++	 && (m->flags & (MODULE_FLAG_LOADED | MODULE_FLAG_BUILTIN))
+ 	) {
+ 		DBG("skipping %s, it is already loaded", name);
+ 		return;
+@@ -394,8 +395,10 @@ static int do_modprobe(struct module_ent
+ 
+ 	if (!(m->flags & MODULE_FLAG_FOUND_IN_MODDEP)) {
+ 		if (!(option_mask32 & INSMOD_OPT_SILENT))
+-			bb_error_msg("module %s not found in modules.dep",
+-				humanly_readable_name(m));
++			bb_error_msg((m->flags & MODULE_FLAG_BUILTIN) ?
++				     "module %s is builtin" :
++				     "module %s not found in modules.dep",
++				     humanly_readable_name(m));
+ 		return -ENOENT;
+ 	}
+ 	DBG("do_modprob'ing %s", m->modname);
+@@ -595,6 +598,11 @@ int modprobe_main(int argc UNUSED_PARAM,
+ 		while (config_read(parser, &s, 1, 1, "# \t", PARSE_NORMAL & ~PARSE_GREEDY))
+ 			get_or_add_modentry(s)->flags |= MODULE_FLAG_LOADED;
+ 		config_close(parser);
++
++		parser = config_open2("modules.builtin", fopen_for_read);
++		while (config_read(parser, &s, 1, 1, "# \t", PARSE_NORMAL))
++			get_or_add_modentry(s)->flags |= MODULE_FLAG_BUILTIN;
++		config_close(parser);
+ 	}
+ 
+ 	if (opt & (OPT_INSERT_ALL | OPT_REMOVE)) {
diff -Nru busybox-1.20.0/debian/patches/series busybox-1.20.0/debian/patches/series
--- busybox-1.20.0/debian/patches/series	2012-07-22 11:59:35.000000000 +0400
+++ busybox-1.20.0/debian/patches/series	2012-09-20 00:58:00.000000000 +0400
@@ -19,5 +19,9 @@
 u-mount-FreeBSD-support.patch
 swaponoff-FreeBSD-support.patch
 
+modprobe-read-modules-builtin.patch
+
 # http://bugs.debian.org/681760
 dont-force-no-alignment-for-s390.patch
+
+stop-checking-ancient-kernel-version.patch
diff -Nru busybox-1.20.0/debian/patches/stop-checking-ancient-kernel-version.patch busybox-1.20.0/debian/patches/stop-checking-ancient-kernel-version.patch
--- busybox-1.20.0/debian/patches/stop-checking-ancient-kernel-version.patch	1970-01-01 03:00:00.000000000 +0300
+++ busybox-1.20.0/debian/patches/stop-checking-ancient-kernel-version.patch	2012-09-20 00:57:42.000000000 +0400
@@ -0,0 +1,54 @@
+From: Michael Tokarev <mjt@tls.msk.ru>
+Subject: stop checking ancient kernel version for NFS mount
+Forwarded: no
+Bug-Debian: http://bugs.debian.org/684611
+
+The nfs mount code checks for ancient kernel 2.2.18 (!) to determine
+which mount protocol to use (v3 or v4).  Stop doing this, and always
+use v4.
+
+This is the only place in debian busybox which uses get_linux_version_code()
+function which can't deal with less-than-3-component kernel version numbers
+(#684611).  (Other places are in modutils/ to determine whenever to use
+pre-2.4 module loading way, which is disabled in debian build).
+
+This is a band-aid patch, to minimize changes, more complete cleanup
+is needed for all this code upstream.
+
+--- a/util-linux/mount.c
++++ b/util-linux/mount.c
+@@ -285,9 +285,6 @@
+ 
+ 
+ struct globals {
+-#if ENABLE_FEATURE_MOUNT_NFS
+-	smalluint nfs_mount_version;
+-#endif
+ #if ENABLE_FEATURE_MOUNT_VERBOSE
+ 	unsigned verbose;
+ #endif
+@@ -296,7 +293,7 @@
+ } FIX_ALIASING;
+ enum { GETMNTENT_BUFSIZE = COMMON_BUFSIZE - offsetof(struct globals, getmntent_buf) };
+ #define G (*(struct globals*)&bb_common_bufsiz1)
+-#define nfs_mount_version (G.nfs_mount_version)
++#define nfs_mount_version 4 /* assume kernel>= 2.4, use v4 nfs mount protocol */
+ #if ENABLE_FEATURE_MOUNT_VERBOSE
+ #define verbose           (G.verbose          )
+ #else
+@@ -937,6 +934,7 @@
+ static void
+ find_kernel_nfs_mount_version(void)
+ {
++#if 0
+ 	int kernel_version;
+ 
+ 	if (nfs_mount_version)
+@@ -950,6 +948,7 @@
+ 			nfs_mount_version = 3;
+ 		/* else v4 since 2.3.99pre4 */
+ 	}
++#endif
+ }
+ 
+ static void
diff -Nru busybox-1.20.0/debian/udeb-sizes busybox-1.20.0/debian/udeb-sizes
--- busybox-1.20.0/debian/udeb-sizes	2012-07-06 19:21:26.000000000 +0400
+++ busybox-1.20.0/debian/udeb-sizes	2012-09-20 00:32:27.000000000 +0400
@@ -94,3 +94,17 @@
 ping applet, useful for basic network debugging/testing, +613 bytes.
 Enable it.
 
+20120917 mjt i386 increase FEATURE_COPYBUF_KB from 4 to 64
+   text    data     bss     dec     hex filename
+ 312716    1669    8712  323097   4ee19 busybox+
+ 312953    1677    8712  323342   4ef0e busybox+buf64k
+Icreases copy (cp, wget, ...) speed dramatically.
+
+20120920 mjt i386 PING6 and FEATURE_FANCY_PING
+   text    data     bss     dec     hex filename
+ 312953    1677    8712  323342   4ef0e busybox+
+ 315380    1681    8712  325773   4f88d busybox+fancyping
+ 313441    1677    8712  323830   4f0f6 busybox+ping6
+ 317005    1685    8712  327402   4feea busybox+fancyping+ping6
+4Kb code and 8b data increase for more useful ping and ping6,
+both useful for initial network debugging, especially ping6.


Reply to: