2017-01-17 11:43 GMT-08:00 Kevin Traynor <ktray...@redhat.com>:
> On 01/17/2017 05:43 PM, Ciara Loftus wrote:
>> Instead of counting all polling cycles as processing cycles, only count
>> the cycles where packets were received from the polling.
>
> This makes these stats much clearer. One minor comment below, other than
> that
>
> Acked-by: Kevin Traynor <ktray...@redhat.com>
>
>>
>> Signed-off-by: Georg Schmuecking <georg.schmueck...@ericsson.com>
>> Signed-off-by: Ciara Loftus <ciara.lof...@intel.com>
>> Co-authored-by: Ciara Loftus <ciara.lof...@intel.com>
Minor: the co-authored-by tag should be different from the main author.
This makes it easier to understand how busy a pmd thread is, a valid question
that a sysadmin might have.
The counters were originally introduced to help developers understand how cycles
are spent between drivers(netdev rx) and datapath processing(dpif).
Do you think
it's ok to lose this type of information? Perhaps it is, since a
developer can also
use a profiler, I'm not sure.
Maybe we could 'last_cycles' as it is and introduce a separate counter to get
the idle/busy ratio. I'm not 100% sure this is the best way.
What do you guys think?
Thanks,
Daniele
>> ---
>> v2:
>> - Rebase
>> ---
>> lib/dpif-netdev.c | 57
>> ++++++++++++++++++++++++++++++++++++++++++-------------
>> 1 file changed, 44 insertions(+), 13 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 3901129..3854c79 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -272,7 +272,10 @@ enum dp_stat_type {
>>
>> enum pmd_cycles_counter_type {
>> PMD_CYCLES_POLLING, /* Cycles spent polling NICs. */
>
> this is not used anymore and can be removed
>
>> - PMD_CYCLES_PROCESSING, /* Cycles spent processing packets */
>> + PMD_CYCLES_IDLE, /* Cycles spent idle or unsuccessful
>> polling */
>> + PMD_CYCLES_PROCESSING, /* Cycles spent successfully polling and
>> + * processing polled packets */
>> +
>> PMD_N_CYCLES
>> };
>>
>> @@ -747,10 +750,10 @@ pmd_info_show_stats(struct ds *reply,
>> }
>>
>> ds_put_format(reply,
>> - "\tpolling cycles:%"PRIu64" (%.02f%%)\n"
>> + "\tidle cycles:%"PRIu64" (%.02f%%)\n"
>> "\tprocessing cycles:%"PRIu64" (%.02f%%)\n",
>> - cycles[PMD_CYCLES_POLLING],
>> - cycles[PMD_CYCLES_POLLING] / (double)total_cycles * 100,
>> + cycles[PMD_CYCLES_IDLE],
>> + cycles[PMD_CYCLES_IDLE] / (double)total_cycles * 100,
>> cycles[PMD_CYCLES_PROCESSING],
>> cycles[PMD_CYCLES_PROCESSING] / (double)total_cycles *
>> 100);
>>
>> @@ -2892,30 +2895,43 @@ cycles_count_end(struct dp_netdev_pmd_thread *pmd,
>> non_atomic_ullong_add(&pmd->cycles.n[type], interval);
>> }
>>
>> -static void
>> +/* Calculate the intermediate cycle result and add to the counter 'type' */
>> +static inline void
>> +cycles_count_intermediate(struct dp_netdev_pmd_thread *pmd,
>> + enum pmd_cycles_counter_type type)
I'd add an OVS_REQUIRES(&cycles_counter_fake_mutex)
>> + OVS_NO_THREAD_SAFETY_ANALYSIS
>> +{
>> + unsigned long long new_cycles = cycles_counter();
>> + unsigned long long interval = new_cycles - pmd->last_cycles;
>> + pmd->last_cycles = new_cycles;
>> +
>> + non_atomic_ullong_add(&pmd->cycles.n[type], interval);
>> +}
>> +
>> +static int
>> dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread *pmd,
>> struct netdev_rxq *rx,
>> odp_port_t port_no)
>> {
>> struct dp_packet_batch batch;
>> int error;
>> + int batch_cnt = 0;
>>
>> dp_packet_batch_init(&batch);
>> - cycles_count_start(pmd);
>> error = netdev_rxq_recv(rx, &batch);
>> - cycles_count_end(pmd, PMD_CYCLES_POLLING);
>> if (!error) {
>> *recirc_depth_get() = 0;
>>
>> - cycles_count_start(pmd);
>> + batch_cnt = batch.count;
>> dp_netdev_input(pmd, &batch, port_no);
>> - cycles_count_end(pmd, PMD_CYCLES_PROCESSING);
>> } else if (error != EAGAIN && error != EOPNOTSUPP) {
>> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
>>
>> VLOG_ERR_RL(&rl, "error receiving data from %s: %s",
>> netdev_rxq_get_name(rx), ovs_strerror(error));
>> }
>> +
>> + return batch_cnt;
>> }
>>
>> static struct tx_port *
>> @@ -3377,21 +3393,27 @@ dpif_netdev_run(struct dpif *dpif)
>> struct dp_netdev *dp = get_dp_netdev(dpif);
>> struct dp_netdev_pmd_thread *non_pmd;
>> uint64_t new_tnl_seq;
>> + int process_packets = 0;
>>
>> ovs_mutex_lock(&dp->port_mutex);
>> non_pmd = dp_netdev_get_pmd(dp, NON_PMD_CORE_ID);
>> if (non_pmd) {
>> ovs_mutex_lock(&dp->non_pmd_mutex);
>> + cycles_count_start(non_pmd);
>> HMAP_FOR_EACH (port, node, &dp->ports) {
>> if (!netdev_is_pmd(port->netdev)) {
>> int i;
>>
>> for (i = 0; i < port->n_rxq; i++) {
>> - dp_netdev_process_rxq_port(non_pmd, port->rxqs[i].rx,
>> - port->port_no);
>> + process_packets +=
>> + dp_netdev_process_rxq_port(non_pmd,
>> + port->rxqs[i].rx,
>> + port->port_no);
>> }
>> }
>> }
>> + cycles_count_end(non_pmd, process_packets ? PMD_CYCLES_PROCESSING
>> + : PMD_CYCLES_IDLE);
>> dpif_netdev_xps_revalidate_pmd(non_pmd, time_msec(), false);
>> ovs_mutex_unlock(&dp->non_pmd_mutex);
>>
>> @@ -3516,6 +3538,7 @@ pmd_thread_main(void *f_)
>> bool exiting;
>> int poll_cnt;
>> int i;
>> + int process_packets = 0;
>>
>> poll_list = NULL;
>>
>> @@ -3542,10 +3565,12 @@ reload:
>> lc = UINT_MAX;
>> }
>>
>> + cycles_count_start(pmd);
>> for (;;) {
>> for (i = 0; i < poll_cnt; i++) {
>> - dp_netdev_process_rxq_port(pmd, poll_list[i].rx,
>> - poll_list[i].port_no);
>> + process_packets +=
>> + dp_netdev_process_rxq_port(pmd, poll_list[i].rx,
>> + poll_list[i].port_no);
>> }
>>
>> if (lc++ > 1024) {
>> @@ -3564,8 +3589,14 @@ reload:
>> break;
>> }
>> }
>> + cycles_count_intermediate(pmd, process_packets ?
>> PMD_CYCLES_PROCESSING
>> + : PMD_CYCLES_IDLE);
>> + process_packets = 0;
>> }
>>
>> + cycles_count_end(pmd, process_packets ? PMD_CYCLES_PROCESSING
>> + : PMD_CYCLES_IDLE);
>> +
>> poll_cnt = pmd_load_queues_and_ports(pmd, &poll_list);
>> exiting = latch_is_set(&pmd->exit_latch);
>> /* Signal here to make sure the pmd finishes
>>
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
