[systemd-commits] 4 commits - src/libsystemd-network src/udev

Tom Gundersen tomegun at kemper.freedesktop.org
Sun Apr 6 10:46:52 PDT 2014


 src/libsystemd-network/dhcp-internal.h    |    2 +-
 src/libsystemd-network/dhcp-network.c     |   28 ++++++++++++++++++++++++----
 src/libsystemd-network/dhcp-option.c      |   12 ++++--------
 src/libsystemd-network/dhcp-packet.c      |   28 ++--------------------------
 src/libsystemd-network/dhcp-protocol.h    |    4 +++-
 src/libsystemd-network/sd-dhcp-client.c   |   24 +++++++++++++++++-------
 src/libsystemd-network/test-dhcp-client.c |    2 +-
 src/udev/udev-builtin-net_id.c            |    5 +++--
 8 files changed, 55 insertions(+), 50 deletions(-)

New commits:
commit 7429b07f822348dc5a87208ce107f5f6bf02656d
Author: Tom Gundersen <teg at jklm.no>
Date:   Sun Apr 6 19:23:33 2014 +0200

    sd-dhcp-client: improve BPF
    
    Try a bit harder to make the kernel drop packets not for us. This should reduce
    the number of wakeups from n^2 to n in the number of dhcp clients, which admittedly
    only makes a differenc in very extreme cases.

diff --git a/src/libsystemd-network/dhcp-internal.h b/src/libsystemd-network/dhcp-internal.h
index 064b13b..324c2a8 100644
--- a/src/libsystemd-network/dhcp-internal.h
+++ b/src/libsystemd-network/dhcp-internal.h
@@ -29,7 +29,7 @@
 
 #include "dhcp-protocol.h"
 
-int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link);
+int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link, uint32_t xid);
 int dhcp_network_bind_udp_socket(int index, be32_t address, uint16_t port);
 int dhcp_network_send_raw_socket(int s, const union sockaddr_union *link,
                                  const void *packet, size_t len);
diff --git a/src/libsystemd-network/dhcp-network.c b/src/libsystemd-network/dhcp-network.c
index a9a15b4..902c9d9 100644
--- a/src/libsystemd-network/dhcp-network.c
+++ b/src/libsystemd-network/dhcp-network.c
@@ -23,6 +23,7 @@
 #include <string.h>
 #include <linux/if_packet.h>
 #include <net/ethernet.h>
+#include <net/if_arp.h>
 #include <stdio.h>
 #include <unistd.h>
 #include <linux/filter.h>
@@ -31,18 +32,34 @@
 
 #include "dhcp-internal.h"
 
-int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link)
-{
+int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link, uint32_t xid) {
         struct sock_filter filter[] = {
             BPF_STMT(BPF_LD + BPF_W + BPF_LEN, 0),                                 /* A <- packet length */
             BPF_JUMP(BPF_JMP + BPF_JGE + BPF_K, sizeof(DHCPPacket), 1, 0),         /* packet >= DHCPPacket ? */
             BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
+            /* TODO: match ip.version */
             BPF_STMT(BPF_LD + BPF_B + BPF_ABS, offsetof(DHCPPacket, ip.protocol)), /* A <- IP protocol */
             BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, IPPROTO_UDP, 1, 0),                /* IP protocol == UDP ? */
             BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
             BPF_STMT(BPF_LD + BPF_H + BPF_ABS, offsetof(DHCPPacket, udp.dest)),    /* A <- UDP destination port */
             BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, DHCP_PORT_CLIENT, 1, 0),           /* UDP destination port == DHCP client port ? */
             BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
+            BPF_STMT(BPF_LD + BPF_B + BPF_ABS, offsetof(DHCPPacket, dhcp.op)),     /* A <- DHCP op */
+            BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, BOOTREPLY, 1, 0),                  /* op == BOOTREPLY ? */
+            BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
+            BPF_STMT(BPF_LD + BPF_B + BPF_ABS, offsetof(DHCPPacket, dhcp.htype)),  /* A <- DHCP header type */
+            BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, ARPHRD_ETHER, 1, 0),               /* header type == ARPHRD_ETHER ? */
+            BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
+            BPF_STMT(BPF_LD + BPF_B + BPF_ABS, offsetof(DHCPPacket, dhcp.hlen)),   /* A <- mac address length */
+            BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, ETHER_ADDR_LEN, 1, 0),             /* address length == ETHER_ADDR_LEN ? */
+            BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
+            BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof(DHCPPacket, dhcp.xid)),    /* A <- client identifier */
+            BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, xid, 1, 0),                        /* client identifier == xid ? */
+            BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
+            /* TODO: match chaddr */
+            BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof(DHCPPacket, dhcp.magic)),  /* A <- DHCP magic cookie */
+            BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, DHCP_MAGIC_COOKIE, 1, 0),          /* cookie == DHCP magic cookie ? */
+            BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
             BPF_STMT(BPF_RET + BPF_K, 65535),                                      /* return all */
         };
         struct sock_fprog fprog = {
diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c
index 722f862..da41c47 100644
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@@ -593,7 +593,7 @@ static int client_start(sd_dhcp_client *client) {
 
         client->xid = random_u32();
 
-        r = dhcp_network_bind_raw_socket(client->index, &client->link);
+        r = dhcp_network_bind_raw_socket(client->index, &client->link, client->xid);
 
         if (r < 0) {
                 client_stop(client, r);
@@ -636,7 +636,7 @@ static int client_timeout_t2(sd_event_source *s, uint64_t usec, void *userdata)
         client->state = DHCP_STATE_REBINDING;
         client->attempt = 1;
 
-        r = dhcp_network_bind_raw_socket(client->index, &client->link);
+        r = dhcp_network_bind_raw_socket(client->index, &client->link, client->xid);
         if (r < 0) {
                 client_stop(client, r);
                 return 0;
diff --git a/src/libsystemd-network/test-dhcp-client.c b/src/libsystemd-network/test-dhcp-client.c
index 2d4d590..4420436 100644
--- a/src/libsystemd-network/test-dhcp-client.c
+++ b/src/libsystemd-network/test-dhcp-client.c
@@ -190,7 +190,7 @@ int dhcp_network_send_raw_socket(int s, const union sockaddr_union *link,
         return 575;
 }
 
-int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link)
+int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link, uint32_t id)
 {
         if (socketpair(AF_UNIX, SOCK_STREAM, 0, test_fd) < 0)
                 return -errno;

commit 0c79c68d93d721d37ba088fb50dbf07bb0d447e5
Author: Tom Gundersen <teg at jklm.no>
Date:   Sun Apr 6 19:35:36 2014 +0200

    sd-dhcp-client: eagerly drop too small packets
    
    If they are too small to fit the IP+UDP+DHCP headers they can be of no use, so
    don't waste resources parsing them. This is at the cost of losing some verbosity
    in the logging.

diff --git a/src/libsystemd-network/dhcp-network.c b/src/libsystemd-network/dhcp-network.c
index 8bfb2d5..a9a15b4 100644
--- a/src/libsystemd-network/dhcp-network.c
+++ b/src/libsystemd-network/dhcp-network.c
@@ -34,11 +34,14 @@
 int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link)
 {
         struct sock_filter filter[] = {
+            BPF_STMT(BPF_LD + BPF_W + BPF_LEN, 0),                                 /* A <- packet length */
+            BPF_JUMP(BPF_JMP + BPF_JGE + BPF_K, sizeof(DHCPPacket), 1, 0),         /* packet >= DHCPPacket ? */
+            BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
             BPF_STMT(BPF_LD + BPF_B + BPF_ABS, offsetof(DHCPPacket, ip.protocol)), /* A <- IP protocol */
-            BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, IPPROTO_UDP, 1, 0),                /* IP protocol = UDP? */
+            BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, IPPROTO_UDP, 1, 0),                /* IP protocol == UDP ? */
             BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
             BPF_STMT(BPF_LD + BPF_H + BPF_ABS, offsetof(DHCPPacket, udp.dest)),    /* A <- UDP destination port */
-            BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, DHCP_PORT_CLIENT, 1, 0),           /* UDP destination port = DHCP client? */
+            BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, DHCP_PORT_CLIENT, 1, 0),           /* UDP destination port == DHCP client port ? */
             BPF_STMT(BPF_RET + BPF_K, 0),                                          /* ignore */
             BPF_STMT(BPF_RET + BPF_K, 65535),                                      /* return all */
         };
diff --git a/src/libsystemd-network/dhcp-packet.c b/src/libsystemd-network/dhcp-packet.c
index 0549f57..102ed09 100644
--- a/src/libsystemd-network/dhcp-packet.c
+++ b/src/libsystemd-network/dhcp-packet.c
@@ -113,13 +113,6 @@ int dhcp_packet_verify_headers(DHCPPacket *packet, size_t len, bool checksum) {
 
         /* IP */
 
-        if (len < DHCP_IP_SIZE) {
-                log_dhcp_client(client, "ignoring packet: packet (%zu bytes) "
-                                " smaller than IP header (%u bytes)", len,
-                                DHCP_IP_SIZE);
-                return -EINVAL;
-        }
-
         if (packet->ip.version != IPVERSION) {
                 log_dhcp_client(client, "ignoring packet: not IPv4");
                 return -EINVAL;
@@ -152,13 +145,6 @@ int dhcp_packet_verify_headers(DHCPPacket *packet, size_t len, bool checksum) {
                 return -EINVAL;
         }
 
-        if (len < DHCP_IP_UDP_SIZE) {
-                log_dhcp_client(client, "ignoring packet: packet (%zu bytes) "
-                                " smaller than IP+UDP header (%u bytes)", len,
-                                DHCP_IP_UDP_SIZE);
-                return -EINVAL;
-        }
-
         if (len < hdrlen + be16toh(packet->udp.len)) {
                 log_dhcp_client(client, "ignoring packet: packet (%zu bytes) "
                                 "smaller than expected (%zu) by UDP header", len,
diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c
index 5824e6e..722f862 100644
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@@ -931,12 +931,6 @@ static int client_handle_message(sd_dhcp_client *client, DHCPMessage *message,
         assert(client->event);
         assert(message);
 
-        if (len < DHCP_MESSAGE_SIZE) {
-                log_dhcp_client(client, "message too small (%d bytes): "
-                                "ignoring", len);
-                return 0;
-        }
-
         if (be32toh(message->magic) != DHCP_MAGIC_COOKIE) {
                 log_dhcp_client(client, "not a DHCP message: ignoring");
                 return 0;
@@ -1081,7 +1075,11 @@ static int client_receive_message_udp(sd_event_source *s, int fd,
                 return -ENOMEM;
 
         len = read(fd, message, buflen);
-        if (len < 0)
+        if (len < 0) {
+                log_dhcp_client(client, "could not receive message from UDP "
+                                "socket: %s", strerror(errno));
+                return 0;
+        } else if ((size_t)len < sizeof(DHCPMessage))
                 return 0;
 
         return client_handle_message(client, message, len);
@@ -1122,7 +1120,8 @@ static int client_receive_message_raw(sd_event_source *s, int fd,
                 log_dhcp_client(client, "could not receive message from raw "
                                 "socket: %s", strerror(errno));
                 return 0;
-        }
+        } else if ((size_t)len < sizeof(DHCPPacket))
+                return 0;
 
         for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
                 if (cmsg->cmsg_level == SOL_PACKET && cmsg->cmsg_type == PACKET_AUXDATA) {

commit 3b7ca119fdc501e21f017695dc9b6f82fdbd1d93
Author: Tom Gundersen <teg at jklm.no>
Date:   Sun Apr 6 14:05:32 2014 +0200

    sd-dhcp-client: move magic cookie into DHCPMessage struct
    
    Also move the checking of it to the main message handler, rather than the
    options parser.
    
    Fix a bug, so we now drop the packet if any of the magic bytes don't match.
    Before we used to only drop the packet if they were all wrong.

diff --git a/src/libsystemd-network/dhcp-option.c b/src/libsystemd-network/dhcp-option.c
index 4d45b3b..1b92e86 100644
--- a/src/libsystemd-network/dhcp-option.c
+++ b/src/libsystemd-network/dhcp-option.c
@@ -149,17 +149,13 @@ int dhcp_option_parse(DHCPMessage *message, size_t len,
         if (!message)
                 return -EINVAL;
 
-        if (len < sizeof(DHCPMessage) + 4)
+        if (len < sizeof(DHCPMessage))
                 return -EINVAL;
 
-        len -= sizeof(DHCPMessage) + 4;
+        len -= sizeof(DHCPMessage);
 
-        if (opt[0] != 0x63 && opt[1] != 0x82 && opt[2] != 0x53 &&
-                        opt[3] != 0x63)
-                return -EINVAL;
-
-        res = parse_options(&opt[4], len, &overload, &message_type,
-                        cb, user_data);
+        res = parse_options(opt, len, &overload, &message_type,
+                            cb, user_data);
         if (res < 0)
                 return res;
 
diff --git a/src/libsystemd-network/dhcp-packet.c b/src/libsystemd-network/dhcp-packet.c
index 4f90c28..0549f57 100644
--- a/src/libsystemd-network/dhcp-packet.c
+++ b/src/libsystemd-network/dhcp-packet.c
@@ -43,23 +43,13 @@ int dhcp_message_init(DHCPMessage *message, uint8_t op, uint32_t xid,
 
         assert(op == BOOTREQUEST || op == BOOTREPLY);
 
-        *opt = (uint8_t *)(message + 1);
-
-        if (*optlen < 4)
-                return -ENOBUFS;
-        *optlen -= 4;
-
         message->op = op;
         message->htype = ARPHRD_ETHER;
         message->hlen = ETHER_ADDR_LEN;
         message->xid = htobe32(xid);
+        message->magic = htobe32(DHCP_MAGIC_COOKIE);
 
-        (*opt)[0] = 0x63;
-        (*opt)[1] = 0x82;
-        (*opt)[2] = 0x53;
-        (*opt)[3] = 0x63;
-
-        *opt += 4;
+        *opt = (uint8_t *)(message + 1);
 
         err = dhcp_option_append(opt, optlen, DHCP_OPTION_MESSAGE_TYPE, 1,
                                  &type);
diff --git a/src/libsystemd-network/dhcp-protocol.h b/src/libsystemd-network/dhcp-protocol.h
index 9aa9618..400e953 100644
--- a/src/libsystemd-network/dhcp-protocol.h
+++ b/src/libsystemd-network/dhcp-protocol.h
@@ -43,6 +43,7 @@ struct DHCPMessage {
         uint8_t chaddr[16];
         uint8_t sname[64];
         uint8_t file[128];
+        be32_t magic;
 } _packed_;
 
 typedef struct DHCPMessage DHCPMessage;
@@ -58,7 +59,8 @@ typedef struct DHCPPacket DHCPPacket;
 #define DHCP_IP_SIZE            (int32_t)(sizeof(struct iphdr))
 #define DHCP_IP_UDP_SIZE        (int32_t)(sizeof(struct udphdr) + DHCP_IP_SIZE)
 #define DHCP_MESSAGE_SIZE       (int32_t)(sizeof(DHCPMessage))
-#define DHCP_MIN_OPTIONS_SIZE   312
+#define DHCP_MIN_OPTIONS_SIZE   308
+#define DHCP_MAGIC_COOKIE       (uint32_t)(0x63825363)
 
 enum {
         DHCP_PORT_SERVER                        = 67,
diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c
index 06b2d1c..5824e6e 100644
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@@ -22,6 +22,7 @@
 #include <string.h>
 #include <stdio.h>
 #include <net/ethernet.h>
+#include <net/if_arp.h>
 #include <sys/param.h>
 #include <sys/ioctl.h>
 
@@ -936,6 +937,11 @@ static int client_handle_message(sd_dhcp_client *client, DHCPMessage *message,
                 return 0;
         }
 
+        if (be32toh(message->magic) != DHCP_MAGIC_COOKIE) {
+                log_dhcp_client(client, "not a DHCP message: ignoring");
+                return 0;
+        }
+
         if (message->op != BOOTREPLY) {
                 log_dhcp_client(client, "not a BOOTREPLY message: ignoring");
                 return 0;
@@ -948,6 +954,11 @@ static int client_handle_message(sd_dhcp_client *client, DHCPMessage *message,
                 return 0;
         }
 
+        if (message->htype != ARPHRD_ETHER || message->hlen != ETHER_ADDR_LEN) {
+                log_dhcp_client(client, "not an ethernet packet");
+                return 0;
+        }
+
         if (memcmp(&message->chaddr[0], &client->client_id.mac_addr,
                    ETH_ALEN)) {
                 log_dhcp_client(client, "received chaddr does not match "

commit 19aa72f74e41045510b4af3f1415b419d42ff20b
Author: Tom Gundersen <teg at jklm.no>
Date:   Sun Apr 6 18:00:26 2014 +0200

    udev: net_id - use constants rather than magic numbers

diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c
index aa08196..c80c30a 100644
--- a/src/udev/udev-builtin-net_id.c
+++ b/src/udev/udev-builtin-net_id.c
@@ -94,6 +94,7 @@
 #include <string.h>
 #include <errno.h>
 #include <net/if.h>
+#include <net/if_arp.h>
 #include <linux/pci_regs.h>
 
 #include "udev.h"
@@ -451,10 +452,10 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool
                 return EXIT_FAILURE;
         i = strtoul(s, NULL, 0);
         switch (i) {
-        case 1: /* ARPHRD_ETHER */
+        case ARPHRD_ETHER:
                 prefix = "en";
                 break;
-        case 256: /* ARPHRD_SLIP */
+        case ARPHRD_SLIP:
                 prefix = "sl";
                 break;
         default:



More information about the systemd-commits mailing list