From: Peter Spreadborough <[email protected]> Incorrect packet counts were being returned for ovs flows. This was happening when stats counter was doing read-clear for flow stats at a rate faster than OVS was reading the stats from us. In this scenario SC would read a count and as a result the HW counter would be reset, SC would then do a second read which would replace the current count and it would be lost. The fix is to never do read-clear and always update with the latest full count. If the application requests a reset then the current count is returned minus the last value read and the last value read updated with the current count, hence just the delta is returned.
Signed-off-by: Peter Spreadborough <[email protected]> Reviewed-by: Kishore Padmanabha <[email protected]> Reviewed-by: Farah Smith <[email protected]> --- drivers/net/bnxt/tf_ulp/ulp_sc_mgr.c | 79 +++++++++++++++++++--------- drivers/net/bnxt/tf_ulp/ulp_sc_mgr.h | 2 + 2 files changed, 57 insertions(+), 24 deletions(-) diff --git a/drivers/net/bnxt/tf_ulp/ulp_sc_mgr.c b/drivers/net/bnxt/tf_ulp/ulp_sc_mgr.c index b246b90fe2..24012e30b1 100644 --- a/drivers/net/bnxt/tf_ulp/ulp_sc_mgr.c +++ b/drivers/net/bnxt/tf_ulp/ulp_sc_mgr.c @@ -26,6 +26,8 @@ #define ULP_TFC_CNTR_ALIGN 32 #define ULP_TFC_ACT_WORD_SZ 32 +#define ULP_SC_MAX_COUNT 0xFFFFFFFFFFFFFFFFULL + static const struct bnxt_ulp_sc_core_ops * bnxt_ulp_sc_ops_get(struct bnxt_ulp_context *ctxt) { @@ -182,7 +184,7 @@ ulp_sc_mgr_deinit(struct bnxt_ulp_context *ctxt) return 0; } -#define ULP_SC_PERIOD_US 256 +#define ULP_SC_PERIOD_US 100000 #define ULP_SC_CTX_DELAY 10000 static uint32_t ulp_stats_cache_main_loop(void *arg) @@ -265,12 +267,12 @@ static uint32_t ulp_stats_cache_main_loop(void *arg) (uint64_t)sce; rc = sc_ops->ulp_stats_cache_update(tfcp, - sce->dir, - data, - sce->handle, - &words, - &batch_info, - sce->reset); + sce->dir, + &ulp_sc_info->read_data_iova[batch], + sce->handle, + &words, + &batch_info, + false); if (unlikely(rc)) { /* Abort this batch */ PMD_DRV_LOG_LINE(ERR, @@ -278,9 +280,6 @@ static uint32_t ulp_stats_cache_main_loop(void *arg) break; } - if (sce->reset) - sce->reset = false; - /* Next */ batch++; sce++; @@ -308,9 +307,15 @@ static uint32_t ulp_stats_cache_main_loop(void *arg) PMD_DRV_LOG_LINE(ERR, "batch:%d result:%d", batch, batch_info.result[batch]); } else { - count = (struct ulp_sc_tfc_stats_cache_entry *) - ((uintptr_t)batch_info.em_hdl[batch]); - memcpy(&count->packet_count, data, ULP_TFC_ACT_WORD_SZ); + uint64_t *cptr = (uint64_t *)data; + uintptr_t em_hdl = batch_info.em_hdl[batch]; + + count = (struct ulp_sc_tfc_stats_cache_entry *)em_hdl; + if (*cptr != count->packet_count) { + count->packet_count = *cptr; + cptr++; + count->byte_count = *cptr; + } } data += ULP_SC_PAGE_SIZE; @@ -440,6 +445,8 @@ int ulp_sc_mgr_query_count_get(struct bnxt_ulp_context *ctxt, uint32_t f2_cnt; uint64_t *t; uint64_t bs; + uint64_t packet_count; + uint64_t byte_count; int rc = 0; /* Get stats cache info */ @@ -450,8 +457,22 @@ int ulp_sc_mgr_query_count_get(struct bnxt_ulp_context *ctxt, sce = ulp_sc_info->stats_cache_tbl; sce += flow_id; + /* Save the counts to local variables since they could be modified + * in the stats cache loop. + */ + packet_count = sce->packet_count; + byte_count = sce->byte_count; + /* To handle the parent flow */ if (sce->flags & ULP_SC_ENTRY_FLAG_PARENT) { + struct ulp_sc_tfc_stats_cache_entry *f1_sce = sce; + + if (!(f1_sce->flags & ULP_SC_ENTRY_FLAG_VALID)) + return -EBUSY; + + packet_count = 0; + byte_count = 0; + flow_db = bnxt_ulp_cntxt_ptr2_flow_db_get(ctxt); if (!flow_db) { BNXT_DRV_DBG(ERR, "parent child db validation failed\n"); @@ -490,26 +511,36 @@ int ulp_sc_mgr_query_count_get(struct bnxt_ulp_context *ctxt, /* no counter action, then ignore flows */ if (!(sce->flags & ULP_SC_ENTRY_FLAG_VALID)) continue; - count->hits += sce->packet_count; - count->hits_set = 1; - count->bytes += sce->byte_count; - count->bytes_set = 1; + + packet_count += sce->packet_count; + byte_count += sce->byte_count; } while (bs && f2_cnt); } + + sce = f1_sce; } else { /* To handle regular or child flows */ /* If entry is not valid return an error */ if (!(sce->flags & ULP_SC_ENTRY_FLAG_VALID)) return -EBUSY; + } - count->hits = sce->packet_count; - count->hits_set = 1; - count->bytes = sce->byte_count; - count->bytes_set = 1; - - if (count->reset) - sce->reset = true; + if (count->reset) { + /* Calculate packet count delta */ + count->hits = (packet_count - sce->last_packet_count) & ULP_SC_MAX_COUNT; + count->bytes = (byte_count - sce->last_byte_count) & ULP_SC_MAX_COUNT; + } else { + count->hits = packet_count; + count->bytes = byte_count; } + + /* Save the raw packet count */ + sce->last_packet_count = packet_count; + sce->last_byte_count = byte_count; + + count->bytes_set = 1; + count->hits_set = 1; + return rc; } diff --git a/drivers/net/bnxt/tf_ulp/ulp_sc_mgr.h b/drivers/net/bnxt/tf_ulp/ulp_sc_mgr.h index 29d0b0a1a4..631e2f77b2 100644 --- a/drivers/net/bnxt/tf_ulp/ulp_sc_mgr.h +++ b/drivers/net/bnxt/tf_ulp/ulp_sc_mgr.h @@ -30,6 +30,8 @@ struct ulp_sc_tfc_stats_cache_entry { uint64_t byte_count; uint64_t count_fields1; uint64_t count_fields2; + uint64_t last_packet_count; + uint64_t last_byte_count; bool reset; }; -- 2.39.5 (Apple Git-154)
