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

ALeX Wang ee07b291 at gmail.com
Sat Aug 29 00:52:48 UTC 2015


Thanks, fixed the typo and adopted your suggested change,~

On 28 August 2015 at 09:18, Jarno Rajahalme <jrajahalme at nicira.com> wrote:

>
> > 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
>
>


-- 
Alex Wang,
Open vSwitch developer



More information about the dev mailing list