From 1b89cf56483991c1b6eaa2dcd375630f4623ba3a Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Thu, 10 Apr 2014 19:40:48 +0200 Subject: [PATCH] sd-rtnl: don't drop multi-part messages We still only return the first message part in callback/synchronous calls. --- src/libsystemd/sd-rtnl/rtnl-internal.h | 4 +- src/libsystemd/sd-rtnl/rtnl-message.c | 141 ++++++++++++++----------- src/libsystemd/sd-rtnl/sd-rtnl.c | 104 +++++++++--------- 3 files changed, 131 insertions(+), 118 deletions(-) diff --git a/src/libsystemd/sd-rtnl/rtnl-internal.h b/src/libsystemd/sd-rtnl/rtnl-internal.h index 9003dad31..9d857ed99 100644 --- a/src/libsystemd/sd-rtnl/rtnl-internal.h +++ b/src/libsystemd/sd-rtnl/rtnl-internal.h @@ -107,7 +107,9 @@ struct sd_rtnl_message { int message_new(sd_rtnl *rtnl, sd_rtnl_message **ret, uint16_t type); int socket_write_message(sd_rtnl *nl, sd_rtnl_message *m); -int socket_read_message(sd_rtnl *nl, sd_rtnl_message **ret); +int socket_read_message(sd_rtnl *nl); + +int rtnl_rqueue_make_room(sd_rtnl *rtnl); int rtnl_message_read_internal(sd_rtnl_message *m, unsigned short type, void **data); int rtnl_message_parse(sd_rtnl_message *m, diff --git a/src/libsystemd/sd-rtnl/rtnl-message.c b/src/libsystemd/sd-rtnl/rtnl-message.c index e5854de4c..edf567235 100644 --- a/src/libsystemd/sd-rtnl/rtnl-message.c +++ b/src/libsystemd/sd-rtnl/rtnl-message.c @@ -42,11 +42,10 @@ #define GET_CONTAINER(m, i) ((i) < (m)->n_containers ? (struct rtattr*)((uint8_t*)(m)->hdr + (m)->container_offsets[i]) : NULL) #define PUSH_CONTAINER(m, new) (m)->container_offsets[(m)->n_containers ++] = (uint8_t*)(new) - (uint8_t*)(m)->hdr; -static int message_new_empty(sd_rtnl *rtnl, sd_rtnl_message **ret, size_t initial_size) { +static int message_new_empty(sd_rtnl *rtnl, sd_rtnl_message **ret) { sd_rtnl_message *m; assert_return(ret, -EINVAL); - assert_return(initial_size >= sizeof(struct nlmsghdr), -EINVAL); /* Note that 'rtnl' is curretly unused, if we start using it internally we must take care to avoid problems due to mutual references between @@ -57,15 +56,8 @@ static int message_new_empty(sd_rtnl *rtnl, sd_rtnl_message **ret, size_t initia if (!m) return -ENOMEM; - m->hdr = malloc0(initial_size); - if (!m->hdr) { - free(m); - return -ENOMEM; - } - m->n_ref = REFCNT_INIT; - m->hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; m->sealed = false; *ret = m; @@ -74,6 +66,7 @@ static int message_new_empty(sd_rtnl *rtnl, sd_rtnl_message **ret, size_t initia } int message_new(sd_rtnl *rtnl, sd_rtnl_message **ret, uint16_t type) { + _cleanup_rtnl_message_unref_ sd_rtnl_message *m = NULL; const NLType *nl_type; size_t size; int r; @@ -84,15 +77,25 @@ int message_new(sd_rtnl *rtnl, sd_rtnl_message **ret, uint16_t type) { assert(nl_type->type == NLA_NESTED); - size = NLMSG_SPACE(nl_type->size); - - r = message_new_empty(rtnl, ret, size); + r = message_new_empty(rtnl, &m); if (r < 0) return r; - (*ret)->container_type_system[0] = nl_type->type_system; - (*ret)->hdr->nlmsg_len = size; - (*ret)->hdr->nlmsg_type = type; + size = NLMSG_SPACE(nl_type->size); + + assert(size >= sizeof(struct nlmsghdr)); + m->hdr = malloc0(size); + if (!m->hdr) + return -ENOMEM; + + m->hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; + + m->container_type_system[0] = nl_type->type_system; + m->hdr->nlmsg_len = size; + m->hdr->nlmsg_type = type; + + *ret = m; + m = NULL; return 0; } @@ -1014,33 +1017,28 @@ int socket_write_message(sd_rtnl *nl, sd_rtnl_message *m) { * If nothing useful was received 0 is returned. * On failure, a negative error code is returned. */ -int socket_read_message(sd_rtnl *nl, sd_rtnl_message **ret) { - _cleanup_rtnl_message_unref_ sd_rtnl_message *m = NULL; - const NLType *nl_type; - struct nlmsghdr *new_hdr; +int socket_read_message(sd_rtnl *rtnl) { + _cleanup_free_ void *buffer = NULL; + struct nlmsghdr *new_msg; union { struct sockaddr sa; struct sockaddr_nl nl; } addr; - socklen_t addr_len; + socklen_t addr_len = sizeof(addr); size_t need, len; - int r; + int r, ret = 0; - assert(nl); - assert(ret); - - r = message_receive_need(nl, &need); - if (r < 0) - return r; + assert(rtnl); - r = message_new_empty(nl, &m, need); + r = message_receive_need(rtnl, &need); if (r < 0) return r; - addr_len = sizeof(addr); + buffer = malloc0(need); + if (!buffer) + return -ENOMEM; - r = recvfrom(nl->fd, m->hdr, need, - 0, &addr.sa, &addr_len); + r = recvfrom(rtnl->fd, buffer, need, 0, &addr.sa, &addr_len); if (r < 0) return (errno == EAGAIN) ? 0 : -errno; /* no data */ else if (r == 0) @@ -1050,47 +1048,64 @@ int socket_read_message(sd_rtnl *nl, sd_rtnl_message **ret) { return -EIO; /* not a netlink message */ else if (addr.nl.nl_pid != 0) return 0; /* not from the kernel */ - else if ((size_t) r < sizeof(struct nlmsghdr) || - (size_t) r < m->hdr->nlmsg_len) - return -EIO; /* too small (we do accept too big though) */ - else if (m->hdr->nlmsg_pid && m->hdr->nlmsg_pid != nl->sockaddr.nl.nl_pid) - return 0; /* not broadcast and not for us */ else - len = (size_t) r; + len = (size_t)r; - /* silently drop noop messages */ - if (m->hdr->nlmsg_type == NLMSG_NOOP) - return 0; + for (new_msg = buffer; NLMSG_OK(new_msg, len); new_msg = NLMSG_NEXT(new_msg, len)) { + _cleanup_rtnl_message_unref_ sd_rtnl_message *m = NULL; + const NLType *nl_type; - /* check that we support this message type */ - r = type_system_get_type(NULL, &nl_type, m->hdr->nlmsg_type); - if (r < 0) { - if (r == -ENOTSUP) - log_debug("sd-rtnl: ignored message with unknown type: %u", - m->hdr->nlmsg_type); + if (new_msg->nlmsg_pid && new_msg->nlmsg_pid != rtnl->sockaddr.nl.nl_pid) + /* not broadcast and not for us */ + continue; - return 0; - } + /* silently drop noop messages */ + if (new_msg->nlmsg_type == NLMSG_NOOP) + continue; - /* check that the size matches the message type */ - if (len < NLMSG_LENGTH(nl_type->size)) - return -EIO; + /* check that we support this message type */ + r = type_system_get_type(NULL, &nl_type, new_msg->nlmsg_type); + if (r < 0) { + if (r == -ENOTSUP) + log_debug("sd-rtnl: ignored message with unknown type: %u", + new_msg->nlmsg_type); - /* we probably allocated way too much memory, give it back */ - new_hdr = realloc(m->hdr, len); - if (!new_hdr) - return -ENOMEM; - m->hdr = new_hdr; + continue; + } - /* seal and parse the top-level message */ - r = sd_rtnl_message_rewind(m); - if (r < 0) - return r; + /* check that the size matches the message type */ + if (new_msg->nlmsg_len < NLMSG_LENGTH(nl_type->size)) + continue; - *ret = m; - m = NULL; + r = message_new_empty(rtnl, &m); + if (r < 0) + return r; + + m->hdr = memdup(new_msg, new_msg->nlmsg_len); + if (!m->hdr) + return -ENOMEM; + + /* seal and parse the top-level message */ + r = sd_rtnl_message_rewind(m); + if (r < 0) + return r; + + r = rtnl_rqueue_make_room(rtnl); + if (r < 0) + return r; + + rtnl->rqueue[rtnl->rqueue_size ++] = m; + m = NULL; + ret += new_msg->nlmsg_len; + + /* reached end of multi-part message, or not a multi-part + message at all */ + if (new_msg->nlmsg_type == NLMSG_DONE || + !(new_msg->nlmsg_flags & NLM_F_MULTI)) + break; + } - return len; + return ret; } int sd_rtnl_message_rewind(sd_rtnl_message *m) { diff --git a/src/libsystemd/sd-rtnl/sd-rtnl.c b/src/libsystemd/sd-rtnl/sd-rtnl.c index 8e709bb66..816018a6c 100644 --- a/src/libsystemd/sd-rtnl/sd-rtnl.c +++ b/src/libsystemd/sd-rtnl/sd-rtnl.c @@ -203,29 +203,35 @@ int sd_rtnl_send(sd_rtnl *nl, return 1; } +int rtnl_rqueue_make_room(sd_rtnl *rtnl) { + assert(rtnl); + + if (rtnl->rqueue_size >= RTNL_RQUEUE_MAX) + return -ENOBUFS; + + if (!GREEDY_REALLOC(rtnl->rqueue, rtnl->rqueue_allocated, rtnl->rqueue_size + 1)) + return -ENOMEM; + + return 0; +} + static int dispatch_rqueue(sd_rtnl *rtnl, sd_rtnl_message **message) { - sd_rtnl_message *z = NULL; int r; assert(rtnl); assert(message); - if (rtnl->rqueue_size > 0) { - /* Dispatch a queued message */ - - *message = rtnl->rqueue[0]; - rtnl->rqueue_size --; - memmove(rtnl->rqueue, rtnl->rqueue + 1, sizeof(sd_rtnl_message*) * rtnl->rqueue_size); - - return 1; + if (rtnl->rqueue_size <= 0) { + /* Try to read a new message */ + r = socket_read_message(rtnl); + if (r <= 0) + return r; } - /* Try to read a new message */ - r = socket_read_message(rtnl, &z); - if (r <= 0) - return r; - - *message = z; + /* Dispatch a queued message */ + *message = rtnl->rqueue[0]; + rtnl->rqueue_size --; + memmove(rtnl->rqueue, rtnl->rqueue + 1, sizeof(sd_rtnl_message*) * rtnl->rqueue_size); return 1; } @@ -554,20 +560,20 @@ int sd_rtnl_call_async_cancel(sd_rtnl *nl, uint32_t serial) { return 1; } -int sd_rtnl_call(sd_rtnl *nl, +int sd_rtnl_call(sd_rtnl *rtnl, sd_rtnl_message *message, uint64_t usec, sd_rtnl_message **ret) { usec_t timeout; uint32_t serial; - bool room = false; + unsigned i = 0; int r; - assert_return(nl, -EINVAL); - assert_return(!rtnl_pid_changed(nl), -ECHILD); + assert_return(rtnl, -EINVAL); + assert_return(!rtnl_pid_changed(rtnl), -ECHILD); assert_return(message, -EINVAL); - r = sd_rtnl_send(nl, message, &serial); + r = sd_rtnl_send(rtnl, message, &serial); if (r < 0) return r; @@ -575,53 +581,43 @@ int sd_rtnl_call(sd_rtnl *nl, for (;;) { usec_t left; - _cleanup_rtnl_message_unref_ sd_rtnl_message *incoming = NULL; - - if (!room) { - sd_rtnl_message **q; - - if (nl->rqueue_size >= RTNL_RQUEUE_MAX) - return -ENOBUFS; - /* Make sure there's room for queueing this - * locally, before we read the message */ + while (i < rtnl->rqueue_size) { + sd_rtnl_message *incoming; + uint32_t received_serial; - q = realloc(nl->rqueue, (nl->rqueue_size + 1) * sizeof(sd_rtnl_message*)); - if (!q) - return -ENOMEM; - - nl->rqueue = q; - room = true; - } - - r = socket_read_message(nl, &incoming); - if (r < 0) - return r; - if (incoming) { - uint32_t received_serial = rtnl_message_get_serial(incoming); + incoming = rtnl->rqueue[i]; + received_serial = rtnl_message_get_serial(incoming); if (received_serial == serial) { + /* found a match, remove from rqueue and return it */ + memmove(rtnl->rqueue + i,rtnl->rqueue + i + 1, + sizeof(sd_rtnl_message*) * (rtnl->rqueue_size - i - 1)); + rtnl->rqueue_size--; + r = sd_rtnl_message_get_errno(incoming); - if (r < 0) + if (r < 0) { + sd_rtnl_message_unref(incoming); return r; + } if (ret) { *ret = incoming; - incoming = NULL; - } + } else + sd_rtnl_message_unref(incoming); return 1; } - /* Room was allocated on the queue above */ - nl->rqueue[nl->rqueue_size ++] = incoming; - incoming = NULL; - room = false; - /* Try to read more, right away */ - continue; + i ++; } - if (r != 0) + + r = socket_read_message(rtnl); + if (r < 0) + return r; + if (r > 0) + /* receieved message, so try to process straight away */ continue; if (timeout > 0) { @@ -635,11 +631,11 @@ int sd_rtnl_call(sd_rtnl *nl, } else left = (uint64_t) -1; - r = rtnl_poll(nl, true, left); + r = rtnl_poll(rtnl, true, left); if (r < 0) return r; - r = dispatch_wqueue(nl); + r = dispatch_wqueue(rtnl); if (r < 0) return r; } -- 2.30.2