klibc - Mar 2011 - [PATCH 1/3] Only peek and discard packets from specified device.

This patch fixes a bug on systems with multiple connected network devices.
As packet_peek uses all devices to receive data instead of a specific
device. As the return value was never reset it was possible that packets
from other devices were returned by packet_peek. That means that the
ifindex did not match any ifindex of the specified devices the packet was
never removed and packets for the correct device were never processed.

This patch enhance packet_peek and packet_discard to only work on packages
for the specified device instead of all packets.

Signed-off-by: Ulrich Dangel <uli at spamt.net>
---
 usr/kinit/ipconfig/bootp_proto.c |    2 +-
 usr/kinit/ipconfig/dhcp_proto.c  |    2 +-
 usr/kinit/ipconfig/main.c        |   16 ++++++----------
 usr/kinit/ipconfig/packet.c      |   16 +++++++++-------
 usr/kinit/ipconfig/packet.h      |    6 +++---
 5 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/usr/kinit/ipconfig/bootp_proto.c b/usr/kinit/ipconfig/bootp_proto.c
index baf9d3e..f2cc90c 100644
--- a/usr/kinit/ipconfig/bootp_proto.c
+++ b/usr/kinit/ipconfig/bootp_proto.c
@@ -169,7 +169,7 @@ int bootp_recv_reply(struct netdev *dev)
 	};
 	int ret;
 
-	ret = packet_recv(iov, 3);
+	ret = packet_recv(dev, iov, 3);
 	if (ret <= 0)
 		return ret;
 
diff --git a/usr/kinit/ipconfig/dhcp_proto.c b/usr/kinit/ipconfig/dhcp_proto.c
index fc0494d..993db52 100644
--- a/usr/kinit/ipconfig/dhcp_proto.c
+++ b/usr/kinit/ipconfig/dhcp_proto.c
@@ -147,7 +147,7 @@ static int dhcp_recv(struct netdev *dev)
 	};
 	int ret;
 
-	ret = packet_recv(iov, 3);
+	ret = packet_recv(dev, iov, 3);
 	if (ret <= 0)
 		return ret;
 
diff --git a/usr/kinit/ipconfig/main.c b/usr/kinit/ipconfig/main.c
index d501bec..1e48083 100644
--- a/usr/kinit/ipconfig/main.c
+++ b/usr/kinit/ipconfig/main.c
@@ -304,23 +304,19 @@ struct netdev *ifaces;
  */
 static int do_pkt_recv(int pkt_fd, time_t now)
 {
-	int ifindex, ret;
+	int ret = 0;
 	struct state *s;
 
-	ret = packet_peek(&ifindex);
-	if (ret == 0)
-		return ret;
-
 	for (s = slist; s; s = s->next) {
-		if (s->dev->ifindex == ifindex) {
+		ret = packet_peek(s->dev);
+		if (ret) {
 			ret = process_receive_event(s, now);
+			if (ret == 0) {
+				packet_discard(s->dev);
+			}
 			break;
 		}
 	}
-
-	if (ret == 0)
-		packet_discard();
-
 	return ret;
 }
 
diff --git a/usr/kinit/ipconfig/packet.c b/usr/kinit/ipconfig/packet.c
index 84267b7..993a2fa 100644
--- a/usr/kinit/ipconfig/packet.c
+++ b/usr/kinit/ipconfig/packet.c
@@ -167,17 +167,18 @@ int packet_send(struct netdev *dev, struct iovec *iov, int
iov_len)
 }
 
 /*
- * Fetches a bootp packet, but doesn't remove it.
+ * Fetches a bootp packet from specified device, but doesn't remove it.
  * Returns:
  *  0 = Error
  * >0 = A packet of size "ret" is available for interface ifindex
  */
-int packet_peek(int *ifindex)
+int packet_peek(struct netdev *dev)
 {
 	struct sockaddr_ll sll;
 	struct iphdr iph;
 	int ret, sllen = sizeof(struct sockaddr_ll);
 
+	sll.sll_ifindex = dev->ifindex;
 	/*
 	 * Peek at the IP header.
 	 */
@@ -192,21 +193,22 @@ int packet_peek(int *ifindex)
 	if (iph.ihl < 5 || iph.version != IPVERSION)
 		goto discard_pkt;
 
-	*ifindex = sll.sll_ifindex;
 
 	return ret;
 
 discard_pkt:
-	packet_discard();
+	packet_discard(dev);
 	return 0;
 }
 
-void packet_discard(void)
+void packet_discard(struct netdev *dev)
 {
 	struct iphdr iph;
 	struct sockaddr_ll sll;
 	socklen_t sllen = sizeof(sll);
 
+	sll.sll_ifindex = dev->ifindex;
+
 	recvfrom(pkt_fd, &iph, sizeof(iph), 0,
 		 (struct sockaddr *)&sll, &sllen);
 }
@@ -219,7 +221,7 @@ void packet_discard(void)
 *   0 = Discarded packet (non-DHCP/BOOTP traffic)
  * >0 = Size of packet
  */
-int packet_recv(struct iovec *iov, int iov_len)
+int packet_recv(struct netdev* dev, struct iovec *iov, int iov_len)
 {
 	struct iphdr *ip, iph;
 	struct udphdr *udp;
@@ -293,6 +295,6 @@ free_pkt:
 
 discard_pkt:
 	dprintf("discarded\n");
-	packet_discard();
+	packet_discard(dev);
 	return 0;
 }
diff --git a/usr/kinit/ipconfig/packet.h b/usr/kinit/ipconfig/packet.h
index 627d282..524f393 100644
--- a/usr/kinit/ipconfig/packet.h
+++ b/usr/kinit/ipconfig/packet.h
@@ -6,8 +6,8 @@ struct iovec;
 int packet_open(void);
 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);
+int packet_peek(struct netdev *dev);
+void packet_discard(struct netdev *dev);
+int packet_recv(struct netdev *dev, struct iovec *iov, int iov_len);
 
 #endif /* IPCONFIG_PACKET_H */
-- 
1.7.1

packet_peek is not really necessary as it does basically nothing besides
testing if a packet is availabe. This is already done in packet_recv.
Adjusted do_pkt_recv to only use process_receive_event.

packet_recv was modified to only receive and use packets from the specified
interface.

process_receive_event was adjusted to handle discared/ignored packages from
the corresponding recv functions. As packet_recv could return 0 for
ignored packets process_receive_event handled them as processed/handled
packets resulting in an endless loop if new (non valid dhcp/bootp) packets
were received.

As a side effect ignored packets are now shown automatically in the debug
output.

Signed-off-by: Ulrich Dangel <uli at spamt.net>
---
 usr/kinit/ipconfig/main.c   |   24 +++++++++++++-----------
 usr/kinit/ipconfig/packet.c |   43 +++++++------------------------------------
 usr/kinit/ipconfig/packet.h |    1 -
 3 files changed, 20 insertions(+), 48 deletions(-)

diff --git a/usr/kinit/ipconfig/main.c b/usr/kinit/ipconfig/main.c
index 1e48083..43b6637 100644
--- a/usr/kinit/ipconfig/main.c
+++ b/usr/kinit/ipconfig/main.c
@@ -170,7 +170,7 @@ static void complete_device(struct netdev *dev)
 
 /*
  * Returns:
- *  0 = Not handled, the packet is still in the queue
+ *  0 = Not handled, try again later
  *  1 = Handled
  */
 static int process_receive_event(struct state *s, time_t now)
@@ -187,6 +187,9 @@ static int process_receive_event(struct state *s, time_t
now)
 		case -1:
 			s->state = DEVST_ERROR;
 			break;
+		case 0:
+			handled = 0;
+			break;
 		case 1:
 			s->state = DEVST_COMPLETE;
 			dprintf("\n   bootp reply\n");
@@ -200,6 +203,9 @@ static int process_receive_event(struct state *s, time_t
now)
 		case -1:
 			s->state = DEVST_ERROR;
 			break;
+		case 0:
+			handled = 0;
+			break;
 		case DHCPOFFER:	/* Offer received */
 			s->state = DEVST_DHCPREQ;
 			dhcp_send_request(s->dev);
@@ -213,6 +219,9 @@ static int process_receive_event(struct state *s, time_t
now)
 		case -1:	/* error */
 			s->state = DEVST_ERROR;
 			break;
+		case 0:
+			handled = 0;
+			break;
 		case DHCPACK:	/* ACK received */
 			s->state = DEVST_COMPLETE;
 			break;
@@ -299,23 +308,16 @@ struct netdev *ifaces;
 
 /*
  * Returns:
- *  0 = Error, packet not received or discarded
+ *  0 = No dhcp/bootp packet was received
  *  1 = A packet was received and handled
  */
 static int do_pkt_recv(int pkt_fd, time_t now)
 {
-	int ret = 0;
+	int ret;
 	struct state *s;
 
 	for (s = slist; s; s = s->next) {
-		ret = packet_peek(s->dev);
-		if (ret) {
-			ret = process_receive_event(s, now);
-			if (ret == 0) {
-				packet_discard(s->dev);
-			}
-			break;
-		}
+		ret |= process_receive_event(s, now);
 	}
 	return ret;
 }
diff --git a/usr/kinit/ipconfig/packet.c b/usr/kinit/ipconfig/packet.c
index 993a2fa..ea5a25d 100644
--- a/usr/kinit/ipconfig/packet.c
+++ b/usr/kinit/ipconfig/packet.c
@@ -166,41 +166,6 @@ int packet_send(struct netdev *dev, struct iovec *iov, int
iov_len)
 	return sendmsg(pkt_fd, &msg, 0);
 }
 
-/*
- * Fetches a bootp packet from specified device, but doesn't remove it.
- * Returns:
- *  0 = Error
- * >0 = A packet of size "ret" is available for interface ifindex
- */
-int packet_peek(struct netdev *dev)
-{
-	struct sockaddr_ll sll;
-	struct iphdr iph;
-	int ret, sllen = sizeof(struct sockaddr_ll);
-
-	sll.sll_ifindex = dev->ifindex;
-	/*
-	 * Peek at the IP header.
-	 */
-	ret = recvfrom(pkt_fd, &iph, sizeof(struct iphdr),
-		       MSG_PEEK, (struct sockaddr *)&sll, &sllen);
-	if (ret == -1)
-		return 0;
-
-	if (sll.sll_family != AF_PACKET)
-		goto discard_pkt;
-
-	if (iph.ihl < 5 || iph.version != IPVERSION)
-		goto discard_pkt;
-
-
-	return ret;
-
-discard_pkt:
-	packet_discard(dev);
-	return 0;
-}
-
 void packet_discard(struct netdev *dev)
 {
 	struct iphdr iph;
@@ -235,9 +200,15 @@ int packet_recv(struct netdev* dev, struct iovec *iov, int
iov_len)
 		.msg_flags	= 0
 	};
 	int ret, iphl;
+	struct sockaddr_ll sll;
+	socklen_t sllen = sizeof(sll);
+
+	sll.sll_ifindex = dev->ifindex;
+	msg.msg_name = &sll;
+	msg.msg_namelen = sllen;
 
 	ret = recvfrom(pkt_fd, &iph, sizeof(struct iphdr),
-		       MSG_PEEK, NULL, NULL);
+		       MSG_PEEK, (struct sockaddr *)&sll, &sllen);
 	if (ret == -1)
 		return -1;
 
diff --git a/usr/kinit/ipconfig/packet.h b/usr/kinit/ipconfig/packet.h
index 524f393..f6cef52 100644
--- a/usr/kinit/ipconfig/packet.h
+++ b/usr/kinit/ipconfig/packet.h
@@ -6,7 +6,6 @@ struct iovec;
 int packet_open(void);
 void packet_close(void);
 int packet_send(struct netdev *dev, struct iovec *iov, int iov_len);
-int packet_peek(struct netdev *dev);
 void packet_discard(struct netdev *dev);
 int packet_recv(struct netdev *dev, struct iovec *iov, int iov_len);
 
-- 
1.7.1

It may happen that process_receive_event is called with already configured
devices resulting in multiple prints of the device configuration.

This patch changes the behaviour of process_receive_event and ignores
already configured devices.

Signed-off-by: Ulrich Dangel <uli at spamt.net>
---
 usr/kinit/ipconfig/main.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/usr/kinit/ipconfig/main.c b/usr/kinit/ipconfig/main.c
index 43b6637..76708a9 100644
--- a/usr/kinit/ipconfig/main.c
+++ b/usr/kinit/ipconfig/main.c
@@ -180,6 +180,8 @@ static int process_receive_event(struct state *s, time_t
now)
 	switch (s->state) {
 	case DEVST_ERROR:
 		return 0; /* Not handled */
+	case DEVST_COMPLETE:
+		return 0; /* Not handled as already configured */
 
 	case DEVST_BOOTP:
 		s->restart_state = DEVST_BOOTP;
-- 
1.7.1

On Mon, 28 Mar 2011, Ulrich Dangel wrote:
> This patch fixes a bug on systems with multiple connected network devices.
> As packet_peek uses all devices to receive data instead of a specific
> device. As the return value was never reset it was possible that packets
> from other devices were returned by packet_peek. That means that the
> ifindex did not match any ifindex of the specified devices the packet was
> never removed and packets for the correct device were never processed.
> 
> This patch enhance packet_peek and packet_discard to only work on packages
> for the specified device instead of all packets.
> 
> Signed-off-by: Ulrich Dangel <uli at spamt.net>
> ---
>  usr/kinit/ipconfig/bootp_proto.c |    2 +-
>  usr/kinit/ipconfig/dhcp_proto.c  |    2 +-
>  usr/kinit/ipconfig/main.c        |   16 ++++++----------
>  usr/kinit/ipconfig/packet.c      |   16 +++++++++-------
>  usr/kinit/ipconfig/packet.h      |    6 +++---
>  5 files changed, 20 insertions(+), 22 deletions(-)
> 
applied and pushed out.
(please be patient on kernel.org git mirrors).

thank you.

-- 
maks

On Mon, 28 Mar 2011, Ulrich Dangel wrote:
> packet_peek is not really necessary as it does basically nothing besides
> testing if a packet is availabe. This is already done in packet_recv.
> Adjusted do_pkt_recv to only use process_receive_event.
> 
> packet_recv was modified to only receive and use packets from the specified
> interface.
> 
> process_receive_event was adjusted to handle discared/ignored packages from
> the corresponding recv functions. As packet_recv could return 0 for
> ignored packets process_receive_event handled them as processed/handled
> packets resulting in an endless loop if new (non valid dhcp/bootp) packets
> were received.
> 
> As a side effect ignored packets are now shown automatically in the debug
> output.
> 
> Signed-off-by: Ulrich Dangel <uli at spamt.net>
> ---
>  usr/kinit/ipconfig/main.c   |   24 +++++++++++++-----------
>  usr/kinit/ipconfig/packet.c |   43
+++++++------------------------------------
>  usr/kinit/ipconfig/packet.h |    1 -
>  3 files changed, 20 insertions(+), 48 deletions(-)
applied and pushed out.

-- 
maks

On Mon, 28 Mar 2011, Ulrich Dangel wrote:
> It may happen that process_receive_event is called with already configured
> devices resulting in multiple prints of the device configuration.
> 
> This patch changes the behaviour of process_receive_event and ignores
> already configured devices.
> 
> Signed-off-by: Ulrich Dangel <uli at spamt.net>
> ---
>  usr/kinit/ipconfig/main.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
applied (only sligtly modified respective patch subjects) and pushed out.

thank you.

-- 
maks