Denys Vlasenko wrote:
>
>
> Had to edit it quite a bit. You added #include's which were not needed.
> You removed memcmp without explaining why.
> send_decline()'s stype was different from surrounding code.
>
> +static int arpcheck = 0;
> +static int decline_wait = 10;
>
> These statics are not needed, local variables will work as well.
>
> I fixed these things, and possibly added my own bugs ;)
>
> Please test attached patch, is it still doing what you need?
>
> Question: do you really need "-W seconds"?
> Why not reuse "-A seconds" value?
> --
> vda
>
>
Hi vda,
The patch still does what I need. Thanks alot for your comments and
modifications!
But I had problems building it at first. Could you explain to me what
this snippet does?
UDHCPC_NEEDS_ARPING-$(CONFIG_FEATURE_UDHCPC_ARPING) = y
lib-$(UDHCPC_NEEDS_ARPING) += arpping.o
The build process failed to link in arpping.o for me so I had to work
around it to make it
compile. I can't find any other references to UDHCPC_NEEDS_ARPING in the
codebase.
About the -W seconds. I added it because of what was pointed out in the
RFC2131:
5. The client receives the DHCPACK message with configuration
parameters. The client SHOULD perform a final check on the
parameters (e.g., ARP for allocated network address), and notes the
duration of the lease specified in the DHCPACK message. At this
point, the client is configured. If the client detects that the
address is already in use (e.g., through the use of ARP), the
client MUST send a DHCPDECLINE message to the server and restarts
the configuration process. The client SHOULD wait a minimum of ten
seconds before restarting the configuration process to avoid
excessive network traffic in case of looping.
It is the matter of the minimum of 10 seconds that made me include it.
But maybe it is better to reuse the --tryagain flag
and set a lower bound on it?
- Jonas
> ------------------------------------------------------------------------
>
> diff -d -urpN busybox.1/include/usage.h busybox.2/include/usage.h
> --- busybox.1/include/usage.h 2007-11-17 19:56:45.000000000 -0800
> +++ busybox.2/include/usage.h 2007-11-21 14:28:52.000000000 -0800
> @@ -2183,8 +2183,8 @@
> "and * (run both after creating and before deleting). The commands
> run in\n" \
> "the /dev directory, and use system() which calls /bin/sh.\n\n" \
> ) \
> - "Config file parsing stops on the first matching line. If no
> config\n"\
> - "entry is matched, devices are created with default 0:0 660. (Make\n"\
> + "Config file parsing stops on the first matching line. If no
> config\n" \
> + "entry is matched, devices are created with default 0:0 660. (Make\n"
> \
> "the last line match .* to override this.)\n\n" \
> )
>
> @@ -3843,44 +3843,50 @@ USE_FEATURE_RUN_PARTS_FANCY("\n -l Print
> " [-p pidfile] [-r IP] [-s script]"
> #define udhcpc_full_usage \
> USE_GETOPT_LONG( \
> - " -V,--vendorclass=CLASSID Set vendor class identifier" \
> - "\n -i,--interface=INTERFACE Interface to use (default:
> eth0)" \
> + " -V,--vendorclass=CLASSID Vendor class identifier" \
> + "\n -i,--interface=INTERFACE Interface to use (default
> eth0)" \
> "\n -H,-h,--hostname=HOSTNAME Client hostname" \
> - "\n -c,--clientid=CLIENTID Set client identifier" \
> + "\n -c,--clientid=CLIENTID Client identifier" \
> "\n -C,--clientid-none Suppress default client identifier" \
> - "\n -p,--pidfile=file Store process ID of daemon in file" \
> + "\n -p,--pidfile=file Create pidfile" \
> "\n -r,--request=IP IP address to request" \
> "\n -s,--script=file Run file at dhcp events (default:
> /usr/share/udhcpc/default.script)" \
> - "\n -t,--retries=N Send up to N request packets"\
> - "\n -T,--timeout=N Try to get a lease for N seconds
> (default: 3)"\
> - "\n -A,--tryagain=N Wait N seconds (default: 60) after
> failure"\
> + "\n -t,--retries=N Send up to N request packets" \
> + "\n -T,--timeout=N Try to get a lease for N seconds
> (default 3)" \
> + "\n -A,--tryagain=N Wait N seconds (default 60) after
> failure" \
> "\n -f,--foreground Run in foreground" \
> - "\n -b,--background Background if lease cannot be immediately
> negotiated" \
> + "\n -b,--background Background if lease is not immediately
> obtained" \
> "\n -S,--syslog Log to syslog too" \
> - "\n -n,--now Exit with failure if lease cannot be
> immediately negotiated" \
> + "\n -n,--now Exit with failure if lease is not immediately
> obtained" \
> "\n -q,--quit Quit after obtaining lease" \
> "\n -R,--release Release IP on quit" \
> - "\n -v,--version Display version" \
> + USE_FEATURE_UDHCPC_ARPING( \
> + "\n -a,--arping Use arping to validate offered address" \
> + "\n -W,--wait=N Wait N seconds after declining (default 10)" \
> + ) \
> ) \
> SKIP_GETOPT_LONG( \
> - " -V CLASSID Set vendor class identifier" \
> + " -V CLASSID Vendor class identifier" \
> "\n -i INTERFACE Interface to use (default: eth0)" \
> "\n -H,-h HOSTNAME Client hostname" \
> - "\n -c CLIENTID Set client identifier" \
> + "\n -c CLIENTID Client identifier" \
> "\n -C Suppress default client identifier" \
> - "\n -p file Store process ID of daemon in file" \
> + "\n -p file Create pidfile" \
> "\n -r IP IP address to request" \
> "\n -s file Run file at dhcp events (default:
> /usr/share/udhcpc/default.script)" \
> - "\n -t N Send up to N request packets"\
> - "\n -T N Try to get a lease for N seconds (default: 3)"\
> - "\n -A N Wait N seconds (default: 60) after failure"\
> + "\n -t N Send up to N request packets" \
> + "\n -T N Try to get a lease for N seconds (default 3)" \
> + "\n -A N Wait N seconds (default 60) after failure" \
> "\n -f Run in foreground" \
> - "\n -b Background if lease cannot be immediately
> negotiated" \
> + "\n -b Background if lease is not immediately
> obtained" \
> "\n -S Log to syslog too" \
> - "\n -n Exit with failure if lease cannot be
> immediately negotiated" \
> + "\n -n Exit with failure if lease is not immediately
> obtained" \
> "\n -q Quit after obtaining lease" \
> "\n -R Release IP on quit" \
> - "\n -v Display version" \
> + USE_FEATURE_UDHCPC_ARPING( \
> + "\n -a Use arping to validate offered address" \
> + "\n -W N Wait N seconds after declining (default 10)" \
> + ) \
> )
>
> #define udhcpd_trivial_usage \
> diff -d -urpN busybox.1/networking/udhcp/Config.in
> busybox.2/networking/udhcp/Config.in
> --- busybox.1/networking/udhcp/Config.in 2007-11-15 21:02:19.000000000
> -0800
> +++ busybox.2/networking/udhcp/Config.in 2007-11-21 13:12:36.000000000
> -0800
> @@ -54,6 +54,16 @@ config APP_UDHCPC
>
> See http://udhcp.busybox.net for further details.
>
> +config FEATURE_UDHCPC_ARPING
> + bool "Ask udhcpc to verify that the offered address is free, using
> arpping"
> + default y
> + depends on APP_UDHCPC
> + help
> + If selected, udhcpc will use arpping to make sure the offered address
> + is really available. The client will DHCPDECLINE the offer if the
> + address is in use, and restart the discover process.
> +
> +
> config FEATURE_UDHCP_DEBUG
> bool "Compile udhcp with noisy debugging messages"
> default n
> diff -d -urpN busybox.1/networking/udhcp/Kbuild
> busybox.2/networking/udhcp/Kbuild
> --- busybox.1/networking/udhcp/Kbuild 2007-11-15 21:02:19.000000000 -0800
> +++ busybox.2/networking/udhcp/Kbuild 2007-11-21 13:57:24.000000000 -0800
> @@ -10,10 +10,16 @@ lib-$(CONFIG_APP_UDHCPC) += commo
> signalpipe.o socket.o
> lib-$(CONFIG_APP_UDHCPD) += common.o options.o packet.o \
> signalpipe.o socket.o
> +
> lib-$(CONFIG_APP_UDHCPC) += dhcpc.o clientpacket.o clientsocket.o \
> script.o
> +
> +UDHCPC_NEEDS_ARPING-$(CONFIG_FEATURE_UDHCPC_ARPING) = y
> +lib-$(UDHCPC_NEEDS_ARPING) += arpping.o
> +
> lib-$(CONFIG_APP_UDHCPD) += dhcpd.o arpping.o files.o leases.o \
> serverpacket.o static_leases.o
> +
> lib-$(CONFIG_APP_DUMPLEASES) += dumpleases.o
> lib-$(CONFIG_APP_DHCPRELAY) += dhcprelay.o
> lib-$(CONFIG_FEATURE_RFC3397) += domain_codec.o
> diff -d -urpN busybox.1/networking/udhcp/arpping.c
> busybox.2/networking/udhcp/arpping.c
> --- busybox.1/networking/udhcp/arpping.c 2007-11-15 21:02:19.000000000
> -0800
> +++ busybox.2/networking/udhcp/arpping.c 2007-11-21 14:29:30.000000000
> -0800
> @@ -32,12 +32,16 @@ struct arpMsg {
> uint8_t pad[18]; /* 2a pad for min. ethernet payload (60 bytes)
> */
> } ATTRIBUTE_PACKED;
>
> +enum {
> + ARP_MSG_SIZE = 0x2a
> +};
> +
>
> /* Returns 1 if no reply received */
>
> int arpping(uint32_t test_ip, uint32_t from_ip, uint8_t *from_mac, const
> char *interface)
> {
> - int timeout_ms = 2000;
> + int timeout_ms;
> struct pollfd pfd[1];
> #define s (pfd[0].fd) /* socket */
> int rv = 1; /* "no reply received" yet */
> @@ -51,7 +55,7 @@ int arpping(uint32_t test_ip, uint32_t f
> }
>
> if (setsockopt_broadcast(s) == -1) {
> - bb_perror_msg("cannot setsocketopt on raw socket");
> + bb_perror_msg("cannot enable bcast on raw socket");
> goto ret;
> }
>
> @@ -67,28 +71,35 @@ int arpping(uint32_t test_ip, uint32_t f
> arp.operation = htons(ARPOP_REQUEST); /* ARP op code */
> memcpy(arp.sHaddr, from_mac, 6); /* source hardware
> address */
> memcpy(arp.sInaddr, &from_ip, sizeof(from_ip)); /* source IP address */
> - /* tHaddr */ /* target hardware
> address */
> + /* tHaddr is zero-fiiled */ /* target hardware
> address */
> memcpy(arp.tInaddr, &test_ip, sizeof(test_ip)); /* target IP address */
>
> memset(&addr, 0, sizeof(addr));
> safe_strncpy(addr.sa_data, interface, sizeof(addr.sa_data));
> - if (sendto(s, &arp, sizeof(arp), 0, &addr, sizeof(addr)) < 0)
> + if (sendto(s, &arp, sizeof(arp), 0, &addr, sizeof(addr)) < 0) {
> + // TODO: error message? caller didn't expect us to fail,
> + // just returning 1 "no reply received" misleads it.
> goto ret;
> + }
>
> /* wait for arp reply, and check it */
> + timeout_ms = 2000;
> do {
> int r;
> unsigned prevTime = monotonic_us();
>
> pfd[0].events = POLLIN;
> r = safe_poll(pfd, 1, timeout_ms);
> - if (r < 0) {
> + if (r < 0)
> break;
> - } else if (r) {
> - if (read(s, &arp, sizeof(arp)) < 0)
> + if (r) {
> + r = read(s, &arp, sizeof(arp));
> + if (r < 0)
> break;
> - if (arp.operation == htons(ARPOP_REPLY)
> - && memcmp(arp.tHaddr, from_mac, 6) == 0
> + if (r >= ARP_MSG_SIZE
> + && arp.operation == htons(ARPOP_REPLY)
> + /* don't check it: Linux doesn't return proper tHaddr
> (fixed in 2.6.24?) */
> + /* && memcmp(arp.tHaddr, from_mac, 6) == 0 */
> && *((uint32_t *) arp.sInaddr) == test_ip
> ) {
> rv = 0;
> diff -d -urpN busybox.1/networking/udhcp/clientpacket.c
> busybox.2/networking/udhcp/clientpacket.c
> --- busybox.1/networking/udhcp/clientpacket.c 2007-11-15 21:02:19.000000000
> -0800
> +++ busybox.2/networking/udhcp/clientpacket.c 2007-11-21 13:59:30.000000000
> -0800
> @@ -69,6 +69,22 @@ static void add_requests(struct dhcpMess
>
> }
>
> +#if ENABLE_FEATURE_UDHCPC_ARPING
> +/* Unicast a DHCP decline message */
> +int send_decline(uint32_t xid, uint32_t server)
> +{
> + struct dhcpMessage packet;
> +
> + init_packet(&packet, DHCPDECLINE);
> + packet.xid = xid;
> + add_requests(&packet);
> +
> + bb_info_msg("Sending decline...");
> +
> + return udhcp_raw_packet(&packet, INADDR_ANY, CLIENT_PORT,
> INADDR_BROADCAST,
> + SERVER_PORT, MAC_BCAST_ADDR, client_config.ifindex);
> +}
> +#endif
>
> /* Broadcast a DHCP discover packet to the network, with an optionally
> requested IP */
> int send_discover(uint32_t xid, uint32_t requested)
> diff -d -urpN busybox.1/networking/udhcp/common.h
> busybox.2/networking/udhcp/common.h
> --- busybox.1/networking/udhcp/common.h 2007-11-15 21:02:19.000000000
> -0800
> +++ busybox.2/networking/udhcp/common.h 2007-11-21 13:32:30.000000000
> -0800
> @@ -76,7 +76,7 @@ void udhcp_run_script(struct dhcpMessage
>
> void udhcp_sp_setup(void);
> int udhcp_sp_fd_set(fd_set *rfds, int extra_fd);
> -int udhcp_sp_read(fd_set *rfds);
> +int udhcp_sp_read(const fd_set *rfds);
> int raw_socket(int ifindex);
> int read_interface(const char *interface, int *ifindex, uint32_t *addr,
> uint8_t *arp);
> int listen_socket(/*uint32_t ip,*/ int port, const char *inf);
> diff -d -urpN busybox.1/networking/udhcp/dhcpc.c
> busybox.2/networking/udhcp/dhcpc.c
> --- busybox.1/networking/udhcp/dhcpc.c 2007-11-15 21:02:19.000000000
> -0800
> +++ busybox.2/networking/udhcp/dhcpc.c 2007-11-21 14:27:41.000000000
> -0800
> @@ -145,6 +145,13 @@ int udhcpc_main(int argc, char **argv)
> {
> uint8_t *temp, *message;
> char *str_c, *str_V, *str_h, *str_F, *str_r, *str_T, *str_A, *str_t;
> + int tryagain_timeout = 60;
> + int discover_timeout = 3;
> + int discover_retries = 3;
> +#if ENABLE_FEATURE_UDHCPC_ARPING
> + int decline_wait = 10;
> + char *str_W;
> +#endif
> uint32_t xid = 0;
> uint32_t lease = 0; /* can be given as 32-bit quantity */
> unsigned t1 = 0, t2 = 0; /* what a wonderful names */
> @@ -180,6 +187,10 @@ int udhcpc_main(int argc, char **argv)
> OPT_v = 1 << 17,
> OPT_S = 1 << 18,
> OPT_A = 1 << 19,
> +#if ENABLE_FEATURE_UDHCPC_ARPING
> + OPT_a = 1 << 20,
> + OPT_W = 1 << 21,
> +#endif
> };
> #if ENABLE_GETOPT_LONG
> static const char udhcpc_longopts[] ALIGN1 =
> @@ -203,14 +214,15 @@ int udhcpc_main(int argc, char **argv)
> "retries\0" Required_argument "t"
> "tryagain\0" Required_argument "A"
> "syslog\0" No_argument "S"
> +#if ENABLE_FEATURE_UDHCPC_ARPING
> + "arping\0" No_argument "a"
> + "wait\0" Required_argument "W"
> +#endif
> ;
> #endif
> /* Default options. */
> client_config.interface = "eth0";
> client_config.script = DEFAULT_SCRIPT;
> - client_config.retries = 3;
> - client_config.timeout = 3;
> - client_config.tryagain = 60;
>
> /* Parse command line */
> opt_complementary = "c--C:C--c" // mutually exclusive
> @@ -218,10 +230,12 @@ int udhcpc_main(int argc, char **argv)
> #if ENABLE_GETOPT_LONG
> applet_long_options = udhcpc_longopts;
> #endif
> - opt = getopt32(argv, "c:CV:fbH:h:F:i:np:qRr:s:T:t:vSA:",
> - &str_c, &str_V, &str_h, &str_h, &str_F,
> + opt = getopt32(argv, "c:CV:fbH:h:F:i:np:qRr:s:T:t:vSA:"
> + USE_FEATURE_UDHCPC_ARPING("aW:")
> + , &str_c, &str_V, &str_h, &str_h, &str_F,
> &client_config.interface, &client_config.pidfile, &str_r,
> &client_config.script, &str_T, &str_t, &str_A
> + USE_FEATURE_UDHCPC_ARPING(, &str_W)
> );
>
> if (opt & OPT_c)
> @@ -259,11 +273,11 @@ int udhcpc_main(int argc, char **argv)
> requested_ip = inet_addr(str_r);
> // if (opt & OPT_s) client_config.script = ...
> if (opt & OPT_T)
> - client_config.timeout = xatoi_u(str_T);
> + discover_timeout = xatoi_u(str_T);
> if (opt & OPT_t)
> - client_config.retries = xatoi_u(str_t);
> + discover_retries = xatoi_u(str_t);
> if (opt & OPT_A)
> - client_config.tryagain = xatoi_u(str_A);
> + tryagain_timeout = xatoi_u(str_A);
> if (opt & OPT_v) {
> puts("version "BB_VER);
> return 0;
> @@ -274,6 +288,11 @@ int udhcpc_main(int argc, char **argv)
> logmode |= LOGMODE_SYSLOG;
> }
>
> +#if ENABLE_FEATURE_UDHCPC_ARPING
> + if (opt & OPT_W)
> + decline_wait = xatou_range(str_W, 10, INT_MAX);
> +#endif
> +
> if (read_interface(client_config.interface, &client_config.ifindex,
> NULL, client_config.arp))
> return 1;
> @@ -339,14 +358,14 @@ int udhcpc_main(int argc, char **argv)
> /* timeout dropped to zero */
> switch (state) {
> case INIT_SELECTING:
> - if (packet_num < client_config.retries) {
> + if (packet_num < discover_retries) {
> if (packet_num == 0)
> xid = random_xid();
>
> /* send discover packet */
> send_discover(xid, requested_ip); /*
> broadcast */
>
> - timeout = now + client_config.timeout;
> + timeout = now + discover_timeout;
> packet_num++;
> } else {
> udhcp_run_script(NULL, "leasefail");
> @@ -360,12 +379,12 @@ int udhcpc_main(int argc, char **argv)
> }
> /* wait to try again */
> packet_num = 0;
> - timeout = now + client_config.tryagain;
> + timeout = now + tryagain_timeout;
> }
> break;
> case RENEW_REQUESTED:
> case REQUESTING:
> - if (packet_num < client_config.retries) {
> + if (packet_num < discover_retries) {
> /* send request packet */
> if (state == RENEW_REQUESTED)
> send_renew(xid, server_addr,
> requested_ip); /* unicast */
> @@ -491,6 +510,29 @@ int udhcpc_main(int argc, char **argv)
> lease = ntohl(lease);
> }
>
> +#if ENABLE_FEATURE_UDHCPC_ARPING
> + if (opt & OPT_a) {
> + if (!arpping(packet.yiaddr,
> + (uint32_t) 0,
> + client_config.arp,
> +
> client_config.interface)
> + ) {
> + bb_info_msg("offered
> address is in use,"
> + " declining");
> + send_decline(xid,
> server_addr);
> +
> + if (state != REQUESTING)
> +
> udhcp_run_script(NULL, "deconfig");
> + state = INIT_SELECTING;
> + timeout = now;
> + requested_ip = 0;
> + packet_num = 0;
> + change_mode(LISTEN_RAW);
> + sleep(decline_wait); /*
> avoid excessive network traffic */
> + break;
> + }
> + }
> +#endif
> /* enter bound state */
> t1 = lease / 2;
>
> diff -d -urpN busybox.1/networking/udhcp/dhcpc.h
> busybox.2/networking/udhcp/dhcpc.h
> --- busybox.1/networking/udhcp/dhcpc.h 2007-11-15 21:02:19.000000000
> -0800
> +++ busybox.2/networking/udhcp/dhcpc.h 2007-11-21 14:24:23.000000000
> -0800
> @@ -1,5 +1,6 @@
> /* vi: set sw=4 ts=4: */
> /* dhcpc.h */
> +
> #ifndef _DHCPC_H
> #define _DHCPC_H
>
> @@ -28,9 +29,6 @@ struct client_config_t {
> uint8_t *hostname; /* Optional hostname to use */
> uint8_t *fqdn; /* Optional fully qualified domain name
> to use */
> int ifindex; /* Index number of the interface to use
> */
> - int retries; /* Max number of request packets */
> - int timeout; /* Number of seconds to try to get a
> lease */
> - int tryagain; /* Number of seconds to try to get a
> lease */
> uint8_t arp[6]; /* Our arp address */
> };
>
> @@ -42,6 +40,9 @@ struct client_config_t {
> uint32_t random_xid(void);
> int send_discover(uint32_t xid, uint32_t requested);
> int send_selecting(uint32_t xid, uint32_t server, uint32_t requested);
> +#if ENABLE_FEATURE_UDHCPC_ARPING
> +int send_decline(uint32_t xid, uint32_t server);
> +#endif
> int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr);
> int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr);
> int send_release(uint32_t server, uint32_t ciaddr);
> diff -d -urpN busybox.1/networking/udhcp/signalpipe.c
> busybox.2/networking/udhcp/signalpipe.c
> --- busybox.1/networking/udhcp/signalpipe.c 2007-11-15 21:02:19.000000000
> -0800
> +++ busybox.2/networking/udhcp/signalpipe.c 2007-11-21 13:32:23.000000000
> -0800
> @@ -66,7 +66,7 @@ int udhcp_sp_fd_set(fd_set *rfds, int ex
> /* Read a signal from the signal pipe. Returns 0 if there is
> * no signal, -1 on error (and sets errno appropriately), and
> * your signal on success */
> -int udhcp_sp_read(fd_set *rfds)
> +int udhcp_sp_read(const fd_set *rfds)
> {
> unsigned char sig;
>
>
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
