That sounds fine by me.
On 25 February 2014 22:22, Ben Pfaff <b...@nicira.com> wrote:
> Joe, based on Ethan's feedback I'll drop this for now unless you prefer
> otherwise.
>
> On Tue, Feb 25, 2014 at 05:37:05PM -0800, Ethan Jackson wrote:
> > I'm not sure this patch makes sense in the current code. If a
> > revalidation failed, that's probably because the rules used to create
> > the flow changed, in which case we'd be accounting packets to the
> > wrong rules. This is arguable worse than missing the packets all
> > together.
> >
> > Once we change the code to push stats by maintaining a list of rules
> > we need to push, instead of doing a full xlate_actions, I think this
> > makes a lot more sense. In that case, we know we aren't accounting
> > anything to the wrong rules.
> >
> > Ethan
> >
> >
> > On Tue, Feb 25, 2014 at 5:12 PM, Joe Stringer <joestrin...@nicira.com>
> wrote:
> > > A flow's statistics could be partially lost in the following situation:
> > > * A flow is dumped from the datapath
> > > * Some traffic hits that flow
> > > * Revalidation fails for that flow
> > >
> > > Previously, we would delete such a flow without fetching the latest
> > > statistics for it, causing the intermediate statistics to be lost. This
> > > patch fixes the bug by queueing the flow up for deletion and cleanup
> > > along with the other flows that will be deleted.
> > >
> > > Signed-off-by: Joe Stringer <joestrin...@nicira.com>
> > > ---
> > > ofproto/ofproto-dpif-upcall.c | 6 ++++--
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/ofproto/ofproto-dpif-upcall.c
> b/ofproto/ofproto-dpif-upcall.c
> > > index e4f81a1..31d7b9f 100644
> > > --- a/ofproto/ofproto-dpif-upcall.c
> > > +++ b/ofproto/ofproto-dpif-upcall.c
> > > @@ -1534,8 +1534,10 @@ revalidate_udumps(struct revalidator
> *revalidator, struct list *udumps)
> > > ukey->mark = true;
> > >
> > > if (!revalidate_ukey(udpif, udump, ukey)) {
> > > - dpif_flow_del(udpif->dpif, udump->key, udump->key_len,
> NULL);
> > > - ukey_delete(revalidator, ukey);
> > > + struct dump_op *dop = &ops[n_ops++];
> > > +
> > > + dump_op_init(dop, udump->key, udump->key_len, ukey,
> udump);
> > > + continue;
> > > }
> > >
> > > list_remove(&udump->list_node);
> > > --
> > > 1.7.9.5
> > >
> > > _______________________________________________
> > > dev mailing list
> > > dev@openvswitch.org
> > > http://openvswitch.org/mailman/listinfo/dev
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
