Darrell, Thank you for the very thorough review. All look fine. Brief
reply to comments inline.

thanks,
Kevin.

On 08/24/2017 09:34 AM, Darrell Ball wrote:
> 
>     
>     On 8/23/17, 6:34 AM, "Kevin Traynor" <ktray...@redhat.com> wrote:
>     
>         Add counters to dp_netdev_rxq which will later be used for storing the
>         processing cycles of an rxq. Processing cycles will be stored in 
> reference
>         to a defined time interval. We will store the cycles of the current 
> in progress
>         interval, a number of completed intervals and the sum of the completed
>         intervals.
>         
>         cycles_count_intermediate was used to count cycles for a pmd. With 
> some small
>         additions we can also use it to count the cycles used for processing 
> an rxq.
>         
>         Signed-off-by: Kevin Traynor <ktray...@redhat.com>
>         ---
>          lib/dpif-netdev.c | 28 +++++++++++++++++++++++++---
>          1 file changed, 25 insertions(+), 3 deletions(-)
>         
>         diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>         index f35c079..51d4050 100644
>         --- a/lib/dpif-netdev.c
>         +++ b/lib/dpif-netdev.c
>         @@ -182,4 +182,8 @@ struct emc_cache {
>          #define DPCLS_OPTIMIZATION_INTERVAL 1000
>          
>         +/* Number of intervals for which cycles are stored
>         + * and used during rxq to pmd assignment. */
>         +#define PMD_RXQ_INTERVAL_MAX 6
>         +
>          struct dpcls {
>              struct cmap_node node;      /* Within 
> dp_netdev_pmd_thread.classifiers */
>         @@ -340,4 +344,13 @@ enum pmd_cycles_counter_type {
>          };
>          
>         +enum rxq_cycles_counter_type {
>         +    RXQ_CYCLES_PROC_CURR,       /* Cycles spent successfully polling 
> and
>         +                                   processing packets during the 
> current
>         +                                   interval. */
>         +    RXQ_CYCLES_PROC_HIST,       /* Total cycles of all intervals 
> that are used
>         +                                   during rxq to pmd assignment. */
>         +    RXQ_N_CYCLES
>         +};
>         +
>          #define XPS_TIMEOUT_MS 500LL
>          
>         @@ -351,4 +364,9 @@ struct dp_netdev_rxq {
>                                                    particular core. */
>              struct dp_netdev_pmd_thread *pmd;  /* pmd thread that polls this 
> queue. */
>         +
>         +    /* Counters of cycles spent polling and processing packets. */
> 
> Do we need to specify ‘successfully’ polling to avoid confusion ?
> 

Make sense, added

>         +    atomic_ullong cycles[RXQ_N_CYCLES];
> 
> Do you think below deserves a separate description – ‘we store the last 
> PMD_RXQ_INTERVAL_MAX  intervals and sum them up to 
> to yield the cycles used for a given rxq…’ ?
> 

Yes, I added in the comment

>         +    atomic_ullong cycles_intrvl[PMD_RXQ_INTERVAL_MAX];
>         +    unsigned intrvl_idx;               /* Write index for 
> 'cycles_intrvl'. */
>          };
>          
>         @@ -677,5 +695,4 @@ static void pmd_load_cached_ports(struct 
> dp_netdev_pmd_thread *pmd)
>          static inline void
>          dp_netdev_pmd_try_optimize(struct dp_netdev_pmd_thread *pmd);
>         -
>          static void
>          dpif_netdev_xps_revalidate_pmd(const struct dp_netdev_pmd_thread 
> *pmd,
>         @@ -3092,4 +3109,5 @@ cycles_count_end(struct dp_netdev_pmd_thread 
> *pmd,
>          static inline void
>          cycles_count_intermediate(struct dp_netdev_pmd_thread *pmd,
>         +                          struct dp_netdev_rxq *rxq,
>                                    enum pmd_cycles_counter_type type)
>              OVS_NO_THREAD_SAFETY_ANALYSIS
>         @@ -3100,4 +3118,8 @@ cycles_count_intermediate(struct 
> dp_netdev_pmd_thread *pmd,
>          
>              non_atomic_ullong_add(&pmd->cycles.n[type], interval);
>         +    if (rxq && (type == PMD_CYCLES_PROCESSING)) {
>         +        /* Add to the amount of current processing cycles. */
>         +        non_atomic_ullong_add(&rxq->cycles[RXQ_CYCLES_PROC_CURR], 
> interval);
>         +    }
>          }
>          
>         @@ -3668,5 +3690,5 @@ dpif_netdev_run(struct dpif *dpif)
>                                                             port->rxqs[i].rx,
>                                                             port->port_no);
>         -                    cycles_count_intermediate(non_pmd, 
> process_packets ?
>         +                    cycles_count_intermediate(non_pmd, NULL, 
> process_packets ?
>                                                                 
> PMD_CYCLES_PROCESSING
>                                                               : 
> PMD_CYCLES_IDLE);
>         @@ -3855,5 +3877,5 @@ reload:
>                          dp_netdev_process_rxq_port(pmd, poll_list[i].rxq->rx,
>                                                     poll_list[i].port_no);
>         -            cycles_count_intermediate(pmd,
>         +            cycles_count_intermediate(pmd, NULL,
>                                                process_packets ? 
> PMD_CYCLES_PROCESSING
>                                                                : 
> PMD_CYCLES_IDLE);
>         -- 
>         1.8.3.1
>         
>         
>     
>     
> 
> 
> 
> 
> 
> _______________________________________________
> 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

Reply via email to