sd-rtnl: handle empty multi-part message from the kernel

We strips out NLMSG_DONE piece from a multi-part message adding into the
receive queue only the messages containing actual data.

If we send a request to the kernel for getting the forwarding database table (just an example),
the response will be a multi-part message like below:
1. FDB entry 1;
2. FDB entry 2;
3. NLMSG_DONE;

We strip out "3. NLMSG_DONE;" part and places into the receive queue a pointer to
"1. FDB entry 1; 2. FDB entry 2".

But if the FDB table is empty, the respose from the kernel will look like below:
1. NLMSG_DONE;

We strip out "1. NLMSG_DONE;" part and since there is no actual data got, it continues
waiting until reaching timeout.

Therefore, a call to "sd_rtnl_call" to send and wait for a response from kernel will exit
with timeout which is interpreted as error in communication.

This patch puts the NLMSG_DONE message on the receive queue if it ends an empty multi-part
message. This situation is detected in sd_rtnl_call() and in the callback code and NULL is
returned to the caller instead.

[tomegun:
  - added/reworded commit message
  - extend the same support to sd_rtnl_call_async()
  - drop debug logging from library, we only do this if something is really wrong, but an
    empty multi-part message is perfectly normal
  - modernize the code we touch whilst we are at it]

diff --git a/src/libsystemd/sd-rtnl/rtnl-message.c b/src/libsystemd/sd-rtnl/rtnl-message.c
index 5a719003ac354fc83cc968e0c5e0b0e4e65aa6b9..c938471fe30a957ec54427666a1aecf9b17c6872 100644 (file)
--- a/src/libsystemd/sd-rtnl/rtnl-message.c
+++ b/src/libsystemd/sd-rtnl/rtnl-message.c
@@ -1554,7 +1554,9 @@ int socket_read_message(sd_rtnl *rtnl) {
                         /* finished reading multi-part message */
                         done = true;
 
                         /* finished reading multi-part message */
                         done = true;
 

-                        continue;
+                        /* if first is not defined, put NLMSG_DONE into the receive queue. */
+                        if (first)
+                                continue;

                 }
 
                 /* check that we support this message type */
                 }
 
                 /* check that we support this message type */

diff --git a/src/libsystemd/sd-rtnl/rtnl-types.c b/src/libsystemd/sd-rtnl/rtnl-types.c
index e21c8986684394cffaf594cdc778b832ba9400c5..bf7278fe449037eeeb5296ce4bffa04b6861ab5d 100644 (file)
--- a/src/libsystemd/sd-rtnl/rtnl-types.c
+++ b/src/libsystemd/sd-rtnl/rtnl-types.c
@@ -411,6 +411,7 @@ static const NLTypeSystem rtnl_neigh_type_system = {
 };
 
 static const NLType rtnl_types[RTM_MAX + 1] = {
 };
 
 static const NLType rtnl_types[RTM_MAX + 1] = {

+        [NLMSG_DONE]   = { .type = NLA_META, .size = 0 },

         [NLMSG_ERROR]  = { .type = NLA_META, .size = sizeof(struct nlmsgerr) },
         [RTM_NEWLINK]  = { .type = NLA_NESTED, .type_system = &rtnl_link_type_system, .size = sizeof(struct ifinfomsg) },
         [RTM_DELLINK]  = { .type = NLA_NESTED, .type_system = &rtnl_link_type_system, .size = sizeof(struct ifinfomsg) },
         [NLMSG_ERROR]  = { .type = NLA_META, .size = sizeof(struct nlmsgerr) },
         [RTM_NEWLINK]  = { .type = NLA_NESTED, .type_system = &rtnl_link_type_system, .size = sizeof(struct ifinfomsg) },
         [RTM_DELLINK]  = { .type = NLA_NESTED, .type_system = &rtnl_link_type_system, .size = sizeof(struct ifinfomsg) },

diff --git a/src/libsystemd/sd-rtnl/sd-rtnl.c b/src/libsystemd/sd-rtnl/sd-rtnl.c
index 50162c3ee9267a175480573f484be605fdf920e0..40dea1252fcbf5327d97ccb5c8160094f99b3991 100644 (file)
--- a/src/libsystemd/sd-rtnl/sd-rtnl.c
+++ b/src/libsystemd/sd-rtnl/sd-rtnl.c
@@ -421,8 +421,9 @@ static int process_timeout(sd_rtnl *rtnl) {
 }
 
 static int process_reply(sd_rtnl *rtnl, sd_rtnl_message *m) {
 }
 
 static int process_reply(sd_rtnl *rtnl, sd_rtnl_message *m) {

-        struct reply_callback *c;
+        _cleanup_free_ struct reply_callback *c = NULL;

         uint64_t serial;
         uint64_t serial;

+        uint16_t type;

         int r;
 
         assert(rtnl);
         int r;
 
         assert(rtnl);


@@ -436,12 +437,17 @@ static int process_reply(sd_rtnl *rtnl, sd_rtnl_message *m) {
         if (c->timeout != 0)
                 prioq_remove(rtnl->reply_callbacks_prioq, c, &c->prioq_idx);
 
         if (c->timeout != 0)
                 prioq_remove(rtnl->reply_callbacks_prioq, c, &c->prioq_idx);
 

+        r = sd_rtnl_message_get_type(m, &type);
+        if (r < 0)
+                return 0;
+
+        if (type == NLMSG_DONE)
+                m = NULL;
+

         r = c->callback(rtnl, m, c->userdata);
         if (r < 0)
                 log_debug_errno(r, "sd-rtnl: callback failed: %m");
 
         r = c->callback(rtnl, m, c->userdata);
         if (r < 0)
                 log_debug_errno(r, "sd-rtnl: callback failed: %m");
 

-        free(c);
-

         return 1;
 }
 
         return 1;
 }
 


@@ -702,7 +708,6 @@ int sd_rtnl_call(sd_rtnl *rtnl,
                 sd_rtnl_message **ret) {
         usec_t timeout;
         uint32_t serial;
                 sd_rtnl_message **ret) {
         usec_t timeout;
         uint32_t serial;

-        unsigned i = 0;

         int r;
 
         assert_return(rtnl, -EINVAL);
         int r;
 
         assert_return(rtnl, -EINVAL);


@@ -717,36 +722,44 @@ int sd_rtnl_call(sd_rtnl *rtnl,
 
         for (;;) {
                 usec_t left;
 
         for (;;) {
                 usec_t left;

+                unsigned i;

 
 

-                while (i < rtnl->rqueue_size) {
-                        sd_rtnl_message *incoming;
+                for (i = 0; i < rtnl->rqueue_size; i++) {

                         uint32_t received_serial;
 
                         uint32_t received_serial;
 

-                        incoming = rtnl->rqueue[i];
-                        received_serial = rtnl_message_get_serial(incoming);
+                        received_serial = rtnl_message_get_serial(rtnl->rqueue[i]);

 
                         if (received_serial == serial) {
 
                         if (received_serial == serial) {

+                                _cleanup_rtnl_message_unref_ sd_rtnl_message *incoming = NULL;
+                                uint16_t type;
+
+                                incoming = rtnl->rqueue[i];
+

                                 /* 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);
                                 /* 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) {
-                                        sd_rtnl_message_unref(incoming);
+                                if (r < 0)
+                                        return r;
+
+                                r = sd_rtnl_message_get_type(incoming, &type);
+                                if (r < 0)

                                         return r;
                                         return r;

+
+                                if (type == NLMSG_DONE) {
+                                        *ret = NULL;
+                                        return 0;

                                 }
 
                                 if (ret) {
                                         *ret = incoming;
                                 }
 
                                 if (ret) {
                                         *ret = incoming;

-                                } else
-                                        sd_rtnl_message_unref(incoming);
+                                        incoming = NULL;
+                                }

 
                                 return 1;
                         }
 
                                 return 1;
                         }

-
-                        /* Try to read more, right away */
-                        i ++;

                 }
 
                 r = socket_read_message(rtnl);
                 }
 
                 r = socket_read_message(rtnl);