On 11.01.2018 17:19, Jan Scheurich wrote:
> 😊!
> 
> Came to the same conclusion...
> 
> I think we should report PMD_STAT_MASKED_LOOKUP as n_mask_hit. According to 
> my understanding it count exactly what is required. Where do you see the 
> dependency to n_mask (which we don't have easily as we have independent masks 
> per PMD)?
> 
> I will include a bugfix as separate commit in the next version.


It's a bug in your patch #1.
Previously 'n_hit' was calculated as sum of 2 counters.
kernel datapath treats 'n_hit' as a total number of hits and 'n_mask_hit' as a 
number
of masked hits from 'n_hit'. See datapath/datapath.c: ovs_dp_process_packet().

dpctl utility calculates average number of masked hits per packet as
'stats.n_mask_hit / (stats.n_hit + stats.n_missed)' and only if "stats.n_masks 
!= UINT32_MAX".
So, (stats.n_hit + stats.n_missed) is the total numer of packets.

See lib/dpctl.c: show_dpif() for details.


> 
> /Jan
> 
>> -----Original Message-----
>> From: Ilya Maximets [mailto:[email protected]]
>> Sent: Thursday, 11 January, 2018 15:10
>>
>> I found the reason why this test fails. It happens because 'stats->n_hit'
>> in 'dpif_netdev_get_stats' should be the sum of PMD_STAT_EXACT_HIT and
>> PMD_STAT_MASKED_HIT.
>> Additionally, no need to set 'stats->n_mask_hit' because it only useful
>> if 'stats->n_masks' set properly.
>>
>>
>> So, following incremental required to make at least unit tests work:
>> --------------------------------------------------------------------
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index c7a4a21..ed14dde 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -1054,7 +1054,7 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
>> argc, const char *argv[],
>>
>>      ovs_mutex_lock(&dp_netdev_mutex);
>>
>> -    while (argc > 0) {
>> +    while (argc > 1) {
>>          if (!strcmp(argv[1], "-pmd") && argc >= 2) {
>>              if (str_to_uint(argv[2], 10, &core_id)) {
>>                  filter_on_pmd = true;
>> @@ -1464,11 +1464,12 @@ dpif_netdev_get_stats(const struct dpif *dpif, 
>> struct dpif_dp_stats *stats)
>>          stats->n_flows += cmap_count(&pmd->flow_table);
>>          pmd_perf_read_counters(&pmd->perf_stats, pmd_stats);
>>          stats->n_hit += pmd_stats[PMD_STAT_EXACT_HIT];
>> -        stats->n_mask_hit += pmd_stats[PMD_STAT_MASKED_HIT];
>> +        stats->n_hit += pmd_stats[PMD_STAT_MASKED_HIT];
>>          stats->n_missed += pmd_stats[PMD_STAT_MISS];
>>          stats->n_lost += pmd_stats[PMD_STAT_LOST];
>>      }
>>      stats->n_masks = UINT32_MAX;
>> +    stats->n_mask_hit = UINT64_MAX;
>>
>>      return 0;
>>  }
>> diff --git a/tests/pmd.at b/tests/pmd.at
>> index 6e7ba50..80f7414 100644
>> --- a/tests/pmd.at
>> +++ b/tests/pmd.at
>> @@ -178,8 +178,8 @@ pmd thread numa_id <cleared> core_id <cleared>:
>>         emc hits:0
>>         megaflow hits:0
>>         avg. subtable lookups per megaflow hit:0.00
>> -       miss(i.e. lookup miss with success upcall):0
>> -       lost(i.e. lookup miss with failed upcall):0
>> +       miss with success upcall:0
>> +       miss with failed upcall:0
>>  ])
>>
>>  ovs-appctl time/stop
>> @@ -208,8 +208,8 @@ pmd thread numa_id <cleared> core_id <cleared>:
>>         emc hits:19
>>         megaflow hits:0
>>         avg. subtable lookups per megaflow hit:0.00
>> -       miss(i.e. lookup miss with success upcall):1
>> -       lost(i.e. lookup miss with failed upcall):0
>> +       miss with success upcall:1
>> +       miss with failed upcall:0
>>  ])
>>
>>  OVS_VSWITCHD_STOP
>> --------------------------------------------------------------------
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to