[systemd-devel] [PATCH 1/1] sd-rtnl: Introduce container parsing

Tom Gundersen teg at jklm.no
Mon Mar 24 09:28:36 PDT 2014


On Sat, Mar 22, 2014 at 5:13 PM, Susant Sahani <susant at redhat.com> wrote:
> Introducing generic container parsing . Now  supported for type
> FLA_INFO_KIND and IFLA_VLAN_ID which can be extended to other
> container parsing which is based on table based look up.
> ---
>  src/libsystemd/sd-rtnl/rtnl-internal.h | 20 +++++++++
>  src/libsystemd/sd-rtnl/rtnl-message.c  | 79 +++++++++++++++++++++++++++++++---
>  src/libsystemd/sd-rtnl/rtnl-util.c     | 31 +++++++++++++
>  src/libsystemd/sd-rtnl/rtnl-util.h     |  2 +
>  src/libsystemd/sd-rtnl/test-rtnl.c     | 13 +++++-
>  5 files changed, 138 insertions(+), 7 deletions(-)
>
> diff --git a/src/libsystemd/sd-rtnl/rtnl-internal.h b/src/libsystemd/sd-rtnl/rtnl-internal.h
> index f011dbe..eb30682 100644
> --- a/src/libsystemd/sd-rtnl/rtnl-internal.h
> +++ b/src/libsystemd/sd-rtnl/rtnl-internal.h
> @@ -85,6 +85,15 @@ struct sd_rtnl {
>          sd_event *event;
>  };
>
> +struct rtnl_container {
> +        unsigned short container_type;
> +
> +        size_t *rta_offset_tb;
> +        unsigned short rta_tb_size;
> +
> +        LIST_FIELDS(struct rtnl_container, container);
> +};
> +
>  struct sd_rtnl_message {
>          RefCount n_ref;
>
> @@ -96,6 +105,10 @@ struct sd_rtnl_message {
>          size_t next_rta_offset; /* offset from hdr to next rta */
>          size_t *rta_offset_tb;
>          unsigned short rta_tb_size;
> +        struct rtnl_container *container_list[RTNL_CONTAINER_DEPTH];
> +
> +        LIST_HEAD(struct rtnl_container, containers);
> +
>          bool sealed:1;
>  };
>
> @@ -112,6 +125,13 @@ int rtnl_message_parse(sd_rtnl_message *m,
>                         struct rtattr *rta,
>                         unsigned int rt_len);
>
> +int rtnl_container_new(struct rtnl_container **ret, uint16_t container_type);
> +int rtnl_message_parse_container(sd_rtnl_message *m,
> +                                 uint8_t type,
> +                                 uint8_t tb_size,
> +                                 struct rtattr *rta,
> +                                 unsigned int rt_len);
> +
>  /* Make sure callbacks don't destroy the rtnl connection */
>  #define RTNL_DONT_DESTROY(rtnl) \
>          _cleanup_rtnl_unref_ _unused_ sd_rtnl *_dont_destroy_##rtnl = sd_rtnl_ref(rtnl)
> diff --git a/src/libsystemd/sd-rtnl/rtnl-message.c b/src/libsystemd/sd-rtnl/rtnl-message.c
> index e243c7b..c1ade55 100644
> --- a/src/libsystemd/sd-rtnl/rtnl-message.c
> +++ b/src/libsystemd/sd-rtnl/rtnl-message.c
> @@ -58,6 +58,7 @@ int message_new(sd_rtnl *rtnl, sd_rtnl_message **ret, size_t initial_size) {
>
>          m->hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
>          m->sealed = false;
> +        LIST_HEAD_INIT(m->containers);
>
>          if (rtnl)
>                  m->rtnl = sd_rtnl_ref(rtnl);
> @@ -276,10 +277,18 @@ sd_rtnl_message *sd_rtnl_message_ref(sd_rtnl_message *m) {
>  }
>
>  sd_rtnl_message *sd_rtnl_message_unref(sd_rtnl_message *m) {
> +        struct rtnl_container *i, *j;
> +
>          if (m && REFCNT_DEC(m->n_ref) <= 0) {
>                  sd_rtnl_unref(m->rtnl);
>                  free(m->hdr);
>                  free(m->rta_offset_tb);
> +
> +                LIST_FOREACH_SAFE(container, i, j, m->containers) {
> +                        free(i->rta_offset_tb);
> +                        free(i);
> +                }
> +
>                  free(m);
>          }
>
> @@ -752,6 +761,22 @@ int sd_rtnl_message_open_container(sd_rtnl_message *m, unsigned short type) {
>          return -ENOTSUP;
>  }
>
> +int sd_rtnl_message_enter_container(sd_rtnl_message *m, unsigned short type) {
> +        struct rtnl_container *itr;
> +
> +        LIST_FOREACH(container, itr, m->containers) {
> +                if (itr->container_type == type)
> +                        break;
> +        }

Hm, so I don't think this is the right thing to do. The type of a
container (or any message) does not make sense unless you know the
context (i.e., the types of each of its parent containers). The reason
being that the types are just integers, and they are only unique
within a given scope.

I now pushed an alternative patch. Could you have a look if it makes
sense to you?

Also, would be great if you could respin the tunnel patch on top of
this (and following Zbigniew's suggestions for the tests).

Cheers,

Tom

> +        if(!itr)
> +                return -ENODATA;
> +
> +        m->container_list[m->n_containers++] = itr;
> +
> +        return 0;
> +}
> +
>  int sd_rtnl_message_close_container(sd_rtnl_message *m) {
>          assert_return(m, -EINVAL);
>          assert_return(!m->sealed, -EPERM);
> @@ -807,18 +832,34 @@ int sd_rtnl_message_read(sd_rtnl_message *m, unsigned short *type, void **data)
>  }
>
>  int rtnl_message_read_internal(sd_rtnl_message *m, unsigned short type, void **data) {
> +        size_t *rta_offset;
> +
>          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);
>
> -        if(!m->rta_offset_tb[type])
> -                return -ENODATA;
> +        /* We are not inside a container */
> +        if(!m->n_containers) {
> +                if(!m->rta_offset_tb[type])
> +                        return -ENODATA;
>
> -        *data = RTA_DATA((struct rtattr *)((uint8_t *) m->hdr + m->rta_offset_tb[type]));
> +                rta_offset = &m->rta_offset_tb[type];
> +        } else {
> +                struct rtnl_container *c;
>
> -        return 0;
> +                c = m->container_list[m->n_containers - 1];
> +
> +                if(!c->rta_offset_tb[type])
> +                        return -ENODATA;
> +
> +                rta_offset = &c->rta_offset_tb[type];
> +        }
> +
> +        *data = RTA_DATA((struct rtattr *)((uint8_t *) m->hdr + *rta_offset));
> +
> +        return RTA_PAYLOAD((struct rtattr *)((uint8_t *) m->hdr + *rta_offset));
>  }
>
>  int sd_rtnl_message_read_string(sd_rtnl_message *m, unsigned short type, char **data) {
> @@ -998,6 +1039,29 @@ static int message_receive_need(sd_rtnl *rtnl, size_t *need) {
>          return 0;
>  }
>
> +
> +int rtnl_message_parse_container(sd_rtnl_message *m,
> +                                 uint8_t type,
> +                                 uint8_t tb_size,
> +                                 struct rtattr *rta,
> +                                 unsigned int rt_len) {
> +        int r;
> +        struct rtnl_container *c;
> +
> +        r = rtnl_container_new(&c, type);
> +        if(r< 0)
> +                return r;
> +
> +        r = rtnl_message_parse(m, &c->rta_offset_tb, &c->rta_tb_size,
> +                               tb_size + 1, RTA_DATA(rta), RTA_PAYLOAD(rta));
> +        if(r < 0)
> +                return r;
> +
> +        LIST_PREPEND(container, m->containers, c);
> +
> +        return 0;
> +}
> +
>  int rtnl_message_parse(sd_rtnl_message *m,
>                         size_t **rta_offset_tb,
>                         unsigned short *rta_tb_size,
> @@ -1005,6 +1069,7 @@ int rtnl_message_parse(sd_rtnl_message *m,
>                         struct rtattr *rta,
>                         unsigned int rt_len) {
>          int type;
> +        int tb_size;
>          size_t *tb;
>
>          tb = (size_t *) new0(size_t *, max);
> @@ -1016,8 +1081,12 @@ 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])
> +                if (type <= max && !tb[type])
>                          tb[type] = (uint8_t *) rta - (uint8_t *) m->hdr;
> +
> +                tb_size = rtnl_attribute_type_is_container(m, type);
> +                if(tb_size > 0)
> +                        rtnl_message_parse_container(m, type, tb_size, rta, rt_len);
>          }
>
>          *rta_offset_tb = tb;
> diff --git a/src/libsystemd/sd-rtnl/rtnl-util.c b/src/libsystemd/sd-rtnl/rtnl-util.c
> index c8b20d1..5150837 100644
> --- a/src/libsystemd/sd-rtnl/rtnl-util.c
> +++ b/src/libsystemd/sd-rtnl/rtnl-util.c
> @@ -153,3 +153,34 @@ bool rtnl_message_type_is_addr(uint16_t type) {
>                          return false;
>          }
>  }
> +
> +int rtnl_attribute_type_is_container(sd_rtnl_message *m, uint16_t type) {
> +
> +        if(rtnl_message_type_is_link(m->hdr->nlmsg_type)) {
> +           switch(type) {
> +           case IFLA_LINKINFO:
> +                   IFLA_INFO_MAX;
> +           case IFLA_INFO_DATA:
> +                   return IFLA_VLAN_MAX;
> +           default:
> +                   return -1;
> +           }
> +        }
> +
> +        return -1;
> +}
> +
> +int rtnl_container_new(struct rtnl_container **ret, uint16_t type) {
> +        struct rtnl_container *c;
> +
> +        c = new0(struct rtnl_container, 1);
> +        if (!c)
> +                return -ENOMEM;
> +
> +        c->container_type = type;
> +        LIST_INIT(container, c);
> +
> +        *ret = c;
> +
> +        return 0;
> +}
> diff --git a/src/libsystemd/sd-rtnl/rtnl-util.h b/src/libsystemd/sd-rtnl/rtnl-util.h
> index 7fe9222..f804f6f 100644
> --- a/src/libsystemd/sd-rtnl/rtnl-util.h
> +++ b/src/libsystemd/sd-rtnl/rtnl-util.h
> @@ -34,6 +34,8 @@ bool rtnl_message_type_is_link(uint16_t type);
>  bool rtnl_message_type_is_addr(uint16_t type);
>  bool rtnl_message_type_is_route(uint16_t type);
>
> +int rtnl_attribute_type_is_container(sd_rtnl_message *m, uint16_t type);
> +
>  int rtnl_set_link_name(sd_rtnl *rtnl, int ifindex, const char *name);
>  int rtnl_set_link_properties(sd_rtnl *rtnl, int ifindex, const char *alias, const struct ether_addr *mac, unsigned mtu);
>
> diff --git a/src/libsystemd/sd-rtnl/test-rtnl.c b/src/libsystemd/sd-rtnl/test-rtnl.c
> index 6ecd660..d38f87e 100644
> --- a/src/libsystemd/sd-rtnl/test-rtnl.c
> +++ b/src/libsystemd/sd-rtnl/test-rtnl.c
> @@ -344,8 +344,8 @@ static void test_pipe(int ifindex) {
>  static void test_container(void) {
>          _cleanup_rtnl_message_unref_ sd_rtnl_message *m = NULL;
>          uint16_t type;
> -        uint32_t u32_data;
>          void *data;
> +        char *str;
>          int r;
>          struct ifinfomsg *ifi;
>
> @@ -396,7 +396,16 @@ static void test_container(void) {
>          if(r < 0)
>                  return;
>
> -        assert_se(sd_rtnl_message_read_u32(m, IFLA_LINKINFO, &u32_data) == 0);
> +        sd_rtnl_message_enter_container(m, IFLA_LINKINFO);
> +        assert_se(sd_rtnl_message_read_string(m, IFLA_INFO_KIND, &str) == 0);
> +
> +        sd_rtnl_message_enter_container(m, IFLA_INFO_DATA);
> +        assert_se(sd_rtnl_message_read_u16(m, IFLA_VLAN_ID, &type) == 0);
> +
> +        sd_rtnl_message_exit_container(m);
> +
> +        assert_se(sd_rtnl_message_read_string(m, IFLA_INFO_KIND, &str) == 0);
> +        sd_rtnl_message_exit_container(m);
>
>          assert_se(sd_rtnl_message_exit_container(m) == -EINVAL);
>  }
> --
> 1.8.5.3
>


More information about the systemd-devel mailing list