[ovs-dev] [PATCH 2/3] ofproto-dpif: Check frags only once.

[Ben Pfaff]

On Thu, Oct 02, 2014 at 09:20:32AM -0700, Jarno Rajahalme wrote:
> 
> On Sep 26, 2014, at 11:33 AM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > On Wed, Sep 24, 2014 at 11:24:01AM -0700, Jarno Rajahalme wrote:
> >> Earlier I have seen the frags checking suspiciously high in perf
> >> reports, but did not understand why.  Maybe this explains it:
> >> 
> >> Previously we checked frags handling before each flow table lookup.
> >> As the nw_frags field is not writeable, it suffices to check them
> >> once, before the first table lookup.  Also, ofproto-dpif-xlate already
> >> has code for this, but it was run after the first table lookup.  This
> >> check is now done only once, before the first table lookup.
> >> 
> >> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> > 
> > By "frags checking suspiciously high in perf reports", do you mean
> > these deleted lines of code in rule_dpif_lookup_in_table() were
> > showing up high in perf reports?
> > 
> > -    if (ofproto->up.frag_handling != OFPC_FRAG_NX_MATCH) {
> > -        /* We always unwildcard dl_type and nw_frag (for IP), so they
> > -         * need not be unwildcarded here. */
> > -
> > -        if (flow->nw_frag & FLOW_NW_FRAG_ANY) {
> > 
> 
> Yes they did. I could make it a bit better just by changing the
> order of the checks, checking flow->nw_frag first, and
> ofproto->ip.frag_handling later, so it must have been a cache miss
> on ofproto->up.frag_handling.
> This was already some months ago, and I have not repeated the tests recently.
> 
> > I'm really surprised to hear that.
> > 
> 
> To be clear, it was in the range of < 1%, so it was not that high,
> but I was surprised to see it there at all. Maybe I should just
> remove the word ?high? from the commit message?

That would help.

Was the improvement about 1% also?

The new code is at least a little harder to understand.  I am trying
to figure out whether the performance improvement outweighs that cost.
What is your opinion?