Eelco Chaudron <[email protected]> writes: > On 20 Jun 2023, at 16:17, Aaron Conole wrote: > >> Eelco Chaudron <[email protected]> writes: >> >>> Add additional error coverage counters for dpif operation failures. >>> This could help to quickly identify netlink problems when communicating >>> with the OVS kernel module. >>> >>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2070630 >>> Signed-off-by: Eelco Chaudron <[email protected]> >>> --- >> >> One thing that made this difficult to review is the re-ordering of the >> COVERAGE_DEFINE()s. Maybe it would be better to have a separate patch >> that corrects the order to alphabetical, and then a patch that adds the >> new counters for flow put/del/get/execute. WDYT? > > Based on the patch size, I decided not to do this, but if others also > feel it needs to be split up, I can do this.
Either way: Acked-by: Aaron Conole <[email protected]> > //Eelco > > >>> lib/dpif.c | 39 ++++++++++++++++++++++++++------------- >>> 1 file changed, 26 insertions(+), 13 deletions(-) >>> >>> diff --git a/lib/dpif.c b/lib/dpif.c >>> index 3305401fe..b1cbf39c4 100644 >>> --- a/lib/dpif.c >>> +++ b/lib/dpif.c >>> @@ -55,18 +55,22 @@ >>> VLOG_DEFINE_THIS_MODULE(dpif); >>> >>> COVERAGE_DEFINE(dpif_destroy); >>> -COVERAGE_DEFINE(dpif_port_add); >>> -COVERAGE_DEFINE(dpif_port_del); >>> +COVERAGE_DEFINE(dpif_execute); >>> +COVERAGE_DEFINE(dpif_execute_error); >>> +COVERAGE_DEFINE(dpif_execute_with_help); >>> +COVERAGE_DEFINE(dpif_flow_del); >>> +COVERAGE_DEFINE(dpif_flow_del_error); >>> COVERAGE_DEFINE(dpif_flow_flush); >>> COVERAGE_DEFINE(dpif_flow_get); >>> +COVERAGE_DEFINE(dpif_flow_get_error); >>> COVERAGE_DEFINE(dpif_flow_put); >>> -COVERAGE_DEFINE(dpif_flow_del); >>> -COVERAGE_DEFINE(dpif_execute); >>> -COVERAGE_DEFINE(dpif_purge); >>> -COVERAGE_DEFINE(dpif_execute_with_help); >>> -COVERAGE_DEFINE(dpif_meter_set); >>> -COVERAGE_DEFINE(dpif_meter_get); >>> +COVERAGE_DEFINE(dpif_flow_put_error); >>> COVERAGE_DEFINE(dpif_meter_del); >>> +COVERAGE_DEFINE(dpif_meter_get); >>> +COVERAGE_DEFINE(dpif_meter_set); >>> +COVERAGE_DEFINE(dpif_port_add); >>> +COVERAGE_DEFINE(dpif_port_del); >>> +COVERAGE_DEFINE(dpif_purge); >>> >>> static const struct dpif_class *base_dpif_classes[] = { >>> #if defined(__linux__) || defined(_WIN32) >>> @@ -1381,8 +1385,11 @@ dpif_operate(struct dpif *dpif, struct dpif_op >>> **ops, size_t n_ops, >>> >>> COVERAGE_INC(dpif_flow_put); >>> log_flow_put_message(dpif, &this_module, put, error); >>> - if (error && put->stats) { >>> - memset(put->stats, 0, sizeof *put->stats); >>> + if (error) { >>> + COVERAGE_INC(dpif_flow_put_error); >>> + if (put->stats) { >>> + memset(put->stats, 0, sizeof *put->stats); >>> + } >>> } >>> break; >>> } >>> @@ -1392,10 +1399,10 @@ dpif_operate(struct dpif *dpif, struct dpif_op >>> **ops, size_t n_ops, >>> >>> COVERAGE_INC(dpif_flow_get); >>> if (error) { >>> + COVERAGE_INC(dpif_flow_get_error); >>> memset(get->flow, 0, sizeof *get->flow); >>> } >>> log_flow_get_message(dpif, &this_module, get, error); >>> - >>> break; >>> } >>> >>> @@ -1404,8 +1411,11 @@ dpif_operate(struct dpif *dpif, struct dpif_op >>> **ops, size_t n_ops, >>> >>> COVERAGE_INC(dpif_flow_del); >>> log_flow_del_message(dpif, &this_module, del, error); >>> - if (error && del->stats) { >>> - memset(del->stats, 0, sizeof *del->stats); >>> + if (error) { >>> + COVERAGE_INC(dpif_flow_del_error); >>> + if (del->stats) { >>> + memset(del->stats, 0, sizeof *del->stats); >>> + } >>> } >>> break; >>> } >>> @@ -1414,6 +1424,9 @@ dpif_operate(struct dpif *dpif, struct dpif_op **ops, >>> size_t n_ops, >>> COVERAGE_INC(dpif_execute); >>> log_execute_message(dpif, &this_module, &op->execute, >>> false, error); >>> + if (error) { >>> + COVERAGE_INC(dpif_execute_error); >>> + } >>> break; >>> } >>> } _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
