It is possible that a bundle add message fails, but the following
commit succeeds, since the message was not added to the bundle.  Make
ovs-ofctl fail also in these cases.

Also, the commit should not be sent if any of the bundled messages
failed.  To make sure all the errors are received before the commit is
sent, a barrier is required before sending the commit message.

Finally, make vconn collect bundle errors into a list instead of
calling a callback.  This makes bundle error management simpler.

Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
---
 include/openvswitch/vconn.h |  12 +++-
 lib/vconn.c                 | 130 +++++++++++++++++++++++++++++---------------
 tests/ofproto.at            |  34 +++++++-----
 tests/ovs-ofctl.at          |   4 ++
 utilities/ovs-ofctl.c       |  50 +++++++++++++++--
 5 files changed, 167 insertions(+), 63 deletions(-)

diff --git a/include/openvswitch/vconn.h b/include/openvswitch/vconn.h
index f8b6655..c44f0bf 100644
--- a/include/openvswitch/vconn.h
+++ b/include/openvswitch/vconn.h
@@ -55,9 +55,19 @@ int vconn_transact(struct vconn *, struct ofpbuf *, struct 
ofpbuf **);
 int vconn_transact_noreply(struct vconn *, struct ofpbuf *, struct ofpbuf **);
 int vconn_transact_multiple_noreply(struct vconn *, struct ovs_list *requests,
                                     struct ofpbuf **replyp);
+
+/* Bundle errors must be free()d by the caller. */
+struct vconn_bundle_error {
+    struct ovs_list list_node;
+
+    /* OpenFlow header and some of the message contents for error reporting. */
+    struct ofp_header ofp_msg[DIV_ROUND_UP(64, sizeof(struct ofp_header))];
+};
+
+/* Bundle errors must be free()d by the caller. */
 int vconn_bundle_transact(struct vconn *, struct ovs_list *requests,
                           uint16_t bundle_flags,
-                          void (*error_reporter)(const struct ofp_header *));
+                          struct ovs_list *errors);
 
 void vconn_run(struct vconn *);
 void vconn_run_wait(struct vconn *);
diff --git a/lib/vconn.c b/lib/vconn.c
index 917ad28..2759c1a 100644
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -744,9 +744,21 @@ vconn_recv_block(struct vconn *vconn, struct ofpbuf **msgp)
     return retval;
 }
 
+static void
+vconn_add_bundle_error(const struct ofp_header *oh, struct ovs_list *errors)
+{
+    if (errors) {
+        struct vconn_bundle_error *err = xmalloc(sizeof *err);
+        size_t len = ntohs(oh->length);
+
+        memcpy(err->ofp_msg, oh, MIN(len, sizeof err->ofp_msg));
+        ovs_list_push_back(errors, &err->list_node);
+    }
+}
+
 static int
 vconn_recv_xid__(struct vconn *vconn, ovs_be32 xid, struct ofpbuf **replyp,
-                 void (*error_reporter)(const struct ofp_header *))
+                 struct ovs_list *errors)
 {
     for (;;) {
         ovs_be32 recv_xid;
@@ -768,8 +780,8 @@ vconn_recv_xid__(struct vconn *vconn, ovs_be32 xid, struct 
ofpbuf **replyp,
         }
 
         error = ofptype_decode(&type, oh);
-        if (!error && type == OFPTYPE_ERROR && error_reporter) {
-            error_reporter(oh);
+        if (!error && type == OFPTYPE_ERROR) {
+            vconn_add_bundle_error(oh, errors);
         } else {
             VLOG_DBG_RL(&bad_ofmsg_rl, "%s: received reply with xid %08"PRIx32
                         " != expected %08"PRIx32,
@@ -793,8 +805,7 @@ vconn_recv_xid(struct vconn *vconn, ovs_be32 xid, struct 
ofpbuf **replyp)
 
 static int
 vconn_transact__(struct vconn *vconn, struct ofpbuf *request,
-                 struct ofpbuf **replyp,
-                 void (*error_reporter)(const struct ofp_header *))
+                 struct ofpbuf **replyp, struct ovs_list *errors)
 {
     ovs_be32 send_xid = ((struct ofp_header *) request->data)->xid;
     int error;
@@ -804,8 +815,7 @@ vconn_transact__(struct vconn *vconn, struct ofpbuf 
*request,
     if (error) {
         ofpbuf_delete(request);
     }
-    return error ? error : vconn_recv_xid__(vconn, send_xid, replyp,
-                                            error_reporter);
+    return error ? error : vconn_recv_xid__(vconn, send_xid, replyp, errors);
 }
 
 /* Sends 'request' to 'vconn' and blocks until it receives a reply with a
@@ -825,6 +835,22 @@ vconn_transact(struct vconn *vconn, struct ofpbuf *request,
     return vconn_transact__(vconn, request, replyp, NULL);
 }
 
+static int
+vconn_send_barrier(struct vconn *vconn, ovs_be32 *barrier_xid)
+{
+    struct ofpbuf *barrier;
+    int error;
+
+    /* Send barrier. */
+    barrier = ofputil_encode_barrier_request(vconn_get_version(vconn));
+    *barrier_xid = ((struct ofp_header *) barrier->data)->xid;
+    error = vconn_send_block(vconn, barrier);
+    if (error) {
+        ofpbuf_delete(barrier);
+    }
+    return error;
+}
+
 /* Sends 'request' followed by a barrier request to 'vconn', then blocks until
  * it receives a reply to the barrier.  If successful, stores the reply to
  * 'request' in '*replyp', if one was received, and otherwise NULL, then
@@ -842,7 +868,6 @@ vconn_transact_noreply(struct vconn *vconn, struct ofpbuf 
*request,
 {
     ovs_be32 request_xid;
     ovs_be32 barrier_xid;
-    struct ofpbuf *barrier;
     int error;
 
     *replyp = NULL;
@@ -856,11 +881,8 @@ vconn_transact_noreply(struct vconn *vconn, struct ofpbuf 
*request,
     }
 
     /* Send barrier. */
-    barrier = ofputil_encode_barrier_request(vconn_get_version(vconn));
-    barrier_xid = ((struct ofp_header *) barrier->data)->xid;
-    error = vconn_send_block(vconn, barrier);
+    error = vconn_send_barrier(vconn, &barrier_xid);
     if (error) {
-        ofpbuf_delete(barrier);
         return error;
     }
 
@@ -924,7 +946,7 @@ vconn_transact_multiple_noreply(struct vconn *vconn, struct 
ovs_list *requests,
 static enum ofperr
 vconn_bundle_reply_validate(struct ofpbuf *reply,
                             struct ofputil_bundle_ctrl_msg *request,
-                            void (*error_reporter)(const struct ofp_header *))
+                            struct ovs_list *errors)
 {
     const struct ofp_header *oh;
     enum ofptype type;
@@ -938,7 +960,7 @@ vconn_bundle_reply_validate(struct ofpbuf *reply,
     }
 
     if (type == OFPTYPE_ERROR) {
-        error_reporter(oh);
+        vconn_add_bundle_error(oh, errors);
         return ofperr_decode_msg(oh, NULL);
     }
     if (type != OFPTYPE_BUNDLE_CONTROL) {
@@ -964,15 +986,15 @@ vconn_bundle_reply_validate(struct ofpbuf *reply,
 /* Send bundle control message 'bc' of 'type' via 'vconn', and wait for either
  * an error or the corresponding bundle control message response.
  *
- * 'error_reporter' is called for any error responses received, which may be
- * also regarding earlier OpenFlow messages than this bundle control message.
+ * 'errors' is a list to which any OpenFlow errors relating to bundle
+ * processing are appended.  Caller is responsible for reelasing the memory of
+ * each node in the list on return.
  *
  * Returns errno value, or 0 when successful. */
 static int
 vconn_bundle_control_transact(struct vconn *vconn,
                               struct ofputil_bundle_ctrl_msg *bc,
-                              uint16_t type,
-                              void (*error_reporter)(const struct ofp_header 
*))
+                              uint16_t type, struct ovs_list *errors)
 {
     struct ofpbuf *request, *reply;
     int error;
@@ -981,21 +1003,12 @@ vconn_bundle_control_transact(struct vconn *vconn,
     bc->type = type;
     request = ofputil_encode_bundle_ctrl_request(vconn->version, bc);
     ofpmsg_update_length(request);
-    error = vconn_transact__(vconn, request, &reply, error_reporter);
+    error = vconn_transact__(vconn, request, &reply, errors);
     if (error) {
         return error;
     }
 
-    ofperr = vconn_bundle_reply_validate(reply, bc, error_reporter);
-    if (ofperr) {
-        VLOG_WARN_RL(&bad_ofmsg_rl, "Bundle %s failed (%s).",
-                     type == OFPBCT_OPEN_REQUEST ? "open"
-                     : type == OFPBCT_CLOSE_REQUEST ? "close"
-                     : type == OFPBCT_COMMIT_REQUEST ? "commit"
-                     : type == OFPBCT_DISCARD_REQUEST ? "discard"
-                     : "control message",
-                     ofperr_to_string(ofperr));
-    }
+    ofperr = vconn_bundle_reply_validate(reply, bc, errors);
     ofpbuf_delete(reply);
 
     return ofperr ? EPROTO : 0;
@@ -1003,8 +1016,7 @@ vconn_bundle_control_transact(struct vconn *vconn,
 
 /* Checks if error responses can be received on 'vconn'. */
 static void
-vconn_recv_error(struct vconn *vconn,
-                 void (*error_reporter)(const struct ofp_header *))
+vconn_recv_error(struct vconn *vconn, struct ovs_list *errors)
 {
     int error;
 
@@ -1020,7 +1032,7 @@ vconn_recv_error(struct vconn *vconn,
             oh = reply->data;
             ofperr = ofptype_decode(&type, oh);
             if (!ofperr && type == OFPTYPE_ERROR) {
-                error_reporter(oh);
+                vconn_add_bundle_error(oh, errors);
             } else {
                 VLOG_DBG_RL(&bad_ofmsg_rl,
                             "%s: received unexpected reply with xid %08"PRIx32,
@@ -1031,10 +1043,32 @@ vconn_recv_error(struct vconn *vconn,
     } while (!error);
 }
 
+/* Sends a barrier and waits for the barrier response and stores any errors
+ * that are received before the barrier response. */
+static int
+vconn_bundle_barrier_transact(struct vconn *vconn, struct ovs_list *errors)
+{
+    struct ofpbuf *reply;
+    ovs_be32 barrier_xid;
+    int error;
+
+    error = vconn_send_barrier(vconn, &barrier_xid);
+    if (error) {
+        return error;
+    }
+
+    error = vconn_recv_xid__(vconn, barrier_xid, &reply, errors);
+    if (error) {
+        return error;
+    }
+    ofpbuf_delete(reply);
+    return 0;
+}
+
 static int
 vconn_bundle_add_msg(struct vconn *vconn, struct ofputil_bundle_ctrl_msg *bc,
                      struct ofpbuf *msg,
-                     void (*error_reporter)(const struct ofp_header *))
+                     struct ovs_list *errors)
 {
     struct ofputil_bundle_add_msg bam;
     struct ofpbuf *request;
@@ -1052,45 +1086,55 @@ vconn_bundle_add_msg(struct vconn *vconn, struct 
ofputil_bundle_ctrl_msg *bc,
     if (!error) {
         /* Check for an error return, so that the socket buffer does not become
          * full of errors. */
-        vconn_recv_error(vconn, error_reporter);
+        vconn_recv_error(vconn, errors);
     }
     return error;
 }
 
 int
 vconn_bundle_transact(struct vconn *vconn, struct ovs_list *requests,
-                      uint16_t flags,
-                      void (*error_reporter)(const struct ofp_header *))
+                      uint16_t flags, struct ovs_list *errors)
 {
     struct ofputil_bundle_ctrl_msg bc;
     struct ofpbuf *request;
     int error;
 
+    ovs_list_init(errors);
+
     memset(&bc, 0, sizeof bc);
     bc.flags = flags;
     error = vconn_bundle_control_transact(vconn, &bc, OFPBCT_OPEN_REQUEST,
-                                          error_reporter);
+                                          errors);
     if (error) {
         return error;
     }
 
     LIST_FOR_EACH (request, list_node, requests) {
-        error = vconn_bundle_add_msg(vconn, &bc, request, error_reporter);
+        error = vconn_bundle_add_msg(vconn, &bc, request, errors);
         if (error) {
             break;
         }
     }
 
     if (!error) {
+        /* A failing message does not invalidate the bundle, but the message is
+         * simply not added to the bundle.  Since we do not want to commit if
+         * any of the messages failed, we need to explicitly sync with barrier
+         * before we issue the commit message. */
+        error = vconn_bundle_barrier_transact(vconn, errors);
+    }
+    if (!error && !ovs_list_is_empty(errors)) {
+        error = EPROTO;
+    }
+
+    /* Commit only if no errors are received. */
+    if (!error) {
         error = vconn_bundle_control_transact(vconn, &bc,
                                               OFPBCT_COMMIT_REQUEST,
-                                              error_reporter);
+                                              errors);
     } else {
-        /* Do not overwrite the error code from vconn_bundle_add_msg().
-         * Any error in discard should be either reported or logged, so it
-         * should not get lost. */
         vconn_bundle_control_transact(vconn, &bc, OFPBCT_DISCARD_REQUEST,
-                                      error_reporter);
+                                      errors);
     }
     return error;
 }
diff --git a/tests/ofproto.at b/tests/ofproto.at
index e5fb54c..56f63d0 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -4409,6 +4409,8 @@ OFPT_FLOW_MOD (OF1.4): ADD 
in_port=2,dl_src=00:88:99:aa:bb:cc idle:70 actions=ou
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
  bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): DEL table:255 in_port=2,dl_src=00:88:99:aa:bb:cc 
actions=drop
+vconn|DBG|unix: received: OFPT_BARRIER_REQUEST (OF1.4):
+vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY (OF1.4):
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
@@ -4440,6 +4442,8 @@ OFPT_FLOW_MOD (OF1.4): MOD actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
  bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): MOD_STRICT in_port=2,dl_src=00:77:88:99:aa:bb 
actions=output:7
+vconn|DBG|unix: received: OFPT_BARRIER_REQUEST (OF1.4):
+vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY (OF1.4):
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
@@ -4474,6 +4478,8 @@ OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 
in_port=2,dl_src=00:66:77:88:99:aa a
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
  bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:66:77:88:99:aa actions=drop
+vconn|DBG|unix: received: OFPT_BARRIER_REQUEST (OF1.4):
+vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY (OF1.4):
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
@@ -4528,15 +4534,11 @@ delete in_port=2 dl_src=00:88:99:aa:bb:cc
 add table=254 actions=drop
 ])
 
-AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt 2>&1 | sed '/|WARN|/d
-s/unix:.*br0\.mgmt/unix:br0.mgmt/' | sed 's/(.* error)/(error)/'],
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt 2>&1 | sed '/talking 
to/,$d'],
 [0], [dnl
-OFPT_ERROR (OF1.4) (xid=0xb): OFPBRC_EPERM
-OFPT_FLOW_MOD (OF1.4) (xid=0xb): ADD table:254 actions=drop
-OFPT_ERROR (OF1.4) (xid=0xd): OFPBFC_MSG_FAILED
-OFPT_BUNDLE_CONTROL (OF1.4) (xid=0xd):
+Error OFPBRC_EPERM for: OFPT_FLOW_MOD (OF1.4) (xid=0xb): ADD table:254 
actions=drop
+Error OFPBFC_MSG_FAILED for: OFPT_BUNDLE_CONTROL (OF1.4) (xid=0xe):
  bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
-ovs-ofctl: talking to unix:br0.mgmt (error)
 ])
 
 AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
@@ -4894,6 +4896,8 @@ OFPT_FLOW_MOD (OF1.3): ADD 
in_port=2,dl_src=00:88:99:aa:bb:cc idle:70 actions=ou
 vconn|DBG|unix: received: ONFT_BUNDLE_ADD_MESSAGE (OF1.3):
  bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.3): DEL table:255 in_port=2,dl_src=00:88:99:aa:bb:cc 
actions=drop
+vconn|DBG|unix: received: OFPT_BARRIER_REQUEST (OF1.3):
+vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY (OF1.3):
 vconn|DBG|unix: received: ONFT_BUNDLE_CONTROL (OF1.3):
  bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): ONFT_BUNDLE_CONTROL (OF1.3):
@@ -4925,6 +4929,8 @@ OFPT_FLOW_MOD (OF1.3): MOD actions=drop
 vconn|DBG|unix: received: ONFT_BUNDLE_ADD_MESSAGE (OF1.3):
  bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.3): MOD_STRICT in_port=2,dl_src=00:77:88:99:aa:bb 
actions=output:7
+vconn|DBG|unix: received: OFPT_BARRIER_REQUEST (OF1.3):
+vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY (OF1.3):
 vconn|DBG|unix: received: ONFT_BUNDLE_CONTROL (OF1.3):
  bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): ONFT_BUNDLE_CONTROL (OF1.3):
@@ -4959,6 +4965,8 @@ OFPT_FLOW_MOD (OF1.3): DEL_STRICT table:255 
in_port=2,dl_src=00:66:77:88:99:aa a
 vconn|DBG|unix: received: ONFT_BUNDLE_ADD_MESSAGE (OF1.3):
  bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.3): ADD in_port=2,dl_src=00:66:77:88:99:aa actions=drop
+vconn|DBG|unix: received: OFPT_BARRIER_REQUEST (OF1.3):
+vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY (OF1.3):
 vconn|DBG|unix: received: ONFT_BUNDLE_CONTROL (OF1.3):
  bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): ONFT_BUNDLE_CONTROL (OF1.3):
@@ -4981,7 +4989,7 @@ OVS_VSWITCHD_STOP
 AT_CLEANUP
 
 
-AT_SETUP([ofproto - failing bundle commit (OpenFlow 1.3)])
+AT_SETUP([ofproto - failing bundle add message (OpenFlow 1.3)])
 AT_KEYWORDS([monitor])
 OVS_VSWITCHD_START
 
@@ -5013,15 +5021,11 @@ delete in_port=2 dl_src=00:88:99:aa:bb:cc
 add table=254 actions=drop
 ])
 
-AT_CHECK([ovs-ofctl -O OpenFlow13 --bundle add-flows br0 flows.txt 2>&1 | sed 
'/|WARN|/d
-s/unix:.*br0\.mgmt/unix:br0.mgmt/' | sed 's/(.* error)/(error)/'],
+AT_CHECK([ovs-ofctl -O OpenFlow13 --bundle add-flows br0 flows.txt 2>&1 | sed 
'/talking to/,$d'],
 [0], [dnl
-OFPT_ERROR (OF1.3) (xid=0xb): OFPBRC_EPERM
-OFPT_FLOW_MOD (OF1.3) (xid=0xb): ADD table:254 actions=drop
-OFPT_ERROR (OF1.3) (xid=0xd): OFPBFC_MSG_FAILED
-ONFT_BUNDLE_CONTROL (OF1.3) (xid=0xd):
+Error OFPBRC_EPERM for: OFPT_FLOW_MOD (OF1.3) (xid=0xb): ADD table:254 
actions=drop
+Error OFPBFC_MSG_FAILED for: ONFT_BUNDLE_CONTROL (OF1.3) (xid=0xe):
  bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
-ovs-ofctl: talking to unix:br0.mgmt (error)
 ])
 
 AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 23effd6..00db247 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -2972,6 +2972,8 @@ OFPT_FLOW_MOD (OF1.4): ADD table:7 dl_vlan=7 importance:7 
actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
  bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): ADD table:8 dl_vlan=8 importance:8 actions=drop
+vconn|DBG|unix: received: OFPT_BARRIER_REQUEST (OF1.4):
+vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY (OF1.4):
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
@@ -3019,6 +3021,8 @@ OFPT_FLOW_MOD (OF1.4): ADD table:7 dl_vlan=7 
importance:17 actions=drop
 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
  bundle_id=0 flags=atomic ordered
 OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:8 dl_vlan=8 actions=drop
+vconn|DBG|unix: received: OFPT_BARRIER_REQUEST (OF1.4):
+vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY (OF1.4):
 vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
  bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
 vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index e373aa4..dd82e8d 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -660,18 +660,60 @@ transact_multiple_noreply(struct vconn *vconn, struct 
ovs_list *requests)
     ofpbuf_delete(reply);
 }
 
+/* Frees the error messages as they are printed. */
 static void
-bundle_error_reporter(const struct ofp_header *oh)
+bundle_print_errors(struct ovs_list *errors, struct ovs_list *requests)
 {
-    ofp_print(stderr, oh, ntohs(oh->length), verbosity + 1);
+    struct vconn_bundle_error *error, *next;
+    struct ofpbuf *bmsg;
+
+    INIT_CONTAINER(bmsg, requests, list_node);
+
+    LIST_FOR_EACH_SAFE (error, next, list_node, errors) {
+        enum ofperr ofperr;
+        struct ofpbuf payload;
+
+        ofperr = ofperr_decode_msg(error->ofp_msg, &payload);
+        if (!ofperr) {
+            fprintf(stderr, "***decode error***");
+        } else {
+            /* Default to the likely truncated message. */
+            const struct ofp_header *ofp_msg = payload.data;
+            size_t msg_len = payload.size;
+
+            /* Find the failing message from the requests list to be able to
+             * dump the whole message.  We assume the errors are returned in
+             * the same order as in which the messages are sent to get O(n)
+             * rather than O(n^2) processing here.  If this heuristics fails we
+             * may print the truncated hexdumps instead. */
+            LIST_FOR_EACH_CONTINUE (bmsg, list_node, requests) {
+                const struct ofp_header *oh = bmsg->data;
+
+                if (oh->xid == error->ofp_msg->xid) {
+                    ofp_msg = oh;
+                    msg_len = bmsg->size;
+                    break;
+                }
+            }
+            fprintf(stderr, "Error %s for: ", ofperr_get_name(ofperr));
+            ofp_print(stderr, ofp_msg, msg_len, verbosity + 1);
+        }
+        free(error);
+    }
     fflush(stderr);
 }
 
 static void
 bundle_transact(struct vconn *vconn, struct ovs_list *requests, uint16_t flags)
 {
-    run(vconn_bundle_transact(vconn, requests, flags, bundle_error_reporter),
-        "talking to %s", vconn_get_name(vconn));
+    struct ovs_list errors;
+    int retval = vconn_bundle_transact(vconn, requests, flags, &errors);
+
+    bundle_print_errors(&errors, requests);
+
+    if (retval) {
+        ovs_fatal(retval, "talking to %s", vconn_get_name(vconn));
+    }
 }
 
 /* Sends 'request', which should be a request that only has a reply if an error
-- 
2.1.4

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to