From 2d3e72365747259ba990fa380689e24f1fdd2a93 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 22 May 2016 14:25:02 +0200 Subject: [PATCH 1/3] sd-ipv4acd: drop HASH_KEY definition, as it is unused --- src/libsystemd-network/sd-ipv4acd.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libsystemd-network/sd-ipv4acd.c b/src/libsystemd-network/sd-ipv4acd.c index cc7436db6b..c336c6a62d 100644 --- a/src/libsystemd-network/sd-ipv4acd.c +++ b/src/libsystemd-network/sd-ipv4acd.c @@ -480,8 +480,6 @@ static bool ether_addr_is_nul(const struct ether_addr *addr) { return memcmp(addr, &nul_addr, sizeof(struct ether_addr)) == 0; } -#define HASH_KEY SD_ID128_MAKE(df,04,22,98,3f,ad,14,52,f9,87,2e,d1,9c,70,e2,f2) - int sd_ipv4acd_start(sd_ipv4acd *ll) { int r; From e78f9587abfe03ae49acb388becb64c8353a6cb2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 22 May 2016 14:26:06 +0200 Subject: [PATCH 2/3] sd-ipv4acd: do not define ether_addr_is_nul() redundantly we already have ether_addr_is_null() in ether-addr-util.h, let's use it here, too. --- src/libsystemd-network/sd-ipv4acd.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/libsystemd-network/sd-ipv4acd.c b/src/libsystemd-network/sd-ipv4acd.c index c336c6a62d..68563ad423 100644 --- a/src/libsystemd-network/sd-ipv4acd.c +++ b/src/libsystemd-network/sd-ipv4acd.c @@ -28,6 +28,7 @@ #include "alloc-util.h" #include "arp-util.h" +#include "ether-addr-util.h" #include "fd-util.h" #include "in-addr-util.h" #include "list.h" @@ -472,14 +473,6 @@ int sd_ipv4acd_is_running(sd_ipv4acd *ll) { return ll->state != IPV4ACD_STATE_INIT; } -static bool ether_addr_is_nul(const struct ether_addr *addr) { - const struct ether_addr nul_addr = {}; - - assert(addr); - - return memcmp(addr, &nul_addr, sizeof(struct ether_addr)) == 0; -} - int sd_ipv4acd_start(sd_ipv4acd *ll) { int r; @@ -487,7 +480,7 @@ int sd_ipv4acd_start(sd_ipv4acd *ll) { assert_return(ll->event, -EINVAL); assert_return(ll->index > 0, -EINVAL); assert_return(ll->address != 0, -EINVAL); - assert_return(!ether_addr_is_nul(&ll->mac_addr), -EINVAL); + assert_return(!ether_addr_is_null(&ll->mac_addr), -EINVAL); assert_return(ll->state == IPV4ACD_STATE_INIT, -EBUSY); ll->defend_window = 0; From e095f51dd16c530e56d8eb96960c0517be12d9bb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 22 May 2016 14:26:36 +0200 Subject: [PATCH 3/3] ipv4acd/ipv4ll: stop using read() on SOCK_DGRAM sockets This is a follow-up to cf447cb62d01137f4cbd1cd14b83b88823542bbf. Let's generally follow the rule to not use read() on SOCK_DGRAM sockets, let's always use recv() on that. Also, don't abort IPV4ACD logic in case we read a short packet. Simply log and ignore. --- src/libsystemd-network/sd-ipv4acd.c | 32 +++++++++++++++++++++------- src/libsystemd-network/test-ipv4ll.c | 9 ++++---- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/libsystemd-network/sd-ipv4acd.c b/src/libsystemd-network/sd-ipv4acd.c index 68563ad423..c1f43c824b 100644 --- a/src/libsystemd-network/sd-ipv4acd.c +++ b/src/libsystemd-network/sd-ipv4acd.c @@ -156,8 +156,10 @@ static void ipv4acd_set_state(sd_ipv4acd *ll, IPv4ACDState st, bool reset_counte static void ipv4acd_client_notify(sd_ipv4acd *ll, int event) { assert(ll); - if (ll->cb) - ll->cb(ll, event, ll->userdata); + if (!ll->cb) + return; + + ll->cb(ll, event, ll->userdata); } static void ipv4acd_stop(sd_ipv4acd *ll) { @@ -347,22 +349,36 @@ static void ipv4acd_on_conflict(sd_ipv4acd *ll) { ipv4acd_client_notify(ll, SD_IPV4ACD_EVENT_CONFLICT); } -static int ipv4acd_on_packet(sd_event_source *s, int fd, - uint32_t revents, void *userdata) { +static int ipv4acd_on_packet( + sd_event_source *s, + int fd, + uint32_t revents, + void *userdata) { + sd_ipv4acd *ll = userdata; struct ether_arp packet; + ssize_t n; int r; + assert(s); assert(ll); assert(fd >= 0); - r = read(fd, &packet, sizeof(struct ether_arp)); - if (r < (int) sizeof(struct ether_arp)) + n = recv(fd, &packet, sizeof(struct ether_arp), 0); + if (n < 0) { + r = log_ipv4acd_debug_errno(ll, errno, "Failed to read ARP packet: %m"); goto out; + } + if ((size_t) n != sizeof(struct ether_arp)) { + log_ipv4acd_debug(ll, "Ignoring too short ARP packet."); + return 0; + } switch (ll->state) { + case IPV4ACD_STATE_ANNOUNCING: case IPV4ACD_STATE_RUNNING: + if (ipv4acd_arp_conflict(ll, &packet)) { usec_t ts; @@ -381,15 +397,15 @@ static int ipv4acd_on_packet(sd_event_source *s, int fd, } else ipv4acd_on_conflict(ll); } - break; + case IPV4ACD_STATE_WAITING_PROBE: case IPV4ACD_STATE_PROBING: case IPV4ACD_STATE_WAITING_ANNOUNCE: /* BPF ensures this packet indicates a conflict */ ipv4acd_on_conflict(ll); - break; + default: assert_not_reached("Invalid state."); } diff --git a/src/libsystemd-network/test-ipv4ll.c b/src/libsystemd-network/test-ipv4ll.c index a233e0378c..8cdbfb8ed8 100644 --- a/src/libsystemd-network/test-ipv4ll.c +++ b/src/libsystemd-network/test-ipv4ll.c @@ -38,7 +38,8 @@ static int test_fd[2]; static int basic_request_handler_bind = 0; static int basic_request_handler_stop = 0; -static void* basic_request_handler_userdata = (void*)0xCABCAB; +static void* basic_request_handler_userdata = (void*) 0xCABCAB; + static void basic_request_handler(sd_ipv4ll *ll, int event, void *userdata) { assert_se(userdata == basic_request_handler_userdata); @@ -181,16 +182,16 @@ static void test_basic_request(sd_event *e) { /* PROBE */ sd_event_run(e, (uint64_t) -1); - assert_se(read(test_fd[1], &arp, sizeof(struct ether_arp)) == sizeof(struct ether_arp)); + assert_se(recv(test_fd[1], &arp, sizeof(struct ether_arp), 0) == sizeof(struct ether_arp)); if (extended) { /* PROBE */ sd_event_run(e, (uint64_t) -1); - assert_se(read(test_fd[1], &arp, sizeof(struct ether_arp)) == sizeof(struct ether_arp)); + assert_se(recv(test_fd[1], &arp, sizeof(struct ether_arp), 0) == sizeof(struct ether_arp)); /* PROBE */ sd_event_run(e, (uint64_t) -1); - assert_se(read(test_fd[1], &arp, sizeof(struct ether_arp)) == sizeof(struct ether_arp)); + assert_se(recv(test_fd[1], &arp, sizeof(struct ether_arp), 0) == sizeof(struct ether_arp)); sd_event_run(e, (uint64_t) -1); assert_se(basic_request_handler_bind == 1);