Thanks, fixed the typo and adopted your suggested change,~
On 28 August 2015 at 09:18, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
>
> > On Aug 27, 2015, at 10:25 PM, Alex Wang <ee07b...@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.maxim...@samsung.com>
> > Signed-off-by: Alex Wang <ee07b...@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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
