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

Message ID 1505306164-6665-1-git-send-email-antonio.fischetti@intel.com
State New
Headers show
Series
  • [ovs-dev,1/2] dpctl: manage ret value when dumping CT entries.
Related show

Commit Message

Fischetti, Antonio Sept. 13, 2017, 12:36 p.m.
Manage error value returned by ct_dpif_dump_next.

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

Comments

Greg Rose Sept. 18, 2017, 6:08 p.m. | #1
On 09/13/2017 05:36 AM, antonio.fischetti@intel.com wrote:
> Manage error value returned by ct_dpif_dump_next.
> 
> Signed-off-by: Antonio Fischetti <antonio.fischetti@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 <gvrose8192@gmail.com>

Patch

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");