[PATCH-suggestion] The pidfile problem of udhcpc

Denis Vlasenko vda.linux at googlemail.com
Tue Jul 3 08:51:44 UTC 2007


On Monday 02 July 2007 16:54, Mats Erik Andersson wrote:
> Hi again,
> 
> I have analyzed and located the problem with udhcpc
> for todays snapshot of Busybox when one uses
> 
> # CONFIG_FEATURE_PIDFILE is not set
> 
> Attached are three files:
> 
>   utsaga_ifdown.txt     is a manual recovery and error listing
> 
>   ifupdown_without_pidfile.diff    is a functional patch, see below
> 
>   bb_udhcpc_script_error_handling.diff    mends a related problem
> 
> 
> My original issue was that when ENABLE_FEATURE_PIDFILE=0 causes
> an error
> 
>    ifdown eth0    when eth0 is dhcp-controled
>       --> udhcpc -->  ifupdown  -->  execute -->
>          --> cat /var/run/udhcpc.eth0.pid    has an empty argument
>            ----> kill udhcpc fails and 'ifdown eth0' fails midway

So it's not a problem with udhcpd. It works perfectly fine, it just
does not create pidfile.

The problem is with ifupdown.

The general problem with ifupdown is that it is "copulated in vertical
fashion" by design. It tries to do the job of shell script in C,
and this is invariably doomed to fail. You need ifup/ifdown
to be adaptable by local admins, and C is an extremely poor choice
for that.

We are doomed to have problems with ifup/ifdown. Just look as this code:

static const struct dhcp_client_t ext_dhcp_clients[] = {
        { "dhcpcd", "<up cmd>", "<down cmd>" },
        { "dhclient", ........ },
        { "pump", ........ },
        { "udhcpc", ........ },
};

static int dhcp_down(struct interface_defn_t *ifd, execfn *exec)
{
#if ENABLE_FEATURE_IFUPDOWN_EXTERNAL_DHCP
        int i ;
        for (i = 0; i < ARRAY_SIZE(ext_dhcp_clients); i++) {
                if (exists_execable(ext_dhcp_clients[i].name))
                        return execute(ext_dhcp_clients[i].stopcmd, ifd, exec);
        }
        bb_error_msg("no dhcp clients found, using static interface shutdown");
        return static_down(ifd, exec);
#elif ENABLE_APP_UDHCPC
        return execute("kill "
                       "`cat /var/run/udhcpc.%iface%.pid` 2>/dev/null", ifd, exec);
#else
        return 0; /* no dhcp support */
#endif
}

How the hell it is supposed to work reliably this way?? Just imagine that
admin is using pump and ifup/ifdown. It works. Then, for whatever reason,
admin installs dhclient, but does NOT use it. ifdown will STOP WORKING,
just because it will see installed dhclient binary in e.g. /usr/bin/dhclient!
Ain't it idiotic?

I am sure we will fix problem with udhcpc pidfiles, one way or another.
But I seriously urge people to not use ifup/ifdown.
Use something less brain damaged.
--
vda



More information about the busybox mailing list