[ovs-dev] [xlate v2 4/6] ofproto-dpif: Move odp_actions from subfacet to facet.

Ethan Jackson ethan at nicira.com
Tue May 28 21:35:11 UTC 2013


> There's something funny going on with rule_dpif_lookup() calls here.
> handle_flow_miss() calls rule_dpif_lookup().  It only uses it in the
> handle_flow_miss_without_facet() case, and only to pass it to
> handle_flow_miss_without_facet().  In the
> handle_flow_miss_with_facet() case, it still calls rule_dpif_lookup(),
> but it throws away the result without ever looking at it at all, and
> then facet_create() calls rule_dpif_lookup() again.

Agreed, it's odd.  Fixed in v3.

> Previously, in creating a facet+subfacet for a slow-path flow due to
> one packet showing up, we would do exactly one xlate.  Now, I believe
> we do one xlate in facet_create() and then one xlate in the loop in
> handle_flow_miss_with_facet().  I don't think the difference in
> performance is significant, because xlate of slow-path flows should be
> cheap, but I thought it'd mention anyway in case you hadn't noticed
> the change.

Yep, I was aware.  Didn't think it mattered.

>
> I found the new version of facet_check_consistency() hard to follow.
> How about something more like this?

I like your version better.  Folded it in.

> I think I see an opportunity for improvement in facet_lookup_valid():
> it would be cheaper if facet_revalidate()'s return value indicated
> whether the facet had been removed.  (This isn't new.)  Something like
> this:

I've done this as a separate patch in v3.

Ethan



More information about the dev mailing list