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


dev mailing list

Reply via email to