Thanks for doing this cleanup, my instant revalidation series should be
much simpler after this.

It looks like this needs rebasing, but it is pretty trivial

I've gone through the code, I only have a couple minor of comments about
how commits are organized.

Other than that it looks good to me.

For the series:

Acked-by: Daniele Di Proietto <diproiet...@vmware.com>


2016-09-20 18:47 GMT-07:00 Joe Stringer <j...@ovn.org>:

> The function revalidate_ukey() has bothered me for some time. It's long
> enough
> with enough local variables that it's easy to forget to properly cleanup
> after
> a function call. Error conditions are interspersed with optimizations to
> shortcut revalidation so it's easy to forget how changes are supposed to
> apply
> in the optimized cases. In some cases we only revalidate to check the
> flow/mask/actions, in other cases we only submit stats/side-effects via the
> xlate_cache. Modifications to 'ukey' and its members is also scattered
> through
> the function.
>
> A recent proposal to add an additional parameter to this function prompted
> me
> to see if there's a better way to structure this function. As a result, the
> revalidation shortcut logic is all constrained to about 12 lines in one
> place,
> the stats attribution happens in just one place for all flows, and the
> translation code from the delete path is now shared with the dump path. The
> code is longer overall, but hopefully it's a bit easier to follow.
>
> Joe Stringer (6):
>   revalidator: Refactor ukey->xout translation.
>   revalidator: Don't modify ukey from xlate_ukey().
>   revalidator: Refactor revalidation early exit.
>   revalidator: Reuse xlate_ukey from deletion.
>   revalidator: Defer stats push to end of validation.
>   revalidator: Simplify full-revalidation code.
>
>  ofproto/ofproto-dpif-upcall.c | 281 +++++++++++++++++++++++++-----
> ------------
>  ofproto/ofproto-dpif-xlate.c  |  18 ---
>  ofproto/ofproto-dpif-xlate.h  |   1 -
>  3 files changed, 170 insertions(+), 130 deletions(-)
>
> --
> 2.9.3
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to