[ovs-dev] [PATCH V2 2/2] dpif-netdev: Purge all ukeys when reconfigure pmd.

Jarno Rajahalme jrajahalme at nicira.com
Fri Aug 28 16:18:32 UTC 2015


> On Aug 27, 2015, at 10:25 PM, Alex Wang <ee07b291 at gmail.com> wrote:
> 
> When dpdk configuration changes, all pmd threads are recreated
> and rx queues of each port are reloaded.  After this process,
> rx queue could be mapped to a different pmd thread other than
> the one before reconfiguration.  However, this is totally
> transparent to ofproto layer modules.  So, if the ofproto-dpif-upcall
> module still holds ukeys generated before pmd thread recreation,
> this old ukey will collide with the ukey for the new upcalls
> from same traffic flow, causing flow installation failure.
> 
> To fix the bug, this commit adds a new call-back function
> in dpif layer for notifying upper layer the purging of datapath
> (e.g. pmd thread deletion in dpif-netdev).  So, the
> ofproto-dpif-upcall module can react properly with deleting
> the ukeys and with collecting flows' last stats.
> 
> Reported-by: Ilya Maximets <i.maximets at samsung.com>
> Signed-off-by: Alex Wang <ee07b291 at gmail.com>
> 
> ---
> PATCH->V2:
> - PATCH does not fix the problem,
> - call the purge_cb between pmd thread stopping and deletion.
>  (which should be the right place)
> ---
> lib/dpif-netdev.c             | 23 ++++++++++++++++++++---
> lib/dpif-netlink.c            |  1 +
> lib/dpif-provider.h           | 11 ++++++++++-
> lib/dpif.c                    |  8 ++++++++
> lib/dpif.h                    | 13 +++++++++++++
> ofproto/ofproto-dpif-upcall.c | 36 ++++++++++++++++++++++++++++++++++++
> 6 files changed, 88 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index f841876..2d187da 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -204,6 +204,11 @@ struct dp_netdev {
>     upcall_callback *upcall_cb;  /* Callback function for executing upcalls. */
>     void *upcall_aux;
> 
> +    /* Callback function for notifying the purging of dp flows (during
> +     * reseting pmd deletion). */
> +    dp_purge_callback *dp_purge_cb;
> +    void *dp_purge_aux;
> +
>     /* Stores all 'struct dp_netdev_pmd_thread's. */
>     struct cmap poll_threads;
> 
> @@ -2851,7 +2856,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread *pmd)
> /* Stops the pmd thread, removes it from the 'dp->poll_threads',
>  * and unrefs the struct. */
> static void
> -dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
> +dp_netdev_del_pmd(struct dp_netdev *dp, struct dp_netdev_pmd_thread *pmd)
> {
>     /* Uninit the 'flow_cache' since there is
>      * no actual thread uninit it for NON_PMD_CORE_ID. */
> @@ -2863,6 +2868,8 @@ dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
>         ovs_numa_unpin_core(pmd->core_id);
>         xpthread_join(pmd->thread, NULL);
>     }
> +    /* Purges the 'pmd''s flows after stoping the thread. */

“stoping” -> “stopping”

I would also like the comment elaborated like this: “, but before destroying the flows, so that the flow stats can be collected."

> +    dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);
>     cmap_remove(&pmd->dp->poll_threads, &pmd->node, hash_int(pmd->core_id, 0));
>     dp_netdev_pmd_unref(pmd);
> }


  Jarno




More information about the dev mailing list