Bryan O'Sullivan
2003-Apr-29 11:08 UTC
[klibc] [PATCH] Fix busy-looping behaviour in ipconfig
The ipconfig code does not drop incoming packets that it can't handle. Since the packet socket sends ipconfig its own broadcast requests, ipconfig has the unfortunate behaviour of eating 100% of the CPU time when it does not receive an immediate response to its outgoing requests. This patch fixes the problem, by ensuring that packets are dropped if they are not handled. It also introduces a "-n" (don't configure the interface) and "-p" (use a different pair of port numbers) option, which were necessary to identify and solve this problem, and which should be added on account of being generally useful. ipconfig.h | 15 +++++++++ main.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++-------------- packet.c | 30 ++++++++++++++++--- packet.h | 2 - 4 files changed, 113 insertions(+), 26 deletions(-) diff -Nru a/ipconfig/ipconfig.h b/ipconfig/ipconfig.h --- /dev/null Wed Dec 31 16:00:00 1969 +++ b/ipconfig/ipconfig.h Tue Apr 29 11:01:50 2003 @@ -0,0 +1,15 @@ +/* + * ipconfig/ipconfig.h + */ + +#define LOCAL_PORT 68 +#define REMOTE_PORT (LOCAL_PORT - 1) + +extern __u16 local_port; +extern __u16 remote_port; + +#if 0 +#define DEBUG(x) printf x +#else +#define DEBUG(x) do { } while(0) +#endif diff -Nru a/ipconfig/main.c b/ipconfig/main.c --- a/ipconfig/main.c Tue Apr 29 11:01:50 2003 +++ b/ipconfig/main.c Tue Apr 29 11:01:50 2003 @@ -2,12 +2,14 @@ * ipconfig/main.c */ #include <poll.h> +#include <limits.h> #include <stdio.h> #include <string.h> #include <stdlib.h> #include <time.h> #include <arpa/inet.h> +#include "ipconfig.h" #include "netdev.h" #include "bootp_packet.h" #include "bootp_proto.h" @@ -15,6 +17,7 @@ #include "packet.h" static const char *progname; +static char do_not_config; struct state { int state; @@ -57,6 +60,9 @@ static void configure_device(struct netdev *dev) { + if (do_not_config) + return; + if (netdev_setaddress(dev)) printf("IP-Config: failed to set addresses on %s\n", dev->name); if (netdev_setdefaultroute(dev)) @@ -118,8 +124,10 @@ } } -static void process_receive_event(struct state *s, time_t now) +static int process_receive_event(struct state *s, time_t now) { + int handled = 1; + switch (s->state) { case DEVST_BOOTP: s->restart_state = DEVST_BOOTP; @@ -173,8 +181,12 @@ case DEVST_ERROR: /* error occurred, try again in 10 seconds */ s->expire = now + 10; + default: + handled = 0; break; } + + return handled; } static void process_timeout_event(struct state *s, time_t now) @@ -231,20 +243,23 @@ static struct state *slist; -static void do_pkt_recv(int pkt_fd, time_t now) +static int do_pkt_recv(int pkt_fd, time_t now) { int ifindex, ret; struct state *s; ret = packet_peek(&ifindex); if (ret < 0) - return; + goto bail; for (s = slist; s; s = s->next) if (s->dev->ifindex == ifindex) { - process_receive_event(s, now); + ret |= process_receive_event(s, now); break; } + + bail: + return ret; } static int loop(void) @@ -254,7 +269,7 @@ struct state *s; int pkt_fd; int nr = 0; - time_t now; + struct timeval now, prev; pkt_fd = packet_open(); if (pkt_fd == -1) { @@ -265,10 +280,12 @@ fds[0].fd = pkt_fd; fds[0].events = POLLRDNORM; - now = time(NULL); + gettimeofday(&now, NULL); while (1) { int timeout = 60; int pending = 0; + int timeout_ms; + int x; for (s = slist; s; s = s->next) { if (s->state == DEVST_COMPLETE) @@ -276,24 +293,41 @@ pending++; - if (s->expire - now <= 0) - process_timeout_event(s, now); + if (s->expire - now.tv_sec <= 0) + process_timeout_event(s, now.tv_sec); - if (timeout > s->expire - now) - timeout = s->expire - now; + if (timeout > s->expire - now.tv_sec) + timeout = s->expire - now.tv_sec; } if (pending == 0) break; - if (timeout < 0) - timeout = 0; - - nr = poll(fds, NR_FDS, timeout * 1000); - now = time(NULL); - - if (fds[0].revents & POLLRDNORM) - do_pkt_recv(pkt_fd, now); + timeout_ms = timeout * 1000; + + for (x = 0; x < 2; x++) { + int delta_ms; + + if (timeout_ms <= 0) + timeout_ms = 1; + + nr = poll(fds, NR_FDS, timeout_ms); + prev = now; + gettimeofday(&now, NULL); + + if ((fds[0].revents & POLLRDNORM) && + do_pkt_recv(pkt_fd, now.tv_sec) == 1) { + break; + } + packet_discard(); + + delta_ms = (now.tv_sec - prev.tv_sec) * 1000; + delta_ms += (now.tv_usec - prev.tv_usec) / 1000; + + DEBUG(("Delta: %d ms\n", delta_ms)); + + timeout_ms -= delta_ms; + } } packet_close(); @@ -365,15 +399,33 @@ int main(int argc, char *argv[]) { - int c; + int c, port; + progname = argv[0]; + do { - c = getopt(argc, argv, "td:"); + c = getopt(argc, argv, "d:np:t"); if (c == EOF) break; switch (c) { + case 'p': + port = atoi(optarg); + if (port <= 0 || port > USHRT_MAX) { + fprintf(stderr, + "%s: port number %d out of range\n", + progname, port); + exit(1); + } + local_port = port; + remote_port = local_port - 1; + printf("IP-Config: binding source port to %d, " + "dest to %d\n", local_port, remote_port); + break; case 't': + break; + case 'n': + do_not_config = 1; break; case 'd': add_device(optarg); diff -Nru a/ipconfig/packet.c b/ipconfig/packet.c --- a/ipconfig/packet.c Tue Apr 29 11:01:50 2003 +++ b/ipconfig/packet.c Tue Apr 29 11:01:50 2003 @@ -15,11 +15,15 @@ #include <netinet/ip.h> #include <netinet/udp.h> +#include "ipconfig.h" #include "netdev.h" #include "packet.h" static int pkt_fd; +__u16 local_port = LOCAL_PORT; +__u16 remote_port = REMOTE_PORT; + int packet_open(void) { int fd, one = 1; @@ -83,8 +87,8 @@ .daddr = INADDR_BROADCAST, }, .udp = { - .source = __constant_htons(68), - .dest = __constant_htons(67), + .source = __constant_htons(LOCAL_PORT), + .dest = __constant_htons(REMOTE_PORT), .len = 0, .check = 0, }, @@ -108,6 +112,11 @@ }; int i, len = 0; + if (local_port != LOCAL_PORT) { + ipudp_hdrs.udp.source = htons(local_port); + ipudp_hdrs.udp.dest = htons(remote_port); + } + /* * Glue in the ip+udp header iovec */ @@ -160,11 +169,21 @@ return 0; discard_pkt: - recvfrom(pkt_fd, &iph, sizeof(struct iphdr), - 0, (struct sockaddr *)&sll, &sllen); + packet_discard(); return 0; } +void packet_discard(void) +{ + struct iphdr iph; + struct sockaddr_ll sll; + socklen_t sllen = sizeof(sll); + + recvfrom(pkt_fd, &iph, sizeof(iph), 0, + (struct sockaddr *) &sll, &sllen); +} + + /* * Receive a bootp packet. The options are listed in iov[1...iov_len]. * iov[0] must point to the bootp packet header. @@ -215,7 +234,8 @@ ret -= 4 * ip->ihl; - if (udp->source != htons(67) || udp->dest != htons(68)) + if (udp->source != htons(remote_port) || + udp->dest != htons(local_port)) goto free_pkt; if (ntohs(udp->len) > ret) diff -Nru a/ipconfig/packet.h b/ipconfig/packet.h --- a/ipconfig/packet.h Tue Apr 29 11:01:50 2003 +++ b/ipconfig/packet.h Tue Apr 29 11:01:50 2003 @@ -5,5 +5,5 @@ void packet_close(void); int packet_send(struct netdev *dev, struct iovec *iov, int iov_len); int packet_peek(int *ifindex); +void packet_discard(void); int packet_recv(struct iovec *iov, int iov_len); -
Apparently Analagous Threads
- [PATCH] ipconfig: handle multiple interfaces correctly
- [klibc:master] ipconfig: handle multiple interfaces correctly
- [PATCH] Add configurable timeout to ipconfig
- PATCH: ipconfig may discard useful packets
- [PATCH 1/3] Only peek and discard packets from specified device.