Revert [da213b6490] because the fd we're reading from could be a generic

file descriptor and have no concept of packet boundaries
(this is true for BPF).
Re-implement, but cleaner and with more commentary.

diff --git a/arp.c b/arp.c
index 5b35ccdd3ed9a0e2b9cee8a61c2731f9e50e3e12..04cfc17b56accbf065b1fc677f7d9cc221c297b2 100644 (file)
--- a/arp.c
+++ b/arp.c
@@ -119,32 +119,19 @@ arp_report_conflicted(const struct arp_state *astate,
 }
 
 static void
-arp_packet(void *arg)
+arp_packet(struct interface *ifp, uint8_t *data, size_t len)
 {
-       struct interface *ifp = arg;
        const struct interface *ifn;
-       uint8_t buf[ARP_LEN];
        struct arphdr ar;
        struct arp_msg arm;
-       ssize_t bytes;
-       struct iarp_state *state;
+       const struct iarp_state *state;
        struct arp_state *astate, *astaten;
-       unsigned char *hw_s, *hw_t;
-       int flags;
+       uint8_t *hw_s, *hw_t;
 
-       state = ARP_STATE(ifp);
-       flags = 0;
-       bytes = if_readraw(ifp, state->fd, buf, sizeof(buf), &flags);
-       if (bytes == -1) {
-               logger(ifp->ctx, LOG_ERR,
-                   "%s: arp if_readrawpacket: %m", ifp->name);
-               arp_close(ifp);
-               return;
-       }
        /* We must have a full ARP header */
-       if ((size_t)bytes < sizeof(ar))
+       if (len < sizeof(ar))
                return;
-       memcpy(&ar, buf, sizeof(ar));
+       memcpy(&ar, data, sizeof(ar));
        /* Families must match */
        if (ar.ar_hrd != htons(ifp->family))
                return;
@@ -162,10 +149,10 @@ arp_packet(void *arg)
                return;
 
        /* Get pointers to the hardware addreses */
-       hw_s = buf + sizeof(ar);
+       hw_s = data + sizeof(ar);
        hw_t = hw_s + ar.ar_hln + ar.ar_pln;
        /* Ensure we got all the data */
-       if ((hw_t + ar.ar_hln + ar.ar_pln) - buf > bytes)
+       if ((size_t)((hw_t + ar.ar_hln + ar.ar_pln) - data) > len)
                return;
        /* Ignore messages from ourself */
        TAILQ_FOREACH(ifn, ifp->ctx->ifaces, next) {
@@ -187,12 +174,39 @@ arp_packet(void *arg)
        memcpy(&arm.tip.s_addr, hw_t + ar.ar_hln, ar.ar_pln);
 
        /* Run the conflicts */
+       state = ARP_CSTATE(ifp);
        TAILQ_FOREACH_SAFE(astate, &state->arp_states, next, astaten) {
                if (astate->conflicted_cb)
                        astate->conflicted_cb(astate, &arm);
        }
 }
 
+static void
+arp_read(void *arg)
+{
+       struct interface *ifp = arg;
+       const struct iarp_state *state;
+       uint8_t buf[ARP_LEN];
+       int flags;
+       ssize_t bytes;
+
+       /* Some RAW mechanisms are generic file descriptors, not sockets.
+        * This means we have no kernel call to just get one packet,
+        * so we have to process the entire buffer. */
+       state = ARP_CSTATE(ifp);
+       flags = 0;
+       while (!(flags & RAW_EOF)) {
+               bytes = if_readraw(ifp, state->fd, buf, sizeof(buf), &flags);
+               if (bytes == -1) {
+                       logger(ifp->ctx, LOG_ERR,
+                           "%s: arp if_readrawpacket: %m", ifp->name);
+                       arp_close(ifp);
+                       return;
+               }
+               arp_packet(ifp, buf, (size_t)bytes);
+       }
+}
+
 int
 arp_open(struct interface *ifp)
 {
@@ -206,7 +220,7 @@ arp_open(struct interface *ifp)
                            __func__, ifp->name);
                        return -1;
                }
-               eloop_event_add(ifp->ctx->eloop, state->fd, arp_packet, ifp);
+               eloop_event_add(ifp->ctx->eloop, state->fd, arp_read, ifp);
        }
        return state->fd;
 }

diff --git a/dhcp.c b/dhcp.c
index 0d473882fa4030ba4fa8ef6e99fff4337e48c980..f1fba03f11c5ac2386ecafc80b4c4ead7cd54a98 100644 (file)
--- a/dhcp.c
+++ b/dhcp.c
@@ -3125,27 +3125,15 @@ valid_udp_packet(uint8_t *data, size_t data_len, struct in_addr *from,
 }
 
 static void
-dhcp_handlepacket(void *arg)
+dhcp_handlepacket(struct interface *ifp, uint8_t *data, size_t len, int flags)
 {
-       struct interface *ifp = arg;
-       uint8_t *bootp, buf[MTU_MAX];
-       size_t bytes;
+       uint8_t *bootp;
        struct in_addr from;
-       int i, flags;
+       int i;
+       size_t udp_len;
        const struct dhcp_state *state = D_CSTATE(ifp);
 
-       /* Need this API due to BPF */
-       flags = 0;
-       bootp = NULL;
-       bytes = (size_t)if_readraw(ifp, state->raw_fd,buf, sizeof(buf), &flags);
-       if ((ssize_t)bytes == -1) {
-               logger(ifp->ctx, LOG_ERR,
-                   "%s: dhcp if_readrawpacket: %m", ifp->name);
-               dhcp_close(ifp);
-               arp_close(ifp);
-               return;
-       }
-       if (valid_udp_packet(buf, bytes, &from, flags & RAW_PARTIALCSUM) == -1)
+       if (valid_udp_packet(data, len, &from, flags & RAW_PARTIALCSUM) == -1)
        {
                logger(ifp->ctx, LOG_ERR, "%s: invalid UDP packet from %s",
                    ifp->name, inet_ntoa(from));
@@ -3170,6 +3158,7 @@ dhcp_handlepacket(void *arg)
                    "%s: server %s is not destination",
                    ifp->name, inet_ntoa(from));
        }
+
        /*
         * DHCP has a variable option area rather than a fixed vendor area.
         * Because DHCP uses the BOOTP protocol it should still send BOOTP
@@ -3177,19 +3166,45 @@ dhcp_handlepacket(void *arg)
         * However some servers send a truncated vendor area.
         * dhcpcd can work fine without the vendor area being sent.
         */
-       bytes = get_udp_data(&bootp, buf);
-       if (bytes < offsetof(struct bootp, vend)) {
+       udp_len = get_udp_data(&bootp, data);
+       if (udp_len < offsetof(struct bootp, vend)) {
                logger(ifp->ctx, LOG_ERR,
                    "%s: truncated packet (%zu) from %s",
-                   ifp->name, bytes, inet_ntoa(from));
+                   ifp->name, udp_len, inet_ntoa(from));
                return;
        }
        /* But to make our IS_DHCP macro easy, ensure the vendor
         * area has at least 4 octets. */
-       while (bytes < offsetof(struct bootp, vend) + 4)
-               bootp[bytes++] = '\0';
+       while (udp_len < offsetof(struct bootp, vend) + 4)
+               bootp[udp_len++] = '\0';
 
-       dhcp_handledhcp(ifp, (struct bootp *)bootp, bytes, &from);
+       dhcp_handledhcp(ifp, (struct bootp *)bootp, udp_len, &from);
+}
+
+static void
+dhcp_readpacket(void *arg)
+{
+       struct interface *ifp = arg;
+       uint8_t buf[MTU_MAX];
+       ssize_t bytes;
+       int flags;
+       const struct dhcp_state *state = D_CSTATE(ifp);
+
+       /* Some RAW mechanisms are generic file descriptors, not sockets.
+        * This means we have no kernel call to just get one packet,
+        * so we have to process the entire buffer. */
+       flags = 0;
+       while (!(flags & RAW_EOF)) {
+               bytes = if_readraw(ifp, state->raw_fd, buf,sizeof(buf), &flags);
+               if (bytes == -1) {
+                       logger(ifp->ctx, LOG_ERR,
+                           "%s: dhcp if_readrawpacket: %m", ifp->name);
+                       dhcp_close(ifp);
+                       arp_close(ifp);
+                       return;
+               }
+               dhcp_handlepacket(ifp, buf, (size_t)bytes, flags);
+       }
 }
 
 static void
@@ -3231,7 +3246,7 @@ dhcp_open(struct interface *ifp)
                        return -1;
                }
                eloop_event_add(ifp->ctx->eloop,
-                   state->raw_fd, dhcp_handlepacket, ifp);
+                   state->raw_fd, dhcp_readpacket, ifp);
        }
        return 0;
 }

diff --git a/if-bsd.c b/if-bsd.c
index 1628a1219b637ac9d11e88456c37a818f0aa0d7b..ec46684f3e7a5b8f25b4f6ee1391998a92ba8ce8 100644 (file)
--- a/if-bsd.c
+++ b/if-bsd.c
@@ -526,14 +526,15 @@ if_readraw(struct interface *ifp, int fd, void *data, size_t len, int *flags)
 next:
                state->buffer_pos += BPF_WORDALIGN(packet.bh_hdrlen +
                    packet.bh_caplen);
-               if (state->buffer_pos >= state->buffer_len)
+               if (state->buffer_pos >= state->buffer_len) {
                        state->buffer_len = state->buffer_pos = 0;
+                       *flags |= RAW_EOF;
+               }
                if (bytes != -1)
                        return bytes;
        }
 }
 
-
 int
 if_address(unsigned char cmd, const struct ipv4_addr *ia)
 {

diff --git a/if-linux.c b/if-linux.c
index 4bb800bc75041be6d45daed25603585877e95d83..f3dc5fe80740a9d0e7ceaafae77a040bd48b0c9a 100644 (file)
--- a/if-linux.c
+++ b/if-linux.c
@@ -1334,7 +1334,7 @@ if_readraw(__unused struct interface *ifp, int fd,
        bytes = recvmsg(fd, &msg, 0);
        if (bytes == -1)
                return -1;
-       *flags = 0;
+       *flags = RAW_EOF; /* We only ever read one packet. */
        if (bytes) {
 #ifdef PACKET_AUXDATA
                for (cmsg = CMSG_FIRSTHDR(&msg);

diff --git a/if.h b/if.h
index 79d04ed6f8dc6b60223956f7264b916934adf4d5..249dd2e43fecaccfe850c8fef1751925f594bd8f 100644 (file)
--- a/if.h
+++ b/if.h
@@ -80,7 +80,8 @@
            ((addr & IN_CLASSB_NET) == 0xc0a80000))
 #endif
 
-#define RAW_PARTIALCSUM                1 << 0
+#define RAW_EOF                        1 << 0
+#define RAW_PARTIALCSUM                2 << 0
 
 #ifdef __sun
 /* Solaris getifaddrs is very un-suitable for dhcpcd.