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