[klibc] PATCH: ipconfig may discard useful packets
Άλκης Γεωργόπουλος
alkisg at gmail.com
Sat Jun 14 12:25:43 PDT 2008
Thank you Maximilian, this was much easier!
I still have problems with my email setup, I haven't found the time to
migrate my mailbox from Windows yet. And gmail (webmail) doesn't allow
many customizations, so I'm also uploading the patch to
http://users.sch.gr/alkisg/temp/0001-Signed-off-by.patch
I'm sending a patch regarding the discard_packet() function, please
consider it for inclusion.
Signed-off-by: Άλκης Γεωργόπουλος (alkisg at gmail.com)
In main.c, function loop(), there's a call to packet_discard().
Unfortunately, there was a logic error in the return values of
some functions, which caused it either to
1. Be called when a useful packet was in the queue,
such as DHCPACK, and so this packet was discarded, a
timeout occured and the correct address was acknowledged
only by a retransmittion after a few seconds, or
2. Be called when no packets where in the queue, so
packet_discard() just delayed until some new packet
arrived, and this new packet was again discarded.
I've seen many times ipconfig needing many seconds to
configure a card (in my school we have 2 dhcp servers, maybe
this makes the problem more common).
I think I corrected the function return values, and in
my tests (with either one or two dhcp servers) now ipconfig
gets an address in only a few msec.
Sorry, but this means that many functions were modified,
but in the good side, the return values were commented.
I did my best to avoid any regressions, but a second look
at the code or extra testing may be needed.
A scenario where the bug occurs:
In main.c:
nr = do_pkt_recv(pkt_fd, now.tv_sec);
if (nr == 1)
break;
else if (nr == 0)
packet_discard(); <== This one
The following functions may be called:
do_pkt_recv > process_receive_event > e.g. dhcp_recv_offer > dhcp_recv
> packet_recv
packet_recv may receive a not-appropriate packet, and jump to
discard_pkt, in which case it *discards* the packet and returns 0.
This return value (0) will propagate to main, so (nr == 0) and
packet_discard() will be called *again*, causing the NIC to wait until
it happens to
receive some packet.
Kind regards,
Alkis Georgopoulos
---
usr/kinit/ipconfig/bootp_proto.c | 6 +++++-
usr/kinit/ipconfig/dhcp_proto.c | 10 +++++-----
usr/kinit/ipconfig/main.c | 32 +++++++++++++++++++++-----------
usr/kinit/ipconfig/packet.c | 15 ++++++++++++---
4 files changed, 43 insertions(+), 20 deletions(-)
diff --git a/usr/kinit/ipconfig/bootp_proto.c b/usr/kinit/ipconfig/bootp_proto.c
index 236bde9..7df3137 100644
--- a/usr/kinit/ipconfig/bootp_proto.c
+++ b/usr/kinit/ipconfig/bootp_proto.c
@@ -153,6 +153,10 @@ int bootp_parse(struct netdev *dev, struct bootp_hdr *hdr,
/*
* Receive a bootp reply and parse packet
+ * Returns:
+ *-1 = Error in packet_recv, try again later
+ * 0 = Unexpected packet, discarded
+ * 1 = Correctly received and parsed packet
*/
int bootp_recv_reply(struct netdev *dev)
{
@@ -167,7 +171,7 @@ int bootp_recv_reply(struct netdev *dev)
ret = packet_recv(iov, 3);
if (ret <= 0)
- return ret;
+ return -1;
if (ret < sizeof(struct bootp_hdr) ||
bootp.op != BOOTP_REPLY || /* RFC951 7.5 */
diff --git a/usr/kinit/ipconfig/dhcp_proto.c b/usr/kinit/ipconfig/dhcp_proto.c
index d4f2c09..82cd1ed 100644
--- a/usr/kinit/ipconfig/dhcp_proto.c
+++ b/usr/kinit/ipconfig/dhcp_proto.c
@@ -72,7 +72,7 @@ static struct iovec dhcp_request_iov[] = {
/*
* Parse a DHCP response packet
* Returns:
- * 0 = Not handled
+ * 0 = Unexpected packet, not parsed
* 2 = DHCPOFFER (from dhcp_proto.h)
* 5 = DHCPACK
* 6 = DHCPNACK
@@ -128,8 +128,8 @@ static int dhcp_parse(struct netdev *dev, struct
bootp_hdr *hdr,
/*
* Receive and parse a DHCP packet
* Returns:
- *-1 = Error in packet_recv
- * 0 = Not handled
+ *-1 = Error in packet_recv, try again later
+ * 0 = Unexpected packet, discarded
* 2 = DHCPOFFER (from dhcp_proto.h)
* 5 = DHCPACK
* 6 = DHCPNACK
@@ -146,8 +146,8 @@ static int dhcp_recv(struct netdev *dev)
int ret;
ret = packet_recv(iov, 3);
- if (ret <= 0)
- return ret;
+ if (ret == 0)
+ return -1;
DEBUG(("\n dhcp xid %08x ", dev->bootp.xid));
diff --git a/usr/kinit/ipconfig/main.c b/usr/kinit/ipconfig/main.c
index 2ded0f3..4e728e0 100644
--- a/usr/kinit/ipconfig/main.c
+++ b/usr/kinit/ipconfig/main.c
@@ -169,6 +169,11 @@ static void complete_device(struct netdev *dev)
ifaces = dev;
}
+/*
+ * Returns:
+ * 0 = Not handled, the packet is still in the queue
+ * 1 = Handled
+ */
static int process_receive_event(struct state *s, time_t now)
{
int handled = 1;
@@ -214,6 +219,10 @@ static int process_receive_event(struct state *s,
time_t now)
break;
}
break;
+
+ default:
+ handled = 0;
+ break;
}
switch (s->state) {
@@ -224,9 +233,6 @@ static int process_receive_event(struct state *s,
time_t now)
case DEVST_ERROR:
/* error occurred, try again in 10 seconds */
s->expire = now + 10;
- default:
- DEBUG(("\n"));
- handled = 0;
break;
}
@@ -288,23 +294,30 @@ static void process_timeout_event(struct state
*s, time_t now)
static struct state *slist;
struct netdev *ifaces;
+/*
+ * Returns:
+ * 0 = Error, packet not received or discarded
+ * 1 = A packet was received and handled
+ */
static int do_pkt_recv(int pkt_fd, time_t now)
{
int ifindex, ret;
struct state *s;
ret = packet_peek(&ifindex);
- if (ret < 0)
- goto bail;
+ if (ret == 0)
+ return ret;
for (s = slist; s; s = s->next) {
if (s->dev->ifindex == ifindex) {
- ret |= process_receive_event(s, now);
+ ret = process_receive_event(s, now);
break;
}
}
- bail:
+ if (ret == 0)
+ packet_discard();
+
return ret;
}
@@ -371,11 +384,8 @@ static int loop(void)
gettimeofday(&now, NULL);
if ((fds[0].revents & POLLRDNORM)) {
- nr = do_pkt_recv(pkt_fd, now.tv_sec);
- if (nr == 1)
+ if (do_pkt_recv(pkt_fd, now.tv_sec) == 1)
break;
- else if (nr == 0)
- packet_discard();
}
if (loop_timeout >= 0 &&
diff --git a/usr/kinit/ipconfig/packet.c b/usr/kinit/ipconfig/packet.c
index 283cf02..4bc01b5 100644
--- a/usr/kinit/ipconfig/packet.c
+++ b/usr/kinit/ipconfig/packet.c
@@ -165,6 +165,12 @@ int packet_send(struct netdev *dev, struct iovec
*iov, int iov_len)
return sendmsg(pkt_fd, &msg, 0);
}
+/*
+ * Fetches a bootp packet, but doesn't remove it.
+ * Returns:
+ * 0 = Error
+ * >0 = A packet of size "ret" is available for interface ifindex
+ */
int packet_peek(int *ifindex)
{
struct sockaddr_ll sll;
@@ -177,7 +183,7 @@ int packet_peek(int *ifindex)
ret = recvfrom(pkt_fd, &iph, sizeof(struct iphdr),
MSG_PEEK, (struct sockaddr *)&sll, &sllen);
if (ret == -1)
- return -1;
+ return 0;
if (sll.sll_family != AF_PACKET)
goto discard_pkt;
@@ -187,7 +193,7 @@ int packet_peek(int *ifindex)
*ifindex = sll.sll_ifindex;
- return 0;
+ return ret;
discard_pkt:
packet_discard();
@@ -207,6 +213,9 @@ void packet_discard(void)
/*
* Receive a bootp packet. The options are listed in iov[1...iov_len].
* iov[0] must point to the bootp packet header.
+ * Returns:
+ * 0 = Error, try again later
+ * >0 = Size of packet
*/
int packet_recv(struct iovec *iov, int iov_len)
{
@@ -226,7 +235,7 @@ int packet_recv(struct iovec *iov, int iov_len)
ret = recvfrom(pkt_fd, &iph, sizeof(struct iphdr),
MSG_PEEK, NULL, NULL);
if (ret == -1)
- return -1;
+ return 0;
if (iph.ihl < 5 || iph.version != IPVERSION)
goto discard_pkt;
--
1.5.4.3
More information about the klibc
mailing list