2017-02-21 6:49 GMT-08:00 Ilya Maximets <i.maxim...@samsung.com>:
> There are many reasons for reloading of pmd threads:
>         * reconfiguration of one of the ports.
>         * Adjusting of static_tx_qid.
>         * Adding new tx/rx ports.
>
> In many cases EMC is still useful after reload and uninit
> will only lead to unnecessary upcalls/classifier lookups.
>
> Such behaviour slows down the datapath. Uninit itself slows
> down the reload path. All this factors leads to additional
> unexpected latencies/drops on events not directly connected
> to current PMD thread.
>
> Lets not uninitialize emc cache on reload path.
> 'emc_cache_slow_sweep()' and replacements should free all
> the old/unwanted entries.
>
> Signed-off-by: Ilya Maximets <i.maxim...@samsung.com>
> ---
>
> In discussion of original patch[1] Pravin Shelar said:
> '''
>   emc cache flows should be free on reload, otherwise flows
>   might stick around for longer time.
> '''
>
> After that (for another reason) slow sweep was introduced.
> There are few reasons to move uninit out of reload path
> described in commit-message.
>
> So, this patch sent as RFC, because I wanted to hear some
> opinions about current situation and drawbacks of such
> solution.
>
> Any thoughts?
> Thanks.
>
> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2014-August/287852.html
>

I'm in favor of this approach. Now that we have slow sweep we don't need to
clear it at every reload.

Just curious, did you measure any difference in reload time with this patch?

 We could init and uninit the EMC from the main thread in
dp_netdev_configure_pmd() and dp_netdev_del_pmd().  This way we wouldn't
need a special case for the non-pmd thread in the code, but it would be slower.
In any case it seems fine with me

Thanks,

Daniele

>  lib/dpif-netdev.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index e2b4f39..3bd568d 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3648,9 +3648,9 @@ pmd_thread_main(void *f_)
>      ovs_numa_thread_setaffinity_core(pmd->core_id);
>      dpdk_set_lcore_id(pmd->core_id);
>      poll_cnt = pmd_load_queues_and_ports(pmd, &poll_list);
> +    emc_cache_init(&pmd->flow_cache);
>  reload:
>      pmd_alloc_static_tx_qid(pmd);
> -    emc_cache_init(&pmd->flow_cache);
>
>      /* List port/core affinity */
>      for (i = 0; i < poll_cnt; i++) {
> @@ -3697,13 +3697,13 @@ reload:
>       * reloading the updated configuration. */
>      dp_netdev_pmd_reload_done(pmd);
>
> -    emc_cache_uninit(&pmd->flow_cache);
>      pmd_free_static_tx_qid(pmd);
>
>      if (!exiting) {
>          goto reload;
>      }
>
> +    emc_cache_uninit(&pmd->flow_cache);
>      free(poll_list);
>      pmd_free_cached_ports(pmd);
>      return NULL;
> --
> 2.7.4
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to