[ovs-dev] [RFC PATCH 1/6] dpif-netdev: Add rxq processing cycle counters.
Stokes, Ian
ian.stokes at intel.com
Fri Jun 2 17:40:11 UTC 2017
> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
> bounces at openvswitch.org] On Behalf Of Kevin Traynor
> Sent: Friday, May 5, 2017 5:34 PM
> To: dev at openvswitch.org
> Subject: [ovs-dev] [RFC PATCH 1/6] dpif-netdev: Add rxq processing cycle
> counters.
>
> Add two counters to dp_netdev_rxq which will be used for storing the
> processing cycles of the rxq. Processing cycles will stored in reference
> to a defined interval. One counter is used to count cycles during the
> current in progress interval, while the other is used to store the cycles
> of the last complete interval.
>
> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
> ---
> lib/dpif-netdev.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 7352d6f..d2a02af
> 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -343,4 +343,6 @@ struct dp_netdev_rxq {
> particular core. */
> struct dp_netdev_pmd_thread *pmd; /* pmd thread that will poll this
> queue. */
> + atomic_ullong cyc_curr; /* Current pmd interval proc
> cycles. */
> + atomic_ullong cyc_last; /* Last pmd interval proc cycles.
> */
> };
>
> @@ -665,4 +667,14 @@ 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
> +dp_netdev_rxq_set_cyc_curr(struct dp_netdev_rxq *rx,
> + unsigned long long cycles); static uint64_t
> +dp_netdev_rxq_get_cyc_curr(const struct dp_netdev_rxq *rx); static void
> +dp_netdev_rxq_set_cyc_last(struct dp_netdev_rxq *rx,
> + unsigned long long cycles); static
> +uint64_t dp_netdev_rxq_get_cyc_last(const struct dp_netdev_rxq *rx);
>
> static void
> @@ -3046,4 +3058,35 @@ cycles_count_intermediate(struct
> dp_netdev_pmd_thread *pmd, }
>
> +static void
> +dp_netdev_rxq_set_cyc_curr(struct dp_netdev_rxq *rx,
> + unsigned long long cycles) {
> + atomic_store_relaxed(&rx->cyc_curr, cycles); }
> +
> +static uint64_t
> +dp_netdev_rxq_get_cyc_curr(const struct dp_netdev_rxq *rx) {
> + unsigned long long tmp;
> + atomic_read_relaxed(&rx->cyc_curr, &tmp);
> + return tmp;
> +}
> +
> +static void
> +dp_netdev_rxq_set_cyc_last(struct dp_netdev_rxq *rx,
> + unsigned long long cycles) {
> + atomic_store_relaxed(&rx->cyc_last, cycles); }
> +
> +static uint64_t
> +dp_netdev_rxq_get_cyc_last(const struct dp_netdev_rxq *rx) {
> + unsigned long long tmp;
> + atomic_read_relaxed(&rx->cyc_last, &tmp);
> + return tmp;
> +}
> +
Some code duplication above for the get/set methods above.
I was thinking could you have 1 function for dp_netdev_rxq_set_cyc and 1 for dp_netdev_rxq_get_cyc and use an enum to signal if its 'cyc_last' or 'cyc_curr'. But a concern with this approach is that the logical decision on the enum type will impact negatively on performance.
I know this is an RFC so it probably doesn't matter but as a heads up you'll get compilation warnings of the functions not being used as is after applying this patch. Maybe something to consider when rolling out the next patchset.
Also clang complains of the following:
lib/dpif-netdev.c:3071:5: error: address argument to atomic operation must be a pointer to non-const _Atomic type ('const atomic_ullong *' (aka 'const _Atomic(unsigned long long) *') invalid)
atomic_read_relaxed(&rx->cyc_curr, &tmp);
^ ~~~~~~~~~~~~~
./lib/ovs-atomic.h:395:5: note: expanded from macro 'atomic_read_relaxed'
atomic_read_explicit(VAR, DST, memory_order_relaxed)
^ ~~~
./lib/ovs-atomic-clang.h:53:15: note: expanded from macro 'atomic_read_explicit'
(*(DST) = __c11_atomic_load(SRC, ORDER), \
^ ~~~
lib/dpif-netdev.c:3086:5: error: address argument to atomic operation must be a pointer to non-const _Atomic type ('const atomic_ullong *' (aka 'const _Atomic(unsigned long long) *') invalid)
atomic_read_relaxed(&rx->cyc_last, &tmp);
^ ~~~~~~~~~~~~~
./lib/ovs-atomic.h:395:5: note: expanded from macro 'atomic_read_relaxed'
atomic_read_explicit(VAR, DST, memory_order_relaxed)
^ ~~~
./lib/ovs-atomic-clang.h:53:15: note: expanded from macro 'atomic_read_explicit'
(*(DST) = __c11_atomic_load(SRC, ORDER), \
Sparse throws errors for the same issue
lib/dpif-netdev.c:3075:5: warning: incorrect type in argument 1 (different modifiers)
lib/dpif-netdev.c:3075:5: expected void *<noident>
lib/dpif-netdev.c:3075:5: got unsigned long long const *<noident>
lib/dpif-netdev.c:3075:5: warning: incorrect type in argument 1 (different modifiers)
lib/dpif-netdev.c:3075:5: expected void *<noident>
lib/dpif-netdev.c:3075:5: got unsigned long long const *<noident>
lib/dpif-netdev.c:3090:5: warning: incorrect type in argument 1 (different modifiers)
lib/dpif-netdev.c:3090:5: expected void *<noident>
lib/dpif-netdev.c:3090:5: got unsigned long long const *<noident>
lib/dpif-netdev.c:3090:5: warning: incorrect type in argument 1 (different modifiers)
lib/dpif-netdev.c:3090:5: expected void *<noident>
lib/dpif-netdev.c:3090:5: got unsigned long long const *<noident>
> +
> static int
> dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread *pmd,
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list