[systemd-commits] 3 commits - src/libsystemd src/network src/systemd

Tom Gundersen tomegun at kemper.freedesktop.org
Mon Mar 24 09:03:46 PDT 2014


 src/libsystemd/sd-rtnl/rtnl-internal.h |    4 
 src/libsystemd/sd-rtnl/rtnl-message.c  |  411 ++++++++++++++++++---------------
 src/libsystemd/sd-rtnl/rtnl-util.h     |    2 
 src/libsystemd/sd-rtnl/sd-rtnl.c       |   22 +
 src/libsystemd/sd-rtnl/test-rtnl.c     |  162 +++----------
 src/network/networkd-manager.c         |    2 
 src/network/networkd-netdev.c          |   54 +++-
 src/network/networkd.h                 |    2 
 src/systemd/sd-rtnl.h                  |    2 
 9 files changed, 334 insertions(+), 327 deletions(-)

New commits:
commit e634cd409669130f77e552d2a57c1fca08738014
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Mar 24 11:58:22 2014 +0100

    sd-rtnl: message parsing - log when ignoring message attributes

diff --git a/src/libsystemd/sd-rtnl/rtnl-message.c b/src/libsystemd/sd-rtnl/rtnl-message.c
index 330f57f..652dc6e 100644
--- a/src/libsystemd/sd-rtnl/rtnl-message.c
+++ b/src/libsystemd/sd-rtnl/rtnl-message.c
@@ -1040,7 +1040,7 @@ int rtnl_message_parse(sd_rtnl_message *m,
                        int max,
                        struct rtattr *rta,
                        unsigned int rt_len) {
-        int type;
+        unsigned short type;
         size_t *tb;
 
         tb = (size_t *) new0(size_t *, max);
@@ -1052,8 +1052,15 @@ int rtnl_message_parse(sd_rtnl_message *m,
         for (; RTA_OK(rta, rt_len); rta = RTA_NEXT(rta, rt_len)) {
                 type = rta->rta_type;
 
-                if (type <= max)
-                        tb[type] = (uint8_t *) rta - (uint8_t *) m->hdr;
+                if (type > max) {
+                        log_debug("rtnl: message parse - ignore out of range attribute type");
+                        continue;
+                }
+
+                if (tb[type])
+                        log_debug("rtnl: message parse - overwriting repeated attribute");
+
+                tb[type] = (uint8_t *) rta - (uint8_t *) m->hdr;
         }
 
         *rta_offset_tb = tb;

commit d39edfc72f9296078a18014627bf0a2543b60627
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Mar 24 00:07:46 2014 +0100

    networkd: netdev - verify that newlink messages has the expected kind
    
    We match 'newlink' messages with expected netdev's based on their names. Now also
    make sure that the receieved link has the expected kind.

diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
index 46815e0..684b1c7 100644
--- a/src/network/networkd-manager.c
+++ b/src/network/networkd-manager.c
@@ -324,7 +324,7 @@ static int manager_rtnl_process_link(sd_rtnl *rtnl, sd_rtnl_message *message, vo
 
                 r = netdev_get(m, name, &netdev);
                 if (r >= 0) {
-                        r = netdev_set_ifindex(netdev, ifindex);
+                        r = netdev_set_ifindex(netdev, message);
                         if (r < 0)
                                 log_debug("could not set ifindex of netdev '%s' to %d: %s",
                                           name, ifindex, strerror(-r));
diff --git a/src/network/networkd-netdev.c b/src/network/networkd-netdev.c
index 3a670b3..356341a 100644
--- a/src/network/networkd-netdev.c
+++ b/src/network/networkd-netdev.c
@@ -157,7 +157,7 @@ static int netdev_enter_ready(NetDev *netdev) {
 static int netdev_getlink_handler(sd_rtnl *rtnl, sd_rtnl_message *m,
                                 void *userdata) {
         NetDev *netdev = userdata;
-        int r, ifindex;
+        int r;
 
         assert(netdev);
 
@@ -174,19 +174,7 @@ static int netdev_getlink_handler(sd_rtnl *rtnl, sd_rtnl_message *m,
                 return 1;
         }
 
-        r = sd_rtnl_message_link_get_ifindex(m, &ifindex);
-        if (r < 0) {
-                log_struct_netdev(LOG_ERR, netdev,
-                                "MESSAGE=%s: could not get ifindex: %s",
-                                netdev->name, strerror(-r),
-                                "ERRNO=%d", -r,
-                                NULL);
-                return 1;
-        }
-
-        r = netdev_set_ifindex(netdev, ifindex);
-        if (r < 0)
-                log_warning_netdev(netdev, "could not set ifindex to %d", ifindex);
+        netdev_set_ifindex(netdev, m);
 
         return 1;
 }
@@ -392,10 +380,46 @@ int netdev_enslave(NetDev *netdev, Link *link, sd_rtnl_message_handler_t callbac
         return 0;
 }
 
-int netdev_set_ifindex(NetDev *netdev, int ifindex) {
+int netdev_set_ifindex(NetDev *netdev, sd_rtnl_message *message) {
+        const char *kind;
+        char *received_kind;
+        int r, ifindex;
+
         assert(netdev);
         assert(ifindex > 0);
 
+        kind = netdev_kind_to_string(netdev->kind);
+        if (!kind)
+                log_error_netdev(netdev, "Could not get kind");
+
+        r = sd_rtnl_message_enter_container(message, IFLA_LINKINFO);
+        if (r < 0) {
+                log_error_netdev(netdev, "Could not get LINKINFO");
+                return r;
+        }
+
+        r = sd_rtnl_message_read_string(message, IFLA_INFO_KIND, &received_kind);
+        if (r < 0) {
+                log_error_netdev(netdev, "Could not get KIND");
+                return r;
+        }
+
+        if (!streq(kind, received_kind)) {
+                log_error_netdev(netdev, "Received newlink with wrong KIND");
+                netdev_enter_failed(netdev);
+                return r;
+        }
+
+        r = sd_rtnl_message_link_get_ifindex(message, &ifindex);
+        if (r < 0) {
+                log_struct_netdev(LOG_ERR, netdev,
+                                "MESSAGE=%s: could not get ifindex: %s",
+                                netdev->name, strerror(-r),
+                                "ERRNO=%d", -r,
+                                NULL);
+                return r;
+        }
+
         if (netdev->ifindex > 0) {
                 if (netdev->ifindex == ifindex)
                         return 0;
diff --git a/src/network/networkd.h b/src/network/networkd.h
index 239ef1c..8144031 100644
--- a/src/network/networkd.h
+++ b/src/network/networkd.h
@@ -264,7 +264,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(NetDev*, netdev_free);
 #define _cleanup_netdev_free_ _cleanup_(netdev_freep)
 
 int netdev_get(Manager *manager, const char *name, NetDev **ret);
-int netdev_set_ifindex(NetDev *netdev, int ifindex);
+int netdev_set_ifindex(NetDev *netdev, sd_rtnl_message *newlink);
 int netdev_enslave(NetDev *netdev, Link *link, sd_rtnl_message_handler_t cb);
 
 const char *netdev_kind_to_string(NetDevKind d) _const_;

commit 3dd215e056ee9ff23175eca66686ff9b7a566dbf
Author: Tom Gundersen <teg at jklm.no>
Date:   Sun Mar 23 21:45:46 2014 +0100

    sd-rtnl: add sd_rtnl_message_enter_container()
    
    Extend rta_offset_tb into a stack of offset tables, one for each parent of the
    current container, and make sd_rtnl_message_{enter,exit}_container() pop/push
    to this stack.
    
    Also make sd_rtnl_message_rewind() parse the top-level container, and use this
    when reading a message from the socket.
    
    This changes the API by dropping the now redundant sd_rtnl_message_read()
    method.

diff --git a/src/libsystemd/sd-rtnl/rtnl-internal.h b/src/libsystemd/sd-rtnl/rtnl-internal.h
index f011dbe..21a270a 100644
--- a/src/libsystemd/sd-rtnl/rtnl-internal.h
+++ b/src/libsystemd/sd-rtnl/rtnl-internal.h
@@ -94,8 +94,8 @@ struct sd_rtnl_message {
         size_t container_offsets[RTNL_CONTAINER_DEPTH]; /* offset from hdr to each container's start */
         unsigned n_containers; /* number of containers */
         size_t next_rta_offset; /* offset from hdr to next rta */
-        size_t *rta_offset_tb;
-        unsigned short rta_tb_size;
+        size_t *rta_offset_tb[RTNL_CONTAINER_DEPTH];
+        unsigned short rta_tb_size[RTNL_CONTAINER_DEPTH];
         bool sealed:1;
 };
 
diff --git a/src/libsystemd/sd-rtnl/rtnl-message.c b/src/libsystemd/sd-rtnl/rtnl-message.c
index 23c8099..330f57f 100644
--- a/src/libsystemd/sd-rtnl/rtnl-message.c
+++ b/src/libsystemd/sd-rtnl/rtnl-message.c
@@ -24,6 +24,7 @@
 #include <stdbool.h>
 #include <unistd.h>
 #include <linux/veth.h>
+#include <linux/if_bridge.h>
 
 #include "util.h"
 #include "refcnt.h"
@@ -34,8 +35,6 @@
 #include "rtnl-internal.h"
 
 #define GET_CONTAINER(m, i) ((i) < (m)->n_containers ? (struct rtattr*)((uint8_t*)(m)->hdr + (m)->container_offsets[i]) : NULL)
-#define NEXT_RTA(m) ((struct rtattr*)((uint8_t*)(m)->hdr + (m)->next_rta_offset))
-#define UPDATE_RTA(m, new) (m)->next_rta_offset = (uint8_t*)(new) - (uint8_t*)(m)->hdr;
 #define PUSH_CONTAINER(m, new) (m)->container_offsets[(m)->n_containers ++] = (uint8_t*)(new) - (uint8_t*)(m)->hdr;
 
 int message_new(sd_rtnl *rtnl, sd_rtnl_message **ret, size_t initial_size) {
@@ -119,8 +118,6 @@ int sd_rtnl_message_new_route(sd_rtnl *rtnl, sd_rtnl_message **ret,
 
         rtm = NLMSG_DATA((*ret)->hdr);
 
-        UPDATE_RTA(*ret, RTM_RTA(rtm));
-
         rtm->rtm_family = rtm_family;
         rtm->rtm_scope = RT_SCOPE_UNIVERSE;
         rtm->rtm_type = RTN_UNICAST;
@@ -183,8 +180,6 @@ int sd_rtnl_message_new_link(sd_rtnl *rtnl, sd_rtnl_message **ret,
         ifi->ifi_family = AF_UNSPEC;
         ifi->ifi_index = index;
 
-        UPDATE_RTA(*ret, IFLA_RTA(ifi));
-
         return 0;
 }
 
@@ -263,8 +258,6 @@ int sd_rtnl_message_new_addr(sd_rtnl *rtnl, sd_rtnl_message **ret,
         else if (family == AF_INET6)
                 ifa->ifa_prefixlen = 128;
 
-        UPDATE_RTA(*ret, IFA_RTA(ifa));
-
         return 0;
 }
 
@@ -277,9 +270,14 @@ sd_rtnl_message *sd_rtnl_message_ref(sd_rtnl_message *m) {
 
 sd_rtnl_message *sd_rtnl_message_unref(sd_rtnl_message *m) {
         if (m && REFCNT_DEC(m->n_ref) <= 0) {
+                unsigned i;
+
                 sd_rtnl_unref(m->rtnl);
                 free(m->hdr);
-                free(m->rta_offset_tb);
+
+                for (i = 0; i < m->n_containers; i++)
+                        free(m->rta_offset_tb[i]);
+
                 free(m);
         }
 
@@ -762,63 +760,19 @@ int sd_rtnl_message_close_container(sd_rtnl_message *m) {
         return 0;
 }
 
-int sd_rtnl_message_read(sd_rtnl_message *m, unsigned short *type, void **data) {
-        size_t remaining_size;
-        uint16_t rtm_type;
-        int r;
-
-        assert_return(m, -EINVAL);
-        assert_return(m->sealed, -EPERM);
-        assert_return(m->next_rta_offset, -EINVAL);
-        assert_return(type, -EINVAL);
-        assert_return(data, -EINVAL);
-
-        /* only read until the end of the current container */
-        if (m->n_containers)
-                remaining_size = GET_CONTAINER(m, m->n_containers - 1)->rta_len -
-                                 (m->next_rta_offset -
-                                  m->container_offsets[m->n_containers - 1]);
-        else
-                remaining_size = m->hdr->nlmsg_len - m->next_rta_offset;
-
-        if (!RTA_OK(NEXT_RTA(m), remaining_size))
-                return 0;
-
-        /* if we read a container, return its type, but do not enter it*/
-        r = sd_rtnl_message_get_type(m, &rtm_type);
-        if (r < 0)
-                return r;
-
-        *type = NEXT_RTA(m)->rta_type;
-
-        if (rtnl_message_type_is_link(rtm_type) &&
-            ((m->n_containers == 0 &&
-              NEXT_RTA(m)->rta_type == IFLA_LINKINFO) ||
-             (m->n_containers == 1 &&
-              GET_CONTAINER(m, 0)->rta_type == IFLA_LINKINFO &&
-              NEXT_RTA(m)->rta_type == IFLA_INFO_DATA)))
-                *data = NULL;
-        else
-                *data = RTA_DATA(NEXT_RTA(m));
-
-        UPDATE_RTA(m, RTA_NEXT(NEXT_RTA(m), remaining_size));
-
-        return 1;
-}
-
 int rtnl_message_read_internal(sd_rtnl_message *m, unsigned short type, void **data) {
         struct rtattr *rta;
 
         assert_return(m, -EINVAL);
         assert_return(m->sealed, -EPERM);
         assert_return(data, -EINVAL);
-        assert_return(m->rta_offset_tb, -EINVAL);
-        assert_return(type < m->rta_tb_size, -EINVAL);
+        assert_return(m->rta_offset_tb[m->n_containers], -EINVAL);
+        assert_return(type < m->rta_tb_size[m->n_containers], -EINVAL);
 
-        if(!m->rta_offset_tb[type])
+        if(!m->rta_offset_tb[m->n_containers][type])
                 return -ENODATA;
 
-        rta = (struct rtattr*)((uint8_t *) m->hdr + m->rta_offset_tb[type]);
+        rta = (struct rtattr*)((uint8_t *) m->hdr + m->rta_offset_tb[m->n_containers][type]);
 
         *data = RTA_DATA(rta);
 
@@ -934,19 +888,106 @@ int sd_rtnl_message_read_in6_addr(sd_rtnl_message *m, unsigned short type, struc
         assert_return(data, -EINVAL);
 
         r = rtnl_message_read_internal(m, type, &attr_data);
-        if(r < 0)
+        if (r < 0)
                 return r;
+        else if ((size_t)r < sizeof(struct in6_addr))
+                return -EIO;
 
         memcpy(data, attr_data, sizeof(struct in6_addr));
 
         return 0;
 }
 
+int sd_rtnl_message_enter_container(sd_rtnl_message *m, unsigned short type) {
+        uint16_t rtm_type;
+        unsigned short parent_type;
+        void *container;
+        size_t container_length;
+        int max, r;
+
+        assert_return(m, -EINVAL);
+        assert_return(m->n_containers < RTNL_CONTAINER_DEPTH, -EINVAL);
+
+        r = rtnl_message_read_internal(m, type, &container);
+        if (r < 0)
+                return r;
+        else
+                container_length = r;
+
+        r = sd_rtnl_message_get_type(m, &rtm_type);
+        if (r < 0)
+                return r;
+
+        if (rtnl_message_type_is_link(rtm_type)) {
+                switch (m->n_containers) {
+                        case 0:
+                                switch (type) {
+                                        case IFLA_LINKINFO:
+                                                max = IFLA_INFO_MAX;
+                                                break;
+                                        default:
+                                                return -ENOTSUP;
+                                }
+                                break;
+                        case 1:
+                                parent_type = GET_CONTAINER(m, 0)->rta_type;
+                                switch (parent_type) {
+                                        case IFLA_LINKINFO:
+                                                switch (type) {
+                                                        case IFLA_INFO_DATA: {
+                                                                char *kind;
+
+                                                                r = sd_rtnl_message_read_string(m, IFLA_INFO_KIND, &kind);
+                                                                if (r < 0)
+                                                                        return r;
+
+                                                                if (streq(kind, "vlan")) {
+                                                                        max = IFLA_VLAN_MAX;
+                                                                } else if (streq(kind, "bridge")) {
+                                                                        max = IFLA_BRIDGE_MAX;
+                                                                } else if (streq(kind, "veth")) {
+                                                                        max = VETH_INFO_MAX;
+                                                                        container = IFLA_RTA(container);
+                                                                } else
+                                                                        return -ENOTSUP;
+
+                                                                break;
+                                                        }
+                                                        default:
+                                                                return -ENOTSUP;
+                                                }
+                                                break;
+                                        default:
+                                                return -ENOTSUP;
+                                }
+                                break;
+                        default:
+                                return -ENOTSUP;
+                }
+        }
+
+        r = rtnl_message_parse(m,
+                               &m->rta_offset_tb[m->n_containers + 1],
+                               &m->rta_tb_size[m->n_containers + 1],
+                               max,
+                               container,
+                               container_length);
+        if (r < 0)
+                return r;
+
+        m->n_containers ++;
+
+        return 0;
+}
+
 int sd_rtnl_message_exit_container(sd_rtnl_message *m) {
         assert_return(m, -EINVAL);
         assert_return(m->sealed, -EINVAL);
         assert_return(m->n_containers > 0, -EINVAL);
 
+        free(m->rta_offset_tb[m->n_containers]);
+        m->rta_offset_tb[m->n_containers] = NULL;
+
         m->n_containers --;
 
         return 0;
@@ -973,27 +1014,6 @@ int sd_rtnl_message_get_errno(sd_rtnl_message *m) {
         return err->error;
 }
 
-int rtnl_message_seal(sd_rtnl *nl, sd_rtnl_message *m) {
-        int r;
-
-        assert(m);
-        assert(m->hdr);
-
-        if (m->sealed)
-                return -EPERM;
-
-        if (nl)
-                m->hdr->nlmsg_seq = nl->serial++;
-
-        m->sealed = true;
-
-        r = sd_rtnl_message_rewind(m);
-        if (r < 0)
-                return r;
-
-        return 0;
-}
-
 static int message_receive_need(sd_rtnl *rtnl, size_t *need) {
         assert(rtnl);
         assert(need);
@@ -1069,15 +1089,15 @@ int socket_write_message(sd_rtnl *nl, sd_rtnl_message *m) {
  * On failure, a negative error code is returned.
  */
 int socket_read_message(sd_rtnl *nl, sd_rtnl_message **ret) {
-        sd_rtnl_message *m;
+        _cleanup_rtnl_message_unref_ sd_rtnl_message *m = NULL;
+        struct nlmsghdr *new_hdr;
         union {
                 struct sockaddr sa;
                 struct sockaddr_nl nl;
         } addr;
         socklen_t addr_len;
+        size_t need, len;
         int r;
-        ssize_t k;
-        size_t need;
 
         assert(nl);
         assert(ret);
@@ -1090,149 +1110,165 @@ int socket_read_message(sd_rtnl *nl, sd_rtnl_message **ret) {
         if (r < 0)
                 return r;
 
-        /* don't allow sealing/appending to received messages */
-        m->sealed = true;
-
         addr_len = sizeof(addr);
 
-        k = recvfrom(nl->fd, m->hdr, need,
+        r = recvfrom(nl->fd, m->hdr, need,
                         0, &addr.sa, &addr_len);
-        if (k < 0)
-                k = (errno == EAGAIN) ? 0 : -errno; /* no data */
-        else if (k == 0)
-                k = -ECONNRESET; /* connection was closed by the kernel */
+        if (r < 0)
+                return (errno == EAGAIN) ? 0 : -errno; /* no data */
+        else if (r == 0)
+                return -ECONNRESET; /* connection was closed by the kernel */
         else if (addr_len != sizeof(addr.nl) ||
                         addr.nl.nl_family != AF_NETLINK)
-                k = -EIO; /* not a netlink message */
+                return -EIO; /* not a netlink message */
         else if (addr.nl.nl_pid != 0)
-                k = 0; /* not from the kernel */
-        else if ((size_t) k < sizeof(struct nlmsghdr) ||
-                        (size_t) k < m->hdr->nlmsg_len)
-                k = -EIO; /* too small (we do accept too big though) */
+                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)
-                k = 0; /* not broadcast and not for us */
-
-        if (k > 0)
-                switch (m->hdr->nlmsg_type) {
-                        /* check that the size matches the message type */
-                        case NLMSG_ERROR:
-                                if (m->hdr->nlmsg_len < NLMSG_LENGTH(sizeof(struct nlmsgerr)))
-                                        k = -EIO;
-                                break;
-                        case RTM_NEWLINK:
-                        case RTM_SETLINK:
-                        case RTM_DELLINK:
-                        case RTM_GETLINK:
-                                if (m->hdr->nlmsg_len < NLMSG_LENGTH(sizeof(struct ifinfomsg)))
-                                        k = -EIO;
-                                else {
-                                        struct ifinfomsg *ifi;
-
-                                        ifi = NLMSG_DATA(m->hdr);
-                                        UPDATE_RTA(m, IFLA_RTA(ifi));
-
-                                        r = rtnl_message_parse(m,
-                                                               &m->rta_offset_tb,
-                                                               &m->rta_tb_size,
-                                                               IFLA_MAX,
-                                                               IFLA_RTA(ifi),
-                                                               IFLA_PAYLOAD(m->hdr));
-
-                                }
-                                break;
-                        case RTM_NEWADDR:
-                        case RTM_DELADDR:
-                        case RTM_GETADDR:
-                                if (m->hdr->nlmsg_len < NLMSG_LENGTH(sizeof(struct ifaddrmsg)))
-                                        k = -EIO;
-                                else {
-                                        struct ifaddrmsg *ifa;
+                return 0; /* not broadcast and not for us */
+        else
+                len = (size_t) r;
+
+        /* check that the size matches the message type */
+        switch (m->hdr->nlmsg_type) {
+
+        case NLMSG_ERROR:
+                if (len < NLMSG_LENGTH(sizeof(struct nlmsgerr)))
+                        return -EIO;
+                break;
+
+        case RTM_NEWLINK:
+        case RTM_SETLINK:
+        case RTM_DELLINK:
+        case RTM_GETLINK:
+                if (len < NLMSG_LENGTH(sizeof(struct ifinfomsg)))
+                        return -EIO;
+                break;
+
+        case RTM_NEWADDR:
+        case RTM_DELADDR:
+        case RTM_GETADDR:
+                if (len < NLMSG_LENGTH(sizeof(struct ifaddrmsg)))
+                        return -EIO;
+                break;
+        case RTM_NEWROUTE:
+        case RTM_DELROUTE:
+        case RTM_GETROUTE:
+                if (len < NLMSG_LENGTH(sizeof(struct rtmsg)))
+                        return -EIO;
+                break;
+        case NLMSG_NOOP:
+                return 0;
+        default:
+                log_debug("sd-rtnl: ignored message with unknown type");
+                return 0;
+        }
 
-                                        ifa = NLMSG_DATA(m->hdr);
-                                        UPDATE_RTA(m, IFA_RTA(ifa));
-
-                                        r = rtnl_message_parse(m,
-                                                               &m->rta_offset_tb,
-                                                               &m->rta_tb_size,
-                                                               IFA_MAX,
-                                                               IFA_RTA(ifa),
-                                                               IFA_PAYLOAD(m->hdr));
-                                }
-                                break;
-                        case RTM_NEWROUTE:
-                        case RTM_DELROUTE:
-                        case RTM_GETROUTE:
-                                if (m->hdr->nlmsg_len < NLMSG_LENGTH(sizeof(struct rtmsg)))
-                                        k = -EIO;
-                                else {
-                                        struct rtmsg *rtm;
+        /* 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;
 
-                                        rtm = NLMSG_DATA(m->hdr);
-                                        UPDATE_RTA(m, RTM_RTA(rtm));
-
-                                        r = rtnl_message_parse(m,
-                                                               &m->rta_offset_tb,
-                                                               &m->rta_tb_size,
-                                                               RTA_MAX,
-                                                               RTM_RTA(rtm),
-                                                               RTM_PAYLOAD(m->hdr));
-                                }
-                                break;
-                        case NLMSG_NOOP:
-                                k = 0;
-                                break;
-                        default:
-                                k = 0; /* ignoring message of unknown type */
-                }
+        /* seal and parse the top-level message */
+        r = sd_rtnl_message_rewind(m);
+        if (r < 0)
+                return r;
 
-        if (k <= 0)
-                sd_rtnl_message_unref(m);
-        else {
-                /* we probably allocated way too much memory, give it back */
-                m->hdr = realloc(m->hdr, m->hdr->nlmsg_len);
-                *ret = m;
-        }
+        *ret = m;
+        m = NULL;
 
-        return k;
+        return len;
 }
 
 int sd_rtnl_message_rewind(sd_rtnl_message *m) {
         struct ifinfomsg *ifi;
         struct ifaddrmsg *ifa;
         struct rtmsg *rtm;
+        unsigned i;
+        int r;
 
         assert_return(m, -EINVAL);
-        assert_return(m->sealed, -EPERM);
         assert_return(m->hdr, -EINVAL);
 
+        /* don't allow appending to message once parsed */
+        if (!m->sealed)
+                rtnl_message_seal(m);
+
+        for (i = 1; i <= m->n_containers; i++) {
+                free(m->rta_offset_tb[i]);
+                m->rta_offset_tb[i] = NULL;
+                m->rta_tb_size[i] = 0;
+        }
+
+        m->n_containers = 0;
+
+        if (m->rta_offset_tb[0]) {
+                /* top-level attributes have already been parsed */
+                return 0;
+        }
+
+        /* parse top-level attributes */
         switch(m->hdr->nlmsg_type) {
+                case NLMSG_NOOP:
+                case NLMSG_ERROR:
+                        break;
                 case RTM_NEWLINK:
                 case RTM_SETLINK:
                 case RTM_GETLINK:
                 case RTM_DELLINK:
                         ifi = NLMSG_DATA(m->hdr);
-                        UPDATE_RTA(m, IFLA_RTA(ifi));
+
+                        r = rtnl_message_parse(m,
+                                               &m->rta_offset_tb[0],
+                                               &m->rta_tb_size[0],
+                                               IFLA_MAX,
+                                               IFLA_RTA(ifi),
+                                               IFLA_PAYLOAD(m->hdr));
+                        if (r < 0)
+                                return r;
 
                         break;
                 case RTM_NEWADDR:
                 case RTM_GETADDR:
                 case RTM_DELADDR:
                         ifa = NLMSG_DATA(m->hdr);
-                        UPDATE_RTA(m, IFA_RTA(ifa));
+
+                        r = rtnl_message_parse(m,
+                                               &m->rta_offset_tb[0],
+                                               &m->rta_tb_size[0],
+                                               IFA_MAX,
+                                               IFA_RTA(ifa),
+                                               IFA_PAYLOAD(m->hdr));
+                        if (r < 0)
+                                return r;
 
                         break;
                 case RTM_NEWROUTE:
                 case RTM_GETROUTE:
                 case RTM_DELROUTE:
                         rtm = NLMSG_DATA(m->hdr);
-                        UPDATE_RTA(m, RTM_RTA(rtm));
+
+                        r = rtnl_message_parse(m,
+                                               &m->rta_offset_tb[0],
+                                               &m->rta_tb_size[0],
+                                               RTA_MAX,
+                                               RTM_RTA(rtm),
+                                               RTM_PAYLOAD(m->hdr));
 
                         break;
                 default:
                         return -ENOTSUP;
         }
 
-        m->n_containers = 0;
-
         return 0;
 }
+
+void rtnl_message_seal(sd_rtnl_message *m) {
+        assert(m);
+        assert(!m->sealed);
+
+        m->sealed = true;
+}
diff --git a/src/libsystemd/sd-rtnl/rtnl-util.h b/src/libsystemd/sd-rtnl/rtnl-util.h
index 7fe9222..06d5699 100644
--- a/src/libsystemd/sd-rtnl/rtnl-util.h
+++ b/src/libsystemd/sd-rtnl/rtnl-util.h
@@ -28,7 +28,7 @@
 
 int rtnl_message_new_synthetic_error(int error, uint32_t serial, sd_rtnl_message **ret);
 uint32_t rtnl_message_get_serial(sd_rtnl_message *m);
-int rtnl_message_seal(sd_rtnl *nl, sd_rtnl_message *m);
+void rtnl_message_seal(sd_rtnl_message *m);
 
 bool rtnl_message_type_is_link(uint16_t type);
 bool rtnl_message_type_is_addr(uint16_t type);
diff --git a/src/libsystemd/sd-rtnl/sd-rtnl.c b/src/libsystemd/sd-rtnl/sd-rtnl.c
index a2760d0..551e95b 100644
--- a/src/libsystemd/sd-rtnl/sd-rtnl.c
+++ b/src/libsystemd/sd-rtnl/sd-rtnl.c
@@ -190,6 +190,19 @@ sd_rtnl *sd_rtnl_unref(sd_rtnl *rtnl) {
         return NULL;
 }
 
+static void rtnl_seal_message(sd_rtnl *rtnl, sd_rtnl_message *m) {
+        assert(rtnl);
+        assert(!rtnl_pid_changed(rtnl));
+        assert(m);
+        assert(m->hdr);
+
+        m->hdr->nlmsg_seq = rtnl->serial++;
+
+        rtnl_message_seal(m);
+
+        return;
+}
+
 int sd_rtnl_send(sd_rtnl *nl,
                  sd_rtnl_message *message,
                  uint32_t *serial) {
@@ -198,10 +211,9 @@ int sd_rtnl_send(sd_rtnl *nl,
         assert_return(nl, -EINVAL);
         assert_return(!rtnl_pid_changed(nl), -ECHILD);
         assert_return(message, -EINVAL);
+        assert_return(!message->sealed, -EPERM);
 
-        r = rtnl_message_seal(nl, message);
-        if (r < 0)
-                return r;
+        rtnl_seal_message(nl, message);
 
         if (nl->wqueue_size <= 0) {
                 /* send directly */
@@ -254,10 +266,8 @@ static int dispatch_rqueue(sd_rtnl *rtnl, sd_rtnl_message **message) {
 
         /* Try to read a new message */
         r = socket_read_message(rtnl, &z);
-        if (r < 0)
+        if (r <= 0)
                 return r;
-        if (r == 0)
-                return 0;
 
         *message = z;
 
diff --git a/src/libsystemd/sd-rtnl/test-rtnl.c b/src/libsystemd/sd-rtnl/test-rtnl.c
index 6d9159c..7e9f607 100644
--- a/src/libsystemd/sd-rtnl/test-rtnl.c
+++ b/src/libsystemd/sd-rtnl/test-rtnl.c
@@ -32,10 +32,10 @@
 
 static void test_link_configure(sd_rtnl *rtnl, int ifindex) {
         _cleanup_rtnl_message_unref_ sd_rtnl_message *message;
-        uint16_t type;
         const char *mac = "98:fe:94:3f:c6:18", *name = "test";
-        unsigned int mtu = 1450;
-        void *data;
+        unsigned int mtu = 1450, mtu_out;
+        char *name_out;
+        struct ether_addr mac_out;
 
         /* we'd really like to test NEWLINK, but let's not mess with the running kernel */
         assert_se(sd_rtnl_message_new_link(rtnl, &message, RTM_GETLINK, ifindex) >= 0);
@@ -44,28 +44,23 @@ static void test_link_configure(sd_rtnl *rtnl, int ifindex) {
         assert_se(sd_rtnl_message_append_u32(message, IFLA_MTU, mtu) >= 0);
 
         assert_se(sd_rtnl_call(rtnl, message, 0, NULL) == 1);
+        assert_se(sd_rtnl_message_rewind(message) >= 0);
 
-        assert_se(sd_rtnl_message_read(message, &type, &data) > 0);
-        assert_se(type == IFLA_IFNAME);
-        assert_se(streq(name, (char *) data));
+        assert_se(sd_rtnl_message_read_string(message, IFLA_IFNAME, &name_out) >= 0);
+        assert_se(streq(name, name_out));
 
-        assert_se(sd_rtnl_message_read(message, &type, &data) > 0);
-        assert_se(type == IFLA_ADDRESS);
-        assert_se(streq(mac, ether_ntoa(data)));
+        assert_se(sd_rtnl_message_read_ether_addr(message, IFLA_ADDRESS, &mac_out) >= 0);
+        assert_se(streq(mac, ether_ntoa(&mac_out)));
 
-        assert_se(sd_rtnl_message_read(message, &type, &data) > 0);
-        assert_se(type == IFLA_MTU);
-        assert_se(mtu == *(unsigned int *) data);
+        assert_se(sd_rtnl_message_read_u32(message, IFLA_MTU, &mtu_out) >= 0);
+        assert_se(mtu == mtu_out);
 }
 
 static void test_link_get(sd_rtnl *rtnl, int ifindex) {
         sd_rtnl_message *m;
         sd_rtnl_message *r;
         unsigned int mtu = 1500;
-        unsigned int *mtu_reply;
-        void *data;
         char *str_data;
-        uint16_t type;
         uint8_t u8_data;
         uint32_t u32_data;
         struct ether_addr eth_data;
@@ -87,47 +82,6 @@ static void test_link_get(sd_rtnl *rtnl, int ifindex) {
 
         assert_se(sd_rtnl_call(rtnl, m, -1, &r) == 1);
 
-        /* u8 read back */
-        assert_se(sd_rtnl_message_read(m, &type, &data) == 1);
-        assert_se(type == IFLA_CARRIER);
-
-        assert_se(sd_rtnl_message_read(m, &type, &data) == 1);
-        assert_se(type == IFLA_OPERSTATE);
-
-        assert_se(sd_rtnl_message_read(m, &type, &data) == 1);
-        assert_se(type == IFLA_LINKMODE);
-
-        /* u32 read back */
-        assert_se(sd_rtnl_message_read(m, &type, (void **) &mtu_reply) == 1);
-        assert_se(type == IFLA_MTU);
-        assert_se(*mtu_reply == mtu);
-
-        assert_se(sd_rtnl_message_read(m, &type, &data) == 1);
-        assert_se(type == IFLA_GROUP);
-
-        assert_se(sd_rtnl_message_read(m, &type, &data) == 1);
-        assert_se(type == IFLA_TXQLEN);
-
-        assert_se(sd_rtnl_message_read(m, &type, &data) == 1);
-        assert_se(type == IFLA_NUM_TX_QUEUES);
-
-        assert_se(sd_rtnl_message_read(m, &type, &data) == 1);
-        assert_se(type == IFLA_NUM_RX_QUEUES);
-
-        while (sd_rtnl_message_read(r, &type, &data) > 0) {
-                switch (type) {
-//                        case IFLA_MTU:
-//                                assert_se(*(unsigned int *) data == 65536);
-//                                break;
-//                        case IFLA_QDISC:
-//                                assert_se(streq((char *) data, "noqueue"));
-//                                break;
-                        case IFLA_IFNAME:
-                                assert_se(streq((char *) data, "lo"));
-                                break;
-                }
-        }
-
         assert_se(sd_rtnl_message_read_string(r, IFLA_IFNAME, &str_data) == 0);
 
         assert_se(sd_rtnl_message_read_u8(r, IFLA_CARRIER, &u8_data) == 0);
@@ -171,13 +125,10 @@ static void test_address_get(sd_rtnl *rtnl, int ifindex) {
 
 static void test_route(void) {
         _cleanup_rtnl_message_unref_ sd_rtnl_message *req;
-        struct in_addr addr;
-        uint32_t index = 2;
-        uint16_t type;
-        void *data;
-        uint32_t u32_data;
-        int r;
         struct rtmsg *rtm;
+        struct in_addr addr, addr_data;
+        uint32_t index = 2, u32_data;
+        int r;
 
         r = sd_rtnl_message_new_route(NULL, &req, RTM_NEWROUTE, AF_INET);
         if (r < 0) {
@@ -199,15 +150,13 @@ static void test_route(void) {
                 return;
         }
 
-        assert_se(rtnl_message_seal(NULL, req) >= 0);
+        assert_se(sd_rtnl_message_rewind(req) >= 0);
 
-        assert_se(sd_rtnl_message_read(req, &type, &data) > 0);
-        assert_se(type == RTA_GATEWAY);
-        assert_se(((struct in_addr *)data)->s_addr == addr.s_addr);
+        assert_se(sd_rtnl_message_read_in_addr(req, RTA_GATEWAY, &addr_data) >= 0);
+        assert_se(addr_data.s_addr == addr.s_addr);
 
-        assert_se(sd_rtnl_message_read(req, &type, &data) > 0);
-        assert_se(type == RTA_OIF);
-        assert_se(*(uint32_t *) data == index);
+        assert_se(sd_rtnl_message_read_u32(req, RTA_OIF, &u32_data) >= 0);
+        assert_se(u32_data == index);
 
         rtm = NLMSG_DATA(req->hdr);
         r = rtnl_message_parse(req,
@@ -234,9 +183,7 @@ static void test_multiple(void) {
 }
 
 static int link_handler(sd_rtnl *rtnl, sd_rtnl_message *m, void *userdata) {
-        void *data;
-        uint16_t type;
-        char *ifname = userdata;
+        char *ifname = userdata, *data;
 
         assert_se(rtnl);
         assert_se(m);
@@ -244,19 +191,8 @@ static int link_handler(sd_rtnl *rtnl, sd_rtnl_message *m, void *userdata) {
         log_info("got link info about %s", ifname);
         free(ifname);
 
-        while (sd_rtnl_message_read(m, &type, &data) > 0) {
-                switch (type) {
-//                        case IFLA_MTU:
-//                                assert_se(*(unsigned int *) data == 65536);
-//                                break;
-//                        case IFLA_QDISC:
-//                                assert_se(streq((char *) data, "noqueue"));
-//                                break;
-                        case IFLA_IFNAME:
-                                assert_se(streq((char *) data, "lo"));
-                                break;
-                }
-        }
+        assert_se(sd_rtnl_message_read_string(m, IFLA_IFNAME, &data) >= 0);
+        assert_se(streq(data, "lo"));
 
         return 1;
 }
@@ -288,12 +224,17 @@ static void test_event_loop(int ifindex) {
 
 static int pipe_handler(sd_rtnl *rtnl, sd_rtnl_message *m, void *userdata) {
         int *counter = userdata;
+        int r;
 
         (*counter) --;
 
-        log_info("got reply, %d left in pipe", *counter);
+        r = sd_rtnl_message_get_errno(m);
+
+        log_info("%d left in pipe. got reply: %s", *counter, strerror(-r));
+
+        assert_se(r >= 0);
 
-        return sd_rtnl_message_get_errno(m);
+        return 1;
 }
 
 static void test_async(int ifindex) {
@@ -343,48 +284,38 @@ static void test_pipe(int ifindex) {
 
 static void test_container(void) {
         _cleanup_rtnl_message_unref_ sd_rtnl_message *m = NULL;
-        uint16_t type;
+        struct ifinfomsg *ifi;
+        uint16_t u16_data;
         uint32_t u32_data;
-        void *data;
+        char *string_data;
         int r;
-        struct ifinfomsg *ifi;
 
         assert_se(sd_rtnl_message_new_link(NULL, &m, RTM_NEWLINK, 0) >= 0);
 
         assert_se(sd_rtnl_message_open_container(m, IFLA_LINKINFO) >= 0);
         assert_se(sd_rtnl_message_open_container(m, IFLA_LINKINFO) == -ENOTSUP);
-        assert_se(sd_rtnl_message_append_string(m, IFLA_INFO_KIND, "kind") >= 0);
+        assert_se(sd_rtnl_message_append_string(m, IFLA_INFO_KIND, "vlan") >= 0);
         assert_se(sd_rtnl_message_open_container(m, IFLA_INFO_DATA) >= 0);
         assert_se(sd_rtnl_message_open_container(m, IFLA_INFO_DATA) == -ENOTSUP);
         assert_se(sd_rtnl_message_append_u16(m, IFLA_VLAN_ID, 100) >= 0);
         assert_se(sd_rtnl_message_close_container(m) >= 0);
-        assert_se(sd_rtnl_message_append_string(m, IFLA_INFO_KIND, "kind") >= 0);
+        assert_se(sd_rtnl_message_append_string(m, IFLA_INFO_KIND, "vlan") >= 0);
         assert_se(sd_rtnl_message_close_container(m) >= 0);
         assert_se(sd_rtnl_message_close_container(m) == -EINVAL);
 
-        assert_se(rtnl_message_seal(NULL, m) >= 0);
-
-        assert_se(sd_rtnl_message_read(m, &type, &data) >= 0);
-        assert_se(type == IFLA_LINKINFO);
-        assert_se(data == NULL);
-/*
-        assert_se(sd_rtnl_message_read(m, &type, &data) >= 0);
-        assert_se(type == IFLA_INFO_KIND);
-        assert_se(streq("kind", (char *)data));
-        assert_se(sd_rtnl_message_read(m, &type, &data) >= 0);
-        assert_se(type == IFLA_INFO_DATA);
-        assert_se(data == NULL);
-        assert_se(sd_rtnl_message_read(m, &type, &data) >= 0);
-        assert_se(type == IFLA_VLAN_ID);
-        assert_se(*(uint16_t *)data == 100);
-        assert_se(sd_rtnl_message_read(m, &type, &data) == 0);
+        assert_se(sd_rtnl_message_rewind(m) >= 0);
+
+        assert_se(sd_rtnl_message_enter_container(m, IFLA_LINKINFO) >= 0);
+        assert_se(sd_rtnl_message_read_string(m, IFLA_INFO_KIND, &string_data) >= 0);
+        assert_se(streq("vlan", string_data));
+
+        assert_se(sd_rtnl_message_enter_container(m, IFLA_INFO_DATA) >= 0);
+        assert_se(sd_rtnl_message_read_u16(m, IFLA_VLAN_ID, &u16_data) >= 0);
         assert_se(sd_rtnl_message_exit_container(m) >= 0);
-        assert_se(sd_rtnl_message_read(m, &type, &data) >= 0);
-        assert_se(type == IFLA_INFO_KIND);
-        assert_se(streq("kind", (char *)data));
-        assert_se(sd_rtnl_message_read(m, &type, &data) == 0);
+
+        assert_se(sd_rtnl_message_read_string(m, IFLA_INFO_KIND, &string_data) >= 0);
+        assert_se(streq("vlan", string_data));
         assert_se(sd_rtnl_message_exit_container(m) >= 0);
-*/
 
         ifi = NLMSG_DATA(m->hdr);
         r = rtnl_message_parse(m,
@@ -420,7 +351,7 @@ int main(void) {
         sd_rtnl *rtnl;
         sd_rtnl_message *m;
         sd_rtnl_message *r;
-        void *data;
+        char *string_data;
         int if_loopback;
         uint16_t type;
 
@@ -452,13 +383,12 @@ int main(void) {
         assert_se(sd_rtnl_message_get_type(m, &type) >= 0);
         assert_se(type == RTM_GETLINK);
 
-        assert_se(sd_rtnl_message_read(m, &type, &data) == -EPERM);
+        assert_se(sd_rtnl_message_read_string(m, IFLA_IFNAME, &string_data) == -EPERM);
 
         assert_se(sd_rtnl_call(rtnl, m, 0, &r) == 1);
         assert_se(sd_rtnl_message_get_type(r, &type) >= 0);
         assert_se(type == RTM_NEWLINK);
 
-        assert_se(sd_rtnl_message_read(m, &type, &data) == 0);
         assert_se((r = sd_rtnl_message_unref(r)) == NULL);
 
         assert_se(sd_rtnl_call(rtnl, m, -1, &r) == -EPERM);
diff --git a/src/systemd/sd-rtnl.h b/src/systemd/sd-rtnl.h
index 0a24873..54e5142 100644
--- a/src/systemd/sd-rtnl.h
+++ b/src/systemd/sd-rtnl.h
@@ -103,7 +103,6 @@ int sd_rtnl_message_append_ether_addr(sd_rtnl_message *m, unsigned short type, c
 int sd_rtnl_message_open_container(sd_rtnl_message *m, unsigned short type);
 int sd_rtnl_message_close_container(sd_rtnl_message *m);
 
-int sd_rtnl_message_read(sd_rtnl_message *m, unsigned short *type, void **data);
 int sd_rtnl_message_read_string(sd_rtnl_message *m, unsigned short type, char **data);
 int sd_rtnl_message_read_u8(sd_rtnl_message *m, unsigned short type, uint8_t *data);
 int sd_rtnl_message_read_u16(sd_rtnl_message *m, unsigned short type, uint16_t *data);
@@ -111,6 +110,7 @@ int sd_rtnl_message_read_u32(sd_rtnl_message *m, unsigned short type, uint32_t *
 int sd_rtnl_message_read_ether_addr(sd_rtnl_message *m, unsigned short type, struct ether_addr *data);
 int sd_rtnl_message_read_in_addr(sd_rtnl_message *m, unsigned short type, struct in_addr *data);
 int sd_rtnl_message_read_in6_addr(sd_rtnl_message *m, unsigned short type, struct in6_addr *data);
+int sd_rtnl_message_enter_container(sd_rtnl_message *m, unsigned short type);
 int sd_rtnl_message_exit_container(sd_rtnl_message *m);
 
 int sd_rtnl_message_rewind(sd_rtnl_message *m);



More information about the systemd-commits mailing list