Re: [ovs-dev] [PATCH 1/2] dpctl: manage ret value when dumping CT entries.

[Greg Rose]

On 09/13/2017 05:36 AM, antonio.fische...@intel.com wrote:

Manage error value returned by ct_dpif_dump_next.

Signed-off-by: Antonio Fischetti <antonio.fische...@intel.com>
---
  lib/dpctl.c | 28 +++++++++++++++++++++++++---
  1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index 8951d6e..86d0f90 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -1263,6 +1263,7 @@ dpctl_dump_conntrack(int argc, const char *argv[],
      struct dpif *dpif;
      char *name;
      int error;
+    int ret;

if (argc > 1 && ovs_scan(argv[argc - 1], "zone=%"SCNu16, &zone)) {

          pzone = &zone;
@@ -1286,7 +1287,7 @@ dpctl_dump_conntrack(int argc, const char *argv[],
          return error;
      }

- while (!ct_dpif_dump_next(dump, &cte)) {

+    while (!(ret = ct_dpif_dump_next(dump, &cte))) {
          struct ds s = DS_EMPTY_INITIALIZER;

ct_dpif_format_entry(&cte, &s, dpctl_p->verbosity,

@@ -1296,6 +1297,13 @@ dpctl_dump_conntrack(int argc, const char *argv[],
          dpctl_print(dpctl_p, "%s\n", ds_cstr(&s));
          ds_destroy(&s);
      }
+    if (ret && ret != EOF) {
+        dpctl_error(dpctl_p, ret, "dumping conntrack entry");
+        ct_dpif_dump_done(dump);
+        dpif_close(dpif);
+        return ret;
+    }
+
      ct_dpif_dump_done(dump);
      dpif_close(dpif);
      return error;
@@ -1348,6 +1356,7 @@ dpctl_ct_stats_show(int argc, const char *argv[],
      int proto_stats[CT_STATS_MAX];
      int tcp_conn_per_states[CT_DPIF_TCPS_MAX_NUM];
      int error;
+    int ret;

while (argc > 1 && lastargc != argc) {

          lastargc = argc;
@@ -1384,7 +1393,7 @@ dpctl_ct_stats_show(int argc, const char *argv[],
      }

int tot_conn = 0;

-    while (!ct_dpif_dump_next(dump, &cte)) {
+    while (!(ret = ct_dpif_dump_next(dump, &cte))) {
          ct_dpif_entry_uninit(&cte);
          tot_conn++;
          switch (cte.tuple_orig.ip_proto) {
@@ -1425,6 +1434,12 @@ dpctl_ct_stats_show(int argc, const char *argv[],
              break;
          }
      }
+    if (ret && ret != EOF) {
+        dpctl_error(dpctl_p, ret, "dumping conntrack entry");
+        ct_dpif_dump_done(dump);
+        dpif_close(dpif);
+        return ret;
+    }

dpctl_print(dpctl_p, "Connections Stats:\n Total: %d\n", tot_conn);

      if (proto_stats[CT_STATS_TCP]) {
@@ -1482,6 +1497,7 @@ dpctl_ct_bkts(int argc, const char *argv[],
      uint16_t *pzone = NULL;
      int tot_bkts = 0;
      int error;
+    int ret;

if (argc > 1 && !strncmp(argv[argc - 1], CT_BKTS_GT, strlen(CT_BKTS_GT))) {

          if (ovs_scan(argv[argc - 1], CT_BKTS_GT"%"SCNu16, &gt)) {
@@ -1521,7 +1537,7 @@ dpctl_ct_bkts(int argc, const char *argv[],
      int tot_conn = 0;
      uint32_t *conn_per_bkts = xzalloc(tot_bkts * sizeof(uint32_t));

- while (!ct_dpif_dump_next(dump, &cte)) {

+    while (!(ret = ct_dpif_dump_next(dump, &cte))) {
          ct_dpif_entry_uninit(&cte);
          tot_conn++;
          if (tot_bkts > 0) {
@@ -1533,6 +1549,12 @@ dpctl_ct_bkts(int argc, const char *argv[],
              }
          }
      }
+    if (ret && ret != EOF) {
+        dpctl_error(dpctl_p, ret, "dumping conntrack entry");
+        ct_dpif_dump_done(dump);
+        dpif_close(dpif);
+        return ret;
+    }

dpctl_print(dpctl_p, "Current Connections: %d\n", tot_conn);

      dpctl_print(dpctl_p, "\n");

This looks fine to me but I don't know of any way to test it - I guess we'd 
need to force a dump error.

Reviewed-by: Greg Rose <gvrose8...@gmail.com>

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev