Few comments Antonio Darrell
On 9/13/17, 5:37 AM, "[email protected] on behalf of [email protected]" <[email protected] on behalf of [email protected]> wrote: Manage error value returned by ct_dpif_dump_next. Signed-off-by: Antonio Fischetti <[email protected]> --- 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; + } + [Darrell] Maybe we can reuse ‘error’ ? if (error && error != EOF) { and just do dpctl_error(dpctl_p, error, "dumping conntrack entry"); and then fall thru for cleanup ? 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; + } [Darrell] Can we reuse ‘error’, just print error and fall thru. ? It looks like it is safe to print whatever we know, which could be useful. Otherwise, if we have an error in dump_next, we may never be able to see any useful info. for debugging the same error or something else. 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, >)) { @@ -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; + } [Darrell] Same comment here: Can we reuse ‘error’, just print error and fall thru. ? It looks like it is safe to print whatever we know, which could be useful. Otherwise, if we have an error in dump_next, we may never be able to see any useful info. for debugging the same error or something else. dpctl_print(dpctl_p, "Current Connections: %d\n", tot_conn); dpctl_print(dpctl_p, "\n"); -- 2.4.11 _______________________________________________ dev mailing list [email protected] https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=HjzfhOU1f9U5hUM2bWAuizS921ZAAEjbIRHSM65FIAQ&s=u0r4kn1mAoWiO_rjM95TwOt5t-7hC2jiO3y_QKHHXNw&e= _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
